Skip to content
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

2550 parser error ignore customization #2730

Merged
merged 21 commits into from
Mar 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
c4098cd
Added (failing) test for ignoring, added custom mode to the serializa…
Kasdejong Feb 21, 2024
abf8895
PocoSerializationEngine.cs now supports streams for its methods, and …
Kasdejong Feb 21, 2024
1d033b3
Added obsoletion comments for directly creating FhirJsonConverters
Kasdejong Feb 21, 2024
6aa43bf
Added optional (de)serializer settings to the SerializationEngineFactory
Kasdejong Feb 21, 2024
8190c2d
clarifications in obsoletion comments
Kasdejong Feb 21, 2024
e8d4ffe
Added engine to converters
Kasdejong Feb 21, 2024
9520fbe
Merge branch 'develop' into 2550-parser-error-ignore-customization
Kasdejong Feb 22, 2024
88f866d
renamed interface for serialization engine compatible with json conve…
Kasdejong Feb 22, 2024
423d51e
fixed it yayyy
Kasdejong Feb 22, 2024
710c70f
Predicate-based implementation, WIP
Kasdejong Mar 5, 2024
ed33831
Fixed comments
Kasdejong Mar 5, 2024
db3e780
Added enforcing extension to JsonSerializerOptionsExtensions.cs
Kasdejong Mar 5, 2024
a862495
tests are WIP
Kasdejong Mar 5, 2024
d826479
Added tests and refined a comment
Kasdejong Mar 6, 2024
9117e37
Merge branch 'develop' into 2550-parser-error-ignore-customization
mmsmits Mar 6, 2024
65a2aba
fixed bug where ostrich mode would override other custom ignore filters
Kasdejong Mar 6, 2024
0b4222a
Merge branch 'develop' into 2550-parser-error-ignore-customization
Kasdejong Mar 6, 2024
ba3cf03
hopefully fixed merge request suggestions
Kasdejong Mar 14, 2024
b0c032d
this test was not supposed to be in this branch
Kasdejong Mar 19, 2024
4dd09dc
More PR change requests
Kasdejong Mar 19, 2024
59af59c
Merge branch 'develop' into 2550-parser-error-ignore-customization
mmsmits Mar 19, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,12 @@ public static BaseFhirClient WithOstrichModeSerializer(this BaseFhirClient clien
return client;
}

public static BaseFhirClient WithCustomIgnoreListSerializer(this BaseFhirClient client, string[] ignoreList)
{
client.Settings.SerializationEngine = FhirSerializationEngineFactory.Custom(client.Inspector, ignoreList.ToPredicate());
return client;
}

/// <summary>
/// Configures the FhirClient to use the legacy serializer, configured to ignore all errors.
/// NB: This may cause data loss.
Expand Down
97 changes: 54 additions & 43 deletions src/Hl7.Fhir.Base/Serialization/FhirJsonConverter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@

using Hl7.Fhir.Introspection;
using Hl7.Fhir.Model;
using Hl7.Fhir.Utility;
using System;
using System.Collections.Generic;
using System.Reflection;
using System.Text.Json;
using System.Text.Json.Serialization;
Expand All @@ -22,78 +24,91 @@ namespace Hl7.Fhir.Serialization
/// <summary>
/// A converter factory to construct FhirJsonConverters for subclasses of <see cref="Base"/>.
/// </summary>
public class FhirJsonConverterFactory : JsonConverterFactory
public class FhirJsonConverterFactory(ModelInspector inspector, FhirJsonPocoSerializerSettings serializerSettings, FhirJsonPocoDeserializerSettings deserializerSettings) : JsonConverterFactory
{
private readonly ModelInspector _inspector;
private readonly FhirJsonPocoSerializerSettings _serializerSettings;
private readonly FhirJsonPocoDeserializerSettings _deserializerSettings;
internal PocoSerializationEngine? Engine { get; set; }

private PocoSerializationEngine createDefaultEngine()
{
return (PocoSerializationEngine)FhirSerializationEngineFactory.Strict(inspector, serializerSettings, deserializerSettings);
}

public FhirJsonConverterFactory(
Assembly assembly,
FhirJsonPocoSerializerSettings serializerSettings,
FhirJsonPocoDeserializerSettings deserializerSettings) : this(ModelInspector.ForAssembly(assembly), serializerSettings, deserializerSettings)
Assembly assembly, FhirJsonPocoSerializerSettings serializerSettings, FhirJsonPocoDeserializerSettings deserializerSettings) : this(ModelInspector.ForAssembly(assembly), serializerSettings, deserializerSettings)
{
// Nothing
}

