From 07098806ea41e33f21c018064d0a6e043db2ab6f Mon Sep 17 00:00:00 2001 From: Sergey Galuzo Date: Fri, 26 Jun 2026 16:11:10 -0700 Subject: [PATCH 01/31] check in correct place v0 --- .../BulkDeleteProcessingJobTests.cs | 42 ++--- ...meterCacheRefreshBackgroundServiceTests.cs | 2 - .../BulkDelete/BulkDeleteProcessingJob.cs | 18 --- .../Parameters/ISearchParameterOperations.cs | 2 - .../Parameters/SearchParameterOperations.cs | 18 --- .../CosmosDbSearchParameterStatusDataStore.cs | 19 ++- .../Controllers/BulkDeleteControllerTests.cs | 27 ---- .../Controllers/BulkDeleteController.cs | 25 --- .../SearchParameterStateUpdateHandlerTests.cs | 11 -- .../SearchParameterDefinitionManagerTests.cs | 2 - .../SearchParameterValidatorTests.cs | 13 -- .../SearchParameterStateUpdateHandler.cs | 2 - .../Parameters/SearchParameterValidator.cs | 2 - .../Features/ExceptionExtension.cs | 8 + ...SqlServerSearchParameterStatusDataStore.cs | 5 + .../Operations/Reindex/ReindexJobTests.cs | 2 - .../CosmosDbFhirStorageTestsFixture.cs | 7 +- .../Persistence/FhirStorageTestsFixture.cs | 1 - ...rverSearchParameterStatusDataStoreTests.cs | 146 ++++++++++++++++++ 19 files changed, 199 insertions(+), 153 deletions(-) diff --git a/src/Microsoft.Health.Fhir.Core.UnitTests/Features/Operations/BulkDelete/BulkDeleteProcessingJobTests.cs b/src/Microsoft.Health.Fhir.Core.UnitTests/Features/Operations/BulkDelete/BulkDeleteProcessingJobTests.cs index 482a682b11..8729441555 100644 --- a/src/Microsoft.Health.Fhir.Core.UnitTests/Features/Operations/BulkDelete/BulkDeleteProcessingJobTests.cs +++ b/src/Microsoft.Health.Fhir.Core.UnitTests/Features/Operations/BulkDelete/BulkDeleteProcessingJobTests.cs @@ -36,7 +36,6 @@ public class BulkDeleteProcessingJobTests { private IDeletionService _deleter; private BulkDeleteProcessingJob _processingJob; - private ISearchParameterOperations _searchParameterOperations; private ISearchService _searchService; private IQueueClient _queueClient; @@ -47,8 +46,7 @@ public BulkDeleteProcessingJobTests() .Returns(Task.FromResult(new SearchResult(5, new List>()))); _queueClient = Substitute.For(); _deleter = Substitute.For(); - _searchParameterOperations = Substitute.For(); - _processingJob = new BulkDeleteProcessingJob(_deleter.CreateMockScopeFactory(), Substitute.For>(), Substitute.For(), _searchParameterOperations, _searchService.CreateMockScopeFactory(), _queueClient); + _processingJob = new BulkDeleteProcessingJob(_deleter.CreateMockScopeFactory(), Substitute.For>(), Substitute.For(), _searchService.CreateMockScopeFactory(), _queueClient); } [Fact] @@ -113,37 +111,29 @@ public async Task GivenProcessingJob_WhenJobIsRunWithMultipleResourceTypes_ThenF [Fact] public async Task GivenProcessingJobForSearchParameter_WhenReindexStartsBeforeExecution_ThenConflictIsThrown() { - var definition = new BulkDeleteDefinition(JobType.BulkDeleteProcessing, DeleteOperation.HardDelete, KnownResourceTypes.SearchParameter, new List>(), new List(), "https:\\test.com", "https:\\test.com", "test"); - var jobInfo = new JobInfo - { - Id = 1, - Definition = JsonConvert.SerializeObject(definition), - }; + var definition = new BulkDeleteDefinition(JobType.BulkDeleteProcessing, DeleteOperation.HardDelete, "SearchParameter", new List>(), new List(), "https:\\\\test.com", "https:\\\\test.com", "test"); + var jobInfo = new JobInfo() { Id = 1, Definition = JsonConvert.SerializeObject(definition) }; - _searchParameterOperations - .When(x => x.EnsureNoActiveReindexJobAsync(Arg.Any())) - .Do(_ => throw new FhirJobConflictException("reindex running")); + // Simulate the database layer throwing JobConflictException when trying to delete SearchParameter during active reindex + _deleter.DeleteMultipleAsync(Arg.Any(), Arg.Any(), Arg.Any>()).Returns>>(x => throw new FhirJobConflictException("A reindex job is currently running.")); - await Assert.ThrowsAsync(() => _processingJob.ExecuteAsync(jobInfo, CancellationToken.None)); - await _deleter.DidNotReceiveWithAnyArgs().DeleteMultipleAsync(default, default, default); + await Assert.ThrowsAsync(async () => await _processingJob.ExecuteAsync(jobInfo, CancellationToken.None)); } [Fact] - public async Task GivenProcessingJobWithSearchParameterLaterInTheChain_WhenReindexStartsBeforeExecution_ThenConflictIsThrownBeforeAnyDelete() + public async Task GivenBulkDeleteWithSearchParameterLaterInTheChain_WhenReindexStartsBeforeExecution_ThenConflictIsThrownBeforeAnyDelete() { - var definition = new BulkDeleteDefinition(JobType.BulkDeleteProcessing, DeleteOperation.HardDelete, "Patient,SearchParameter", new List>(), new List(), "https:\\test.com", "https:\\test.com", "test"); - var jobInfo = new JobInfo - { - Id = 1, - Definition = JsonConvert.SerializeObject(definition), - }; + var definition = new BulkDeleteDefinition(JobType.BulkDeleteProcessing, DeleteOperation.HardDelete, "Patient,SearchParameter", new List>(), new List(), "https:\\\\test.com", "https:\\\\test.com", "test"); + var jobInfo = new JobInfo() { Id = 1, Definition = JsonConvert.SerializeObject(definition) }; + + // First resource type (Patient) succeeds + var patientResults = new Dictionary { { "Patient", 5 } }; - _searchParameterOperations - .When(x => x.EnsureNoActiveReindexJobAsync(Arg.Any())) - .Do(_ => throw new FhirJobConflictException("reindex running")); + // Second resource type (SearchParameter) fails due to active reindex - database layer throws conflict + _deleter.DeleteMultipleAsync(Arg.Any(), Arg.Any(), Arg.Any>()).Returns(x => Task.FromResult>(patientResults), x => throw new FhirJobConflictException("A reindex job is currently running.")); - await Assert.ThrowsAsync(() => _processingJob.ExecuteAsync(jobInfo, CancellationToken.None)); - await _deleter.DidNotReceiveWithAnyArgs().DeleteMultipleAsync(default, default, default); + // The conflict should propagate through the workflow + await Assert.ThrowsAsync(async () => await _processingJob.ExecuteAsync(jobInfo, CancellationToken.None)); } } } diff --git a/src/Microsoft.Health.Fhir.Core.UnitTests/Features/Search/Registry/SearchParameterCacheRefreshBackgroundServiceTests.cs b/src/Microsoft.Health.Fhir.Core.UnitTests/Features/Search/Registry/SearchParameterCacheRefreshBackgroundServiceTests.cs index 939be58b4a..2ab72b6823 100644 --- a/src/Microsoft.Health.Fhir.Core.UnitTests/Features/Search/Registry/SearchParameterCacheRefreshBackgroundServiceTests.cs +++ b/src/Microsoft.Health.Fhir.Core.UnitTests/Features/Search/Registry/SearchParameterCacheRefreshBackgroundServiceTests.cs @@ -413,7 +413,6 @@ public async Task WhenBackgroundIsBlockedByAPI_BackgroundShouldSkipRefresh() Substitute.For(), Substitute.For(), Substitute.For(), - () => Substitute.For>(), Substitute.For>>(), Substitute.For>(), Substitute.For(), @@ -464,7 +463,6 @@ public async Task WhenAPIIsBlockedByBackground_ApiShouldWait() Substitute.For(), Substitute.For(), Substitute.For(), - () => Substitute.For>(), Substitute.For>>(), Substitute.For>(), Substitute.For(), diff --git a/src/Microsoft.Health.Fhir.Core/Features/Operations/BulkDelete/BulkDeleteProcessingJob.cs b/src/Microsoft.Health.Fhir.Core/Features/Operations/BulkDelete/BulkDeleteProcessingJob.cs index 59aeb4d8d1..1e1b7dcd87 100644 --- a/src/Microsoft.Health.Fhir.Core/Features/Operations/BulkDelete/BulkDeleteProcessingJob.cs +++ b/src/Microsoft.Health.Fhir.Core/Features/Operations/BulkDelete/BulkDeleteProcessingJob.cs @@ -33,7 +33,6 @@ public class BulkDeleteProcessingJob : IJob private readonly Func> _deleterFactory; private readonly RequestContextAccessor _contextAccessor; private readonly IMediator _mediator; - private readonly ISearchParameterOperations _searchParameterOperations; private readonly Func> _searchService; private readonly IQueueClient _queueClient; @@ -41,14 +40,12 @@ public BulkDeleteProcessingJob( Func> deleterFactory, RequestContextAccessor contextAccessor, IMediator mediator, - ISearchParameterOperations searchParameterOperations, Func> searchService, IQueueClient queueClient) { _deleterFactory = EnsureArg.IsNotNull(deleterFactory, nameof(deleterFactory)); _contextAccessor = EnsureArg.IsNotNull(contextAccessor, nameof(contextAccessor)); _mediator = EnsureArg.IsNotNull(mediator, nameof(mediator)); - _searchParameterOperations = EnsureArg.IsNotNull(searchParameterOperations, nameof(searchParameterOperations)); _searchService = EnsureArg.IsNotNull(searchService, nameof(searchService)); _queueClient = EnsureArg.IsNotNull(queueClient, nameof(queueClient)); } @@ -83,11 +80,6 @@ public async Task ExecuteAsync(JobInfo jobInfo, CancellationToken cancel Exception exception = null; List types = definition.Type.SplitByOrSeparator().ToList(); - if (CanAffectSearchParameters(types, definition.ExcludedResourceTypes)) - { - await _searchParameterOperations.EnsureNoActiveReindexJobAsync(cancellationToken); - } - try { resourcesDeleted = await deleter.Value.DeleteMultipleAsync( @@ -144,15 +136,5 @@ public async Task ExecuteAsync(JobInfo jobInfo, CancellationToken cancel _contextAccessor.RequestContext = existingFhirRequestContext; } } - - private static bool CanAffectSearchParameters(IReadOnlyCollection resourceTypes, IList excludedResourceTypes) - { - if (excludedResourceTypes?.Any(x => string.Equals(x, KnownResourceTypes.SearchParameter, StringComparison.OrdinalIgnoreCase)) == true) - { - return false; - } - - return resourceTypes.Any(x => string.Equals(x, KnownResourceTypes.SearchParameter, StringComparison.OrdinalIgnoreCase)); - } } } diff --git a/src/Microsoft.Health.Fhir.Core/Features/Search/Parameters/ISearchParameterOperations.cs b/src/Microsoft.Health.Fhir.Core/Features/Search/Parameters/ISearchParameterOperations.cs index 07acdb427b..11dbd57673 100644 --- a/src/Microsoft.Health.Fhir.Core/Features/Search/Parameters/ISearchParameterOperations.cs +++ b/src/Microsoft.Health.Fhir.Core/Features/Search/Parameters/ISearchParameterOperations.cs @@ -23,8 +23,6 @@ public interface ISearchParameterOperations Task UpdateSearchParameterStatusAsync(IReadOnlyCollection searchParameterUris, SearchParameterStatus status, CancellationToken cancellationToken, bool ignoreSearchParameterNotSupportedException = false); - Task EnsureNoActiveReindexJobAsync(CancellationToken cancellationToken); - /// /// This method should be called to get any updates to search param cache /// diff --git a/src/Microsoft.Health.Fhir.Core/Features/Search/Parameters/SearchParameterOperations.cs b/src/Microsoft.Health.Fhir.Core/Features/Search/Parameters/SearchParameterOperations.cs index 48262cdcf4..0c41a4b027 100644 --- a/src/Microsoft.Health.Fhir.Core/Features/Search/Parameters/SearchParameterOperations.cs +++ b/src/Microsoft.Health.Fhir.Core/Features/Search/Parameters/SearchParameterOperations.cs @@ -35,7 +35,6 @@ public class SearchParameterOperations : ISearchParameterOperations, IDisposable private readonly IModelInfoProvider _modelInfoProvider; private readonly ISearchParameterSupportResolver _searchParameterSupportResolver; private readonly IDataStoreSearchParameterValidator _dataStoreSearchParameterValidator; - private readonly Func> _fhirOperationDataStoreFactory; private readonly Func> _searchServiceFactory; private readonly IScopeProvider _fhirDataStoreFactory; private readonly IResourceWrapperFactory _resourceWrapperFactory; @@ -50,7 +49,6 @@ public SearchParameterOperations( IModelInfoProvider modelInfoProvider, ISearchParameterSupportResolver searchParameterSupportResolver, IDataStoreSearchParameterValidator dataStoreSearchParameterValidator, - Func> fhirOperationDataStoreFactory, Func> searchServiceFactory, IScopeProvider fhirDataStoreFactory, IResourceWrapperFactory resourceWrapperFactory, @@ -61,7 +59,6 @@ public SearchParameterOperations( EnsureArg.IsNotNull(modelInfoProvider, nameof(modelInfoProvider)); EnsureArg.IsNotNull(searchParameterSupportResolver, nameof(searchParameterSupportResolver)); EnsureArg.IsNotNull(dataStoreSearchParameterValidator, nameof(dataStoreSearchParameterValidator)); - EnsureArg.IsNotNull(fhirOperationDataStoreFactory, nameof(fhirOperationDataStoreFactory)); EnsureArg.IsNotNull(searchServiceFactory, nameof(searchServiceFactory)); EnsureArg.IsNotNull(fhirDataStoreFactory, nameof(fhirDataStoreFactory)); EnsureArg.IsNotNull(resourceWrapperFactory, nameof(resourceWrapperFactory)); @@ -72,7 +69,6 @@ public SearchParameterOperations( _modelInfoProvider = modelInfoProvider; _searchParameterSupportResolver = searchParameterSupportResolver; _dataStoreSearchParameterValidator = dataStoreSearchParameterValidator; - _fhirOperationDataStoreFactory = fhirOperationDataStoreFactory; _searchServiceFactory = searchServiceFactory; _fhirDataStoreFactory = fhirDataStoreFactory; _resourceWrapperFactory = resourceWrapperFactory; @@ -107,17 +103,6 @@ public string GetSearchParameterHash(string resourceType) } } - public async Task EnsureNoActiveReindexJobAsync(CancellationToken cancellationToken) - { - using IScoped fhirOperationDataStore = _fhirOperationDataStoreFactory(); - (bool found, string id) activeReindexJob = await fhirOperationDataStore.Value.CheckActiveReindexJobsAsync(cancellationToken); - - if (activeReindexJob.found) - { - throw new JobConflictException(string.Format(Core.Resources.ChangesToSearchParametersNotAllowedWhileReindexing, activeReindexJob.id)); - } - } - public async Task ValidateSearchParameterAsync(ITypedElement searchParam, CancellationToken cancellationToken, DateTimeOffset? lastUpdated = null) { var searchParameterWrapper = new SearchParameterWrapper(searchParam); @@ -199,8 +184,6 @@ public async Task DeleteSearchParameterAsync(RawResource searchParamResource, Ca try { - await EnsureNoActiveReindexJobAsync(cancellationToken); - _logger.LogInformation("DeleteSearchParameterAsync: Refreshing cache"); await GetAndApplySearchParameterUpdates(cancellationToken); var status = isHardDelete ? SearchParameterStatus.PendingHardDelete : SearchParameterStatus.PendingDelete; @@ -232,7 +215,6 @@ public async Task DeleteSearchParameterAsync(RawResource searchParamResource, Ca public async Task UpdateSearchParameterStatusAsync(IReadOnlyCollection searchParameterUris, SearchParameterStatus status, CancellationToken cancellationToken, bool ignoreSearchParameterNotSupportedException = false) { - await EnsureNoActiveReindexJobAsync(cancellationToken); await _searchParameterStatusManager.UpdateSearchParameterStatusAsync(searchParameterUris, status, cancellationToken, ignoreSearchParameterNotSupportedException); } diff --git a/src/Microsoft.Health.Fhir.CosmosDb/Features/Storage/Registry/CosmosDbSearchParameterStatusDataStore.cs b/src/Microsoft.Health.Fhir.CosmosDb/Features/Storage/Registry/CosmosDbSearchParameterStatusDataStore.cs index fbf380f082..e001d31ad5 100644 --- a/src/Microsoft.Health.Fhir.CosmosDb/Features/Storage/Registry/CosmosDbSearchParameterStatusDataStore.cs +++ b/src/Microsoft.Health.Fhir.CosmosDb/Features/Storage/Registry/CosmosDbSearchParameterStatusDataStore.cs @@ -13,8 +13,10 @@ using Microsoft.Health.Core; using Microsoft.Health.Extensions.DependencyInjection; using Microsoft.Health.Fhir.Core.Extensions; +using Microsoft.Health.Fhir.Core.Features.Operations; using Microsoft.Health.Fhir.Core.Features.Search.Registry; using Microsoft.Health.Fhir.CosmosDb.Core.Configs; +using Microsoft.Health.JobManagement; namespace Microsoft.Health.Fhir.CosmosDb.Features.Storage.Registry { @@ -22,19 +24,23 @@ public sealed class CosmosDbSearchParameterStatusDataStore : ISearchParameterSta { private readonly Func> _containerScopeFactory; private readonly ICosmosQueryFactory _queryFactory; + private readonly IQueueClient _queueClient; private readonly SemaphoreSlim _statusListSemaphore = new(1, 1); public CosmosDbSearchParameterStatusDataStore( Func> containerScopeFactory, CosmosDataStoreConfiguration cosmosDataStoreConfiguration, - ICosmosQueryFactory queryFactory) + ICosmosQueryFactory queryFactory, + IQueueClient queueClient) { EnsureArg.IsNotNull(containerScopeFactory, nameof(containerScopeFactory)); EnsureArg.IsNotNull(cosmosDataStoreConfiguration, nameof(cosmosDataStoreConfiguration)); EnsureArg.IsNotNull(queryFactory, nameof(queryFactory)); + EnsureArg.IsNotNull(queueClient, nameof(queueClient)); _containerScopeFactory = containerScopeFactory; _queryFactory = queryFactory; + _queueClient = queueClient; } public string SearchParamCacheUpdateProcessName => null; @@ -96,6 +102,17 @@ public async Task UpsertStatuses(IReadOnlyList st return; } + // Check for active reindex jobs unless this call is from a reindex job itself + if (!reindexId.HasValue || reindexId.Value <= 0) + { + var activeJobs = await _queueClient.GetActiveJobsByQueueTypeAsync((byte)QueueType.Reindex, returnParentOnly: true, cancellationToken); + if (activeJobs.Count > 0) + { + var jobId = activeJobs[0].Id; + throw new Fhir.Core.Features.Operations.JobConflictException(string.Format(Fhir.Core.Resources.ChangesToSearchParametersNotAllowedWhileReindexing, jobId)); + } + } + foreach (IEnumerable statusBatch in statuses.TakeBatch(100)) { using IScoped clientScope = _containerScopeFactory.Invoke(); diff --git a/src/Microsoft.Health.Fhir.Shared.Api.UnitTests/Controllers/BulkDeleteControllerTests.cs b/src/Microsoft.Health.Fhir.Shared.Api.UnitTests/Controllers/BulkDeleteControllerTests.cs index 278b145834..20eaa8652d 100644 --- a/src/Microsoft.Health.Fhir.Shared.Api.UnitTests/Controllers/BulkDeleteControllerTests.cs +++ b/src/Microsoft.Health.Fhir.Shared.Api.UnitTests/Controllers/BulkDeleteControllerTests.cs @@ -47,12 +47,10 @@ public class BulkDeleteControllerTests private readonly BulkDeleteController _controller; private readonly HttpRequest _httpRequest; private readonly IMediator _mediator; - private readonly ISearchParameterOperations _searchParameterOperations; public BulkDeleteControllerTests() { _mediator = Substitute.For(); - _searchParameterOperations = Substitute.For(); _mediator.Send( Arg.Any(), Arg.Any()) @@ -89,7 +87,6 @@ public BulkDeleteControllerTests() _controller = new BulkDeleteController( _mediator, - _searchParameterOperations, () => scopedSearchService, urlResolver); _controller.ControllerContext = new ControllerContext( @@ -201,30 +198,6 @@ await Run( purgeHistory)); } - [Fact] - public async Task GivenBulkDeleteForSearchParameter_WhenReindexIsRunning_ThenConflictIsThrown() - { - _searchParameterOperations - .When(x => x.EnsureNoActiveReindexJobAsync(Arg.Any())) - .Do(_ => throw new FhirJobConflictException("reindex running")); - - await Assert.ThrowsAsync(() => _controller.BulkDeleteByResourceType(KnownResourceTypes.SearchParameter, new HardDeleteModel(), false, false)); - await _mediator.DidNotReceiveWithAnyArgs().Send(default, default); - } - - [Fact] - public async Task GivenBulkDeleteWithSearchParameterExcluded_WhenReindexIsRunning_ThenRequestStillSucceeds() - { - _searchParameterOperations - .When(x => x.EnsureNoActiveReindexJobAsync(Arg.Any())) - .Do(_ => throw new FhirJobConflictException("reindex running")); - - var response = await _controller.BulkDelete(new HardDeleteModel(), false, false, KnownResourceTypes.SearchParameter); - - Assert.IsType(response); - await _searchParameterOperations.DidNotReceive().EnsureNoActiveReindexJobAsync(Arg.Any()); - } - [Theory] [InlineData(0, HttpStatusCode.Accepted)] [InlineData(1, HttpStatusCode.OK)] diff --git a/src/Microsoft.Health.Fhir.Shared.Api/Controllers/BulkDeleteController.cs b/src/Microsoft.Health.Fhir.Shared.Api/Controllers/BulkDeleteController.cs index 15df6132bf..81e556ef87 100644 --- a/src/Microsoft.Health.Fhir.Shared.Api/Controllers/BulkDeleteController.cs +++ b/src/Microsoft.Health.Fhir.Shared.Api/Controllers/BulkDeleteController.cs @@ -40,7 +40,6 @@ namespace Microsoft.Health.Fhir.Api.Controllers public class BulkDeleteController : Controller { private readonly IMediator _mediator; - private readonly ISearchParameterOperations _searchParameterOperations; private readonly Func> _searchService; private readonly IUrlResolver _urlResolver; @@ -55,12 +54,10 @@ public class BulkDeleteController : Controller public BulkDeleteController( IMediator mediator, - ISearchParameterOperations searchParameterOperations, Func> searchService, IUrlResolver urlResolver) { _mediator = EnsureArg.IsNotNull(mediator, nameof(mediator)); - _searchParameterOperations = EnsureArg.IsNotNull(searchParameterOperations, nameof(searchParameterOperations)); _searchService = EnsureArg.IsNotNull(searchService, nameof(searchService)); _urlResolver = EnsureArg.IsNotNull(urlResolver, nameof(urlResolver)); } @@ -158,33 +155,11 @@ private async Task SendDeleteRequest(string typeParameter, bool h excludedResourceTypesList = excludedResourceTypes.Split(',').ToList(); } - if (await CanAffectSearchParametersAsync(typeParameter, excludedResourceTypesList)) - { - await _searchParameterOperations.EnsureNoActiveReindexJobAsync(HttpContext.RequestAborted); - } - CreateBulkDeleteResponse result = await _mediator.BulkDeleteAsync(deleteOperation, typeParameter, searchParameters, softDeleteCleanup, excludedResourceTypesList, removeReferences, HttpContext.RequestAborted); var response = JobResult.Accepted(); response.SetContentLocationHeader(_urlResolver, OperationsConstants.BulkDelete, result.Id.ToString()); return response; } - - private async Task CanAffectSearchParametersAsync(string resourceType, IList excludedResourceTypes) - { - if (excludedResourceTypes?.Any(x => string.Equals(x, KnownResourceTypes.SearchParameter, StringComparison.OrdinalIgnoreCase)) == true) - { - return false; - } - - if (!string.IsNullOrWhiteSpace(resourceType)) - { - return resourceType.SplitByOrSeparator().Any(x => string.Equals(x, KnownResourceTypes.SearchParameter, StringComparison.OrdinalIgnoreCase)); - } - - using IScoped searchService = _searchService.Invoke(); - IReadOnlyList usedResourceTypes = await searchService.Value.GetUsedResourceTypes(HttpContext.RequestAborted); - return usedResourceTypes.Any(x => string.Equals(x, KnownResourceTypes.SearchParameter, StringComparison.OrdinalIgnoreCase)); - } } } diff --git a/src/Microsoft.Health.Fhir.Shared.Core.UnitTests/Features/Operations/SearchParameterState/SearchParameterStateUpdateHandlerTests.cs b/src/Microsoft.Health.Fhir.Shared.Core.UnitTests/Features/Operations/SearchParameterState/SearchParameterStateUpdateHandlerTests.cs index e21caf1fdc..798c19e58d 100644 --- a/src/Microsoft.Health.Fhir.Shared.Core.UnitTests/Features/Operations/SearchParameterState/SearchParameterStateUpdateHandlerTests.cs +++ b/src/Microsoft.Health.Fhir.Shared.Core.UnitTests/Features/Operations/SearchParameterState/SearchParameterStateUpdateHandlerTests.cs @@ -111,7 +111,6 @@ public SearchParameterStateUpdateHandlerTests() _cancellationToken = CancellationToken.None; _authorizationService.CheckAccess(DataActions.SearchParameter, _cancellationToken).Returns(DataActions.SearchParameter); - _searchParameterOperations.EnsureNoActiveReindexJobAsync(Arg.Any()).Returns(Task.CompletedTask); _searchParameterOperations.UpdateSearchParameterStatusAsync(Arg.Any>(), Arg.Any(), Arg.Any(), Arg.Any()).Returns(Task.CompletedTask); var searchParamDefinitionStore = new List @@ -335,16 +334,6 @@ public async Task GivenARequestToUpdateSearchParameterStatus_WhenRequestIsValied Assert.Contains("Status=PendingDisable", loggers.logger.LogRecords[0].State.ToString()); } - [Fact] - public async Task GivenReindexRunningBeforeStatusUpdate_WhenHandlingRequest_ThenJobConflictIsThrown() - { - _searchParameterOperations - .When(x => x.EnsureNoActiveReindexJobAsync(Arg.Any())) - .Do(_ => throw new FhirJobConflictException("reindex running")); - - await Assert.ThrowsAsync(() => _searchParameterStateUpdateHandler.Handle(new SearchParameterStateUpdateRequest(new List>()), default)); - } - private (IAuditLogger auditLogger, TestLogger logger) CreateTestAuditLogger() { IOptions optionsConfig = Substitute.For>(); diff --git a/src/Microsoft.Health.Fhir.Shared.Core.UnitTests/Features/Search/SearchParameters/SearchParameterDefinitionManagerTests.cs b/src/Microsoft.Health.Fhir.Shared.Core.UnitTests/Features/Search/SearchParameters/SearchParameterDefinitionManagerTests.cs index 394f643b9d..230b9ae3d0 100644 --- a/src/Microsoft.Health.Fhir.Shared.Core.UnitTests/Features/Search/SearchParameters/SearchParameterDefinitionManagerTests.cs +++ b/src/Microsoft.Health.Fhir.Shared.Core.UnitTests/Features/Search/SearchParameters/SearchParameterDefinitionManagerTests.cs @@ -148,7 +148,6 @@ public SearchParameterDefinitionManagerTests() var searchParameterDataStoreValidator = Substitute.For(); searchParameterDataStoreValidator.ValidateSearchParameter(Arg.Any(), out Arg.Any()).Returns(true, null); - var fhirOperationDataStore = Substitute.For(); var searchService = Substitute.For(); var fhirDataStore = Substitute.For(); @@ -158,7 +157,6 @@ public SearchParameterDefinitionManagerTests() ModelInfoProvider.Instance, _searchParameterSupportResolver, searchParameterDataStoreValidator, - () => fhirOperationDataStore.CreateMockScope(), () => searchService.CreateMockScope(), fhirDataStore.CreateMockScopeProvider(), Substitute.For(), diff --git a/src/Microsoft.Health.Fhir.Shared.Core.UnitTests/Features/Search/SearchParameters/SearchParameterValidatorTests.cs b/src/Microsoft.Health.Fhir.Shared.Core.UnitTests/Features/Search/SearchParameters/SearchParameterValidatorTests.cs index 7ae8ef6fd1..89d9eb17c1 100644 --- a/src/Microsoft.Health.Fhir.Shared.Core.UnitTests/Features/Search/SearchParameters/SearchParameterValidatorTests.cs +++ b/src/Microsoft.Health.Fhir.Shared.Core.UnitTests/Features/Search/SearchParameters/SearchParameterValidatorTests.cs @@ -68,7 +68,6 @@ public SearchParameterValidatorTests() x[3] = searchParameterInfo; return true; }); - _searchParameterOperations.EnsureNoActiveReindexJobAsync(CancellationToken.None).Returns(Task.CompletedTask); _searchParameterOperations.SearchParamLastUpdated.Returns(System.DateTimeOffset.UtcNow); } @@ -318,18 +317,6 @@ public async Task GivenCustomSearchParameter_WhenUpdating_ThenNoExceptionThrown( await validator.ValidateSearchParameterInput(searchParam, "PUT", CancellationToken.None); } - [Fact] - public async Task GivenActiveReindexJob_WhenValidatingSearchParam_ThenJobConflictExceptionThrown() - { - _searchParameterOperations - .When(x => x.EnsureNoActiveReindexJobAsync(Arg.Any())) - .Do(_ => throw new System.Exception("reindex running")); - - var exception = await Assert.ThrowsAsync(() => CreateValidator().ValidateSearchParameterInput(new SearchParameter { Url = "http://unique" }, "POST", CancellationToken.None)); - - Assert.Equal("reindex running", exception.Message); - } - private SearchParameterValidator CreateValidator() => new SearchParameterValidator( _authorizationService, diff --git a/src/Microsoft.Health.Fhir.Shared.Core/Features/Operations/SearchParameterState/SearchParameterStateUpdateHandler.cs b/src/Microsoft.Health.Fhir.Shared.Core/Features/Operations/SearchParameterState/SearchParameterStateUpdateHandler.cs index 977a212fff..3c99f1dbb6 100644 --- a/src/Microsoft.Health.Fhir.Shared.Core/Features/Operations/SearchParameterState/SearchParameterStateUpdateHandler.cs +++ b/src/Microsoft.Health.Fhir.Shared.Core/Features/Operations/SearchParameterState/SearchParameterStateUpdateHandler.cs @@ -59,8 +59,6 @@ public async Task Handle(SearchParameterStat await _authorizationService.CheckAccess(DataActions.SearchParameter, true, cancellationToken); - await _searchParameterOperations.EnsureNoActiveReindexJobAsync(cancellationToken); - _resourceSearchParameterStatus = await _searchParameterStatusManager.GetAllSearchParameterStatus(cancellationToken); Dictionary> searchParametersToUpdate = ParseRequestForUpdate(request, out List invalidSearchParameters); diff --git a/src/Microsoft.Health.Fhir.Shared.Core/Features/Search/Parameters/SearchParameterValidator.cs b/src/Microsoft.Health.Fhir.Shared.Core/Features/Search/Parameters/SearchParameterValidator.cs index c656183392..9e8bb21c79 100644 --- a/src/Microsoft.Health.Fhir.Shared.Core/Features/Search/Parameters/SearchParameterValidator.cs +++ b/src/Microsoft.Health.Fhir.Shared.Core/Features/Search/Parameters/SearchParameterValidator.cs @@ -73,8 +73,6 @@ public SearchParameterValidator( { await _authorizationService.CheckAccess(DataActions.Reindex, true, cancellationToken); - await _searchParameterOperations.EnsureNoActiveReindexJobAsync(cancellationToken); - if (string.IsNullOrEmpty(searchParam.Url) && (method.Equals(HttpDeleteName, StringComparison.Ordinal) || method.Equals(HttpPatchName, StringComparison.Ordinal))) { // Return out if this is delete OR patch call and no Url so FHIRController can move to next action diff --git a/src/Microsoft.Health.Fhir.SqlServer/Features/ExceptionExtension.cs b/src/Microsoft.Health.Fhir.SqlServer/Features/ExceptionExtension.cs index d907d13e2d..2f50794775 100644 --- a/src/Microsoft.Health.Fhir.SqlServer/Features/ExceptionExtension.cs +++ b/src/Microsoft.Health.Fhir.SqlServer/Features/ExceptionExtension.cs @@ -35,6 +35,14 @@ public static bool IsSearchParameterConcurrencyConflict(this Exception e) && e.Message.Contains("expected last updated", StringComparison.OrdinalIgnoreCase); } + public static bool IsReindexJobConflict(this Exception e) + { + var sqlEx = e as SqlException; + return sqlEx != null + && sqlEx.Number == 50002 + && e.Message.Contains("reindex job is in progress", StringComparison.OrdinalIgnoreCase); + } + private static bool HasDeadlockErrorPattern(string str) { return str.Contains("deadlock", StringComparison.OrdinalIgnoreCase); diff --git a/src/Microsoft.Health.Fhir.SqlServer/Features/Storage/Registry/SqlServerSearchParameterStatusDataStore.cs b/src/Microsoft.Health.Fhir.SqlServer/Features/Storage/Registry/SqlServerSearchParameterStatusDataStore.cs index 0fd095aa98..b9bfe40116 100644 --- a/src/Microsoft.Health.Fhir.SqlServer/Features/Storage/Registry/SqlServerSearchParameterStatusDataStore.cs +++ b/src/Microsoft.Health.Fhir.SqlServer/Features/Storage/Registry/SqlServerSearchParameterStatusDataStore.cs @@ -151,6 +151,11 @@ public async Task UpsertStatuses(IReadOnlyList st _logger.LogWarning(ex, $"Optimistic concurrency conflict occurred while calling dbo.MergeSearchParams. ReindexId={reindexId ?? 0}"); throw new BadRequestException(Core.Resources.SearchParameterConcurrencyConflict); } + catch (SqlException ex) when (ex.IsReindexJobConflict()) + { + _logger.LogWarning(ex, $"Reindex job conflict occurred while calling dbo.MergeSearchParams. ReindexId={reindexId ?? 0}"); + throw new JobConflictException(ex.Message); + } } // Synchronize the FHIR model dictionary with the data in SQL search parameter status table diff --git a/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Features/Operations/Reindex/ReindexJobTests.cs b/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Features/Operations/Reindex/ReindexJobTests.cs index 4d307a8fad..94ac82fb59 100644 --- a/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Features/Operations/Reindex/ReindexJobTests.cs +++ b/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Features/Operations/Reindex/ReindexJobTests.cs @@ -151,7 +151,6 @@ public async Task InitializeAsync() ModelInfoProvider.Instance, _searchParameterSupportResolver, _dataStoreSearchParameterValidator, - () => _fhirOperationDataStore.CreateMockScope(), () => _searchService, _scopedDataStore.CreateMockScopeProviderFromScoped(), _resourceWrapperFactory, @@ -1535,7 +1534,6 @@ private async Task InitializeSecondFHIRService() ModelInfoProvider.Instance, _searchParameterSupportResolver, _dataStoreSearchParameterValidator, - () => _fhirOperationDataStore.CreateMockScope(), () => _searchService, _scopedDataStore.CreateMockScopeProviderFromScoped(), _resourceWrapperFactory, 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 1c38f43830..537ee2b15d 100644 --- a/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Persistence/CosmosDbFhirStorageTestsFixture.cs +++ b/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Persistence/CosmosDbFhirStorageTestsFixture.cs @@ -248,10 +248,15 @@ public virtual async Task InitializeAsync() var documentClient = new NonDisposingScope(_container); + var mockQueueClientForSearchParams = Substitute.For(); + mockQueueClientForSearchParams.GetActiveJobsByQueueTypeAsync(Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(Task.FromResult((IReadOnlyList)Array.Empty())); + _searchParameterStatusDataStore = new CosmosDbSearchParameterStatusDataStore( () => documentClient, _cosmosDataStoreConfiguration, - cosmosDocumentQueryFactory); + cosmosDocumentQueryFactory, + mockQueueClientForSearchParams); var bundleConfiguration = new BundleConfiguration() { SupportsBundleOrchestrator = true }; var bundleOptions = Substitute.For>(); diff --git a/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Persistence/FhirStorageTestsFixture.cs b/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Persistence/FhirStorageTestsFixture.cs index 74ea56cce6..1fa5379e11 100644 --- a/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Persistence/FhirStorageTestsFixture.cs +++ b/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Persistence/FhirStorageTestsFixture.cs @@ -331,7 +331,6 @@ public async Task InitializeAsync() ModelInfoProvider.Instance, searchParameterSupportResolver, dataStoreSearchParameterValidator, - () => OperationDataStore.CreateMockScope(), () => SearchService.CreateMockScope(), DataStore.CreateMockScopeProvider(), resourceWrapperFactory, diff --git a/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Persistence/SqlServerSearchParameterStatusDataStoreTests.cs b/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Persistence/SqlServerSearchParameterStatusDataStoreTests.cs index 871a41af47..06abfe341b 100644 --- a/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Persistence/SqlServerSearchParameterStatusDataStoreTests.cs +++ b/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Persistence/SqlServerSearchParameterStatusDataStoreTests.cs @@ -580,5 +580,151 @@ public async Task GivenGetSearchParameterStatuses_WhenCalled_ThenReturnsConsiste Assert.True(sqlStatus.Id > 0, "SqlServerResourceSearchParameterStatus should have a valid Id"); }); } + + [Fact] + public async Task GivenActiveReindexJob_WhenUpsertingSearchParameterStatus_ThenJobConflictExceptionIsThrown() + { + // This test validates that search parameter status updates are blocked when a reindex job is running + // This ensures bulk delete operations targeting SearchParameter resources will fail during reindex + + await _fixture.SearchParameterOperations.GetAndApplySearchParameterUpdates(CancellationToken.None); + + // Arrange - Create a test search parameter status + var testUri = "http://hl7.org/fhir/SearchParameter/Test-ReindexConflict-" + Guid.NewGuid(); + var status = new ResourceSearchParameterStatus + { + Uri = new Uri(testUri), + Status = SearchParameterStatus.Enabled, + IsPartiallySupported = false, + LastUpdated = _fixture.SearchParameterOperations.SearchParamLastUpdated, + }; + + // Start a reindex job + var reindexJobRecord = new Core.Features.Operations.Reindex.Models.ReindexJobRecord(new List()); + var reindexJob = await _fixture.OperationDataStore.CreateReindexJobAsync(reindexJobRecord, CancellationToken.None); + + try + { + // Act & Assert - Upserting without reindexId should throw JobConflictException + var exception = await Assert.ThrowsAsync(async () => + { + await _fixture.SearchParameterStatusDataStore.UpsertStatuses([status], CancellationToken.None, reindexId: null); + }); + + Assert.Contains("reindex", exception.Message, StringComparison.OrdinalIgnoreCase); + Assert.Contains(reindexJob.JobRecord.Id, exception.Message); + } + finally + { + // Cleanup - Cancel the reindex job + await _fixture.OperationDataStore.UpdateReindexJobAsync( + new Core.Features.Operations.Reindex.Models.ReindexJobRecord(new List()) + { + Id = reindexJob.JobRecord.Id, + Status = OperationStatus.Canceled, + }, + reindexJob.ETag, + CancellationToken.None); + } + } + + [Fact] + public async Task GivenActiveReindexJob_WhenUpsertingWithReindexId_ThenSucceeds() + { + // This test validates that the reindex job itself can update search parameter statuses + // by passing its job ID + + await _fixture.SearchParameterOperations.GetAndApplySearchParameterUpdates(CancellationToken.None); + + // Arrange - Create a test search parameter status + var testUri = "http://hl7.org/fhir/SearchParameter/Test-ReindexBypass-" + Guid.NewGuid(); + var status = new ResourceSearchParameterStatus + { + Uri = new Uri(testUri), + Status = SearchParameterStatus.Enabled, + IsPartiallySupported = false, + LastUpdated = _fixture.SearchParameterOperations.SearchParamLastUpdated, + }; + + // Start a reindex job + var reindexJobRecord = new Core.Features.Operations.Reindex.Models.ReindexJobRecord(new List()); + var reindexJob = await _fixture.OperationDataStore.CreateReindexJobAsync(reindexJobRecord, CancellationToken.None); + var jobId = long.Parse(reindexJob.JobRecord.Id); + + try + { + // Act - Upserting with reindexId should succeed (bypass the check) + await _fixture.SearchParameterStatusDataStore.UpsertStatuses([status], CancellationToken.None, reindexId: jobId); + + // Assert - Verify the status was created + var allStatuses = await _fixture.SearchParameterStatusDataStore.GetSearchParameterStatuses(CancellationToken.None); + var createdStatus = allStatuses.FirstOrDefault(s => s.Uri.OriginalString == testUri); + Assert.NotNull(createdStatus); + Assert.Equal(SearchParameterStatus.Enabled, createdStatus.Status); + } + finally + { + // Cleanup + await _testHelper.DeleteSearchParameterStatusAsync(testUri); + await _fixture.OperationDataStore.UpdateReindexJobAsync( + new Core.Features.Operations.Reindex.Models.ReindexJobRecord(new List()) + { + Id = reindexJob.JobRecord.Id, + Status = OperationStatus.Canceled, + }, + reindexJob.ETag, + CancellationToken.None); + } + } + + [Fact] + public async Task GivenNoActiveReindexJob_WhenUpsertingSearchParameterStatus_ThenSucceeds() + { + // This test validates that when no reindex is running, search parameter updates work normally + // This ensures bulk delete operations can proceed when no reindex is active + + await _fixture.SearchParameterOperations.GetAndApplySearchParameterUpdates(CancellationToken.None); + + // Arrange - Ensure no active reindex jobs + var (found, id) = await _fixture.OperationDataStore.CheckActiveReindexJobsAsync(CancellationToken.None); + if (found) + { + // Cancel any active reindex job first + var existingJob = await _fixture.OperationDataStore.GetReindexJobByIdAsync(id, CancellationToken.None); + await _fixture.OperationDataStore.UpdateReindexJobAsync( + new Core.Features.Operations.Reindex.Models.ReindexJobRecord(new List()) + { + Id = existingJob.JobRecord.Id, + Status = OperationStatus.Canceled, + }, + existingJob.ETag, + CancellationToken.None); + } + + var testUri = "http://hl7.org/fhir/SearchParameter/Test-NoReindex-" + Guid.NewGuid(); + var status = new ResourceSearchParameterStatus + { + Uri = new Uri(testUri), + Status = SearchParameterStatus.Enabled, + IsPartiallySupported = false, + LastUpdated = _fixture.SearchParameterOperations.SearchParamLastUpdated, + }; + + try + { + // Act - Should succeed without any reindex job running + await _fixture.SearchParameterStatusDataStore.UpsertStatuses([status], CancellationToken.None); + + // Assert - Verify the status was created + var allStatuses = await _fixture.SearchParameterStatusDataStore.GetSearchParameterStatuses(CancellationToken.None); + var createdStatus = allStatuses.FirstOrDefault(s => s.Uri.OriginalString == testUri); + Assert.NotNull(createdStatus); + Assert.Equal(SearchParameterStatus.Enabled, createdStatus.Status); + } + finally + { + await _testHelper.DeleteSearchParameterStatusAsync(testUri); + } + } } } From 547afc2e7e1a59cc6968cfc99e02782fc2122c24 Mon Sep 17 00:00:00 2001 From: Sergey Galuzo Date: Fri, 26 Jun 2026 16:16:39 -0700 Subject: [PATCH 02/31] bulk delete test fixes --- .../BulkDeleteProcessingJobTests.cs | 39 ++++++++++++++++--- 1 file changed, 33 insertions(+), 6 deletions(-) diff --git a/src/Microsoft.Health.Fhir.Core.UnitTests/Features/Operations/BulkDelete/BulkDeleteProcessingJobTests.cs b/src/Microsoft.Health.Fhir.Core.UnitTests/Features/Operations/BulkDelete/BulkDeleteProcessingJobTests.cs index 8729441555..22c59cc660 100644 --- a/src/Microsoft.Health.Fhir.Core.UnitTests/Features/Operations/BulkDelete/BulkDeleteProcessingJobTests.cs +++ b/src/Microsoft.Health.Fhir.Core.UnitTests/Features/Operations/BulkDelete/BulkDeleteProcessingJobTests.cs @@ -121,19 +121,46 @@ public async Task GivenProcessingJobForSearchParameter_WhenReindexStartsBeforeEx } [Fact] - public async Task GivenBulkDeleteWithSearchParameterLaterInTheChain_WhenReindexStartsBeforeExecution_ThenConflictIsThrownBeforeAnyDelete() + public async Task GivenBulkDeleteWithSearchParameterFirst_WhenReindexIsActive_ThenConflictIsThrownImmediately() + { + var definition = new BulkDeleteDefinition(JobType.BulkDeleteProcessing, DeleteOperation.HardDelete, "SearchParameter,Patient", new List>(), new List(), "https:\\\\test.com", "https:\\\\test.com", "test"); + var jobInfo = new JobInfo() { Id = 1, Definition = JsonConvert.SerializeObject(definition) }; + + // SearchParameter processed first - database layer throws conflict due to active reindex + _deleter.DeleteMultipleAsync(Arg.Any(), Arg.Any(), Arg.Any>()).Returns>>(x => throw new FhirJobConflictException("A reindex job is currently running.")); + + // The conflict should propagate through the workflow, preventing any deletions + await Assert.ThrowsAsync(async () => await _processingJob.ExecuteAsync(jobInfo, CancellationToken.None)); + } + + [Fact] + public async Task GivenBulkDeleteWithSearchParameterSecond_WhenReindexStartsBetweenDeletes_ThenFollowUpJobFails() { var definition = new BulkDeleteDefinition(JobType.BulkDeleteProcessing, DeleteOperation.HardDelete, "Patient,SearchParameter", new List>(), new List(), "https:\\\\test.com", "https:\\\\test.com", "test"); var jobInfo = new JobInfo() { Id = 1, Definition = JsonConvert.SerializeObject(definition) }; - // First resource type (Patient) succeeds + // Patient processed first and succeeds (reindex hasn't started yet) var patientResults = new Dictionary { { "Patient", 5 } }; + _deleter.DeleteMultipleAsync(Arg.Any(), Arg.Any(), Arg.Any>()).Returns(Task.FromResult>(patientResults)); - // Second resource type (SearchParameter) fails due to active reindex - database layer throws conflict - _deleter.DeleteMultipleAsync(Arg.Any(), Arg.Any(), Arg.Any>()).Returns(x => Task.FromResult>(patientResults), x => throw new FhirJobConflictException("A reindex job is currently running.")); + // Execute the first job - should succeed and enqueue follow-up job for SearchParameter + var result = await _processingJob.ExecuteAsync(jobInfo, CancellationToken.None); + var bulkDeleteResult = JsonConvert.DeserializeObject(result); + Assert.Equal(5, bulkDeleteResult.ResourcesDeleted["Patient"]); - // The conflict should propagate through the workflow - await Assert.ThrowsAsync(async () => await _processingJob.ExecuteAsync(jobInfo, CancellationToken.None)); + // Verify a follow-up job was queued for SearchParameter + var calls = _queueClient.ReceivedCalls(); + var definitions = (string[])calls.First().GetArguments()[1]; + Assert.Single(definitions); + var followUpDefinition = JsonConvert.DeserializeObject(definitions[0]); + Assert.Equal("SearchParameter", followUpDefinition.Type); + + // Now simulate the follow-up job running, but reindex has started between the two jobs + _deleter.ClearReceivedCalls(); + _deleter.DeleteMultipleAsync(Arg.Any(), Arg.Any(), Arg.Any>()).Returns>>(x => throw new FhirJobConflictException("A reindex job is currently running.")); + + var followUpJobInfo = new JobInfo() { Id = 2, Definition = definitions[0] }; + await Assert.ThrowsAsync(async () => await _processingJob.ExecuteAsync(followUpJobInfo, CancellationToken.None)); } } } From a6e40a827ddd048d5ed0bfa8aec66c77e467014e Mon Sep 17 00:00:00 2001 From: Sergey Galuzo Date: Fri, 26 Jun 2026 19:36:42 -0700 Subject: [PATCH 03/31] search param status tests --- .../Features/Schema/Migrations/115.diff.sql | 84 +++++++++++++++++++ .../Features/Schema/SchemaVersion.cs | 1 + .../Features/Schema/SchemaVersionConstants.cs | 2 +- .../Schema/Sql/Sprocs/MergeSearchParams.sql | 31 ++----- .../Microsoft.Health.Fhir.SqlServer.csproj | 2 +- ...rverSearchParameterStatusDataStoreTests.cs | 80 ++++++++---------- 6 files changed, 129 insertions(+), 71 deletions(-) create mode 100644 src/Microsoft.Health.Fhir.SqlServer/Features/Schema/Migrations/115.diff.sql diff --git a/src/Microsoft.Health.Fhir.SqlServer/Features/Schema/Migrations/115.diff.sql b/src/Microsoft.Health.Fhir.SqlServer/Features/Schema/Migrations/115.diff.sql new file mode 100644 index 0000000000..c78844d08b --- /dev/null +++ b/src/Microsoft.Health.Fhir.SqlServer/Features/Schema/Migrations/115.diff.sql @@ -0,0 +1,84 @@ +ALTER PROCEDURE dbo.MergeSearchParams @SearchParams dbo.SearchParamList READONLY, @ReindexId bigint +-- @ReindexId = 0 - new code but not reindex. @ReindexId > 0 - new code and reindex. +AS +set nocount on +DECLARE @SP varchar(100) = object_name(@@procid) + ,@Mode varchar(200) = 'Cnt='+convert(varchar,(SELECT count(*) FROM @SearchParams))+' R='+convert(varchar,@ReindexId) + ,@st datetime = getUTCdate() + ,@LastUpdated datetimeoffset(7) = convert(datetimeoffset(7), sysUTCdatetime()) + ,@MaxLastUpdated datetimeoffset(7) + ,@msg varchar(4000) + ,@Rows int + ,@Uri varchar(4000) + ,@Status varchar(20) + ,@InputTrancount int = @@trancount + ,@ActiveJobId bigint + ,@ExpectedLastUpdated datetimeoffset(7) = (SELECT max(LastUpdated) FROM @SearchParams) + +SET @Mode = @Mode +' L='+isnull(convert(varchar(23),@ExpectedLastUpdated,126),'NULL') + +BEGIN TRY + IF @ReindexId IS NULL OR @ReindexId < 0 RAISERROR('@ReindexId cannot be null or negative', 18, 127) + + DECLARE @SearchParamsCopy dbo.SearchParamList + INSERT INTO @SearchParamsCopy SELECT * FROM @SearchParams + WHILE EXISTS (SELECT * FROM @SearchParamsCopy) + BEGIN + SELECT TOP 1 @Uri = Uri, @Status = Status FROM @SearchParamsCopy + SET @msg = 'Status='+@Status+' Uri='+@Uri + EXECUTE dbo.LogEvent @Process=@SP,@Mode=@Mode,@Status='Start',@Text=@msg + DELETE FROM @SearchParamsCopy WHERE Uri = @Uri + END + + IF @InputTrancount = 0 -- transaction can start in MergeResourcesAndSearchParams + BEGIN + SET TRANSACTION ISOLATION LEVEL SERIALIZABLE + + BEGIN TRANSACTION + END + + -- check if job id is valid + IF @ReindexId > 0 + AND NOT EXISTS (SELECT 1 FROM dbo.JobQueue WHERE PartitionId = @ReindexId % 16 AND QueueType = 6 AND JobId = @ReindexId AND Status = 1) + RAISERROR('Reindex job is not running', 18, 127) + + IF @ReindexId = 0 + BEGIN + -- Check if reindex job is running + EXECUTE dbo.GetActiveJobs @QueueType = 6, @IsExistsCheck = 1, @GroupId = @ActiveJobId OUT + SET @msg = 'Reindex job is in progress. Job Id='+convert(varchar,@ActiveJobId) + IF @ActiveJobId IS NOT NULL THROW 50002, @msg, 1 + + -- Check for concurrency conflicts using LastUpdated + SET @MaxLastUpdated = (SELECT max(LastUpdated) FROM dbo.SearchParam) + IF @MaxLastUpdated <> @ExpectedLastUpdated + BEGIN + SET @msg = 'Optimistic concurrency conflict detected : expected last updated = '+convert(varchar(23),@ExpectedLastUpdated,126)+' max last updated = '+convert(varchar(23),@MaxLastUpdated,126); + THROW 50001, @msg, 1 + END + END + + MERGE INTO dbo.SearchParam S + USING @SearchParams I ON I.Uri = S.Uri + WHEN MATCHED THEN + UPDATE + SET Status = I.Status + ,LastUpdated = @LastUpdated + ,IsPartiallySupported = I.IsPartiallySupported + WHEN NOT MATCHED BY TARGET THEN + INSERT ( Uri, Status, LastUpdated, IsPartiallySupported) + VALUES (I.Uri, I.Status, @LastUpdated, I.IsPartiallySupported); + SET @Rows = @@rowcount + + SET @msg = 'LastUpdated='+convert(varchar(23),@LastUpdated,126) + + IF @InputTrancount = 0 COMMIT TRANSACTION + + EXECUTE dbo.LogEvent @Process=@SP,@Mode=@Mode,@Status='End',@Start=@st,@Action='Merge',@Rows=@Rows,@Text=@msg +END TRY +BEGIN CATCH + IF @@trancount > 0 ROLLBACK TRANSACTION + EXECUTE dbo.LogEvent @Process=@SP,@Mode=@Mode,@Status='Error',@Start=@st; + THROW +END CATCH +GO diff --git a/src/Microsoft.Health.Fhir.SqlServer/Features/Schema/SchemaVersion.cs b/src/Microsoft.Health.Fhir.SqlServer/Features/Schema/SchemaVersion.cs index fa93b22312..7817e88223 100644 --- a/src/Microsoft.Health.Fhir.SqlServer/Features/Schema/SchemaVersion.cs +++ b/src/Microsoft.Health.Fhir.SqlServer/Features/Schema/SchemaVersion.cs @@ -124,5 +124,6 @@ public enum SchemaVersion V112 = 112, V113 = 113, V114 = 114, + V115 = 115, } } diff --git a/src/Microsoft.Health.Fhir.SqlServer/Features/Schema/SchemaVersionConstants.cs b/src/Microsoft.Health.Fhir.SqlServer/Features/Schema/SchemaVersionConstants.cs index 0c0adfc085..b49a942b56 100644 --- a/src/Microsoft.Health.Fhir.SqlServer/Features/Schema/SchemaVersionConstants.cs +++ b/src/Microsoft.Health.Fhir.SqlServer/Features/Schema/SchemaVersionConstants.cs @@ -8,7 +8,7 @@ namespace Microsoft.Health.Fhir.SqlServer.Features.Schema public static class SchemaVersionConstants { public const int Min = (int)SchemaVersion.V113; - public const int Max = (int)SchemaVersion.V114; + public const int Max = (int)SchemaVersion.V115; public const int MinForUpgrade = (int)SchemaVersion.V111; // this is used for upgrade tests only public const int SearchParameterStatusSchemaVersion = (int)SchemaVersion.V6; public const int SupportForReferencesWithMissingTypeVersion = (int)SchemaVersion.V7; diff --git a/src/Microsoft.Health.Fhir.SqlServer/Features/Schema/Sql/Sprocs/MergeSearchParams.sql b/src/Microsoft.Health.Fhir.SqlServer/Features/Schema/Sql/Sprocs/MergeSearchParams.sql index e06cbb4647..23468865ff 100644 --- a/src/Microsoft.Health.Fhir.SqlServer/Features/Schema/Sql/Sprocs/MergeSearchParams.sql +++ b/src/Microsoft.Health.Fhir.SqlServer/Features/Schema/Sql/Sprocs/MergeSearchParams.sql @@ -1,5 +1,5 @@ -CREATE PROCEDURE dbo.MergeSearchParams @SearchParams dbo.SearchParamList READONLY, @ReindexId bigint = -1 --- @ReindexId = -1 old code, @ReindexId = 0 - new code but not reindex. @ReindexId > 0 - new code and reindex. +CREATE PROCEDURE dbo.MergeSearchParams @SearchParams dbo.SearchParamList READONLY, @ReindexId bigint +-- @ReindexId = 0 - new code but not reindex. @ReindexId > 0 - new code and reindex. AS set nocount on DECLARE @SP varchar(100) = object_name(@@procid) @@ -18,7 +18,7 @@ DECLARE @SP varchar(100) = object_name(@@procid) SET @Mode = @Mode +' L='+isnull(convert(varchar(23),@ExpectedLastUpdated,126),'NULL') BEGIN TRY - IF @ReindexId IS NULL RAISERROR('@ReindexId cannot be null', 18, 127) + IF @ReindexId IS NULL OR @ReindexId < 0 RAISERROR('@ReindexId cannot be null or negative', 18, 127) DECLARE @SearchParamsCopy dbo.SearchParamList INSERT INTO @SearchParamsCopy SELECT * FROM @SearchParams @@ -42,17 +42,14 @@ BEGIN TRY AND NOT EXISTS (SELECT 1 FROM dbo.JobQueue WHERE PartitionId = @ReindexId % 16 AND QueueType = 6 AND JobId = @ReindexId AND Status = 1) RAISERROR('Reindex job is not running', 18, 127) - IF @ReindexId < 0 -- this can be ivoked for new and old code. + IF @ReindexId = 0 BEGIN + -- Check if reindex job is running EXECUTE dbo.GetActiveJobs @QueueType = 6, @IsExistsCheck = 1, @GroupId = @ActiveJobId OUT SET @msg = 'Reindex job is in progress. Job Id='+convert(varchar,@ActiveJobId) IF @ActiveJobId IS NOT NULL THROW 50002, @msg, 1 - END - -- Check for concurrency conflicts using LastUpdated - -- Ignore any checks for reindex, as it owns statuses when it runs. - IF @ReindexId = 0 -- max(LastUpdated) logic - BEGIN + -- Check for concurrency conflicts using LastUpdated SET @MaxLastUpdated = (SELECT max(LastUpdated) FROM dbo.SearchParam) IF @MaxLastUpdated <> @ExpectedLastUpdated BEGIN @@ -61,22 +58,6 @@ BEGIN TRY END END - -- Remove this old logic when code starts using max last updated - IF @ReindexId = -1 - BEGIN - -- Only the top 60 are included in the message to avoid hitting the 8000 character limit, but all conflicts will cause the transaction to roll back - SELECT @msg = string_agg(S.Uri, ', ') - FROM ( - SELECT TOP 60 S.Uri - FROM @SearchParams I JOIN dbo.SearchParam S ON S.Uri = I.Uri - WHERE I.LastUpdated != S.LastUpdated) S - IF @msg IS NOT NULL - BEGIN - SET @msg = concat('Optimistic concurrency conflict detected for search parameters: ', @msg); - THROW 50001, @msg, 1 - END - END - MERGE INTO dbo.SearchParam S USING @SearchParams I ON I.Uri = S.Uri WHEN MATCHED THEN diff --git a/src/Microsoft.Health.Fhir.SqlServer/Microsoft.Health.Fhir.SqlServer.csproj b/src/Microsoft.Health.Fhir.SqlServer/Microsoft.Health.Fhir.SqlServer.csproj index cad0d46dbb..b4b929fc04 100644 --- a/src/Microsoft.Health.Fhir.SqlServer/Microsoft.Health.Fhir.SqlServer.csproj +++ b/src/Microsoft.Health.Fhir.SqlServer/Microsoft.Health.Fhir.SqlServer.csproj @@ -1,7 +1,7 @@  - 114 + 115 Features\Schema\Migrations\$(LatestSchemaVersion).sql LatestSchemaVersion-$(LatestSchemaVersion) diff --git a/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Persistence/SqlServerSearchParameterStatusDataStoreTests.cs b/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Persistence/SqlServerSearchParameterStatusDataStoreTests.cs index 06abfe341b..6f8f6884ae 100644 --- a/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Persistence/SqlServerSearchParameterStatusDataStoreTests.cs +++ b/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Persistence/SqlServerSearchParameterStatusDataStoreTests.cs @@ -15,8 +15,11 @@ using Microsoft.Health.Fhir.SqlServer.Features.Storage.Registry; using Microsoft.Health.Fhir.Tests.Common; using Microsoft.Health.Fhir.Tests.Common.FixtureParameters; +using Microsoft.Health.JobManagement; using Microsoft.Health.Test.Utilities; +using Newtonsoft.Json; using Xunit; +using JobConflictException = Microsoft.Health.Fhir.Core.Features.Operations.JobConflictException; namespace Microsoft.Health.Fhir.Tests.Integration.Persistence { @@ -599,32 +602,32 @@ public async Task GivenActiveReindexJob_WhenUpsertingSearchParameterStatus_ThenJ LastUpdated = _fixture.SearchParameterOperations.SearchParamLastUpdated, }; - // Start a reindex job + // Start a reindex job using real queue client var reindexJobRecord = new Core.Features.Operations.Reindex.Models.ReindexJobRecord(new List()); - var reindexJob = await _fixture.OperationDataStore.CreateReindexJobAsync(reindexJobRecord, CancellationToken.None); + var definition = JsonConvert.SerializeObject(reindexJobRecord); + var jobInfos = await _fixture.QueueClient.EnqueueAsync((byte)QueueType.Reindex, [definition], null, false, CancellationToken.None); + var jobId = jobInfos[0].Id; + var jobInfo = await _fixture.QueueClient.DequeueAsync((byte)QueueType.Reindex, "test", 0, CancellationToken.None, jobId); try { // Act & Assert - Upserting without reindexId should throw JobConflictException var exception = await Assert.ThrowsAsync(async () => { - await _fixture.SearchParameterStatusDataStore.UpsertStatuses([status], CancellationToken.None, reindexId: null); + await _fixture.SearchParameterStatusDataStore.UpsertStatuses([status], CancellationToken.None); }); Assert.Contains("reindex", exception.Message, StringComparison.OrdinalIgnoreCase); - Assert.Contains(reindexJob.JobRecord.Id, exception.Message); + Assert.Contains(jobId.ToString(), exception.Message); } finally { - // Cleanup - Cancel the reindex job - await _fixture.OperationDataStore.UpdateReindexJobAsync( - new Core.Features.Operations.Reindex.Models.ReindexJobRecord(new List()) - { - Id = reindexJob.JobRecord.Id, - Status = OperationStatus.Canceled, - }, - reindexJob.ETag, - CancellationToken.None); + // Cleanup + if (jobInfo != null) + { + jobInfo.Status = JobStatus.Cancelled; + await _fixture.QueueClient.CompleteJobAsync(jobInfo, false, CancellationToken.None); + } } } @@ -634,8 +637,6 @@ public async Task GivenActiveReindexJob_WhenUpsertingWithReindexId_ThenSucceeds( // This test validates that the reindex job itself can update search parameter statuses // by passing its job ID - await _fixture.SearchParameterOperations.GetAndApplySearchParameterUpdates(CancellationToken.None); - // Arrange - Create a test search parameter status var testUri = "http://hl7.org/fhir/SearchParameter/Test-ReindexBypass-" + Guid.NewGuid(); var status = new ResourceSearchParameterStatus @@ -646,14 +647,16 @@ public async Task GivenActiveReindexJob_WhenUpsertingWithReindexId_ThenSucceeds( LastUpdated = _fixture.SearchParameterOperations.SearchParamLastUpdated, }; - // Start a reindex job - var reindexJobRecord = new Core.Features.Operations.Reindex.Models.ReindexJobRecord(new List()); - var reindexJob = await _fixture.OperationDataStore.CreateReindexJobAsync(reindexJobRecord, CancellationToken.None); - var jobId = long.Parse(reindexJob.JobRecord.Id); + // Start a reindex job using real queue client + var definition = JsonConvert.SerializeObject(new Core.Features.Operations.Reindex.Models.ReindexJobRecord(new List())); + var jobInfos = await _fixture.QueueClient.EnqueueAsync((byte)QueueType.Reindex, [definition], null, false, CancellationToken.None); + var jobId = jobInfos[0].Id; + var jobInfo = await _fixture.QueueClient.DequeueAsync((byte)QueueType.Reindex, "test", 0, CancellationToken.None, jobId); + Assert.NotNull(jobInfo); try { - // Act - Upserting with reindexId should succeed (bypass the check) + // Act - Upserting with reindexId should succeed (bypass the concurrency check) await _fixture.SearchParameterStatusDataStore.UpsertStatuses([status], CancellationToken.None, reindexId: jobId); // Assert - Verify the status was created @@ -666,14 +669,11 @@ public async Task GivenActiveReindexJob_WhenUpsertingWithReindexId_ThenSucceeds( { // Cleanup await _testHelper.DeleteSearchParameterStatusAsync(testUri); - await _fixture.OperationDataStore.UpdateReindexJobAsync( - new Core.Features.Operations.Reindex.Models.ReindexJobRecord(new List()) - { - Id = reindexJob.JobRecord.Id, - Status = OperationStatus.Canceled, - }, - reindexJob.ETag, - CancellationToken.None); + if (jobInfo != null) + { + jobInfo.Status = JobStatus.Cancelled; + await _fixture.QueueClient.CompleteJobAsync(jobInfo, false, CancellationToken.None); + } } } @@ -683,24 +683,16 @@ public async Task GivenNoActiveReindexJob_WhenUpsertingSearchParameterStatus_The // This test validates that when no reindex is running, search parameter updates work normally // This ensures bulk delete operations can proceed when no reindex is active - await _fixture.SearchParameterOperations.GetAndApplySearchParameterUpdates(CancellationToken.None); - - // Arrange - Ensure no active reindex jobs - var (found, id) = await _fixture.OperationDataStore.CheckActiveReindexJobsAsync(CancellationToken.None); - if (found) - { - // Cancel any active reindex job first - var existingJob = await _fixture.OperationDataStore.GetReindexJobByIdAsync(id, CancellationToken.None); - await _fixture.OperationDataStore.UpdateReindexJobAsync( - new Core.Features.Operations.Reindex.Models.ReindexJobRecord(new List()) - { - Id = existingJob.JobRecord.Id, - Status = OperationStatus.Canceled, - }, - existingJob.ETag, - CancellationToken.None); + // Arrange - Clean up any active reindex jobs + var activeJobs = await _fixture.QueueClient.GetActiveJobsByQueueTypeAsync((byte)QueueType.Reindex, false, CancellationToken.None); + foreach (var activeJob in activeJobs) + { + activeJob.Status = JobStatus.Cancelled; + await _fixture.QueueClient.CompleteJobAsync(activeJob, false, CancellationToken.None); } + await _fixture.SearchParameterOperations.GetAndApplySearchParameterUpdates(CancellationToken.None); + var testUri = "http://hl7.org/fhir/SearchParameter/Test-NoReindex-" + Guid.NewGuid(); var status = new ResourceSearchParameterStatus { From 5c411f88432e6bbff02ffd29bcd9eee904386cb0 Mon Sep 17 00:00:00 2001 From: Sergey Galuzo Date: Sat, 27 Jun 2026 10:18:44 -0700 Subject: [PATCH 04/31] merge sp --- .../Features/Schema/Migrations/115.diff.sql | 13 ++++++------- .../Schema/Sql/Sprocs/MergeSearchParams.sql | 12 +++++------- 2 files changed, 11 insertions(+), 14 deletions(-) diff --git a/src/Microsoft.Health.Fhir.SqlServer/Features/Schema/Migrations/115.diff.sql b/src/Microsoft.Health.Fhir.SqlServer/Features/Schema/Migrations/115.diff.sql index c78844d08b..757f8a68b5 100644 --- a/src/Microsoft.Health.Fhir.SqlServer/Features/Schema/Migrations/115.diff.sql +++ b/src/Microsoft.Health.Fhir.SqlServer/Features/Schema/Migrations/115.diff.sql @@ -1,9 +1,9 @@ -ALTER PROCEDURE dbo.MergeSearchParams @SearchParams dbo.SearchParamList READONLY, @ReindexId bigint --- @ReindexId = 0 - new code but not reindex. @ReindexId > 0 - new code and reindex. +ALTER PROCEDURE dbo.MergeSearchParams @SearchParams dbo.SearchParamList READONLY, @ReindexId bigint = NULL +-- @ReindexId IS NULL - not reindex. @ReindexId IS NOT NULL - reindex. AS set nocount on DECLARE @SP varchar(100) = object_name(@@procid) - ,@Mode varchar(200) = 'Cnt='+convert(varchar,(SELECT count(*) FROM @SearchParams))+' R='+convert(varchar,@ReindexId) + ,@Mode varchar(200) = 'Cnt='+convert(varchar,(SELECT count(*) FROM @SearchParams))+' R='+isnull(convert(varchar,@ReindexId),'NULL') ,@st datetime = getUTCdate() ,@LastUpdated datetimeoffset(7) = convert(datetimeoffset(7), sysUTCdatetime()) ,@MaxLastUpdated datetimeoffset(7) @@ -18,8 +18,6 @@ DECLARE @SP varchar(100) = object_name(@@procid) SET @Mode = @Mode +' L='+isnull(convert(varchar(23),@ExpectedLastUpdated,126),'NULL') BEGIN TRY - IF @ReindexId IS NULL OR @ReindexId < 0 RAISERROR('@ReindexId cannot be null or negative', 18, 127) - DECLARE @SearchParamsCopy dbo.SearchParamList INSERT INTO @SearchParamsCopy SELECT * FROM @SearchParams WHILE EXISTS (SELECT * FROM @SearchParamsCopy) @@ -38,11 +36,11 @@ BEGIN TRY END -- check if job id is valid - IF @ReindexId > 0 + IF @ReindexId IS NOT NULL AND @ReindexId <> 0 -- TODO: remove AND @ReindexId <> 0 after deployment AND NOT EXISTS (SELECT 1 FROM dbo.JobQueue WHERE PartitionId = @ReindexId % 16 AND QueueType = 6 AND JobId = @ReindexId AND Status = 1) RAISERROR('Reindex job is not running', 18, 127) - IF @ReindexId = 0 + IF @ReindexId IS NULL OR @ReindexId = 0 -- TODO: remove OR @ReindexId = 0 after deployment BEGIN -- Check if reindex job is running EXECUTE dbo.GetActiveJobs @QueueType = 6, @IsExistsCheck = 1, @GroupId = @ActiveJobId OUT @@ -82,3 +80,4 @@ BEGIN CATCH THROW END CATCH GO + diff --git a/src/Microsoft.Health.Fhir.SqlServer/Features/Schema/Sql/Sprocs/MergeSearchParams.sql b/src/Microsoft.Health.Fhir.SqlServer/Features/Schema/Sql/Sprocs/MergeSearchParams.sql index 23468865ff..5ea956df29 100644 --- a/src/Microsoft.Health.Fhir.SqlServer/Features/Schema/Sql/Sprocs/MergeSearchParams.sql +++ b/src/Microsoft.Health.Fhir.SqlServer/Features/Schema/Sql/Sprocs/MergeSearchParams.sql @@ -1,9 +1,9 @@ -CREATE PROCEDURE dbo.MergeSearchParams @SearchParams dbo.SearchParamList READONLY, @ReindexId bigint --- @ReindexId = 0 - new code but not reindex. @ReindexId > 0 - new code and reindex. +CREATE PROCEDURE dbo.MergeSearchParams @SearchParams dbo.SearchParamList READONLY, @ReindexId bigint = NULL +-- @ReindexId IS NULL - not reindex. @ReindexId IS NOT NULL - reindex. AS set nocount on DECLARE @SP varchar(100) = object_name(@@procid) - ,@Mode varchar(200) = 'Cnt='+convert(varchar,(SELECT count(*) FROM @SearchParams))+' R='+convert(varchar,@ReindexId) + ,@Mode varchar(200) = 'Cnt='+convert(varchar,(SELECT count(*) FROM @SearchParams))+' R='+isnull(convert(varchar,@ReindexId),'NULL') ,@st datetime = getUTCdate() ,@LastUpdated datetimeoffset(7) = convert(datetimeoffset(7), sysUTCdatetime()) ,@MaxLastUpdated datetimeoffset(7) @@ -18,8 +18,6 @@ DECLARE @SP varchar(100) = object_name(@@procid) SET @Mode = @Mode +' L='+isnull(convert(varchar(23),@ExpectedLastUpdated,126),'NULL') BEGIN TRY - IF @ReindexId IS NULL OR @ReindexId < 0 RAISERROR('@ReindexId cannot be null or negative', 18, 127) - DECLARE @SearchParamsCopy dbo.SearchParamList INSERT INTO @SearchParamsCopy SELECT * FROM @SearchParams WHILE EXISTS (SELECT * FROM @SearchParamsCopy) @@ -38,11 +36,11 @@ BEGIN TRY END -- check if job id is valid - IF @ReindexId > 0 + IF @ReindexId IS NOT NULL AND @ReindexId <> 0 -- TODO: remove AND @ReindexId <> 0 after deployment AND NOT EXISTS (SELECT 1 FROM dbo.JobQueue WHERE PartitionId = @ReindexId % 16 AND QueueType = 6 AND JobId = @ReindexId AND Status = 1) RAISERROR('Reindex job is not running', 18, 127) - IF @ReindexId = 0 + IF @ReindexId IS NULL OR @ReindexId = 0 -- TODO: remove OR @ReindexId = 0 after deployment BEGIN -- Check if reindex job is running EXECUTE dbo.GetActiveJobs @QueueType = 6, @IsExistsCheck = 1, @GroupId = @ActiveJobId OUT From c9f0224613ef941ef575a53e763df6fbefbf7252 Mon Sep 17 00:00:00 2001 From: Sergey Galuzo Date: Sat, 27 Jun 2026 10:19:07 -0700 Subject: [PATCH 05/31] Operations store to use real queue client --- .../SqlServerFhirStorageTestsFixture.cs | 17 +++++++++------ ...rverSearchParameterStatusDataStoreTests.cs | 21 +++++++++---------- 2 files changed, 21 insertions(+), 17 deletions(-) diff --git a/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Persistence/SqlServerFhirStorageTestsFixture.cs b/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Persistence/SqlServerFhirStorageTestsFixture.cs index f6865c655d..beae5cf138 100644 --- a/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Persistence/SqlServerFhirStorageTestsFixture.cs +++ b/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Persistence/SqlServerFhirStorageTestsFixture.cs @@ -184,8 +184,12 @@ public async Task InitializeAsync() NullLogger.Instance); SqlServerFhirModel = sqlServerFhirModel; - // the test queue client may not be enough for these tests. will need to look back into this. - var queueClient = new TestQueueClient(); + // Create the real SqlQueueClient to use throughout the fixture + var sqlQueueClient = new SqlQueueClient(SchemaInformation, SqlRetryService, NullLogger.Instance); + _sqlQueueClient = sqlQueueClient; + + // TestQueueClient is still used by test helper for export job cleanup + var testQueueClient = new TestQueueClient(); // Add custom logic to set up the AzurePipelinesCredential if we are running in Azure Pipelines string federatedClientID = EnvironmentVariables.GetEnvironmentVariable(KnownEnvironmentVariableNames.AzureSubscriptionClientId); @@ -199,7 +203,7 @@ public async Task InitializeAsync() SqlAuthenticationProvider.SetProvider(SqlAuthenticationMethod.ActiveDirectoryWorkloadIdentity, new SqlAzurePipelinesWorkloadIdentityAuthenticationProvider(azurePipelinesCredential)); } - _testHelper = new SqlServerFhirStorageTestHelper(_initialConnectionString, MasterDatabaseName, sqlServerFhirModel, SqlConnectionBuilder, queueClient, SchemaInformation); + _testHelper = new SqlServerFhirStorageTestHelper(_initialConnectionString, MasterDatabaseName, sqlServerFhirModel, SqlConnectionBuilder, testQueueClient, SchemaInformation); await _testHelper.CreateAndInitializeDatabase(_databaseName, _maximumSupportedSchemaVersion, CancellationToken.None); var searchParameterToSearchValueTypeMap = new SearchParameterToSearchValueTypeMap(); @@ -247,11 +251,12 @@ public async Task InitializeAsync() importErrorSerializer, new SqlStoreClient(SqlRetryService, NullLogger.Instance, SchemaInformation)); - _fhirOperationDataStore = new SqlServerFhirOperationDataStore(SqlConnectionWrapperFactory, queueClient, NullLogger.Instance, NullLoggerFactory.Instance); - - var sqlQueueClient = new SqlQueueClient(SchemaInformation, SqlRetryService, NullLogger.Instance); + // Create both operation data stores using the real SqlQueueClient _sqlServerFhirOperationDataStore = new SqlServerFhirOperationDataStore(SqlConnectionWrapperFactory, sqlQueueClient, NullLogger.Instance, NullLoggerFactory.Instance); + // Use the real SqlQueueClient instead of TestQueueClient for IFhirOperationDataStore + _fhirOperationDataStore = _sqlServerFhirOperationDataStore; + _fhirRequestContextAccessor.RequestContext.CorrelationId.Returns(Guid.NewGuid().ToString()); _fhirRequestContextAccessor.RequestContext.RouteName.Returns("routeName"); diff --git a/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Persistence/SqlServerSearchParameterStatusDataStoreTests.cs b/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Persistence/SqlServerSearchParameterStatusDataStoreTests.cs index 6f8f6884ae..24b668afeb 100644 --- a/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Persistence/SqlServerSearchParameterStatusDataStoreTests.cs +++ b/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Persistence/SqlServerSearchParameterStatusDataStoreTests.cs @@ -602,12 +602,10 @@ public async Task GivenActiveReindexJob_WhenUpsertingSearchParameterStatus_ThenJ LastUpdated = _fixture.SearchParameterOperations.SearchParamLastUpdated, }; - // Start a reindex job using real queue client + // Create a reindex job using OperationDataStore var reindexJobRecord = new Core.Features.Operations.Reindex.Models.ReindexJobRecord(new List()); - var definition = JsonConvert.SerializeObject(reindexJobRecord); - var jobInfos = await _fixture.QueueClient.EnqueueAsync((byte)QueueType.Reindex, [definition], null, false, CancellationToken.None); - var jobId = jobInfos[0].Id; - var jobInfo = await _fixture.QueueClient.DequeueAsync((byte)QueueType.Reindex, "test", 0, CancellationToken.None, jobId); + var reindexJobWrapper = await _fixture.OperationDataStore.CreateReindexJobAsync(reindexJobRecord, CancellationToken.None); + var jobId = long.Parse(reindexJobWrapper.JobRecord.Id); try { @@ -622,7 +620,8 @@ public async Task GivenActiveReindexJob_WhenUpsertingSearchParameterStatus_ThenJ } finally { - // Cleanup + // Cleanup - Cancel the reindex job + var jobInfo = await _fixture.QueueClient.DequeueAsync((byte)QueueType.Reindex, "test-cleanup", 0, CancellationToken.None, jobId); if (jobInfo != null) { jobInfo.Status = JobStatus.Cancelled; @@ -644,13 +643,13 @@ public async Task GivenActiveReindexJob_WhenUpsertingWithReindexId_ThenSucceeds( Uri = new Uri(testUri), Status = SearchParameterStatus.Enabled, IsPartiallySupported = false, - LastUpdated = _fixture.SearchParameterOperations.SearchParamLastUpdated, + LastUpdated = DateTimeOffset.UtcNow, // no need to set value to SearchParamLastUpdated because reindex is excluded from concurrency checks }; - // Start a reindex job using real queue client - var definition = JsonConvert.SerializeObject(new Core.Features.Operations.Reindex.Models.ReindexJobRecord(new List())); - var jobInfos = await _fixture.QueueClient.EnqueueAsync((byte)QueueType.Reindex, [definition], null, false, CancellationToken.None); - var jobId = jobInfos[0].Id; + // Start a reindex job using OperationDataStore + var reindexJobRecord = new Core.Features.Operations.Reindex.Models.ReindexJobRecord(new List()); + var reindexJobWrapper = await _fixture.OperationDataStore.CreateReindexJobAsync(reindexJobRecord, CancellationToken.None); + var jobId = long.Parse(reindexJobWrapper.JobRecord.Id); var jobInfo = await _fixture.QueueClient.DequeueAsync((byte)QueueType.Reindex, "test", 0, CancellationToken.None, jobId); Assert.NotNull(jobInfo); From eea25bcf6514c083559bbf01d3555c5b497ef468 Mon Sep 17 00:00:00 2001 From: Sergey Galuzo Date: Sun, 28 Jun 2026 09:18:45 -0700 Subject: [PATCH 06/31] Separating true queue client and test one --- .../FhirOperationDataStoreExportTests.cs | 22 +++++-------------- .../FhirOperationDataStoreReindexTests.cs | 7 +----- .../CosmosDbFhirStorageTestsFixture.cs | 7 ++++++ .../Persistence/FhirStorageTestsFixture.cs | 13 +++++++++++ .../SqlServerFhirStorageTestsFixture.cs | 22 ++++++++++++++----- 5 files changed, 43 insertions(+), 28 deletions(-) diff --git a/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Features/Operations/FhirOperationDataStoreExportTests.cs b/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Features/Operations/FhirOperationDataStoreExportTests.cs index c881568706..a311bcabad 100644 --- a/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Features/Operations/FhirOperationDataStoreExportTests.cs +++ b/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Features/Operations/FhirOperationDataStoreExportTests.cs @@ -33,19 +33,21 @@ public class FhirOperationDataStoreExportTests : IClassFixture, IAsyncLifetime { private readonly IFhirOperationDataStore _operationDataStore; - private readonly IFhirStorageTestHelper _testHelper; public FhirOperationDataStoreReindexTests(FhirStorageTestsFixture fixture) { - _operationDataStore = fixture.OperationDataStore; - _testHelper = fixture.TestHelper; + _operationDataStore = fixture.TestSqlServerOperationDataStore ?? fixture.OperationDataStore; } public async Task InitializeAsync() { - await _testHelper.DeleteAllReindexJobRecordsAsync(); - await CancelActiveReindexJobIfExists(); - GetTestQueueClient().ClearJobs(); await AssertNoReindexJobsExist(); 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 537ee2b15d..353a65e476 100644 --- a/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Persistence/CosmosDbFhirStorageTestsFixture.cs +++ b/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Persistence/CosmosDbFhirStorageTestsFixture.cs @@ -78,6 +78,7 @@ public class CosmosDbFhirStorageTestsFixture : IServiceProvider, IAsyncLifetime private SearchParameterStatusManager _searchParameterStatusManager; private CosmosClient _cosmosClient; private CosmosQueueClient _queueClient; + private TestQueueClient _testQueueClient; private CosmosFhirOperationDataStore _cosmosFhirOperationDataStore; private ISearchIndexer _searchIndexer; @@ -335,6 +336,7 @@ public virtual async Task InitializeAsync() await _searchParameterDefinitionManager.EnsureInitializedAsync(CancellationToken.None); var queueClient = new TestQueueClient(); + _testQueueClient = queueClient; _fhirOperationDataStore = new CosmosFhirOperationDataStore( queueClient, documentClient, @@ -452,6 +454,11 @@ object IServiceProvider.GetService(Type serviceType) return _searchIndexer; } + if (serviceType == typeof(TestQueueClient)) + { + return _testQueueClient; + } + return null; } } diff --git a/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Persistence/FhirStorageTestsFixture.cs b/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Persistence/FhirStorageTestsFixture.cs index 1fa5379e11..585597ecfa 100644 --- a/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Persistence/FhirStorageTestsFixture.cs +++ b/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Persistence/FhirStorageTestsFixture.cs @@ -182,6 +182,19 @@ internal IFhirOperationDataStore SqlServerOperationDataStore } } + internal IFhirOperationDataStore TestSqlServerOperationDataStore + { + get + { + if (_fixture is SqlServerFhirStorageTestsFixture sqlFixture) + { + return sqlFixture.TestSqlServerOperationDataStore; + } + + return null; + } + } + public IQueueClient QueueClient => _fixture.GetRequiredService(); public IServiceProvider Service => _fixture; diff --git a/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Persistence/SqlServerFhirStorageTestsFixture.cs b/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Persistence/SqlServerFhirStorageTestsFixture.cs index beae5cf138..82c21d74ab 100644 --- a/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Persistence/SqlServerFhirStorageTestsFixture.cs +++ b/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Persistence/SqlServerFhirStorageTestsFixture.cs @@ -69,6 +69,7 @@ public class SqlServerFhirStorageTestsFixture : IServiceProvider, IAsyncLifetime private SqlServerFhirDataStore _fhirDataStore; private IFhirOperationDataStore _fhirOperationDataStore; private SqlServerFhirOperationDataStore _sqlServerFhirOperationDataStore; + private SqlServerFhirOperationDataStore _testSqlServerFhirOperationDataStore; private SqlServerFhirStorageTestHelper _testHelper; private SchemaUpgradeRunner _schemaUpgradeRunner; private FilebasedSearchParameterStatusDataStore _filebasedSearchParameterStatusDataStore; @@ -77,6 +78,7 @@ public class SqlServerFhirStorageTestsFixture : IServiceProvider, IAsyncLifetime private SupportedSearchParameterDefinitionManager _supportedSearchParameterDefinitionManager; private SearchParameterStatusManager _searchParameterStatusManager; private SqlQueueClient _sqlQueueClient; + private TestQueueClient _testQueueClient; private FhirSqlServerConfiguration _fhirSqlConfiguration; public SqlServerFhirStorageTestsFixture() @@ -110,6 +112,8 @@ internal SqlServerFhirStorageTestsFixture(int maximumSupportedSchemaVersion, str internal SqlServerFhirOperationDataStore SqlServerOperationDataStore => _sqlServerFhirOperationDataStore; + internal SqlServerFhirOperationDataStore TestSqlServerOperationDataStore => _testSqlServerFhirOperationDataStore; + internal SqlTransactionHandler SqlTransactionHandler { get; private set; } internal SqlConnectionWrapperFactory SqlConnectionWrapperFactory { get; private set; } @@ -188,8 +192,8 @@ public async Task InitializeAsync() var sqlQueueClient = new SqlQueueClient(SchemaInformation, SqlRetryService, NullLogger.Instance); _sqlQueueClient = sqlQueueClient; - // TestQueueClient is still used by test helper for export job cleanup - var testQueueClient = new TestQueueClient(); + // TestQueueClient is still used by export tests + _testQueueClient = new TestQueueClient(); // Add custom logic to set up the AzurePipelinesCredential if we are running in Azure Pipelines string federatedClientID = EnvironmentVariables.GetEnvironmentVariable(KnownEnvironmentVariableNames.AzureSubscriptionClientId); @@ -203,7 +207,7 @@ public async Task InitializeAsync() SqlAuthenticationProvider.SetProvider(SqlAuthenticationMethod.ActiveDirectoryWorkloadIdentity, new SqlAzurePipelinesWorkloadIdentityAuthenticationProvider(azurePipelinesCredential)); } - _testHelper = new SqlServerFhirStorageTestHelper(_initialConnectionString, MasterDatabaseName, sqlServerFhirModel, SqlConnectionBuilder, testQueueClient, SchemaInformation); + _testHelper = new SqlServerFhirStorageTestHelper(_initialConnectionString, MasterDatabaseName, sqlServerFhirModel, SqlConnectionBuilder, _testQueueClient, SchemaInformation); await _testHelper.CreateAndInitializeDatabase(_databaseName, _maximumSupportedSchemaVersion, CancellationToken.None); var searchParameterToSearchValueTypeMap = new SearchParameterToSearchValueTypeMap(); @@ -251,10 +255,13 @@ public async Task InitializeAsync() importErrorSerializer, new SqlStoreClient(SqlRetryService, NullLogger.Instance, SchemaInformation)); - // Create both operation data stores using the real SqlQueueClient + // Create operation data store with real SqlQueueClient for reindex tests _sqlServerFhirOperationDataStore = new SqlServerFhirOperationDataStore(SqlConnectionWrapperFactory, sqlQueueClient, NullLogger.Instance, NullLoggerFactory.Instance); - // Use the real SqlQueueClient instead of TestQueueClient for IFhirOperationDataStore + // Create operation data store with TestQueueClient for export tests + _testSqlServerFhirOperationDataStore = new SqlServerFhirOperationDataStore(SqlConnectionWrapperFactory, _testQueueClient, NullLogger.Instance, NullLoggerFactory.Instance); + + // Use the real SqlQueueClient for IFhirOperationDataStore _fhirOperationDataStore = _sqlServerFhirOperationDataStore; _fhirRequestContextAccessor.RequestContext.CorrelationId.Returns(Guid.NewGuid().ToString()); @@ -415,6 +422,11 @@ object IServiceProvider.GetService(Type serviceType) return _sqlQueueClient; } + if (serviceType == typeof(TestQueueClient)) + { + return _testQueueClient; + } + if (serviceType == typeof(TestSqlHashCalculator)) { return SqlQueryHashCalculator as TestSqlHashCalculator; From 51b723b58deaa119d88dd610739da2af4a430e09 Mon Sep 17 00:00:00 2001 From: Sergey Galuzo Date: Sun, 28 Jun 2026 09:38:39 -0700 Subject: [PATCH 07/31] Removed not used factory --- .../SearchParameterCacheRefreshBackgroundServiceTests.cs | 2 -- .../Search/Parameters/SearchParameterOperations.cs | 7 ------- .../SearchParameterDefinitionManagerTests.cs | 1 - .../Features/Operations/Reindex/ReindexJobTests.cs | 2 -- .../Persistence/FhirStorageTestsFixture.cs | 1 - 5 files changed, 13 deletions(-) diff --git a/src/Microsoft.Health.Fhir.Core.UnitTests/Features/Search/Registry/SearchParameterCacheRefreshBackgroundServiceTests.cs b/src/Microsoft.Health.Fhir.Core.UnitTests/Features/Search/Registry/SearchParameterCacheRefreshBackgroundServiceTests.cs index 2ab72b6823..e1e0a0a5f7 100644 --- a/src/Microsoft.Health.Fhir.Core.UnitTests/Features/Search/Registry/SearchParameterCacheRefreshBackgroundServiceTests.cs +++ b/src/Microsoft.Health.Fhir.Core.UnitTests/Features/Search/Registry/SearchParameterCacheRefreshBackgroundServiceTests.cs @@ -415,7 +415,6 @@ public async Task WhenBackgroundIsBlockedByAPI_BackgroundShouldSkipRefresh() Substitute.For(), Substitute.For>>(), Substitute.For>(), - Substitute.For(), Substitute.For>()); // Start a long-running API call that holds the semaphore @@ -465,7 +464,6 @@ public async Task WhenAPIIsBlockedByBackground_ApiShouldWait() Substitute.For(), Substitute.For>>(), Substitute.For>(), - Substitute.For(), Substitute.For>()); var mockLogger = Substitute.For>(); diff --git a/src/Microsoft.Health.Fhir.Core/Features/Search/Parameters/SearchParameterOperations.cs b/src/Microsoft.Health.Fhir.Core/Features/Search/Parameters/SearchParameterOperations.cs index 0c41a4b027..b0617a1d6b 100644 --- a/src/Microsoft.Health.Fhir.Core/Features/Search/Parameters/SearchParameterOperations.cs +++ b/src/Microsoft.Health.Fhir.Core/Features/Search/Parameters/SearchParameterOperations.cs @@ -6,7 +6,6 @@ using System; using System.Collections.Generic; using System.Linq; -using System.Text; using System.Threading; using System.Threading.Tasks; using EnsureThat; @@ -16,12 +15,10 @@ 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.Definition; using Microsoft.Health.Fhir.Core.Features.Definition.BundleWrappers; using Microsoft.Health.Fhir.Core.Features.Operations; using Microsoft.Health.Fhir.Core.Features.Persistence; -using Microsoft.Health.Fhir.Core.Features.Search; using Microsoft.Health.Fhir.Core.Features.Search.Registry; using Microsoft.Health.Fhir.Core.Models; using Newtonsoft.Json.Linq; @@ -37,7 +34,6 @@ public class SearchParameterOperations : ISearchParameterOperations, IDisposable private readonly IDataStoreSearchParameterValidator _dataStoreSearchParameterValidator; private readonly Func> _searchServiceFactory; private readonly IScopeProvider _fhirDataStoreFactory; - private readonly IResourceWrapperFactory _resourceWrapperFactory; private readonly ILogger _logger; private DateTimeOffset? _searchParamLastUpdated; private readonly SemaphoreSlim _refreshSemaphore; @@ -51,7 +47,6 @@ public SearchParameterOperations( IDataStoreSearchParameterValidator dataStoreSearchParameterValidator, Func> searchServiceFactory, IScopeProvider fhirDataStoreFactory, - IResourceWrapperFactory resourceWrapperFactory, ILogger logger) { EnsureArg.IsNotNull(searchParameterStatusManager, nameof(searchParameterStatusManager)); @@ -61,7 +56,6 @@ public SearchParameterOperations( EnsureArg.IsNotNull(dataStoreSearchParameterValidator, nameof(dataStoreSearchParameterValidator)); EnsureArg.IsNotNull(searchServiceFactory, nameof(searchServiceFactory)); EnsureArg.IsNotNull(fhirDataStoreFactory, nameof(fhirDataStoreFactory)); - EnsureArg.IsNotNull(resourceWrapperFactory, nameof(resourceWrapperFactory)); EnsureArg.IsNotNull(logger, nameof(logger)); _searchParameterStatusManager = searchParameterStatusManager; @@ -71,7 +65,6 @@ public SearchParameterOperations( _dataStoreSearchParameterValidator = dataStoreSearchParameterValidator; _searchServiceFactory = searchServiceFactory; _fhirDataStoreFactory = fhirDataStoreFactory; - _resourceWrapperFactory = resourceWrapperFactory; _logger = logger; _refreshSemaphore = new SemaphoreSlim(1, 1); } diff --git a/src/Microsoft.Health.Fhir.Shared.Core.UnitTests/Features/Search/SearchParameters/SearchParameterDefinitionManagerTests.cs b/src/Microsoft.Health.Fhir.Shared.Core.UnitTests/Features/Search/SearchParameters/SearchParameterDefinitionManagerTests.cs index 230b9ae3d0..f6469a79ca 100644 --- a/src/Microsoft.Health.Fhir.Shared.Core.UnitTests/Features/Search/SearchParameters/SearchParameterDefinitionManagerTests.cs +++ b/src/Microsoft.Health.Fhir.Shared.Core.UnitTests/Features/Search/SearchParameters/SearchParameterDefinitionManagerTests.cs @@ -159,7 +159,6 @@ public SearchParameterDefinitionManagerTests() searchParameterDataStoreValidator, () => searchService.CreateMockScope(), fhirDataStore.CreateMockScopeProvider(), - Substitute.For(), NullLogger.Instance); } diff --git a/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Features/Operations/Reindex/ReindexJobTests.cs b/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Features/Operations/Reindex/ReindexJobTests.cs index 94ac82fb59..f35e21e859 100644 --- a/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Features/Operations/Reindex/ReindexJobTests.cs +++ b/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Features/Operations/Reindex/ReindexJobTests.cs @@ -153,7 +153,6 @@ public async Task InitializeAsync() _dataStoreSearchParameterValidator, () => _searchService, _scopedDataStore.CreateMockScopeProviderFromScoped(), - _resourceWrapperFactory, NullLogger.Instance); // Start background service so it triggers GetAndApplySearchParameterUpdates which signals the TCS. @@ -1536,7 +1535,6 @@ private async Task InitializeSecondFHIRService() _dataStoreSearchParameterValidator, () => _searchService, _scopedDataStore.CreateMockScopeProviderFromScoped(), - _resourceWrapperFactory, NullLogger.Instance); } diff --git a/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Persistence/FhirStorageTestsFixture.cs b/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Persistence/FhirStorageTestsFixture.cs index 585597ecfa..1c53995b21 100644 --- a/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Persistence/FhirStorageTestsFixture.cs +++ b/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Persistence/FhirStorageTestsFixture.cs @@ -346,7 +346,6 @@ public async Task InitializeAsync() dataStoreSearchParameterValidator, () => SearchService.CreateMockScope(), DataStore.CreateMockScopeProvider(), - resourceWrapperFactory, NullLogger.Instance); var deleter = new DeletionService( From 26612608ce99422fd2b0795bd20567aec925a761 Mon Sep 17 00:00:00 2001 From: Sergey Galuzo Date: Sun, 28 Jun 2026 09:45:10 -0700 Subject: [PATCH 08/31] remove uused fields --- .../Features/Operations/Reindex/ReindexJobTests.cs | 3 --- 1 file changed, 3 deletions(-) diff --git a/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Features/Operations/Reindex/ReindexJobTests.cs b/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Features/Operations/Reindex/ReindexJobTests.cs index f35e21e859..eaf8ebbcc3 100644 --- a/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Features/Operations/Reindex/ReindexJobTests.cs +++ b/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Features/Operations/Reindex/ReindexJobTests.cs @@ -84,14 +84,12 @@ public class ReindexJobTests : IClassFixture, IAsyncLif private readonly ISearchIndexer _searchIndexer = Substitute.For(); private ISupportedSearchParameterDefinitionManager _supportedSearchParameterDefinitionManager; private SearchParameterStatusManager _searchParameterStatusManager; - private ISupportedSearchParameterDefinitionManager _supportedSearchParameterDefinitionManager2; private SearchParameterStatusManager _searchParameterStatusManager2; private readonly ISearchParameterSupportResolver _searchParameterSupportResolver = Substitute.For(); private readonly ITestOutputHelper _output; private IScoped _searchService; - private readonly RequestContextAccessor _contextAccessor = Substitute.For>(); private ISearchParameterOperations _searchParameterOperations = null; private ISearchParameterOperations _searchParameterOperations2 = null; private readonly IDataStoreSearchParameterValidator _dataStoreSearchParameterValidator = Substitute.For(); @@ -1522,7 +1520,6 @@ private async Task InitializeSecondFHIRService() _fixture.DataStore.CreateMockScopeProvider(), NullLogger.Instance); await _searchParameterDefinitionManager2.EnsureInitializedAsync(CancellationToken.None); - _supportedSearchParameterDefinitionManager2 = new SupportedSearchParameterDefinitionManager(_searchParameterDefinitionManager2); _searchParameterStatusManager2 = new SearchParameterStatusManager(_fixture.SearchParameterStatusDataStore, _searchParameterDefinitionManager2, _searchParameterSupportResolver, mediator, NullLogger.Instance); await _searchParameterStatusManager2.EnsureInitializedAsync(CancellationToken.None); From 81df9e1636a9feb44ec61112e6ba9d507c97d89e Mon Sep 17 00:00:00 2001 From: Sergey Galuzo Date: Sun, 28 Jun 2026 09:55:57 -0700 Subject: [PATCH 09/31] Remove not used inputs from CreateReindexRequestHandler --- .../Reindex/CreateReindexRequestHandler.cs | 15 +-------------- .../Operations/Reindex/ReindexJobTests.cs | 4 +--- 2 files changed, 2 insertions(+), 17 deletions(-) diff --git a/src/Microsoft.Health.Fhir.Core/Features/Operations/Reindex/CreateReindexRequestHandler.cs b/src/Microsoft.Health.Fhir.Core/Features/Operations/Reindex/CreateReindexRequestHandler.cs index 959b27291f..533eaed410 100644 --- a/src/Microsoft.Health.Fhir.Core/Features/Operations/Reindex/CreateReindexRequestHandler.cs +++ b/src/Microsoft.Health.Fhir.Core/Features/Operations/Reindex/CreateReindexRequestHandler.cs @@ -3,8 +3,6 @@ // Licensed under the MIT License (MIT). See LICENSE in the repo root for license information. // ------------------------------------------------------------------------------------------------- -using System.Collections.Generic; -using System.Linq; using System.Threading; using System.Threading.Tasks; using EnsureThat; @@ -12,10 +10,7 @@ using Microsoft.Extensions.Options; using Microsoft.Health.Core.Features.Security.Authorization; using Microsoft.Health.Fhir.Core.Configs; -using Microsoft.Health.Fhir.Core.Exceptions; -using Microsoft.Health.Fhir.Core.Features.Definition; using Microsoft.Health.Fhir.Core.Features.Operations.Reindex.Models; -using Microsoft.Health.Fhir.Core.Features.Search.Parameters; using Microsoft.Health.Fhir.Core.Features.Security; using Microsoft.Health.Fhir.Core.Features.Security.Authorization; using Microsoft.Health.Fhir.Core.Messages.Reindex; @@ -28,27 +23,19 @@ public class CreateReindexRequestHandler : IRequestHandler _authorizationService; private readonly ReindexJobConfiguration _reindexJobConfiguration; - private readonly ISearchParameterDefinitionManager _searchParameterDefinitionManager; - private readonly ISearchParameterOperations _searchParameterOperations; public CreateReindexRequestHandler( IFhirOperationDataStore fhirOperationDataStore, IAuthorizationService authorizationService, - IOptions reindexJobConfiguration, - ISearchParameterDefinitionManager searchParameterDefinitionManager, - ISearchParameterOperations searchParameterOperations) + IOptions reindexJobConfiguration) { EnsureArg.IsNotNull(fhirOperationDataStore, nameof(fhirOperationDataStore)); EnsureArg.IsNotNull(authorizationService, nameof(authorizationService)); EnsureArg.IsNotNull(reindexJobConfiguration, nameof(reindexJobConfiguration)); - EnsureArg.IsNotNull(searchParameterDefinitionManager, nameof(searchParameterDefinitionManager)); - EnsureArg.IsNotNull(searchParameterOperations, nameof(searchParameterOperations)); _fhirOperationDataStore = fhirOperationDataStore; _authorizationService = authorizationService; _reindexJobConfiguration = reindexJobConfiguration.Value; - _searchParameterDefinitionManager = searchParameterDefinitionManager; - _searchParameterOperations = searchParameterOperations; } public async Task Handle(CreateReindexRequest request, CancellationToken cancellationToken) diff --git a/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Features/Operations/Reindex/ReindexJobTests.cs b/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Features/Operations/Reindex/ReindexJobTests.cs index eaf8ebbcc3..3718fcc07a 100644 --- a/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Features/Operations/Reindex/ReindexJobTests.cs +++ b/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Features/Operations/Reindex/ReindexJobTests.cs @@ -162,9 +162,7 @@ public async Task InitializeAsync() _createReindexRequestHandler = new CreateReindexRequestHandler( _fhirOperationDataStore, DisabledFhirAuthorizationService.Instance, - _optionsReindexConfig, - _searchParameterDefinitionManager, - _searchParameterOperations); + _optionsReindexConfig); _reindexSingleResourceRequestHandler = new ReindexSingleResourceRequestHandler( DisabledFhirAuthorizationService.Instance, From 927d91acd0c528f46e377863662cda0b249c0d3b Mon Sep 17 00:00:00 2001 From: Sergey Galuzo Date: Sun, 28 Jun 2026 10:02:26 -0700 Subject: [PATCH 10/31] Remove not used from BulkDeleteController --- .../Controllers/BulkDeleteControllerTests.cs | 7 ------- .../Controllers/BulkDeleteController.cs | 6 ------ 2 files changed, 13 deletions(-) diff --git a/src/Microsoft.Health.Fhir.Shared.Api.UnitTests/Controllers/BulkDeleteControllerTests.cs b/src/Microsoft.Health.Fhir.Shared.Api.UnitTests/Controllers/BulkDeleteControllerTests.cs index 20eaa8652d..8a32362f53 100644 --- a/src/Microsoft.Health.Fhir.Shared.Api.UnitTests/Controllers/BulkDeleteControllerTests.cs +++ b/src/Microsoft.Health.Fhir.Shared.Api.UnitTests/Controllers/BulkDeleteControllerTests.cs @@ -79,15 +79,8 @@ public BulkDeleteControllerTests() Arg.Any()) .Returns(new Uri(OperationResultUrl)); - var searchService = Substitute.For(); - searchService.GetUsedResourceTypes(Arg.Any()) - .Returns(Task.FromResult>(new List { KnownResourceTypes.Patient })); - var scopedSearchService = Substitute.For>(); - scopedSearchService.Value.Returns(searchService); - _controller = new BulkDeleteController( _mediator, - () => scopedSearchService, urlResolver); _controller.ControllerContext = new ControllerContext( new ActionContext( diff --git a/src/Microsoft.Health.Fhir.Shared.Api/Controllers/BulkDeleteController.cs b/src/Microsoft.Health.Fhir.Shared.Api/Controllers/BulkDeleteController.cs index 81e556ef87..d31a47a01c 100644 --- a/src/Microsoft.Health.Fhir.Shared.Api/Controllers/BulkDeleteController.cs +++ b/src/Microsoft.Health.Fhir.Shared.Api/Controllers/BulkDeleteController.cs @@ -11,7 +11,6 @@ using MediatR; using Microsoft.AspNetCore.Mvc; using Microsoft.Health.Api.Features.Audit; -using Microsoft.Health.Extensions.DependencyInjection; using Microsoft.Health.Fhir.Api.Extensions; using Microsoft.Health.Fhir.Api.Features.ActionResults; using Microsoft.Health.Fhir.Api.Features.Filters; @@ -26,8 +25,6 @@ using Microsoft.Health.Fhir.Core.Features.Operations.BulkDelete; using Microsoft.Health.Fhir.Core.Features.Operations.BulkDelete.Messages; using Microsoft.Health.Fhir.Core.Features.Routing; -using Microsoft.Health.Fhir.Core.Features.Search; -using Microsoft.Health.Fhir.Core.Features.Search.Parameters; using Microsoft.Health.Fhir.Core.Messages.Delete; using Microsoft.Health.Fhir.Core.Models; using Microsoft.Health.Fhir.ValueSets; @@ -40,7 +37,6 @@ namespace Microsoft.Health.Fhir.Api.Controllers public class BulkDeleteController : Controller { private readonly IMediator _mediator; - private readonly Func> _searchService; private readonly IUrlResolver _urlResolver; private readonly HashSet _excludedParameters = new(new PropertyEqualityComparer(StringComparison.OrdinalIgnoreCase, s => s)) @@ -54,11 +50,9 @@ public class BulkDeleteController : Controller public BulkDeleteController( IMediator mediator, - Func> searchService, IUrlResolver urlResolver) { _mediator = EnsureArg.IsNotNull(mediator, nameof(mediator)); - _searchService = EnsureArg.IsNotNull(searchService, nameof(searchService)); _urlResolver = EnsureArg.IsNotNull(urlResolver, nameof(urlResolver)); } From 29d7e4fa9984461f6bc5a2164bb021686826b9e7 Mon Sep 17 00:00:00 2001 From: Sergey Galuzo Date: Sun, 28 Jun 2026 15:28:44 -0700 Subject: [PATCH 11/31] refactor to use common logic to create person search param --- .../Rest/Reindex/ReindexTests.cs | 114 +++++------------- 1 file changed, 27 insertions(+), 87 deletions(-) diff --git a/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/Reindex/ReindexTests.cs b/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/Reindex/ReindexTests.cs index f1df1cf5f7..1f747860ee 100644 --- a/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/Reindex/ReindexTests.cs +++ b/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/Reindex/ReindexTests.cs @@ -107,27 +107,9 @@ async Task CreatePersonSearchParamsAsync() { var bundle = new Bundle { Type = Bundle.BundleType.Batch, Entry = new List() }; -#if R5 - var resourceTypes = new List([Enum.Parse("Person")]); -#else - var resourceTypes = new List([Enum.Parse("Person")]); -#endif - foreach (var code in codes) { - var searchParam = new SearchParameter - { - Id = code, - Url = $"{urlPrefix}{code}", - Name = code, - Code = code, - Status = PublicationStatus.Active, - Type = SearchParamType.Token, - Expression = "Person.id", - Description = "any", - Base = resourceTypes, - }; - + var searchParam = CreateSearchParameterForPerson(code, $"{urlPrefix}{code}"); bundle.Entry.Add(new EntryComponent { Request = new RequestComponent { Method = Bundle.HTTPVerb.PUT, Url = $"SearchParameter/{code}" }, Resource = searchParam }); } @@ -214,24 +196,7 @@ public async Task GivenConcurrentSearchParamCreates_SomeShouldFail(bool isBundle async Task CreatePersonSearchParamAsync(string code, bool isBundle, bool isParal) { var bundle = new Bundle { Type = Bundle.BundleType.Batch, Entry = new List() }; -#if R5 - var resourceTypes = new List([Enum.Parse("Person")]); -#else - var resourceTypes = new List([Enum.Parse("Person")]); -#endif - - var searchParam = new SearchParameter - { - Id = code, - Url = $"{urlPrefix}{code}", - Name = code, - Code = code, - Status = PublicationStatus.Active, - Type = SearchParamType.Token, - Expression = "Person.id", - Description = "any", - Base = resourceTypes, - }; + var searchParam = CreateSearchParameterForPerson(code, $"{urlPrefix}{code}"); if (isBundle) { @@ -305,27 +270,9 @@ async Task CreatePersonSearchParamsAsync() { var bundle = new Bundle { Type = Bundle.BundleType.Batch, Entry = new List() }; -#if R5 - var resourceTypes = new List([Enum.Parse("Person")]); -#else - var resourceTypes = new List([Enum.Parse("Person")]); -#endif - foreach (var code in codes) { - var searchParam = new SearchParameter - { - Id = code, - Url = $"{urlPrefix}{code}", - Name = code, - Code = code, - Status = PublicationStatus.Active, - Type = SearchParamType.Token, - Expression = "Person.id", - Description = "any", - Base = resourceTypes, - }; - + var searchParam = CreateSearchParameterForPerson(code, $"{urlPrefix}{code}"); bundle.Entry.Add(new EntryComponent { Request = new RequestComponent { Method = Bundle.HTTPVerb.PUT, Url = $"SearchParameter/{code}" }, Resource = searchParam }); } @@ -360,19 +307,7 @@ public async Task GivenTwoSearchParamsWithCodeConflictOnDerived_ThenBadRequestIs var bundle = new Bundle { Type = Bundle.BundleType.Batch, Entry = [] }; var id = ids[0]; - var searchParam = new SearchParameter - { - Id = id, - Url = $"{urlPrefix}c-1", - Name = code, - Code = code, - Status = PublicationStatus.Active, - Type = SearchParamType.Token, - Expression = "Person.id", - Description = "any", - Base = personTypes, - }; - + var searchParam = CreateSearchParameterForPerson(code, $"{urlPrefix}c-1", id); bundle.Entry.Add(new EntryComponent { Request = new RequestComponent { Method = Bundle.HTTPVerb.PUT, Url = $"SearchParameter/{id}" }, Resource = searchParam }); id = ids[1]; @@ -605,28 +540,11 @@ public async Task GivenTwoPersonSearchParams_ThenBadRequestIsReturnedForConflict async Task CreatePersonSearchParamsAsync() { var bundle = new Bundle { Type = isBatch ? Bundle.BundleType.Batch : Bundle.BundleType.Transaction, Entry = [] }; -#if R5 - var resourceTypes = new List() { VersionIndependentResourceTypesAll.Person }; -#else - var resourceTypes = new List() { ResourceType.Person }; -#endif for (var i = 0; i < ids.Count; i++) { var code = codes[i]; var id = ids[i]; - var searchParam = new SearchParameter - { - Id = id, - Url = urls[i], - Name = code, - Code = code, - Status = PublicationStatus.Active, - Type = SearchParamType.Token, - Expression = "Person.id", - Description = "any", - Base = resourceTypes, - }; - + var searchParam = CreateSearchParameterForPerson(code, urls[i], id); bundle.Entry.Add(new EntryComponent { Request = new RequestComponent { Method = Bundle.HTTPVerb.PUT, Url = $"SearchParameter/{id}" }, Resource = searchParam }); } @@ -1288,6 +1206,28 @@ private async Task CreateCustomSearchParameterAsync(string code } } + private SearchParameter CreateSearchParameterForPerson(string code, string url, string id = null) + { +#if R5 + var resourceTypes = new List { VersionIndependentResourceTypesAll.Person }; +#else + var resourceTypes = new List { ResourceType.Person }; +#endif + + return new SearchParameter + { + Id = id ?? code, + Url = url, + Name = code, + Code = code, + Status = PublicationStatus.Active, + Type = SearchParamType.Token, + Expression = "Person.id", + Description = "any", + Base = resourceTypes, + }; + } + private async Task CreateSpecimenResourceAsync(string id, string name) { var specimen = new Specimen From 0c1e82908e95d228621004015eaaed6d5a763b11 Mon Sep 17 00:00:00 2001 From: Sergey Galuzo Date: Sun, 28 Jun 2026 17:48:05 -0700 Subject: [PATCH 12/31] Added conflict on create test --- .../Storage/SqlServerFhirDataStore.cs | 5 ++++ .../Rest/Reindex/ReindexTests.cs | 26 ++++++++++++++----- 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/src/Microsoft.Health.Fhir.SqlServer/Features/Storage/SqlServerFhirDataStore.cs b/src/Microsoft.Health.Fhir.SqlServer/Features/Storage/SqlServerFhirDataStore.cs index 04e4c46db3..33c05ff739 100644 --- a/src/Microsoft.Health.Fhir.SqlServer/Features/Storage/SqlServerFhirDataStore.cs +++ b/src/Microsoft.Health.Fhir.SqlServer/Features/Storage/SqlServerFhirDataStore.cs @@ -217,6 +217,11 @@ public async Task MergeAsync(IReadOnlyList CreatePersonSearchParamsAsync() foreach (var code in codes) { - var searchParam = CreateSearchParameterForPerson(code, $"{urlPrefix}{code}"); + var searchParam = CreatePersonSearchParam(code, $"{urlPrefix}{code}"); bundle.Entry.Add(new EntryComponent { Request = new RequestComponent { Method = Bundle.HTTPVerb.PUT, Url = $"SearchParameter/{code}" }, Resource = searchParam }); } @@ -196,7 +196,7 @@ public async Task GivenConcurrentSearchParamCreates_SomeShouldFail(bool isBundle async Task CreatePersonSearchParamAsync(string code, bool isBundle, bool isParal) { var bundle = new Bundle { Type = Bundle.BundleType.Batch, Entry = new List() }; - var searchParam = CreateSearchParameterForPerson(code, $"{urlPrefix}{code}"); + var searchParam = CreatePersonSearchParam(code, $"{urlPrefix}{code}"); if (isBundle) { @@ -272,7 +272,7 @@ async Task CreatePersonSearchParamsAsync() foreach (var code in codes) { - var searchParam = CreateSearchParameterForPerson(code, $"{urlPrefix}{code}"); + var searchParam = CreatePersonSearchParam(code, $"{urlPrefix}{code}"); bundle.Entry.Add(new EntryComponent { Request = new RequestComponent { Method = Bundle.HTTPVerb.PUT, Url = $"SearchParameter/{code}" }, Resource = searchParam }); } @@ -307,7 +307,7 @@ public async Task GivenTwoSearchParamsWithCodeConflictOnDerived_ThenBadRequestIs var bundle = new Bundle { Type = Bundle.BundleType.Batch, Entry = [] }; var id = ids[0]; - var searchParam = CreateSearchParameterForPerson(code, $"{urlPrefix}c-1", id); + var searchParam = CreatePersonSearchParam(code, $"{urlPrefix}c-1", id); bundle.Entry.Add(new EntryComponent { Request = new RequestComponent { Method = Bundle.HTTPVerb.PUT, Url = $"SearchParameter/{id}" }, Resource = searchParam }); id = ids[1]; @@ -544,7 +544,7 @@ async Task CreatePersonSearchParamsAsync() { var code = codes[i]; var id = ids[i]; - var searchParam = CreateSearchParameterForPerson(code, urls[i], id); + var searchParam = CreatePersonSearchParam(code, urls[i], id); bundle.Entry.Add(new EntryComponent { Request = new RequestComponent { Method = Bundle.HTTPVerb.PUT, Url = $"SearchParameter/{id}" }, Resource = searchParam }); } @@ -1116,6 +1116,20 @@ await VerifySearchParameterIsWorkingAsync( } } + [Fact] + public async Task GivenSearchParamCreate_WhenReindexIsRunning_ThenConflictIsReturned() + { + var reindex = await _fixture.TestFhirClient.PostReindexJobAsync(new Parameters { Parameter = [] }); + Assert.Equal(HttpStatusCode.Created, reindex.reponse.Response.StatusCode); + + var code = "conflict-test"; + var searchParam = CreatePersonSearchParam(code, $"http://e2e.org/{code}"); + var exception = await Assert.ThrowsAsync(async () => await _fixture.TestFhirClient.CreateAsync(searchParam)); + + Assert.Equal(HttpStatusCode.Conflict, exception.StatusCode); + Assert.Contains("reindex", exception.Message, StringComparison.OrdinalIgnoreCase); + } + // left as async to minimize changes private async Task CreatePersonResourceAsync(string id, string name) { @@ -1206,7 +1220,7 @@ private async Task CreateCustomSearchParameterAsync(string code } } - private SearchParameter CreateSearchParameterForPerson(string code, string url, string id = null) + private SearchParameter CreatePersonSearchParam(string code, string url, string id = null) { #if R5 var resourceTypes = new List { VersionIndependentResourceTypesAll.Person }; From 073aa219f41f5381e132c3cdf6a46dc95e9925a2 Mon Sep 17 00:00:00 2001 From: Sergey Galuzo Date: Sun, 28 Jun 2026 18:01:42 -0700 Subject: [PATCH 13/31] Added conflict on delete --- .../Rest/Reindex/ReindexTests.cs | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/Reindex/ReindexTests.cs b/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/Reindex/ReindexTests.cs index b85d496892..effab7e2ed 100644 --- a/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/Reindex/ReindexTests.cs +++ b/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/Reindex/ReindexTests.cs @@ -1130,6 +1130,33 @@ public async Task GivenSearchParamCreate_WhenReindexIsRunning_ThenConflictIsRetu Assert.Contains("reindex", exception.Message, StringComparison.OrdinalIgnoreCase); } + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task GivenSearchParamDelete_WhenReindexIsRunning_ThenConflictIsReturned(bool hardDelete) + { + var code = hardDelete ? "hard-delete-conflict-test" : "soft-delete-conflict-test"; + var searchParam = CreatePersonSearchParam(code, $"http://e2e.org/{code}"); + var created = await _fixture.TestFhirClient.CreateAsync(searchParam); + Assert.NotNull(created.Resource); + + var reindex = await _fixture.TestFhirClient.PostReindexJobAsync(new Parameters { Parameter = [] }); + Assert.Equal(HttpStatusCode.Created, reindex.reponse.Response.StatusCode); + + FhirClientException exception; + if (hardDelete) + { + exception = await Assert.ThrowsAsync(async () => await _fixture.TestFhirClient.HardDeleteAsync(created.Resource)); + } + else + { + exception = await Assert.ThrowsAsync(async () => await _fixture.TestFhirClient.DeleteAsync(created.Resource)); + } + + Assert.Equal(HttpStatusCode.Conflict, exception.StatusCode); + Assert.Contains("reindex", exception.Message, StringComparison.OrdinalIgnoreCase); + } + // left as async to minimize changes private async Task CreatePersonResourceAsync(string id, string name) { From 80a45e142598b0173a6b7db8d24fbb42f48aa5a3 Mon Sep 17 00:00:00 2001 From: Sergey Galuzo Date: Mon, 29 Jun 2026 13:32:10 -0700 Subject: [PATCH 14/31] fixing bulk delete test --- .../BulkDelete/BulkDeleteProcessingJob.cs | 12 ++++++- .../Handlers/GetBulkDeleteHandler.cs | 15 ++++++-- .../Resources/Delete/DeletionService.cs | 2 +- .../Rest/Reindex/ReindexTests.cs | 35 +++++++++++++++++++ 4 files changed, 60 insertions(+), 4 deletions(-) diff --git a/src/Microsoft.Health.Fhir.Core/Features/Operations/BulkDelete/BulkDeleteProcessingJob.cs b/src/Microsoft.Health.Fhir.Core/Features/Operations/BulkDelete/BulkDeleteProcessingJob.cs index 1e1b7dcd87..1a1d714130 100644 --- a/src/Microsoft.Health.Fhir.Core/Features/Operations/BulkDelete/BulkDeleteProcessingJob.cs +++ b/src/Microsoft.Health.Fhir.Core/Features/Operations/BulkDelete/BulkDeleteProcessingJob.cs @@ -98,7 +98,17 @@ public async Task ExecuteAsync(JobInfo jobInfo, CancellationToken cancel catch (IncompleteOperationException> ex) { resourcesDeleted = ex.PartialResults; - result.Issues.Add(ex.Message); + + if (ex.InnerException is AggregateException aggEx && aggEx.InnerExceptions.Any(ie => ie is JobConflictException)) + { + var conflictEx = aggEx.InnerExceptions.OfType().First(); + result.Issues.Add($"JobConflictException: {conflictEx.Message}"); + } + else + { + result.Issues.Add(ex.Message); + } + exception = ex; } diff --git a/src/Microsoft.Health.Fhir.Core/Features/Operations/BulkDelete/Handlers/GetBulkDeleteHandler.cs b/src/Microsoft.Health.Fhir.Core/Features/Operations/BulkDelete/Handlers/GetBulkDeleteHandler.cs index f9fd4f3a22..c601e3e5d5 100644 --- a/src/Microsoft.Health.Fhir.Core/Features/Operations/BulkDelete/Handlers/GetBulkDeleteHandler.cs +++ b/src/Microsoft.Health.Fhir.Core/Features/Operations/BulkDelete/Handlers/GetBulkDeleteHandler.cs @@ -84,7 +84,15 @@ public async Task Handle(GetBulkDeleteRequest request, Ca } else { - issues.Add(new OperationOutcomeIssue(OperationOutcomeConstants.IssueSeverity.Error, OperationOutcomeConstants.IssueType.Exception, detailsText: issue)); + if (issue.StartsWith("JobConflictException:", StringComparison.Ordinal)) + { + failureResultCode = HttpStatusCode.Conflict; + issues.Add(new OperationOutcomeIssue(OperationOutcomeConstants.IssueSeverity.Error, OperationOutcomeConstants.IssueType.Conflict, detailsText: issue.Substring("JobConflictException:".Length).TrimStart())); + } + else + { + issues.Add(new OperationOutcomeIssue(OperationOutcomeConstants.IssueSeverity.Error, OperationOutcomeConstants.IssueType.Exception, detailsText: issue)); + } } } } @@ -94,7 +102,10 @@ public async Task Handle(GetBulkDeleteRequest request, Ca } // Need a way to get a failure result code. Most likely will be 503, 400, or 403 - failureResultCode = HttpStatusCode.InternalServerError; + if (failureResultCode == HttpStatusCode.OK) + { + failureResultCode = HttpStatusCode.InternalServerError; + } } else if (job.Status == JobStatus.Cancelled) { diff --git a/src/Microsoft.Health.Fhir.Shared.Core/Features/Resources/Delete/DeletionService.cs b/src/Microsoft.Health.Fhir.Shared.Core/Features/Resources/Delete/DeletionService.cs index 0dc7cb7fe5..d253f9e263 100644 --- a/src/Microsoft.Health.Fhir.Shared.Core/Features/Resources/Delete/DeletionService.cs +++ b/src/Microsoft.Health.Fhir.Shared.Core/Features/Resources/Delete/DeletionService.cs @@ -364,7 +364,7 @@ private async Task> DeleteMultipleAsyncInternal(Condit } }); var aggregateException = new AggregateException(exceptions); - throw new IncompleteOperationException>(aggregateException, resourceTypesDeleted); + throw new IncompleteOperationException>(aggregateException, resourceTypesDeleted); } return resourceTypesDeleted; diff --git a/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/Reindex/ReindexTests.cs b/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/Reindex/ReindexTests.cs index effab7e2ed..1680c59a39 100644 --- a/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/Reindex/ReindexTests.cs +++ b/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/Reindex/ReindexTests.cs @@ -8,6 +8,7 @@ using System.Diagnostics; using System.Linq; using System.Net; +using System.Net.Http; using System.Reflection.Metadata; using System.Security.Cryptography; using System.Threading; @@ -1157,6 +1158,40 @@ public async Task GivenSearchParamDelete_WhenReindexIsRunning_ThenConflictIsRetu Assert.Contains("reindex", exception.Message, StringComparison.OrdinalIgnoreCase); } + [Fact] + public async Task GivenSearchParamBulkDelete_WhenReindexIsRunning_ThenJobFailsWithConflict() + { + var code = "bulk-delete-conflict-test"; + var searchParam = CreatePersonSearchParam(code, $"http://e2e.org/{code}"); + var created = await _fixture.TestFhirClient.CreateAsync(searchParam); + Assert.NotNull(created.Resource); + + var reindex = await _fixture.TestFhirClient.PostReindexJobAsync(new Parameters { Parameter = [] }); + Assert.Equal(HttpStatusCode.Created, reindex.reponse.Response.StatusCode); + + var bulkDeleteRequest = new HttpRequestMessage(HttpMethod.Delete, $"SearchParameter/$bulk-delete?url={Uri.EscapeDataString(searchParam.Url)}"); + bulkDeleteRequest.Headers.Add("Prefer", "respond-async"); + + var response = await _fixture.HttpClient.SendAsync(bulkDeleteRequest); + Assert.Equal(HttpStatusCode.Accepted, response.StatusCode); + + var location = response.Content.Headers.ContentLocation; + Assert.NotNull(location); + + var jobResult = await _fixture.TestFhirClient.WaitForBulkJobStatus("Bulk delete", location); + Assert.Equal(HttpStatusCode.Conflict, jobResult.Response.StatusCode); + + var issuesParam = jobResult.Resource.Parameter.FirstOrDefault(p => p.Name == "Issues"); + Assert.NotNull(issuesParam); + var issueResource = issuesParam.Resource as OperationOutcome; + Assert.NotNull(issueResource); + Assert.NotEmpty(issueResource.Issue); + + var conflictIssue = issueResource.Issue.FirstOrDefault(i => i.Code == OperationOutcome.IssueType.Conflict); + Assert.NotNull(conflictIssue); + Assert.Contains("reindex", conflictIssue.Details.Text, StringComparison.OrdinalIgnoreCase); + } + // left as async to minimize changes private async Task CreatePersonResourceAsync(string id, string name) { From 0dd731679eac6967b126dd750298e7a52f8dc995 Mon Sep 17 00:00:00 2001 From: Sergey Galuzo Date: Mon, 29 Jun 2026 14:05:16 -0700 Subject: [PATCH 15/31] idic --- .../Features/Resources/Delete/DeletionServiceTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Microsoft.Health.Fhir.Shared.Core.UnitTests/Features/Resources/Delete/DeletionServiceTests.cs b/src/Microsoft.Health.Fhir.Shared.Core.UnitTests/Features/Resources/Delete/DeletionServiceTests.cs index 2ddbe4eee5..c8f6979040 100644 --- a/src/Microsoft.Health.Fhir.Shared.Core.UnitTests/Features/Resources/Delete/DeletionServiceTests.cs +++ b/src/Microsoft.Health.Fhir.Shared.Core.UnitTests/Features/Resources/Delete/DeletionServiceTests.cs @@ -266,7 +266,7 @@ public async Task GivenSearchParameterDelete_WhenConcurrencyConflictExhaustsRetr .DeleteSearchParameterAsync(Arg.Any(), Arg.Any(), Arg.Any()) .Returns(_ => throw new BadRequestException(Core.Resources.SearchParameterConcurrencyConflict)); - var exception = await Assert.ThrowsAsync>>(async () => + var exception = await Assert.ThrowsAsync>>(async () => await _service.DeleteMultipleAsync(request, CancellationToken.None)); Assert.Contains(" Deletion.3", exception.InnerException.Message); From 5a375684acc1ffe62973aedc1944b26f12df8521 Mon Sep 17 00:00:00 2001 From: SergeyGaluzo <95932081+SergeyGaluzo@users.noreply.github.com> Date: Mon, 29 Jun 2026 14:19:25 -0700 Subject: [PATCH 16/31] Potential fix for pull request finding 'CodeQL / Missing Dispose call on local IDisposable' Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> --- .../Rest/Reindex/ReindexTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/Reindex/ReindexTests.cs b/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/Reindex/ReindexTests.cs index 1680c59a39..d25de949c4 100644 --- a/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/Reindex/ReindexTests.cs +++ b/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/Reindex/ReindexTests.cs @@ -1169,7 +1169,7 @@ public async Task GivenSearchParamBulkDelete_WhenReindexIsRunning_ThenJobFailsWi var reindex = await _fixture.TestFhirClient.PostReindexJobAsync(new Parameters { Parameter = [] }); Assert.Equal(HttpStatusCode.Created, reindex.reponse.Response.StatusCode); - var bulkDeleteRequest = new HttpRequestMessage(HttpMethod.Delete, $"SearchParameter/$bulk-delete?url={Uri.EscapeDataString(searchParam.Url)}"); + using var bulkDeleteRequest = new HttpRequestMessage(HttpMethod.Delete, $"SearchParameter/$bulk-delete?url={Uri.EscapeDataString(searchParam.Url)}"); bulkDeleteRequest.Headers.Add("Prefer", "respond-async"); var response = await _fixture.HttpClient.SendAsync(bulkDeleteRequest); From 0ff7acfe51b379d4536a527e50a776a2c18523e1 Mon Sep 17 00:00:00 2001 From: SergeyGaluzo <95932081+SergeyGaluzo@users.noreply.github.com> Date: Mon, 29 Jun 2026 14:19:50 -0700 Subject: [PATCH 17/31] Potential fix for pull request finding 'CodeQL / Dereferenced variable may be null' Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> --- .../Rest/Reindex/ReindexTests.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/Reindex/ReindexTests.cs b/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/Reindex/ReindexTests.cs index d25de949c4..84d0751b57 100644 --- a/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/Reindex/ReindexTests.cs +++ b/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/Reindex/ReindexTests.cs @@ -1183,8 +1183,7 @@ public async Task GivenSearchParamBulkDelete_WhenReindexIsRunning_ThenJobFailsWi var issuesParam = jobResult.Resource.Parameter.FirstOrDefault(p => p.Name == "Issues"); Assert.NotNull(issuesParam); - var issueResource = issuesParam.Resource as OperationOutcome; - Assert.NotNull(issueResource); + var issueResource = Assert.IsType(issuesParam.Resource); Assert.NotEmpty(issueResource.Issue); var conflictIssue = issueResource.Issue.FirstOrDefault(i => i.Code == OperationOutcome.IssueType.Conflict); From b0b1837c4153cb567c40ef982f6577f1ca48506d Mon Sep 17 00:00:00 2001 From: SergeyGaluzo <95932081+SergeyGaluzo@users.noreply.github.com> Date: Mon, 29 Jun 2026 14:20:06 -0700 Subject: [PATCH 18/31] Potential fix for pull request finding 'CodeQL / Missed ternary opportunity' Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> --- .../Rest/Reindex/ReindexTests.cs | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/Reindex/ReindexTests.cs b/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/Reindex/ReindexTests.cs index 84d0751b57..292feb6879 100644 --- a/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/Reindex/ReindexTests.cs +++ b/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/Reindex/ReindexTests.cs @@ -1145,14 +1145,9 @@ public async Task GivenSearchParamDelete_WhenReindexIsRunning_ThenConflictIsRetu Assert.Equal(HttpStatusCode.Created, reindex.reponse.Response.StatusCode); FhirClientException exception; - if (hardDelete) - { - exception = await Assert.ThrowsAsync(async () => await _fixture.TestFhirClient.HardDeleteAsync(created.Resource)); - } - else - { - exception = await Assert.ThrowsAsync(async () => await _fixture.TestFhirClient.DeleteAsync(created.Resource)); - } + exception = hardDelete + ? await Assert.ThrowsAsync(async () => await _fixture.TestFhirClient.HardDeleteAsync(created.Resource)) + : await Assert.ThrowsAsync(async () => await _fixture.TestFhirClient.DeleteAsync(created.Resource)); Assert.Equal(HttpStatusCode.Conflict, exception.StatusCode); Assert.Contains("reindex", exception.Message, StringComparison.OrdinalIgnoreCase); From f6e663663acf271cee970a17b6ea43924bd99441 Mon Sep 17 00:00:00 2001 From: Sergey Galuzo Date: Tue, 30 Jun 2026 14:42:47 -0700 Subject: [PATCH 19/31] 116 with new message --- .../Features/Schema/Migrations/{115.diff.sql => 116.diff.sql} | 3 +-- .../Features/Schema/SchemaVersion.cs | 1 + .../Features/Schema/SchemaVersionConstants.cs | 2 +- .../Features/Schema/Sql/Sprocs/MergeSearchParams.sql | 2 +- .../Microsoft.Health.Fhir.SqlServer.csproj | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) rename src/Microsoft.Health.Fhir.SqlServer/Features/Schema/Migrations/{115.diff.sql => 116.diff.sql} (94%) diff --git a/src/Microsoft.Health.Fhir.SqlServer/Features/Schema/Migrations/115.diff.sql b/src/Microsoft.Health.Fhir.SqlServer/Features/Schema/Migrations/116.diff.sql similarity index 94% rename from src/Microsoft.Health.Fhir.SqlServer/Features/Schema/Migrations/115.diff.sql rename to src/Microsoft.Health.Fhir.SqlServer/Features/Schema/Migrations/116.diff.sql index 757f8a68b5..f0b960260f 100644 --- a/src/Microsoft.Health.Fhir.SqlServer/Features/Schema/Migrations/115.diff.sql +++ b/src/Microsoft.Health.Fhir.SqlServer/Features/Schema/Migrations/116.diff.sql @@ -44,7 +44,7 @@ BEGIN TRY BEGIN -- Check if reindex job is running EXECUTE dbo.GetActiveJobs @QueueType = 6, @IsExistsCheck = 1, @GroupId = @ActiveJobId OUT - SET @msg = 'Reindex job is in progress. Job Id='+convert(varchar,@ActiveJobId) + SET @msg = 'Changes to search parameters are not allowed while a reindex job is ongoing. Wait for the reindex job with Id: '+convert(varchar,@ActiveJobId)+' to finish, or cancel it' IF @ActiveJobId IS NOT NULL THROW 50002, @msg, 1 -- Check for concurrency conflicts using LastUpdated @@ -80,4 +80,3 @@ BEGIN CATCH THROW END CATCH GO - diff --git a/src/Microsoft.Health.Fhir.SqlServer/Features/Schema/SchemaVersion.cs b/src/Microsoft.Health.Fhir.SqlServer/Features/Schema/SchemaVersion.cs index 7817e88223..340aebb356 100644 --- a/src/Microsoft.Health.Fhir.SqlServer/Features/Schema/SchemaVersion.cs +++ b/src/Microsoft.Health.Fhir.SqlServer/Features/Schema/SchemaVersion.cs @@ -125,5 +125,6 @@ public enum SchemaVersion V113 = 113, V114 = 114, V115 = 115, + V116 = 116, } } diff --git a/src/Microsoft.Health.Fhir.SqlServer/Features/Schema/SchemaVersionConstants.cs b/src/Microsoft.Health.Fhir.SqlServer/Features/Schema/SchemaVersionConstants.cs index b49a942b56..854d345f6b 100644 --- a/src/Microsoft.Health.Fhir.SqlServer/Features/Schema/SchemaVersionConstants.cs +++ b/src/Microsoft.Health.Fhir.SqlServer/Features/Schema/SchemaVersionConstants.cs @@ -8,7 +8,7 @@ namespace Microsoft.Health.Fhir.SqlServer.Features.Schema public static class SchemaVersionConstants { public const int Min = (int)SchemaVersion.V113; - public const int Max = (int)SchemaVersion.V115; + public const int Max = (int)SchemaVersion.V116; public const int MinForUpgrade = (int)SchemaVersion.V111; // this is used for upgrade tests only public const int SearchParameterStatusSchemaVersion = (int)SchemaVersion.V6; public const int SupportForReferencesWithMissingTypeVersion = (int)SchemaVersion.V7; diff --git a/src/Microsoft.Health.Fhir.SqlServer/Features/Schema/Sql/Sprocs/MergeSearchParams.sql b/src/Microsoft.Health.Fhir.SqlServer/Features/Schema/Sql/Sprocs/MergeSearchParams.sql index 5ea956df29..8b5a48d424 100644 --- a/src/Microsoft.Health.Fhir.SqlServer/Features/Schema/Sql/Sprocs/MergeSearchParams.sql +++ b/src/Microsoft.Health.Fhir.SqlServer/Features/Schema/Sql/Sprocs/MergeSearchParams.sql @@ -44,7 +44,7 @@ BEGIN TRY BEGIN -- Check if reindex job is running EXECUTE dbo.GetActiveJobs @QueueType = 6, @IsExistsCheck = 1, @GroupId = @ActiveJobId OUT - SET @msg = 'Reindex job is in progress. Job Id='+convert(varchar,@ActiveJobId) + SET @msg = 'Changes to search parameters are not allowed while a reindex job is ongoing. Wait for the reindex job with Id: '+convert(varchar,@ActiveJobId)+' to finish, or cancel it' IF @ActiveJobId IS NOT NULL THROW 50002, @msg, 1 -- Check for concurrency conflicts using LastUpdated diff --git a/src/Microsoft.Health.Fhir.SqlServer/Microsoft.Health.Fhir.SqlServer.csproj b/src/Microsoft.Health.Fhir.SqlServer/Microsoft.Health.Fhir.SqlServer.csproj index b4b929fc04..cfc71e739d 100644 --- a/src/Microsoft.Health.Fhir.SqlServer/Microsoft.Health.Fhir.SqlServer.csproj +++ b/src/Microsoft.Health.Fhir.SqlServer/Microsoft.Health.Fhir.SqlServer.csproj @@ -1,7 +1,7 @@  - 115 + 116 Features\Schema\Migrations\$(LatestSchemaVersion).sql LatestSchemaVersion-$(LatestSchemaVersion) From 80a7679933535d90b2b8cc19770a7d918a91cce6 Mon Sep 17 00:00:00 2001 From: Sergey Galuzo Date: Tue, 30 Jun 2026 15:38:12 -0700 Subject: [PATCH 20/31] message corrected --- .../Features/Schema/Migrations/116.diff.sql | 2 +- .../Features/Schema/Sql/Sprocs/MergeSearchParams.sql | 2 +- .../Storage/Registry/SqlServerSearchParameterStatusDataStore.cs | 2 +- .../Features/Storage/SqlServerFhirDataStore.cs | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Microsoft.Health.Fhir.SqlServer/Features/Schema/Migrations/116.diff.sql b/src/Microsoft.Health.Fhir.SqlServer/Features/Schema/Migrations/116.diff.sql index f0b960260f..97f29bcef2 100644 --- a/src/Microsoft.Health.Fhir.SqlServer/Features/Schema/Migrations/116.diff.sql +++ b/src/Microsoft.Health.Fhir.SqlServer/Features/Schema/Migrations/116.diff.sql @@ -44,7 +44,7 @@ BEGIN TRY BEGIN -- Check if reindex job is running EXECUTE dbo.GetActiveJobs @QueueType = 6, @IsExistsCheck = 1, @GroupId = @ActiveJobId OUT - SET @msg = 'Changes to search parameters are not allowed while a reindex job is ongoing. Wait for the reindex job with Id: '+convert(varchar,@ActiveJobId)+' to finish, or cancel it' + SET @msg = 'Changes to search parameters are not allowed while a reindex job is in progress. Wait for the reindex job with Id: '+convert(varchar,@ActiveJobId)+' to finish, or cancel it' IF @ActiveJobId IS NOT NULL THROW 50002, @msg, 1 -- Check for concurrency conflicts using LastUpdated diff --git a/src/Microsoft.Health.Fhir.SqlServer/Features/Schema/Sql/Sprocs/MergeSearchParams.sql b/src/Microsoft.Health.Fhir.SqlServer/Features/Schema/Sql/Sprocs/MergeSearchParams.sql index 8b5a48d424..8e5b208edf 100644 --- a/src/Microsoft.Health.Fhir.SqlServer/Features/Schema/Sql/Sprocs/MergeSearchParams.sql +++ b/src/Microsoft.Health.Fhir.SqlServer/Features/Schema/Sql/Sprocs/MergeSearchParams.sql @@ -44,7 +44,7 @@ BEGIN TRY BEGIN -- Check if reindex job is running EXECUTE dbo.GetActiveJobs @QueueType = 6, @IsExistsCheck = 1, @GroupId = @ActiveJobId OUT - SET @msg = 'Changes to search parameters are not allowed while a reindex job is ongoing. Wait for the reindex job with Id: '+convert(varchar,@ActiveJobId)+' to finish, or cancel it' + SET @msg = 'Changes to search parameters are not allowed while a reindex job is in progress. Wait for the reindex job with Id: '+convert(varchar,@ActiveJobId)+' to finish, or cancel it' IF @ActiveJobId IS NOT NULL THROW 50002, @msg, 1 -- Check for concurrency conflicts using LastUpdated diff --git a/src/Microsoft.Health.Fhir.SqlServer/Features/Storage/Registry/SqlServerSearchParameterStatusDataStore.cs b/src/Microsoft.Health.Fhir.SqlServer/Features/Storage/Registry/SqlServerSearchParameterStatusDataStore.cs index b9bfe40116..7a29f111fc 100644 --- a/src/Microsoft.Health.Fhir.SqlServer/Features/Storage/Registry/SqlServerSearchParameterStatusDataStore.cs +++ b/src/Microsoft.Health.Fhir.SqlServer/Features/Storage/Registry/SqlServerSearchParameterStatusDataStore.cs @@ -153,7 +153,7 @@ public async Task UpsertStatuses(IReadOnlyList st } catch (SqlException ex) when (ex.IsReindexJobConflict()) { - _logger.LogWarning(ex, $"Reindex job conflict occurred while calling dbo.MergeSearchParams. ReindexId={reindexId ?? 0}"); + _logger.LogWarning(ex, $"Error calling dbo.MergeSearchParams. {ex.Message}"); throw new JobConflictException(ex.Message); } } diff --git a/src/Microsoft.Health.Fhir.SqlServer/Features/Storage/SqlServerFhirDataStore.cs b/src/Microsoft.Health.Fhir.SqlServer/Features/Storage/SqlServerFhirDataStore.cs index 33c05ff739..83f551425b 100644 --- a/src/Microsoft.Health.Fhir.SqlServer/Features/Storage/SqlServerFhirDataStore.cs +++ b/src/Microsoft.Health.Fhir.SqlServer/Features/Storage/SqlServerFhirDataStore.cs @@ -219,7 +219,7 @@ public async Task MergeAsync(IReadOnlyList Date: Tue, 30 Jun 2026 16:47:29 -0700 Subject: [PATCH 21/31] removed not used code path --- .../SearchParameterStateUpdateHandler.cs | 25 +------------------ 1 file changed, 1 insertion(+), 24 deletions(-) diff --git a/src/Microsoft.Health.Fhir.Shared.Core/Features/Operations/SearchParameterState/SearchParameterStateUpdateHandler.cs b/src/Microsoft.Health.Fhir.Shared.Core/Features/Operations/SearchParameterState/SearchParameterStateUpdateHandler.cs index 3c99f1dbb6..6a76bd1af2 100644 --- a/src/Microsoft.Health.Fhir.Shared.Core/Features/Operations/SearchParameterState/SearchParameterStateUpdateHandler.cs +++ b/src/Microsoft.Health.Fhir.Shared.Core/Features/Operations/SearchParameterState/SearchParameterStateUpdateHandler.cs @@ -118,7 +118,7 @@ private Dictionary> ParseRequestForUpdate(Se return searchParametersToUpdate; } - private SearchParameterStateUpdateResponse CreateBundleResponse(Dictionary> searchParametersToUpdate, List invalidSearchParameters, bool isReindexRunning = false) + private SearchParameterStateUpdateResponse CreateBundleResponse(Dictionary> searchParametersToUpdate, List invalidSearchParameters) { // Create the bundle from the result. var bundle = new Bundle(); @@ -191,29 +191,6 @@ private SearchParameterStateUpdateResponse CreateBundleResponse(Dictionary - { - new OperationOutcome.IssueComponent - { - Severity = OperationOutcome.IssueSeverity.Error, - Code = OperationOutcome.IssueType.Conflict, - Details = new CodeableConcept - { - Text = Core.Resources.ReindexRunningException, - }, - }, - }, - }, - }); - } - bundle.Type = Bundle.BundleType.BatchResponse; bundle.Total = invalidSearchParameters?.Count + searchParametersToUpdate?.Count; bundle.Meta = new Meta From 17f42c4c7d5036de65cb3753b34a9d9652de41d0 Mon Sep 17 00:00:00 2001 From: Sergey Galuzo Date: Wed, 1 Jul 2026 11:53:52 -0700 Subject: [PATCH 22/31] Changes to handle case wheb resource is hard deleted before cleanup. --- .../Rest/BulkDeleteTests.cs | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/BulkDeleteTests.cs b/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/BulkDeleteTests.cs index e9dab0a672..a603916252 100644 --- a/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/BulkDeleteTests.cs +++ b/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/BulkDeleteTests.cs @@ -518,8 +518,18 @@ async Task CleanupAsync() { foreach (var resource in resources) { - var status = (await _fhirClient.HardDeleteAsync(resource, false)).StatusCode; - Assert.True(status == HttpStatusCode.NotFound || status == HttpStatusCode.NoContent, $"expected=({HttpStatusCode.NotFound},{HttpStatusCode.NoContent}) actual={status}"); + try + { + var readResponse = await _fhirClient.ReadAsync($"{ResourceType.SearchParameter}/{resource.Id}"); + if (readResponse.StatusCode == HttpStatusCode.OK) + { + var deleteStatus = (await _fhirClient.HardDeleteAsync(resource, false)).StatusCode; + Assert.True(deleteStatus == HttpStatusCode.NotFound || deleteStatus == HttpStatusCode.NoContent, $"expected=({HttpStatusCode.NotFound},{HttpStatusCode.NoContent}) actual={deleteStatus}"); + } + } + catch (FhirClientException ex) when (ex.StatusCode == HttpStatusCode.NotFound || ex.StatusCode == HttpStatusCode.Gone) + { + } } } From 2ef7ffb8a2cc6bc5b875d7b7eaf3593236092c5e Mon Sep 17 00:00:00 2001 From: Sergey Galuzo Date: Wed, 1 Jul 2026 13:09:12 -0700 Subject: [PATCH 23/31] Passing NULL by default --- .../Registry/SqlServerSearchParameterStatusDataStore.cs | 6 +++++- .../Features/Storage/SqlServerFhirDataStore.cs | 1 - 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/Microsoft.Health.Fhir.SqlServer/Features/Storage/Registry/SqlServerSearchParameterStatusDataStore.cs b/src/Microsoft.Health.Fhir.SqlServer/Features/Storage/Registry/SqlServerSearchParameterStatusDataStore.cs index 7a29f111fc..5f136eaeb6 100644 --- a/src/Microsoft.Health.Fhir.SqlServer/Features/Storage/Registry/SqlServerSearchParameterStatusDataStore.cs +++ b/src/Microsoft.Health.Fhir.SqlServer/Features/Storage/Registry/SqlServerSearchParameterStatusDataStore.cs @@ -139,7 +139,11 @@ public async Task UpsertStatuses(IReadOnlyList st using var cmd = new SqlCommand(); cmd.CommandType = CommandType.StoredProcedure; cmd.CommandText = "dbo.MergeSearchParams"; - cmd.Parameters.AddWithValue("@ReindexId", reindexId ?? 0); + if (reindexId.HasValue) + { + cmd.Parameters.AddWithValue("@ReindexId", reindexId.Value); + } + new SearchParamListTableValuedParameterDefinition("@SearchParams").AddParameter(cmd.Parameters, new SearchParamListRowGenerator().GenerateRows(statuses)); try diff --git a/src/Microsoft.Health.Fhir.SqlServer/Features/Storage/SqlServerFhirDataStore.cs b/src/Microsoft.Health.Fhir.SqlServer/Features/Storage/SqlServerFhirDataStore.cs index 83f551425b..18202bc4eb 100644 --- a/src/Microsoft.Health.Fhir.SqlServer/Features/Storage/SqlServerFhirDataStore.cs +++ b/src/Microsoft.Health.Fhir.SqlServer/Features/Storage/SqlServerFhirDataStore.cs @@ -823,7 +823,6 @@ internal async Task MergeResourcesWrapperAsync(long transactionId, bool singleTr { cmd.CommandText = "dbo.MergeResourcesAndSearchParams"; new SearchParamListTableValuedParameterDefinition("@SearchParams").AddParameter(cmd.Parameters, new SearchParamListRowGenerator().GenerateRows(pendingStatuses)); - cmd.Parameters.AddWithValue("@ReindexId", 0); } else { From 324dcdd971769a66ac654125a3ccfb0c4f58b4f1 Mon Sep 17 00:00:00 2001 From: Sergey Galuzo Date: Wed, 1 Jul 2026 15:02:05 -0700 Subject: [PATCH 24/31] Simplified bulk delete tests --- .../Rest/BulkDeleteTests.cs | 78 ++++--------------- 1 file changed, 16 insertions(+), 62 deletions(-) diff --git a/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/BulkDeleteTests.cs b/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/BulkDeleteTests.cs index a603916252..f1e51d78c7 100644 --- a/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/BulkDeleteTests.cs +++ b/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/BulkDeleteTests.cs @@ -480,8 +480,6 @@ public async Task GivenBulkDeleteRequestWithMultipleExcludedResourceTypes_WhenCo public async Task GivenBulkDeleteRequest_WhenSearchParametersDeleted_ThenSearchParameterStatusShouldBeUpdated(bool hardDelete) { const int searchParameterStatusTimeoutSeconds = 60; - TimeSpan bulkDeleteRetryTimeout = TimeSpan.FromMinutes(2); - TimeSpan bulkDeleteRetryDelay = TimeSpan.FromSeconds(5); CheckBulkDeleteEnabled(); @@ -489,30 +487,24 @@ public async Task GivenBulkDeleteRequest_WhenSearchParametersDeleted_ThenSearchP var bundle = (Bundle)TagResources((Bundle)Samples.GetJsonSample("SearchParameter-USCoreIG").ToPoco(), tag); var resources = bundle.Entry.Select(x => x.Resource).ToList(); - if (_fixture.DataStore == DataStore.CosmosDb) - { - var retryStopwatch = Stopwatch.StartNew(); - var attempt = 1; + await CleanupAsync(); - while (true) - { - try - { - await ExecuteAttemptAsync(); - return; - } - catch (Exception ex) when (ShouldRetryBulkDeleteAttempt(ex, retryStopwatch.Elapsed, bulkDeleteRetryTimeout)) - { - _output.WriteLine( - $"Bulk delete SearchParameter attempt {attempt} failed after {retryStopwatch.Elapsed.TotalSeconds:F0}s. " + - $"Retrying in {bulkDeleteRetryDelay.TotalSeconds:F0}s. Error: {ex.Message}"); - attempt++; - await Task.Delay(bulkDeleteRetryDelay); - } - } - } + await CreateAsync(); + + await EnsureCreateAsync(); + + await CheckSearchParameterStatusAsync(SearchParameterStatus.Supported, TimeSpan.FromSeconds(searchParameterStatusTimeoutSeconds)); + + var queryParams = new Dictionary { { KnownQueryParameterNames.BulkHardDelete, hardDelete ? "true" : "false" } }; + using var request = GenerateBulkDeleteRequest(tag, $"{ResourceType.SearchParameter}/$bulk-delete", queryParams); + using var response = await _httpClient.SendAsync(request); + Assert.Equal(HttpStatusCode.Accepted, response.StatusCode); + + await CheckBulkDeleteStatusAsync(response.Content.Headers.ContentLocation, bundle.Entry.Count); - await ExecuteAttemptAsync(); + await EnsureBulkDeleteAsync(); + + await CheckSearchParameterStatusAsync(SearchParameterStatus.PendingDelete, TimeSpan.FromSeconds(searchParameterStatusTimeoutSeconds)); async Task CleanupAsync() { @@ -551,35 +543,6 @@ async Task EnsureCreateAsync() } } - async Task ExecuteAttemptAsync() - { - try - { - await CleanupAsync(); - - await CreateAsync(); - - await EnsureCreateAsync(); - - await CheckSearchParameterStatusAsync(SearchParameterStatus.Supported, TimeSpan.FromSeconds(searchParameterStatusTimeoutSeconds)); - - var queryParams = new Dictionary { { KnownQueryParameterNames.BulkHardDelete, hardDelete ? "true" : "false" } }; - using var request = GenerateBulkDeleteRequest(tag, $"{ResourceType.SearchParameter}/$bulk-delete", queryParams); - using var response = await _httpClient.SendAsync(request); - Assert.Equal(HttpStatusCode.Accepted, response.StatusCode); - - await CheckBulkDeleteStatusAsync(response.Content.Headers.ContentLocation, bundle.Entry.Count); - - await EnsureBulkDeleteAsync(); - - await CheckSearchParameterStatusAsync(SearchParameterStatus.PendingDelete, TimeSpan.FromSeconds(searchParameterStatusTimeoutSeconds)); - } - finally - { - await CleanupAsync(); - } - } - async Task CheckBulkDeleteStatusAsync(Uri location, long expectedCount) { var result = (await _fhirClient.WaitForBulkJobStatus("Bulk Delete", location)).Resource; @@ -609,15 +572,6 @@ async Task CheckBulkDeleteStatusAsync(Uri location, long expectedCount) Assert.True(expectedCount == actualCount, $"expected={expectedCount} actual={actualCount}"); } - static bool ShouldRetryBulkDeleteAttempt(Exception exception, TimeSpan elapsed, TimeSpan timeout) - { - return elapsed < timeout && - (exception is XunitException || - exception is FhirClientException || - exception is HttpRequestException || - exception is OperationCanceledException); - } - async Task EnsureBulkDeleteAsync() { var response = await _fhirClient.SearchAsync(ResourceType.SearchParameter, $"_tag={tag}"); From 28ed3e50f47b45d2677b78a01d45e0672fc488e7 Mon Sep 17 00:00:00 2001 From: Sergey Galuzo Date: Wed, 1 Jul 2026 16:58:21 -0700 Subject: [PATCH 25/31] Save corrected error message --- src/Microsoft.Health.Fhir.Core/Resources.Designer.cs | 4 ++-- src/Microsoft.Health.Fhir.Core/Resources.resx | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Microsoft.Health.Fhir.Core/Resources.Designer.cs b/src/Microsoft.Health.Fhir.Core/Resources.Designer.cs index d8170bd831..fad9a8f430 100644 --- a/src/Microsoft.Health.Fhir.Core/Resources.Designer.cs +++ b/src/Microsoft.Health.Fhir.Core/Resources.Designer.cs @@ -19,7 +19,7 @@ namespace Microsoft.Health.Fhir.Core { // class via a tool like ResGen or Visual Studio. // To add or remove a member, edit your .ResX file then rerun ResGen // with the /str option, or rebuild your VS project. - [global::System.CodeDom.Compiler.GeneratedCodeAttribute("System.Resources.Tools.StronglyTypedResourceBuilder", "18.0.0.0")] + [global::System.CodeDom.Compiler.GeneratedCodeAttribute("System.Resources.Tools.StronglyTypedResourceBuilder", "17.0.0.0")] [global::System.Diagnostics.DebuggerNonUserCodeAttribute()] [global::System.Runtime.CompilerServices.CompilerGeneratedAttribute()] internal class Resources { @@ -160,7 +160,7 @@ internal static string ChainedResourceTypesNotAllowedDueToScope { } /// - /// Looks up a localized string similar to Changes to search parameters is not allowed while a reindex job is ongoing. Wait for the reindex job with Id: {0} to finish, or cancel it.. + /// Looks up a localized string similar to Changes to search parameters are not allowed while a reindex job is in progress. Wait for the reindex job with Id: {0} to finish, or cancel it.. /// internal static string ChangesToSearchParametersNotAllowedWhileReindexing { get { diff --git a/src/Microsoft.Health.Fhir.Core/Resources.resx b/src/Microsoft.Health.Fhir.Core/Resources.resx index 292a276b16..f36b370c3e 100644 --- a/src/Microsoft.Health.Fhir.Core/Resources.resx +++ b/src/Microsoft.Health.Fhir.Core/Resources.resx @@ -500,7 +500,7 @@ The search parameter with Uri '{0}' is defined by the FHIR specification and cannot be updated or deleted. Custom search parameters must use a different URL. - Changes to search parameters is not allowed while a reindex job is ongoing. Wait for the reindex job with Id: {0} to finish, or cancel it. + Changes to search parameters are not allowed while a reindex job is in progress. Wait for the reindex job with Id: {0} to finish, or cancel it. {0} Id of currently active Reindex job. From 107b26f489f2c8bf398fe03536521a2b46b948ac Mon Sep 17 00:00:00 2001 From: Sergey Galuzo Date: Wed, 1 Jul 2026 19:00:17 -0700 Subject: [PATCH 26/31] Attempt to fix cosmos and tests --- .../Features/Storage/CosmosFhirDataStore.cs | 40 ++++++++++- .../Rest/Reindex/ReindexTests.cs | 67 +++++++++++++++++++ 2 files changed, 104 insertions(+), 3 deletions(-) diff --git a/src/Microsoft.Health.Fhir.CosmosDb/Features/Storage/CosmosFhirDataStore.cs b/src/Microsoft.Health.Fhir.CosmosDb/Features/Storage/CosmosFhirDataStore.cs index 31b1ff5e4c..9363341b6b 100644 --- a/src/Microsoft.Health.Fhir.CosmosDb/Features/Storage/CosmosFhirDataStore.cs +++ b/src/Microsoft.Health.Fhir.CosmosDb/Features/Storage/CosmosFhirDataStore.cs @@ -271,6 +271,29 @@ public async Task UpsertAsync(ResourceWrapperOperation resource, { try { + // For SearchParameter deletes, skip resource write and only persist status (matching SQL behavior) + if (string.Equals(resource.Wrapper.ResourceTypeName, KnownResourceTypes.SearchParameter, StringComparison.Ordinal) && + resource.Wrapper.IsDeleted) + { + var pendingStatus = GetPendingSearchParameterStatus(); + if (pendingStatus?.Status == SearchParameterStatus.PendingDelete || + pendingStatus?.Status == SearchParameterStatus.PendingHardDelete) + { + // Read existing resource to return correct version for response + var existingResource = await GetAsync(new ResourceKey(resource.Wrapper.ResourceTypeName, resource.Wrapper.ResourceId), cancellationToken); + if (existingResource == null) + { + throw new ResourceNotFoundException(string.Format(Microsoft.Health.Fhir.Core.Resources.ResourceNotFoundById, resource.Wrapper.ResourceTypeName, resource.Wrapper.ResourceId)); + } + + // Persist only the status, skip resource write (deferred to reindex job) + await PersistPendingSearchParameterStatusUpdateAsync(cancellationToken); + + // Return existing resource version to match SQL behavior + return new UpsertOutcome(existingResource, SaveOutcomeType.Updated); + } + } + var upsertOutcome = await InternalUpsertAsync( resource.Wrapper, resource.WeakETag, @@ -502,22 +525,33 @@ await retryPolicy.ExecuteAsync( } } - private async Task PersistPendingSearchParameterStatusUpdateAsync(CancellationToken cancellationToken) + private ResourceSearchParameterStatus GetPendingSearchParameterStatus() { var context = _requestContextAccessor.RequestContext; if (context?.Properties == null) { - return; + return null; } if (!context.Properties.TryGetValue(SearchParameterRequestContextPropertyNames.PendingStatus, out var value) || value is not ResourceSearchParameterStatus status) + { + return null; + } + + return status; + } + + private async Task PersistPendingSearchParameterStatusUpdateAsync(CancellationToken cancellationToken) + { + var status = GetPendingSearchParameterStatus(); + if (status == null) { return; } await _searchParameterStatusDataStore.UpsertStatuses([status], cancellationToken); - context.Properties.Remove(SearchParameterRequestContextPropertyNames.PendingStatus); + _requestContextAccessor.RequestContext.Properties.Remove(SearchParameterRequestContextPropertyNames.PendingStatus); } public async Task GetAsync(ResourceKey key, CancellationToken cancellationToken) diff --git a/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/Reindex/ReindexTests.cs b/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/Reindex/ReindexTests.cs index 292feb6879..7cfccb7549 100644 --- a/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/Reindex/ReindexTests.cs +++ b/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/Reindex/ReindexTests.cs @@ -1186,6 +1186,73 @@ public async Task GivenSearchParamBulkDelete_WhenReindexIsRunning_ThenJobFailsWi Assert.Contains("reindex", conflictIssue.Details.Text, StringComparison.OrdinalIgnoreCase); } + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task GivenSearchParamDelete_ThenResourceIsNotDeleted_AndStatusIsPendingDelete_AndAfterReindex_ResourceAndStatusAreDeleted(bool hardDelete) + { + var code = hardDelete ? "hard-delete-reindex-test" : "soft-delete-reindex-test"; + var url = $"http://e2e.org/{code}"; + var searchParam = CreatePersonSearchParam(code, url); + + var created = await _fixture.TestFhirClient.CreateAsync(searchParam); + Assert.NotNull(created.Resource); + Assert.Equal(code, created.Resource.Id); + _output.WriteLine($"Created SearchParameter/{code}"); + + if (hardDelete) + { + await _fixture.TestFhirClient.HardDeleteAsync(created.Resource); + _output.WriteLine($"Hard deleted SearchParameter/{code}"); + } + else + { + await _fixture.TestFhirClient.DeleteAsync(created.Resource); + _output.WriteLine($"Soft deleted SearchParameter/{code}"); + } + + var resourceAfterDelete = await _fixture.TestFhirClient.ReadAsync($"SearchParameter/{code}"); + Assert.NotNull(resourceAfterDelete?.Resource); + _output.WriteLine($"Verified SearchParameter/{code} still exists after delete"); + + var statusResponse = await _fixture.TestFhirClient.SearchAsync($"SearchParameter/$status?url={Uri.EscapeDataString(url)}"); + Assert.NotNull(statusResponse?.Resource); + var statusBundle = statusResponse.Resource; + Assert.NotNull(statusBundle.Entry); + Assert.NotEmpty(statusBundle.Entry); + var statusParam = statusBundle.Entry[0].Resource as Parameters; + Assert.NotNull(statusParam); + var statusValue = statusParam.Parameter.FirstOrDefault(p => p.Name == "status")?.Value as Code; + Assert.NotNull(statusValue); + var expectedStatus = hardDelete ? "PendingHardDelete" : "PendingDelete"; + Assert.Equal(expectedStatus, statusValue.Value); + _output.WriteLine($"Verified SearchParameter status is {expectedStatus}"); + + var reindexJob = await _fixture.TestFhirClient.PostReindexJobAsync(new Parameters { Parameter = [] }); + Assert.Equal(HttpStatusCode.Created, reindexJob.reponse.Response.StatusCode); + _output.WriteLine($"Created reindex job: {reindexJob.uri}"); + var jobStatus = await WaitForJobCompletionAsync(reindexJob.uri, TimeSpan.FromSeconds(300)); + Assert.Equal(OperationStatus.Completed, jobStatus); + _output.WriteLine($"Reindex job completed"); + + var notFoundEx = await Assert.ThrowsAsync(async () => await _fixture.TestFhirClient.ReadAsync($"SearchParameter/{code}")); + Assert.Equal(HttpStatusCode.NotFound, notFoundEx.StatusCode); + _output.WriteLine($"Verified SearchParameter/{code} is now deleted (404)"); + + var finalStatusResponse = await _fixture.TestFhirClient.SearchAsync($"SearchParameter/$status?url={Uri.EscapeDataString(url)}"); + Assert.NotNull(finalStatusResponse?.Resource); + var finalStatusBundle = finalStatusResponse.Resource; + Assert.NotNull(finalStatusBundle.Entry); + Assert.NotEmpty(finalStatusBundle.Entry); + + var finalStatusParam = finalStatusBundle.Entry[0].Resource as Parameters; + Assert.NotNull(finalStatusParam); + var finalStatusValue = finalStatusParam.Parameter.FirstOrDefault(p => p.Name == "status")?.Value as Code; + Assert.NotNull(finalStatusValue); + Assert.Equal("Deleted", finalStatusValue.Value); + _output.WriteLine($"Verified final SearchParameter status is 'Deleted'"); + } + // left as async to minimize changes private async Task CreatePersonResourceAsync(string id, string name) { From ad1659d8ae9a3dba82ced95305cbf63ab5105c03 Mon Sep 17 00:00:00 2001 From: Sergey Galuzo Date: Thu, 2 Jul 2026 08:23:03 -0700 Subject: [PATCH 27/31] extra test for hard/soft delete --- .../Rest/Reindex/ReindexTests.cs | 49 +++---------------- 1 file changed, 8 insertions(+), 41 deletions(-) diff --git a/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/Reindex/ReindexTests.cs b/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/Reindex/ReindexTests.cs index 7cfccb7549..390f37f794 100644 --- a/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/Reindex/ReindexTests.cs +++ b/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/Reindex/ReindexTests.cs @@ -1189,45 +1189,24 @@ public async Task GivenSearchParamBulkDelete_WhenReindexIsRunning_ThenJobFailsWi [Theory] [InlineData(false)] [InlineData(true)] - public async Task GivenSearchParamDelete_ThenResourceIsNotDeleted_AndStatusIsPendingDelete_AndAfterReindex_ResourceAndStatusAreDeleted(bool hardDelete) + public async Task GivenSearchParamDelete_ThenResourceIdDeletedAfterReindex(bool hardDelete) { var code = hardDelete ? "hard-delete-reindex-test" : "soft-delete-reindex-test"; var url = $"http://e2e.org/{code}"; var searchParam = CreatePersonSearchParam(code, url); - var created = await _fixture.TestFhirClient.CreateAsync(searchParam); + var created = await _fixture.TestFhirClient.UpdateAsync(searchParam); Assert.NotNull(created.Resource); Assert.Equal(code, created.Resource.Id); _output.WriteLine($"Created SearchParameter/{code}"); - if (hardDelete) - { - await _fixture.TestFhirClient.HardDeleteAsync(created.Resource); - _output.WriteLine($"Hard deleted SearchParameter/{code}"); - } - else - { - await _fixture.TestFhirClient.DeleteAsync(created.Resource); - _output.WriteLine($"Soft deleted SearchParameter/{code}"); - } + await _fixture.TestFhirClient.DeleteAsync($"SearchParameter/{code}{(hardDelete ? "?hardDelete=true" : string.Empty)}"); + _output.WriteLine($"{(hardDelete ? "Hard" : "Soft")} deleted SearchParameter/{code}"); var resourceAfterDelete = await _fixture.TestFhirClient.ReadAsync($"SearchParameter/{code}"); Assert.NotNull(resourceAfterDelete?.Resource); _output.WriteLine($"Verified SearchParameter/{code} still exists after delete"); - var statusResponse = await _fixture.TestFhirClient.SearchAsync($"SearchParameter/$status?url={Uri.EscapeDataString(url)}"); - Assert.NotNull(statusResponse?.Resource); - var statusBundle = statusResponse.Resource; - Assert.NotNull(statusBundle.Entry); - Assert.NotEmpty(statusBundle.Entry); - var statusParam = statusBundle.Entry[0].Resource as Parameters; - Assert.NotNull(statusParam); - var statusValue = statusParam.Parameter.FirstOrDefault(p => p.Name == "status")?.Value as Code; - Assert.NotNull(statusValue); - var expectedStatus = hardDelete ? "PendingHardDelete" : "PendingDelete"; - Assert.Equal(expectedStatus, statusValue.Value); - _output.WriteLine($"Verified SearchParameter status is {expectedStatus}"); - var reindexJob = await _fixture.TestFhirClient.PostReindexJobAsync(new Parameters { Parameter = [] }); Assert.Equal(HttpStatusCode.Created, reindexJob.reponse.Response.StatusCode); _output.WriteLine($"Created reindex job: {reindexJob.uri}"); @@ -1235,22 +1214,10 @@ public async Task GivenSearchParamDelete_ThenResourceIsNotDeleted_AndStatusIsPen Assert.Equal(OperationStatus.Completed, jobStatus); _output.WriteLine($"Reindex job completed"); - var notFoundEx = await Assert.ThrowsAsync(async () => await _fixture.TestFhirClient.ReadAsync($"SearchParameter/{code}")); - Assert.Equal(HttpStatusCode.NotFound, notFoundEx.StatusCode); - _output.WriteLine($"Verified SearchParameter/{code} is now deleted (404)"); - - var finalStatusResponse = await _fixture.TestFhirClient.SearchAsync($"SearchParameter/$status?url={Uri.EscapeDataString(url)}"); - Assert.NotNull(finalStatusResponse?.Resource); - var finalStatusBundle = finalStatusResponse.Resource; - Assert.NotNull(finalStatusBundle.Entry); - Assert.NotEmpty(finalStatusBundle.Entry); - - var finalStatusParam = finalStatusBundle.Entry[0].Resource as Parameters; - Assert.NotNull(finalStatusParam); - var finalStatusValue = finalStatusParam.Parameter.FirstOrDefault(p => p.Name == "status")?.Value as Code; - Assert.NotNull(finalStatusValue); - Assert.Equal("Deleted", finalStatusValue.Value); - _output.WriteLine($"Verified final SearchParameter status is 'Deleted'"); + var deleteEx = await Assert.ThrowsAsync(async () => await _fixture.TestFhirClient.ReadAsync($"SearchParameter/{code}")); + var expectedStatusCode = hardDelete ? HttpStatusCode.NotFound : HttpStatusCode.Gone; + Assert.Equal(expectedStatusCode, deleteEx.StatusCode); + _output.WriteLine($"Verified SearchParameter/{code} returns {expectedStatusCode}"); } // left as async to minimize changes From fccfdf6516add0e6185b4d9423b5b3ed948b1d50 Mon Sep 17 00:00:00 2001 From: Sergey Galuzo Date: Thu, 2 Jul 2026 08:24:18 -0700 Subject: [PATCH 28/31] name --- .../Rest/Reindex/ReindexTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/Reindex/ReindexTests.cs b/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/Reindex/ReindexTests.cs index 390f37f794..05e33920a2 100644 --- a/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/Reindex/ReindexTests.cs +++ b/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/Reindex/ReindexTests.cs @@ -1189,7 +1189,7 @@ public async Task GivenSearchParamBulkDelete_WhenReindexIsRunning_ThenJobFailsWi [Theory] [InlineData(false)] [InlineData(true)] - public async Task GivenSearchParamDelete_ThenResourceIdDeletedAfterReindex(bool hardDelete) + public async Task GivenSearchParamDelete_ThenResourceDeletedAfterReindex(bool hardDelete) { var code = hardDelete ? "hard-delete-reindex-test" : "soft-delete-reindex-test"; var url = $"http://e2e.org/{code}"; From c8453b2685d7e66cc0c8106c4cbe2eb8247cf1a3 Mon Sep 17 00:00:00 2001 From: Sergey Galuzo Date: Sun, 5 Jul 2026 14:46:36 -0700 Subject: [PATCH 29/31] Removed redundant test --- .../Rest/BulkDeleteTests.cs | 166 ------------------ 1 file changed, 166 deletions(-) diff --git a/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/BulkDeleteTests.cs b/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/BulkDeleteTests.cs index 88db981b3f..00acbfd99c 100644 --- a/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/BulkDeleteTests.cs +++ b/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/BulkDeleteTests.cs @@ -479,172 +479,6 @@ public async Task GivenBulkDeleteRequestWithMultipleExcludedResourceTypes_WhenCo Assert.Single(locationResults.Resource.Entry); } - // Before SP cache update fixes: Skip = "The test adds and deletes custom SPs causing the SP cache going out of sync with the store making the test flaky. Disable it for now until the issue of the SP cache out of sync is resolved. - [Theory] - [Trait(Traits.Category, Categories.IndexAndReindex)] - [InlineData(true)] - [InlineData(false)] - public async Task GivenBulkDeleteRequest_WhenSearchParametersDeleted_ThenSearchParameterStatusShouldBeUpdated(bool hardDelete) - { - const int searchParameterStatusTimeoutSeconds = 60; - - CheckBulkDeleteEnabled(); - - var tag = Guid.NewGuid().ToString(); - var bundle = (Bundle)TagResources((Bundle)Samples.GetJsonSample("SearchParameter-USCoreIG").ToPoco(), tag); - var resources = bundle.Entry.Select(x => x.Resource).ToList(); - - await CleanupAsync(); - - await CreateAsync(); - - await EnsureCreateAsync(); - - await CheckSearchParameterStatusAsync(SearchParameterStatus.Supported, TimeSpan.FromSeconds(searchParameterStatusTimeoutSeconds)); - - var queryParams = new Dictionary { { KnownQueryParameterNames.BulkHardDelete, hardDelete ? "true" : "false" } }; - using var request = GenerateBulkDeleteRequest(tag, $"{ResourceType.SearchParameter}/$bulk-delete", queryParams); - using var response = await _httpClient.SendAsync(request); - Assert.Equal(HttpStatusCode.Accepted, response.StatusCode); - - await CheckBulkDeleteStatusAsync(response.Content.Headers.ContentLocation, bundle.Entry.Count); - - await EnsureBulkDeleteAsync(); - - var expectedStatus = hardDelete ? SearchParameterStatus.PendingHardDelete : SearchParameterStatus.PendingDelete; - await CheckSearchParameterStatusAsync(expectedStatus, TimeSpan.FromSeconds(searchParameterStatusTimeoutSeconds)); - - async Task CleanupAsync() - { - foreach (var resource in resources) - { - var status = (await _fhirClient.HardDeleteAsync(resource, false)).StatusCode; - Assert.True(status == HttpStatusCode.NotFound || status == HttpStatusCode.NoContent, $"expected=({HttpStatusCode.NotFound},{HttpStatusCode.NoContent}) actual={status}"); - } - } - - async Task CreateAsync() - { - foreach (var resource in resources) - { - var status = (await _fhirClient.UpdateAsync(resource)).StatusCode; - Assert.True(status == HttpStatusCode.Created || status == HttpStatusCode.OK, $"expected=({HttpStatusCode.Created},{HttpStatusCode.OK}) actual={status}"); - } - } - - async Task EnsureCreateAsync() - { - foreach (var resource in resources) - { - var status = (await _fhirClient.ReadAsync($"{ResourceType.SearchParameter}/{resource.Id}")).StatusCode; - Assert.True(status == HttpStatusCode.OK, $"expected={HttpStatusCode.OK} actual={status}"); - } - } - - async Task CheckBulkDeleteStatusAsync(Uri location, long expectedCount) - { - var result = (await _fhirClient.WaitForBulkJobStatus("Bulk Delete", location)).Resource; - var actualCount = 0L; - var issuesChecked = 0; - foreach (var parameter in result.Parameter) - { - if (parameter.Name == "Issues") - { - issuesChecked++; - } - else if (parameter.Name == "ResourceDeletedCount") - { - foreach (var part in parameter.Part) - { - Assert.True(part.Name == KnownResourceTypes.SearchParameter, $"Unexpected type={part.Name}"); - actualCount = (long)((Integer64)part.Value).Value; - } - } - else - { - throw new Exception($"Unexpected parameter {parameter.Name}"); - } - } - - Assert.True(issuesChecked == 0, $"issues={issuesChecked}"); - Assert.True(expectedCount == actualCount, $"expected={expectedCount} actual={actualCount}"); - } - - async Task EnsureBulkDeleteAsync() - { - // SearchParameter resources should still exist after bulk delete - only status is updated - var response = await _fhirClient.SearchAsync(ResourceType.SearchParameter, $"_tag={tag}"); - Assert.Equal(HttpStatusCode.OK, response.StatusCode); - var count = response.Resource?.Entry.Count ?? 0; - Assert.True(count == resources.Count, $"Expected {resources.Count} search parameters to still exist (only status updated), but found {count}."); - } - - async Task CheckSearchParameterStatusAsync(SearchParameterStatus expectedStatus, TimeSpan timeout) - { - if (_fixture.DataStore == DataStore.CosmosDb) - { - return; - } - - var expectedStatusText = expectedStatus.ToString(); - var pollingInterval = TimeSpan.FromSeconds(2); - var stopwatch = Stopwatch.StartNew(); - string lastObservedState = "No status responses recorded."; - - while (stopwatch.Elapsed < timeout) - { - var statusesMatched = true; - var observedStates = new List(); - - foreach (var url in resources.Select(resource => ((SearchParameter)resource).Url)) - { - try - { - var response = await _fhirClient.ReadAsync($"{ResourceType.SearchParameter}/$status?url={url}"); - if (response.StatusCode != HttpStatusCode.OK) - { - statusesMatched = false; - observedStates.Add($"url={url} response={response.StatusCode}"); - continue; - } - - var part = response.Resource.Parameter - .FirstOrDefault(x => x.Part.Any(p => string.Equals(p.Name, "url", StringComparison.OrdinalIgnoreCase) && string.Equals(p.Value?.ToString(), url, StringComparison.OrdinalIgnoreCase))); - - var status = part?.Part - .Where(x => string.Equals(x.Name, "status", StringComparison.OrdinalIgnoreCase)) - .Select(x => x.Value?.ToString()) - .FirstOrDefault(); - - observedStates.Add($"url={url} actual={status ?? ""}"); - - if (part == null || !string.Equals(status, expectedStatusText, StringComparison.Ordinal)) - { - statusesMatched = false; - } - } - catch (Exception ex) - { - statusesMatched = false; - observedStates.Add($"url={url} error={ex.GetType().Name}: {ex.Message}"); - } - } - - lastObservedState = string.Join("; ", observedStates); - - if (statusesMatched) - { - return; - } - - await Task.Delay(pollingInterval); - } - - throw new XunitException( - $"Timed out after {timeout.TotalSeconds:F0}s waiting for SearchParameter status '{expectedStatusText}'. Last observed state: {lastObservedState}"); - } - } - [SkippableFact] #pragma warning disable CS1998 // Async method lacks 'await' operators and will run synchronously public async Task GivenABulkDeleteJob_WhenRemovingReferences_ThenReferencesAreRemoved() From eb92bf7965fb704e94b769e242507eb673f698c4 Mon Sep 17 00:00:00 2001 From: Sergey Galuzo Date: Sun, 5 Jul 2026 14:49:30 -0700 Subject: [PATCH 30/31] Moved bulk delete tests back --- .../Rest/BulkDeleteTests.cs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/BulkDeleteTests.cs b/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/BulkDeleteTests.cs index 00acbfd99c..9b93a74a0b 100644 --- a/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/BulkDeleteTests.cs +++ b/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/BulkDeleteTests.cs @@ -52,7 +52,6 @@ public BulkDeleteTests(HttpIntegrationTestFixture fixture, ITestOutputHelper out } [SkippableFact] - [Trait(Traits.Category, Categories.IndexAndReindex)] // temporarily moved till reindex conflict is moved to the storage layer public async Task GivenVariousResourcesOfDifferentTypes_WhenBulkDeleted_ThenAllAreDeleted() { CheckBulkDeleteEnabled(); @@ -83,7 +82,6 @@ public async Task GivenResourcesOfOneType_WhenBulkDeletedByType_ThenAllOfThatTyp } [SkippableFact] - [Trait(Traits.Category, Categories.IndexAndReindex)] // temporarily moved till reindex conflict is moved to the storage layer public async Task GivenBulkDeleteRequestWithInvalidSearchParameters_WhenRequested_ThenBadRequestIsReturned() { CheckBulkDeleteEnabled(); @@ -100,7 +98,6 @@ public async Task GivenBulkDeleteRequestWithInvalidSearchParameters_WhenRequeste } [SkippableFact] - [Trait(Traits.Category, Categories.IndexAndReindex)] // temporarily moved till reindex conflict is moved to the storage layer public async Task GivenSoftBulkDeleteRequest_WhenCompleted_ThenHistoricalRecordsExist() { CheckBulkDeleteEnabled(); @@ -128,7 +125,6 @@ public async Task GivenSoftBulkDeleteRequest_WhenCompleted_ThenHistoricalRecords [SkippableTheory] [InlineData(KnownQueryParameterNames.BulkHardDelete)] [InlineData(KnownQueryParameterNames.HardDelete)] - [Trait(Traits.Category, Categories.IndexAndReindex)] // temporarily moved till reindex conflict is moved to the storage layer public async Task GivenHardBulkDeleteRequest_WhenCompleted_ThenHistoricalRecordsDontExist(string hardDeleteKey) { CheckBulkDeleteEnabled(); @@ -158,7 +154,6 @@ public async Task GivenHardBulkDeleteRequest_WhenCompleted_ThenHistoricalRecords } [SkippableFact] - [Trait(Traits.Category, Categories.IndexAndReindex)] // temporarily moved till reindex conflict is moved to the storage layer public async Task GivenPurgeBulkDeleteRequest_WhenCompleted_ThenHistoricalRecordsDontExistAndCurrentRecordExists() { CheckBulkDeleteEnabled(); @@ -393,7 +388,6 @@ public async Task GivenBulkHardDeleteJobWithMoreThanOnePageOfIncludeResults_When } [SkippableFact] - [Trait(Traits.Category, Categories.IndexAndReindex)] // temporarily moved till reindex conflict is moved to the storage layer public async Task GivenBulkDeleteRequestWithMultipleExcludedResourceTypes_WhenCompleted_ThenExcludedResourcesAreNotDeleted() { CheckBulkDeleteEnabled(); From e7ad4a81450be50450689d3856007f269fe71b4e Mon Sep 17 00:00:00 2001 From: Sergey Galuzo Date: Sun, 5 Jul 2026 17:16:18 -0700 Subject: [PATCH 31/31] fix bulk delete logic error handling --- .../BulkDelete/BulkDeleteProcessingJob.cs | 30 +++++++++++++++---- .../Rest/Reindex/ReindexTests.cs | 16 ++++++---- 2 files changed, 35 insertions(+), 11 deletions(-) diff --git a/src/Microsoft.Health.Fhir.Core/Features/Operations/BulkDelete/BulkDeleteProcessingJob.cs b/src/Microsoft.Health.Fhir.Core/Features/Operations/BulkDelete/BulkDeleteProcessingJob.cs index 1a1d714130..a6c3eb8800 100644 --- a/src/Microsoft.Health.Fhir.Core/Features/Operations/BulkDelete/BulkDeleteProcessingJob.cs +++ b/src/Microsoft.Health.Fhir.Core/Features/Operations/BulkDelete/BulkDeleteProcessingJob.cs @@ -98,18 +98,36 @@ public async Task ExecuteAsync(JobInfo jobInfo, CancellationToken cancel catch (IncompleteOperationException> ex) { resourcesDeleted = ex.PartialResults; + bool conflictFound = false; - if (ex.InnerException is AggregateException aggEx && aggEx.InnerExceptions.Any(ie => ie is JobConflictException)) + if (ex.InnerException is AggregateException aggEx) { - var conflictEx = aggEx.InnerExceptions.OfType().First(); - result.Issues.Add($"JobConflictException: {conflictEx.Message}"); + foreach (var innerEx in aggEx.InnerExceptions) + { + if (innerEx is IncompleteOperationException> incompleteEx && incompleteEx.InnerException is JobConflictException conflictEx) + { + result.Issues.Add($"JobConflictException: {conflictEx.Message}"); + exception = conflictEx; + conflictFound = true; + } + else if (innerEx is JobConflictException directConflictEx) + { + result.Issues.Add($"JobConflictException: {directConflictEx.Message}"); + exception = directConflictEx; + conflictFound = true; + } + else + { + result.Issues.Add($"{innerEx.GetType().Name}: {innerEx.Message}"); + } + } } - else + + if (!conflictFound) { result.Issues.Add(ex.Message); + exception = ex; } - - exception = ex; } foreach (var (key, value) in resourcesDeleted) diff --git a/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/Reindex/ReindexTests.cs b/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/Reindex/ReindexTests.cs index 22efcd761e..724d4d3bb0 100644 --- a/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/Reindex/ReindexTests.cs +++ b/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/Reindex/ReindexTests.cs @@ -1261,7 +1261,13 @@ public async Task GivenSearchParamBulkDelete_ThenConflictWhenReindexAndSuccessOn using var bulkDeleteRequest = new HttpRequestMessage(HttpMethod.Delete, deleteUrl); bulkDeleteRequest.Headers.Add("Prefer", "respond-async"); var response = await _fixture.HttpClient.SendAsync(bulkDeleteRequest); - Assert.Equal(HttpStatusCode.Conflict, response.StatusCode); + Assert.Equal(HttpStatusCode.Accepted, response.StatusCode); + var location = response.Content.Headers.ContentLocation; + Assert.NotNull(location); + + // Wait for bulk delete job to complete - it should fail due to reindex conflict during execution + var jobResult = await _fixture.TestFhirClient.WaitForBulkJobStatus("Bulk delete", location); + Assert.Equal(HttpStatusCode.Conflict, jobResult.Response.StatusCode); var reindexStatus = await WaitForJobCompletionAsync(reindex.uri, TimeSpan.FromSeconds(300)); Assert.Equal(OperationStatus.Completed, reindexStatus); @@ -1274,11 +1280,11 @@ public async Task GivenSearchParamBulkDelete_ThenConflictWhenReindexAndSuccessOn bulkDeleteRequest2.Headers.Add("Prefer", "respond-async"); response = await _fixture.HttpClient.SendAsync(bulkDeleteRequest2); Assert.Equal(HttpStatusCode.Accepted, response.StatusCode); - var location = response.Content.Headers.ContentLocation; - Assert.NotNull(location); + var location2 = response.Content.Headers.ContentLocation; + Assert.NotNull(location2); - var jobResult = await _fixture.TestFhirClient.WaitForBulkJobStatus("Bulk delete", location); - Assert.Equal(HttpStatusCode.OK, jobResult.Response.StatusCode); + var jobResult2 = await _fixture.TestFhirClient.WaitForBulkJobStatus("Bulk delete", location2); + Assert.Equal(HttpStatusCode.OK, jobResult2.Response.StatusCode); // resource not touched resource = await _fixture.TestFhirClient.ReadAsync($"SearchParameter/{code}");