diff --git a/Src/Newtonsoft.Json.Schema.Tests/Issues/Issue0339Tests.cs b/Src/Newtonsoft.Json.Schema.Tests/Issues/Issue0339Tests.cs new file mode 100644 index 00000000..7da40e6a --- /dev/null +++ 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 schemaErrors = new List(); + 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 schemaErrors = new List(); + 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 index 2b37ab94..fb674065 100644 --- 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 index 2fcf0bd0..54ad6166 100644 --- 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;