Skip to content

Fix thread-safety issue with SerializationFilter when reusing JsonSerializerOptions #3218

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
Jul 24, 2025
Merged
21 changes: 15 additions & 6 deletions src/Hl7.Fhir.Base/Serialization/BaseFhirJsonPocoSerializer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -86,10 +86,19 @@ public string SerializeToString(IReadOnlyDictionary<string, object> members)
private void serializeInternal(
IReadOnlyDictionary<string, object> members,
Utf8JsonWriter writer,
bool skipValue)
bool skipValue,
SerializationFilter? filter = null)
{
writer.WriteStartObject();
var filter = Settings.SummaryFilter;

// Get filter only once at the top level, then pass it through recursive calls
if (filter == null)
{
// Use factory if available, otherwise fall back to the static instance for backward compatibility
#pragma warning disable CS0618 // Type or member is obsolete
filter = Settings.SummaryFilterFactory?.Invoke() ?? Settings.SummaryFilter;
#pragma warning restore CS0618 // Type or member is obsolete
}

if (members is Resource r)
writer.WriteString("resourceType", r.TypeName);
Expand Down Expand Up @@ -132,12 +141,12 @@ private void serializeInternal(
writer.WriteStartArray();

foreach (var value in coll)
serializeMemberValue(value, writer, requiredType);
serializeMemberValue(value, writer, filter, requiredType);

writer.WriteEndArray();
}
else
serializeMemberValue(member.Value, writer, requiredType);
serializeMemberValue(member.Value, writer, filter, requiredType);
}

filter?.LeaveMember(member.Key, member.Value, propertyMapping);
Expand All @@ -159,10 +168,10 @@ private static string addSuffixToElementName(string elementName, object elementV
return typeName is null ? elementName : elementName + char.ToUpperInvariant(typeName[0]) + typeName.Substring(1);
}

private void serializeMemberValue(object value, Utf8JsonWriter writer, Type? requiredType = null)
private void serializeMemberValue(object value, Utf8JsonWriter writer, SerializationFilter? filter, Type? requiredType = null)
{
if (value is IReadOnlyDictionary<string, object> complex)
serializeInternal(complex, writer, skipValue: false);
serializeInternal(complex, writer, skipValue: false, filter: filter);
else
SerializePrimitiveValue(value, writer, requiredType);
}
Expand Down
37 changes: 32 additions & 5 deletions src/Hl7.Fhir.Base/Serialization/BaseFhirXmlPocoSerializer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,12 @@ public BaseFhirXmlPocoSerializer(FhirRelease release)
}

/// <summary>
/// Serializes the given dictionary with FHIR data into Json.
/// Serializes the given dictionary with FHIR data into XML.
/// </summary>
public void Serialize(IReadOnlyDictionary<string, object> members, XmlWriter writer, SerializationFilter? summary = default)
/// <param name="members">The dictionary containing FHIR data to serialize.</param>
/// <param name="writer">The XmlWriter to write the serialized data to.</param>
/// <param name="summaryFilterFactory">A factory function that creates a new filter instance for each serialization operation. This ensures thread-safety when reusing serializer instances in concurrent scenarios.</param>
public void Serialize(IReadOnlyDictionary<string, object> members, XmlWriter writer, Func<SerializationFilter>? summaryFilterFactory)
{
writer.WriteStartDocument();

Expand All @@ -57,17 +60,41 @@ public void Serialize(IReadOnlyDictionary<string, object> members, XmlWriter wri
writer.WriteStartElement(rootElementName, XmlNs.FHIR);
}

serializeInternal(members, writer, summary);
var filter = summaryFilterFactory?.Invoke();
serializeInternal(members, writer, filter);

if (simulateRoot) writer.WriteEndElement();
writer.WriteEndDocument();
}

/// <summary>
/// Serializes the given dictionary with FHIR data into UTF8 encoded Json.
/// Serializes the given dictionary with FHIR data into XML.
/// </summary>
/// <param name="members">The dictionary containing FHIR data to serialize.</param>
/// <param name="writer">The XmlWriter to write the serialized data to.</param>
/// <param name="summary">The serialization filter to apply. NOTE: For thread-safety when reusing serializer instances, pass a fresh filter instance for each serialization operation.</param>
[Obsolete("Use the overload that takes Func<SerializationFilter> summaryFilterFactory instead to ensure thread-safety when reusing serializer instances in concurrent scenarios. This method will be removed in a future version.")]
public void Serialize(IReadOnlyDictionary<string, object> members, XmlWriter writer, SerializationFilter? summary = default)
{
Serialize(members, writer, summary != null ? () => summary : (Func<SerializationFilter>?)null);
}

/// <summary>
/// Serializes the given dictionary with FHIR data into UTF8 encoded XML.
/// </summary>
/// <param name="members">The dictionary containing FHIR data to serialize.</param>
/// <param name="summaryFilterFactory">A factory function that creates a new filter instance for each serialization operation. This ensures thread-safety when reusing serializer instances in concurrent scenarios.</param>
public string SerializeToString(IReadOnlyDictionary<string, object> members, Func<SerializationFilter>? summaryFilterFactory) =>
SerializationUtil.WriteXmlToString(w => Serialize(members, w, summaryFilterFactory));

