Feature/firely 6 update

[brendankowitz]

Description

This pull request introduces significant changes to how FHIR resources are represented and processed within the codebase, moving from the use of ITypedElement to PocoNode for resource elements. This update impacts a wide range of files, refactoring interfaces, implementations, and test code to align with the new model. Additionally, there are several dependency version updates and some package version downgrades.

The most important changes are:

Core Model Refactoring

• Refactored ResourceElement and related APIs to use PocoNode instead of ITypedElement, affecting constructors, properties, and internal logic in ResourceElement, ResourceTypeExtensions, and all related usages. This change improves consistency and interoperability with the FHIR .NET library. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11]
• Updated the IReferenceToElementResolver interface and its implementation to return PocoNode instead of ITypedElement. [1] [2] [3]
• Changed the IModelInfoProvider.GetEvaluationContext signature to accept a resolver function returning PocoNode. [1] [2]

Dependency and Package Version Updates

• Downgraded or updated several package versions in Directory.Packages.props, including HealthcareSharedPackageVersion, DotNetSdkPackageVersion, Azure.Identity, Microsoft.Data.SqlClient, and Newtonsoft.Json. Also, updated Hl7FhirVersion and OpenIddict packages to newer versions. [1] [2] [3] [4]
• Added new package references, such as IdentityServer4 and updated Microsoft.EntityFrameworkCore.InMemory to a newer version. [1] [2]

Test Adjustments

• Updated unit tests to use the new PocoNode-based APIs and adjusted mock setups and test logic accordingly. [1] [2] [3] [4] [5] [6]

Minor Logic Changes

• Improved FHIRPath evaluation and search parameter extraction to operate on PocoNode instances, ensuring correct FHIRPath context and evaluation. [1] [2] [3]
• Fixed logic in CreateBulkUpdateHandler to correctly extract the operation value from FHIR parameters, handling FhirString specifically.

These changes collectively modernize the resource handling pipeline and align the codebase with the latest FHIR .NET best practices.

Related issues

Addresses [issue #].

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)

[brendankowitz]

bad things happened to this merge 😞

To fix the issue, simply remove the redundant cast (int)i and use i directly as the argument to GetHttpVerb. This means editing the one line within the method GivenABatchOperation_WhenAppendedMultipleResourcesInParallelWaitForAllToBeAppended_ThenCompleteWithSuccess so that GetHttpVerb((int)i) becomes GetHttpVerb(i). No imports, methods, or additional definitions are required.

The best way to fix the problem is to remove the redundant cast (int)i on line 220. Instead, pass i directly to the GetHttpVerb function. No changes are needed to any imports, method signatures, or additional code outside of removing the unnecessary cast. Only a single edit to line 220 in the file is required.

To fix this issue, add the readonly modifier to the field _parser. This ensures that _parser can only be set during the field's declaration or within the class's constructor, preventing unintended assignment elsewhere. Specifically, locate the declaration private FhirJsonDeserializer _parser; (line 23) in the file ImportResourceParser.cs and change it to private readonly FhirJsonDeserializer _parser;. No other code or imports need to be changed, as assignment occurs appropriately in the constructor.

---

To fix the issue, combine the two if statements at lines 30 and 32 into a single conditional by combining their conditions with &&. Use proper parentheses to ensure correct operator precedence and take care that the inline variable (reference) declared by the pattern-matching is expression is only used if the type match succeeds. In recent C# versions, this is permitted directly in the conditional. Replace:

if (resource is ResourceReference reference)
{
    if (reference.Reference != null && reference.Reference.Equals(target, StringComparison.OrdinalIgnoreCase))
    {
        // ...
    }
}

with:

if (resource is ResourceReference reference 
    && reference.Reference != null 
    && reference.Reference.Equals(target, StringComparison.OrdinalIgnoreCase))
{
    // ...
}

Only lines within the method RemoveReferenceRecursive in file src/Microsoft.Health.Fhir.Shared.Core/Features/Resources/Delete/ReferenceRemover.cs need updating.

No additional imports, methods, or definitions are required.

To fix this problem, simply remove the unused assignment var fhirJsonSerializer = new FhirJsonSerializer(); from line 329 in the method. This does not affect the program logic, since the instantiated object is never used, nor does its construction have any necessary, visible side-effect in this context. No other changes are needed elsewhere, and no imports or definitions need to be modified or added.

In general, to fix a "useless assignment to local variable" you either (a) remove the unused variable and its assignment if they are truly unnecessary, or (b) start using the variable where it was intended to be used, if the code is incomplete. Here, the parser variable is created but never used, and the test already performs JSON conversion via parameters.ToJson(), so the parser is clearly redundant.

The best fix without changing functionality is to delete the declaration/assignment line var parser = new FhirJsonDeserializer(); in GivenInvalidProfile_WhenValidateCalled_ThenBadRequestReturned in test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/ValidateTests.cs. No other code in the method needs to be updated, and no new imports or helper methods are required. This change simply removes dead code and does not affect the test’s behavior.

To fix the problem, the _fhirJsonDeserializer field should be marked readonly so it cannot be reassigned after the object is constructed. For consistency and the same reasoning, the other similar dependency fields (_resourceWrapperFactory, _fhirJsonSerializer, _modelInfoProvider) should also be made readonly, provided they are initialized in the constructor and not reassigned elsewhere. This change does not alter runtime behavior as long as there are no later assignments; it only enforces the existing usage pattern at compile time.

Concretely, in tools/Microsoft.Health.Fhir.R4.ResourceParser/ResourceWrapperParser.cs, update the field declarations near the top of the ResourceWrapperParser class (lines 27–30) by adding the readonly modifier to each of these four private fields:

• private ResourceWrapperFactory _resourceWrapperFactory;
• private FhirJsonDeserializer _fhirJsonDeserializer;
• private FhirJsonSerializer _fhirJsonSerializer;
• private IModelInfoProvider _modelInfoProvider;

No additional methods, imports, or definitions are required; this is a modifier-only change.

In general, to fix this kind of issue you merge nested if statements that both guard the same block of logic and have no else parts into a single if whose condition is the logical && of the original conditions. This keeps behavior identical but avoids unnecessary nesting and makes intent clearer.

In this specific case in src/Microsoft.Health.Fhir.Core/Extensions/BaseExtensions.cs, within the for (int j = 0; j < sourceList.Count; j++) loop, there is an outer if (sourceList[j] is Base sourceChild && otherList[j] is Base otherChild) and then an inner if (!sourceChild.EqualValues(...)). CodeQL, however, is flagging the pattern at line 85 as nested ifs that can be combined. The actual nested-if pattern exists one level deeper: currently the code is:

for (int j = 0; j < sourceList.Count; j++)
{
    if (sourceList[j] is Base sourceChild && otherList[j] is Base otherChild)
    {
        if (!sourceChild.EqualValues(sourceElement.Key, otherChild, otherElement.Key))
        {
            return false;
        }
    }
}

We can combine the two if statements into one by moving the EqualValues check into the combined condition. The resulting code:

for (int j = 0; j < sourceList.Count; j++)
{
    if (sourceList[j] is Base sourceChild && otherList[j] is Base otherChild &&
        !sourceChild.EqualValues(sourceElement.Key, otherChild, otherElement.Key))
    {
        return false;
    }
}

preserves the logic: EqualValues is only evaluated when both list entries are Base instances, and return false is executed when they are both Base and not equal. No new imports or helper methods are required; this is a local change inside EqualValues(Base source, string sourceName, Base other, string otherName).