Merge pull request #2367 from FirelyTeam/bugfix/2273-serializer-doesn…

…t-report-missing-mandatory-elements

2273 Report missing mandatory elements

src/Hl7.Fhir.Base/Serialization/DataAnnotationDeserialzationValidator.cs

@@ -62,7 +62,33 @@ public void ValidateInstance(object? instance, in InstanceDeserializationContext
                 .SetPositionInfo(new PositionInfo((int)context.LineNumber, (int)context.LinePosition))
                 .SetLocation(context.Path);
 
-            reportedErrors = runAttributeValidation(instance, context.InstanceMapping.ValidationAttributes, validationContext);
+            if (instance is null)
+            {
+                reportedErrors = null;
+                return;
+            }
+
+            IEnumerable<CodedValidationException>? errors = null;
+
+            // Make sure we detect missing values - go over all members that have cardinality constraints
+            // and invoke those if there is no value (if there was a value, ValidateProperty will have been
+            // called on it while deserializing the member).
+            foreach (var propMapping in context.InstanceMapping.PropertyMappings)
+            {
+                var cardinality = propMapping.ValidationAttributes.OfType<CardinalityAttribute>().SingleOrDefault();
+                if (cardinality is not null && cardinality.Min > 0)
+                {
+                    // Note that some Value accessors (for Code<T>.Value for example) can throw, but there are
+                    // no Cardinality constraints on those, so we don't have to worry about that now.
+                    var propValue = propMapping.GetValue(instance);
+
+                    if (propValue is null || ReflectionHelper.IsRepeatingElement(propValue, out var list) && list.Count == 0)
+                        errors = add(errors, runAttributeValidation(propValue, new[] { cardinality }, validationContext));
+                }
+            }
+
+            // Validate the attributes on this instance itself
+            errors = add(errors, runAttributeValidation(instance, context.InstanceMapping.ValidationAttributes, validationContext));
 
             // Now, just like Validator.Validate, run the IValidatableObject if applicable
             if (instance is IValidatableObject ivo)
@@ -72,11 +98,21 @@ public void ValidateInstance(object? instance, in InstanceDeserializationContext
                 {
                     var codedErrors = extraErrors.OfType<CodedValidationResult>().Select(cvr => cvr.ValidationException);
                     if (codedErrors.Count() != extraErrors.Count)
-                        throw new InvalidOperationException($"Validation attributes should return a {nameof(CodedValidationResult)}.");
+                        throw new InvalidOperationException($"IValidatableObject.Validates should return one or more {nameof(CodedValidationResult)}.");
 
-                    reportedErrors = (reportedErrors is not null ? reportedErrors.Concat(codedErrors) : codedErrors).ToArray();
+                    errors = add(errors, codedErrors);
                 }
             }
+
+            reportedErrors = errors?.ToArray();
+            return;           
+        }
+
+        private IEnumerable<CodedValidationException>? add(IEnumerable<CodedValidationException>? errors, IEnumerable<CodedValidationException>? moreErrors)
+        {
+            return moreErrors is null ?
+                errors
+                : errors is not null ? errors.Concat(moreErrors) : moreErrors;
         }
 
         private CodedValidationException[]? runAttributeValidation(
@@ -86,22 +122,16 @@ public void ValidateInstance(object? instance, in InstanceDeserializationContext
         {
 
             // Avoid allocation of a list for every validation until we really have something to report.
-            List<CodedValidationException>? errors = null;
+            IEnumerable<CodedValidationException>? errors = null;
 
             foreach (var va in attributes)
             {
                 if (va.GetValidationResult(candidateValue, validationContext) is object vr)
                 {
                     if (vr is CodedValidationResult cvr)
-                        addError(cvr.ValidationException);
+                        errors = add(errors, new[] { cvr.ValidationException });
                     else
-                        throw new InvalidOperationException($"Validation attributes should return a {nameof(CodedValidationResult)}.");
-
-                    void addError(CodedValidationException e)
-                    {
-                        if (errors is null) errors = new();
-                        errors.Add(e);
-                    }
+                        throw new InvalidOperationException($"Validation attributes should return a {nameof(CodedValidationResult)}.");                  
                 }
             }
 

src/Hl7.Fhir.Support.Poco.Tests/NewPocoSerializers/FhirJsonDeserializationTests.cs

@@ -391,7 +391,7 @@ public void TestData(Type t, object testObject, JsonTokenType token, Action<obje
                     ERR.RESOURCETYPE_UNEXPECTED, ERR.UNKNOWN_PROPERTY_FOUND);
             yield return data<Extension>(new { value = "no type suffix" }, ERR.CHOICE_ELEMENT_HAS_NO_TYPE);
             yield return data<Extension>(new { valueUnknown = "incorrect type suffix" }, ERR.CHOICE_ELEMENT_HAS_UNKOWN_TYPE);
-            yield return data<Extension>(new { valueBoolean = true }, JsonTokenType.EndObject);
+            yield return data<Extension>(new { valueBoolean = true, url = "http://something.nl" }, JsonTokenType.EndObject);
             yield return data<Extension>(new { valueUnknown = "incorrect type suffix", unknown = "unknown" },
                     ERR.CHOICE_ELEMENT_HAS_UNKOWN_TYPE, ERR.UNKNOWN_PROPERTY_FOUND);
         }
@@ -442,7 +442,7 @@ static void checkAll(object parsed)
 
         public static IEnumerable<object?[]> TestValidatePrimitiveData()
         {
-            yield return data<Narrative>(new { div = "<div xmlns=\"http://www.w3.org/1999/xhtml\"><p>correct</p></div>" });
+            yield return data<Narrative>(new { div = "<div xmlns=\"http://www.w3.org/1999/xhtml\"><p>correct</p></div>", status = "additional" });
             yield return data<Narrative>(new { div = "this isn't xml" }, COVE.NARRATIVE_XML_IS_MALFORMED);
             yield return data<Narrative>(new { div = "<puinhoop />" }, COVE.NARRATIVE_XML_IS_INVALID);
 
@@ -787,6 +787,19 @@ public void CanParseMixedClass()
             mc2.HandledByTextJson.Should().Be("Hi!");
             mc2.FhirPatient?.Active.Should().Be(true);
         }
+
+        [TestMethod]
+        public void ReportsMissingMandatoryElements()
+        {
+            var (codesystem, errors) = deserializeComplex(typeof(TestCodeSystem), new
+            {
+                resourceType = "CodeSystem",
+                content = "example"
+            }, out _, new());
+
+            // should contain error that mandatory item "status" is missing.
+            errors.Should().ContainSingle(ce => ce.ErrorCode == "PVAL105");
+        }
     }
 }
 #nullable restore