Implementing a duplicate clinical reference (approach 1).

[v-isyamauchi-gh]

Description

Implementing a duplicate clinical reference feature (approach 1).

Related issues

Addresses [issue #158013].

User Story 158013: Duplicate url information on 2 properties for DocumentReference

Testing

Tested by adding UTs.

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)

To fix the problem, we should replace the foreach loop at line 384 that transforms each element in duplicateResources into a SearchResultEntry containing a ResourceWrapper with a single line that projects the transformation using LINQ's .Select. Specifically, we will use .Select to create a list of SearchResultEntry objects from the duplicateResources collection, and then assign the resulting list to entries.

• In src/Microsoft.Health.Fhir.Shared.Core.UnitTests/Features/Guidance/ClinicalReferenceDuplicatorTests.cs, replace the initialization and population of entries within the conditional block around lines 381-396 with one assignment: entries = duplicateResources.Select(...).ToList();.
• No new methods or imports are needed since System.Linq is already imported.

---

The best way to address this problem is to remove the assignment to resourceWrapper on line 149. Since the method's returned value is unused, and there's no evidence in the code that CreateResourceWrapper(resourceElement) must be called for necessary side effects, the assignment and the function call can be safely deleted.

Specifically, in DuplicateClinicalReferenceBehaviorTests.cs, within the method body of GivenDeleteRequest_WhenDuplicateClinicalReferenceIsEnabled_ThenThenDuplicateResourceShouldBeDeleted, simply delete the line:

var resourceWrapper = CreateResourceWrapper(resourceElement);

No imports, method definitions, or variable definitions need to be added or changed.

To properly fix this issue, add an explicit null check for the coreFeatureConfiguration parameter before accessing its .Value property. This can be achieved by using EnsureArg.IsNotNull(coreFeatureConfiguration, nameof(coreFeatureConfiguration)); before the existing check on coreFeatureConfiguration.Value. This ensures that the constructor will fail early and clearly if a null parameter is ever passed, and will not cause a fail-later NullReferenceException. No changes to functionality are introduced; only the missing null parameter guard is added. This change should be applied at the very top of the constructor, specifically in the region of lines 29–31.

---

To fix the problem, refactor the foreach loop at line 455 that accumulates asynchronously-updated wrappers into a single statement using LINQ's Select and Task.WhenAll. Specifically, project the resourceWrappersFound collection to a collection of tasks using Select, then use Task.WhenAll to await all those tasks in parallel, and finally convert the resulting array to a list. Replace the manual foreach loop with this one-liner, assigning the result to duplicateResourceWrappers.

• Changes are required in src/Microsoft.Health.Fhir.Shared.Core/Features/Guidance/ClinicalReferenceDuplicator.cs, at the block starting at line 455.
• No extra methods or imports are required; all necessary types (Select, Task.WhenAll, .ToList()) are already available and relevant namespaces are already imported.
• No side effects are performed inside the loop; no change to program logic or state is expected.

---

To fix the problem, the assignment to codes should be removed from line 595 in ClinicalReferenceDuplicator.cs. The result of crefs.SelectMany(x => x.codes) is never used via the codes variable; instead, the same expression is used directly in the subsequent foreach loop. Thus, simply delete the line. No further changes are needed. No additional imports, definitions, or method changes are required.

To fix the problem, add a null check for y before dereferencing it in the Select clause. The best approach is to check if y is not null before casting y.Value as Coding. This can be done by modifying the lambda expression in the Select statement such that it returns (Coding)y.Value only if y is not null, otherwise returns null, and then filter out nulls before further processing (via Where, for example).

The code to change is in src/Microsoft.Health.Fhir.Shared.Core/Features/Guidance/ClinicalReferenceDuplicator.cs, specifically lines 630-634 for the block under the #else conditional. We will update the SelectMany so that y is checked for null before casting y.Value to Coding, and filter the result to non-null values.