public FhirJsonConverterFactory(
ModelInspector inspector,
FhirJsonPocoSerializerSettings serializerSettings,
FhirJsonPocoDeserializerSettings deserializerSettings)
internal void SetEnforcedErrors(IEnumerable<string> toEnforce)
{
_inspector = inspector;
_serializerSettings = serializerSettings;
_deserializerSettings = deserializerSettings;
Engine ??= createDefaultEngine();
Engine.IgnoreFilter = Engine.IgnoreFilter.And(toEnforce.ToPredicate().Negate());
}

internal void SetIgnoredErrors(IEnumerable<string> toIgnore)
{
Engine ??= createDefaultEngine();
Engine.IgnoreFilter = Engine.IgnoreFilter.Or(toIgnore.ToPredicate());
}

internal void SetMode(DeserializerModes mode)
{
Engine = mode switch
{
DeserializerModes.Recoverable => (PocoSerializationEngine)FhirSerializationEngineFactory.Recoverable(inspector, serializerSettings, deserializerSettings),
DeserializerModes.BackwardsCompatible => (PocoSerializationEngine)FhirSerializationEngineFactory.BackwardsCompatible(inspector, serializerSettings, deserializerSettings),
DeserializerModes.Ostrich => (PocoSerializationEngine)FhirSerializationEngineFactory.Ostrich(inspector, serializerSettings, deserializerSettings),
_ => createDefaultEngine()
};
}

public override bool CanConvert(Type typeToConvert) => typeof(Base).IsAssignableFrom(typeToConvert);

public override JsonConverter? CreateConverter(Type typeToConvert, JsonSerializerOptions options)
{
return (JsonConverter)Activator.CreateInstance(
typeof(FhirJsonConverter<>).MakeGenericType(typeToConvert),
new object[] { _inspector, _serializerSettings, _deserializerSettings })!;
Engine ??= createDefaultEngine();
return (JsonConverter?)Activator.CreateInstance(
typeof(FhirJsonConverter<>).MakeGenericType(typeToConvert), BindingFlags.NonPublic | BindingFlags.Instance, null,
[ Engine ], null, null);
}
}


/// <summary>
/// FHIR Resource and datatype converter for FHIR deserialization.
/// </summary>
public class FhirJsonConverter<F> : JsonConverter<F> where F : Base
public class FhirJsonConverter<F> : JsonConverter<F>
where F : Base
{
// internal for testing purposes
private readonly PocoSerializationEngine _engine;

private FhirJsonConverter(IFhirSerializationEngine engine)
{
this._engine = (PocoSerializationEngine)engine;
}

/// <summary>
/// Constructs a <see cref="JsonConverter{T}"/> that (de)serializes FHIR json for the
/// POCOs in a given assembly.
/// </summary>
/// <param name="assembly">The assembly containing classes to be used for deserialization.</param>
/// <param name="serializerSettings">The optional features used during serialization.</param>
/// <param name="deserializerSettings">The optional features used during deserialization.</param>
[Obsolete("Using this directly is not recommended. Instead, try creating a converter using the .ForFhir static method of the JsonSerializerOptions class")]
public FhirJsonConverter(
Assembly assembly,
FhirJsonPocoSerializerSettings serializerSettings,
FhirJsonPocoDeserializerSettings deserializerSettings)
Assembly assembly): this(ModelInspector.ForAssembly(assembly))
{
var inspector = ModelInspector.ForAssembly(assembly);

_deserializer = new BaseFhirJsonPocoDeserializer(assembly, deserializerSettings);
_serializer = new BaseFhirJsonPocoSerializer(inspector.FhirRelease, serializerSettings);
// nothing
}

