Fixed display and detection of circular references

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

@@ -130,7 +130,7 @@ internal ResultReport ValidateResourceSchema(IScopedNode input, ValidationSettin
                 {
                     state.Global.ResourcesValidated += 1;
                     return base.ValidateInternal(input, vc, state);
-                });
+                }, input.ToScopedNode());
         }
 
         /// <inheritdoc/>

src/Firely.Fhir.Validation/Schema/ValidationLogger.cs

@@ -6,16 +6,17 @@
  * available at https://github.com/FirelyTeam/firely-validator-api/blob/main/LICENSE
  */
 
+using Hl7.Fhir.ElementModel;
 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
@@ -68,29 +69,28 @@ public override string ToString() =>
         /// <param name="state">The validation state</param>
         /// <param name="profileUrl">Profile against which we are validating</param>
         /// <param name="validator">Validation to start when it has not been run before.</param>
+        /// <param name="instance"></param>
         /// <returns>The result of calling the validator, or a historic result if there is one.</returns>
 #pragma warning disable CS0618 // Type or member is obsolete
-        public ResultReport Start(ValidationState state, string profileUrl, Func<ResultReport> validator)
+        public ResultReport Start(ValidationState state, string profileUrl, Func<ResultReport> validator, ScopedNode instance)
 #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, instance);
             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, instance)}).")
                         .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 +106,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, ScopedNode instance)
+        {
+            var visited = new HashSet<string>();
+#pragma warning restore CS0618 // Type or member is obsolete
+            return string.Join(
+                " -> ",
+                state.Parents().TakeWhile(parentState => visited.Add(getFullLocation(parentState, instance))).Reverse()
+            );
+        }
+
+#pragma warning disable CS0618 // Type or member is obsolete
+        private string getFullLocation(ValidationState state, ScopedNode instance) =>
+#pragma warning restore CS0618 // Type or member is obsolete
+            state.Instance.ResourceUrl is null
+                ? instance.Location
+                : $"{state.Instance.ResourceUrl}#{instance.Location}";
 
         /// <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; private 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) }};
     }
 }

test/Firely.Fhir.Validation.Compilation.Tests.Shared/FhirTests/ValidationManifestTest.cs

@@ -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
         /// </summary>
-        [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() });