Support multiple pages of include results in bulk delete

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

@@ -255,44 +255,6 @@ await RunGetBulkDeleteTest(
                 new GetBulkDeleteResponse(ToParameters(resultsDictionary).ToArray(), issues, System.Net.HttpStatusCode.OK));
         }
 
-        [Fact]
-        public async Task GivenMiscountedBulkDeleteJob_WhenStatusRequested_ThenStatusIsReturned()
-        {
-            var patientResult1 = new BulkDeleteResult();
-            patientResult1.ResourcesDeleted.Add(KnownResourceTypes.Patient, 15);
-
-            var resourcesDeleted = new List<Tuple<string, Base>>
-            {
-                new(KnownResourceTypes.Patient, new Integer64(15)),
-            };
-
-            var resultsDictionary = new Dictionary<string, ICollection<Tuple<string, Base>>>()
-            {
-                { _countLabel, resourcesDeleted },
-            };
-
-            var issues = new List<OperationOutcomeIssue>()
-            {
-                new(
-                    OperationOutcomeConstants.IssueSeverity.Warning,
-                    OperationOutcomeConstants.IssueType.Informational,
-                    detailsText: "There was a count mismatch when checking the job results. This could mean a job was restarted unexpetedly or resources were deleted by another process while the job was running. Please double check that all desired resources have been deleted. Audit logs can be referenced to get a list of the resources deleted during this operation."),
-            };
-
-            await RunGetBulkDeleteTest(
-                new List<Tuple<JobInfo, int>>()
-                {
-                    new Tuple<JobInfo, int>(
-                        new()
-                        {
-                            Status = JobStatus.Completed,
-                            Result = JsonConvert.SerializeObject(patientResult1),
-                        },
-                        17),
-                },
-                new GetBulkDeleteResponse(ToParameters(resultsDictionary).ToArray(), issues, System.Net.HttpStatusCode.OK));
-        }
-
         [Fact]
         public async Task GivenUnauthorizedUser_WhenStatusRequested_ThenUnauthorizedIsReturned()
         {
@@ -318,7 +280,7 @@ private async Task RunGetBulkDeleteTest(IReadOnlyList<Tuple<JobInfo, int>> jobs,
 
             foreach (var job in jobs)
             {
-                var definition = JsonConvert.SerializeObject(new BulkDeleteDefinition(JobType.BulkDeleteProcessing, DeleteOperation.HardDelete, null, null, "test", "test", "test", job.Item2));
+                var definition = JsonConvert.SerializeObject(new BulkDeleteDefinition(JobType.BulkDeleteProcessing, DeleteOperation.HardDelete, null, null, "test", "test", "test"));
                 job.Item1.Definition = definition;
             }
 

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

@@ -41,10 +41,11 @@ public static class SearchServiceExtensions
         /// <param name="continuationToken">An optional ContinuationToken</param>
         /// <param name="versionType">The versions of a resource to return</param>
         /// <param name="onlyIds">Whether to return only resource ids or the full resource</param>
+        /// <param name="isIncludesOperation">Whether to search for included resources</param>
         /// <param name="logger">The logger</param>
         /// <returns>Search collection and a continuationToken</returns>
         /// <exception cref="PreconditionFailedException">Returns this exception when all passed in params match the search result unusedParams</exception>
-        internal static async Task<(IReadOnlyCollection<SearchResultEntry> Results, string ContinuationToken)> ConditionalSearchAsync(
+        internal static async Task<(IReadOnlyCollection<SearchResultEntry> Results, string ContinuationToken, string IncludesContinuationToken)> ConditionalSearchAsync(
             this ISearchService searchService,
             string instanceType,
             IReadOnlyList<Tuple<string, string>> conditionalParameters,
@@ -53,6 +54,7 @@ public static class SearchServiceExtensions
             string continuationToken = null,
             ResourceVersionType versionType = ResourceVersionType.Latest,
             bool onlyIds = false,
+            bool isIncludesOperation = false,
             ILogger logger = null)
         {
             // Filters search parameters that can limit the number of results (e.g. _count=1)
@@ -67,48 +69,51 @@ public static class SearchServiceExtensions
                 filteredParameters.Add(Tuple.Create(KnownQueryParameterNames.Count, count.ToString()));
             }
 
+            SearchResult results;
             var matchedResults = new List<SearchResultEntry>();
             var includeResults = new List<SearchResultEntry>();
-            string lastContinuationToken = continuationToken;
             LongRunningOperationStatistics statistics = new LongRunningOperationStatistics(operationName: "conditionalSearchAsync");
             try
             {
                 statistics.StartCollectingResults();
-                do
+
+                var searchParameters = new List<Tuple<string, string>>(filteredParameters);
+                if (!string.IsNullOrEmpty(continuationToken))
                 {
-                    var searchParameters = new List<Tuple<string, string>>(filteredParameters);
-                    if (!string.IsNullOrEmpty(lastContinuationToken))
+                    if (isIncludesOperation)
+                    {
+                        searchParameters.Add(Tuple.Create(KnownQueryParameterNames.IncludesContinuationToken, ContinuationTokenEncoder.Encode(continuationToken)));
+                    }
+                    else
                     {
-                        searchParameters.Add(Tuple.Create(KnownQueryParameterNames.ContinuationToken, ContinuationTokenEncoder.Encode(lastContinuationToken)));
+                        searchParameters.Add(Tuple.Create(KnownQueryParameterNames.ContinuationToken, ContinuationTokenEncoder.Encode(continuationToken)));
                     }
+                }
 
-                    statistics.Iterate();
+                statistics.Iterate();
 
-                    SearchResult results = await searchService.SearchAsync(instanceType, searchParameters.ToImmutableList(), cancellationToken, resourceVersionTypes: versionType, onlyIds: onlyIds);
-                    lastContinuationToken = results?.ContinuationToken;
+                results = await searchService.SearchAsync(instanceType, searchParameters.ToImmutableList(), cancellationToken, resourceVersionTypes: versionType, onlyIds: onlyIds, isIncludesOperation: isIncludesOperation);
 
-                    // Check if all parameters passed in were unused, this would result in no search parameters being applied to search results
-                    int? totalUnusedParameters = results?.UnsupportedSearchParameters.Count;
-                    if (totalUnusedParameters == userProvidedParameterCount)
-                    {
-                        logger?.LogInformation("PreconditionFailed: ConditionalOperationNotSelectiveEnough");
-                        throw new PreconditionFailedException(string.Format(CultureInfo.InvariantCulture, Core.Resources.ConditionalOperationNotSelectiveEnough, instanceType));
-                    }
+                // Check if all parameters passed in were unused, this would result in no search parameters being applied to search results
+                int? totalUnusedParameters = results?.UnsupportedSearchParameters.Count;
+                if (totalUnusedParameters == userProvidedParameterCount)
+                {
+                    logger?.LogInformation("PreconditionFailed: ConditionalOperationNotSelectiveEnough");
+                    throw new PreconditionFailedException(string.Format(CultureInfo.InvariantCulture, Core.Resources.ConditionalOperationNotSelectiveEnough, instanceType));
+                }
 
-                    if (results?.Results?.Any() == true)
-                    {
-                        matchedResults.AddRange(
-                            results?.Results
-                                .Where(x => x.SearchEntryMode == ValueSets.SearchEntryMode.Match)
-                                .Take(Math.Max(count.HasValue ? 0 : results.Results.Count(), count.GetValueOrDefault() - matchedResults.Count)));
+                if (results?.Results?.Any() == true)
+                {
+                    matchedResults.AddRange(
+                        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));
-                    }
+                    // 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));
             }
             finally
             {
@@ -128,7 +133,7 @@ public static class SearchServiceExtensions
             }
 
             var resultsToReturn = matchedResults.Concat(includeResults).ToList();
-            return (resultsToReturn, lastContinuationToken);
+            return (resultsToReturn, isIncludesOperation ? results?.IncludesContinuationToken : results?.ContinuationToken, results?.IncludesContinuationToken);
         }
     }
 }

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

