From 475fe3120c36cdd71f25d4bada085efc2a115cea Mon Sep 17 00:00:00 2001 From: Brendan Kowitz Date: Mon, 22 Apr 2024 16:02:04 -0700 Subject: [PATCH 1/7] Evaluate Routing constraints when processing bundles --- .../Features/Resources/Bundle/BundleRouter.cs | 69 +++++++++++++++++-- 1 file changed, 62 insertions(+), 7 deletions(-) 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..d237c4a355 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 @@ -13,8 +13,11 @@ 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; @@ -26,15 +29,25 @@ namespace Microsoft.Health.Fhir.Api.Features.Resources.Bundle 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 _parameterPolicies; + 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 parameterPolicies, + IEnumerable matcherPolicies, + EndpointDataSource endpointDataSource, + EndpointSelector endpointSelector, + ILogger logger) { EnsureArg.IsNotNull(endpointDataSource, nameof(endpointDataSource)); EnsureArg.IsNotNull(logger, nameof(logger)); - + _parameterPolicies = parameterPolicies; + _matcherPolicies = matcherPolicies; _endpointDataSource = endpointDataSource; + _endpointSelector = endpointSelector; _logger = logger; } @@ -60,25 +73,64 @@ private void SetRouteContext(RouteContext context) var path = context.HttpContext.Request.Path; var method = context.HttpContext.Request.Method; - foreach (var endpoint in endpoints) + foreach (RouteEndpoint endpoint in endpoints) { var routeValues = new RouteValueDictionary(); - var templateMatcher = new TemplateMatcher(TemplateParser.Parse(endpoint.RoutePattern.RawText), new RouteValueDictionary()); + var routeDefaults = new RouteValueDictionary(endpoint.RoutePattern.Defaults); + var templateMatcher = new TemplateMatcher(TemplateParser.Parse(endpoint.RoutePattern.RawText), routeDefaults); + if (!templateMatcher.TryMatch(path, routeValues)) { continue; } - var httpMethodAttribute = endpoint.Metadata.GetMetadata(); + // Older MVC constraint processing... + RoutePattern pattern = RoutePatternFactory.Parse(endpoint.RoutePattern.RawText, routeDefaults, null); + TemplateBinder templateBinder = _parameterPolicies.Create(pattern); + + if (!templateBinder.TryProcessConstraints(context.HttpContext, routeValues, out var parameterName, out var constraint)) + { + continue; + } + + /* + HttpMethodAttribute httpMethodAttribute = endpoint.Metadata.GetMetadata(); if (httpMethodAttribute != null && !httpMethodAttribute.HttpMethods.Any(x => x.Equals(method, StringComparison.OrdinalIgnoreCase))) { continue; } + */ routeCandidates.Add(new KeyValuePair(endpoint, routeValues)); } - (RouteEndpoint routeEndpointMatch, RouteValueDictionary routeValuesMatch) = FindRouteEndpoint(context.HttpContext, routeCandidates); + var candidateSet = new CandidateSet( + routeCandidates.Select(x => x.Key).Cast().ToArray(), + routeCandidates.Select(x => x.Value).ToArray(), + Enumerable.Repeat(1, routeCandidates.Count).ToArray()); + + RouteEndpoint[] potentialRoutes = routeCandidates.Select(x => x.Key).ToArray(); + + _matcherPolicies + .OrderBy(x => x.Order) + .OfType() + .Where(x => x.AppliesToEndpoints(potentialRoutes)) + .ToList() + .ForEach(x => x.ApplyAsync(context.HttpContext, candidateSet).GetAwaiter().GetResult()); + + _endpointSelector.SelectAsync( + context.HttpContext, + candidateSet) + .GetAwaiter() + .GetResult(); + + context.Handler = context.HttpContext.GetEndpoint()?.RequestDelegate; + context.RouteData = new RouteData(context.HttpContext.GetRouteData()); + context.HttpContext.Request.RouteValues = context.HttpContext.GetRouteData().Values; + + /* + // (RouteEndpoint routeEndpointMatch, RouteValueDictionary routeValuesMatch) = FindRouteEndpoint(context.HttpContext, routeCandidates); + if (routeEndpointMatch != null && routeValuesMatch != null) { if (routeEndpointMatch.RoutePattern?.RequiredValues != null) @@ -98,6 +150,7 @@ private void SetRouteContext(RouteContext context) { _logger.LogError("Matching route endpoint not found for the given request."); } + */ } catch { @@ -105,7 +158,8 @@ private void SetRouteContext(RouteContext context) } } - private static (RouteEndpoint routeEndpoint, RouteValueDictionary routeValues) FindRouteEndpoint( + /* + private (RouteEndpoint routeEndpoint, RouteValueDictionary routeValues) FindRouteEndpoint( HttpContext context, IList> routeCandidates) { @@ -146,5 +200,6 @@ private static (RouteEndpoint routeEndpoint, RouteValueDictionary routeValues) F return (null, null); } + */ } } From 437f8b5b04070a08e6719955ff21cf03af049877 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fernando=20Henrique=20Inoc=C3=AAncio=20Borba=20Ferreira?= Date: Mon, 22 Apr 2024 16:39:51 -0700 Subject: [PATCH 2/7] Adding tests to validate bundle internal calls to _history and _history/version. --- .../Rest/BundleEdgeCaseTests.cs | 60 +++++++++++++++++++ 1 file changed, 60 insertions(+) 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() From 5fb6510baf2e1d6d6b643d00f3227509c6ff69da Mon Sep 17 00:00:00 2001 From: Brendan Kowitz Date: Mon, 22 Apr 2024 17:33:34 -0700 Subject: [PATCH 3/7] Improve fix --- .../Features/Resources/Bundle/BundleRouter.cs | 160 +++++------------- 1 file changed, 38 insertions(+), 122 deletions(-) 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 d237c4a355..15305a4129 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 @@ -42,8 +42,12 @@ public BundleRouter( EndpointSelector endpointSelector, ILogger logger) { + EnsureArg.IsNotNull(parameterPolicies, nameof(parameterPolicies)); + EnsureArg.IsNotNull(matcherPolicies, nameof(matcherPolicies)); EnsureArg.IsNotNull(endpointDataSource, nameof(endpointDataSource)); + EnsureArg.IsNotNull(endpointSelector, nameof(endpointSelector)); EnsureArg.IsNotNull(logger, nameof(logger)); + _parameterPolicies = parameterPolicies; _matcherPolicies = matcherPolicies; _endpointDataSource = endpointDataSource; @@ -56,150 +60,62 @@ 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 List>(); + 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; - - foreach (RouteEndpoint endpoint in endpoints) - { - var routeValues = new RouteValueDictionary(); - var routeDefaults = new RouteValueDictionary(endpoint.RoutePattern.Defaults); - var templateMatcher = new TemplateMatcher(TemplateParser.Parse(endpoint.RoutePattern.RawText), routeDefaults); - - if (!templateMatcher.TryMatch(path, routeValues)) - { - continue; - } - - // Older MVC constraint processing... - RoutePattern pattern = RoutePatternFactory.Parse(endpoint.RoutePattern.RawText, routeDefaults, null); - TemplateBinder templateBinder = _parameterPolicies.Create(pattern); - - if (!templateBinder.TryProcessConstraints(context.HttpContext, routeValues, out var parameterName, out var constraint)) - { - continue; - } - - /* - HttpMethodAttribute httpMethodAttribute = endpoint.Metadata.GetMetadata(); - if (httpMethodAttribute != null && !httpMethodAttribute.HttpMethods.Any(x => x.Equals(method, StringComparison.OrdinalIgnoreCase))) - { - continue; - } - */ - - routeCandidates.Add(new KeyValuePair(endpoint, routeValues)); - } - - var candidateSet = new CandidateSet( - routeCandidates.Select(x => x.Key).Cast().ToArray(), - routeCandidates.Select(x => x.Value).ToArray(), - Enumerable.Repeat(1, routeCandidates.Count).ToArray()); + var routeValues = new RouteValueDictionary(); + var routeDefaults = new RouteValueDictionary(endpoint.RoutePattern.Defaults); - RouteEndpoint[] potentialRoutes = routeCandidates.Select(x => x.Key).ToArray(); + RoutePattern pattern = endpoint.RoutePattern; + TemplateBinder templateBinder = _parameterPolicies.Create(pattern); - _matcherPolicies - .OrderBy(x => x.Order) - .OfType() - .Where(x => x.AppliesToEndpoints(potentialRoutes)) - .ToList() - .ForEach(x => x.ApplyAsync(context.HttpContext, candidateSet).GetAwaiter().GetResult()); + var templateMatcher = new TemplateMatcher(new RouteTemplate(pattern), routeDefaults); - _endpointSelector.SelectAsync( - context.HttpContext, - candidateSet) - .GetAwaiter() - .GetResult(); - - context.Handler = context.HttpContext.GetEndpoint()?.RequestDelegate; - context.RouteData = new RouteData(context.HttpContext.GetRouteData()); - context.HttpContext.Request.RouteValues = context.HttpContext.GetRouteData().Values; - - /* - // (RouteEndpoint routeEndpointMatch, RouteValueDictionary routeValuesMatch) = FindRouteEndpoint(context.HttpContext, routeCandidates); - - if (routeEndpointMatch != null && routeValuesMatch != null) + 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) + 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 (RouteEndpoint routeEndpoint, RouteValueDictionary routeValues) FindRouteEndpoint( - HttpContext context, - IList> routeCandidates) - { - if (routeCandidates.Count == 0) - { - return (null, null); + routeCandidates.Add(new KeyValuePair(endpoint, routeValues)); } - if (routeCandidates.Count == 1) - { - return (routeCandidates[0].Key, routeCandidates[0].Value); - } + var candidateSet = new CandidateSet( + routeCandidates.Select(x => x.Key).Cast().ToArray(), + routeCandidates.Select(x => x.Value).ToArray(), + Enumerable.Repeat(1, routeCandidates.Count).ToArray()); - // 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. + RouteEndpoint[] potentialRoutes = routeCandidates.Select(x => x.Key).ToArray(); - var method = context.Request.Method; - if (method.Equals(HttpMethods.Post, StringComparison.OrdinalIgnoreCase)) + foreach (IEndpointSelectorPolicy policy in _matcherPolicies + .OrderBy(x => x.Order) + .OfType() + .Where(x => x.AppliesToEndpoints(potentialRoutes))) { - var conditional = context.Request.Headers.ContainsKey(KnownHeaders.IfNoneExist); - var pair = routeCandidates.SingleOrDefault(r => (r.Key.Metadata.GetMetadata() != null) == conditional); - return (pair.Key, pair.Value); + await policy.ApplyAsync(context.HttpContext, candidateSet); } - else if (method.Equals(HttpMethods.Patch, StringComparison.OrdinalIgnoreCase)) + + await _endpointSelector.SelectAsync(context.HttpContext, candidateSet); + + Endpoint selectedEndpoint = context.HttpContext.GetEndpoint(); + if (selectedEndpoint != null) { - 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); - } - } + context.Handler = selectedEndpoint.RequestDelegate; + context.RouteData = new RouteData(context.HttpContext.GetRouteData()); + context.HttpContext.Request.RouteValues = context.HttpContext.GetRouteData().Values; } - - return (null, null); } - */ } } From ab3bc1325ac020710f3851e25190219a3d1a9e61 Mon Sep 17 00:00:00 2001 From: Brendan Kowitz Date: Mon, 22 Apr 2024 17:49:56 -0700 Subject: [PATCH 4/7] Applies to isn't needed --- .../Features/Resources/Bundle/BundleRouter.cs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) 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 15305a4129..752ba7b9da 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 @@ -78,11 +78,13 @@ public async Task RouteAsync(RouteContext context) var templateMatcher = new TemplateMatcher(new RouteTemplate(pattern), routeDefaults); + // Pattern match if (!templateMatcher.TryMatch(path, routeValues)) { continue; } + // Eliminate routes that don't match constraints if (!templateBinder.TryProcessConstraints(context.HttpContext, routeValues, out var parameterName, out IRouteConstraint constraint)) { _logger.LogDebug("Constraint '{ConstraintType}' not met for parameter '{ParameterName}'", constraint, parameterName); @@ -99,10 +101,10 @@ public async Task RouteAsync(RouteContext context) RouteEndpoint[] potentialRoutes = routeCandidates.Select(x => x.Key).ToArray(); + // Covered things like Consumes, HttpVerbs etc... foreach (IEndpointSelectorPolicy policy in _matcherPolicies .OrderBy(x => x.Order) - .OfType() - .Where(x => x.AppliesToEndpoints(potentialRoutes))) + .OfType()) { await policy.ApplyAsync(context.HttpContext, candidateSet); } From 8aed25500dd3076b912c48f1e6e3440c77817617 Mon Sep 17 00:00:00 2001 From: Brendan Kowitz Date: Tue, 23 Apr 2024 11:20:23 -0700 Subject: [PATCH 5/7] Removes unused variable --- .../Features/Resources/Bundle/BundleRouter.cs | 2 -- 1 file changed, 2 deletions(-) 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 752ba7b9da..82d05ea014 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 @@ -99,8 +99,6 @@ public async Task RouteAsync(RouteContext context) routeCandidates.Select(x => x.Value).ToArray(), Enumerable.Repeat(1, routeCandidates.Count).ToArray()); - RouteEndpoint[] potentialRoutes = routeCandidates.Select(x => x.Key).ToArray(); - // Covered things like Consumes, HttpVerbs etc... foreach (IEndpointSelectorPolicy policy in _matcherPolicies .OrderBy(x => x.Order) From 63c64ce1594f2ee7730ec9567cce4ac796654bb6 Mon Sep 17 00:00:00 2001 From: Brendan Kowitz Date: Fri, 26 Apr 2024 16:43:53 -0700 Subject: [PATCH 6/7] Some matches are not RouteEndpoints (i.e. 404s) --- .../Features/Resources/Bundle/BundleRouter.cs | 28 +++++++++++++------ 1 file changed, 20 insertions(+), 8 deletions(-) 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 82d05ea014..02f57748ff 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,8 +5,10 @@ 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; @@ -25,8 +27,10 @@ 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 _parameterPolicies; @@ -64,7 +68,7 @@ public async Task RouteAsync(RouteContext context) { EnsureArg.IsNotNull(context, nameof(context)); - var routeCandidates = new List>(); + var routeCandidates = new Dictionary(); IEnumerable endpoints = _endpointDataSource.Endpoints.OfType(); PathString path = context.HttpContext.Request.Path; @@ -91,7 +95,7 @@ public async Task RouteAsync(RouteContext context) continue; } - routeCandidates.Add(new KeyValuePair(endpoint, routeValues)); + routeCandidates.Add(endpoint, routeValues); } var candidateSet = new CandidateSet( @@ -99,7 +103,7 @@ public async Task RouteAsync(RouteContext context) routeCandidates.Select(x => x.Value).ToArray(), Enumerable.Repeat(1, routeCandidates.Count).ToArray()); - // Covered things like Consumes, HttpVerbs etc... + // Policies apply filters / matches on attributes such as Consumes, HttpVerbs etc... foreach (IEndpointSelectorPolicy policy in _matcherPolicies .OrderBy(x => x.Order) .OfType()) @@ -110,11 +114,19 @@ public async Task RouteAsync(RouteContext context) await _endpointSelector.SelectAsync(context.HttpContext, candidateSet); Endpoint selectedEndpoint = context.HttpContext.GetEndpoint(); - if (selectedEndpoint != null) + + // 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) { + RouteData data = context.HttpContext.GetRouteData(); context.Handler = selectedEndpoint.RequestDelegate; - context.RouteData = new RouteData(context.HttpContext.GetRouteData()); - context.HttpContext.Request.RouteValues = context.HttpContext.GetRouteData().Values; + context.RouteData = new RouteData(data); + context.HttpContext.Request.RouteValues = context.RouteData.Values; + } + else + { + _logger.LogDebug("No RouteEndpoint found for '{Path}'", HttpUtility.UrlEncode(path)); } } } From b011e75ccf24777eb043c4d7cce14cfbe0c84c29 Mon Sep 17 00:00:00 2001 From: Brendan Kowitz Date: Wed, 8 May 2024 16:38:10 -0700 Subject: [PATCH 7/7] fix variable name --- .../Features/Resources/Bundle/BundleRouter.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) 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 02f57748ff..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 @@ -33,26 +33,26 @@ namespace Microsoft.Health.Fhir.Api.Features.Resources.Bundle /// internal class BundleRouter : IRouter { - private readonly TemplateBinderFactory _parameterPolicies; + private readonly TemplateBinderFactory _templateBinderFactory; private readonly IEnumerable _matcherPolicies; private readonly EndpointDataSource _endpointDataSource; private readonly EndpointSelector _endpointSelector; private readonly ILogger _logger; public BundleRouter( - TemplateBinderFactory parameterPolicies, + TemplateBinderFactory templateBinderFactory, IEnumerable matcherPolicies, EndpointDataSource endpointDataSource, EndpointSelector endpointSelector, ILogger logger) { - EnsureArg.IsNotNull(parameterPolicies, nameof(parameterPolicies)); + 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)); - _parameterPolicies = parameterPolicies; + _templateBinderFactory = templateBinderFactory; _matcherPolicies = matcherPolicies; _endpointDataSource = endpointDataSource; _endpointSelector = endpointSelector; @@ -78,7 +78,7 @@ public async Task RouteAsync(RouteContext context) var routeDefaults = new RouteValueDictionary(endpoint.RoutePattern.Defaults); RoutePattern pattern = endpoint.RoutePattern; - TemplateBinder templateBinder = _parameterPolicies.Create(pattern); + TemplateBinder templateBinder = _templateBinderFactory.Create(pattern); var templateMatcher = new TemplateMatcher(new RouteTemplate(pattern), routeDefaults);