/// <summary>
/// Serializes the given dictionary with FHIR data into UTF8 encoded XML.
/// </summary>
/// <param name="members">The dictionary containing FHIR data to serialize.</param>
/// <param name="summary">The serialization filter to apply. NOTE: For thread-safety when reusing serializer instances, pass a fresh filter instance for each serialization operation.</param>
[Obsolete("Use the overload that takes Func<SerializationFilter> summaryFilterFactory instead to ensure thread-safety when reusing serializer instances in concurrent scenarios. This method will be removed in a future version.")]
public string SerializeToString(IReadOnlyDictionary<string, object> members, SerializationFilter? summary = default) =>
SerializationUtil.WriteXmlToString(w => Serialize(members, w, summary));
SerializeToString(members, summary != null ? () => summary : (Func<SerializationFilter>?)null);

/// <summary>
/// Serializes the given dictionary with FHIR data into Json, optionally skipping the "value" element.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,14 @@ public record FhirJsonPocoSerializerSettings
/// <summary>
/// Specifies the filter to use for summary serialization.
/// </summary>
[Obsolete("Use SummaryFilterFactory instead to ensure thread-safety when reusing JsonSerializerOptions instances. This property will be removed in a future version.")]
public SerializationFilter? SummaryFilter { get; set; } = default;

/// <summary>
/// Specifies a factory function that creates a new filter instance for each serialization operation.
/// This ensures thread-safety when reusing JsonSerializerOptions instances in concurrent scenarios.
/// </summary>
public Func<SerializationFilter>? SummaryFilterFactory { get; set; } = default;
}
}

Expand Down
103 changes: 77 additions & 26 deletions src/Hl7.Fhir.Base/Serialization/SerializationFilter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#nullable enable

using Hl7.Fhir.Introspection;
using System;

