Fixing duplicate errors in OperationOutcome. (#4017)

* Fixing duplicate entries in OperationOutcome. * Adding UTs for ResourceProfileValidator * Removing unnecessary namespaces.
microsoft · Aug 14, 2024 · a3b5eff · a3b5eff
1 parent 0f93927
commit a3b5eff
Show file tree

Hide file tree

Showing 2 changed files with 95 additions and 5 deletions.
diff --git a/src/Microsoft.Health.Fhir.Core/Features/Validation/ResourceProfileValidator.cs b/src/Microsoft.Health.Fhir.Core/Features/Validation/ResourceProfileValidator.cs
@@ -76,13 +76,14 @@ public override ValidationResult Validate(ValidationContext<ResourceElement> con
                             error);
                         failures.Add(validationFailure);
                     }
+
+                    failures.ForEach(x => context.AddFailure(x));
                 }
 
                 ValidationResult baseValidation = base.Validate(context);
                 failures.AddRange(baseValidation.Errors);
             }
 
-            failures.ForEach(x => context.AddFailure(x));
             return new ValidationResult(failures);
         }
     }

diff --git a/...ft.Health.Fhir.Shared.Core.UnitTests/Features/Validation/ResourceProfileValidatorTests.cs b/...ft.Health.Fhir.Shared.Core.UnitTests/Features/Validation/ResourceProfileValidatorTests.cs
@@ -5,8 +5,11 @@
 
 using System;
 using System.Collections.Generic;
+using System.Reflection;
+using FluentValidation;
 using FluentValidation.Results;
 using Hl7.Fhir.ElementModel;
+using Hl7.Fhir.Support;
 using Microsoft.Extensions.Primitives;
 using Microsoft.Health.Core.Features.Context;
 using Microsoft.Health.Fhir.Core.Features.Context;
@@ -25,6 +28,7 @@ public class ResourceProfileValidatorTests
 {
     private readonly ResourceProfileValidator _validator;
     private readonly IProfileValidator _profileValidator;
+    private readonly IModelAttributeValidator _modelAttributeValidator;
     private readonly IFhirRequestContext _fhirRequestContext;
     private readonly OperationOutcomeIssue _operationOutcomeWarningIssue = new(OperationOutcomeConstants.IssueSeverity.Warning, OperationOutcomeConstants.IssueType.Invalid, detailsText: "Warning text.");
     private readonly Dictionary<string, StringValues> _strictHeader = new Dictionary<string, StringValues> { { "Prefer", "handling=strict" } };
@@ -37,11 +41,12 @@ public ResourceProfileValidatorTests()
         requestContextAccessor.RequestContext.Returns(_fhirRequestContext);
 
         _profileValidator = Substitute.For<IProfileValidator>();
+        _modelAttributeValidator = Substitute.For<IModelAttributeValidator>();
         _validator = new ResourceProfileValidator(
-        Substitute.For<IModelAttributeValidator>(),
-        _profileValidator,
-        requestContextAccessor,
-        true);
+            _modelAttributeValidator,
+            _profileValidator,
+            requestContextAccessor,
+            true);
     }
 
     [Fact]
@@ -100,9 +105,93 @@ public void GivenAResource_WhenValidatingWithAnErrorAndStrictHandling_ThenTheVal
         Assert.False(result.IsValid);
     }
 
