Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Oct 28, 2025

Null entries in FHIR resource collections (Extension, Identifier, etc.) were causing NullReferenceException during DeepCopy operations in SDK 6.

Changes

Updated for SDK 6 (v6.0.2)
This PR has been rebased onto the SDK 6 develop branch to target the correct version where the bug was reported.

DeepCopy

  • Fixed Base.Extensions.DeepCopyInternal<T>() to handle null items without throwing NullReferenceException
  • Null items are preserved in the copied collection (maintains list integrity)
  • Uses null-conditional operator: item is not null ? item.DeepCopy() : null!

Serialization Behavior

  • Nulls in collections are preserved during JSON/XML serialization
  • No changes made to serialization logic - nulls will appear in serialized output

Tests

  • Added comprehensive tests using SDK 6 APIs (FhirJsonSerializer/FhirXmlSerializer)
  • Tests verify DeepCopy doesn't throw on nulls, preserves nulls in collections, and serialization keeps nulls in output

Example

var patient = new Patient { Id = "example" };
patient.Extension = [null, new Extension("http://test", new FhirString("abcd"))];

// Serialization output (nulls preserved):
// {"extension": [null, {"url":"http://test", "valueString":"abcd"}]}

var copy = (Patient)patient.DeepCopy(); // No longer throws NullReferenceException
// copy.Extension.Count == 2 (null is preserved)

SDK 6 Architecture: The DeepCopy logic is in Base.Extensions.cs in SDK 6 (moved from IDeepCopyable.cs in SDK 5).

Original prompt

This section details on the original issue you should resolve

<issue_title>SDK 6 Throws NullReferenceException when serializing with a null list member</issue_title>
<issue_description>Describe the bug

Our developers sometimes adds null Extension objects inside Extensions list.

var patient = new Patient { Id = "example" };
patient.Extension = [null, new("http://test", new FhirString("abcd"))];

// Expected json: {"resourceType": "Patient", "id": "example", "extension": [{"url":"http://test", "valueString": "abcd"}]}

// Wrong json: {"resourceType": "Patient", "id": "example", "extension": [null, {"url":"http://test", "valueString": "abcd"}]}

When we used the Newtonsoft.Json serializer, this would be automatically be set ignored during serialization, but using the SystemTextJson serializer prior to SDK 6, we added a custom SerializationFilter that would filter out these nulls,

sealed class NullIgnoringSerializationFilter(SerializationFilter? original) : SerializationFilter
{
    public override void EnterObject(object value, ClassMapping? mapping)
        => original?.EnterObject(value, mapping);

    public override void LeaveMember(string name, object value, PropertyMapping? mapping)
        => original?.LeaveMember(name, value, mapping);

    public override void LeaveObject(object value, ClassMapping? mapping)
        => original?.LeaveObject(value, mapping);

    public override bool TryEnterMember(string name, object value, PropertyMapping? mapping)
    {
        if (value is IList collection)
        {
            while (collection.Contains(null))
            {
                collection.Remove(null!);
            }
        }

        return original?.TryEnterMember(name, value, mapping) ?? true;
    }
}

However when tested with SDK6, whenever a null is encountered in a list, this exception always shows up:

