From 5f974470281be5a91231c8c5571462df829174d1 Mon Sep 17 00:00:00 2001 From: TimLovellSmith Date: Mon, 23 May 2016 11:01:27 -0700 Subject: [PATCH] Modify the circular detection logic for circular 'allOf' inheritance so that there can again be a passing test 'ClientModelWithCircularDependencyThrowsError'. --- .../Swagger.Tests/SwaggerModelerTests.cs | 4 ++- .../Swagger/Properties/Resources.Designer.cs | 9 ++++++ .../Swagger/Properties/Resources.resx | 3 ++ AutoRest/Modelers/Swagger/SchemaResolver.cs | 31 +++++++++++++++++++ 4 files changed, 46 insertions(+), 1 deletion(-) diff --git a/AutoRest/Modelers/Swagger.Tests/SwaggerModelerTests.cs b/AutoRest/Modelers/Swagger.Tests/SwaggerModelerTests.cs index 0aac57a71e45f..8c7af5c952207 100644 --- a/AutoRest/Modelers/Swagger.Tests/SwaggerModelerTests.cs +++ b/AutoRest/Modelers/Swagger.Tests/SwaggerModelerTests.cs @@ -185,7 +185,9 @@ public void ClientModelWithCircularDependencyThrowsError() Namespace = "Test", Input = Path.Combine("Swagger", "swagger-allOf-circular.json") }); - Assert.Throws(() => modeler.Build()); + var ex = Assert.Throws(() => modeler.Build()); + Assert.Contains("circular", ex.Message, StringComparison.InvariantCultureIgnoreCase); + Assert.Contains("siamese", ex.Message, StringComparison.InvariantCultureIgnoreCase); } [Fact] diff --git a/AutoRest/Modelers/Swagger/Properties/Resources.Designer.cs b/AutoRest/Modelers/Swagger/Properties/Resources.Designer.cs index e2335bc14a76a..bb600eb0be9a3 100644 --- a/AutoRest/Modelers/Swagger/Properties/Resources.Designer.cs +++ b/AutoRest/Modelers/Swagger/Properties/Resources.Designer.cs @@ -60,6 +60,15 @@ internal Resources() { } } + /// + /// Looks up a localized string similar to Found a type set '{0}' which is circularly defined.. + /// + internal static string CircularBaseSchemaSet { + get { + return ResourceManager.GetString("CircularBaseSchemaSet", resourceCulture); + } + } + /// /// Looks up a localized string similar to Circular reference detected: {0}. /// diff --git a/AutoRest/Modelers/Swagger/Properties/Resources.resx b/AutoRest/Modelers/Swagger/Properties/Resources.resx index 057ff414a1b2e..18588c7695845 100644 --- a/AutoRest/Modelers/Swagger/Properties/Resources.resx +++ b/AutoRest/Modelers/Swagger/Properties/Resources.resx @@ -117,6 +117,9 @@ System.Resources.ResXResourceWriter, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089 + + Found a type set '{0}' which is circularly defined. + Circular reference detected: {0} diff --git a/AutoRest/Modelers/Swagger/SchemaResolver.cs b/AutoRest/Modelers/Swagger/SchemaResolver.cs index 6fbf3603c69ec..562b51e737d12 100644 --- a/AutoRest/Modelers/Swagger/SchemaResolver.cs +++ b/AutoRest/Modelers/Swagger/SchemaResolver.cs @@ -82,7 +82,9 @@ public void ExpandAllOf(Schema schema) if (schema.AllOf != null) { + CheckCircularAllOf(schema, null, null); var references = schema.AllOf.Where(s => s.Reference != null).ToList(); + if (references.Count == 1) { if (schema.Extends != null) @@ -101,6 +103,7 @@ public void ExpandAllOf(Schema schema) Properties = schema.Properties }; + var schemaList = new List().Concat(schema.AllOf) .Concat(new List {propertiesOnlySchema}); @@ -170,6 +173,34 @@ public void ExpandAllOf(Schema schema) } } + void CheckCircularAllOf(Schema schema, HashSet visited, Stack referenceChain) + { + visited = visited ?? new HashSet(); + referenceChain = referenceChain ?? new Stack(); + if (!visited.Add(schema)) // was already present in the set + { + var setDescription = "(" + String.Join(", ", referenceChain) + ")"; + throw new InvalidOperationException( + string.Format(CultureInfo.InvariantCulture, + Properties.Resources.CircularBaseSchemaSet, setDescription)); + } + + if (schema.AllOf != null) + { + foreach (var reference in schema.AllOf.Select(s => s.Reference).Where(r => r != null)) + { + referenceChain.Push(reference); + + var deref = Dereference(reference); + CheckCircularAllOf(deref, visited, referenceChain); + + Debug.Assert(reference == referenceChain.Peek()); + referenceChain.Pop(); + } + } + visited.Remove(schema); + } + /// /// Determine equivalence between the types described by two schemas. /// Limit the comparison to exclude comparison of complexe inline schemas.