Fix errors from dynamic scope when validation schema load (#340)

JamesNK · Apr 27, 2024 · 05524f9 · 05524f9
1 parent 461fc98
commit 05524f9
Show file tree

Hide file tree

Showing 3 changed files with 131 additions and 8 deletions.
diff --git a/Src/Newtonsoft.Json.Schema.Tests/Issues/Issue0339Tests.cs b/Src/Newtonsoft.Json.Schema.Tests/Issues/Issue0339Tests.cs
@@ -0,0 +1,123 @@
+#region License
+// Copyright (c) Newtonsoft. All Rights Reserved.
+// License: https://raw.github.com/JamesNK/Newtonsoft.Json.Schema/master/LICENSE.md
+#endregion
+
+using System;
+using System.Collections.Generic;
+using System.IO;
+using System.Linq;
+using System.Text;
+using Newtonsoft.Json.Linq;
+using Newtonsoft.Json.Schema.Generation;
+using Newtonsoft.Json.Serialization;
+#if DNXCORE50
+using Xunit;
+using Test = Xunit.FactAttribute;
+using Assert = Newtonsoft.Json.Schema.Tests.XUnitAssert;
+#else
+using NUnit.Framework;
+#endif
+
+namespace Newtonsoft.Json.Schema.Tests.Issues
+{
+    [TestFixture]
+    public class Issue0339Tests : TestFixtureBase
+    {
+        [Test]
+        public void RecursiveRef_DynamicSelection_NoLoadErrors()
+        {
+            string schemaJson = @"{
+    ""$id"": ""recursiveRef8_main.json"",
+    ""$defs"": {
+        ""inner"": {
+            ""$id"": ""recursiveRef8_inner.json"",
+            ""$recursiveAnchor"": true,
+            ""title"": ""inner"",
+            ""additionalProperties"": {
+                ""$recursiveRef"": ""#""
+            }
+        }
+    },
+    ""if"": {
+        ""propertyNames"": {
+            ""pattern"": ""^[a-m]""
+        }
+    },
+    ""then"": {
+        ""title"": ""any type of node"",
+        ""$id"": ""recursiveRef8_anyLeafNode.json"",
+        ""$recursiveAnchor"": true,
+        ""$ref"": ""recursiveRef8_main.json#/$defs/inner""
+    },
+    ""else"": {
+        ""title"": ""integer node"",
+        ""$id"": ""recursiveRef8_integerNode.json"",
+        ""$recursiveAnchor"": true,
+        ""type"": [ ""object"", ""integer"" ],
+        ""$ref"": ""recursiveRef8_main.json#/$defs/inner""
+    }
+}";
+
+            List<ValidationError> schemaErrors = new List<ValidationError>();
+            JSchemaReaderSettings settings = new JSchemaReaderSettings();
+            settings.ValidationEventHandler += (sender, args) => schemaErrors.Add(args.ValidationError);
+
+            JSchema schema = JSchema.Parse(schemaJson, settings);
+
+            Assert.AreEqual(0, schemaErrors.Count);
+        }
+
+        [Test]
+        public void RecursiveRef_DynamicSelection_Duplicates_HasLoadErrors()
+        {
+            string schemaJson = @"{
+    ""$id"": ""recursiveRef8_main.json"",
+    ""$defs"": {
+        ""inner"": {
+            ""$id"": ""recursiveRef8_inner.json"",
+            ""$recursiveAnchor"": true,
+            ""title"": ""inner"",
+            ""additionalProperties"": {
+                ""$recursiveRef"": ""#""
+            }
+        },
+        ""inner2"": {
+            ""$id"": ""recursiveRef8_inner.json"",
+            ""$recursiveAnchor"": true,
+            ""title"": ""inner"",
+            ""additionalProperties"": {
+                ""$recursiveRef"": ""#""
+            }
+        }
+    },
+    ""if"": {
+        ""propertyNames"": {
+            ""pattern"": ""^[a-m]""
+        }
+    },
+    ""then"": {
+        ""title"": ""any type of node"",
+        ""$id"": ""recursiveRef8_anyLeafNode.json"",
+        ""$recursiveAnchor"": true,
+        ""$ref"": ""recursiveRef8_main.json#/$defs/inner""
+    },
+    ""else"": {
+        ""title"": ""integer node"",
+        ""$id"": ""recursiveRef8_anyLeafNode.json"",
+        ""$recursiveAnchor"": true,
+        ""type"": [ ""object"", ""integer"" ],
+        ""$ref"": ""recursiveRef8_main.json#/$defs/inner2""
+    }
+}";
+
+            List<ValidationError> schemaErrors = new List<ValidationError>();
+            JSchemaReaderSettings settings = new JSchemaReaderSettings();
+            settings.ValidationEventHandler += (sender, args) => schemaErrors.Add(args.ValidationError);
+
+            JSchema schema = JSchema.Parse(schemaJson, settings);
+
+            Assert.AreEqual(4, schemaErrors.Count);
+        }
+    }
+}
diff --git a/Src/Newtonsoft.Json.Schema/Infrastructure/Discovery/JSchemaDiscovery.cs b/Src/Newtonsoft.Json.Schema/Infrastructure/Discovery/JSchemaDiscovery.cs
@@ -49,7 +49,7 @@ private void DiscoverInternal(JSchema schema, string latestPath, bool isDefiniti
         {
             // Resolving the current scope from the path stack is a bit of a hack to avoid passing it to each method.
             // Maybe there should be a discover context that gets passed around and dynamicScope is a value on it?
-            dynamicScope ??= GetDynamicScope();
+            dynamicScope ??= schema.DynamicLoadScope ?? GetDynamicScope();
 
             // give schemas that are dependencies a special state so they are written as a dependency and not inline
             KnownSchemaState resolvedSchemaState = (_state == KnownSchemaState.InlinePending && isDefinitionSchema)
@@ -77,13 +77,12 @@ private void DiscoverInternal(JSchema schema, string latestPath, bool isDefiniti
                 return;
             }
 
-            // check whether a schema with the resolved id is already known
-            // this will be hit when a schema contains duplicate ids or references a schema with a duplicate id
-            bool existingSchema = KnownSchemas.GetById(new KnownSchemaUriKey(schemaKnownId, dynamicScope)) != null;
-
-            if (existingSchema)
+            if (ValidationErrors != null)
             {
-                if (ValidationErrors != null)
+                // check whether a schema with the resolved id is already known
+                // this will be hit when a schema contains duplicate ids or references a schema with a duplicate id
+                var existingSchema = KnownSchemas.GetById(new KnownSchemaUriKey(schemaKnownId, dynamicScope)) != null;
+                if (existingSchema)
                 {
                     ValidationError error = ValidationError.CreateValidationError($"Duplicate schema id '{schemaKnownId.OriginalString}' encountered.", ErrorType.Id, schema, null, schemaKnownId, null, schema, schema.Path!);
                     ValidationErrors.Add(error);

diff --git a/Src/Newtonsoft.Json.Schema/Infrastructure/JSchemaReader.cs b/Src/Newtonsoft.Json.Schema/Infrastructure/JSchemaReader.cs
@@ -133,7 +133,8 @@ public void RaiseValidationErrors()
 
                 foreach (ValidationError error in _validationErrors)
                 {
-                    KnownSchema knownSchema = _schemaDiscovery.KnownSchemas.SingleOrDefault(s => s.Schema == error.Schema);
+                    // Get first match because the schema could have been added multiple times with duplicate IDs.
+                    KnownSchema knownSchema = _schemaDiscovery.KnownSchemas.FirstOrDefault(s => s.Schema == error.Schema);
                     if (knownSchema != null)
                     {
                         error.SchemaId = knownSchema.Id;