Unhandled exception. System.NullReferenceException: Object reference not set to an instance of an object.
   at Hl7.Fhir.Model.BaseExtensions.DeepCopy[T](T source)
   at Hl7.Fhir.Model.BaseExtensions.<>c__8`1.<DeepCopyInternal>b__8_0(T item)
   at System.Linq.Enumerable.SelectListIterator`2.Fill(ReadOnlySpan`1 source, Span`1 destination, Func`2 func)
   at System.Linq.Enumerable.SelectListIterator`2.ToList()
   at Hl7.Fhir.Model.BaseExtensions.DeepCopyInternal[T](IEnumerable`1 source)
   at Hl7.Fhir.Model.DomainResource.CopyToInternal(Base other)
   at Hl7.Fhir.Model.Patient.CopyToInternal(Base other)
   at Hl7.Fhir.Model.Patient.DeepCopyInternal()
   at Hl7.Fhir.Model.BaseExtensions.DeepCopy[T](T source)
   at Hl7.Fhir.Utility.SerializationUtil.MakeSubsettedClone(Base source)
   at Hl7.Fhir.Serialization.BaseFhirJsonSerializer.Serialize(Base instance, Utf8JsonWriter writer, Func`1 filterFactory)
   at Hl7.Fhir.Serialization.FhirJsonConverter`1.Write(Utf8JsonWriter writer, TF poco, JsonSerializerOptions options)
   at System.Text.Json.Serialization.JsonConverter`1.TryWrite(Utf8JsonWriter writer, T& value, JsonSerializerOptions options, WriteStack& state)
   at System.Text.Json.Serialization.JsonConverter`1.WriteCore(Utf8JsonWriter writer, T& value, JsonSerializerOptions options, WriteStack& state)
   at System.Text.Json.Serialization.Metadata.JsonTypeInfo`1.Serialize(Utf8JsonWriter writer, T& rootValue, Object rootValueBoxed)
   at System.Text.Json.JsonSerializer.WriteString[TValue](TValue& value, JsonTypeInfo`1 jsonTypeInfo)
   at System.Text.Json.JsonSerializer.Serialize[TValue](TValue value, JsonSerializerOptions options)
   at Program.<Main>$(String[] args) in D:\repos\FhirSerializationTest\FhirSerializationTest\Program.cs:line 16

I asked our developers to avoid these null list members in the future. Also, it would be good if the serializer can automatically ignore these null members without having to write a serialization filter, or at least, honor the JsonSerializerOptions.DefaultIgnoreCondition when it is set to JsonIgnoreCondition.WhenWritingDefault.

Versions used:
6.0.1, 5.12.2</issue_description>

Comments on the Issue (you are @copilot in this section)


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

Copilot AI self-assigned this Oct 28, 2025
Copilot AI changed the title [WIP] Fix NullReferenceException when serializing null list items Filter null values from collections during serialization and DeepCopy Oct 28, 2025
Copilot AI requested a review from alexzautke October 28, 2025 09:55
Copilot finished work on behalf of alexzautke October 28, 2025 09:55
@ewoutkramer
Copy link
Member

This PR is fixing a bug report for sdk6 and fixes it in sdk5, so I think this is a bogus PR.

@alexzautke alexzautke reopened this Oct 28, 2025
@alexzautke
Copy link
Member

@copilot Account for the comment from @ewoutkramer #3329 (comment)

Copy link
Contributor Author

Copilot AI commented Oct 28, 2025

This PR is fixing a bug report for sdk6 and fixes it in sdk5, so I think this is a bogus PR.

I've reviewed the issue and the current repository state. The repository is currently on SDK version 5.13.0, and I don't see any SDK 6 branch or version.

The issue description mentions "Versions used: 6.0.1, 5.12.2" - could you clarify what "6.0.1" refers to? Is this:

  1. A pre-release/internal version of SDK 6 that I should target instead?
  2. A confusion with FHIR spec version (R6)?
  3. An error in the issue report?

My changes currently fix the null serialization issue in SDK 5.13.0. Should I:

  • Close this PR if the issue only affects SDK 6?
  • Keep the PR if the issue also affects SDK 5?
  • Create a new PR targeting a different branch for SDK 6?

Please advise on the appropriate action.

Copilot finished work on behalf of alexzautke October 28, 2025 20:46
@alexzautke
Copy link
Member

@copilot Rebase the PR with the current develop and you'll that there's an SDK 6

Copy link
Contributor Author

Copilot AI commented Oct 28, 2025

@copilot Rebase the PR with the current develop and you'll that there's an SDK 6

Rebased onto SDK 6 develop branch (v6.0.2). The changes have been updated for the SDK 6 architecture:

Modified files for SDK 6:

  • BaseFhirJsonSerializer.cs - Skip null values when serializing collections
  • BaseFhirXmlSerializer.cs - Skip null values when serializing collections
  • Base.Extensions.cs - Filter nulls in DeepCopyInternal method

Tests:

  • Added comprehensive tests using SDK 6 APIs (FhirJsonSerializer/FhirXmlSerializer)
  • All 4 tests passing

The SDK 6 architecture differs from SDK 5 - the serialization logic moved from BaseFhirJsonPocoSerializer to BaseFhirJsonSerializer, and the DeepCopy logic moved from IDeepCopyable.cs to Base.Extensions.cs.

Copilot finished work on behalf of alexzautke October 28, 2025 21:20
@alexzautke
Copy link
Member

@copilot Based on the comment in #3329 (comment) you should have kept the support for serializing nulls and not skip them. Just avoid the null pointer exception when doing the DeepCopy.

Copy link
Contributor Author

Copilot AI commented Oct 28, 2025

@copilot Based on the comment in #3329 (comment) you should have kept the support for serializing nulls and not skip them. Just avoid t...

Updated the implementation per your feedback (commit 2717f9a).

Changes:

  • Only fixed DeepCopy to avoid NullReferenceException - nulls are now preserved in the copied collection
  • Removed all serialization changes - nulls are now kept in JSON/XML output
  • Modified Base.Extensions.DeepCopyInternal to use null-conditional: item is not null ? item.DeepCopy() : null!

Tests confirm:

  • DeepCopy no longer throws NullReferenceException
  • DeepCopy preserves nulls in collections (maintains list length)
  • JSON serialization outputs null for null items
  • XML serialization handles null items

All 4 tests passing.

Copilot AI changed the title Filter null values from collections during serialization and DeepCopy Fix DeepCopy NullReferenceException while preserving null serialization Oct 28, 2025
Copilot finished work on behalf of alexzautke October 28, 2025 21:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SDK 6 Throws NullReferenceException when serializing with a null list member

4 participants