Add support for include to delete requests (#4811)

src/Microsoft.Health.Fhir.Core.UnitTests/Features/Operations/BulkDelete/BulkDeleteProcessingJobTests.cs

@@ -58,8 +58,11 @@ public async Task GivenProcessingJob_WhenJobIsRun_ThenResourcesAreDeleted()
                 Definition = JsonConvert.SerializeObject(definition),
             };
 
+            var substituteResults = new Dictionary<string, long>();
+            substituteResults.Add("Patient", 3);
+
             _deleter.DeleteMultipleAsync(Arg.Any<ConditionalDeleteResourceRequest>(), Arg.Any<CancellationToken>())
-                .Returns(args => 3);
+                .Returns(args => substituteResults);
 
             var result = JsonConvert.DeserializeObject<BulkDeleteResult>(await _processingJob.ExecuteAsync(jobInfo, CancellationToken.None));
             Assert.Single(result.ResourcesDeleted);
@@ -80,8 +83,11 @@ public async Task GivenProcessingJob_WhenJobIsRunWithMultipleResourceTypes_ThenF
                 Definition = JsonConvert.SerializeObject(definition),
             };
 
+            var substituteResults = new Dictionary<string, long>();
+            substituteResults.Add("Patient", 3);
+
             _deleter.DeleteMultipleAsync(Arg.Any<ConditionalDeleteResourceRequest>(), Arg.Any<CancellationToken>())
-                .Returns(args => 3);
+                .Returns(args => substituteResults);
 
             var result = JsonConvert.DeserializeObject<BulkDeleteResult>(await _processingJob.ExecuteAsync(jobInfo, CancellationToken.None));
             Assert.Single(result.ResourcesDeleted);

src/Microsoft.Health.Fhir.Core/Extensions/SearchServiceExtensions.cs

@@ -26,8 +26,6 @@ public static class SearchServiceExtensions
             KnownQueryParameterNames.Summary,
             KnownQueryParameterNames.Total,
             KnownQueryParameterNames.ContinuationToken,
-            "_include",
-            "_revinclude",
         };
 
         /// <summary>
@@ -70,6 +68,7 @@ public static class SearchServiceExtensions
             }
 
             var matchedResults = new List<SearchResultEntry>();
+            var includeResults = new List<SearchResultEntry>();
             string lastContinuationToken = continuationToken;
             LongRunningOperationStatistics statistics = new LongRunningOperationStatistics(operationName: "conditionalSearchAsync");
             try
@@ -102,6 +101,11 @@ public static class SearchServiceExtensions
                             results?.Results
                                 .Where(x => x.SearchEntryMode == ValueSets.SearchEntryMode.Match)
                                 .Take(Math.Max(count.HasValue ? 0 : results.Results.Count(), count.GetValueOrDefault() - matchedResults.Count)));
+
+                        // This will get include results and outcome results. Outcome results are needed to check for too many includes warning.
+                        includeResults.AddRange(
+                            results?.Results
+                                .Where(x => x.SearchEntryMode != ValueSets.SearchEntryMode.Match));
                     }
                 }
                 while (count.HasValue && matchedResults.Count < count && !string.IsNullOrEmpty(lastContinuationToken));
@@ -123,7 +127,8 @@ public static class SearchServiceExtensions
                 }
             }
 
-            return (matchedResults, lastContinuationToken);
+            var resultsToReturn = matchedResults.Concat(includeResults).ToList();
+            return (resultsToReturn, lastContinuationToken);
         }
     }
 }

src/Microsoft.Health.Fhir.Core/Features/Operations/BulkDelete/BulkDeleteProcessingJob.cs

