diff --git a/src/Microsoft.Health.Fhir.Shared.Api/Features/Resources/Bundle/BundleRouter.cs b/src/Microsoft.Health.Fhir.Shared.Api/Features/Resources/Bundle/BundleRouter.cs index 86c3fd55bd..47d14dd45a 100644 --- a/src/Microsoft.Health.Fhir.Shared.Api/Features/Resources/Bundle/BundleRouter.cs +++ b/src/Microsoft.Health.Fhir.Shared.Api/Features/Resources/Bundle/BundleRouter.cs @@ -5,16 +5,21 @@ 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; @@ -22,19 +27,35 @@ 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.*/ + /// + /// 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. + /// internal class BundleRouter : IRouter { + private readonly TemplateBinderFactory _templateBinderFactory; + private readonly IEnumerable _matcherPolicies; private readonly EndpointDataSource _endpointDataSource; + private readonly EndpointSelector _endpointSelector; private readonly ILogger _logger; - public BundleRouter(EndpointDataSource endpointDataSource, ILogger logger) + public BundleRouter( + TemplateBinderFactory templateBinderFactory, + IEnumerable matcherPolicies, + EndpointDataSource endpointDataSource, + EndpointSelector endpointSelector, + ILogger 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; } @@ -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(); + IEnumerable endpoints = _endpointDataSource.Endpoints.OfType(); + PathString path = context.HttpContext.Request.Path; - private void SetRouteContext(RouteContext context) - { - try + foreach (RouteEndpoint endpoint in endpoints) { - var routeCandidates = new List>(); - var endpoints = _endpointDataSource.Endpoints.OfType(); - 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(); - if (httpMethodAttribute != null && !httpMethodAttribute.HttpMethods.Any(x => x.Equals(method, StringComparison.OrdinalIgnoreCase))) - { - continue; - } - - routeCandidates.Add(new KeyValuePair(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> 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().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()) { - 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() != 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(); - 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); } } } diff --git a/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/BundleEdgeCaseTests.cs b/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/BundleEdgeCaseTests.cs index 7918fb8d3b..e6eae1e3e2 100644 --- a/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/BundleEdgeCaseTests.cs +++ b/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/BundleEdgeCaseTests.cs @@ -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; @@ -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 { new HumanName() { Family = "Rush", Given = new List { $"John" } } }, + Gender = AdministrativeGender.Male, + BirthDate = "1974-12-21", + Text = new Narrative($"
{DateTime.UtcNow.ToString("o")}
"), + }; + 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().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 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()