Fix delete by url

src/Microsoft.Health.Fhir.Core/Features/Search/Parameters/ISearchParameterOperations.cs

@@ -17,11 +17,11 @@ public interface ISearchParameterOperations
     {
         DateTimeOffset SearchParamLastUpdated { get; }
 
-        Task DeleteSearchParameterAsync(RawResource searchParamResource, CancellationToken cancellationToken, bool ignoreSearchParameterNotSupportedException = false, bool isHardDelete = false);
+        Task MarkSearchParameterForDeletionAsync(RawResource searchParamResource, CancellationToken cancellationToken, bool isHardDelete = false);
 
         Task<DateTimeOffset> ValidateSearchParameterAsync(ITypedElement searchParam, CancellationToken cancellationToken, DateTimeOffset? lastUpdated = null);
 
-        Task UpdateSearchParameterStatusAsync(IReadOnlyCollection<string> searchParameterUris, SearchParameterStatus status, CancellationToken cancellationToken, bool ignoreSearchParameterNotSupportedException = false);
+        Task UpdateSearchParameterStatusAsync(IReadOnlyCollection<string> searchParameterUris, SearchParameterStatus status, CancellationToken cancellationToken);
 
         /// <summary>
         /// This method should be called to get any updates to search param cache

src/Microsoft.Health.Fhir.Core/Features/Search/Parameters/SearchParameterOperations.cs

@@ -162,25 +162,22 @@ public async Task<DateTimeOffset> ValidateSearchParameterAsync(ITypedElement sea
         }
 
         /// <summary>
-        /// Marks the Search Parameter as PendingDelete or PendingHardDelete. This is only used by DeletionService.cs and will be removed when refactoring is done
-        /// to allow deletion service to properly handle Hard deletions for Search Parameters (e.g. allow reindex prior to removing resource from DB).
-        /// !!! This method has incorrect name. It does not delete search parameter, it just updates its status.
+        /// Marks the Search Parameter as PendingDelete or PendingHardDelete
         /// </summary>
         /// <param name="searchParamResource">Search Parameter to update to Pending Delete status.</param>
         /// <param name="cancellationToken">Cancellation Token</param>
-        /// <param name="ignoreSearchParameterNotSupportedException">The value indicating whether to ignore SearchParameterNotSupportedException.</param>
         /// <param name="isHardDelete">True for hard delete (PendingHardDelete), false for soft delete (PendingDelete).</param>
-        public async Task DeleteSearchParameterAsync(RawResource searchParamResource, CancellationToken cancellationToken, bool ignoreSearchParameterNotSupportedException = false, bool isHardDelete = false)
+        public async Task MarkSearchParameterForDeletionAsync(RawResource searchParamResource, CancellationToken cancellationToken, bool isHardDelete = false)
         {
             var searchParam = _modelInfoProvider.ToTypedElement(searchParamResource);
             var searchParameterUrl = searchParam.GetStringScalar("url");
 
             try
             {
-                _logger.LogInformation("DeleteSearchParameterAsync: Refreshing cache");
+                _logger.LogInformation("MarkSearchParameterForDeletionAsync: Refreshing cache");
                 await GetAndApplySearchParameterUpdates(cancellationToken);
                 var status = isHardDelete ? SearchParameterStatus.PendingHardDelete : SearchParameterStatus.PendingDelete;
-                _logger.LogInformation("DeleteSearchParameterAsync: Deleting the search parameter '{Url}' with status {Status}", searchParameterUrl, status);
+                _logger.LogInformation("MarkSearchParameterForDeletionAsync: Marking the search parameter '{Url}' with status {Status}", searchParameterUrl, status);
                 await _searchParameterStatusManager.UpdateSearchParameterStatusAsync(new[] { searchParameterUrl }, status, cancellationToken, lastUpdated: SearchParamLastUpdated);
             }
             catch (FhirException fex)
@@ -206,9 +203,9 @@ public async Task DeleteSearchParameterAsync(RawResource searchParamResource, Ca
             }
         }
 
-        public async Task UpdateSearchParameterStatusAsync(IReadOnlyCollection<string> searchParameterUris, SearchParameterStatus status, CancellationToken cancellationToken, bool ignoreSearchParameterNotSupportedException = false)
+        public async Task UpdateSearchParameterStatusAsync(IReadOnlyCollection<string> searchParameterUris, SearchParameterStatus status, CancellationToken cancellationToken)
         {
-            await _searchParameterStatusManager.UpdateSearchParameterStatusAsync(searchParameterUris, status, cancellationToken, ignoreSearchParameterNotSupportedException);
+            await _searchParameterStatusManager.UpdateSearchParameterStatusAsync(searchParameterUris, status, cancellationToken);
         }
 
         /// <summary>

src/Microsoft.Health.Fhir.Core/Features/Search/Registry/ISearchParameterStatusManager.cs

@@ -21,7 +21,7 @@ public interface ISearchParameterStatusManager
 
         Task HandleAsync(SearchParameterDefinitionManagerInitialized notification, CancellationToken cancellationToken);
 
-        Task UpdateSearchParameterStatusAsync(IReadOnlyCollection<string> searchParameterUris, SearchParameterStatus status, CancellationToken cancellationToken, bool ignoreSearchParameterNotSupportedException = false, long? reindexId = null, DateTimeOffset? lastUpdated = null);
+        Task UpdateSearchParameterStatusAsync(IReadOnlyCollection<string> searchParameterUris, SearchParameterStatus status, CancellationToken cancellationToken, long? reindexId = null, DateTimeOffset? lastUpdated = null);
 
         Task<CacheConsistencyResult> CheckCacheConsistencyAsync(DateTime updateEventsSince, DateTime activeHostsSince, CancellationToken cancellationToken);
 

src/Microsoft.Health.Fhir.Core/Features/Search/Registry/SearchParameterStatusManager.cs

@@ -136,7 +136,7 @@ public async Task HandleAsync(SearchParameterDefinitionManagerInitialized notifi
             await EnsureInitializedAsync(cancellationToken);
         }
 
-        public async Task UpdateSearchParameterStatusAsync(IReadOnlyCollection<string> searchParameterUris, SearchParameterStatus status, CancellationToken cancellationToken, bool ignoreSearchParameterNotSupportedException = false, long? reindexId = null, DateTimeOffset? lastUpdated = null)
+        public async Task UpdateSearchParameterStatusAsync(IReadOnlyCollection<string> searchParameterUris, SearchParameterStatus status, CancellationToken cancellationToken, long? reindexId = null, DateTimeOffset? lastUpdated = null)
         {
             EnsureArg.IsNotNull(searchParameterUris);
 

src/Microsoft.Health.Fhir.Shared.Core.UnitTests/Features/Operations/Reindex/ReindexOrchestratorJobTests.cs

@@ -987,7 +987,6 @@ await _searchParameterStatusManager.Received().UpdateSearchParameterStatusAsync(
                         Arg.Is<IReadOnlyCollection<string>>(l => l.Contains(searchParam.Url.ToString())),
                         SearchParameterStatus.Disabled,
                         Arg.Any<CancellationToken>(),
-                        Arg.Any<bool>(),
                         Arg.Any<long?>(),
                         Arg.Any<DateTimeOffset?>());
         }
@@ -1038,7 +1037,6 @@ await _searchParameterStatusManager.Received(1).UpdateSearchParameterStatusAsync
                     urls.Contains(orphanCustom.Url.OriginalString)),
                 SearchParameterStatus.Deleted,
                 Arg.Any<CancellationToken>(),
