diff --git a/src/Microsoft.Health.Fhir.Core/Features/KnownHeaders.cs b/src/Microsoft.Health.Fhir.Core/Features/KnownHeaders.cs index a51cc16ee3..d73ca2ae25 100644 --- a/src/Microsoft.Health.Fhir.Core/Features/KnownHeaders.cs +++ b/src/Microsoft.Health.Fhir.Core/Features/KnownHeaders.cs @@ -30,5 +30,8 @@ public static class KnownHeaders public const string ProfileValidation = "x-ms-profile-validation"; public const string CustomAuditHeaderPrefix = "X-MS-AZUREFHIR-AUDIT-"; public const string FhirUserHeader = "x-ms-fhiruser"; + + // #conditionalQueryParallelism - Header used to activate parallel conditional-query processing. + public const string ConditionalQueryProcessingLogic = "x-conditionalquery-processing-logic"; } } diff --git a/src/Microsoft.Health.Fhir.Core/Features/KnownQueryParameterNames.cs b/src/Microsoft.Health.Fhir.Core/Features/KnownQueryParameterNames.cs index a34719c317..2a9b345416 100644 --- a/src/Microsoft.Health.Fhir.Core/Features/KnownQueryParameterNames.cs +++ b/src/Microsoft.Health.Fhir.Core/Features/KnownQueryParameterNames.cs @@ -60,6 +60,11 @@ public static class KnownQueryParameterNames public const string Container = "_container"; + /// + /// Originally for CosmosDB workloads to hint that this request should run with a max parallel setting. + /// + public const string OptimizeConcurrency = "_optimizeConcurrency"; + /// /// The anonymization configuration /// diff --git a/src/Microsoft.Health.Fhir.Core/Messages/Create/ConditionalCreateResourceRequest.cs b/src/Microsoft.Health.Fhir.Core/Messages/Create/ConditionalCreateResourceRequest.cs index a5f3e40ac3..aa5962c63c 100644 --- a/src/Microsoft.Health.Fhir.Core/Messages/Create/ConditionalCreateResourceRequest.cs +++ b/src/Microsoft.Health.Fhir.Core/Messages/Create/ConditionalCreateResourceRequest.cs @@ -15,7 +15,10 @@ public sealed class ConditionalCreateResourceRequest : ConditionalResourceReques { private static readonly string[] Capabilities = new string[1] { "conditionalCreate = true" }; - public ConditionalCreateResourceRequest(ResourceElement resource, IReadOnlyList> conditionalParameters, BundleResourceContext bundleResourceContext = null) + public ConditionalCreateResourceRequest( + ResourceElement resource, + IReadOnlyList> conditionalParameters, + BundleResourceContext bundleResourceContext = null) : base(resource.InstanceType, conditionalParameters, bundleResourceContext) { EnsureArg.IsNotNull(resource, nameof(resource)); diff --git a/src/Microsoft.Health.Fhir.Core/Messages/Patch/ConditionalPatchResourceRequest.cs b/src/Microsoft.Health.Fhir.Core/Messages/Patch/ConditionalPatchResourceRequest.cs index af1f17ea1f..4d9284076b 100644 --- a/src/Microsoft.Health.Fhir.Core/Messages/Patch/ConditionalPatchResourceRequest.cs +++ b/src/Microsoft.Health.Fhir.Core/Messages/Patch/ConditionalPatchResourceRequest.cs @@ -21,7 +21,7 @@ public ConditionalPatchResourceRequest( string resourceType, PatchPayload payload, IReadOnlyList> conditionalParameters, - BundleResourceContext bundleResourceContext, + BundleResourceContext bundleResourceContext = null, WeakETag weakETag = null) : base(resourceType, conditionalParameters, bundleResourceContext) { diff --git a/src/Microsoft.Health.Fhir.Core/Messages/Upsert/ConditionalUpsertResourceRequest.cs b/src/Microsoft.Health.Fhir.Core/Messages/Upsert/ConditionalUpsertResourceRequest.cs index 8b3a1d54ed..10d267a530 100644 --- a/src/Microsoft.Health.Fhir.Core/Messages/Upsert/ConditionalUpsertResourceRequest.cs +++ b/src/Microsoft.Health.Fhir.Core/Messages/Upsert/ConditionalUpsertResourceRequest.cs @@ -14,7 +14,10 @@ public sealed class ConditionalUpsertResourceRequest : ConditionalResourceReques { private static readonly string[] Capabilities = new string[1] { "conditionalUpdate = true" }; - public ConditionalUpsertResourceRequest(ResourceElement resource, IReadOnlyList> conditionalParameters, BundleResourceContext bundleResourceContext = null) + public ConditionalUpsertResourceRequest( + ResourceElement resource, + IReadOnlyList> conditionalParameters, + BundleResourceContext bundleResourceContext = null) : base(resource.InstanceType, conditionalParameters, bundleResourceContext) { EnsureArg.IsNotNull(resource, nameof(resource)); diff --git a/src/Microsoft.Health.Fhir.CosmosDb/Features/Search/FhirCosmosSearchService.cs b/src/Microsoft.Health.Fhir.CosmosDb/Features/Search/FhirCosmosSearchService.cs index 923e1920b2..343e14d201 100644 --- a/src/Microsoft.Health.Fhir.CosmosDb/Features/Search/FhirCosmosSearchService.cs +++ b/src/Microsoft.Health.Fhir.CosmosDb/Features/Search/FhirCosmosSearchService.cs @@ -14,11 +14,10 @@ using EnsureThat; using Microsoft.Azure.Cosmos; using Microsoft.Extensions.Logging; -using Microsoft.Health.Core; using Microsoft.Health.Core.Features.Context; -using Microsoft.Health.Extensions.DependencyInjection; using Microsoft.Health.Fhir.Core.Exceptions; using Microsoft.Health.Fhir.Core.Extensions; +using Microsoft.Health.Fhir.Core.Features; using Microsoft.Health.Fhir.Core.Features.Context; using Microsoft.Health.Fhir.Core.Features.Persistence; using Microsoft.Health.Fhir.Core.Features.Search; @@ -40,7 +39,6 @@ internal class FhirCosmosSearchService : SearchService private readonly RequestContextAccessor _requestContextAccessor; private readonly CosmosDataStoreConfiguration _cosmosConfig; private readonly ICosmosDbCollectionPhysicalPartitionInfo _physicalPartitionInfo; - private readonly QueryPartitionStatisticsCache _queryPartitionStatisticsCache; private readonly Lazy>> _expressionRewriters; private readonly ILogger _logger; private readonly SearchParameterInfo _resourceTypeSearchParameter; @@ -54,7 +52,6 @@ public FhirCosmosSearchService( RequestContextAccessor requestContextAccessor, CosmosDataStoreConfiguration cosmosConfig, ICosmosDbCollectionPhysicalPartitionInfo physicalPartitionInfo, - QueryPartitionStatisticsCache queryPartitionStatisticsCache, CompartmentSearchRewriter compartmentSearchRewriter, SmartCompartmentSearchRewriter smartCompartmentSearchRewriter, ILogger logger) @@ -65,7 +62,6 @@ public FhirCosmosSearchService( EnsureArg.IsNotNull(requestContextAccessor, nameof(requestContextAccessor)); EnsureArg.IsNotNull(cosmosConfig, nameof(cosmosConfig)); EnsureArg.IsNotNull(physicalPartitionInfo, nameof(physicalPartitionInfo)); - EnsureArg.IsNotNull(queryPartitionStatisticsCache, nameof(queryPartitionStatisticsCache)); EnsureArg.IsNotNull(compartmentSearchRewriter, nameof(compartmentSearchRewriter)); EnsureArg.IsNotNull(smartCompartmentSearchRewriter, nameof(smartCompartmentSearchRewriter)); EnsureArg.IsNotNull(logger, nameof(logger)); @@ -75,7 +71,6 @@ public FhirCosmosSearchService( _requestContextAccessor = requestContextAccessor; _cosmosConfig = cosmosConfig; _physicalPartitionInfo = physicalPartitionInfo; - _queryPartitionStatisticsCache = queryPartitionStatisticsCache; _logger = logger; _resourceTypeSearchParameter = SearchParameterInfo.ResourceTypeSearchParameter; _resourceIdSearchParameter = new SearchParameterInfo(SearchParameterNames.Id, SearchParameterNames.Id); @@ -403,10 +398,9 @@ protected override async Task SearchForReindexInternalAsync( // Additionally, when we query sequentially, we would like to gradually fan out the parallelism, but the Cosmos DB SDK // does not currently properly support that. See https://github.com/Azure/azure-cosmos-dotnet-v3/issues/2290 - QueryPartitionStatistics queryPartitionStatistics = null; IFhirRequestContext fhirRequestContext = null; ConcurrentBag messagesList = null; - if (_physicalPartitionInfo.PhysicalPartitionCount > 1 && queryRequestOptionsOverride == null) + if (queryRequestOptionsOverride == null) { if (searchOptions.Sort?.Count > 0) { @@ -417,21 +411,24 @@ protected override async Task SearchForReindexInternalAsync( if (searchOptions.Expression != null && // without a filter the query will not be selective string.IsNullOrEmpty(searchOptions.ContinuationToken)) { - // Telemetry currently shows that when there is a continuation token, the the query only hits one partition. + // Telemetry currently shows that when there is a continuation token, then the query only hits one partition. // This may not be true forever, in which case we would want to encode the max concurrency in the continuation token. - queryPartitionStatistics = _queryPartitionStatisticsCache.GetQueryPartitionStatistics(searchOptions.Expression); - if (IsQuerySelective(queryPartitionStatistics)) - { - feedOptions.MaxConcurrency = _cosmosConfig.ParallelQueryOptions.MaxQueryConcurrency; - } - - // plant a ConcurrentBag int the request context's properties, so the CosmosResponseProcessor - // knows to add the individual ResponseMessages sent as part of this search. - fhirRequestContext = _requestContextAccessor.RequestContext; if (fhirRequestContext != null) { + // Check if the "Optimize Concurrency" flag is present in the FHIR context, then Cosmos DB + // will be able to maximize the number of concurrent operations. + if (fhirRequestContext.Properties.TryGetValue(KnownQueryParameterNames.OptimizeConcurrency, out object maxParallelAsObject)) + { + if (maxParallelAsObject != null && Convert.ToBoolean(maxParallelAsObject)) + { + feedOptions.MaxConcurrency = _cosmosConfig.ParallelQueryOptions.MaxQueryConcurrency; + } + } + + // Plant a ConcurrentBag int the request context's properties, so the CosmosResponseProcessor + // knows to add the individual ResponseMessages sent as part of this search. messagesList = new ConcurrentBag(); fhirRequestContext.Properties[Constants.CosmosDbResponseMessagesProperty] = messagesList; } @@ -468,68 +465,31 @@ protected override async Task SearchForReindexInternalAsync( (results, nextContinuationToken) = await _fhirDataStore.ExecuteDocumentQueryAsync(sqlQuerySpec, feedOptions, continuationToken, searchOptions.MaxItemCountSpecifiedByClient, feedOptions.MaxConcurrency == null ? searchEnumerationTimeoutOverrideIfSequential : null, cancellationToken); } - if (queryPartitionStatistics != null && messagesList != null) + if (messagesList != null) { if ((results == null || results.Count < desiredItemCount) && !string.IsNullOrEmpty(nextContinuationToken)) { // ExecuteDocumentQueryAsync gave up on filling the pages. This suggests that we would have been better off querying in parallel. - queryPartitionStatistics.Update(_physicalPartitionInfo.PhysicalPartitionCount); _logger.LogInformation( - "Failed to fill items, found {ItemCount}, needed {DesiredItemCount}. Updating statistics to {PhysicalPartitionCount}", + "Failed to fill items, found {ItemCount}, needed {DesiredItemCount}. Physical partition count {PhysicalPartitionCount}", results.Count, desiredItemCount, _physicalPartitionInfo.PhysicalPartitionCount); } - else - { - // determine the number of unique physical partitions queried as part of this search. - int physicalPartitionCount = messagesList.Select(r => r.Headers["x-ms-documentdb-partitionkeyrangeid"]).Distinct().Count(); - queryPartitionStatistics.Update(physicalPartitionCount); - } } return (results, nextContinuationToken, feedOptions.MaxConcurrency); } finally { - if (queryPartitionStatistics != null && fhirRequestContext != null) + if (fhirRequestContext != null) { fhirRequestContext.Properties.Remove(Constants.CosmosDbResponseMessagesProperty); } } } - /// - /// Heuristic for determining whether a query is selective or not. If it is, we should query partitions in parallel. - /// If it is not, we should query sequentially, since we would expect to get a full page of results from the first partition. - /// This is really simple right now - /// - private bool IsQuerySelective(QueryPartitionStatistics queryPartitionStatistics) - { - int? averagePartitionCount = queryPartitionStatistics.GetAveragePartitionCount(); - - if (averagePartitionCount.HasValue && _cosmosConfig.UseQueryStatistics) - { - // this is not a new query - - double fractionOfPartitionsHit = (double)averagePartitionCount.Value / _physicalPartitionInfo.PhysicalPartitionCount; - - if (fractionOfPartitionsHit >= 0.5) - { - _logger.LogInformation( - "Query was Selective. Avg. Partitions: {AvgPartitions} / Physical Partitions: {PhysicalPartitionCount} = {FractionOfPartitionsHit}", - averagePartitionCount.Value, - _physicalPartitionInfo.PhysicalPartitionCount, - fractionOfPartitionsHit); - - return true; - } - } - - return false; - } - private async Task ExecuteCountSearchAsync( QueryDefinition sqlQuerySpec, CancellationToken cancellationToken) diff --git a/src/Microsoft.Health.Fhir.CosmosDb/Features/Storage/QueryPartitionStatisticsCache.cs b/src/Microsoft.Health.Fhir.CosmosDb/Features/Storage/QueryPartitionStatisticsCache.cs deleted file mode 100644 index d76806b4a5..0000000000 --- a/src/Microsoft.Health.Fhir.CosmosDb/Features/Storage/QueryPartitionStatisticsCache.cs +++ /dev/null @@ -1,57 +0,0 @@ -// ------------------------------------------------------------------------------------------------- -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License (MIT). See LICENSE in the repo root for license information. -// ------------------------------------------------------------------------------------------------- - -using System; -using EnsureThat; -using Microsoft.Extensions.Caching.Memory; -using Microsoft.Health.Fhir.Core.Features.Search.Expressions; - -namespace Microsoft.Health.Fhir.CosmosDb.Features.Storage -{ - /// - /// Maintains an LRU cache of , keyed by search expression ignoring values in the expression. - /// - public sealed class QueryPartitionStatisticsCache : IDisposable - { - private readonly MemoryCache _cache = new(new MemoryCacheOptions { SizeLimit = 512 }); - - internal QueryPartitionStatistics GetQueryPartitionStatistics(Expression expression) - { - return _cache.GetOrCreate( - new ExpressionWrapper(expression), - e => - { - e.Size = 1; - return new QueryPartitionStatistics(); - }); - } - - public void Dispose() - { - _cache?.Dispose(); - } - - private class ExpressionWrapper - { - private readonly int _hashCode; - private readonly Expression _expression; - - public ExpressionWrapper(Expression expression) - { - EnsureArg.IsNotNull(expression, nameof(expression)); - - _expression = expression; - - HashCode hashCode = default; - expression.AddValueInsensitiveHashCode(ref hashCode); - _hashCode = hashCode.ToHashCode(); - } - - public override int GetHashCode() => _hashCode; - - public override bool Equals(object obj) => obj is ExpressionWrapper e && _expression.ValueInsensitiveEquals(e._expression); - } - } -} diff --git a/src/Microsoft.Health.Fhir.CosmosDb/Registration/FhirServerBuilderCosmosDbRegistrationExtensions.cs b/src/Microsoft.Health.Fhir.CosmosDb/Registration/FhirServerBuilderCosmosDbRegistrationExtensions.cs index c5e2963259..548063e869 100644 --- a/src/Microsoft.Health.Fhir.CosmosDb/Registration/FhirServerBuilderCosmosDbRegistrationExtensions.cs +++ b/src/Microsoft.Health.Fhir.CosmosDb/Registration/FhirServerBuilderCosmosDbRegistrationExtensions.cs @@ -222,11 +222,6 @@ private static IFhirServerBuilder AddCosmosDbPersistence(this IFhirServerBuilder .AsSelf() .AsImplementedInterfaces(); - services.Add() - .Singleton() - .AsSelf() - .AsImplementedInterfaces(); - services.Add() .Transient() .AsImplementedInterfaces(); diff --git a/src/Microsoft.Health.Fhir.Shared.Api.UnitTests/Features/Resources/Bundle/BundleHandlerEdgeCaseTests.cs b/src/Microsoft.Health.Fhir.Shared.Api.UnitTests/Features/Resources/Bundle/BundleHandlerEdgeCaseTests.cs new file mode 100644 index 0000000000..11e010672a --- /dev/null +++ b/src/Microsoft.Health.Fhir.Shared.Api.UnitTests/Features/Resources/Bundle/BundleHandlerEdgeCaseTests.cs @@ -0,0 +1,219 @@ +// ------------------------------------------------------------------------------------------------- +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License (MIT). See LICENSE in the repo root for license information. +// ------------------------------------------------------------------------------------------------- + +using System; +using System.Collections.Generic; +using System.Collections.Specialized; +using System.Diagnostics.CodeAnalysis; +using System.Globalization; +using System.IO; +using System.Linq; +using System.Threading; +using Hl7.Fhir.Model; +using Hl7.Fhir.Serialization; +using Hl7.Fhir.Specification.Navigation; +using Hl7.FhirPath.Sprache; +using MediatR; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Http.Features; +using Microsoft.AspNetCore.Http.Features.Authentication; +using Microsoft.AspNetCore.Routing; +using Microsoft.CodeAnalysis; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Abstractions; +using Microsoft.Extensions.Options; +using Microsoft.Extensions.Primitives; +using Microsoft.Health.Abstractions.Features.Transactions; +using Microsoft.Health.Api.Features.Audit; +using Microsoft.Health.Core.Features.Context; +using Microsoft.Health.Fhir.Api.Features.Bundle; +using Microsoft.Health.Fhir.Api.Features.Exceptions; +using Microsoft.Health.Fhir.Api.Features.Resources.Bundle; +using Microsoft.Health.Fhir.Api.Features.Routing; +using Microsoft.Health.Fhir.Core.Configs; +using Microsoft.Health.Fhir.Core.Exceptions; +using Microsoft.Health.Fhir.Core.Extensions; +using Microsoft.Health.Fhir.Core.Features; +using Microsoft.Health.Fhir.Core.Features.Context; +using Microsoft.Health.Fhir.Core.Features.Persistence; +using Microsoft.Health.Fhir.Core.Features.Persistence.Orchestration; +using Microsoft.Health.Fhir.Core.Features.Resources; +using Microsoft.Health.Fhir.Core.Features.Resources.Bundle; +using Microsoft.Health.Fhir.Core.Features.Search; +using Microsoft.Health.Fhir.Core.Features.Security.Authorization; +using Microsoft.Health.Fhir.Core.Messages.Bundle; +using Microsoft.Health.Fhir.Core.UnitTests.Features.Context; +using Microsoft.Health.Fhir.Tests.Common; +using Microsoft.Health.Fhir.ValueSets; +using Microsoft.Health.Test.Utilities; +using NSubstitute; +using NSubstitute.Core; +using Xunit; +using static Hl7.Fhir.Model.Bundle; +using Task = System.Threading.Tasks.Task; + +namespace Microsoft.Health.Fhir.Api.UnitTests.Features.Resources.Bundle +{ + [Trait(Traits.OwningTeam, OwningTeam.Fhir)] + [Trait(Traits.Category, Categories.Bundle)] + public class BundleHandlerEdgeCaseTests + { + private DefaultFhirRequestContext _fhirRequestContext; + + public BundleHandlerEdgeCaseTests() + { + _fhirRequestContext = new DefaultFhirRequestContext + { + BaseUri = new Uri("https://localhost/"), + CorrelationId = Guid.NewGuid().ToString(), + ResponseHeaders = new HeaderDictionary(), + RequestHeaders = new HeaderDictionary(), + }; + } + + [Theory] + [InlineData(true)] + [InlineData(false)] + public void GivenABundle_WhenProcessedWithConditionalQueryMaxParallelism_TheFhirContextPropertyBagsShouldBePopulatedAsExpected(bool maxParallelism) + { + // #conditionalQueryParallelism + + // In this test the following steps are executed/validated: + // 1 - When the created HTTP request contains the header "x-conditionalquery-processing-logic" set as "parallel". + // 2 - BundleHandler's constructor recognizes the presence of the header and adds "_optimizeConcurrency" to FHIR Request Context property bag. + // 3 - A validation is executed to ensure that the FHIR Request Context property bag contains the key "_optimizeConcurrency" as it's set with the expected value. + // 4 - If the created HTTP request does not contain the header "x-conditionalquery-processing-logic" set as "parallel", then the key "_optimizeConcurrency" + // is not expected in the FHIR Request Context property bag. + + var bundleHandlerComponents = GetBundleHandlerComponents(new BundleRequestOptions() { MaxParallelism = maxParallelism }); + + var fhirContextPropertyBag = bundleHandlerComponents.FhirRequestContext.Properties; + + if (maxParallelism) + { + Assert.True(fhirContextPropertyBag.ContainsKey(KnownQueryParameterNames.OptimizeConcurrency)); + Assert.Equal(true, fhirContextPropertyBag[KnownQueryParameterNames.OptimizeConcurrency]); + } + else + { + Assert.False(fhirContextPropertyBag.ContainsKey(KnownQueryParameterNames.OptimizeConcurrency)); + } + } + + private (IRouter Router, BundleConfiguration BundleConfiguration, IMediator Mediator, BundleHandler BundleHandler, IFhirRequestContext FhirRequestContext) GetBundleHandlerComponents(BundleRequestOptions options) + { + IRouter router = Substitute.For(); + + var fhirRequestContextAccessor = Substitute.For>(); + fhirRequestContextAccessor.RequestContext.Returns(_fhirRequestContext); + + IHttpContextAccessor httpContextAccessor = Substitute.For(); + + var fhirJsonSerializer = new FhirJsonSerializer(); + var fhirJsonParser = new FhirJsonParser(); + + ISearchService searchService = Substitute.For(); + var resourceReferenceResolver = new ResourceReferenceResolver(searchService, new QueryStringParser()); + + var transactionBundleValidatorLogger = Substitute.For>(); + var transactionBundleValidator = new TransactionBundleValidator(resourceReferenceResolver, transactionBundleValidatorLogger); + + var bundleHttpContextAccessor = new BundleHttpContextAccessor(); + + var bundleConfiguration = new BundleConfiguration(); + var bundleOptions = Substitute.For>(); + bundleOptions.Value.Returns(bundleConfiguration); + + var bundleOrchestratorLogger = Substitute.For>(); + var bundleOrchestrator = new BundleOrchestrator(bundleOptions, bundleOrchestratorLogger); + + IFeatureCollection featureCollection = CreateFeatureCollection(router); + var httpContext = new DefaultHttpContext(featureCollection) + { + Request = + { + Scheme = "https", + Host = new HostString("localhost"), + PathBase = new PathString("/"), + }, + }; + var contextualHeaderDictionary = new HeaderDictionary(); + httpContext.Request.Headers.Returns(contextualHeaderDictionary); + + if (options.MaxParallelism) + { + httpContext.Request.Headers[KnownHeaders.ConditionalQueryProcessingLogic] = new StringValues("parallel"); + } + + httpContextAccessor.HttpContext.Returns(httpContext); + + var transactionHandler = Substitute.For(); + + var resourceIdProvider = new ResourceIdProvider(); + + IAuditEventTypeMapping auditEventTypeMapping = Substitute.For(); + + var mediator = Substitute.For(); + + var bundleHandler = new BundleHandler( + httpContextAccessor, + fhirRequestContextAccessor, + fhirJsonSerializer, + fhirJsonParser, + transactionHandler, + bundleHttpContextAccessor, + bundleOrchestrator, + resourceIdProvider, + transactionBundleValidator, + resourceReferenceResolver, + auditEventTypeMapping, + bundleOptions, + DisabledFhirAuthorizationService.Instance, + mediator, + NullLogger.Instance); + + return (router, bundleConfiguration, mediator, bundleHandler, fhirRequestContextAccessor.RequestContext); + } + + private IFeatureCollection CreateFeatureCollection(IRouter router) + { + var featureCollection = Substitute.For(); + + // Header Dictionary + var headerFeature = new HeaderDictionary(); + featureCollection.Get().Returns(headerFeature); + + // Authentication + var httpAuthenticationFeature = Substitute.For(); + featureCollection.Get().Returns(httpAuthenticationFeature); + + // Routing + var routingFeature = Substitute.For(); + var routeData = new RouteData(); + routeData.Routers.Add(router); + routingFeature.RouteData.Returns(routeData); + featureCollection.Get().Returns(routingFeature); + + var features = new List> + { + new KeyValuePair(typeof(IHeaderDictionary), headerFeature), + new KeyValuePair(typeof(IHttpAuthenticationFeature), httpAuthenticationFeature), + new KeyValuePair(typeof(IRoutingFeature), routingFeature), + }; + + featureCollection[typeof(IHeaderDictionary)].Returns(headerFeature); + featureCollection[typeof(IHttpAuthenticationFeature)].Returns(httpAuthenticationFeature); + featureCollection[typeof(IRoutingFeature)].Returns(routingFeature); + + featureCollection.GetEnumerator().Returns(features.GetEnumerator()); + return featureCollection; + } + + private sealed class BundleRequestOptions() + { + public bool MaxParallelism { get; set; } = false; + } + } +} diff --git a/src/Microsoft.Health.Fhir.Shared.Api.UnitTests/Microsoft.Health.Fhir.Shared.Api.UnitTests.projitems b/src/Microsoft.Health.Fhir.Shared.Api.UnitTests/Microsoft.Health.Fhir.Shared.Api.UnitTests.projitems index c81e3fd16d..ac73328021 100644 --- a/src/Microsoft.Health.Fhir.Shared.Api.UnitTests/Microsoft.Health.Fhir.Shared.Api.UnitTests.projitems +++ b/src/Microsoft.Health.Fhir.Shared.Api.UnitTests/Microsoft.Health.Fhir.Shared.Api.UnitTests.projitems @@ -56,6 +56,7 @@ + diff --git a/src/Microsoft.Health.Fhir.Shared.Api/Controllers/FhirController.cs b/src/Microsoft.Health.Fhir.Shared.Api/Controllers/FhirController.cs index 5f18b6dab9..204606c1eb 100644 --- a/src/Microsoft.Health.Fhir.Shared.Api/Controllers/FhirController.cs +++ b/src/Microsoft.Health.Fhir.Shared.Api/Controllers/FhirController.cs @@ -31,6 +31,7 @@ using Microsoft.Health.Fhir.Api.Features.AnonymousOperations; using Microsoft.Health.Fhir.Api.Features.Filters; using Microsoft.Health.Fhir.Api.Features.Headers; +using Microsoft.Health.Fhir.Api.Features.Resources; using Microsoft.Health.Fhir.Api.Features.Routing; using Microsoft.Health.Fhir.Api.Models; using Microsoft.Health.Fhir.Core.Extensions; @@ -158,9 +159,8 @@ public IActionResult CustomError(int? statusCode = null) [ServiceFilter(typeof(SearchParameterFilterAttribute))] public async Task Create([FromBody] Resource resource) { - BundleResourceContext bundleResourceContext = GetBundleResourceContext(); RawResourceElement response = await _mediator.CreateResourceAsync( - new CreateResourceRequest(resource.ToResourceElement(), bundleResourceContext), + new CreateResourceRequest(resource.ToResourceElement(), GetBundleResourceContext()), HttpContext.RequestAborted); return FhirResult.Create(response, HttpStatusCode.Created) @@ -181,12 +181,13 @@ public async Task ConditionalCreate([FromBody] Resource resource) { StringValues conditionalCreateHeader = HttpContext.Request.Headers[KnownHeaders.IfNoneExist]; + SetupRequestContextWithConditionalQueryMaxParallelism(); + Tuple[] conditionalParameters = QueryHelpers.ParseQuery(conditionalCreateHeader) .SelectMany(query => query.Value, (query, value) => Tuple.Create(query.Key, value)).ToArray(); - BundleResourceContext bundleResourceContext = GetBundleResourceContext(); UpsertResourceResponse createResponse = await _mediator.Send( - new ConditionalCreateResourceRequest(resource.ToResourceElement(), conditionalParameters, bundleResourceContext), + new ConditionalCreateResourceRequest(resource.ToResourceElement(), conditionalParameters, GetBundleResourceContext()), HttpContext.RequestAborted); if (createResponse == null) @@ -213,9 +214,8 @@ public async Task ConditionalCreate([FromBody] Resource resource) [AuditEventType(AuditEventSubType.Update)] public async Task Update([FromBody] Resource resource, [ModelBinder(typeof(WeakETagBinder))] WeakETag ifMatchHeader) { - BundleResourceContext bundleResourceContext = GetBundleResourceContext(); SaveOutcome response = await _mediator.UpsertResourceAsync( - new UpsertResourceRequest(resource.ToResourceElement(), bundleResourceContext, ifMatchHeader), + new UpsertResourceRequest(resource.ToResourceElement(), GetBundleResourceContext(), ifMatchHeader), HttpContext.RequestAborted); return ToSaveOutcomeResult(response); @@ -230,11 +230,12 @@ public async Task Update([FromBody] Resource resource, [ModelBind [AuditEventType(AuditEventSubType.ConditionalUpdate)] public async Task ConditionalUpdate([FromBody] Resource resource) { + SetupRequestContextWithConditionalQueryMaxParallelism(); + IReadOnlyList> conditionalParameters = GetQueriesForSearch(); - BundleResourceContext bundleResourceContext = GetBundleResourceContext(); UpsertResourceResponse response = await _mediator.Send( - new ConditionalUpsertResourceRequest(resource.ToResourceElement(), conditionalParameters, bundleResourceContext), + new ConditionalUpsertResourceRequest(resource.ToResourceElement(), conditionalParameters, GetBundleResourceContext()), HttpContext.RequestAborted); SaveOutcome saveOutcome = response.Outcome; @@ -270,9 +271,8 @@ private FhirResult ToSaveOutcomeResult(SaveOutcome saveOutcome) [AuditEventType(AuditEventSubType.Read)] public async Task Read(string typeParameter, string idParameter) { - BundleResourceContext bundleResourceContext = GetBundleResourceContext(); RawResourceElement response = await _mediator.GetResourceAsync( - new GetResourceRequest(new ResourceKey(typeParameter, idParameter), bundleResourceContext), + new GetResourceRequest(new ResourceKey(typeParameter, idParameter), GetBundleResourceContext()), HttpContext.RequestAborted); return FhirResult.Create(response) @@ -365,9 +365,8 @@ public async Task History( [AuditEventType(AuditEventSubType.VRead)] public async Task VRead(string typeParameter, string idParameter, string vidParameter) { - BundleResourceContext bundleResourceContext = GetBundleResourceContext(); RawResourceElement response = await _mediator.GetResourceAsync( - new GetResourceRequest(new ResourceKey(typeParameter, idParameter, vidParameter), bundleResourceContext), + new GetResourceRequest(new ResourceKey(typeParameter, idParameter, vidParameter), GetBundleResourceContext()), HttpContext.RequestAborted); return FhirResult.Create(response, HttpStatusCode.OK) @@ -386,12 +385,11 @@ public async Task VRead(string typeParameter, string idParameter, [AuditEventType(AuditEventSubType.Delete)] public async Task Delete(string typeParameter, string idParameter, [FromQuery] bool hardDelete) { - BundleResourceContext bundleResourceContext = GetBundleResourceContext(); DeleteResourceResponse response = await _mediator.DeleteResourceAsync( new DeleteResourceRequest( new ResourceKey(typeParameter, idParameter), hardDelete ? DeleteOperation.HardDelete : DeleteOperation.SoftDelete, - bundleResourceContext), + GetBundleResourceContext()), HttpContext.RequestAborted); return FhirResult.NoContent().SetETagHeader(response.WeakETag); @@ -407,12 +405,11 @@ public async Task Delete(string typeParameter, string idParameter [AuditEventType(AuditEventSubType.PurgeHistory)] public async Task PurgeHistory(string typeParameter, string idParameter) { - BundleResourceContext bundleResourceContext = GetBundleResourceContext(); DeleteResourceResponse response = await _mediator.DeleteResourceAsync( new DeleteResourceRequest( new ResourceKey(typeParameter, idParameter), DeleteOperation.PurgeHistory, - bundleResourceContext), + GetBundleResourceContext()), HttpContext.RequestAborted); return FhirResult.NoContent().SetETagHeader(response.WeakETag); @@ -431,7 +428,7 @@ public async Task ConditionalDelete(string typeParameter, [FromQu { IReadOnlyList> conditionalParameters = GetQueriesForSearch(); - BundleResourceContext bundleResourceContext = GetBundleResourceContext(); + SetupRequestContextWithConditionalQueryMaxParallelism(); DeleteResourceResponse response = await _mediator.Send( new ConditionalDeleteResourceRequest( @@ -439,7 +436,7 @@ public async Task ConditionalDelete(string typeParameter, [FromQu conditionalParameters, hardDelete ? DeleteOperation.HardDelete : DeleteOperation.SoftDelete, maxDeleteCount.GetValueOrDefault(1), - bundleResourceContext), + GetBundleResourceContext()), HttpContext.RequestAborted); if (maxDeleteCount.HasValue) @@ -465,13 +462,11 @@ public async Task PatchJson(string typeParameter, string idParame { var payload = new JsonPatchPayload(patchDocument); - BundleResourceContext bundleResourceContext = GetBundleResourceContext(); - UpsertResourceResponse response = await _mediator.PatchResourceAsync( new PatchResourceRequest( new ResourceKey(typeParameter, idParameter), payload, - bundleResourceContext, + GetBundleResourceContext(), ifMatchHeader), HttpContext.RequestAborted); @@ -493,10 +488,10 @@ public async Task ConditionalPatchJson(string typeParameter, [Fro IReadOnlyList> conditionalParameters = GetQueriesForSearch(); var payload = new JsonPatchPayload(patchDocument); - BundleResourceContext bundleResourceContext = GetBundleResourceContext(); + SetupRequestContextWithConditionalQueryMaxParallelism(); UpsertResourceResponse response = await _mediator.ConditionalPatchResourceAsync( - new ConditionalPatchResourceRequest(typeParameter, payload, conditionalParameters, bundleResourceContext, ifMatchHeader), + new ConditionalPatchResourceRequest(typeParameter, payload, conditionalParameters, GetBundleResourceContext(), ifMatchHeader), HttpContext.RequestAborted); return ToSaveOutcomeResult(response.Outcome); } @@ -515,10 +510,9 @@ public async Task ConditionalPatchJson(string typeParameter, [Fro public async Task PatchFhir(string typeParameter, string idParameter, [FromBody] Parameters paramsResource, [ModelBinder(typeof(WeakETagBinder))] WeakETag ifMatchHeader) { var payload = new FhirPathPatchPayload(paramsResource); - BundleResourceContext bundleResourceContext = GetBundleResourceContext(); UpsertResourceResponse response = await _mediator.PatchResourceAsync( - new PatchResourceRequest(new ResourceKey(typeParameter, idParameter), payload, bundleResourceContext, ifMatchHeader), + new PatchResourceRequest(new ResourceKey(typeParameter, idParameter), payload, GetBundleResourceContext(), ifMatchHeader), HttpContext.RequestAborted); return ToSaveOutcomeResult(response.Outcome); } @@ -538,10 +532,10 @@ public async Task ConditionalPatchFhir(string typeParameter, [Fro IReadOnlyList> conditionalParameters = GetQueriesForSearch(); var payload = new FhirPathPatchPayload(paramsResource); - BundleResourceContext bundleResourceContext = GetBundleResourceContext(); + SetupRequestContextWithConditionalQueryMaxParallelism(); UpsertResourceResponse response = await _mediator.ConditionalPatchResourceAsync( - new ConditionalPatchResourceRequest(typeParameter, payload, conditionalParameters, bundleResourceContext, ifMatchHeader), + new ConditionalPatchResourceRequest(typeParameter, payload, conditionalParameters, GetBundleResourceContext(), ifMatchHeader), HttpContext.RequestAborted); return ToSaveOutcomeResult(response.Outcome); } @@ -661,7 +655,7 @@ public async Task BatchAndTransactions([FromBody] Resource bundle /// Returns an instance of with bundle related information, if a resource if part of a bundle. /// /// Returns null if the resource is not part of a bundle. - public BundleResourceContext GetBundleResourceContext() + private BundleResourceContext GetBundleResourceContext() { if (HttpContext?.Request?.Headers != null) { @@ -686,5 +680,18 @@ public BundleResourceContext GetBundleResourceContext() return null; } + + private void SetupRequestContextWithConditionalQueryMaxParallelism() + { + if (HttpContext?.Request?.Headers != null && _fhirRequestContextAccessor != null) + { + ConditionalQueryProcessingLogic processingLogic = HttpContext.GetConditionalQueryProcessingLogic(); + + if (processingLogic == ConditionalQueryProcessingLogic.Parallel) + { + _fhirRequestContextAccessor.RequestContext.DecorateRequestContextWithOptimizedConcurrency(); + } + } + } } } diff --git a/src/Microsoft.Health.Fhir.Shared.Api/Features/Headers/HttpContextExtensions.cs b/src/Microsoft.Health.Fhir.Shared.Api/Features/Headers/HttpContextExtensions.cs new file mode 100644 index 0000000000..11766b6ebd --- /dev/null +++ b/src/Microsoft.Health.Fhir.Shared.Api/Features/Headers/HttpContextExtensions.cs @@ -0,0 +1,91 @@ +// ------------------------------------------------------------------------------------------------- +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License (MIT). See LICENSE in the repo root for license information. +// ------------------------------------------------------------------------------------------------- + +using System; +using System.Collections.Generic; +using System.Linq; +using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.Primitives; +using Microsoft.Health.Fhir.Api.Features.Resources; +using Microsoft.Health.Fhir.Api.Features.Resources.Bundle; +using Microsoft.Health.Fhir.Core.Features; +using Microsoft.Health.Fhir.Core.Features.Context; +using Microsoft.Health.Fhir.Core.Features.Persistence.Orchestration; + +namespace Microsoft.Health.Fhir.Api.Features.Headers +{ + public static class HttpContextExtensions + { + /// + /// Retrieves from the HTTP header information about the conditional-query processing logic to be adopted. + /// + /// HTTP context + public static ConditionalQueryProcessingLogic GetConditionalQueryProcessingLogic(this HttpContext outerHttpContext) + { + var defaultValue = ConditionalQueryProcessingLogic.Sequential; + + if (outerHttpContext == null) + { + return defaultValue; + } + + if (outerHttpContext.Request.Headers.TryGetValue(KnownHeaders.ConditionalQueryProcessingLogic, out StringValues headerValues)) + { + string processingLogicAsString = headerValues.FirstOrDefault(); + if (string.IsNullOrWhiteSpace(processingLogicAsString)) + { + return defaultValue; + } + + ConditionalQueryProcessingLogic processingLogic = (ConditionalQueryProcessingLogic)Enum.Parse(typeof(ConditionalQueryProcessingLogic), processingLogicAsString.Trim(), ignoreCase: true); + return processingLogic; + } + + return defaultValue; + } + + /// + /// Retrieves from the HTTP header information about the bundle processing logic to be adopted. + /// + /// HTTP context + public static BundleProcessingLogic GetBundleProcessingLogic(this HttpContext outerHttpContext) + { + var defaultValue = BundleProcessingLogic.Sequential; + + if (outerHttpContext == null) + { + return defaultValue; + } + + if (outerHttpContext.Request.Headers.TryGetValue(BundleOrchestratorNamingConventions.HttpHeaderBundleProcessingLogic, out StringValues headerValues)) + { + string processingLogicAsString = headerValues.FirstOrDefault(); + if (string.IsNullOrWhiteSpace(processingLogicAsString)) + { + return defaultValue; + } + + BundleProcessingLogic processingLogic = (BundleProcessingLogic)Enum.Parse(typeof(BundleProcessingLogic), processingLogicAsString.Trim(), ignoreCase: true); + return processingLogic; + } + + return defaultValue; + } + + /// + /// Decorate FHIR Request Context with a property bag setting queries to use optimized concurrecy. + /// + /// FHIR request context + public static bool DecorateRequestContextWithOptimizedConcurrency(this IFhirRequestContext requestContext) + { + if (requestContext == null) + { + return false; + } + + return requestContext.Properties.TryAdd(KnownQueryParameterNames.OptimizeConcurrency, true); + } + } +} diff --git a/src/Microsoft.Health.Fhir.Shared.Api/Features/Resources/Bundle/BundleHandler.cs b/src/Microsoft.Health.Fhir.Shared.Api/Features/Resources/Bundle/BundleHandler.cs index d94e4fa43b..0e45425f7a 100644 --- a/src/Microsoft.Health.Fhir.Shared.Api/Features/Resources/Bundle/BundleHandler.cs +++ b/src/Microsoft.Health.Fhir.Shared.Api/Features/Resources/Bundle/BundleHandler.cs @@ -32,6 +32,7 @@ using Microsoft.Health.Fhir.Api.Features.Bundle; using Microsoft.Health.Fhir.Api.Features.ContentTypes; using Microsoft.Health.Fhir.Api.Features.Exceptions; +using Microsoft.Health.Fhir.Api.Features.Headers; #if !STU3 using Microsoft.Health.Fhir.Api.Features.Formatters; #endif @@ -84,16 +85,22 @@ public partial class BundleHandler : IRequestHandler - /// Headers to propagate the the from the inner actions to the outer HTTP request. + /// Headers to propagate from the inner actions to the outer HTTP request. /// private static readonly string[] HeadersToAccumulate = new[] { KnownHeaders.RetryAfter, KnownHeaders.RetryAfterMilliseconds, "x-ms-session-token", "x-ms-request-charge" }; + /// + /// Properties to propagate from the outer HTTP requests to the inner actions. + /// + private static readonly string[] PropertiesToAccumulate = new[] { KnownQueryParameterNames.OptimizeConcurrency }; + private IFhirRequestContext _originalFhirRequestContext; public BundleHandler( @@ -156,7 +163,11 @@ public BundleHandler( _emptyRequestsOrder = new List(); _referenceIdDictionary = new Dictionary(); + // Retrieve bundle processing logic. _bundleProcessingLogic = GetBundleProcessingLogic(outerHttpContext, _logger); + + // Set conditional-query processing logic. + _conditionalQueryProcessingLogic = SetRequestContextWithConditionalQueryProcessingLogic(outerHttpContext, fhirRequestContextAccessor.RequestContext, _logger); } public async Task Handle(BundleRequest request, CancellationToken cancellationToken) @@ -232,31 +243,40 @@ public async Task Handle(BundleRequest request, CancellationToke } } - private BundleProcessingLogic GetBundleProcessingLogic(HttpContext outerHttpContext, ILogger logger) + private static ConditionalQueryProcessingLogic SetRequestContextWithConditionalQueryProcessingLogic(HttpContext outerHttpContext, IFhirRequestContext fhirRequestContext, ILogger logger) { - if (outerHttpContext.Request.Headers.TryGetValue(BundleOrchestratorNamingConventions.HttpHeaderBundleProcessingLogic, out StringValues headerValues)) + try { - string processingLogicAsString = headerValues.FirstOrDefault(); - if (string.IsNullOrWhiteSpace(processingLogicAsString)) - { - return DefaultBundleProcessingLogic; - } + ConditionalQueryProcessingLogic conditionalQueryProcessingLogic = outerHttpContext.GetConditionalQueryProcessingLogic(); - try + if (conditionalQueryProcessingLogic == ConditionalQueryProcessingLogic.Parallel) { - BundleProcessingLogic processingLogic = (BundleProcessingLogic)Enum.Parse(typeof(BundleProcessingLogic), processingLogicAsString.Trim(), ignoreCase: true); - return processingLogic; + fhirRequestContext.DecorateRequestContextWithOptimizedConcurrency(); } - catch (Exception e) - { - _bundleProcessingTypeIsInvalid = true; - logger.LogWarning(e, "Error while extracting the Bundle Processing Logic out of the HTTP Header: {ErrorMessage}", e.Message); - return DefaultBundleProcessingLogic; - } + return conditionalQueryProcessingLogic; + } + catch (Exception e) + { + logger.LogWarning(e, "Error while extracting the Conditional-Query Processing Logic out of the HTTP Header: {ErrorMessage}", e.Message); } - return DefaultBundleProcessingLogic; + return ConditionalQueryProcessingLogic.Sequential; + } + + private BundleProcessingLogic GetBundleProcessingLogic(HttpContext outerHttpContext, ILogger logger) + { + try + { + return outerHttpContext.GetBundleProcessingLogic(); + } + catch (Exception e) + { + _bundleProcessingTypeIsInvalid = true; + logger.LogWarning(e, "Error while extracting the Bundle Processing Logic out of the HTTP Header: {ErrorMessage}", e.Message); + + return DefaultBundleProcessingLogic; + } } private async Task ProcessAllResourcesInABundleAsRequestsAsync(Hl7.Fhir.Model.Bundle responseBundle, BundleProcessingLogic processingLogic, CancellationToken cancellationToken) @@ -520,8 +540,7 @@ private async Task GenerateRequest(EntryComponent entry, int order, Cancellation AddHeaderIfNeeded(KnownHeaders.Prefer, preferValue, httpContext); } - if (requestMethod == HTTPVerb.POST - || requestMethod == HTTPVerb.PUT) + if (requestMethod == HTTPVerb.POST || requestMethod == HTTPVerb.PUT) { httpContext.Request.Headers[HeaderNames.ContentType] = new StringValues(KnownContentTypes.JsonContentType); @@ -605,7 +624,7 @@ private async Task ExecuteRequestsWithSingleHttpVerbInSequenceAs { HttpContext httpContext = resourceContext.Context.HttpContext; - SetupContexts(resourceContext.Context, httpContext); + SetupContexts(resourceContext, httpContext); Func originalResourceIdProvider = _resourceIdProvider.Create; @@ -733,42 +752,107 @@ private static EntryComponent CreateEntryComponent(FhirJsonParser fhirJsonParser return entryComponent; } - private void SetupContexts(RouteContext request, HttpContext httpContext) + /// + /// Reference implementation of 'SetupContexts'. Originally created to support sequential operations and run manipulations on local + /// attributes. This is a non-thread safe method. Not to be used in parallel-operation under the same HTTP request context. + /// + private void SetupContexts(ResourceExecutionContext resourceExecutionContext, HttpContext httpContext) + { + SetupContexts( + request: resourceExecutionContext.Context, + httpVerb: resourceExecutionContext.HttpVerb, + httpContext: httpContext, + bundleOrchestratorOperation: null, // Set to null because this is not running in the context of a parallel-bundle. + requestContext: _originalFhirRequestContext, + auditEventTypeMapping: _auditEventTypeMapping, + requestContextAccessor: _fhirRequestContextAccessor, + bundleHttpContextAccessor: _bundleHttpContextAccessor, + logger: _logger); + } + + /// + /// This method setups new FHIR Request Contexts for downstream requests created by during the bundle processing. + /// In this method, data in memory from HttpContext, RouteContext, RequestContext and RequestContextAcessor are set to be reused + /// by the nested requests. + /// + /// Static implementation of 'SetupContexts'. Originally created to support parallel operations and avoid the manipulation of local + /// attributes, that would cause non-thread safe issues. + /// + private static void SetupContexts( + RouteContext request, + HTTPVerb httpVerb, + HttpContext httpContext, + IBundleOrchestratorOperation bundleOrchestratorOperation, + IFhirRequestContext requestContext, + IAuditEventTypeMapping auditEventTypeMapping, + RequestContextAccessor requestContextAccessor, + IBundleHttpContextAccessor bundleHttpContextAccessor, + ILogger logger) { request.RouteData.Values.TryGetValue("controller", out object controllerName); request.RouteData.Values.TryGetValue("action", out object actionName); request.RouteData.Values.TryGetValue(KnownActionParameterNames.ResourceType, out object resourceType); + var newFhirRequestContext = new FhirRequestContext( httpContext.Request.Method, httpContext.Request.GetDisplayUrl(), - _originalFhirRequestContext.BaseUri.OriginalString, - _originalFhirRequestContext.CorrelationId, + requestContext.BaseUri.OriginalString, + requestContext.CorrelationId, httpContext.Request.Headers, httpContext.Response.Headers) { - Principal = _originalFhirRequestContext.Principal, + Principal = requestContext.Principal, ResourceType = resourceType?.ToString(), - AuditEventType = _auditEventTypeMapping.GetAuditEventType( + AuditEventType = auditEventTypeMapping.GetAuditEventType( controllerName?.ToString(), actionName?.ToString()), ExecutingBatchOrTransaction = true, - AccessControlContext = _originalFhirRequestContext.AccessControlContext.Clone() as AccessControlContext, + AccessControlContext = requestContext.AccessControlContext.Clone() as AccessControlContext, }; - foreach (var scopeRestriction in _originalFhirRequestContext.AccessControlContext.AllowedResourceActions) + + // Copy allowed resource actions to the new FHIR Request Context. + foreach (var scopeRestriction in requestContext.AccessControlContext.AllowedResourceActions) { newFhirRequestContext.AccessControlContext.AllowedResourceActions.Add(scopeRestriction); } - newFhirRequestContext.AccessControlContext.ApplyFineGrainedAccessControl = _originalFhirRequestContext.AccessControlContext.ApplyFineGrainedAccessControl; - _fhirRequestContextAccessor.RequestContext = newFhirRequestContext; + // Copy allowed properties from the existing FHIR Request Context property bag to the new FHIR Request Context. + if (requestContext.Properties.Any()) + { + foreach (string propertyName in PropertiesToAccumulate) + { + if (requestContext.Properties.TryGetValue(propertyName, out object value)) + { + newFhirRequestContext.Properties.Add(propertyName, value); + } + } + } - _bundleHttpContextAccessor.HttpContext = httpContext; + // Propagate Fine Grained Access Control to the new FHIR Request Context. + newFhirRequestContext.AccessControlContext.ApplyFineGrainedAccessControl = requestContext.AccessControlContext.ApplyFineGrainedAccessControl; - foreach (string headerName in HeadersToAccumulate) + // Bundle Orchestrator Operation should not be null for parallel-bundles. + if (bundleOrchestratorOperation != null) { - if (_originalFhirRequestContext.ResponseHeaders.TryGetValue(headerName, out var values)) + // Assign the current Bundle Orchestrator Operation ID as part of the downstream request. + newFhirRequestContext.RequestHeaders.Add(BundleOrchestratorNamingConventions.HttpHeaderOperationTag, bundleOrchestratorOperation.Id.ToString()); + + // Assign the HTTP Verb operation associated with the request as part of the downstream request. + newFhirRequestContext.RequestHeaders.Add(BundleOrchestratorNamingConventions.HttpHeaderBundleResourceHttpVerb, httpVerb.ToString()); + } + + requestContextAccessor.RequestContext = newFhirRequestContext; + bundleHttpContextAccessor.HttpContext = httpContext; + + // Copy allowed headers from the FHIR Request Context response header. + if (requestContext.ResponseHeaders.Any()) + { + foreach (string headerName in HeadersToAccumulate) { - newFhirRequestContext.ResponseHeaders.Add(headerName, values); + if (requestContext.ResponseHeaders.TryGetValue(headerName, out var values)) + { + newFhirRequestContext.ResponseHeaders.Add(headerName, values); + } } } } @@ -837,7 +921,7 @@ private static OperationOutcome CreateOperationOutcome(OperationOutcome.IssueSev private BundleHandlerStatistics CreateNewBundleHandlerStatistics(BundleProcessingLogic processingLogic) { - BundleHandlerStatistics statistics = new BundleHandlerStatistics(_bundleType, processingLogic, _requestCount); + BundleHandlerStatistics statistics = new BundleHandlerStatistics(_bundleType, processingLogic, _conditionalQueryProcessingLogic, _requestCount); statistics.StartCollectingResults(); diff --git a/src/Microsoft.Health.Fhir.Shared.Api/Features/Resources/Bundle/BundleHandlerParallelOperations.cs b/src/Microsoft.Health.Fhir.Shared.Api/Features/Resources/Bundle/BundleHandlerParallelOperations.cs index eba0a55b1d..eb0072e7f5 100644 --- a/src/Microsoft.Health.Fhir.Shared.Api/Features/Resources/Bundle/BundleHandlerParallelOperations.cs +++ b/src/Microsoft.Health.Fhir.Shared.Api/Features/Resources/Bundle/BundleHandlerParallelOperations.cs @@ -278,7 +278,16 @@ private static async Task HandleRequestAsync( resourceIdProvider.Create = () => persistedId; } - SetupContexts(request, httpVerb, httpContext, bundleOperation, originalFhirRequestContext, auditEventTypeMapping, requestContext, bundleHttpContextAccessor); + SetupContexts( + request, + httpVerb, + httpContext, + bundleOperation, + originalFhirRequestContext, + auditEventTypeMapping, + requestContext, + bundleHttpContextAccessor, + logger); // Attempt 1. await request.Handler.Invoke(httpContext); @@ -356,55 +365,6 @@ private static async Task HandleRequestAsync( return entryComponent; } - private static void SetupContexts( - RouteContext request, - HTTPVerb httpVerb, - HttpContext httpContext, - IBundleOrchestratorOperation bundleOperation, - IFhirRequestContext requestContext, - IAuditEventTypeMapping auditEventTypeMapping, - RequestContextAccessor requestContextAccessor, - IBundleHttpContextAccessor bundleHttpContextAccessor) - { - request.RouteData.Values.TryGetValue("controller", out object controllerName); - request.RouteData.Values.TryGetValue("action", out object actionName); - request.RouteData.Values.TryGetValue(KnownActionParameterNames.ResourceType, out object resourceType); - - var newFhirRequestContext = new FhirRequestContext( - httpContext.Request.Method, - httpContext.Request.GetDisplayUrl(), - requestContext.BaseUri.OriginalString, - requestContext.CorrelationId, - httpContext.Request.Headers, - httpContext.Response.Headers) - { - Principal = requestContext.Principal, - ResourceType = resourceType?.ToString(), - AuditEventType = auditEventTypeMapping.GetAuditEventType( - controllerName?.ToString(), - actionName?.ToString()), - ExecutingBatchOrTransaction = true, - }; - - // Assign the current Bundle Orchestrator Operation ID as part of the downstream request. - newFhirRequestContext.RequestHeaders.Add(BundleOrchestratorNamingConventions.HttpHeaderOperationTag, bundleOperation.Id.ToString()); - - // Assign the HTTP Verb operation associated with the request as part of the downstream request. - newFhirRequestContext.RequestHeaders.Add(BundleOrchestratorNamingConventions.HttpHeaderBundleResourceHttpVerb, httpVerb.ToString()); - - requestContextAccessor.RequestContext = newFhirRequestContext; - - bundleHttpContextAccessor.HttpContext = httpContext; - - foreach (string headerName in HeadersToAccumulate) - { - if (requestContext.ResponseHeaders.TryGetValue(headerName, out var values)) - { - newFhirRequestContext.ResponseHeaders.Add(headerName, values); - } - } - } - private struct ResourceExecutionContext { public ResourceExecutionContext(HTTPVerb httpVerb, RouteContext context, int index, string persistedId) diff --git a/src/Microsoft.Health.Fhir.Shared.Api/Features/Resources/Bundle/BundleHandlerStatistics.cs b/src/Microsoft.Health.Fhir.Shared.Api/Features/Resources/Bundle/BundleHandlerStatistics.cs index e4fa5f7999..0805034d72 100644 --- a/src/Microsoft.Health.Fhir.Shared.Api/Features/Resources/Bundle/BundleHandlerStatistics.cs +++ b/src/Microsoft.Health.Fhir.Shared.Api/Features/Resources/Bundle/BundleHandlerStatistics.cs @@ -21,11 +21,16 @@ internal sealed class BundleHandlerStatistics : BaseOperationStatistics private readonly List _entries; - public BundleHandlerStatistics(BundleType? bundleType, BundleProcessingLogic processingLogic, int numberOfResources) + public BundleHandlerStatistics( + BundleType? bundleType, + BundleProcessingLogic bundleProcessingLogic, + ConditionalQueryProcessingLogic conditionalQueryProcessingLogic, + int numberOfResources) : base() { BundleType = bundleType; - ProcessingLogic = processingLogic; + BundleProcessingLogic = bundleProcessingLogic; + ConditionalQueryProcessingLogic = conditionalQueryProcessingLogic; NumberOfResources = numberOfResources; _entries = new List(); } @@ -34,7 +39,9 @@ public BundleHandlerStatistics(BundleType? bundleType, BundleProcessingLogic pro public BundleType? BundleType { get; } - public BundleProcessingLogic ProcessingLogic { get; } + public BundleProcessingLogic BundleProcessingLogic { get; } + + public ConditionalQueryProcessingLogic ConditionalQueryProcessingLogic { get; } public override string GetLoggingCategory() => LoggingCategory; @@ -53,7 +60,8 @@ public override string GetStatisticsAsJson() { label = GetLoggingCategory(), bundleType = BundleType.ToString(), - processingLogic = ProcessingLogic.ToString(), + processingLogic = BundleProcessingLogic.ToString(), + conditionalQuery = ConditionalQueryProcessingLogic.ToString(), numberOfResources = NumberOfResources, executionTime = ElapsedMilliseconds, success = successedRequests, diff --git a/src/Microsoft.Health.Fhir.Shared.Api/Features/Resources/ConditionalQueryProcessingLogic.cs b/src/Microsoft.Health.Fhir.Shared.Api/Features/Resources/ConditionalQueryProcessingLogic.cs new file mode 100644 index 0000000000..71d36b8afc --- /dev/null +++ b/src/Microsoft.Health.Fhir.Shared.Api/Features/Resources/ConditionalQueryProcessingLogic.cs @@ -0,0 +1,13 @@ +// ------------------------------------------------------------------------------------------------- +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License (MIT). See LICENSE in the repo root for license information. +// ------------------------------------------------------------------------------------------------- + +namespace Microsoft.Health.Fhir.Api.Features.Resources +{ + public enum ConditionalQueryProcessingLogic + { + Sequential = 0, + Parallel = 1, + } +} diff --git a/src/Microsoft.Health.Fhir.Shared.Api/Microsoft.Health.Fhir.Shared.Api.projitems b/src/Microsoft.Health.Fhir.Shared.Api/Microsoft.Health.Fhir.Shared.Api.projitems index d8ab1b2e4c..6c96fc1e58 100644 --- a/src/Microsoft.Health.Fhir.Shared.Api/Microsoft.Health.Fhir.Shared.Api.projitems +++ b/src/Microsoft.Health.Fhir.Shared.Api/Microsoft.Health.Fhir.Shared.Api.projitems @@ -29,6 +29,7 @@ + @@ -36,6 +37,7 @@ + diff --git a/src/Microsoft.Health.Fhir.Shared.Client/FhirBundleOptions.cs b/src/Microsoft.Health.Fhir.Shared.Client/FhirBundleOptions.cs new file mode 100644 index 0000000000..25b26d7bc8 --- /dev/null +++ b/src/Microsoft.Health.Fhir.Shared.Client/FhirBundleOptions.cs @@ -0,0 +1,32 @@ +// ------------------------------------------------------------------------------------------------- +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License (MIT). See LICENSE in the repo root for license information. +// ------------------------------------------------------------------------------------------------- + +using System; +using System.Collections.Generic; +using System.Text; +using Azure.Identity; + +namespace Microsoft.Health.Fhir.Client +{ + public sealed class FhirBundleOptions + { +#pragma warning disable IDE1006 // Naming Styles + public static readonly FhirBundleOptions Default = new(); +#pragma warning restore IDE1006 // Naming Styles + + public FhirBundleOptions() + { + BundleProcessingLogic = FhirBundleProcessingLogic.Parallel; + MaximizeConditionalQueryParallelism = false; + ProfileValidation = false; + } + + public FhirBundleProcessingLogic BundleProcessingLogic { get; set; } + + public bool MaximizeConditionalQueryParallelism { get; set; } + + public bool ProfileValidation { get; set; } + } +} diff --git a/src/Microsoft.Health.Fhir.Shared.Client/FhirClient.cs b/src/Microsoft.Health.Fhir.Shared.Client/FhirClient.cs index 5f80cff79c..8682a55be5 100644 --- a/src/Microsoft.Health.Fhir.Shared.Client/FhirClient.cs +++ b/src/Microsoft.Health.Fhir.Shared.Client/FhirClient.cs @@ -23,6 +23,7 @@ namespace Microsoft.Health.Fhir.Client public class FhirClient : IFhirClient { private const string BundleProcessingLogicHeader = "x-bundle-processing-logic"; + private const string ConditionalQueryProcessingLogicHeader = "x-conditionalquery-processing-logic"; private const string IfNoneExistHeaderName = "If-None-Exist"; private const string ProvenanceHeader = "X-Provenance"; private const string IfMatchHeaderName = "If-Match"; @@ -481,32 +482,34 @@ public async Task CheckImportAsync(Uri contentLocation, Can return response; } - public async Task> PostBundleAsync(Resource bundle, FhirBundleProcessingLogic processingLogic = FhirBundleProcessingLogic.Parallel, CancellationToken cancellationToken = default) + public async Task> PostBundleAsync(Resource bundle, FhirBundleOptions bundleOptions = default, CancellationToken cancellationToken = default) { + if (bundleOptions == null) + { + bundleOptions = FhirBundleOptions.Default; + } + using var message = new HttpRequestMessage(HttpMethod.Post, string.Empty) { Content = CreateStringContent(bundle), }; message.Headers.Accept.Add(_mediaType); - message.Headers.Add(BundleProcessingLogicHeader, processingLogic.ToString()); - - using HttpResponseMessage response = await HttpClient.SendAsync(message, cancellationToken); - await EnsureSuccessStatusCodeAsync(response); + // Profile validation. + if (bundleOptions.ProfileValidation) + { + message.Headers.Add(ProfileValidation, bundleOptions.ProfileValidation.ToString()); + } - return await CreateResponseAsync(response); - } + // Bundle processing logic (parallel or sequential). + message.Headers.Add(BundleProcessingLogicHeader, bundleOptions.BundleProcessingLogic.ToString()); - public async Task> PostBundleWithValidationHeaderAsync(Resource bundle, bool profileValidation, FhirBundleProcessingLogic processingLogic = FhirBundleProcessingLogic.Parallel, CancellationToken cancellationToken = default) - { - using var message = new HttpRequestMessage(HttpMethod.Post, string.Empty) + // Conditional query processing logic. + if (bundleOptions.MaximizeConditionalQueryParallelism) { - Content = CreateStringContent(bundle), - }; - message.Headers.Add(ProfileValidation, profileValidation.ToString()); - message.Headers.Accept.Add(_mediaType); - message.Headers.Add(BundleProcessingLogicHeader, processingLogic.ToString()); + message.Headers.Add(ConditionalQueryProcessingLogicHeader, "parallel"); + } using HttpResponseMessage response = await HttpClient.SendAsync(message, cancellationToken); diff --git a/src/Microsoft.Health.Fhir.Shared.Client/IFhirClient.cs b/src/Microsoft.Health.Fhir.Shared.Client/IFhirClient.cs index 5e1e137507..687eb5a3ef 100644 --- a/src/Microsoft.Health.Fhir.Shared.Client/IFhirClient.cs +++ b/src/Microsoft.Health.Fhir.Shared.Client/IFhirClient.cs @@ -51,9 +51,7 @@ Task> FhirPatchAsync(T resource, Parameters patchRequest, str Task> ConditionalFhirPatchAsync(string resourceType, string searchCriteria, Parameters patchRequest, string ifMatchVersion = null, CancellationToken cancellationToken = default) where T : Resource; - Task> PostBundleAsync(Resource bundle, FhirBundleProcessingLogic processingLogic = FhirBundleProcessingLogic.Sequential, CancellationToken cancellationToken = default); - - Task> PostBundleWithValidationHeaderAsync(Resource bundle, bool profileValidation, FhirBundleProcessingLogic processingLogic = FhirBundleProcessingLogic.Sequential, CancellationToken cancellationToken = default); + Task> PostBundleAsync(Resource bundle, FhirBundleOptions bundleOptions = default, CancellationToken cancellationToken = default); Task> ReadAsync(ResourceType resourceType, string resourceId, CancellationToken cancellationToken = default) where T : Resource; diff --git a/src/Microsoft.Health.Fhir.Shared.Client/Microsoft.Health.Fhir.Shared.Client.projitems b/src/Microsoft.Health.Fhir.Shared.Client/Microsoft.Health.Fhir.Shared.Client.projitems index e9a2e385df..61d2185034 100644 --- a/src/Microsoft.Health.Fhir.Shared.Client/Microsoft.Health.Fhir.Shared.Client.projitems +++ b/src/Microsoft.Health.Fhir.Shared.Client/Microsoft.Health.Fhir.Shared.Client.projitems @@ -9,6 +9,7 @@ Microsoft.Health.Fhir.Shared.Client + diff --git a/src/Microsoft.Health.Fhir.Shared.Core.UnitTests/Features/Resources/ResourceHandlerTests_ConditionalDelete.cs b/src/Microsoft.Health.Fhir.Shared.Core.UnitTests/Features/Resources/ResourceHandlerTests_ConditionalDelete.cs index 000fd7da87..38a725887e 100644 --- a/src/Microsoft.Health.Fhir.Shared.Core.UnitTests/Features/Resources/ResourceHandlerTests_ConditionalDelete.cs +++ b/src/Microsoft.Health.Fhir.Shared.Core.UnitTests/Features/Resources/ResourceHandlerTests_ConditionalDelete.cs @@ -40,7 +40,12 @@ public async Task GivenOneMatchingResource_WhenDeletingConditionally_TheServerSh { var mockResultEntry = new SearchResultEntry(CreateMockResourceWrapper(Samples.GetDefaultObservation().UpdateId(Guid.NewGuid().ToString()), false)); - ConditionalDeleteResourceRequest message = SetupConditionalDelete(KnownResourceTypes.Observation, DefaultSearchParams, false, 1, mockResultEntry); + ConditionalDeleteResourceRequest message = SetupConditionalDelete( + KnownResourceTypes.Observation, + DefaultSearchParams, + hardDelete: false, + count: 1, + mockResultEntry); DeleteResourceResponse result = await _mediator.Send(message); @@ -55,7 +60,12 @@ public async Task GivenOneMatchingResource_WhenDeletingConditionallyWithHardDele { var mockResultEntry = new SearchResultEntry(CreateMockResourceWrapper(Samples.GetDefaultObservation().UpdateId(Guid.NewGuid().ToString()), false)); - ConditionalDeleteResourceRequest message = SetupConditionalDelete(KnownResourceTypes.Observation, DefaultSearchParams, true, 1, mockResultEntry); + ConditionalDeleteResourceRequest message = SetupConditionalDelete( + KnownResourceTypes.Observation, + DefaultSearchParams, + hardDelete: true, + count: 1, + mockResultEntry); DeleteResourceResponse result = await _mediator.Send(message); @@ -126,7 +136,12 @@ private ConditionalDeleteResourceRequest SetupConditionalDelete( .Returns(x => new UpsertOutcome(x.ArgAt(0).Wrapper, SaveOutcomeType.Updated)); } - var message = new ConditionalDeleteResourceRequest(resourceType, list, hardDelete ? DeleteOperation.HardDelete : DeleteOperation.SoftDelete, count, bundleResourceContext: null); + var message = new ConditionalDeleteResourceRequest( + resourceType, + list, + hardDelete ? DeleteOperation.HardDelete : DeleteOperation.SoftDelete, + count, + bundleResourceContext: null); return message; } diff --git a/src/Microsoft.Health.Fhir.Shared.Core/Features/Search/SearchOptionsFactory.cs b/src/Microsoft.Health.Fhir.Shared.Core/Features/Search/SearchOptionsFactory.cs index 9e990fbe9d..5d128bbd42 100644 --- a/src/Microsoft.Health.Fhir.Shared.Core/Features/Search/SearchOptionsFactory.cs +++ b/src/Microsoft.Health.Fhir.Shared.Core/Features/Search/SearchOptionsFactory.cs @@ -40,7 +40,7 @@ public class SearchOptionsFactory : ISearchOptionsFactory private readonly ILogger _logger; private readonly SearchParameterInfo _resourceTypeSearchParameter; private readonly CoreFeatureConfiguration _featureConfiguration; - private readonly List _timeTravelParameterNames = new() { KnownQueryParameterNames.GlobalEndSurrogateId, KnownQueryParameterNames.EndSurrogateId, KnownQueryParameterNames.GlobalStartSurrogateId, KnownQueryParameterNames.StartSurrogateId }; + private readonly HashSet _queryHintParameterNames = new() { KnownQueryParameterNames.GlobalEndSurrogateId, KnownQueryParameterNames.EndSurrogateId, KnownQueryParameterNames.GlobalStartSurrogateId, KnownQueryParameterNames.StartSurrogateId }; public SearchOptionsFactory( IExpressionParser expressionParser, @@ -90,7 +90,8 @@ public SearchOptions Create( if (queryParameters != null && queryParameters.Any(_ => _.Item1 == KnownQueryParameterNames.GlobalEndSurrogateId && _.Item2 != null)) { var queryHint = new List<(string param, string value)>(); - foreach (var par in queryParameters.Where(_ => _.Item1 == KnownQueryParameterNames.Type || _timeTravelParameterNames.Contains(_.Item1))) + + foreach (var par in queryParameters.Where(x => x.Item1 == KnownQueryParameterNames.Type || _queryHintParameterNames.Contains(x.Item1))) { queryHint.Add((par.Item1, par.Item2)); } @@ -108,7 +109,7 @@ public SearchOptions Create( // Extract the continuation token, filter out the other known query parameters that's not search related. // Exclude time travel parameters from evaluation to avoid warnings about unsupported parameters - foreach (Tuple query in queryParameters?.Where(_ => !_timeTravelParameterNames.Contains(_.Item1)) ?? Enumerable.Empty>()) + foreach (Tuple query in queryParameters?.Where(_ => !_queryHintParameterNames.Contains(_.Item1)) ?? Enumerable.Empty>()) { if (query.Item1 == KnownQueryParameterNames.ContinuationToken) { diff --git a/src/Microsoft.Health.Fhir.Tests.Common/Microsoft.Health.Fhir.Tests.Common.csproj b/src/Microsoft.Health.Fhir.Tests.Common/Microsoft.Health.Fhir.Tests.Common.csproj index 2a5ed37d84..2d2bf1dff5 100644 --- a/src/Microsoft.Health.Fhir.Tests.Common/Microsoft.Health.Fhir.Tests.Common.csproj +++ b/src/Microsoft.Health.Fhir.Tests.Common/Microsoft.Health.Fhir.Tests.Common.csproj @@ -13,13 +13,15 @@ $(NoWarn);NU5104;NU5100 + - + + diff --git a/src/Microsoft.Health.Fhir.Tests.Common/TestFiles/Normative/Bundle-BatchWithConditionalUpdateByIdentifier.json b/src/Microsoft.Health.Fhir.Tests.Common/TestFiles/Normative/Bundle-BatchWithConditionalUpdateByIdentifier.json new file mode 100644 index 0000000000..090acb91c5 --- /dev/null +++ b/src/Microsoft.Health.Fhir.Tests.Common/TestFiles/Normative/Bundle-BatchWithConditionalUpdateByIdentifier.json @@ -0,0 +1,70 @@ +{ + "resourceType": "Bundle", + "type": "batch", + "entry": [ + { + "fullUrl": "urn:uuid:6fc92493-6bef-402a-bbba-d949f3812cc8", + "resource": { + "resourceType": "Patient", + "identifier": [ + { + "system": "", + "value": "11788759296811" + } + ] + }, + "request": { + "method": "PUT", + "url": "Patient?identifier=|11788759296811" + } + }, + { + "fullUrl": "urn:uuid:9B0ACF58-CBA0-4C52-89F0-FC6C97A2D7DC", + "resource": { + "resourceType": "Patient", + "identifier": [ + { + "system": "", + "value": "11788759296812" + } + ] + }, + "request": { + "method": "PUT", + "url": "Patient?identifier=|11788759296812" + } + }, + { + "fullUrl": "urn:uuid:D0A4C3C0-0DE9-4AED-805E-8AC662ED9E21", + "resource": { + "resourceType": "Patient", + "identifier": [ + { + "system": "", + "value": "11788759296813" + } + ] + }, + "request": { + "method": "PUT", + "url": "Patient?identifier=|11788759296813" + } + }, + { + "fullUrl": "urn:uuid:11435F74-B0E9-4481-AC2D-E2D5F8202A7F", + "resource": { + "resourceType": "Patient", + "identifier": [ + { + "system": "", + "value": "11788759296814" + } + ] + }, + "request": { + "method": "PUT", + "url": "Patient?identifier=|11788759296814" + } + } + ] +} diff --git a/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Microsoft.Health.Fhir.Shared.Tests.E2E.projitems b/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Microsoft.Health.Fhir.Shared.Tests.E2E.projitems index 6d360d6ca6..a7af3d2e13 100644 --- a/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Microsoft.Health.Fhir.Shared.Tests.E2E.projitems +++ b/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Microsoft.Health.Fhir.Shared.Tests.E2E.projitems @@ -14,6 +14,8 @@ + + @@ -24,7 +26,7 @@ - + @@ -121,7 +123,6 @@ - diff --git a/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/BatchTests.cs b/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/BundleBatchTests.cs similarity index 90% rename from test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/BatchTests.cs rename to test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/BundleBatchTests.cs index ab9cb445c8..9009195db1 100644 --- a/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/BatchTests.cs +++ b/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/BundleBatchTests.cs @@ -26,7 +26,7 @@ namespace Microsoft.Health.Fhir.Tests.E2E.Rest [Trait(Traits.OwningTeam, OwningTeam.Fhir)] [Trait(Traits.Category, Categories.Bundle)] [HttpIntegrationFixtureArgumentSets(DataStore.All, Format.All)] - public class BatchTests : IClassFixture + public class BundleBatchTests : IClassFixture { private readonly TestFhirClient _client; private readonly Dictionary _statusCodeMap = new Dictionary() @@ -39,7 +39,7 @@ public class BatchTests : IClassFixture { HttpStatusCode.Forbidden, "403" }, }; - public BatchTests(HttpIntegrationTestFixture fixture) + public BundleBatchTests(HttpIntegrationTestFixture fixture) { _client = fixture.TestFhirClient; } @@ -58,7 +58,7 @@ public async Task GivenAValidBundle_WhenSubmittingABatch_ThenSuccessIsReturnedFo await _client.UpdateAsync(requestBundle.Entry[1].Resource as Patient, cancellationToken: source.Token); - using FhirResponse fhirResponse = await _client.PostBundleAsync(requestBundle, processingLogic: processingLogic, source.Token); + using FhirResponse fhirResponse = await _client.PostBundleAsync(requestBundle, new FhirBundleOptions() { BundleProcessingLogic = processingLogic }, source.Token); Assert.NotNull(fhirResponse); Assert.Equal(HttpStatusCode.OK, fhirResponse.StatusCode); @@ -109,7 +109,7 @@ public async Task GivenAValidBundle_WhenSubmittingABatchTwiceWithAndWithoutChang var requestBundle = Samples.GetDefaultBatch().ToPoco(); using FhirResponse fhirResponse = await _client.PostBundleAsync( requestBundle, - processingLogic: processingLogic); + new FhirBundleOptions() { BundleProcessingLogic = processingLogic }); Assert.NotNull(fhirResponse); Assert.Equal(HttpStatusCode.OK, fhirResponse.StatusCode); @@ -131,7 +131,7 @@ public async Task GivenAValidBundle_WhenSubmittingABatchTwiceWithAndWithoutChang // WhenSubmittingABatchTwiceWithNoDataChange_ThenServerShouldNotCreateAVersionSecondTimeAndSendOk using FhirResponse fhirResponseAfterPostingSameBundle = await _client.PostBundleAsync( requestBundle, - processingLogic); + new FhirBundleOptions() { BundleProcessingLogic = processingLogic }); Assert.NotNull(fhirResponseAfterPostingSameBundle); Assert.Equal(HttpStatusCode.OK, fhirResponseAfterPostingSameBundle.StatusCode); @@ -163,7 +163,7 @@ public async Task GivenAValidBundleWithReadonlyUser_WhenSubmittingABatch_ThenFor TestFhirClient tempClient = _client.CreateClientForUser(TestUsers.ReadOnlyUser, TestApplications.NativeClient); Bundle requestBundle = Samples.GetDefaultBatch().ToPoco(); - using FhirResponse fhirResponse = await tempClient.PostBundleAsync(requestBundle, processingLogic); + using FhirResponse fhirResponse = await tempClient.PostBundleAsync(requestBundle, new FhirBundleOptions() { BundleProcessingLogic = processingLogic }); Assert.NotNull(fhirResponse); Assert.Equal(HttpStatusCode.OK, fhirResponse.StatusCode); @@ -190,7 +190,9 @@ public async Task GivenAValidBundleWithReadonlyUser_WhenSubmittingABatch_ThenFor [InlineData(FhirBundleProcessingLogic.Sequential)] public async Task GivenANonBundleResource_WhenSubmittingABatch_ThenBadRequestIsReturned(FhirBundleProcessingLogic processingLogic) { - using FhirClientException ex = await Assert.ThrowsAsync(() => _client.PostBundleAsync(Samples.GetDefaultObservation().ToPoco(), processingLogic)); + using FhirClientException ex = await Assert.ThrowsAsync(() => _client.PostBundleAsync( + Samples.GetDefaultObservation().ToPoco(), + new FhirBundleOptions() { BundleProcessingLogic = processingLogic })); Assert.Equal(HttpStatusCode.BadRequest, ex.StatusCode); } @@ -201,7 +203,9 @@ public async Task GivenANonBundleResource_WhenSubmittingABatch_ThenBadRequestIsR [InlineData(FhirBundleProcessingLogic.Sequential)] public async Task GivenBundleTypeIsMissing_WhenSubmittingABundle_ThenMethodNotAllowedExceptionIsReturned(FhirBundleProcessingLogic processingLogic) { - using FhirClientException ex = await Assert.ThrowsAsync(() => _client.PostBundleAsync(Samples.GetJsonSample("Bundle-TypeMissing").ToPoco(), processingLogic)); + using FhirClientException ex = await Assert.ThrowsAsync(() => _client.PostBundleAsync( + Samples.GetJsonSample("Bundle-TypeMissing").ToPoco(), + new FhirBundleOptions() { BundleProcessingLogic = processingLogic })); ValidateOperationOutcome(ex.StatusCode.ToString(), ex.OperationOutcome, "MethodNotAllowed", "Bundle type is not present. Possible values are: transaction or batch", IssueType.Forbidden); } @@ -212,6 +216,8 @@ public async Task GivenBundleTypeIsMissing_WhenSubmittingABundle_ThenMethodNotAl [InlineData(false, FhirBundleProcessingLogic.Sequential)] public async Task GivenABatchBundle_WithProfileValidationFlag_ReturnsABundleResponse(bool profileValidation, FhirBundleProcessingLogic processingLogic) { + // This test is flaky when executed locally, but it works well when running in OSS pipeline. + var bundle = new Hl7.Fhir.Model.Bundle { Type = Bundle.BundleType.Batch, @@ -247,9 +253,16 @@ public async Task GivenABatchBundle_WithProfileValidationFlag_ReturnsABundleResp }, }; - using FhirResponse fhirResponse = await _client.PostBundleWithValidationHeaderAsync(bundle, profileValidation, processingLogic: processingLogic); + var bundleOptions = new FhirBundleOptions() + { + ProfileValidation = profileValidation, + BundleProcessingLogic = processingLogic, + }; + using FhirResponse fhirResponse = await _client.PostBundleAsync(bundle, bundleOptions); + Assert.NotNull(fhirResponse); Assert.Equal(HttpStatusCode.OK, fhirResponse.StatusCode); + Bundle bundleResource = fhirResponse.Resource; if (profileValidation) @@ -311,12 +324,22 @@ public async Task GivenATransactionBundle_WithProfileValidationFlag_ReturnsABund if (profileValidation) { - using FhirClientException ex = await Assert.ThrowsAsync(async () => await _client.PostBundleWithValidationHeaderAsync(bundle, profileValidation, processingLogic)); + var bundleOptions = new FhirBundleOptions() + { + ProfileValidation = profileValidation, + BundleProcessingLogic = processingLogic, + }; + using FhirClientException ex = await Assert.ThrowsAsync(async () => await _client.PostBundleAsync(bundle, bundleOptions)); Assert.Equal(HttpStatusCode.BadRequest, ex.StatusCode); } else { - using FhirResponse fhirResponse = await _client.PostBundleWithValidationHeaderAsync(bundle, false, processingLogic); + var bundleOptions = new FhirBundleOptions() + { + ProfileValidation = false, + BundleProcessingLogic = processingLogic, + }; + using FhirResponse fhirResponse = await _client.PostBundleAsync(bundle, bundleOptions); Assert.NotNull(fhirResponse); Assert.Equal(HttpStatusCode.OK, fhirResponse.StatusCode); diff --git a/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/BundleEdgeCaseTests.cs b/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/BundleEdgeCaseTests.cs new file mode 100644 index 0000000000..e1b9c9aebf --- /dev/null +++ b/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/BundleEdgeCaseTests.cs @@ -0,0 +1,95 @@ +// ------------------------------------------------------------------------------------------------- +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License (MIT). See LICENSE in the repo root for license information. +// ------------------------------------------------------------------------------------------------- + +using System; +using System.Linq; +using Hl7.Fhir.Model; +using Microsoft.Azure.Cosmos.Linq; +using Microsoft.Health.Fhir.Client; +using Microsoft.Health.Fhir.Core.Extensions; +using Microsoft.Health.Fhir.Tests.Common; +using Microsoft.Health.Fhir.Tests.Common.FixtureParameters; +using Microsoft.Health.Fhir.Tests.E2E.Common; +using Microsoft.Health.Test.Utilities; +using Xunit; +using Task = System.Threading.Tasks.Task; + +namespace Microsoft.Health.Fhir.Tests.E2E.Rest +{ + [Trait(Traits.OwningTeam, OwningTeam.Fhir)] + [Trait(Traits.Category, Categories.Bundle)] + [HttpIntegrationFixtureArgumentSets(DataStore.CosmosDb, Format.All)] + public class BundleEdgeCaseTests : IClassFixture + { + private readonly TestFhirClient _client; + + public BundleEdgeCaseTests(HttpIntegrationTestFixture fixture) + { + _client = fixture.TestFhirClient; + } + + [Fact] + [Trait(Traits.Priority, Priority.One)] + public async Task GivenABundleWithConditionalUpdateByReference_WhenExecutedWithMaximizedConditionalQueryParallelism_RunsTheQueryInParallelOnCosmosDb() + { + // #conditionalQueryParallelism + + var bundleOptions = new FhirBundleOptions() { MaximizeConditionalQueryParallelism = true, BundleProcessingLogic = FhirBundleProcessingLogic.Parallel }; + + // 1 - Retrieve bundle template from file. + var bundleWithConditionalReference = Samples.GetJsonSample("Bundle-BatchWithConditionalUpdateByIdentifier"); + var bundle = bundleWithConditionalReference.ToPoco(); + + // 2 - Update identifiers with new unique IDs. + Patient[] patients = new Patient[bundle.Entry.Count]; + for (int i = 0; i < bundle.Entry.Count; i++) + { + var patient = bundle.Entry[i].Resource.ToResourceElement().ToPoco(); + var patientIdentifier = Guid.NewGuid().ToString(); + + patient.Identifier.First().Value = patientIdentifier; + + bundle.Entry[i].Request.Url = $"Patient?identifier=|{patientIdentifier}"; + + patients[i] = patient; + } + + // 3 - Submit bundle to create the first version of all resources. + FhirResponse bundleResponse1 = await _client.PostBundleAsync(bundle, bundleOptions); + + // 4 - Retrieve the auto-generate ID of all resources. + var autoGeneratedPatientIds = bundleResponse1.Resource.Entry.Select(x => x.Resource).Select(x => x.Id).ToArray(); + + // 5 - Update the resources in the bundle to force the creation of a new version. + foreach (var patient in patients) + { + patient.Text = new Narrative + { + Status = Narrative.NarrativeStatus.Generated, + Div = $"
Content Updated
", + }; + } + + // 6 - Submit the original bundle once more to force: + // * Conditional-queries to scan resources by identifier. + // * As this's the original bundle, the resources in the bundle do not have the auto-generate IDs, only the identifiers, which will force the conditional-update based on an identifier. + FhirResponse bundleResponse2 = await _client.PostBundleAsync(bundle, bundleOptions); + + // 7 - Final asserts: + // * Assert if the sequence of patients is the same as in the original bundle. + // * Assert if the identifier in the bundle is the same as the identifier returned by the FHIR service. + // * Assert if a new resource version was created (it's supposed to be 2 to all resources in the bundle). + for (int i = 0; i < autoGeneratedPatientIds.Count(); i++) + { + string localResourceIdentier = patients[i].Identifier.First().Value; + string remoteResourceIdentifier = bundleResponse2.Resource.Entry[i].Resource.ToResourceElement().ToPoco().Identifier.First().Value; + + Assert.Equal(autoGeneratedPatientIds[i], bundleResponse2.Resource.Entry[i].Resource.Id); + Assert.Equal(localResourceIdentier, remoteResourceIdentifier); + Assert.Equal("2", bundleResponse2.Resource.Entry[i].Resource.Meta.VersionId); + } + } + } +} diff --git a/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/TransactionTests.cs b/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/BundleTransactionTests.cs similarity index 97% rename from test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/TransactionTests.cs rename to test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/BundleTransactionTests.cs index 4c96524137..534d7fb18a 100644 --- a/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/TransactionTests.cs +++ b/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/BundleTransactionTests.cs @@ -27,11 +27,11 @@ namespace Microsoft.Health.Fhir.Tests.E2E.Rest [Trait(Traits.Category, Categories.Search)] [Trait(Traits.Category, Categories.Transaction)] [HttpIntegrationFixtureArgumentSets(DataStore.SqlServer, Format.All)] - public class TransactionTests : IClassFixture + public class BundleTransactionTests : IClassFixture { private readonly TestFhirClient _client; - public TransactionTests(HttpIntegrationTestFixture fixture) + public BundleTransactionTests(HttpIntegrationTestFixture fixture) { _client = fixture.TestFhirClient; } @@ -111,7 +111,9 @@ public async Task GivenAProperTransactionBundle_WhenTransactionExecutionFails_Th var getIdGuid = Guid.NewGuid().ToString(); requestBundle.Entry[1].Request.Url = requestBundle.Entry[1].Request.Url + getIdGuid; - using var fhirException = await Assert.ThrowsAsync(async () => await _client.PostBundleAsync(requestBundle, FhirBundleProcessingLogic.Sequential)); + using var fhirException = await Assert.ThrowsAsync(async () => await _client.PostBundleAsync( + requestBundle, + new FhirBundleOptions() { BundleProcessingLogic = FhirBundleProcessingLogic.Sequential })); Assert.Equal(HttpStatusCode.NotFound, fhirException.StatusCode); string[] expectedDiagnostics = { "Transaction failed on 'GET' for the requested url '/" + requestBundle.Entry[1].Request.Url + "'.", "Resource type 'Patient' with id '12345" + getIdGuid + "' couldn't be found." }; @@ -314,12 +316,12 @@ public async Task GivenATransactionWithConditionalCreateAndReference_WhenExecute patient.Identifier.First().Value = patientIdentifier; bundle.Entry.First().Request.IfNoneExist = $"identifier=|{patientIdentifier}"; - FhirResponse bundleResponse1 = await _client.PostBundleAsync(bundle, processingLogic: processingLogic); + FhirResponse bundleResponse1 = await _client.PostBundleAsync(bundle, new FhirBundleOptions() { BundleProcessingLogic = processingLogic }); var patientId = bundleResponse1.Resource.Entry.First().Resource.Id; ValidateReferenceToPatient("Bundle 1", bundleResponse1.Resource.Entry[1].Resource, patientId, bundleResponse1); - FhirResponse bundleResponse2 = await _client.PostBundleAsync(bundle, processingLogic: processingLogic); + FhirResponse bundleResponse2 = await _client.PostBundleAsync(bundle, new FhirBundleOptions() { BundleProcessingLogic = processingLogic }); ValidateReferenceToPatient("Bundle 2", bundleResponse2.Resource.Entry[1].Resource, patientId, bundleResponse2); } diff --git a/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/FhirPathPatchTests.cs b/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/FhirPathPatchTests.cs index 7986fef64c..644d19f4f1 100644 --- a/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/FhirPathPatchTests.cs +++ b/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/FhirPathPatchTests.cs @@ -81,7 +81,7 @@ public async Task GivenAServerThatSupportsIt_WhenSubmittingInvalidFhirPatch_Then Assert.Equal(OperationOutcome.IssueType.Invalid, responseObject.Issue[0].Code); } - [SkippableFact] + [SkippableFact(Skip = "This test is skipped for STU3.")] [Trait(Traits.Priority, Priority.One)] public async Task GivenAPatchDocument_WhenSubmittingAParallelBundleWithDuplicatedPatch_ThenServerShouldReturnAnError() { @@ -89,8 +89,7 @@ public async Task GivenAPatchDocument_WhenSubmittingAParallelBundleWithDuplicate var bundleWithPatch = Samples.GetJsonSample("Bundle-FhirPatch").ToPoco(); - // This test required sequential bundle processing. - using FhirResponse fhirResponse = await _client.PostBundleAsync(bundleWithPatch, processingLogic: FhirBundleProcessingLogic.Parallel); + using FhirResponse fhirResponse = await _client.PostBundleAsync(bundleWithPatch, new FhirBundleOptions() { BundleProcessingLogic = FhirBundleProcessingLogic.Parallel }); Assert.Equal(HttpStatusCode.OK, fhirResponse.Response.StatusCode); @@ -110,7 +109,7 @@ public async Task GivenAPatchDocument_WhenSubmittingAParallelBundleWithDuplicate } } - [SkippableFact] + [SkippableFact(Skip = "This test is skipped for STU3.")] [Trait(Traits.Priority, Priority.One)] public async Task GivenAPatchDocument_WhenSubmittingABundleWithFhirPatch_ThenServerShouldPatchCorrectly() { @@ -119,7 +118,7 @@ public async Task GivenAPatchDocument_WhenSubmittingABundleWithFhirPatch_ThenSer var bundleWithPatch = Samples.GetJsonSample("Bundle-FhirPatch").ToPoco(); // This test required sequential bundle processing. - using FhirResponse patched = await _client.PostBundleAsync(bundleWithPatch, processingLogic: FhirBundleProcessingLogic.Sequential); + using FhirResponse patched = await _client.PostBundleAsync(bundleWithPatch, new FhirBundleOptions() { BundleProcessingLogic = FhirBundleProcessingLogic.Sequential }); Assert.Equal(HttpStatusCode.OK, patched.Response.StatusCode); diff --git a/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/JsonPatchTests.cs b/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/JsonPatchTests.cs index 8754f0cb2c..13fbb08e96 100644 --- a/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/JsonPatchTests.cs +++ b/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/JsonPatchTests.cs @@ -74,7 +74,7 @@ public async Task GivenAPatchDocument_WhenSubmittingABundleWithBinaryPatch_ThenS var bundleWithPatch = Samples.GetJsonSample("Bundle-BinaryPatch").ToPoco(); // This test required sequential bundle processing. - using FhirResponse patched = await _client.PostBundleAsync(bundleWithPatch, processingLogic: FhirBundleProcessingLogic.Sequential); + using FhirResponse patched = await _client.PostBundleAsync(bundleWithPatch, new FhirBundleOptions() { BundleProcessingLogic = FhirBundleProcessingLogic.Sequential }); Assert.Equal(HttpStatusCode.OK, patched.Response.StatusCode); diff --git a/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Persistence/CosmosDbFhirStorageTestsFixture.cs b/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Persistence/CosmosDbFhirStorageTestsFixture.cs index b1f4457e37..de8d38b05a 100644 --- a/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Persistence/CosmosDbFhirStorageTestsFixture.cs +++ b/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Persistence/CosmosDbFhirStorageTestsFixture.cs @@ -220,7 +220,6 @@ public async Task InitializeAsync() _fhirRequestContextAccessor, _cosmosDataStoreConfiguration, cosmosDbPhysicalPartitionInfo, - new QueryPartitionStatisticsCache(), compartmentSearchRewriter, smartCompartmentSearchRewriter, NullLogger.Instance);