To fix the problem, refactor the foreach loop at line 731 so that it iterates only over those attachment elements of diagnosticReport.PresentedForm for which !documentReference.Content.Any(...) holds. This is done by replacing the loop:

foreach (var attachment in diagnosticReport.PresentedForm)
{
    if (!documentReference.Content.Any(...))
    {
        // Loop body
    }
}

with:

foreach (var attachment in diagnosticReport.PresentedForm
    .Where(attachment => !documentReference.Content.Any(
        x => ClinicalReferenceDuplicatorHelper.CompareCoding(x.Format, code)
                  && ClinicalReferenceDuplicatorHelper.CompareAttachment(x.Attachment, attachment))))
{
    // Loop body (without guard if)
}

Inside the rewritten loop, remove the redundant if-statement and directly execute the logic.

Only the code in file src/Microsoft.Health.Fhir.Shared.Core/Features/Guidance/ClinicalReferenceDuplicator.cs, lines around 730–745, needs to be changed.

No new imports, methods, or definitions are needed as LINQ is already imported.

---

The best fix is to filter contents with .Where(...) so the loop iterates only over those content objects where !ClinicalReferenceDuplicatorHelper.CompareCodings(content.Profile, profiles) holds. Specifically, refactor the foreach to:

foreach (var content in contents.Where(content => !ClinicalReferenceDuplicatorHelper.CompareCodings(content.Profile, profiles)))
{
    // body as before, without the if statement
}

Then, remove the inner if (now redundant), and dedent the loop body. This reduces the nesting level and clarifies that only content not matching the codings will be processed. No new imports or helpers are required; System.Linq is already imported.

Only the block spanning lines 772–782 in file src/Microsoft.Health.Fhir.Shared.Core/Features/Guidance/ClinicalReferenceDuplicator.cs needs to be changed.

---

To ensure the code does not dereference a possibly-null variable, we should add an explicit check returning true if both x and y are null, and false if only one is null. This pattern matches the one used in other comparison methods throughout the file (e.g., CompareAttachment and CompareContent). Specifically, insert a block after the method signature to return true if both are null, and false if either is null. Then, the rest of the function is only used when both lists are definitely non-null. All changes belong in src/Microsoft.Health.Fhir.Shared.Core/Features/Guidance/ClinicalReferenceDuplicatorHelper.cs within the CompareCodings(IReadOnlyList<DocumentReference.ProfileComponent> x, IReadOnlyList<DocumentReference.ProfileComponent> y) method body.

To safely handle the possibility of x and/or y being null, we should explicitly check for null before calling methods on these variables. Specifically:

• Before using x.All() or y.Any(), we should check if either variable is null.
• Since the logic already checks for count mismatches at line 96, if both are null, this check passes and we end up dereferencing x on line 101.
• The best way to preserve functionality and avoid changing method semantics is to add an explicit check: if both x and y are null, they are considered equal and the function should return true. If one is null and the other is not, return false. If neither is null, proceed with the original logic.
• This fix should replace the region of the code starting from the null-check and covering the method logic in CompareContents (lines 95-102). No new imports or dependencies are needed.

To fix the problem, simply discard the unused variable assignment by replacing var duplicate with _ in the deconstruction. This makes it clear to readers and maintainers of the code that the second value returned by CreateResourceAsync(resource) is intentionally ignored since only source is needed. This should be changed only in the relevant location in the method GivenResource_WhenDeleting_ThenDuplicateResourceShouldBeDeleted. No other method, import or definition changes are needed.

To fix the problem, replace the assignment to var duplicate with a discard assignment (_). This means changing the code from (var source, var duplicate) to (var source, _) on line 67 inside the method GivenResource_WhenUpdating_ThenDuplicateResourceShouldBeUpdated. This will clarify that the second item of the returned tuple is intentionally not used, and will prevent any future confusion about unused variables. No imports or additional definitions are needed for this fix, as discards are standard in C# for ignoring unused tuple elements.

---