+    [Fact]
+    public void GivenAResource_WhenValidatingInvalidResource_ThenTheValidationResultHasNoDuplicateErrors()
+    {
+        ResourceElement resource = Samples.GetJsonSample("ObservationWithNoCode");
+
+        var message = "Element with minimum cardinality 1 cannot be null. At Observation.Code.";
+        var memberName = "Code";
+        var propertyName = $"Observation.{memberName}";
+        _modelAttributeValidator.TryValidate(
+            Arg.Any<ResourceElement>(),
+            Arg.Do<ICollection<System.ComponentModel.DataAnnotations.ValidationResult>>(
+                x =>
+                {
+                    var result = new System.ComponentModel.DataAnnotations.ValidationResult(message, new string[] { memberName });
+                    x.Add(result);
+                }),
+            Arg.Any<bool>())
+            .Returns(false);
+
+        ValidationContext<ResourceElement> context = new ValidationContext<ResourceElement>(resource);
+        ValidationResult result = _validator.Validate(context);
+        Assert.False(result.IsValid);
+        Assert.Single(result.Errors);
+        Assert.Equal(Severity.Error, result.Errors[0].Severity);
+        Assert.Equal(message, result.Errors[0].ErrorMessage);
+        Assert.Equal(propertyName, result.Errors[0].PropertyName);
+
+        var failures = GetFailures(context);
+        Assert.Single(failures);
+        Assert.Equal(Severity.Error, failures[0].Severity);
+        Assert.Equal(message, failures[0].ErrorMessage);
+        Assert.Equal(propertyName, failures[0].PropertyName);
+    }
+
+    [Fact]
+    public void GivenAResource_WhenValidatingInvalidResource_ThenTheValidationResultHasProfileAndContentErrors()
+    {
+        ResourceElement resource = Samples.GetJsonSample("ObservationWithNoCode");
+
+        var message = "Element with minimum cardinality 1 cannot be null. At Observation.Code.";
+        var memberName = "Code";
+        var propertyName = $"Observation.{memberName}";
+        _modelAttributeValidator.TryValidate(
+            Arg.Any<ResourceElement>(),
+            Arg.Do<ICollection<System.ComponentModel.DataAnnotations.ValidationResult>>(
+                x =>
+                {
+                    var result = new System.ComponentModel.DataAnnotations.ValidationResult(message, new string[] { memberName });
+                    x.Add(result);
+                }),
+            Arg.Any<bool>())
+            .Returns(false);
+
+        var detailedMessage = "Error text.";
+        _profileValidator.TryValidate(Arg.Any<ITypedElement>(), Arg.Any<string>())
+            .Returns(new[] { new OperationOutcomeIssue(OperationOutcomeConstants.IssueSeverity.Error, OperationOutcomeConstants.IssueType.Invalid, detailsText: detailedMessage) });
+
+        ValidationContext<ResourceElement> context = new ValidationContext<ResourceElement>(resource);
+        ValidationResult result = _validator.Validate(context);
+        Assert.False(result.IsValid);
+        Assert.Equal(2, result.Errors.Count);
+        Assert.Contains(
+            result.Errors,
+            e => e.Severity == Severity.Error && e.ErrorMessage == message && e.PropertyName == propertyName);
+        Assert.Contains(
+            result.Errors,
+            e => e.Severity == Severity.Error && e.ErrorMessage == detailedMessage);
+
+        var failures = GetFailures(context);
+        Assert.Equal(2, failures.Count);
+        Assert.Contains(
+            failures,
+            e => e.Severity == Severity.Error && e.ErrorMessage == message && e.PropertyName == propertyName);
+        Assert.Contains(
+            failures,
+            e => e.Severity == Severity.Error && e.ErrorMessage == detailedMessage);
+    }
+
     public static IEnumerable<object[]> ErrorIssues()
     {
         yield return new object[] { new OperationOutcomeIssue(OperationOutcomeConstants.IssueSeverity.Error, OperationOutcomeConstants.IssueType.Invalid, detailsText: "Error text.") };
         yield return new object[] { new OperationOutcomeIssue(OperationOutcomeConstants.IssueSeverity.Fatal, OperationOutcomeConstants.IssueType.Invalid, detailsText: "Fatal text.") };
     }
+
+    private static IList<ValidationFailure> GetFailures(ValidationContext<ResourceElement> context)
+    {
+        var failures = context?.GetType().GetProperty("Failures", BindingFlags.Instance | BindingFlags.NonPublic)?.GetValue(context);
+        return failures != null ? new List<ValidationFailure>((IList<ValidationFailure>)failures) : new List<ValidationFailure>();
+    }
 }