diff --git a/src/Firely.Fhir.Validation/Impl/ElementSchema.cs b/src/Firely.Fhir.Validation/Impl/ElementSchema.cs index 7a730eba..50767403 100644 --- a/src/Firely.Fhir.Validation/Impl/ElementSchema.cs +++ b/src/Firely.Fhir.Validation/Impl/ElementSchema.cs @@ -20,7 +20,7 @@ namespace Firely.Fhir.Validation /// schema to be succesful. /// [DataContract] - public class ElementSchema : IGroupValidatable + public class ElementSchema : IGroupValidatable, IEquatable { /// /// The unique id for this schema. @@ -170,6 +170,10 @@ static JToken nest(JToken mem) => /// Whether the schema has members. /// internal bool IsEmpty() => !Members.Any(); + + + /// + public bool Equals(ElementSchema? other) => this.Id == other?.Id; } } #pragma warning restore CS0618 // Type or member is obsolete diff --git a/src/Firely.Fhir.Validation/Impl/ReferencedInstanceValidator.cs b/src/Firely.Fhir.Validation/Impl/ReferencedInstanceValidator.cs index 4304bc45..e883e8b5 100644 --- a/src/Firely.Fhir.Validation/Impl/ReferencedInstanceValidator.cs +++ b/src/Firely.Fhir.Validation/Impl/ReferencedInstanceValidator.cs @@ -227,19 +227,16 @@ private ResultReport validateReferencedResource(string reference, ValidationSett //result += Trace($"Starting validation of referenced resource {reference} ({encounteredKind})"); - // References within the instance are dealt with within the same validator, - // references to external entities will operate within a new instance of a validator (and hence a new tracking context). - // In both cases, the outcome is included in the result. + ValidationState? newState; if (resolution.ReferenceKind != AggregationMode.Referenced) - return Schema.ValidateOne(resolution.ReferencedResource.AsScopedNode(), vc, state.UpdateInstanceLocation(dp => dp.AddInternalReference(resolution.ReferencedResource.Location))); + newState = state.UpdateInstanceLocation(dp => dp.AddInternalReference(resolution.ReferencedResource.Location)) with {Parent = state}; else { - //TODO: We're using state to track the external URL, but this actually would be better - //implemented on the ScopedNode instead - add this (and combine with FullUrl?) there. - var newState = state.NewInstanceScope(); + newState = state.NewInstanceScope(); newState.Instance.ResourceUrl = reference; - return Schema.ValidateOne(resolution.ReferencedResource.AsScopedNode(), vc, newState); } + + return Schema.ValidateOne(resolution.ReferencedResource.AsScopedNode(), vc, newState); } /// diff --git a/src/Firely.Fhir.Validation/Impl/ResourceSchema.cs b/src/Firely.Fhir.Validation/Impl/ResourceSchema.cs index c695210b..37b09728 100644 --- a/src/Firely.Fhir.Validation/Impl/ResourceSchema.cs +++ b/src/Firely.Fhir.Validation/Impl/ResourceSchema.cs @@ -67,7 +67,9 @@ internal override ResultReport ValidateInternal(IEnumerable input, { // Schemas representing the root of a FHIR resource cannot meaningfully be used as a GroupValidatable, // so we'll turn this into a normal IValidatable. - var results = input.Select((i, index) => ValidateInternal(i, vc, state.UpdateInstanceLocation(d => d.ToIndex(index)))); + var results = input.Count() == 1 + ? [ValidateInternal(input.First(), vc, state)] + : input.Select((i, index) => ValidateInternal(i, vc, state.UpdateInstanceLocation(d => d.ToIndex(index)))); return ResultReport.Combine(results.ToList()); } @@ -125,7 +127,7 @@ internal ResultReport ValidateResourceSchema(IScopedNode input, ValidationSettin { return state.Global.RunValidations.Start( state, - Id.ToString(), // is the same as the canonical for resource schemas + Id.ToString(), // is the same as the canonical for resource schemas () => { state.Global.ResourcesValidated += 1; diff --git a/src/Firely.Fhir.Validation/PublicAPI.Unshipped.txt b/src/Firely.Fhir.Validation/PublicAPI.Unshipped.txt index a6e21426..86a9b1c6 100644 --- a/src/Firely.Fhir.Validation/PublicAPI.Unshipped.txt +++ b/src/Firely.Fhir.Validation/PublicAPI.Unshipped.txt @@ -98,6 +98,7 @@ Firely.Fhir.Validation.DefinitionsAssertion.ToJson() -> Newtonsoft.Json.Linq.JTo Firely.Fhir.Validation.ElementSchema Firely.Fhir.Validation.ElementSchema.ElementSchema(Firely.Fhir.Validation.Canonical! id, params Firely.Fhir.Validation.IAssertion![]! members) -> void Firely.Fhir.Validation.ElementSchema.ElementSchema(Firely.Fhir.Validation.Canonical! id, System.Collections.Generic.IEnumerable! members) -> void +Firely.Fhir.Validation.ElementSchema.Equals(Firely.Fhir.Validation.ElementSchema? other) -> bool Firely.Fhir.Validation.ExtensionSchema Firely.Fhir.Validation.ExtensionSchema.ExtensionSchema(Firely.Fhir.Validation.StructureDefinitionInformation! structureDefinition, params Firely.Fhir.Validation.IAssertion![]! members) -> void Firely.Fhir.Validation.ExtensionSchema.ExtensionSchema(Firely.Fhir.Validation.StructureDefinitionInformation! structureDefinition, System.Collections.Generic.IEnumerable! members) -> void diff --git a/src/Firely.Fhir.Validation/Schema/DefinitionPath.cs b/src/Firely.Fhir.Validation/Schema/DefinitionPath.cs index a344a62a..61b8dbd4 100644 --- a/src/Firely.Fhir.Validation/Schema/DefinitionPath.cs +++ b/src/Firely.Fhir.Validation/Schema/DefinitionPath.cs @@ -14,7 +14,7 @@ namespace Firely.Fhir.Validation /// This class is also used to track the location of an instance. /// /// An example skeleton path is: "Resource(canonical).childA.childB[sliceB1].childC -> Datatype(canonical).childA" - internal class DefinitionPath : PathStack + internal record DefinitionPath : PathStack { private DefinitionPath(PathStackEvent? current) : base(current) { @@ -85,7 +85,7 @@ public bool TryGetSliceInfo(out string? sliceInfo) /// /// Start a new DefinitionPath. /// - public static DefinitionPath Start() => new(null); + public static DefinitionPath Start() => new(current: null); /// /// Update the path to include a move to a child element. diff --git a/src/Firely.Fhir.Validation/Schema/InstancePath.cs b/src/Firely.Fhir.Validation/Schema/InstancePath.cs index ef14e915..0188802d 100644 --- a/src/Firely.Fhir.Validation/Schema/InstancePath.cs +++ b/src/Firely.Fhir.Validation/Schema/InstancePath.cs @@ -8,13 +8,14 @@ using System.Collections.Generic; using System.Linq; +using System.Text.RegularExpressions; namespace Firely.Fhir.Validation { /// /// A class representing the location of an instance. /// - internal class InstancePath : PathStack + internal record InstancePath : PathStack { private InstancePath(PathStackEvent? current) : base(current) { @@ -40,7 +41,7 @@ public override string ToString() /// /// Start a new InstancePath. /// - public static InstancePath Start() => new(null); + public static InstancePath Start() => new(current: null); /// /// Start the instance path with a reference to a resource. Can only be used at the beginning of the path. @@ -78,8 +79,20 @@ public InstancePath AddOriginalIndices(IEnumerable indices) => : this; /// - /// Update the path to include a reference to an internal element. + /// Update the path to include a reference to an internal element. This may imply adding an index nav event as well /// - public InstancePath AddInternalReference(string location) => new(new InternalReferenceNavEvent(Current, location)); + public InstancePath AddInternalReference(string location) + { + var match = new Regex(@"(?[^\[]+)(?:\[(?\d+)\])?").Match(location); + var locationWithoutIndex = match.Groups["instance"].Value; + var hasIndex = match.Groups["index"].Success; + + var navEvt = new InternalReferenceNavEvent(Current, locationWithoutIndex); + if(!hasIndex) + return new(navEvt); + + var index = int.Parse(match.Groups["index"].Value); + return new (new IndexNavEvent(navEvt, index)); + } } } \ No newline at end of file diff --git a/src/Firely.Fhir.Validation/Schema/PathStack.cs b/src/Firely.Fhir.Validation/Schema/PathStack.cs index fd67b28f..3c15666a 100644 --- a/src/Firely.Fhir.Validation/Schema/PathStack.cs +++ b/src/Firely.Fhir.Validation/Schema/PathStack.cs @@ -19,7 +19,7 @@ namespace Firely.Fhir.Validation /// /// An example skeleton path is: "Resource(canonical).childA.childB[sliceB1].childC -> Datatype(canonical).childA" /// - internal abstract class PathStack + internal abstract record PathStack { /// /// Construct a new PathStack and place the current event on top of the stack. @@ -39,7 +39,7 @@ protected PathStack(PathStackEvent? current) /// /// A linked list of events that represent a path. /// - internal abstract class PathStackEvent + internal abstract record PathStackEvent { /// /// Add a new event to the path. @@ -60,7 +60,7 @@ internal abstract class PathStackEvent } - internal class ChildNavEvent : PathStackEvent + internal record ChildNavEvent : PathStackEvent { public ChildNavEvent(PathStackEvent? previous, string childName, string? choiceType) : base(previous) { @@ -77,7 +77,7 @@ protected internal override string Render() => public override string ToString() => $"ChildNavEvent: ChildName: {ChildName}, ChoiceType: {ChoiceType}"; } - internal class IndexNavEvent : PathStackEvent + internal record IndexNavEvent : PathStackEvent { public IndexNavEvent(PathStackEvent? previous, int index) : base(previous) { @@ -94,7 +94,7 @@ Previous is OriginalIndicesNavEvent originalIndices public override string ToString() => $"IndexNavEvent: Index: {Index}"; } - internal class OriginalIndicesNavEvent : PathStackEvent + internal record OriginalIndicesNavEvent : PathStackEvent { public OriginalIndicesNavEvent(PathStackEvent? previous, IEnumerable originalIndices) : base(previous) { @@ -108,18 +108,11 @@ public OriginalIndicesNavEvent(PathStackEvent? previous, IEnumerable origin public override string ToString() => $"OriginalIndicesNavEvent: Indices: {string.Join(',', OriginalIndices)}"; } - internal class InternalReferenceNavEvent : PathStackEvent + internal record InternalReferenceNavEvent : PathStackEvent { public InternalReferenceNavEvent(PathStackEvent? previous, string location) : base(previous) { - Location = location.EndsWith(']') ? removeIndexPart(location) : location; // remove last index - - - static string removeIndexPart(string location) - { - var indexStart = location.LastIndexOf('['); - return indexStart > 0 ? location[..indexStart] : location; - } + Location = location; // remove last index } public string Location { get; } @@ -128,14 +121,14 @@ static string removeIndexPart(string location) } #pragma warning disable CS0618 // Type or member is obsolete - internal class InvokeProfileEvent : PathStackEvent + internal record InvokeProfileEvent : PathStackEvent { public InvokeProfileEvent(PathStackEvent? previous, ElementSchema schema) : base(previous) { Schema = schema; } - public ElementSchema Schema { get; } + public ElementSchema Schema { get; } // implements iEquatable, so we can compare path stacks by value protected internal override string Render() { @@ -156,7 +149,7 @@ protected internal override string Render() } #pragma warning restore CS0618 // Type or member is obsolete - internal class CheckSliceEvent : PathStackEvent + internal record CheckSliceEvent : PathStackEvent { public CheckSliceEvent(PathStackEvent? previous, string sliceName) : base(previous) { @@ -168,7 +161,7 @@ public CheckSliceEvent(PathStackEvent? previous, string sliceName) : base(previo protected internal override string Render() => $"[{SliceName}]"; } - internal class StartResourceNavEvent : PathStackEvent + internal record StartResourceNavEvent : PathStackEvent { public StartResourceNavEvent(PathStackEvent? previous, string resource) : base(previous) { diff --git a/src/Firely.Fhir.Validation/Schema/ValidationLogger.cs b/src/Firely.Fhir.Validation/Schema/ValidationLogger.cs index 1707b8b3..7a42477b 100644 --- a/src/Firely.Fhir.Validation/Schema/ValidationLogger.cs +++ b/src/Firely.Fhir.Validation/Schema/ValidationLogger.cs @@ -9,13 +9,13 @@ using Hl7.Fhir.Support; using System; using System.Collections.Generic; +using System.Linq; +using System.Runtime.CompilerServices; #nullable enable namespace Firely.Fhir.Validation { - - /// /// This class is used to track which resources/contained resources/bundled resources are already /// validated, and what the previous result was. Since it keeps track of running validations, it @@ -73,24 +73,22 @@ public override string ToString() => public ResultReport Start(ValidationState state, string profileUrl, Func validator) #pragma warning restore CS0618 // Type or member is obsolete { - var resourceUrl = state.Instance.ResourceUrl; - var fullLocation = (resourceUrl is not null ? resourceUrl + "#" : "") + state.Location.InstanceLocation.ToString(); - + var fullLocation = getFullLocation(state); var key = (fullLocation, profileUrl); if (_data.TryGetValue(key, out var existing)) { if (existing.Result is null) return new IssueAssertion(Issue.CONTENT_REFERENCE_CYCLE_DETECTED, - $"Detected a loop: instance data inside '{fullLocation}' refers back to itself.") + $"Detected a loop: instance data inside '{fullLocation}' refers back to itself (cycle structure: {showRefCycle(state)}).") .AsResult(fullLocation); - else - { - // If the validation has been run before, return an outcome with the same result. - // Note: we don't keep a copy of the original outcome since it has been included in the - // total result (at the first run) and keeping them around costs a lot of memory. - return new ResultReport(existing.Result.Value, new TraceAssertion(fullLocation, $"Repeated validation at {fullLocation} against profile {profileUrl}.")); - } + + // If the validation has been run before, return an outcome with the same result. + // Note: we don't keep a copy of the original outcome since it has been included in the + // total result (at the first run) and keeping them around costs a lot of memory. + return existing.Result is not null + ? new ResultReport(existing.Result.Value, new TraceAssertion(fullLocation, $"Repeated validation at {fullLocation} against profile {profileUrl}.")) + : ResultReport.SUCCESS; } else { @@ -106,6 +104,24 @@ public ResultReport Start(ValidationState state, string profileUrl, Func(); +#pragma warning restore CS0618 // Type or member is obsolete + return string.Join( + " -> ", + state.Parents().TakeWhile(parentState => visited.Add(parentState)).Reverse().Select(getFullLocation) + ); + } + +#pragma warning disable CS0618 // Type or member is obsolete + private string getFullLocation(ValidationState state) => +#pragma warning restore CS0618 // Type or member is obsolete + state.Instance.ResourceUrl is null + ? state.Location.InstanceLocation.ToString() + : $"{state.Instance.ResourceUrl}#{state.Location.InstanceLocation}"; /// /// The number of run or running validations in this logger. diff --git a/src/Firely.Fhir.Validation/Schema/ValidationState.cs b/src/Firely.Fhir.Validation/Schema/ValidationState.cs index 24b37d73..e49b75e8 100644 --- a/src/Firely.Fhir.Validation/Schema/ValidationState.cs +++ b/src/Firely.Fhir.Validation/Schema/ValidationState.cs @@ -8,7 +8,9 @@ using Hl7.FhirPath; using System; +using System.Collections.Generic; using System.ComponentModel; +using System.Transactions; namespace Firely.Fhir.Validation @@ -57,7 +59,7 @@ internal class GlobalState /// (if the resource under validation references an external resource, /// the validation of that resource will have its own . /// - internal class InstanceState + internal record InstanceState { /// /// The URL where the current instance was retrieved (if known). @@ -65,6 +67,18 @@ internal class InstanceState public string? ResourceUrl { get; set; } } + internal ValidationState? Parent { get; set; } + + internal IEnumerable Parents() + { + var current = this; + while (current.Parent != null) + { + yield return current.Parent; + current = current.Parent; + } + } + /// /// State to be kept while validating a single instance. /// @@ -78,6 +92,7 @@ internal ValidationState NewInstanceScope() => { // Global data is shared across ValidationState instances. Global = Global, + Parent = this }; /// @@ -107,26 +122,9 @@ internal class LocationState /// Update the location, returning a new state with the updated location. /// internal ValidationState UpdateLocation(Func pathStackUpdate) => - new() - { - Global = Global, - Instance = Instance, - Location = new LocationState - { - DefinitionPath = pathStackUpdate(Location.DefinitionPath), - InstanceLocation = Location.InstanceLocation // is this correct - } - }; + this with {Location = new LocationState { DefinitionPath = pathStackUpdate(Location.DefinitionPath), InstanceLocation = Location.InstanceLocation }}; + internal ValidationState UpdateInstanceLocation(Func pathStackUpdate) => - new() - { - Global = Global, - Instance = Instance, - Location = new LocationState - { - DefinitionPath = Location.DefinitionPath, // is this correct? - InstanceLocation = pathStackUpdate(Location.InstanceLocation) - } - }; + this with {Location = new LocationState { DefinitionPath = Location.DefinitionPath, InstanceLocation = pathStackUpdate(Location.InstanceLocation) }}; } } \ No newline at end of file diff --git a/src/Firely.Fhir.Validation/Support/IScopedNodeExtensions.cs b/src/Firely.Fhir.Validation/Support/IScopedNodeExtensions.cs index 83d4b9fb..412ea05c 100644 --- a/src/Firely.Fhir.Validation/Support/IScopedNodeExtensions.cs +++ b/src/Firely.Fhir.Validation/Support/IScopedNodeExtensions.cs @@ -27,8 +27,8 @@ internal static class IScopedNodeExtensions /// the methods and . /// [Obsolete("WARNING! For internal API use only. Turning an IScopedNode into an ITypedElement will cause problems for" + - "Location and Definitions. Those properties are not implemented using this method and can cause problems " + - "elsewhere. Please don't use this method unless you know what you are doing.")] + "Location and Definitions. Those properties are not implemented using this method and can cause problems " + + "elsewhere. Please don't use this method unless you know what you are doing.")] public static ITypedElement AsTypedElement(this IScopedNode node) => node switch { TypedElementToIScopedNodeToAdapter adapter => adapter.ScopedNode, @@ -50,7 +50,7 @@ internal static class IScopedNodeExtensions /// /// public static IEnumerable Children(this IEnumerable nodes, string? name = null) => - nodes.SelectMany(n => n.Children(name)); + nodes.SelectMany(n => n.Children(name)); internal static bool Matches(this IScopedNode value, ITypedElement pattern) { @@ -92,7 +92,6 @@ internal static bool IsExactlyEqualTo(this IScopedNode left, ITypedElement right return childrenL.Zip(childrenR, (childL, childR) => childL.Name == childR.Name && childL.IsExactlyEqualTo(childR, ignoreOrder)).All(t => t); } - } } diff --git a/test/Firely.Fhir.Validation.Compilation.Tests.Shared/FhirTestCases b/test/Firely.Fhir.Validation.Compilation.Tests.Shared/FhirTestCases index 8068ff26..99bf0f18 160000 --- a/test/Firely.Fhir.Validation.Compilation.Tests.Shared/FhirTestCases +++ b/test/Firely.Fhir.Validation.Compilation.Tests.Shared/FhirTestCases @@ -1 +1 @@ -Subproject commit 8068ff2636300fe1299af6c62c47247839688786 +Subproject commit 99bf0f180abfdc87461216f94214c4387179341f diff --git a/test/Firely.Fhir.Validation.Compilation.Tests.Shared/FhirTests/ValidationManifestTest.cs b/test/Firely.Fhir.Validation.Compilation.Tests.Shared/FhirTests/ValidationManifestTest.cs index ec4540cc..33058798 100644 --- a/test/Firely.Fhir.Validation.Compilation.Tests.Shared/FhirTests/ValidationManifestTest.cs +++ b/test/Firely.Fhir.Validation.Compilation.Tests.Shared/FhirTests/ValidationManifestTest.cs @@ -32,7 +32,7 @@ public class ValidationManifestTest /// //[Ignore] [DataTestMethod] - [ValidationManifestDataSource(TEST_CASES_MANIFEST, singleTest: "mr-m-simple-nossystem")] + [ValidationManifestDataSource(TEST_CASES_MANIFEST, singleTest: "message-empty-entry")] public void RunSingleTest(TestCase testCase, string baseDirectory) => _runner.RunTestCase(testCase, DotNetValidator.Create(), baseDirectory); @@ -59,7 +59,7 @@ public void RunFirelySdkTests(TestCase testCase, string baseDirectory) /// - The method `ClassCleanup` will gather all the testcases and serialize those to disk. The filename can be altered in /// that method /// - [Ignore("This test is only used to generate the Firely SDK results in the manifest. See the method for more info")] + // [Ignore("This test is only used to generate the Firely SDK results in the manifest. See the method for more info")] [TestMethod] public void AddFirelySdkValidatorResults() => _runner.AddOrEditValidatorResults(TEST_CASES_MANIFEST, new[] { DotNetValidator.Create() }); diff --git a/test/Firely.Fhir.Validation.Tests/Impl/CircularReferenceTests.cs b/test/Firely.Fhir.Validation.Tests/Impl/CircularReferenceTests.cs index f3a3497a..fc0813d2 100644 --- a/test/Firely.Fhir.Validation.Tests/Impl/CircularReferenceTests.cs +++ b/test/Firely.Fhir.Validation.Tests/Impl/CircularReferenceTests.cs @@ -10,6 +10,7 @@ using Hl7.Fhir.ElementModel; using Hl7.Fhir.Support; using Microsoft.VisualStudio.TestTools.UnitTesting; +using System.Linq; namespace Firely.Fhir.Validation.Tests { @@ -90,6 +91,7 @@ public void CircularInContainedResources() var result = test(SCHEMA, pat.DictionaryToTypedElement("Patient")); result.IsSuccessful.Should().BeTrue(); result.Evidence.Should().Contain(ass => (ass as IssueAssertion)!.IssueNumber == Issue.CONTENT_REFERENCE_CYCLE_DETECTED.Code); + result.Warnings.Should().ContainSingle(ass => ass.IssueNumber == Issue.CONTENT_REFERENCE_CYCLE_DETECTED.Code).Subject.Message.Should().ContainAll(["Patient.contained[1]", "Patient.contained[0]"]); } [TestMethod] diff --git a/test/Firely.Fhir.Validation.Tests/Impl/TestInstancePath.cs b/test/Firely.Fhir.Validation.Tests/Impl/TestInstancePath.cs index 44862de8..c2700f26 100644 --- a/test/Firely.Fhir.Validation.Tests/Impl/TestInstancePath.cs +++ b/test/Firely.Fhir.Validation.Tests/Impl/TestInstancePath.cs @@ -52,11 +52,10 @@ public void NavigateToInternalReference() .ToChild("resource") .ToChild("name") .AddInternalReference("Bundle.entry[29]") - .ToIndex(2) .ToChild("resource") .ToChild("name") .ToIndex(1); - testee.ToString().Should().Be("Bundle.entry[2].resource.name[1]"); + testee.ToString().Should().Be("Bundle.entry[29].resource.name[1]"); } [TestMethod]