namespace Hl7.Fhir.Serialization
{
Expand Down Expand Up @@ -41,44 +42,94 @@ public abstract class SerializationFilter
/// <summary>
/// Construct a new filter that conforms to the `_summary=true` summarized form.
/// </summary>
public static SerializationFilter ForSummary() => new BundleFilter(new ElementMetadataFilter() { IncludeInSummary = true });
[Obsolete("Use CreateSummaryFactory() instead to ensure thread-safety when reusing JsonSerializerOptions instances. This method will be removed in a future version.")]
public static SerializationFilter ForSummary() => CreateSummaryFactory()();

/// <summary>
/// Construct a new filter that conforms to the `_summary=text` summarized form.
/// </summary>
public static SerializationFilter ForText() => new BundleFilter(new TopLevelFilter(
new ElementMetadataFilter()
{
IncludeNames = new[] { "text", "id", "meta" },
IncludeMandatory = true
}));

public static SerializationFilter ForCount() => new BundleFilter(new TopLevelFilter(
new ElementMetadataFilter()
{
IncludeMandatory = true,
IncludeNames = new[] { "id", "total", "link" }
}));
[Obsolete("Use CreateTextFactory() instead to ensure thread-safety when reusing JsonSerializerOptions instances. This method will be removed in a future version.")]
public static SerializationFilter ForText() => CreateTextFactory()();

[Obsolete("Use CreateCountFactory() instead to ensure thread-safety when reusing JsonSerializerOptions instances. This method will be removed in a future version.")]
public static SerializationFilter ForCount() => CreateCountFactory()();

/// <summary>
/// Construct a new filter that conforms to the `_summary=data` summarized form.
/// </summary>
public static SerializationFilter ForData() => new BundleFilter(new TopLevelFilter(
new ElementMetadataFilter()
{
IncludeNames = new[] { "text" },
Invert = true
}));
[Obsolete("Use CreateDataFactory() instead to ensure thread-safety when reusing JsonSerializerOptions instances. This method will be removed in a future version.")]
public static SerializationFilter ForData() => CreateDataFactory()();

/// <summary>
/// Construct a new filter that conforms to the `_elements=...` summarized form.
/// </summary>
public static SerializationFilter ForElements(string[] elements) => new BundleFilter(new TopLevelFilter(
new ElementMetadataFilter()
{
IncludeNames = elements,
IncludeMandatory = true
}));
[Obsolete("Use CreateElementsFactory() instead to ensure thread-safety when reusing JsonSerializerOptions instances. This method will be removed in a future version.")]
public static SerializationFilter ForElements(string[] elements) => CreateElementsFactory(elements)();

/// <summary>
/// Create a factory function that produces new filter instances conforming to the `_summary=true` summarized form.
/// Using this factory ensures thread-safety when reusing JsonSerializerOptions instances.
/// </summary>
public static Func<SerializationFilter> CreateSummaryFactory()
{
return () => new BundleFilter(new ElementMetadataFilter() { IncludeInSummary = true });
}

/// <summary>
/// Create a factory function that produces new filter instances conforming to the `_summary=text` summarized form.
/// Using this factory ensures thread-safety when reusing JsonSerializerOptions instances.
/// </summary>
public static Func<SerializationFilter> CreateTextFactory()
{
return () => new BundleFilter(new TopLevelFilter(
new ElementMetadataFilter()
{
IncludeNames = new[] { "text", "id", "meta" },
IncludeMandatory = true
}));
}

/// <summary>
/// Create a factory function that produces new filter instances conforming to the `_summary=count` summarized form.
/// Using this factory ensures thread-safety when reusing JsonSerializerOptions instances.
/// </summary>
public static Func<SerializationFilter> CreateCountFactory()
{
return () => new BundleFilter(new TopLevelFilter(
new ElementMetadataFilter()
{
IncludeMandatory = true,
IncludeNames = new[] { "id", "total", "link" }
}));
}

/// <summary>
/// Create a factory function that produces new filter instances conforming to the `_summary=data` summarized form.
/// Using this factory ensures thread-safety when reusing JsonSerializerOptions instances.
/// </summary>
public static Func<SerializationFilter> CreateDataFactory()
{
return () => new BundleFilter(new TopLevelFilter(
new ElementMetadataFilter()
{
IncludeNames = new[] { "text" },
Invert = true
}));
}

/// <summary>
/// Create a factory function that produces new filter instances conforming to the `_elements=...` summarized form.
/// Using this factory ensures thread-safety when reusing JsonSerializerOptions instances.
/// </summary>
public static Func<SerializationFilter> CreateElementsFactory(string[] elements)
{
return () => new BundleFilter(new TopLevelFilter(
new ElementMetadataFilter()
{
IncludeNames = elements,
IncludeMandatory = true
}));
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ public Resource DeserializeFromXml(string data)
}

/// <inheritdoc />
public string SerializeToXml(Resource instance) => getXmlSerializer().SerializeToString(instance);
public string SerializeToXml(Resource instance) => getXmlSerializer().SerializeToString(instance, (Func<SerializationFilter>?)null);

/// <summary>
/// Deserializes a resource from an XML reader
Expand Down Expand Up @@ -70,5 +70,5 @@ public Base DeserializeElementFromXml(Type targetType, XmlReader reader)
/// </summary>
/// <param name="instance">An instance of Base or any of its children</param>
/// <param name="writer">The XML writer</param>
public void SerializeToXmlWriter(Base instance, XmlWriter writer) => getXmlSerializer().Serialize(instance, writer);
public void SerializeToXmlWriter(Base instance, XmlWriter writer) => getXmlSerializer().Serialize(instance, writer, (Func<SerializationFilter>?)null);
}
Original file line number Diff line number Diff line change
Expand Up @@ -52,5 +52,56 @@ public void SerializesInvalidData()
contactArray.Count().Should().Be(1);
contactArray.First().Elements().Should().BeEmpty();
}

[TestMethod]
public void CanUseFilterFactory()
{
var patient = new Patient
{
Id = "test-patient",
Active = true,
Name = new() { new HumanName { Given = new[] { "John" }, Family = "Doe" } },
Gender = AdministrativeGender.Male
};

var serializer = new BaseFhirXmlPocoSerializer(Specification.FhirRelease.STU3);

// Test the new factory-based method
var elementsFactory = SerializationFilter.CreateElementsFactory(new[] { "id", "active" });
var xmlWithFactory = serializer.SerializeToString(patient, elementsFactory);

// Test the obsolete method for comparison
#pragma warning disable CS0618 // Type or member is obsolete
var filter = SerializationFilter.ForElements(new[] { "id", "active" });
var xmlWithFilter = serializer.SerializeToString(patient, filter);
#pragma warning restore CS0618 // Type or member is obsolete

// Both methods should produce identical output
xmlWithFactory.Should().Be(xmlWithFilter);

// Verify that filtering actually works (should only contain id and active)
var xdoc = XDocument.Parse(xmlWithFactory);
var patientElement = xdoc.Root;

// Should contain id and active elements
patientElement.Elements(XName.Get("id", XmlNs.FHIR)).Should().HaveCount(1);
patientElement.Elements(XName.Get("active", XmlNs.FHIR)).Should().HaveCount(1);

// Should NOT contain name or gender (they were filtered out)
patientElement.Elements(XName.Get("name", XmlNs.FHIR)).Should().BeEmpty();
patientElement.Elements(XName.Get("gender", XmlNs.FHIR)).Should().BeEmpty();
}

[TestMethod]
public void FilterFactoryCreatesNewInstancesEachTime()
{
var elementsFactory = SerializationFilter.CreateElementsFactory(new[] { "id", "active" });

// Each call should return a new instance
var filter1 = elementsFactory();
var filter2 = elementsFactory();

filter1.Should().NotBeSameAs(filter2);
}
}
}
Loading