-                Arg.Any<bool>(),
                 Arg.Is<long?>(id => id == jobInfo.Id),
                 Arg.Any<DateTimeOffset?>());
         }
@@ -1088,7 +1086,6 @@ await _searchParameterStatusManager.DidNotReceive().UpdateSearchParameterStatusA
                 Arg.Any<IReadOnlyCollection<string>>(),
                 SearchParameterStatus.Deleted,
                 Arg.Any<CancellationToken>(),
-                Arg.Any<bool>(),
                 Arg.Is<long?>(id => id == jobInfo.Id),
                 Arg.Any<DateTimeOffset?>());
         }
@@ -1125,7 +1122,6 @@ await _searchParameterStatusManager.DidNotReceive().UpdateSearchParameterStatusA
                 Arg.Any<IReadOnlyCollection<string>>(),
                 SearchParameterStatus.Deleted,
                 Arg.Any<CancellationToken>(),
-                Arg.Any<bool>(),
                 Arg.Is<long?>(id => id == jobInfo.Id),
                 Arg.Any<DateTimeOffset?>());
         }
@@ -1177,7 +1173,6 @@ await _searchParameterStatusManager.Received().UpdateSearchParameterStatusAsync(
                 Arg.Is<List<string>>(l => l.Contains(searchParam.Url.ToString())),
                 SearchParameterStatus.Deleted,
                 Arg.Any<CancellationToken>(),
-                Arg.Any<bool>(),
                 Arg.Any<long?>());
         }
 
