From f6c0e2639b7b1af060df61c22ab61a55fb5d8cd2 Mon Sep 17 00:00:00 2001 From: Brian Postlethwaite Date: Thu, 11 May 2023 16:40:01 -0500 Subject: [PATCH 1/2] #2454 Handle the parsing of json arrays to handle duplicate array values (both orders and extension/value) --- .../BaseFhirJsonPocoDeserializer.cs | 42 ++++++++++++++----- ...SerializationExcexptionHandlersJsonPoco.cs | 41 ++++++++++++++++++ 2 files changed, 73 insertions(+), 10 deletions(-) diff --git a/src/Hl7.Fhir.Base/Serialization/BaseFhirJsonPocoDeserializer.cs b/src/Hl7.Fhir.Base/Serialization/BaseFhirJsonPocoDeserializer.cs index c13007f040..5bcc307b7a 100644 --- a/src/Hl7.Fhir.Base/Serialization/BaseFhirJsonPocoDeserializer.cs +++ b/src/Hl7.Fhir.Base/Serialization/BaseFhirJsonPocoDeserializer.cs @@ -247,7 +247,7 @@ private void deserializeObjectInto( reader.Read(); var empty = true; - var delayedValidations = new DelayedValidations(); + var delayedValidations = new ObjectParsingState(); var oldErrorCount = state.Errors.Count; var (line, pos) = reader.GetLocation(); @@ -297,7 +297,7 @@ private void deserializeObjectInto( // postponed until after all properties have been seen (e.g. Instance and Property validations for // primitive properties, since they may be composed from two properties `name` and `_name` in json // and should only be validated when both have been processed, even if megabytes apart in the json file). - delayedValidations.Run(); + delayedValidations.RunDelayedValidation(); // read past object, unless this is the last EndObject in the top-level Deserialize call if (!stayOnLastToken) reader.Read(); @@ -337,7 +337,7 @@ private void deserializePropertyValueInto( PropertyMapping propertyMapping, ClassMapping propertyValueMapping, ref Utf8JsonReader reader, - DelayedValidations delayedValidations, + ObjectParsingState delayedValidations, FhirJsonPocoDeserializerState state ) { @@ -385,7 +385,7 @@ FhirJsonPocoDeserializerState state // chance to encounter both the `name` and `_name` property. if (delayedValidations is not null && propertyValueMapping.IsFhirPrimitive) { - delayedValidations.Schedule( + delayedValidations.ScheduleDelayedValidation( propertyMapping.Name + PROPERTY_VALIDATION_KEY_SUFFIX, () => PocoDeserializationHelper.RunPropertyValidation(ref result, Settings.Validator!, deserializationContext, state.Errors)); } @@ -454,11 +454,25 @@ FhirJsonPocoDeserializerState state return listInstance; } - internal class DelayedValidations + internal class ObjectParsingState { private readonly Dictionary _validations = new(); + private readonly Dictionary _parsedPropValue = new(); - public void Schedule(string key, Action validation) + public int ParsingPropertyValue(string memberName) + { + if (_parsedPropValue.ContainsKey(memberName)) + return _parsedPropValue[memberName]; + _parsedPropValue.Add(memberName, 0); + return 0; + } + + public void ParsedPropertyValue(string memberName, int count) + { + _parsedPropValue[memberName] = count; + } + + public void ScheduleDelayedValidation(string key, Action validation) { // Add or overwrite the entry for the given key. if (_validations.ContainsKey(key)) _validations.Remove(key); @@ -466,7 +480,7 @@ public void Schedule(string key, Action validation) } //public CodedValidationException[] Run() => _validations.Values.SelectMany(delayed => delayed()).ToArray(); - public void Run() + public void RunDelayedValidation() { foreach (var validation in _validations.Values) validation(); } @@ -482,7 +496,7 @@ public void Run() ClassMapping propertyValueMapping, Type? fhirType, ref Utf8JsonReader reader, - DelayedValidations delayedValidations, + ObjectParsingState delayedValidations, FhirJsonPocoDeserializerState state ) { @@ -510,6 +524,12 @@ FhirJsonPocoDeserializerState state // to simply create a list by Adding(). Not the fastest approach :-( int elementIndex = 0; bool? onlyNulls = null; + elementIndex = delayedValidations.ParsingPropertyValue(propertyName); + if (elementIndex > 0) + { + state.Path.IncrementIndex(elementIndex); + state.Errors.Add(ERR.DUPLICATE_ARRAY(ref reader, state.Path.GetInstancePath())); + } while (reader.TokenType != JsonTokenType.EndArray) { @@ -528,6 +548,8 @@ FhirJsonPocoDeserializerState state existingList[elementIndex] ??= propertyValueMapping.Factory(); onlyNulls = false; _ = DeserializeFhirPrimitive((PrimitiveType)existingList[elementIndex]!, propertyName, propertyValueMapping, fhirType, ref reader, delayedValidations, state); + + delayedValidations.ParsedPropertyValue(propertyName, existingList.Count); } elementIndex += 1; @@ -560,7 +582,7 @@ internal PrimitiveType DeserializeFhirPrimitive( ClassMapping propertyValueMapping, Type? fhirType, ref Utf8JsonReader reader, - DelayedValidations? delayedValidations, + ObjectParsingState? delayedValidations, FhirJsonPocoDeserializerState state ) { @@ -618,7 +640,7 @@ FhirJsonPocoDeserializerState state if (delayedValidations is null) PocoDeserializationHelper.RunInstanceValidation(targetPrimitive, Settings.Validator, context, state.Errors); else - delayedValidations.Schedule( + delayedValidations.ScheduleDelayedValidation( propertyName.TrimStart('_') + INSTANCE_VALIDATION_KEY_SUFFIX, () => { context.PathStack.EnterElement(propertyName.TrimStart('_'), null, propertyValueMapping.IsPrimitive); diff --git a/src/Hl7.Fhir.Serialization.R4B.Tests/SerializationExcexptionHandlersJsonPoco.cs b/src/Hl7.Fhir.Serialization.R4B.Tests/SerializationExcexptionHandlersJsonPoco.cs index d0253368b1..6d1c315e98 100644 --- a/src/Hl7.Fhir.Serialization.R4B.Tests/SerializationExcexptionHandlersJsonPoco.cs +++ b/src/Hl7.Fhir.Serialization.R4B.Tests/SerializationExcexptionHandlersJsonPoco.cs @@ -745,6 +745,47 @@ public void JsonInvalidDuplicateArray() } } + [TestMethod] + public void JsonInvalidDuplicateArrayExtension() + { + // string containing a FHIR Patient with name John Doe, 17 Jan 1970, an invalid gender and an invalid date of birth + string rawData = """ + { + "resourceType": "Patient", + "id": "pat1", + "name": [ + { + "_given": [ null, { "id": "e1" }], + "given": [ "Jane", "John" ], + "given": [ "Rita", true ], + "_given": [ null, { "id": "e2" }] + } + ] + } + """; + // This feels like a breaking case and should be fatal if there are more than 1 name/_name + + try + { + var p = serializeResource(rawData); + DebugDump.OutputJson(p); + Assert.Fail("Expected to throw parsing"); + } + catch (DeserializationFailedException ex) + { + System.Diagnostics.Trace.WriteLine($"{ex.Message}"); + OperationOutcome oc = ex.ToOperationOutcome(); + DebugDump.OutputXml(oc); + DebugDump.OutputJson(ex.PartialResult); + + Assert.AreEqual("Patient.name[0].given[2]", oc.Issue[0].Expression.First()); + Assert.AreEqual(OperationOutcome.IssueSeverity.Warning, oc.Issue[0].Severity); + Assert.AreEqual("JSON128", oc.Issue[0].Details.Coding[0].Code); + + Assert.AreEqual(3, oc.Issue.Count); + } + } + [TestMethod] public void JsonInvalidElementIdArrayPath() { From 5f6b8cb824364c117aac37ee64fd0dd528b21e33 Mon Sep 17 00:00:00 2001 From: Marten Smits Date: Fri, 26 May 2023 13:35:54 +0200 Subject: [PATCH 2/2] processed review comments --- .../BaseFhirJsonPocoDeserializer.cs | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/src/Hl7.Fhir.Base/Serialization/BaseFhirJsonPocoDeserializer.cs b/src/Hl7.Fhir.Base/Serialization/BaseFhirJsonPocoDeserializer.cs index 5bcc307b7a..a798c82b38 100644 --- a/src/Hl7.Fhir.Base/Serialization/BaseFhirJsonPocoDeserializer.cs +++ b/src/Hl7.Fhir.Base/Serialization/BaseFhirJsonPocoDeserializer.cs @@ -247,7 +247,7 @@ private void deserializeObjectInto( reader.Read(); var empty = true; - var delayedValidations = new ObjectParsingState(); + var objectParsingState = new ObjectParsingState(); var oldErrorCount = state.Errors.Count; var (line, pos) = reader.GetLocation(); @@ -284,7 +284,7 @@ private void deserializeObjectInto( try { state.Path.EnterElement(propMapping!.Name, !propMapping.IsCollection ? null : 0, propMapping.IsPrimitive); - deserializePropertyValueInto(target, currentPropertyName, propMapping, propValueMapping!, ref reader, delayedValidations, state); + deserializePropertyValueInto(target, currentPropertyName, propMapping, propValueMapping!, ref reader, objectParsingState, state); } finally { @@ -297,7 +297,7 @@ private void deserializeObjectInto( // postponed until after all properties have been seen (e.g. Instance and Property validations for // primitive properties, since they may be composed from two properties `name` and `_name` in json // and should only be validated when both have been processed, even if megabytes apart in the json file). - delayedValidations.RunDelayedValidation(); + objectParsingState.RunDelayedValidation(); // read past object, unless this is the last EndObject in the top-level Deserialize call if (!stayOnLastToken) reader.Read(); @@ -459,7 +459,7 @@ internal class ObjectParsingState private readonly Dictionary _validations = new(); private readonly Dictionary _parsedPropValue = new(); - public int ParsingPropertyValue(string memberName) + public int GetPropertyIndex(string memberName) { if (_parsedPropValue.ContainsKey(memberName)) return _parsedPropValue[memberName]; @@ -467,7 +467,7 @@ public int ParsingPropertyValue(string memberName) return 0; } - public void ParsedPropertyValue(string memberName, int count) + public void SetPropertyIndex(string memberName, int count) { _parsedPropValue[memberName] = count; } @@ -524,7 +524,7 @@ FhirJsonPocoDeserializerState state // to simply create a list by Adding(). Not the fastest approach :-( int elementIndex = 0; bool? onlyNulls = null; - elementIndex = delayedValidations.ParsingPropertyValue(propertyName); + elementIndex = delayedValidations.GetPropertyIndex(propertyName); if (elementIndex > 0) { state.Path.IncrementIndex(elementIndex); @@ -548,8 +548,8 @@ FhirJsonPocoDeserializerState state existingList[elementIndex] ??= propertyValueMapping.Factory(); onlyNulls = false; _ = DeserializeFhirPrimitive((PrimitiveType)existingList[elementIndex]!, propertyName, propertyValueMapping, fhirType, ref reader, delayedValidations, state); - - delayedValidations.ParsedPropertyValue(propertyName, existingList.Count); + + delayedValidations.SetPropertyIndex(propertyName, existingList.Count); } elementIndex += 1; @@ -642,7 +642,8 @@ FhirJsonPocoDeserializerState state else delayedValidations.ScheduleDelayedValidation( propertyName.TrimStart('_') + INSTANCE_VALIDATION_KEY_SUFFIX, - () => { + () => + { context.PathStack.EnterElement(propertyName.TrimStart('_'), null, propertyValueMapping.IsPrimitive); PocoDeserializationHelper.RunInstanceValidation(targetPrimitive, Settings.Validator, context, state.Errors); context.PathStack.ExitElement();