diff --git a/src/Firely.Fhir.Validation.Compilation.Shared/SchemaBuilder.cs b/src/Firely.Fhir.Validation.Compilation.Shared/SchemaBuilder.cs index fb1f4f33..f2bfebf1 100644 --- a/src/Firely.Fhir.Validation.Compilation.Shared/SchemaBuilder.cs +++ b/src/Firely.Fhir.Validation.Compilation.Shared/SchemaBuilder.cs @@ -263,10 +263,12 @@ internal List ConvertElement(ElementDefinitionNavigator nav, Subsche // Type we're working with was pulled into definition, and we're not in a slice // so we need to validate any invariant rules we might find as well, but we shouldn't pull them twice. // We can skip backbone elements though, as there's nothing to copy. - if (schemaMembers.OfType().Any() && !schemaMembers.OfType().Any() - && string.IsNullOrEmpty(nav.Current.SliceName) && !nav.Current.IsBackboneElement()) + // Invariants that are already on this element (declared in the profile, or copied into the snapshot) are + // not validated a second time, but their presence must not suppress the root invariants of the type + // (https://github.com/FirelyTeam/firely-validator-api/issues/567). + if (schemaMembers.OfType().Any() && string.IsNullOrEmpty(nav.Current.SliceName) && !nav.Current.IsBackboneElement()) { - schemaMembers.Add(new BaseTypeInvariantConstraintsValidator()); + schemaMembers.Add(new BaseTypeInvariantConstraintsValidator(schemaMembers.OfType().Select(x => x.Key))); } } } diff --git a/src/Firely.Fhir.Validation/Impl/BaseTypeInvariantConstraintsValidator.cs b/src/Firely.Fhir.Validation/Impl/BaseTypeInvariantConstraintsValidator.cs index 660ae985..b7055130 100644 --- a/src/Firely.Fhir.Validation/Impl/BaseTypeInvariantConstraintsValidator.cs +++ b/src/Firely.Fhir.Validation/Impl/BaseTypeInvariantConstraintsValidator.cs @@ -26,6 +26,25 @@ namespace Firely.Fhir.Validation; #endif public class BaseTypeInvariantConstraintsValidator : IValidatable { + private readonly string[] _alreadyValidatedKeys; + + /// + /// Creates a validator that runs all invariants found on the root of the element's type. + /// + public BaseTypeInvariantConstraintsValidator() : this([]) + { + } + + /// + /// Creates a validator that runs the invariants found on the root of the element's type, + /// except for those whose key is already validated on the element itself. + /// + /// Keys of the invariants that are already part of the element's schema. + public BaseTypeInvariantConstraintsValidator(System.Collections.Generic.IEnumerable alreadyValidatedKeys) + { + _alreadyValidatedKeys = alreadyValidatedKeys.ToArray(); + } + /// /// Validate input against the expected context and invariants. /// @@ -38,7 +57,7 @@ public ResultReport Validate(PocoNode input, ValidationSettings vc, ValidationSt var typeProfile = vc.TypeNameMapper.MapTypeName(input.Poco.TypeName); return FhirSchemaGroupAnalyzer.FetchSchema(vc.ElementSchemaResolver, state, typeProfile, input.GetLocation()) switch { - (var schema, null, _) => ResultReport.Combine(schema!.Members.Where(vc.Filter).OfType().Select(x => x.ValidateOne(input, vc, state)).ToList()), + (var schema, null, _) => ResultReport.Combine(schema!.Members.Where(vc.Filter).OfType().Where(x => !_alreadyValidatedKeys.Contains(x.Key)).Select(x => x.ValidateOne(input, vc, state)).ToList()), (_, var error, _) => error }; } diff --git a/src/Firely.Fhir.Validation/PublicAPI.Unshipped.txt b/src/Firely.Fhir.Validation/PublicAPI.Unshipped.txt index 74c26f28..90131119 100644 --- a/src/Firely.Fhir.Validation/PublicAPI.Unshipped.txt +++ b/src/Firely.Fhir.Validation/PublicAPI.Unshipped.txt @@ -34,6 +34,7 @@ Firely.Fhir.Validation.BaseType.ToJson() -> Newtonsoft.Json.Linq.JToken! Firely.Fhir.Validation.BaseType.Type.get -> string! Firely.Fhir.Validation.BaseTypeInvariantConstraintsValidator Firely.Fhir.Validation.BaseTypeInvariantConstraintsValidator.BaseTypeInvariantConstraintsValidator() -> void +Firely.Fhir.Validation.BaseTypeInvariantConstraintsValidator.BaseTypeInvariantConstraintsValidator(System.Collections.Generic.IEnumerable! alreadyValidatedKeys) -> void Firely.Fhir.Validation.BaseTypeInvariantConstraintsValidator.ToJson() -> Newtonsoft.Json.Linq.JToken! Firely.Fhir.Validation.BaseTypeInvariantConstraintsValidator.Validate(Hl7.Fhir.Model.PocoNode! input, Firely.Fhir.Validation.ValidationSettings! vc, Firely.Fhir.Validation.ValidationState! state) -> Firely.Fhir.Validation.ResultReport! Firely.Fhir.Validation.BasicValidator diff --git a/test/Firely.Fhir.Validation.Compilation.Tests.Shared/SimpleValidationsTests.cs b/test/Firely.Fhir.Validation.Compilation.Tests.Shared/SimpleValidationsTests.cs index 7456e782..29fc11c7 100644 --- a/test/Firely.Fhir.Validation.Compilation.Tests.Shared/SimpleValidationsTests.cs +++ b/test/Firely.Fhir.Validation.Compilation.Tests.Shared/SimpleValidationsTests.cs @@ -270,6 +270,27 @@ public void ValidateProfiledRunsInvariants(string url) } #endif + [Theory] + [InlineData(TestProfileArtifactSource.PROFILEDPERIODCHILDREN)] + [InlineData(TestProfileArtifactSource.PROFILEDPERIODCHILDRENWITHINVARIANT)] + public void ValidateRootInvariantsOfDatatypeWhoseChildrenAreProfiled(string url) + { + // Coverage.period is not a choice element and not sliced, but the profile walks into it. + // The root invariants of Period (per-1) must still be evaluated (issue 567). + // Other required elements are deliberately left out: only the presence of per-1 is asserted. + var coverage = new Coverage() + { + Meta = new Meta { Profile = [ url ] }, + Period = new Period(new("2017-06-02"), new("2017-06-01")) + }; + + var schema = _fixture.SchemaResolver.GetSchema(url)!; + var result = schema.Validate(coverage.ToTypedElement(), _fixture.NewValidationSettings()); + var oo = result.ToOperationOutcome(); + oo.Success.Should().Be(false); + oo.Issue.Exists(i => i.Details?.Text != null && i.Details.Text.Contains("per-1")).Should().BeTrue($"the Period root invariant per-1 must be reported for {url}"); + } + [Fact] public void ValidateUriStringsInExtension() { diff --git a/test/Firely.Fhir.Validation.Compilation.Tests.Shared/TestProfileArtifactSource.cs b/test/Firely.Fhir.Validation.Compilation.Tests.Shared/TestProfileArtifactSource.cs index c1bb9766..661c8493 100644 --- a/test/Firely.Fhir.Validation.Compilation.Tests.Shared/TestProfileArtifactSource.cs +++ b/test/Firely.Fhir.Validation.Compilation.Tests.Shared/TestProfileArtifactSource.cs @@ -49,6 +49,9 @@ internal class TestProfileArtifactSource : IResourceResolver public const string PROFILEDEXTENSIONTYPEWITHCHILDREN = "http://validationtest.org/fhir/StructureDefinition/ExtensionValueXChildren"; public const string PROFILEDEXTENSIONTYPEWITHSLICE = "http://validationtest.org/fhir/StructureDefinition/ExtensionValuePeriodSlice"; + public const string PROFILEDPERIODCHILDREN = "http://validationtest.org/fhir/StructureDefinition/CoveragePeriodChildren"; + public const string PROFILEDPERIODCHILDRENWITHINVARIANT = "http://validationtest.org/fhir/StructureDefinition/CoveragePeriodChildrenWithInvariant"; + public const string PROFILEDINVARIANTRESOLVE = "http://validationtest.org/fhir/StructureDefinition/ObservationSubjectPatientActive"; public const string EMPTYSNAPSHOTUNKNOWNBASE = "http://validationtest.org/fhir/StructureDefinition/EmptySnapshotDueToUnknownBaseProfile"; @@ -95,6 +98,8 @@ internal class TestProfileArtifactSource : IResourceResolver buildExtensionValueChildConstraints(PROFILEDEXTENSIONTYPEWITHCHILDREN, "value[x]"), buildExtensionValueChildConstraints(PROFILEDEXTENSIONTYPEWITHSLICE, "valuePeriod"), buildExtensionValueChildConstraints(), + buildCoveragePeriodChildren(PROFILEDPERIODCHILDREN, withOwnInvariant: false), + buildCoveragePeriodChildren(PROFILEDPERIODCHILDRENWITHINVARIANT, withOwnInvariant: true), buildEmptySnapshotWithUnknownBaseProfile(), buildSliceWithRepeatingChildrenTestcase(), @@ -129,6 +134,47 @@ private static StructureDefinition buildExtensionValueChildConstraints() return result; } + /// + /// A profile on Coverage that walks into the (non-choice) element Coverage.period by constraining + /// its child start, optionally also adding an invariant on Coverage.period itself. The root + /// invariants of the Period datatype (per-1) must still be run. + /// See: https://github.com/FirelyTeam/firely-validator-api/issues/567 + /// + private static StructureDefinition buildCoveragePeriodChildren(string uri, bool withOwnInvariant) + { + var result = createTestSD( + uri, + "Coverage that makes period.start mandatory", + "Coverage that makes period.start mandatory", + FHIRAllTypes.Coverage + ); + + var period = new ElementDefinition("Coverage.period"); + if (withOwnInvariant) + period.Constraint = + [ + new() + { +#if STU3 + Severity = ElementDefinition.ConstraintSeverity.Error, +#else + Severity = ConstraintSeverity.Error, +#endif + Key = "cov-test", + Expression = "start.exists()", + Human = "Test invariant" + } + ]; + + result.Differential!.Element = + [ + period, + new("Coverage.period.start") { Min = 1 } + ]; + + return result; + } + private static StructureDefinition buildExtensionValueChildConstraints(string uri, string property) { var result = createTestSD(