@@ -1828,7 +1823,6 @@ await _searchParameterStatusManager.Received().UpdateSearchParameterStatusAsync(
                             l.Contains(patientBirthdateParam.Url.ToString())),
                         SearchParameterStatus.Enabled,
                         Arg.Any<CancellationToken>(),
-                        false,
                         Arg.Is<long?>(id => id == jobInfo.Id));
 
                     receivedCall = true;
@@ -2064,7 +2058,6 @@ await _searchParameterStatusManager.Received().UpdateSearchParameterStatusAsync(
                 Arg.Any<IReadOnlyCollection<string>>(),
                 SearchParameterStatus.Enabled,
                 Arg.Any<CancellationToken>(),
-                false,
                 Arg.Is<long?>(id => id == jobInfo.Id));
         }
 
@@ -2233,13 +2226,11 @@ await _searchParameterStatusManager.Received().UpdateSearchParameterStatusAsync(
                         Arg.Is<IReadOnlyCollection<string>>(l => l.Contains(searchParamLowercase.Url.ToString())),
                         SearchParameterStatus.Enabled,
                         Arg.Any<CancellationToken>(),
-                        false,
                         Arg.Is<long?>(id => id == jobInfo.Id));
             await _searchParameterStatusManager.Received().UpdateSearchParameterStatusAsync(
                         Arg.Is<IReadOnlyCollection<string>>(l => l.Contains(searchParamMixedCase.Url.ToString())),
                         SearchParameterStatus.Deleted,
                         Arg.Any<CancellationToken>(),
-                        false,
                         Arg.Is<long?>(id => id == jobInfo.Id));
         }
 
@@ -2314,14 +2305,12 @@ await _searchParameterStatusManager.Received().UpdateSearchParameterStatusAsync(
                         Arg.Is<IReadOnlyCollection<string>>(l => l.Contains(searchParam1.Url.ToString())),
                         SearchParameterStatus.Disabled,
                         Arg.Any<CancellationToken>(),
-                        false,
                         Arg.Is<long?>(id => id == jobInfo.Id));
 
             await _searchParameterStatusManager.Received().UpdateSearchParameterStatusAsync(
                         Arg.Is<IReadOnlyCollection<string>>(l => l.Contains(searchParam2.Url.ToString())),
                         SearchParameterStatus.Disabled,
                         Arg.Any<CancellationToken>(),
-                        false,
                         Arg.Is<long?>(id => id == jobInfo.Id));
         }
 

src/Microsoft.Health.Fhir.Shared.Core.UnitTests/Features/Operations/SearchParameterState/SearchParameterStateUpdateHandlerTests.cs

@@ -111,7 +111,7 @@ public SearchParameterStateUpdateHandlerTests()
             _cancellationToken = CancellationToken.None;
 
             _authorizationService.CheckAccess(DataActions.SearchParameter, _cancellationToken).Returns(DataActions.SearchParameter);