@@ -22,7 +22,6 @@ public BulkDeleteDefinition(
             string url,
             string baseUrl,
             string parentRequestId,
-            long startingResourceCount = 0,
             ResourceVersionType versionType = ResourceVersionType.Latest)
         {
             TypeId = (int)jobType;
@@ -32,7 +31,6 @@ public BulkDeleteDefinition(
             Url = url;
             BaseUrl = baseUrl;
             ParentRequestId = parentRequestId;
-            ExpectedResourceCount = startingResourceCount;
             VersionType = versionType;
         }
 
@@ -62,9 +60,6 @@ protected BulkDeleteDefinition()
         [JsonProperty(JobRecordProperties.ParentRequestId)]
         public string ParentRequestId { get; private set; }
 
-        [JsonProperty(JobRecordProperties.ExpectedResourceCount)]
-        public long ExpectedResourceCount { get; private set; }
-
         [JsonProperty(JobRecordProperties.VersionType)]
         public ResourceVersionType VersionType { get; private set; }
     }

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

@@ -97,7 +97,6 @@ internal static async Task<BulkDeleteDefinition> CreateProcessingDefinition(Bulk
                     baseDefinition.Url,
                     baseDefinition.BaseUrl,
                     baseDefinition.ParentRequestId,
-                    numResources,
                     baseDefinition.VersionType);
             }
 

