Skip to content

Commit

Permalink
Fixes for Bundle Routing (#3822)
Browse files Browse the repository at this point in the history 
* Evaluate Routing constraints when processing bundles
* Adding tests to validate bundle internal calls to _history and _history/version.

---------

Co-authored-by: Fernando Henrique Inocêncio Borba Ferreira <[email protected]>
  • Loading branch information
@brendankowitz @fhibf
brendankowitz and fhibf authored May 13, 2024
1 parent 5a98e34 commit 441fd71
Show file tree
Hide file tree
Showing 2 changed files with 125 additions and 82 deletions.
147 changes: 65 additions & 82 deletions src/Microsoft.Health.Fhir.Shared.Api/Features/Resources/Bundle/BundleRouter.cs
View file
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 @@ public VirtualPathData GetVirtualPath(VirtualPathContext context)
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));
}

return (null, null);
}
}
}
60 changes: 60 additions & 0 deletions test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/BundleEdgeCaseTests.cs
View file
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

0 comments on commit 441fd71

Please sign in to comment.