Improve Import Error Handling for Database Constraint Violations

[v-shahzad]

Description

This change introduces a pre-flight validation step within the $import data pipeline to proactively detect and report database constraint violations on a per-resource basis.

Related issues

Addresses AB# 151709

Testing

For now it just contains sample change for one constraint to get approval for the approach

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)

The best fix is to explicitly filter the sequence using .Where(...) to only enumerate rows matching the filtering logic. Furthermore, as the code breaks on the first match found, it is more idiomatic to use .FirstOrDefault(...) (or .Where(...).FirstOrDefault()) and check if the returned element is not null. This avoids the need for a loop and a manual break, improving readability.

Specifically, replace the loop:

foreach (var row in tokenStringCompositeSearchParamListRowGenerator.GenerateRows(new[] { wrapper }))
{
    if (row.Code1 != null && row.CodeOverflow1 != null &&
        Encoding.UTF8.GetByteCount(row.Code1) < VLatest.TokenStringCompositeSearchParam.Code1.Metadata.MaxLength)
    {
        resource.ImportError = Resources.TokenStringCompositeSearchParamCodeOverflow1;
        conflicts.Add(resource);
        hasConflict = true;
        break;
    }
}

with code that directly seeks the first offending row:

var offendingRow = tokenStringCompositeSearchParamListRowGenerator
    .GenerateRows(new[] { wrapper })
    .FirstOrDefault(row =>
        row.Code1 != null && row.CodeOverflow1 != null &&
        Encoding.UTF8.GetByteCount(row.Code1) < VLatest.TokenStringCompositeSearchParam.Code1.Metadata.MaxLength);

if (offendingRow != null)
{
    resource.ImportError = Resources.TokenStringCompositeSearchParamCodeOverflow1;
    conflicts.Add(resource);
    hasConflict = true;
}

Only the lines in the region of the original loop (lines 790-800) need changing. No new imports are needed because System.Linq is already imported. There is no need for method definitions or additional variable declarations.

---

To fix this, refactor the loop on line 808 so it iterates only over those elements that match the current filter condition. That is, replace:

foreach (var row in tokenQuantityCompositeSearchParamListRowGenerator.GenerateRows(new[] { wrapper }))
{
    if (row.Code1 != null && row.CodeOverflow1 != null &&
        Encoding.UTF8.GetByteCount(row.Code1) < VLatest.TokenQuantityCompositeSearchParam.Code1.Metadata.MaxLength)
    {
        resource.ImportError = Resources.TokenQuantityCompositeSearchParamCodeOverflow1;
        conflicts.Add(resource);
        hasConflict = true;
        break;
    }
}

with:

foreach (var row in tokenQuantityCompositeSearchParamListRowGenerator.GenerateRows(new[] { wrapper })
    .Where(row => row.Code1 != null && row.CodeOverflow1 != null &&
                  Encoding.UTF8.GetByteCount(row.Code1) < VLatest.TokenQuantityCompositeSearchParam.Code1.Metadata.MaxLength))
{
    resource.ImportError = Resources.TokenQuantityCompositeSearchParamCodeOverflow1;
    conflicts.Add(resource);
    hasConflict = true;
    break;
}

This pushes the filter into the enumerated sequence and eliminates unnecessary nesting. No change to imports is necessary because System.Linq is already imported, and the pattern matches those elsewhere in the file.

To fix the problem, refactor the foreach loop at line 844 to use .Where(...) in LINQ, expressing the filtering explicitly. However, since the loop sets some state (resource.ImportError, conflicts.Add(resource), hasConflict) and breaks after the first match, the general idiom is to use .FirstOrDefault(...) or .Any(...) for such early-exit filters.

Specifically:

• Replace the foreach loop with a LINQ expression that finds the first row matching the condition.
• If a matching row is found, execute the same block as before.
• This not only reduces nesting but also clarifies intent.
• No new imports are needed, as LINQ is already imported.
• Only the block at lines 844–854 is changed.

---

To fix the problem, the iteration over referenceTokenCompositeSearchParamListRowGenerator.GenerateRows(new[] { wrapper }) should be replaced with a foreach over the filtered/conditioned sequence using .Where(...), where the lambda matches the filtering condition currently present in the if statement (row.Code2 != null && row.CodeOverflow2 != null && Encoding.UTF8.GetByteCount(row.Code2) < VLatest.ReferenceTokenCompositeSearchParam.Code2.Metadata.MaxLength). Additionally, because the body of the if becomes the body of the loop (now only executed for matching rows), we can remove the if and have a flat block. No new dependencies are required, as LINQ is already imported. Only the relevant lines in the loop need changing (foreach (... in ...) and internal use of if). No other refactoring or functional change is required.

To fix the overly broad catch clause on line 874 of src/Microsoft.Health.Fhir.SqlServer/Features/Storage/SqlServerFhirDataStore.cs, refine the exception handling to catch only exceptions that can reasonably be expected during resource validation—typically ArgumentException, InvalidOperationException, or other relevant exceptions thrown by row generation or resource processing. Replace the generic catch (Exception ex) with either one or several specific catch clauses, or a multi-catch with the most appropriate types.

Edit only the block beginning at line 874:

874:                         catch (Exception ex)
875:                         {
876:                             _logger.LogWarning(ex, string.Format(Resources.ExceptionWhileResourceValidation, resource.Index));
877:                             resource.ImportError = string.Format(Resources.ExceptionWhileResourceValidation, resource.Index);
878:                             conflicts.Add(resource);
879:                             continue;
880:                         }

Change it to catch specific exception types (for example, ArgumentException, InvalidOperationException) and, if needed, add additional types if row generation is known to throw other exceptions.

No new imports are needed, as both exception types are in System. No new definitions or methods are required.

The best way to fix this is to replace the entire block of code that checks each row for validity and sets hasConflict with a more idiomatic LINQ expression. Specifically, use .Any(...) on the generated rows with the same filter condition, and handle registration and error setting inside the conditional branch. This removes the explicit loop and clarifies the logic. Make sure the side effects (setting ImportError, adding to conflicts) happen only the first time the condition is met (which aligns with the current behavior). The replacement should cover lines 771–782, and you may need several lines of surrounding context for clarity. No new imports are required, as System.Linq is already imported.

[feordin]

What kind of impact do we think this will have on $import performance?