Fix duplicate SearchParameter URL creation

[apurvabhaleMS]

Description

CreateOrUpdateSearchParameterBehavior did not prevent creating a new SearchParameter resource whose URL was already owned by a different active resource. This allowed duplicate URLs to accumulate across test runs, which caused SQL error 2627 (unique key constraint violation) when a DELETE bundle processed multiple resources sharing the same URL in parallel.

Tests added:

• Unit: SetAndClearPendingSearchParameterStatus_WhenCalledConcurrently_Only OneResourceReceivesStatus (SqlServerFhirDataStoreUnitTests) — deterministic, proves race with Parallel.For, fails before fix / passes after.
• E2E regression guard: GivenParallelTransactionBundleWithSearchParamAndPatients _WhenPosted_ShouldNotThrowUniqueKeyConstraint (ReindexTests) — probabilistic, runs in CI/EUAP.

Related issues

Addresses AB198804, AB187119

Testing

Describe how this change was tested.

FHIR Team Checklist

• Update the title of the PR to be succinct and less than 65 characters
• Add a milestone to the PR for the sprint that it is merged (i.e. add S47)
• Tag the PR with the type of update: Bug, Build, Dependencies, Enhancement, New-Feature or Documentation
• Tag the PR with Open source, Azure API for FHIR (CosmosDB or common code) or Azure Healthcare APIs (SQL or common code) to specify where this change is intended to be released.
• Tag the PR with Schema Version backward compatible or Schema Version backward incompatible or Schema Version unchanged if this adds or updates Sql script which is/is not backward compatible with the code.
• When changing or adding behavior, if your code modifies the system design or changes design assumptions, please create and include an ADR.
• CI is green before merge
• Review squash-merge requirements

Semver Change (docs)

Patch|Skip|Feature|Breaking (reason)

[Codecov Comments Bot (codecov-commenter)]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 77.15%. Comparing base (ccd2246) to head (1690372).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5710      +/-   ##
==========================================
- Coverage   77.74%   77.15%   -0.60%     
==========================================
  Files        1007     1007              
  Lines       37173    37169       -4     
  Branches     5651     5657       +6     
==========================================
- Hits        28902    28677     -225     
- Misses       6896     7121     +225     
+ Partials     1375     1371       -4     

see 22 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[Copilot]

Pull request overview

This PR addresses a concurrency bug in the SQL Server data store where parallel bundle processing could race while claiming/clearing a pending SearchParameter status from the shared request-context Properties bag, leading to duplicate entries in the SQL TVP and unique constraint violations (SQL 2627) surfaced as HTTP 500s.

Changes:

• Make SetAndClearPendingSearchParameterStatus thread-safe by locking around the TryGetValue + Remove sequence.
• Add a SQL Server unit test intended to validate that only one concurrent caller receives the pending status.
• Add an E2E regression test covering a parallel transaction bundle containing a SearchParameter plus many Patients.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File	Description
src/Microsoft.Health.Fhir.SqlServer/Features/Storage/SqlServerFhirDataStore.cs	Locks request-context properties to atomically claim and clear the pending SearchParameter status under parallel bundle execution.
src/Microsoft.Health.Fhir.SqlServer.UnitTests/Features/Storage/SqlServerFhirDataStoreUnitTests.cs	Adds a concurrency-focused unit regression test for the pending status claim/clear behavior.
test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/Reindex/ReindexTests.cs	Adds an E2E regression guard using a parallel transaction bundle scenario to detect the historical SQL unique constraint failure mode.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

Suppressed comments (3)

src/Microsoft.Health.Fhir.Core/Features/Search/Parameters/CreateOrUpdateSearchParameterBehavior.cs:120

• The URL conflict check logic is duplicated in three places (create, upsert when URL changes, upsert when no previous version). Extracting this into a single private helper (e.g., ValidateUrlNotOwnedAsync(url, cancellationToken)) would reduce repetition and make future behavior changes (like excluding the current resource ID, if needed) safer and easier.

                        // Reject if an active resource already owns the new URL.
                        if (!string.IsNullOrWhiteSpace(newUrl))
                        {
                            var existingByNewUrl = await _searchParameterOperations.GetSearchParametersByUrlsAsync(new[] { newUrl }, cancellationToken);
                            if (existingByNewUrl.ContainsKey(newUrl))
                            {
                                throw new BadRequestException(string.Format(Core.Resources.SearchParameterDefinitionDuplicatedEntry, newUrl));
                            }
                        }

test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/Search/CustomSearchParamTests.cs:90

• This comment is misleading: CreateCustomSearchParameter sets Id explicitly (client-generated), so it’s not “auto-generated” here. Consider rewording to something like “Different ID” / “Different resource” to reflect what the test is actually doing.

                // Different auto-generated ID, same URL — must be rejected.
                SearchParameter sp2 = CreateCustomSearchParameter(repeatChar: 'a');

src/Microsoft.Health.Fhir.Shared.Core.UnitTests/Features/Search/SearchParameters/SearchParameterBehaviorTests.cs:74

• This returns the same mutable Dictionary instance for every call. Returning a fresh dictionary per invocation (via a lambda-based Returns) avoids accidental cross-test coupling if any test or code path ever mutates the returned dictionary.

            _searchParameterOperations.GetSearchParametersByUrlsAsync(Arg.Any<IReadOnlyCollection<string>>(), Arg.Any<CancellationToken>())
                .Returns(System.Threading.Tasks.Task.FromResult(new Dictionary<string, ITypedElement>()));