-            _searchParameterOperations.UpdateSearchParameterStatusAsync(Arg.Any<IReadOnlyCollection<string>>(), Arg.Any<SearchParameterStatus>(), Arg.Any<CancellationToken>(), Arg.Any<bool>()).Returns(Task.CompletedTask);
+            _searchParameterOperations.UpdateSearchParameterStatusAsync(Arg.Any<IReadOnlyCollection<string>>(), Arg.Any<SearchParameterStatus>(), Arg.Any<CancellationToken>()).Returns(Task.CompletedTask);
 
             var searchParamDefinitionStore = new List<SearchParameterInfo>
             {
@@ -239,7 +239,7 @@ public async Task GivenARequestToUpdateSearchParameterStatus_WhenTheStatusIsEnab
             var statusPart = resourceResponse.Parameter[0].Part.Where(p => p.Name == SearchParameterStateProperties.Status).First();
             Assert.True(urlPart.Value.ToString() == ResourceId);
             Assert.True(statusPart.Value.ToString() == SearchParameterStatus.Supported.ToString());
-            await _searchParameterOperations.Received(1).UpdateSearchParameterStatusAsync(Arg.Is<IReadOnlyCollection<string>>(x => x.Count == 1 && x.First() == ResourceId), SearchParameterStatus.Supported, Arg.Any<CancellationToken>(), false);
+            await _searchParameterOperations.Received(1).UpdateSearchParameterStatusAsync(Arg.Is<IReadOnlyCollection<string>>(x => x.Count == 1 && x.First() == ResourceId), SearchParameterStatus.Supported, Arg.Any<CancellationToken>());
         }
 
         [Fact]
@@ -303,7 +303,7 @@ public async Task GivenARequestToUpdateSearchParameterStatus_WhenStatusIsDisable
             var statusPart = resourceResponse.Parameter[0].Part.Where(p => p.Name == SearchParameterStateProperties.Status).First();
             Assert.True(urlPart.Value.ToString() == ResourceId);
             Assert.True(statusPart.Value.ToString() == SearchParameterStatus.PendingDisable.ToString());
-            await _searchParameterOperations.Received(1).UpdateSearchParameterStatusAsync(Arg.Is<IReadOnlyCollection<string>>(x => x.Count == 1 && x.First() == ResourceId), SearchParameterStatus.PendingDisable, Arg.Any<CancellationToken>(), false);
+            await _searchParameterOperations.Received(1).UpdateSearchParameterStatusAsync(Arg.Is<IReadOnlyCollection<string>>(x => x.Count == 1 && x.First() == ResourceId), SearchParameterStatus.PendingDisable, Arg.Any<CancellationToken>());
         }
 
         [Fact]

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

@@ -202,7 +202,7 @@ public async Task GivenSearchParameterDelete_WhenConcurrencyConflictOccurs_ThenR
 
             var attemptCount = 0;
             _searchParameterOperations
