computed circular reference error messages

src/Firely.Fhir.Validation/Impl/ElementSchema.cs

@@ -20,7 +20,7 @@ namespace Firely.Fhir.Validation
     /// schema to be succesful.
     /// </summary>
     [DataContract]
-    public class ElementSchema : IGroupValidatable
+    public class ElementSchema : IGroupValidatable, IEquatable<ElementSchema>
     {
         /// <summary>
         /// The unique id for this schema.
@@ -170,6 +170,10 @@ static JToken nest(JToken mem) =>
         /// Whether the schema has members.
         /// </summary>
         internal bool IsEmpty() => !Members.Any();
+
+
+        /// <inheritdoc />
+        public bool Equals(ElementSchema? other) => this.Id == other?.Id;
     }
 }
 #pragma warning restore CS0618 // Type or member is obsolete

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);
         }
 
         /// <inheritdoc cref="IJsonSerializable.ToJson"/>

src/Firely.Fhir.Validation/Impl/ResourceSchema.cs

@@ -67,7 +67,9 @@ internal override ResultReport ValidateInternal(IEnumerable<IScopedNode> 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;

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<Firely.Fhir.Validation.IAssertion!>! 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<Firely.Fhir.Validation.IAssertion!>! members) -> void

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.
     /// </summary>
     /// <example>An example skeleton path is: "Resource(canonical).childA.childB[sliceB1].childC -> Datatype(canonical).childA"</example>
-    internal class DefinitionPath : PathStack
+    internal record DefinitionPath : PathStack
     {
         private DefinitionPath(PathStackEvent? current) : base(current)
         {
@@ -85,7 +85,7 @@ public bool TryGetSliceInfo(out string? sliceInfo)
         /// <summary>
         /// Start a new DefinitionPath.
         /// </summary>
-        public static DefinitionPath Start() => new(null);
+        public static DefinitionPath Start() => new(current: null);
 
         /// <summary>
         /// Update the path to include a move to a child element.

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
 {
     /// <summary>
     /// A class representing the location of an instance.
     /// </summary>
-    internal class InstancePath : PathStack
+    internal record InstancePath : PathStack
     {
         private InstancePath(PathStackEvent? current) : base(current)
         {
@@ -40,7 +41,7 @@ public override string ToString()
         /// <summary>
         /// Start a new InstancePath.
         /// </summary>
-        public static InstancePath Start() => new(null);
+        public static InstancePath Start() => new(current: null);
 
         /// <summary>
         /// 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<int> indices) =>
                 : this;
 
         /// <summary>
-        /// 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
         /// </summary>
-        public InstancePath AddInternalReference(string location) => new(new InternalReferenceNavEvent(Current, location));
+        public InstancePath AddInternalReference(string location)
+        {
+            var match = new Regex(@"(?<instance>[^\[]+)(?:\[(?<index>\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));
+        }
     }
 }

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"
     /// </summary>
-    internal abstract class PathStack
+    internal abstract record PathStack
     {
         /// <summary>
         /// Construct a new PathStack and place the current event on top of the stack.
@@ -39,7 +39,7 @@ protected PathStack(PathStackEvent? current)
     /// <summary>
     /// A linked list of events that represent a path.
     /// </summary>
-    internal abstract class PathStackEvent
+    internal abstract record PathStackEvent
     {
         /// <summary>
         /// 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<int> originalIndices) : base(previous)
         {
@@ -108,18 +108,11 @@ public OriginalIndicesNavEvent(PathStackEvent? previous, IEnumerable<int> 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)
         {

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
 {
-
-
     /// <summary>
     /// 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<ResultReport> 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<ResultR
                 return result;
             }
         }
+
+#pragma warning disable CS0618 // Type or member is obsolete
+        private string showRefCycle(ValidationState state)
+        {
+            var visited = new HashSet<ValidationState>();
+#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}";
 
         /// <summary>
         /// The number of run or running validations in this logger.

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,14 +59,26 @@ internal class GlobalState
         /// (if the resource under validation references an external resource,
         /// the validation of that resource will have its own <see cref="InstanceState"/>.
         /// </summary>
-        internal class InstanceState
+        internal record InstanceState
         {
             /// <summary>
             /// The URL where the current instance was retrieved (if known).
             /// </summary>
             public string? ResourceUrl { get; set; }
         }
 
+        internal ValidationState? Parent { get; set; }
+
+        internal IEnumerable<ValidationState> Parents()
+        {
+            var current = this;
+            while (current.Parent != null)
+            {
+                yield return current.Parent;
+                current = current.Parent;
+            }
+        }
+
         /// <summary>
         /// State to be kept while validating a single instance.
         /// </summary>
@@ -78,6 +92,7 @@ internal ValidationState NewInstanceScope() =>
           {
               // Global data is shared across ValidationState instances.
               Global = Global,
+              Parent = this
           };
 
         /// <summary>
@@ -107,26 +122,9 @@ internal class LocationState
         /// Update the location, returning a new state with the updated location.
         /// </summary>
         internal ValidationState UpdateLocation(Func<DefinitionPath, DefinitionPath> 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<InstancePath, InstancePath> 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) }};
     }
 }