src/Microsoft.Health.Fhir.Core/Features/Operations/BulkDelete/Handlers/CreateBulkDeleteHandler.cs

@@ -70,7 +70,7 @@ public async Task<CreateBulkDeleteResponse> Handle(CreateBulkDeleteRequest reque
 
             // Should not run bulk delete if any of the search parameters are invalid as it can lead to unpredicatable results
             await _searchService.ConditionalSearchAsync(request.ResourceType, searchParameters, cancellationToken, count: 1, logger: _logger);
-            if (_contextAccessor.RequestContext?.BundleIssues?.Count > 0)
+            if (_contextAccessor.RequestContext?.BundleIssues?.Count > 0 && _contextAccessor.RequestContext.BundleIssues.Any(x => !string.Equals(x.Diagnostics, Core.Resources.TruncatedIncludeMessageForIncludes, StringComparison.OrdinalIgnoreCase)))
             {
                 throw new BadRequestException(_contextAccessor.RequestContext.BundleIssues.Select(issue => issue.Diagnostics).ToList());
             }

src/Microsoft.Health.Fhir.Core/Features/Operations/BulkDelete/Handlers/GetBulkDeleteHandler.cs

@@ -55,7 +55,6 @@ public async Task<GetBulkDeleteResponse> Handle(GetBulkDeleteRequest request, Ca
             var failed = false;
             var cancelled = false;
             var succeeded = true;
-            var addBadCountWarning = false;
             var resourcesDeleted = new Dictionary<string, long>();
             var issues = new List<OperationOutcomeIssue>();
             var failureResultCode = HttpStatusCode.OK;
@@ -120,25 +119,6 @@ public async Task<GetBulkDeleteResponse> Handle(GetBulkDeleteRequest request, Ca
                             resourcesDeleted[key] += result.ResourcesDeleted[key];
                         }
                     }
-
-                    if (job.Status == JobStatus.Completed)
-                    {
-                        var definition = job.DeserializeDefinition<BulkDeleteDefinition>();
-                        if (jobTotal < definition.ExpectedResourceCount)
-                        {
-                            addBadCountWarning = true;
-                        }
-                        else if (jobTotal > definition.ExpectedResourceCount)
-                        {
-                            // I have no clue how this could happen and it imiplies more data was deleted than existed when the job started.
-                            failed = true;
-                            failureResultCode = HttpStatusCode.InternalServerError;
-                            issues.Add(new OperationOutcomeIssue(
-                                OperationOutcomeConstants.IssueSeverity.Error,
-                                OperationOutcomeConstants.IssueType.Exception,
-                                detailsText: "Count mismatch exception. More resources were deleted than existed at the start of the job run. Please review audit logs to check the number and ids of deleted resources."));
-                        }
-                    }
                 }
             }
 
@@ -171,14 +151,6 @@ public async Task<GetBulkDeleteResponse> Handle(GetBulkDeleteRequest request, Ca
                 }
             }
 