@@ -73,14 +73,14 @@ public async Task<string> ExecuteAsync(JobInfo jobInfo, CancellationToken cancel
 
                 _contextAccessor.RequestContext = fhirRequestContext;
                 var result = new BulkDeleteResult();
-                long numDeleted;
+                IDictionary<string, long> resourcesDeleted = new Dictionary<string, long>();
                 using IScoped<IDeletionService> deleter = _deleterFactory.Invoke();
                 Exception exception = null;
                 List<string> types = definition.Type.SplitByOrSeparator().ToList();
 
                 try
                 {
-                    numDeleted = await deleter.Value.DeleteMultipleAsync(
+                    resourcesDeleted = await deleter.Value.DeleteMultipleAsync(
                         new ConditionalDeleteResourceRequest(
                             types[0],
                             (IReadOnlyList<Tuple<string, string>>)definition.SearchParameters,
@@ -91,16 +91,22 @@ public async Task<string> ExecuteAsync(JobInfo jobInfo, CancellationToken cancel
                             allowPartialSuccess: false), // Explicitly setting to call out that this can be changed in the future if we want to. Bulk delete offers the possibility of automatically rerunning the operation until it succeeds, fully automating the process.
                         cancellationToken);
                 }
-                catch (IncompleteOperationException<long> ex)
+                catch (IncompleteOperationException<IDictionary<string, long>> ex)
                 {
-                    numDeleted = ex.PartialResults;
+                    resourcesDeleted = ex.PartialResults;
                     result.Issues.Add(ex.Message);
                     exception = ex;
                 }
 
-                result.ResourcesDeleted.Add(types[0], numDeleted);
+                foreach (var (key, value) in resourcesDeleted)
+                {
+                    if (!result.ResourcesDeleted.TryAdd(key, value))
+                    {
+                        result.ResourcesDeleted[key] += value;
+                    }
+                }
 
-                await _mediator.Publish(new BulkDeleteMetricsNotification(jobInfo.Id, numDeleted), cancellationToken);
+                await _mediator.Publish(new BulkDeleteMetricsNotification(jobInfo.Id, resourcesDeleted.Sum(resource => resource.Value)), cancellationToken);
 
                 if (exception != null)
                 {

src/Microsoft.Health.Fhir.Core/Features/Persistence/IDeletionService.cs

@@ -14,6 +14,6 @@ public interface IDeletionService
     {
         public Task<ResourceKey> DeleteAsync(DeleteResourceRequest request, CancellationToken cancellationToken);
 
-        public Task<long> DeleteMultipleAsync(ConditionalDeleteResourceRequest request, CancellationToken cancellationToken);
+        public Task<IDictionary<string, long>> DeleteMultipleAsync(ConditionalDeleteResourceRequest request, CancellationToken cancellationToken);
     }
 }

src/Microsoft.Health.Fhir.Core/Resources.resx

@@ -760,4 +760,8 @@
   <data name="ReindexRunning" xml:space="preserve">
     <value>A Reindex job is currenlty active. Search parameters cannot be updated until Reindex is complete.</value>
   </data>
+  <data name="TooManyIncludeResults" xml:space="preserve">
+    <value>The number of included results exceeded the limit of {0}</value>
+    <comment>{0} is the limit of how many include results the service will return.</comment>
+  </data>
 </root>

src/Microsoft.Health.Fhir.Shared.Core.UnitTests/Features/Resources/ResourceHandlerTests.cs

@@ -14,9 +14,11 @@
 using MediatR;
 using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.Logging;
+using Microsoft.Extensions.Options;
 using Microsoft.Extensions.Primitives;
 using Microsoft.Health.Core.Features.Security.Authorization;
 using Microsoft.Health.Extensions.DependencyInjection;
+using Microsoft.Health.Fhir.Core.Configs;
 using Microsoft.Health.Fhir.Core.Exceptions;
 using Microsoft.Health.Fhir.Core.Extensions;
 using Microsoft.Health.Fhir.Core.Features.Audit;
@@ -115,10 +117,12 @@ public ResourceHandlerTests()
             var referenceResolver = new ResourceReferenceResolver(_searchService, new TestQueryStringParser(), Substitute.For<ILogger<ResourceReferenceResolver>>());
             _resourceIdProvider = new ResourceIdProvider();
 
+            var coreFeatureConfiguration = new CoreFeatureConfiguration();
+
             var auditLogger = Substitute.For<IAuditLogger>();
             var logger = Substitute.For<ILogger<DeletionService>>();
 
-            var deleter = new DeletionService(_resourceWrapperFactory, lazyConformanceProvider, _fhirDataStore.CreateMockScopeProvider(), _searchService.CreateMockScopeProvider(), _resourceIdProvider, contextAccessor, auditLogger, logger);
+            var deleter = new DeletionService(_resourceWrapperFactory, lazyConformanceProvider, _fhirDataStore.CreateMockScopeProvider(), _searchService.CreateMockScopeProvider(), _resourceIdProvider, contextAccessor, auditLogger, new OptionsWrapper<CoreFeatureConfiguration>(coreFeatureConfiguration), logger);
 
             var conditionalCreateLogger = Substitute.For<ILogger<ConditionalCreateResourceHandler>>();
             var conditionalUpsertLogger = Substitute.For<ILogger<ConditionalUpsertResourceHandler>>();
@@ -129,7 +133,7 @@ public ResourceHandlerTests()
             collection.Add(x => new UpsertResourceHandler(_fhirDataStore, lazyConformanceProvider, _resourceWrapperFactory, _resourceIdProvider, referenceResolver, _authorizationService, ModelInfoProvider.Instance)).Singleton().AsSelf().AsImplementedInterfaces();
             collection.Add(x => new ConditionalCreateResourceHandler(_fhirDataStore, lazyConformanceProvider, _resourceWrapperFactory, _searchService, x.GetService<IMediator>(), _resourceIdProvider, _authorizationService, conditionalCreateLogger)).Singleton().AsSelf().AsImplementedInterfaces();
             collection.Add(x => new ConditionalUpsertResourceHandler(_fhirDataStore, lazyConformanceProvider, _resourceWrapperFactory, _searchService, x.GetService<IMediator>(), _resourceIdProvider, _authorizationService, conditionalUpsertLogger)).Singleton().AsSelf().AsImplementedInterfaces();
-            collection.Add(x => new ConditionalDeleteResourceHandler(_fhirDataStore, lazyConformanceProvider, _resourceWrapperFactory, _searchService, x.GetService<IMediator>(), _resourceIdProvider, _authorizationService, deleter, contextAccessor, conditionalDeleteLogger)).Singleton().AsSelf().AsImplementedInterfaces();
+            collection.Add(x => new ConditionalDeleteResourceHandler(_fhirDataStore, lazyConformanceProvider, _resourceWrapperFactory, _searchService, x.GetService<IMediator>(), _resourceIdProvider, _authorizationService, deleter, contextAccessor, new OptionsWrapper<CoreFeatureConfiguration>(coreFeatureConfiguration), conditionalDeleteLogger)).Singleton().AsSelf().AsImplementedInterfaces();
             collection.Add(x => new GetResourceHandler(_fhirDataStore, lazyConformanceProvider, _resourceWrapperFactory, _resourceIdProvider, _dataResourceFilter, _authorizationService, contextAccessor, _searchService)).Singleton().AsSelf().AsImplementedInterfaces();
             collection.Add(x => new DeleteResourceHandler(_fhirDataStore, lazyConformanceProvider, _resourceWrapperFactory, _resourceIdProvider, _authorizationService, deleter)).Singleton().AsSelf().AsImplementedInterfaces();
 

src/Microsoft.Health.Fhir.Shared.Core/Features/Resources/ConditionalResourceHandler.cs

@@ -54,27 +54,28 @@ public async Task<TResponse> Handle(TRequest request, CancellationToken cancella
                 throw new UnauthorizedFhirActionException();
             }
 
-            var matchedResults = await _searchService.ConditionalSearchAsync(
+            var results = await _searchService.ConditionalSearchAsync(
                 request.ResourceType,
                 request.ConditionalParameters,
                 cancellationToken,
                 logger: _logger);
 
-            int count = matchedResults.Results.Count;
-            if (count == 0)
+            var matches = results.Results.Where(result => result.SearchEntryMode == ValueSets.SearchEntryMode.Match).ToList();
+            int matchCount = matches.Count;
+            if (matchCount == 0)
             {
                 _logger.LogInformation("Conditional handler: Not Match. ResourceType={ResourceType}", request.ResourceType);
                 return await HandleNoMatch(request, cancellationToken);
             }
-            else if (count == 1)
+            else if (matchCount == 1)
             {
                 _logger.LogInformation("Conditional handler: One Match Found. ResourceType={ResourceType}", request.ResourceType);
-                return await HandleSingleMatch(request, matchedResults.Results.First(), cancellationToken);
+                return await HandleSingleMatch(request, matches.First(), cancellationToken);
             }
             else
             {
                 // Multiple matches: The server returns a 412 Precondition Failed error indicating the client's criteria were not selective enough
-                _logger.LogInformation("PreconditionFailed: Conditional handler: Multiple Matches Found. ResourceType={ResourceType}, NumberOfMatches={NumberOfMatches}", request.ResourceType, count);
+                _logger.LogInformation("PreconditionFailed: Conditional handler: Multiple Matches Found. ResourceType={ResourceType}, NumberOfMatches={NumberOfMatches}", request.ResourceType, matchCount);
                 throw new PreconditionFailedException(string.Format(CultureInfo.InvariantCulture, Core.Resources.ConditionalOperationNotSelectiveEnough, request.ResourceType));
             }
         }

src/Microsoft.Health.Fhir.Shared.Core/Features/Resources/Delete/ConditionalDeleteResourceHandler.cs

@@ -13,8 +13,10 @@
 using MediatR;
 using Microsoft.Build.Framework;
 using Microsoft.Extensions.Logging;
+using Microsoft.Extensions.Options;
 using Microsoft.Health.Core.Features.Context;
 using Microsoft.Health.Core.Features.Security.Authorization;
+using Microsoft.Health.Fhir.Core.Configs;
 using Microsoft.Health.Fhir.Core.Exceptions;
 using Microsoft.Health.Fhir.Core.Extensions;
 using Microsoft.Health.Fhir.Core.Features.Conformance;
@@ -31,6 +33,7 @@ public class ConditionalDeleteResourceHandler : BaseResourceHandler, IRequestHan
         private readonly ISearchService _searchService;
         private readonly IDeletionService _deleter;
         private readonly RequestContextAccessor<IFhirRequestContext> _fhirContext;
+        private readonly CoreFeatureConfiguration _configuration;
         private readonly ILogger<ConditionalDeleteResourceHandler> _logger;
 
         public ConditionalDeleteResourceHandler(
@@ -43,17 +46,20 @@ public ConditionalDeleteResourceHandler(
             IAuthorizationService<DataActions> authorizationService,
             IDeletionService deleter,
             RequestContextAccessor<IFhirRequestContext> fhirContext,
+            IOptions<CoreFeatureConfiguration> configuration,
             ILogger<ConditionalDeleteResourceHandler> logger)
             : base(fhirDataStore, conformanceProvider, resourceWrapperFactory, resourceIdProvider, authorizationService)
         {
             EnsureArg.IsNotNull(mediator, nameof(mediator));
             EnsureArg.IsNotNull(searchService, nameof(searchService));
             EnsureArg.IsNotNull(deleter, nameof(deleter));
+            EnsureArg.IsNotNull(configuration.Value, nameof(configuration));
             EnsureArg.IsNotNull(logger, nameof(logger));
 
             _searchService = searchService;
             _deleter = deleter;
             _fhirContext = fhirContext;
+            _configuration = configuration.Value;
             _logger = logger;
         }
 
@@ -86,33 +92,47 @@ public async Task<DeleteResourceResponse> Handle(ConditionalDeleteResourceReques
 
         private async Task<DeleteResourceResponse> DeleteSingleAsync(ConditionalDeleteResourceRequest request, CancellationToken cancellationToken)
         {
-            var matchedResults = await _searchService.ConditionalSearchAsync(
+            var results = await _searchService.ConditionalSearchAsync(
                 request.ResourceType,
                 request.ConditionalParameters,
                 cancellationToken,
                 logger: _logger);
 
-            int count = matchedResults.Results.Count;
+            int count = results.Results.Where(result => result.SearchEntryMode == ValueSets.SearchEntryMode.Match).Count();
+            bool tooManyIncludeResults = _fhirContext.RequestContext.BundleIssues.Any(x => string.Equals(x.Diagnostics, Core.Resources.TruncatedIncludeMessage, StringComparison.OrdinalIgnoreCase));
+
             if (count == 0)
             {
                 return new DeleteResourceResponse(0);
             }
-            else if (count == 1)
+            else if (count == 1 && !tooManyIncludeResults)
             {
-                var result = await _deleter.DeleteAsync(
-                    new DeleteResourceRequest(
-                        request.ResourceType,
-                        matchedResults.Results.First().Resource.ResourceId,
-                        request.DeleteOperation,
-                        bundleResourceContext: request.BundleResourceContext),
-                    cancellationToken);
+                if (results.Results.Count == 1)
+                {
+                    var result = await _deleter.DeleteAsync(
+                        new DeleteResourceRequest(
+                            request.ResourceType,
+                            results.Results.First().Resource.ResourceId,
+                            request.DeleteOperation,
+                            bundleResourceContext: request.BundleResourceContext),
+                        cancellationToken);
 
-                if (string.IsNullOrWhiteSpace(result.VersionId))
+                    if (string.IsNullOrWhiteSpace(result.VersionId))
+                    {
+                        return new DeleteResourceResponse(result);
+                    }
+
+                    return new DeleteResourceResponse(result, weakETag: WeakETag.FromVersionId(result.VersionId));
+                }
+                else
                 {
-                    return new DeleteResourceResponse(result);
+                    // Include results were present, use delete multiple to handle them.
+                    return await DeleteMultipleAsync(request, cancellationToken);
                 }
-
-                return new DeleteResourceResponse(result, weakETag: WeakETag.FromVersionId(result.VersionId));
+            }
+            else if (count == 1 && tooManyIncludeResults)
+            {
+                throw new BadRequestException(string.Format(CultureInfo.InvariantCulture, Core.Resources.TooManyIncludeResults, _configuration.DefaultIncludeCountPerSearch));
             }
             else
             {
@@ -124,7 +144,7 @@ private async Task<DeleteResourceResponse> DeleteSingleAsync(ConditionalDeleteRe
 
         private async Task<DeleteResourceResponse> DeleteMultipleAsync(ConditionalDeleteResourceRequest request, CancellationToken cancellationToken)
         {
-            long numDeleted = await _deleter.DeleteMultipleAsync(request, cancellationToken);
+            long numDeleted = (await _deleter.DeleteMultipleAsync(request, cancellationToken)).Sum(result => result.Value);
             return new DeleteResourceResponse((int)numDeleted);
         }
     }