Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fixes for Bundle Routing #3822

Merged
merged 7 commits into from
May 13, 2024
Merged

Fixes for Bundle Routing #3822

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
475fe31
Evaluate Routing constraints when processing bundles
brendankowitz Apr 22, 2024
437f8b5
Adding tests to validate bundle internal calls to _history and _histo…
fhibf Apr 22, 2024
5fb6510
Improve fix
brendankowitz Apr 23, 2024
ab3bc13
Applies to isn't needed
brendankowitz Apr 23, 2024
8aed255
Removes unused variable
brendankowitz Apr 23, 2024
63c64ce
Some matches are not RouteEndpoints (i.e. 404s)
brendankowitz Apr 26, 2024
b011e75
fix variable name
brendankowitz May 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
147 changes: 65 additions & 82 deletions src/Microsoft.Health.Fhir.Shared.Api/Features/Resources/Bundle/BundleRouter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,36 +5,57 @@

using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Linq;
using System.Net.Http;
using System.Security.Policy;
using System.Threading.Tasks;
using System.Web;
using AngleSharp.Io;
using EnsureThat;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Mvc;
using Microsoft.AspNetCore.Mvc.Infrastructure;
using Microsoft.AspNetCore.Mvc.Routing;
using Microsoft.AspNetCore.Routing;
using Microsoft.AspNetCore.Routing.Matching;
using Microsoft.AspNetCore.Routing.Patterns;
using Microsoft.AspNetCore.Routing.Template;
using Microsoft.Extensions.Logging;
using Microsoft.Health.Fhir.Api.Features.ActionConstraints;
using Microsoft.Health.Fhir.Core.Features;

