Remove pre-emptive constraint validation in import merge operations

[Copilot]

Per PR #5050 feedback: pre-emptive C# constraint validation adds compute overhead on every merge operation and duplicates database constraint logic. This change defers validation to SQL Server, catches constraint violations (error 547) only when they occur, and aborts import immediately on first error.

Changes

SqlServerFhirDataStore.cs

• Removed ValidateConstraintViolations method (~160 lines of C# validation logic)
• Added ExtractConstraintViolationDetails to parse SQL exception messages:
  • Extracts constraint type (CHECK/FOREIGN KEY/PRIMARY KEY) and name via regex
  • Maps error to resource indices in batch
• Updated ImportResourcesAsync exception handling to catch constraint violations and return formatted errors

SqlImporter.cs

• Added abort-on-first-error logic in ImportResourcesInBuffer
• Detects constraint violations by error message keywords, throws to halt processing

ImportProcessingJobTests.cs

• Added test case for constraint violation abort behavior

Example

Before:

// Validate all resources upfront
var constraintCheckResults = ValidateConstraintViolations(resources);
if (constraintCheckResults.Conflicts.Any()) {
    conflicts.AddRange(constraintCheckResults.Conflicts);
}
// Then merge validated resources
await Merge(constraintCheckResults.ValidResources, ...);

After:

try {
    // Merge directly, let SQL enforce constraints
    await Merge(resources, ...);
} catch (SqlException ex) when (ex.Number == 547) {
    // Extract details from SQL error only when constraint violation occurs
    return new List<string> { ExtractConstraintViolationDetails(ex, resources) };
}

Impact

• Normal case: Eliminates ~160 lines of validation per merge operation
• Error case: Import aborts immediately, no further processing
• Net: -150 LOC, zero duplication between C# and SQL constraint logic

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

• microsofthealthoss.pkgs.visualstudio.com
  • Triggering command: dotnet build ./src/Microsoft.Health.Fhir.SqlServer/Microsoft.Health.Fhir.SqlServer.csproj -c Release (dns block)
  • Triggering command: /opt/hostedtoolcache/CodeQL/2.23.1/x64/codeql/csharp/tools/linux64/Semmle.Autobuild.CSharp (dns block)
  • Triggering command: /home/REDACTED/work/fhir-server/.codeql-scratch/dbs/csharp/working/.dotnet/dotnet restore --no-dependencies /home/REDACTED/work/fhir-server/fhir-server/Microsoft.Health.Fhir.sln --packages /home/REDACTED/work/fhir-server/.codeql-scratch/dbs/csharp/working/packages /p:DisableImplicitNuGetFallbackFolder=true --verbosity normal /p:TargetFrameworkRootPath=/home/REDACTED/work/fhir-server/.codeql-scratch/dbs/csharp/working/emptyFakeDotnetRoot /p:NetCoreTargetingPackRoot=/home/REDACTED/work/fhir-server/.codeql-scratch/dbs/csharp/working/emptyFakeDotnetRoot (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

• Configure Actions setup steps to set up my environment, which run before the firewall is enabled
• Add the appropriate URLs or hosts to the custom allowlist in this repository's Copilot coding agent settings (admins only)

Original prompt

a comment in the PR #5050 on this branch states: Adding checks before calling merge adds extra load on app compute for each normal merge call. I suggest running this check only if merge fails with constraint violation. This way there should be zero effect on performance for case when there are not constraint errors.

I do not like having any code duplicated (C# constraint code and the database). Can we get away with reporting some details of SQL error and attributing it to the resource lines in the input file for the batch we processed? This might not be that precise, but we will not have any code duplication.

We also should abort import on first constraint error and should not spend app compute on analyzing all the input resources.

Please add a test for this functionality.

---

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

[SergeyGaluzo]

There are a lot of style errors, I cannot build locally.

[SergeyGaluzo]

It is not clear why error message is attributed to the first resource in the batch.

[SergeyGaluzo]

It is not clear how invalid operation can translate to bad request

[SergeyGaluzo]

Maybe it is valid, but what is missing is E2E test.

[SergeyGaluzo]

please address my comments