/// <summary>
/// Constructs a <see cref="JsonConverter{T}"/> that (de)serializes FHIR json for the
/// POCOs in a given assembly.
/// </summary>
/// <param name="inspector">The <see cref="ModelInspector" /> containing classes to be used for deserialization.</param>
/// <param name="serializerSettings">The optional features used during serialization.</param>
/// <param name="deserializerSettings">The optional features used during deserialization.</param>
[Obsolete("Using this directly is not recommended. Instead, try creating a converter using the .ForFhir static method of the JsonSerializerOptions class")]
public FhirJsonConverter(
ModelInspector inspector,
FhirJsonPocoSerializerSettings serializerSettings,
FhirJsonPocoDeserializerSettings deserializerSettings)
ModelInspector inspector) : this(FhirSerializationEngineFactory.Strict(inspector))
{
_deserializer = new BaseFhirJsonPocoDeserializer(inspector, deserializerSettings);
_serializer = new BaseFhirJsonPocoSerializer(inspector.FhirRelease, serializerSettings);
}

/// <summary>
Expand All @@ -105,20 +120,16 @@ public FhirJsonConverter(
/// <remarks>Since the standard serializer/deserializer will allow you to override its behaviour to produce
/// custom behaviour, this constructor will allow the developer to use such custom serializers/deserializers instead
/// of the defaults.</remarks>
public FhirJsonConverter(BaseFhirJsonPocoDeserializer deserializer, BaseFhirJsonPocoSerializer serializer)
[Obsolete("Using this directly is not recommended. Instead, try creating a converter using the .ForFhir static method of the JsonSerializerOptions class")]
public FhirJsonConverter(BaseFhirJsonPocoDeserializer deserializer, BaseFhirJsonPocoSerializer serializer) : this(FhirSerializationEngineFactory.WithCustomJsonSerializers(deserializer, serializer))
{
_deserializer = deserializer;
_serializer = serializer;
}

/// <summary>
/// Determines whether the specified type can be converted.
/// </summary>
public override bool CanConvert(Type objectType) => typeof(F) == objectType;

private readonly BaseFhirJsonPocoDeserializer _deserializer;
private readonly BaseFhirJsonPocoSerializer _serializer;

/// <summary>
/// The filter used to serialize a summary of the resource.
/// </summary>
Expand All @@ -129,7 +140,7 @@ public FhirJsonConverter(BaseFhirJsonPocoDeserializer deserializer, BaseFhirJson
/// </summary>
public override void Write(Utf8JsonWriter writer, F poco, JsonSerializerOptions options)
{
_serializer.Serialize(poco, writer);
_engine.SerializeToJsonWriter(poco, writer);
}

/// <summary>
Expand All @@ -138,8 +149,8 @@ public override void Write(Utf8JsonWriter writer, F poco, JsonSerializerOptions
public override F Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
{
return typeof(Resource).IsAssignableFrom(typeToConvert)
? (F)(Base)_deserializer.DeserializeResource(ref reader)
: (F)_deserializer.DeserializeObject(typeToConvert, ref reader);
? (F)(Base)_engine.DeserializeFromJson(ref reader)
: (F)_engine.DeserializeObjectFromJson(typeToConvert, ref reader);
}
}
}
Expand Down
50 changes: 26 additions & 24 deletions src/Hl7.Fhir.Base/Serialization/FhirJsonException.cs
Original file line number Diff line number Diff line change
Expand Up @@ -123,40 +123,42 @@ public class FhirJsonException : ExtendedCodedException
internal static FhirJsonException PRIMITIVE_ARRAYS_ONLY_NULL(ref Utf8JsonReader reader, string instancePath) => Initialize(ref reader, instancePath, PRIMITIVE_ARRAYS_ONLY_NULL_CODE, "Arrays need to have at least one non-null element.", OO_Sev.Warning, OO_Typ.Structure);

/// <summary>
/// Whether this issue leads to dataloss or not. Recoverable issues mean that all data present in the parsed data could be retrieved and
/// List of issues which do NOT lead to data loss. Recoverable issues mean that all data present in the parsed data could be retrieved and
/// captured in the POCO model, even if the syntax or the data was not fully FHIR compliant.
/// </summary>
#pragma warning disable CS0618 // Type or member is obsolete
internal static bool IsRecoverableIssue(FhirJsonException e) =>
e.ErrorCode is EXPECTED_PRIMITIVE_NOT_NULL_CODE or
INCORRECT_BASE64_DATA_CODE or
STRING_ISNOTAN_INSTANT_CODE or
NUMBER_CANNOT_BE_PARSED_CODE or
UNEXPECTED_JSON_TOKEN_CODE or
LONG_CANNOT_BE_PARSED_CODE or
LONG_INCORRECT_FORMAT_CODE or
EXPECTED_START_OF_ARRAY_CODE or
USE_OF_UNDERSCORE_ILLEGAL_CODE or
RESOURCETYPE_UNEXPECTED_CODE or
OBJECTS_CANNOT_BE_EMPTY_CODE or
ARRAYS_CANNOT_BE_EMPTY_CODE or
PRIMITIVE_ARRAYS_INCOMPAT_SIZE_CODE or
PRIMITIVE_ARRAYS_ONLY_NULL_CODE or
PROPERTY_MAY_NOT_BE_EMPTY_CODE or
DUPLICATE_ARRAY_CODE;
internal static string[] RecoverableIssues =
[
EXPECTED_PRIMITIVE_NOT_NULL_CODE,
INCORRECT_BASE64_DATA_CODE,
STRING_ISNOTAN_INSTANT_CODE,
NUMBER_CANNOT_BE_PARSED_CODE,
UNEXPECTED_JSON_TOKEN_CODE,
LONG_CANNOT_BE_PARSED_CODE,
LONG_INCORRECT_FORMAT_CODE,
EXPECTED_START_OF_ARRAY_CODE,
USE_OF_UNDERSCORE_ILLEGAL_CODE,
RESOURCETYPE_UNEXPECTED_CODE,
OBJECTS_CANNOT_BE_EMPTY_CODE,
ARRAYS_CANNOT_BE_EMPTY_CODE,
PRIMITIVE_ARRAYS_INCOMPAT_SIZE_CODE,
PRIMITIVE_ARRAYS_ONLY_NULL_CODE,
PROPERTY_MAY_NOT_BE_EMPTY_CODE,
DUPLICATE_ARRAY_CODE
];
#pragma warning restore CS0618 // Type or member is obsolete

/// <summary>
/// An issue is allowable for backwards compatibility if it could be caused because an older parser encounters data coming from a newer
/// FHIR release. This means allowing unknown elements, codes and types in a choice element. Note that the POCO model cannot capture
/// these newer elements and data, so this means data loss may occur.
/// </summary>
internal static bool AllowedForBackwardsCompatibility(CodedException e) =>
e.ErrorCode is CodedValidationException.INVALID_CODED_VALUE_CODE or
//EXPECTED_PRIMITIVE_NOT_OBJECT_CODE or
//EXPECTED_PRIMITIVE_NOT_ARRAY_CODE or
CHOICE_ELEMENT_HAS_UNKOWN_TYPE_CODE or
UNKNOWN_PROPERTY_FOUND_CODE;
internal static string[] BackwardsCompatibilityAllowedIssues =
[
CodedValidationException.INVALID_CODED_VALUE_CODE,
CHOICE_ELEMENT_HAS_UNKOWN_TYPE_CODE,
UNKNOWN_PROPERTY_FOUND_CODE
];

public FhirJsonException(string code, string message)
: base(code, message, null, null, null, OO_Sev.Error, OO_Typ.Unknown)
Expand Down
43 changes: 24 additions & 19 deletions src/Hl7.Fhir.Base/Serialization/FhirXmlException.cs
Original file line number Diff line number Diff line change
Expand Up @@ -88,33 +88,38 @@ internal static FhirXmlException ELEMENT_HAS_NO_VALUE_OR_CHILDREN(string instanc
internal static FhirXmlException ENCOUNTERED_DTD_REFERENCES(XmlReader reader, string instancePath) => Initialize(reader, instancePath, ENCOUNTERED_DTD_REFERENCES_CODE, "There SHALL be no DTD references in FHIR resources (because of the XXE security exploit)", OO_Sev.Warning, OO_Typ.Structure);

/// <summary>
/// Whether this issue leads to dataloss or not. Recoverable issues mean that all data present in the parsed data could be retrieved and
/// List of issues which do NOT lead to data loss. Recoverable issues mean that all data present in the parsed data could be retrieved and
/// captured in the POCO model, even if the syntax or the data was not fully FHIR compliant.
/// </summary>
internal static bool IsRecoverableIssue(FhirXmlException e) =>
e.ErrorCode is EMPTY_ELEMENT_NAMESPACE_CODE or
INCORRECT_ELEMENT_NAMESPACE_CODE or
INCORRECT_XHTML_NAMESPACE_CODE or
INCORRECT_ATTRIBUTE_NAMESPACE_CODE or
INCORRECT_BASE64_DATA_CODE or
VALUE_IS_NOT_OF_EXPECTED_TYPE_CODE or
ELEMENT_OUT_OF_ORDER_CODE or
ELEMENT_NOT_IN_SEQUENCE_CODE or
ATTRIBUTE_HAS_EMPTY_VALUE_CODE or
ELEMENT_HAS_NO_VALUE_OR_CHILDREN_CODE or
SCHEMALOCATION_DISALLOWED_CODE or
ENCOUNTERED_DTD_REFERENCES_CODE;
internal static string[] RecoverableIssues =
[
EMPTY_ELEMENT_NAMESPACE_CODE,
INCORRECT_ELEMENT_NAMESPACE_CODE,
INCORRECT_XHTML_NAMESPACE_CODE,
INCORRECT_ATTRIBUTE_NAMESPACE_CODE,
INCORRECT_BASE64_DATA_CODE,
VALUE_IS_NOT_OF_EXPECTED_TYPE_CODE,
ELEMENT_OUT_OF_ORDER_CODE,
ELEMENT_NOT_IN_SEQUENCE_CODE,
ATTRIBUTE_HAS_EMPTY_VALUE_CODE,
ELEMENT_HAS_NO_VALUE_OR_CHILDREN_CODE,
SCHEMALOCATION_DISALLOWED_CODE,
ENCOUNTERED_DTD_REFERENCES_CODE
];


/// <summary>
/// An issue is allowable for backwards compatibility if it could be caused because an older parser encounters data coming from a newer
/// FHIR release. This means allowing unknown elements, attributes, codes and types in a choice element. Note that the POCO model cannot capture
/// these newer elements and data, so this means data loss may occur.
/// </summary>
internal static bool AllowedForBackwardsCompatibility(CodedException e) =>
e.ErrorCode is CodedValidationException.INVALID_CODED_VALUE_CODE or
UNKNOWN_ELEMENT_CODE or
CHOICE_ELEMENT_HAS_UNKNOWN_TYPE_CODE or
UNKNOWN_ATTRIBUTE_CODE;
internal static string[] BackwardsCompatibilityAllowedIssues =
[
CodedValidationException.INVALID_CODED_VALUE_CODE,
UNKNOWN_ELEMENT_CODE,
CHOICE_ELEMENT_HAS_UNKNOWN_TYPE_CODE,
UNKNOWN_ATTRIBUTE_CODE
];

public FhirXmlException(string code, string message)
: base(code, message, null, null, null, OO_Sev.Error, OO_Typ.Unknown)
Expand Down
33 changes: 33 additions & 0 deletions src/Hl7.Fhir.Base/Serialization/FilterPredicateExtensions.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
#nullable enable

using Hl7.Fhir.Serialization;
using Hl7.Fhir.Utility;
using System;
using System.Collections.Generic;
using System.Linq;

namespace Hl7.Fhir.Serialization;

internal static class FilterPredicateExtensions
{
internal static Predicate<CodedException> IsRecoverableIssue =>
FhirXmlException.RecoverableIssues.Concat(FhirJsonException.RecoverableIssues).ToPredicate();

internal static Predicate<CodedException> IsBackwardsCompatibilityIssue =>
FhirXmlException.BackwardsCompatibilityAllowedIssues.Concat(FhirJsonException.BackwardsCompatibilityAllowedIssues).ToPredicate();

internal static Predicate<CodedException> ToPredicate(this IEnumerable<string> ignoreList) =>
ce => ignoreList.Contains(ce.ErrorCode);

internal static Predicate<CodedException> And(this Predicate<CodedException> a, Predicate<CodedException>? b) =>
b is not null ? ce => a(ce) && b(ce) : a;

internal static Predicate<CodedException> Or(this Predicate<CodedException> a, Predicate<CodedException>? b) =>
b is not null ? ce => a(ce) || b(ce) : a;

internal static Predicate<CodedException> Negate(this Predicate<CodedException> a) =>
ce => !a(ce);

}

#nullable restore
Loading