namespace Microsoft.Health.Fhir.Api.Features.Resources.Bundle
{
/* BundleRouter creates the routingContext for bundles with enabled endpoint routing.It fetches all RouteEndpoints using EndpointDataSource(based on controller actions)
and find the best endpoint match based on the request httpContext to build the routeContext for bundle request to route to appropriate action.*/
/// <summary>
/// BundleRouter creates the routingContext for bundles with enabled endpoint routing.It fetches all RouteEndpoints using EndpointDataSource(based on controller actions)
/// and find the best endpoint match based on the request httpContext to build the routeContext for bundle request to route to appropriate action.
/// </summary>
internal class BundleRouter : IRouter
{
private readonly TemplateBinderFactory _templateBinderFactory;
private readonly IEnumerable<MatcherPolicy> _matcherPolicies;
private readonly EndpointDataSource _endpointDataSource;
private readonly EndpointSelector _endpointSelector;
private readonly ILogger<BundleRouter> _logger;

public BundleRouter(EndpointDataSource endpointDataSource, ILogger<BundleRouter> logger)
public BundleRouter(
TemplateBinderFactory templateBinderFactory,
IEnumerable<MatcherPolicy> matcherPolicies,
EndpointDataSource endpointDataSource,
EndpointSelector endpointSelector,
ILogger<BundleRouter> logger)
{
EnsureArg.IsNotNull(templateBinderFactory, nameof(templateBinderFactory));
EnsureArg.IsNotNull(matcherPolicies, nameof(matcherPolicies));
EnsureArg.IsNotNull(endpointDataSource, nameof(endpointDataSource));
EnsureArg.IsNotNull(endpointSelector, nameof(endpointSelector));
EnsureArg.IsNotNull(logger, nameof(logger));

_templateBinderFactory = templateBinderFactory;
_matcherPolicies = matcherPolicies;
_endpointDataSource = endpointDataSource;
_endpointSelector = endpointSelector;
_logger = logger;
}

Expand All @@ -43,108 +64,70 @@
throw new System.NotImplementedException();
}

public Task RouteAsync(RouteContext context)
public async Task RouteAsync(RouteContext context)
{
EnsureArg.IsNotNull(context, nameof(context));

SetRouteContext(context);
return Task.CompletedTask;
}
var routeCandidates = new Dictionary<RouteEndpoint, RouteValueDictionary>();
IEnumerable<RouteEndpoint> endpoints = _endpointDataSource.Endpoints.OfType<RouteEndpoint>();
PathString path = context.HttpContext.Request.Path;

private void SetRouteContext(RouteContext context)
{
try
foreach (RouteEndpoint endpoint in endpoints)
{
var routeCandidates = new List<KeyValuePair<RouteEndpoint, RouteValueDictionary>>();
var endpoints = _endpointDataSource.Endpoints.OfType<RouteEndpoint>();
var path = context.HttpContext.Request.Path;
var method = context.HttpContext.Request.Method;
var routeValues = new RouteValueDictionary();
var routeDefaults = new RouteValueDictionary(endpoint.RoutePattern.Defaults);

foreach (var endpoint in endpoints)
{
var routeValues = new RouteValueDictionary();
var templateMatcher = new TemplateMatcher(TemplateParser.Parse(endpoint.RoutePattern.RawText), new RouteValueDictionary());
if (!templateMatcher.TryMatch(path, routeValues))
{
continue;
}

var httpMethodAttribute = endpoint.Metadata.GetMetadata<HttpMethodAttribute>();
if (httpMethodAttribute != null && !httpMethodAttribute.HttpMethods.Any(x => x.Equals(method, StringComparison.OrdinalIgnoreCase)))
{
continue;
}

routeCandidates.Add(new KeyValuePair<RouteEndpoint, RouteValueDictionary>(endpoint, routeValues));
}
RoutePattern pattern = endpoint.RoutePattern;
TemplateBinder templateBinder = _templateBinderFactory.Create(pattern);

(RouteEndpoint routeEndpointMatch, RouteValueDictionary routeValuesMatch) = FindRouteEndpoint(context.HttpContext, routeCandidates);
if (routeEndpointMatch != null && routeValuesMatch != null)
var templateMatcher = new TemplateMatcher(new RouteTemplate(pattern), routeDefaults);

// Pattern match
if (!templateMatcher.TryMatch(path, routeValues))
{
if (routeEndpointMatch.RoutePattern?.RequiredValues != null)
{
foreach (var requiredValue in routeEndpointMatch.RoutePattern.RequiredValues)
{
routeValuesMatch.Add(requiredValue.Key, requiredValue.Value);
}
}

context.Handler = routeEndpointMatch.RequestDelegate;
context.RouteData = new RouteData(routeValuesMatch);
context.HttpContext.Request.RouteValues = routeValuesMatch;
continue;
}

if (context.Handler == null)
// Eliminate routes that don't match constraints
if (!templateBinder.TryProcessConstraints(context.HttpContext, routeValues, out var parameterName, out IRouteConstraint constraint))
{
_logger.LogError("Matching route endpoint not found for the given request.");
_logger.LogDebug("Constraint '{ConstraintType}' not met for parameter '{ParameterName}'", constraint, parameterName);
continue;
}
}
catch
{
throw;
}
}

private static (RouteEndpoint routeEndpoint, RouteValueDictionary routeValues) FindRouteEndpoint(
HttpContext context,
IList<KeyValuePair<RouteEndpoint, RouteValueDictionary>> routeCandidates)
{
if (routeCandidates.Count == 0)
{
return (null, null);
routeCandidates.Add(endpoint, routeValues);
}

if (routeCandidates.Count == 1)
var candidateSet = new CandidateSet(
routeCandidates.Select(x => x.Key).Cast<Endpoint>().ToArray(),
routeCandidates.Select(x => x.Value).ToArray(),
Enumerable.Repeat(1, routeCandidates.Count).ToArray());

// Policies apply filters / matches on attributes such as Consumes, HttpVerbs etc...
foreach (IEndpointSelectorPolicy policy in _matcherPolicies
.OrderBy(x => x.Order)
.OfType<IEndpointSelectorPolicy>())
{
return (routeCandidates[0].Key, routeCandidates[0].Value);
await policy.ApplyAsync(context.HttpContext, candidateSet);
}

// Note: When there are more than one route endpoint candidates, we need to find the best match
// by looking at the request method, path, and headers. The logic of finding the best match
// as of now is based on the implementation of FhirController actions and attributes.
// TODO: Find a more generic way of implementing the logic.
await _endpointSelector.SelectAsync(context.HttpContext, candidateSet);

var method = context.Request.Method;
if (method.Equals(HttpMethods.Post, StringComparison.OrdinalIgnoreCase))
Endpoint selectedEndpoint = context.HttpContext.GetEndpoint();

// A RouteEndpoint should map to an MVC controller.
// When this isn't a RouteEndpoint it can be a 404 or a middleware endpoint mapping.
if (selectedEndpoint is RouteEndpoint)
{
var conditional = context.Request.Headers.ContainsKey(KnownHeaders.IfNoneExist);
var pair = routeCandidates.SingleOrDefault(r => (r.Key.Metadata.GetMetadata<ConditionalConstraintAttribute>() != null) == conditional);
return (pair.Key, pair.Value);
RouteData data = context.HttpContext.GetRouteData();
context.Handler = selectedEndpoint.RequestDelegate;
context.RouteData = new RouteData(data);
context.HttpContext.Request.RouteValues = context.RouteData.Values;
}
else if (method.Equals(HttpMethods.Patch, StringComparison.OrdinalIgnoreCase))
else
{
var contentType = context.Request.Headers.ContentType;
foreach (var candidate in routeCandidates)
{
var consumes = candidate.Key.Metadata.OfType<ConsumesAttribute>();
if (consumes.Any(c => c.ContentTypes.Any(t => t.Equals(contentType, StringComparison.OrdinalIgnoreCase))))
{
return (candidate.Key, candidate.Value);
}
}
_logger.LogDebug("No RouteEndpoint found for '{Path}'", HttpUtility.UrlEncode(path));
Dismissed Show dismissed Hide dismissed
}

return (null, null);
}
}
}
60 changes: 60 additions & 0 deletions test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/BundleEdgeCaseTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using System;
using System.Collections.Generic;
using System.Linq;
using System.Net;
using System.Threading;
using System.Threading.Tasks;
using Hl7.Fhir.Model;
Expand Down Expand Up @@ -96,6 +97,65 @@ public async Task GivenABundleWithConditionalUpdateByReference_WhenExecutedWithM
}
}

