From ce3355092ff6415bc054455d1662ba182fd33c0b Mon Sep 17 00:00:00 2001 From: Vincent Biret Date: Thu, 27 May 2021 10:24:08 -0400 Subject: [PATCH 1/3] - fixes a bug where a null schema would make the generation process derail --- src/Kiota.Builder/Extensions/OpenApiSchemaExtensions.cs | 2 +- .../Extensions/OpenApiSchemaExtensionsTests.cs | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/Kiota.Builder/Extensions/OpenApiSchemaExtensions.cs b/src/Kiota.Builder/Extensions/OpenApiSchemaExtensions.cs index 9e834047d5..f15c88cd00 100644 --- a/src/Kiota.Builder/Extensions/OpenApiSchemaExtensions.cs +++ b/src/Kiota.Builder/Extensions/OpenApiSchemaExtensions.cs @@ -34,7 +34,7 @@ public static string GetSchemaTitle(this OpenApiSchema schema) { public static IEnumerable GetSchemaReferenceIds(this OpenApiSchema schema, HashSet visitedSchemas = null) { if(visitedSchemas == null) visitedSchemas = new(); - if(!visitedSchemas.Contains(schema)) { + if(schema != null && !visitedSchemas.Contains(schema)) { visitedSchemas.Add(schema); var result = new List(); if(!string.IsNullOrEmpty(schema.Reference?.Id)) diff --git a/tests/Kiota.Builder.Tests/Extensions/OpenApiSchemaExtensionsTests.cs b/tests/Kiota.Builder.Tests/Extensions/OpenApiSchemaExtensionsTests.cs index 69d94cf5b4..879afd1372 100644 --- a/tests/Kiota.Builder.Tests/Extensions/OpenApiSchemaExtensionsTests.cs +++ b/tests/Kiota.Builder.Tests/Extensions/OpenApiSchemaExtensionsTests.cs @@ -5,6 +5,10 @@ namespace Kiota.Builder.Extensions.Tests { public class OpenApiSchemaExtensionsTests { + [Fact] + public void Defensive() { + Assert.Empty(OpenApiSchemaExtensions.GetSchemaReferenceIds(null)); + } [Fact] public void GetSchemaTitleAllOf() { var schema = new OpenApiSchema { From 7e115ecdaeb8d27d6e12587b8ad725554ef4a501 Mon Sep 17 00:00:00 2001 From: Vincent Biret Date: Thu, 27 May 2021 10:48:57 -0400 Subject: [PATCH 2/3] - reduces get schemas reference ids complexity --- .../Extensions/OpenApiSchemaExtensions.cs | 16 ++++---- .../OpenApiSchemaExtensionsTests.cs | 40 +++++++++---------- 2 files changed, 28 insertions(+), 28 deletions(-) diff --git a/src/Kiota.Builder/Extensions/OpenApiSchemaExtensions.cs b/src/Kiota.Builder/Extensions/OpenApiSchemaExtensions.cs index f15c88cd00..7d316743e1 100644 --- a/src/Kiota.Builder/Extensions/OpenApiSchemaExtensions.cs +++ b/src/Kiota.Builder/Extensions/OpenApiSchemaExtensions.cs @@ -41,14 +41,14 @@ public static IEnumerable GetSchemaReferenceIds(this OpenApiSchema schem result.Add(schema.Reference.Id); if(!string.IsNullOrEmpty(schema.Items?.Reference?.Id)) result.Add(schema.Items.Reference.Id); - if(schema.Properties != null) - result.AddRange(schema.Properties.Values.SelectMany(x => x.GetSchemaReferenceIds(visitedSchemas))); - if(schema.AnyOf != null) - result.AddRange(schema.AnyOf.SelectMany(x => x.GetSchemaReferenceIds(visitedSchemas))); - if(schema.AllOf != null) - result.AddRange(schema.AllOf.SelectMany(x => x.GetSchemaReferenceIds(visitedSchemas))); - if(schema.OneOf != null) - result.AddRange(schema.OneOf.SelectMany(x => x.GetSchemaReferenceIds(visitedSchemas))); + var subSchemaReferences = (schema.Properties?.Values ?? Enumerable.Empty()) + .Union(schema.AnyOf ?? Enumerable.Empty()) + .Union(schema.AllOf ?? Enumerable.Empty()) + .Union(schema.OneOf ?? Enumerable.Empty()) + .SelectMany(x => x.GetSchemaReferenceIds(visitedSchemas)) + .ToList();// this to list is important otherwise the any marks the schemas as visited and add range doesn't find anything + if(subSchemaReferences.Any()) + result.AddRange(subSchemaReferences); return result; } else return Enumerable.Empty(); diff --git a/tests/Kiota.Builder.Tests/Extensions/OpenApiSchemaExtensionsTests.cs b/tests/Kiota.Builder.Tests/Extensions/OpenApiSchemaExtensionsTests.cs index 879afd1372..eea256a627 100644 --- a/tests/Kiota.Builder.Tests/Extensions/OpenApiSchemaExtensionsTests.cs +++ b/tests/Kiota.Builder.Tests/Extensions/OpenApiSchemaExtensionsTests.cs @@ -22,8 +22,8 @@ public void GetSchemaTitleAllOf() { } }; var names = schema.GetSchemaTitles(); - Assert.Equal("microsoft.graph.entity", names.First()); - Assert.Equal("microsoft.graph.user", names.Last()); + Assert.Contains("microsoft.graph.entity", names); + Assert.Contains("microsoft.graph.user", names); Assert.Equal("microsoft.graph.user", schema.GetSchemaTitle()); } [Fact] @@ -43,8 +43,8 @@ public void GetSchemaTitleAllOfNested() { } }; var names = schema.GetSchemaTitles(); - Assert.Equal("microsoft.graph.entity", names.First()); - Assert.Equal("microsoft.graph.user", names.Last()); + Assert.Contains("microsoft.graph.entity", names); + Assert.Contains("microsoft.graph.user", names); Assert.Equal("microsoft.graph.user", schema.GetSchemaTitle()); } [Fact] @@ -60,8 +60,8 @@ public void GetSchemaTitleAnyOf() { } }; var names = schema.GetSchemaTitles(); - Assert.Equal("microsoft.graph.entity", names.First()); - Assert.Equal("microsoft.graph.user", names.Last()); + Assert.Contains("microsoft.graph.entity", names); + Assert.Contains("microsoft.graph.user", names); Assert.Equal("microsoft.graph.user", schema.GetSchemaTitle()); } [Fact] @@ -77,8 +77,8 @@ public void GetSchemaTitleOneOf() { } }; var names = schema.GetSchemaTitles(); - Assert.Equal("microsoft.graph.entity", names.First()); - Assert.Equal("microsoft.graph.user", names.Last()); + Assert.Contains("microsoft.graph.entity", names); + Assert.Contains("microsoft.graph.user", names); Assert.Equal("microsoft.graph.user", schema.GetSchemaTitle()); } [Fact] @@ -89,7 +89,7 @@ public void GetSchemaTitleItems() { }, }; var names = schema.GetSchemaTitles(); - Assert.Equal("microsoft.graph.entity", names.First()); + Assert.Contains("microsoft.graph.entity", names); Assert.Equal("microsoft.graph.entity", schema.GetSchemaTitle()); Assert.Single(names); } @@ -99,7 +99,7 @@ public void GetSchemaTitleTitle() { Title = "microsoft.graph.entity" }; var names = schema.GetSchemaTitles(); - Assert.Equal("microsoft.graph.entity", names.First()); + Assert.Contains("microsoft.graph.entity", names); Assert.Equal("microsoft.graph.entity", schema.GetSchemaTitle()); Assert.Single(names); } @@ -128,8 +128,8 @@ public void GetReferenceIdsAllOf() { } }; var names = schema.GetSchemaReferenceIds(); - Assert.Equal("microsoft.graph.entity", names.First()); - Assert.Equal("microsoft.graph.user", names.Last()); + Assert.Contains("microsoft.graph.entity", names); + Assert.Contains("microsoft.graph.user", names); } [Fact] public void GetReferenceIdsAllOfNested() { @@ -152,8 +152,8 @@ public void GetReferenceIdsAllOfNested() { } }; var names = schema.GetSchemaReferenceIds(); - Assert.Equal("microsoft.graph.entity", names.First()); - Assert.Equal("microsoft.graph.user", names.Last()); + Assert.Contains("microsoft.graph.entity", names); + Assert.Contains("microsoft.graph.user", names); } [Fact] public void GetReferenceIdsAnyOf() { @@ -172,8 +172,8 @@ public void GetReferenceIdsAnyOf() { } }; var names = schema.GetSchemaReferenceIds(); - Assert.Equal("microsoft.graph.entity", names.First()); - Assert.Equal("microsoft.graph.user", names.Last()); + Assert.Contains("microsoft.graph.entity", names); + Assert.Contains("microsoft.graph.user", names); } [Fact] public void GetReferenceIdsOneOf() { @@ -192,8 +192,8 @@ public void GetReferenceIdsOneOf() { } }; var names = schema.GetSchemaReferenceIds(); - Assert.Equal("microsoft.graph.entity", names.First()); - Assert.Equal("microsoft.graph.user", names.Last()); + Assert.Contains("microsoft.graph.entity", names); + Assert.Contains("microsoft.graph.user", names); } [Fact] public void GetReferenceIdsItems() { @@ -205,7 +205,7 @@ public void GetReferenceIdsItems() { }, }; var names = schema.GetSchemaReferenceIds(); - Assert.Equal("microsoft.graph.entity", names.First()); + Assert.Contains("microsoft.graph.entity", names); Assert.Single(names); } [Fact] @@ -216,7 +216,7 @@ public void GetReferenceIdsTitle() { } }; var names = schema.GetSchemaReferenceIds(); - Assert.Equal("microsoft.graph.entity", names.First()); + Assert.Contains("microsoft.graph.entity", names); Assert.Single(names); } [Fact] From 386219f45d15c19ad1390a3e68f61a349115d47c Mon Sep 17 00:00:00 2001 From: Vincent Biret Date: Thu, 27 May 2021 11:15:17 -0400 Subject: [PATCH 3/3] - improves coverage of defensive programing --- .../OpenApiSchemaExtensionsTests.cs | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/tests/Kiota.Builder.Tests/Extensions/OpenApiSchemaExtensionsTests.cs b/tests/Kiota.Builder.Tests/Extensions/OpenApiSchemaExtensionsTests.cs index eea256a627..8fd7ac0f5d 100644 --- a/tests/Kiota.Builder.Tests/Extensions/OpenApiSchemaExtensionsTests.cs +++ b/tests/Kiota.Builder.Tests/Extensions/OpenApiSchemaExtensionsTests.cs @@ -8,6 +8,26 @@ public class OpenApiSchemaExtensionsTests { [Fact] public void Defensive() { Assert.Empty(OpenApiSchemaExtensions.GetSchemaReferenceIds(null)); + var schema = new OpenApiSchema{ + AnyOf = null + }; + Assert.Null(schema.AnyOf); + Assert.Empty(schema.GetSchemaReferenceIds()); + schema = new() { + AllOf = null + }; + Assert.Null(schema.AllOf); + Assert.Empty(schema.GetSchemaReferenceIds()); + schema = new() { + OneOf = null + }; + Assert.Null(schema.OneOf); + Assert.Empty(schema.GetSchemaReferenceIds()); + schema = new() { + Properties = null + }; + Assert.Null(schema.Properties); + Assert.Empty(schema.GetSchemaReferenceIds()); } [Fact] public void GetSchemaTitleAllOf() {