-                .DeleteSearchParameterAsync(Arg.Any<RawResource>(), Arg.Any<CancellationToken>(), Arg.Any<bool>(), Arg.Any<bool>())
+                .MarkSearchParameterForDeletionAsync(Arg.Any<RawResource>(), Arg.Any<CancellationToken>(), Arg.Any<bool>())
                 .Returns(callInfo =>
                 {
                     attemptCount++;
@@ -263,7 +263,7 @@ public async Task GivenSearchParameterDelete_WhenConcurrencyConflictExhaustsRetr
             _dataStoreFactory.GetScopedDataStore().Returns(scopedDataStore);
 
             _searchParameterOperations
-                .DeleteSearchParameterAsync(Arg.Any<RawResource>(), Arg.Any<CancellationToken>(), Arg.Any<bool>(), Arg.Any<bool>())
+                .MarkSearchParameterForDeletionAsync(Arg.Any<RawResource>(), Arg.Any<CancellationToken>(), Arg.Any<bool>())
                 .Returns(_ => throw new BadRequestException(Core.Resources.SearchParameterConcurrencyConflict));
 
             var exception = await Assert.ThrowsAsync<IncompleteOperationException<IDictionary<string, long>>>(async () =>

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

@@ -20,6 +20,7 @@
 using Microsoft.Extensions.Options;
 using Microsoft.Health.Abstractions.Exceptions;
 using Microsoft.Health.Core.Features.Audit;
+using Microsoft.Health.Core.Features.Context;
 using Microsoft.Health.Fhir.Core.Configs;
 using Microsoft.Health.Fhir.Core.Exceptions;
 using Microsoft.Health.Fhir.Core.Extensions;
@@ -47,7 +48,7 @@ public class DeletionService : IDeletionService, IDisposable
         private readonly IScopeProvider<ISearchService> _searchServiceFactory;
         private readonly ResourceIdProvider _resourceIdProvider;
         private readonly AsyncRetryPolicy _retryPolicy;
-        private readonly FhirRequestContextAccessor _contextAccessor;
+        private readonly RequestContextAccessor<IFhirRequestContext> _contextAccessor;
         private readonly IAuditLogger _auditLogger;
         private readonly CoreFeatureConfiguration _configuration;
         private readonly IFhirRuntimeConfiguration _fhirRuntimeConfiguration;
@@ -65,7 +66,7 @@ public DeletionService(
             IDeletionServiceDataStoreFactory dataStoreFactory,
             IScopeProvider<ISearchService> searchServiceFactory,
             ResourceIdProvider resourceIdProvider,
-            FhirRequestContextAccessor contextAccessor,
+            RequestContextAccessor<IFhirRequestContext> contextAccessor,
             IAuditLogger auditLogger,
             IOptions<CoreFeatureConfiguration> configuration,
             IFhirRuntimeConfiguration fhirRuntimeConfiguration,
@@ -109,6 +110,16 @@ public async Task<ResourceKey> DeleteAsync(DeleteResourceRequest request, Cancel
             switch (request.DeleteOperation)
             {
                 case DeleteOperation.SoftDelete:
+                    if (key.ResourceType == KnownResourceTypes.SearchParameter
+                        //// we should skip direct status updates when context contains pending status
+                        && (_contextAccessor.RequestContext == null
+                            || _contextAccessor.RequestContext.Properties == null
+                            || !_contextAccessor.RequestContext.Properties.ContainsKey(SearchParameterRequestContextPropertyNames.PendingStatus)))
+                    {
+                        await DeleteSearchParameter(fhirDataStore, key, false, cancellationToken);
+                        break;
+                    }
+
                     ResourceWrapper deletedWrapper = CreateSoftDeletedWrapper(key.ResourceType, request.ResourceKey.Id);
 
                     bool keepHistory = await _conformanceProvider.Value.CanKeepHistory(key.ResourceType, cancellationToken);
@@ -120,12 +131,7 @@ public async Task<ResourceKey> DeleteAsync(DeleteResourceRequest request, Cancel
                 case DeleteOperation.HardDelete:
                     if (key.ResourceType == KnownResourceTypes.SearchParameter)
                     {
-                        var resourceWrapper = await fhirDataStore.GetAsync(key, cancellationToken);
-                        if (resourceWrapper != null && !resourceWrapper.IsDeleted)
-                        {
-                            await _retryPolicy.ExecuteAsync(async () => await _searchParameterOperations.DeleteSearchParameterAsync(resourceWrapper.RawResource, cancellationToken, ignoreSearchParameterNotSupportedException: true, isHardDelete: true));
-                        }
-
+                        await DeleteSearchParameter(fhirDataStore, key, true, cancellationToken);
                         break;
                     }
 
@@ -141,6 +147,15 @@ public async Task<ResourceKey> DeleteAsync(DeleteResourceRequest request, Cancel
             return new ResourceKey(key.ResourceType, key.Id, version);
         }
 
+        private async Task DeleteSearchParameter(IFhirDataStore dataStore, ResourceKey key, bool isHardDelete, CancellationToken cancellationToken)
+        {
+            var resourceWrapper = await dataStore.GetAsync(key, cancellationToken);
+            if (resourceWrapper != null && !resourceWrapper.IsDeleted)
+            {
+                await _retryPolicy.ExecuteAsync(async () => await _searchParameterOperations.MarkSearchParameterForDeletionAsync(resourceWrapper.RawResource, cancellationToken, isHardDelete: isHardDelete));
+            }
+        }
+
         public async Task<IDictionary<string, long>> DeleteMultipleAsync(ConditionalDeleteResourceRequest request, CancellationToken cancellationToken, IList<string> excludedResourceTypes = null)
         {
             return await DeleteMultipleAsyncInternal(request, MaxParallelThreads, excludedResourceTypes, null, cancellationToken);
@@ -709,7 +724,7 @@ private async Task DeleteSearchParameterWithLockAsync(SearchResultEntry item, bo
                 await SearchParameterRetry.ExecuteAsync(
                     async () =>
                     {
-                        await _searchParameterOperations.DeleteSearchParameterAsync(item.Resource.RawResource, cancellationToken, ignoreSearchParameterNotSupportedException: true, isHardDelete: isHardDelete);
+                        await _searchParameterOperations.MarkSearchParameterForDeletionAsync(item.Resource.RawResource, cancellationToken, isHardDelete: isHardDelete);
                     },
                     "Deletion");
             }