[Fact]
public async Task WhenProcessingABundle_IfItContainsHistoryEndpointRequests_ThenReturnTheResourcesAsExpected()
{
CancellationToken cancellationToken = CancellationToken.None;

// 1 - Post first patient who is created as the base resources to handle all following operations.
Patient patient = new Patient()
{
Name = new List<HumanName> { new HumanName() { Family = "Rush", Given = new List<string> { $"John" } } },
Gender = AdministrativeGender.Male,
BirthDate = "1974-12-21",
Text = new Narrative($"<div>{DateTime.UtcNow.ToString("o")}</div>"),
};
var firstPatientResponse = await _client.PostAsync("Patient", patient.ToJson(), cancellationToken);
Assert.True(firstPatientResponse.Response.IsSuccessStatusCode, "First patient ingestion did not complete as expected.");
string patientId = firstPatientResponse.Resource.ToResourceElement().ToPoco<Patient>().Id;

Bundle bundle = new Bundle() { Type = BundleType.Batch };

// 2 - Create a query on top of _history endpoint.
EntryComponent entryComponent = new EntryComponent()
{
Resource = null,
Request = new RequestComponent()
{
Method = HTTPVerb.GET,
Url = $"Patient/{patientId}/_history",
},
};
bundle.Entry.Add(entryComponent);

// 3 - Create a query on top of _history/version endpoint.
entryComponent = new EntryComponent()
{
Resource = null,
Request = new RequestComponent()
{
Method = HTTPVerb.GET,
Url = $"Patient/{patientId}/_history/1",
},
};
bundle.Entry.Add(entryComponent);

FhirResponse<Bundle> bundleResponse = await _client.PostBundleAsync(bundle, new Client.FhirBundleOptions(), cancellationToken);
Assert.True(bundleResponse.StatusCode == HttpStatusCode.OK, "Bundle ingestion did not complete as expected.");

// 4 - Validate the response of _history endpoint.
EntryComponent firstEntry = bundleResponse.Resource.Entry.First();
Assert.True(firstEntry.Response.Status == "200", $"The HTTP status code for the _history query is '{firstEntry.Response.Status}'.");
Assert.True(firstEntry.Resource is Bundle, "The resource returned by the _history query is not a Bundle.");
Assert.True(((Bundle)firstEntry.Resource).Entry.First().Resource.Id == patientId, "The resource returned by the _history query is not the original Patient.");

// 5 - Validate the response of _history/version endpoint.
EntryComponent secondEntry = bundleResponse.Resource.Entry.Last();
Assert.True(secondEntry.Response.Status == "200", $"The HTTP status code for the _history/version query is '{secondEntry.Response.Status}'.");
Assert.True(secondEntry.Resource is Patient, "The resource returned by the _history/version query is not a Patient.");
Assert.True(secondEntry.Resource.Id == patientId, "The resource returned by the _history/version query is not the original Patient.");
}

[Fact]
[Trait(Traits.Priority, Priority.One)]
public async Task WhenProcessingMultipleBundlesWithTheSameResource_AndIncreasingTheExpectedVersionInParallel_ThenUpdateTheResourcesAsExpected()
Expand Down
Loading