From abdd1cf511905cdca008810c0c2b0e0713884d68 Mon Sep 17 00:00:00 2001 From: Marten Smits Date: Wed, 23 Aug 2023 16:44:18 +0200 Subject: [PATCH 1/3] Create duplicate invariant key validator --- .../StructureDefinitionBuilder.cs | 20 ++++ ...ir.Validation.Compilation.Shared.projitems | 1 + .../SchemaBuilders/StandardBuilders.cs | 3 +- .../StructureDefinitionValidator.cs | 70 ++++++++++++ ...idation.Compilation.Tests.Shared.projitems | 1 + ...tructureDefinitionSchemaValidationTests.cs | 105 ++++++++++++++++++ 6 files changed, 199 insertions(+), 1 deletion(-) create mode 100644 src/Firely.Fhir.Validation.Compilation.Shared/EnterpriseSchemaBuilders/StructureDefinitionBuilder.cs create mode 100644 src/Firely.Fhir.Validation/EnterpriseValidators/StructureDefinitionValidator.cs create mode 100644 test/Firely.Fhir.Validation.Compilation.Tests.Shared/StructureDefinitionSchemaValidationTests.cs diff --git a/src/Firely.Fhir.Validation.Compilation.Shared/EnterpriseSchemaBuilders/StructureDefinitionBuilder.cs b/src/Firely.Fhir.Validation.Compilation.Shared/EnterpriseSchemaBuilders/StructureDefinitionBuilder.cs new file mode 100644 index 00000000..69db32c2 --- /dev/null +++ b/src/Firely.Fhir.Validation.Compilation.Shared/EnterpriseSchemaBuilders/StructureDefinitionBuilder.cs @@ -0,0 +1,20 @@ +using Hl7.Fhir.Specification.Navigation; +using System.Collections.Generic; + +namespace Firely.Fhir.Validation.Compilation +{ + /// + /// The schema builder for the . + /// + public class StructureDefinitionBuilder : ISchemaBuilder + { + /// + public IEnumerable Build(ElementDefinitionNavigator nav, ElementConversionMode? conversionMode = ElementConversionMode.Full) + { + if (nav.Current.Path == "StructureDefinition") + { + yield return new StructureDefinitionValidator(); + }; + } + } +} diff --git a/src/Firely.Fhir.Validation.Compilation.Shared/Firely.Fhir.Validation.Compilation.Shared.projitems b/src/Firely.Fhir.Validation.Compilation.Shared/Firely.Fhir.Validation.Compilation.Shared.projitems index e9fea457..87d6dd7d 100644 --- a/src/Firely.Fhir.Validation.Compilation.Shared/Firely.Fhir.Validation.Compilation.Shared.projitems +++ b/src/Firely.Fhir.Validation.Compilation.Shared/Firely.Fhir.Validation.Compilation.Shared.projitems @@ -11,6 +11,7 @@ + diff --git a/src/Firely.Fhir.Validation.Compilation.Shared/SchemaBuilders/StandardBuilders.cs b/src/Firely.Fhir.Validation.Compilation.Shared/SchemaBuilders/StandardBuilders.cs index c23ae57d..63f68d43 100644 --- a/src/Firely.Fhir.Validation.Compilation.Shared/SchemaBuilders/StandardBuilders.cs +++ b/src/Firely.Fhir.Validation.Compilation.Shared/SchemaBuilders/StandardBuilders.cs @@ -33,7 +33,8 @@ public StandardBuilders(IAsyncResourceResolver source) new ContentReferenceBuilder(), new TypeReferenceBuilder(source), new CanonicalBuilder(), - new ElementDefinitionBuilder() + new ElementDefinitionBuilder(), + new StructureDefinitionBuilder() }; } diff --git a/src/Firely.Fhir.Validation/EnterpriseValidators/StructureDefinitionValidator.cs b/src/Firely.Fhir.Validation/EnterpriseValidators/StructureDefinitionValidator.cs new file mode 100644 index 00000000..fc4ccd8b --- /dev/null +++ b/src/Firely.Fhir.Validation/EnterpriseValidators/StructureDefinitionValidator.cs @@ -0,0 +1,70 @@ +using Hl7.Fhir.ElementModel; +using Hl7.Fhir.Support; +using Hl7.FhirPath.Sprache; +using Newtonsoft.Json.Linq; +using System.Collections.Generic; +using System.Linq; +using System.Runtime.Serialization; + +namespace Firely.Fhir.Validation +{ + /// + /// An that represents a FHIR StructureDefinition + /// + [DataContract] + public class StructureDefinitionValidator : IValidatable + { + /// + public JToken ToJson() => new JProperty("elementDefinition", new JObject()); + + /// + public ResultReport Validate(ITypedElement input, ValidationContext vc, ValidationState state) + { + var evidence = new List(); + + //this can be expanded with other validate functionality + evidence.AddRange(validateInvariantUniqueness(input, state)); + + return ResultReport.FromEvidence(evidence); + } + + /// + /// Validates if the invariants defined in the snapshot and differentials have a unique key. + /// Duplicate keys can exist when the element paths are also the same (e.g. in slices). + /// + /// + /// + /// + private static IEnumerable validateInvariantUniqueness(ITypedElement input, ValidationState state) + { + var snapshotElements = input.Children("snapshot").SelectMany(c => c.Children("element")); + var diffElements = input.Children("differential").SelectMany(c => c.Children("element")); + + var snapshotEvidence = validateInvariantUniqueness(snapshotElements); + var diffEvidence = validateInvariantUniqueness(diffElements); + + return snapshotEvidence.Concat(diffEvidence).Select(i => i.AsResult(input, state)); + } + + private static IEnumerable validateInvariantUniqueness(IEnumerable elements) + { + //Selects the combination of key and elementDefintion path for the duplicate keys where the paths are not also the same. + + IEnumerable<(string Key, string Path)> PathsPerInvariantKey = elements + .SelectMany(e => e.Children("constraint") + .Select(c => (Key: c.Children("key") + .Single().Value.ToString(), + Path: e.Children("path") + .Single().Value.ToString()))); + + IEnumerable<(string Key, IEnumerable Paths)> PathsPerDuplicateInvariantKey = PathsPerInvariantKey.GroupBy(pair => pair.Key) + .Select(group => (Key: group.Key, Paths: group.Select(pair => pair.Path) // select all paths, per invariant key + .Distinct())) //Distinct to remove paths that are encountered multiple times per invariant + .Where(kv => kv.Paths.Count() > 1); //Remove entries that only have a single path. These are not incorrect. + + return PathsPerDuplicateInvariantKey.Select(c => new IssueAssertion(Issue.PROFILE_ELEMENTDEF_INCORRECT, $"Duplicate key '{c.Key}' in paths: {string.Join(", ", c.Paths)}")); + } + } +} + +#nullable restore diff --git a/test/Firely.Fhir.Validation.Compilation.Tests.Shared/Firely.Fhir.Validation.Compilation.Tests.Shared.projitems b/test/Firely.Fhir.Validation.Compilation.Tests.Shared/Firely.Fhir.Validation.Compilation.Tests.Shared.projitems index 24f3077d..e472bdf7 100644 --- a/test/Firely.Fhir.Validation.Compilation.Tests.Shared/Firely.Fhir.Validation.Compilation.Tests.Shared.projitems +++ b/test/Firely.Fhir.Validation.Compilation.Tests.Shared/Firely.Fhir.Validation.Compilation.Tests.Shared.projitems @@ -1804,6 +1804,7 @@ + diff --git a/test/Firely.Fhir.Validation.Compilation.Tests.Shared/StructureDefinitionSchemaValidationTests.cs b/test/Firely.Fhir.Validation.Compilation.Tests.Shared/StructureDefinitionSchemaValidationTests.cs new file mode 100644 index 00000000..42aefb31 --- /dev/null +++ b/test/Firely.Fhir.Validation.Compilation.Tests.Shared/StructureDefinitionSchemaValidationTests.cs @@ -0,0 +1,105 @@ +using Firely.Fhir.Validation.Compilation.Tests; +using FluentAssertions; +using Hl7.Fhir.ElementModel; +using Hl7.Fhir.Model; +using System.Linq; +using Xunit; + +namespace Firely.Fhir.Validation.Tests.Impl +{ + public class StructureDefinitionSchemaTests : IClassFixture + { + internal SchemaBuilderFixture _fixture; + + public StructureDefinitionSchemaTests(SchemaBuilderFixture fixture) => _fixture = fixture; + + + [Fact] + public void ValidateElementDefinitioninProfileValueType() + { + var structureDefSchema = _fixture.SchemaResolver.GetSchema("http://hl7.org/fhir/StructureDefinition/StructureDefinition"); + + var elementDef1 = new ElementDefinition + { + Path = "Patient.gender", + Constraint = new() + { + new() + { + Key = "key1", + Expression = "foo.bar" + }, + new() + { + Key = "key2", + Expression = "bar.foo" + } + } + }; + var elementDef2 = new ElementDefinition + { + Path = "Patient.gender", + Constraint = new() + { + new() + { + Key = "key1", + Expression = "foo.bar" + }, + } + }; + var elementDef3 = new ElementDefinition + { + Path = "Patient.birthdate", + Constraint = new() + { + new() + { + Key = "key2", + Expression = "bar.foo" + } + } + }; + var elementDef4 = new ElementDefinition + { + Path = "Patient.birthdate", + Constraint = new() + { + new() + { + Key = "key4", + Expression = "bar.foo" + } + } + }; + + var profile = new StructureDefinition + { + Snapshot = new() + { + Element = new() { + + elementDef1, + elementDef2, + elementDef3, + elementDef4 + } + }, + Differential = new() + { + Element = new() { + + elementDef1, + elementDef2, + elementDef3, + elementDef4 + } + } + }; + + var results = structureDefSchema!.Validate(profile.ToTypedElement(), _fixture.NewValidationContext()); + + results.Warnings.Select(w => w.Message).Should().Contain($"Duplicate key 'key2' in paths: Patient.gender, Patient.birthdate"); + } + } +} From 5d540a9dabaed1306301696543dca8c4e2e89d30 Mon Sep 17 00:00:00 2001 From: mmsmits Date: Tue, 19 Sep 2023 17:30:03 +0200 Subject: [PATCH 2/3] minor improvements --- .../EnterpriseValidators/StructureDefinitionValidator.cs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/Firely.Fhir.Validation/EnterpriseValidators/StructureDefinitionValidator.cs b/src/Firely.Fhir.Validation/EnterpriseValidators/StructureDefinitionValidator.cs index fc4ccd8b..819b046d 100644 --- a/src/Firely.Fhir.Validation/EnterpriseValidators/StructureDefinitionValidator.cs +++ b/src/Firely.Fhir.Validation/EnterpriseValidators/StructureDefinitionValidator.cs @@ -20,10 +20,8 @@ public class StructureDefinitionValidator : IValidatable /// public ResultReport Validate(ITypedElement input, ValidationContext vc, ValidationState state) { - var evidence = new List(); - //this can be expanded with other validate functionality - evidence.AddRange(validateInvariantUniqueness(input, state)); + var evidence = validateInvariantUniqueness(input, state).ToList(); return ResultReport.FromEvidence(evidence); } @@ -46,7 +44,7 @@ private static IEnumerable validateInvariantUniqueness(ITypedEleme return snapshotEvidence.Concat(diffEvidence).Select(i => i.AsResult(input, state)); } - private static IEnumerable validateInvariantUniqueness(IEnumerable elements) + private static List validateInvariantUniqueness(IEnumerable elements) { //Selects the combination of key and elementDefintion path for the duplicate keys where the paths are not also the same. @@ -62,7 +60,7 @@ private static IEnumerable validateInvariantUniqueness(IEnumerab .Distinct())) //Distinct to remove paths that are encountered multiple times per invariant .Where(kv => kv.Paths.Count() > 1); //Remove entries that only have a single path. These are not incorrect. - return PathsPerDuplicateInvariantKey.Select(c => new IssueAssertion(Issue.PROFILE_ELEMENTDEF_INCORRECT, $"Duplicate key '{c.Key}' in paths: {string.Join(", ", c.Paths)}")); + return PathsPerDuplicateInvariantKey.Select(c => new IssueAssertion(Issue.PROFILE_ELEMENTDEF_INCORRECT, $"Duplicate key '{c.Key}' in paths: {string.Join(", ", c.Paths)}")).ToList(); } } } From 2fcaa74177b59a483f8d65fcff98e20257604744 Mon Sep 17 00:00:00 2001 From: mmsmits Date: Thu, 21 Sep 2023 13:11:43 +0200 Subject: [PATCH 3/3] fixing review comments --- .../EnterpriseValidators/StructureDefinitionValidator.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Firely.Fhir.Validation/EnterpriseValidators/StructureDefinitionValidator.cs b/src/Firely.Fhir.Validation/EnterpriseValidators/StructureDefinitionValidator.cs index 819b046d..6c793624 100644 --- a/src/Firely.Fhir.Validation/EnterpriseValidators/StructureDefinitionValidator.cs +++ b/src/Firely.Fhir.Validation/EnterpriseValidators/StructureDefinitionValidator.cs @@ -21,7 +21,7 @@ public class StructureDefinitionValidator : IValidatable public ResultReport Validate(ITypedElement input, ValidationContext vc, ValidationState state) { //this can be expanded with other validate functionality - var evidence = validateInvariantUniqueness(input, state).ToList(); + var evidence = validateInvariantUniqueness(input, state); return ResultReport.FromEvidence(evidence); } @@ -33,7 +33,7 @@ public ResultReport Validate(ITypedElement input, ValidationContext vc, Validati /// /// /// - private static IEnumerable validateInvariantUniqueness(ITypedElement input, ValidationState state) + private static List validateInvariantUniqueness(ITypedElement input, ValidationState state) { var snapshotElements = input.Children("snapshot").SelectMany(c => c.Children("element")); var diffElements = input.Children("differential").SelectMany(c => c.Children("element")); @@ -41,7 +41,7 @@ private static IEnumerable validateInvariantUniqueness(ITypedEleme var snapshotEvidence = validateInvariantUniqueness(snapshotElements); var diffEvidence = validateInvariantUniqueness(diffElements); - return snapshotEvidence.Concat(diffEvidence).Select(i => i.AsResult(input, state)); + return snapshotEvidence.Concat(diffEvidence).Select(i => i.AsResult(input, state)).ToList(); } private static List validateInvariantUniqueness(IEnumerable elements)