-            if (addBadCountWarning)
-            {
-                issues.Add(new OperationOutcomeIssue(
-                    OperationOutcomeConstants.IssueSeverity.Warning,
-                    OperationOutcomeConstants.IssueType.Informational,
-                    detailsText: "There was a count mismatch when checking the job results. This could mean a job was restarted unexpetedly or resources were deleted by another process while the job was running. Please double check that all desired resources have been deleted. Audit logs can be referenced to get a list of the resources deleted during this operation."));
-            }
-
             if (failed && issues.Count > 0)
             {
                 return new GetBulkDeleteResponse(fhirResults, issues, failureResultCode);

src/Microsoft.Health.Fhir.Core/Messages/Conditional/ConditionalResourceRequest.cs

@@ -27,7 +27,7 @@ protected ConditionalResourceRequest(string resourceType, IReadOnlyList<Tuple<st
 
         public string ResourceType { get; }
 
-        public IReadOnlyList<Tuple<string, string>> ConditionalParameters { get; }
+        public IReadOnlyList<Tuple<string, string>> ConditionalParameters { get; set; }
 
         public BundleResourceContext BundleResourceContext { get; }
 

src/Microsoft.Health.Fhir.Core/Messages/Delete/ConditionalDeleteResourceRequest.cs

@@ -24,7 +24,8 @@ public ConditionalDeleteResourceRequest(
             BundleResourceContext bundleResourceContext = null,
             bool deleteAll = false,
             ResourceVersionType versionType = ResourceVersionType.Latest,
-            bool allowPartialSuccess = false)
+            bool allowPartialSuccess = false,
+            bool isIncludesRequest = false)
             : base(resourceType, conditionalParameters, bundleResourceContext)
         {
             EnsureArg.IsNotNull(conditionalParameters, nameof(conditionalParameters));
@@ -34,6 +35,7 @@ public ConditionalDeleteResourceRequest(
             DeleteAll = deleteAll;
             VersionType = versionType;
             AllowPartialSuccess = allowPartialSuccess;
+            IsIncludesRequest = isIncludesRequest;
         }
 
         public DeleteOperation DeleteOperation { get; }
@@ -46,6 +48,22 @@ public ConditionalDeleteResourceRequest(
 
         public bool AllowPartialSuccess { get; }
 
+        public bool IsIncludesRequest { get; set; }
+
         protected override IEnumerable<string> GetCapabilities() => Capabilities;
+
+        public ConditionalDeleteResourceRequest Clone()
+        {
+            return new ConditionalDeleteResourceRequest(
+                ResourceType,
+                new List<Tuple<string, string>>(ConditionalParameters),
+                DeleteOperation,
+                MaxDeleteCount,
+                BundleResourceContext,
+                DeleteAll,
+                VersionType,
+                AllowPartialSuccess,
+                IsIncludesRequest);
+        }
     }
 }

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

@@ -37,6 +37,7 @@
 using Microsoft.Health.Fhir.Core.Messages.Create;
 using Microsoft.Health.Fhir.Core.Messages.Delete;
 using Microsoft.Health.Fhir.Core.Models;
+using Microsoft.Health.Fhir.Core.Registration;
 using Microsoft.Health.Fhir.Core.UnitTests.Extensions;
 using Microsoft.Health.Fhir.Tests.Common;
 using Microsoft.Health.Fhir.Tests.Common.Mocks;
@@ -123,7 +124,7 @@ public ResourceHandlerTests()
             var auditLogger = Substitute.For<IAuditLogger>();
             var logger = Substitute.For<ILogger<DeletionService>>();
 
-            var deleter = new DeletionService(_resourceWrapperFactory, lazyConformanceProvider, _fhirDataStore.CreateMockScopeProvider(), _searchService.CreateMockScopeProvider(), _resourceIdProvider, contextAccessor, auditLogger, new OptionsWrapper<CoreFeatureConfiguration>(coreFeatureConfiguration), logger);
+            var deleter = new DeletionService(_resourceWrapperFactory, lazyConformanceProvider, _fhirDataStore.CreateMockScopeProvider(), _searchService.CreateMockScopeProvider(), _resourceIdProvider, contextAccessor, auditLogger, new OptionsWrapper<CoreFeatureConfiguration>(coreFeatureConfiguration), Substitute.For<IFhirRuntimeConfiguration>(), logger);
 
             var conditionalCreateLogger = Substitute.For<ILogger<ConditionalCreateResourceHandler>>();
             var conditionalUpsertLogger = Substitute.For<ILogger<ConditionalUpsertResourceHandler>>();