Skip to content

Commit

Permalink
- fixes a bug where allOf structure with one entry and properties wou…
Browse files Browse the repository at this point in the history 
…ld not be considered as inheritance case

Signed-off-by: Vincent Biret <[email protected]>
  • Loading branch information
@baywet
baywet committed Mar 21, 2024
1 parent 6ed0fc9 commit 7576832
Show file tree
Hide file tree
Showing 5 changed files with 273 additions and 54 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Fixed a bug where multiple Visual Studio Code instances would make the extension install/update fail. [#3686](https://github.com/microsoft/kiota/issues/3686)
- Fixed a bug where models properties named "additionalData" or "backingstore" would be ignored. [#4224](https://github.com/microsoft/kiota/issues/4224)
- Fixed a bug where multiple allOf entries type would not get merged if they were part of a discriminator. [#4325](https://github.com/microsoft/kiota/issues/4325)
- Fixed a bug where allOf structure with one entry and properties would not be considered as inheritance case. [#4346](https://github.com/microsoft/kiota/issues/4346)
- PREVIEW: Renamed the config commands to workspace. [#4310](https://github.com/microsoft/kiota/issues/4310)
- PREVIEW: Moved preview configuration files to the .kiota directory. [#4310](https://github.com/microsoft/kiota/issues/4310)
- PREVIEW: Moved the copy descriptions to dedicated folders. [#4310](https://github.com/microsoft/kiota/issues/4310)
Expand Down
32 changes: 14 additions & 18 deletions src/Kiota.Builder/Extensions/OpenApiSchemaExtensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,41 +10,37 @@ public static class OpenApiSchemaExtensions
{
private static readonly Func<OpenApiSchema, IList<OpenApiSchema>> classNamesFlattener = x =>
(x.AnyOf ?? Enumerable.Empty<OpenApiSchema>()).Union(x.AllOf).Union(x.OneOf).ToList();
public static IEnumerable<string> GetSchemaNames(this OpenApiSchema schema)
public static IEnumerable<string> GetSchemaNames(this OpenApiSchema schema, bool directOnly = false)
{
if (schema == null)
return Enumerable.Empty<string>();
if (schema.Items != null)
return [];
if (!directOnly && schema.Items != null)
return schema.Items.GetSchemaNames();
if (!string.IsNullOrEmpty(schema.Reference?.Id))
return new[] { schema.Reference.Id.Split('/')[^1].Split('.')[^1] };
if (schema.AnyOf.Any())
return [schema.Reference.Id.Split('/')[^1].Split('.')[^1]];
if (!directOnly && schema.AnyOf.Any())
return schema.AnyOf.FlattenIfRequired(classNamesFlattener);
if (schema.AllOf.Any())
if (!directOnly && schema.AllOf.Any())
return schema.AllOf.FlattenIfRequired(classNamesFlattener);
if (schema.OneOf.Any())
if (!directOnly && schema.OneOf.Any())
return schema.OneOf.FlattenIfRequired(classNamesFlattener);
if (!string.IsNullOrEmpty(schema.Title))
return new[] { schema.Title };
if (!string.IsNullOrEmpty(schema.Xml?.Name))
return new[] { schema.Xml.Name };
return Enumerable.Empty<string>();
return [];
}
internal static IEnumerable<OpenApiSchema> FlattenSchemaIfRequired(this IList<OpenApiSchema> schemas, Func<OpenApiSchema, IList<OpenApiSchema>> subsequentGetter)
{
if (schemas is null) return Enumerable.Empty<OpenApiSchema>();
if (schemas is null) return [];
return schemas.Count == 1 ?
schemas.FlattenEmptyEntries(subsequentGetter, 1) :
schemas;
}
private static IEnumerable<string> FlattenIfRequired(this IList<OpenApiSchema> schemas, Func<OpenApiSchema, IList<OpenApiSchema>> subsequentGetter)
{
return schemas.FlattenSchemaIfRequired(subsequentGetter).Where(static x => !string.IsNullOrEmpty(x.Title)).Select(static x => x.Title);
return schemas.FlattenSchemaIfRequired(subsequentGetter).SelectMany(static x => x.GetSchemaNames());
}

public static string GetSchemaName(this OpenApiSchema schema)
public static string GetSchemaName(this OpenApiSchema schema, bool directOnly = false)
{
return schema.GetSchemaNames().LastOrDefault()?.TrimStart('$') ?? string.Empty;// OData $ref
return schema.GetSchemaNames(directOnly).LastOrDefault()?.TrimStart('$') ?? string.Empty;// OData $ref
}

public static bool IsReferencedSchema(this OpenApiSchema schema)
Expand Down Expand Up @@ -173,7 +169,7 @@ public static IEnumerable<string> GetSchemaReferenceIds(this OpenApiSchema schem
return result.Distinct();
}

return Enumerable.Empty<string>();
return [];
}
private static IEnumerable<OpenApiSchema> FlattenEmptyEntries(this IEnumerable<OpenApiSchema> schemas, Func<OpenApiSchema, IList<OpenApiSchema>> subsequentGetter, int? maxDepth = default)
{
Expand All @@ -188,7 +184,7 @@ private static IEnumerable<OpenApiSchema> FlattenEmptyEntries(this IEnumerable<O
foreach (var item in result)
{
var subsequentItems = subsequentGetter(item);
if (string.IsNullOrEmpty(item.Title) && subsequentItems.Any())
if (subsequentItems.Any())
permutations.Add(item, subsequentItems.FlattenEmptyEntries(subsequentGetter, maxDepth.HasValue ? --maxDepth : default));
}
if (permutations.Count > 0)
Expand Down
9 changes: 5 additions & 4 deletions src/Kiota.Builder/KiotaBuilder.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1513,18 +1513,18 @@ private CodeType CreateModelDeclarationAndType(OpenApiUrlTreeNode currentNode, O
}
private CodeType CreateInheritedModelDeclaration(OpenApiUrlTreeNode currentNode, OpenApiSchema schema, OpenApiOperation? operation, string classNameSuffix, CodeNamespace codeNamespace, bool isRequestBody, string typeNameForInlineSchema)
{
var allOfs = (schema.IsSemanticallyMeaningful() ? new OpenApiSchema[] { schema } : []).Union(schema.AllOf.FlattenSchemaIfRequired(static x => x.AllOf));
var rootSchemaIsMeaningful = schema.IsSemanticallyMeaningful();
var allOfs = (rootSchemaIsMeaningful ? new OpenApiSchema[] { schema } : []).Union(schema.AllOf.FlattenSchemaIfRequired(static x => x.AllOf));
CodeElement? codeDeclaration = null;
var className = string.Empty;
var codeNamespaceFromParent = GetShortestNamespace(codeNamespace, schema);
foreach (var currentSchema in allOfs)
{
var referenceId = GetReferenceIdFromOriginalSchema(currentSchema, schema);
var shortestNamespaceName = GetModelsNamespaceNameFromReferenceId(referenceId);
var shortestNamespace = string.IsNullOrEmpty(referenceId) ? codeNamespaceFromParent : rootNamespace?.FindOrAddNamespace(shortestNamespaceName);
className = (currentSchema.GetSchemaName() is string cName && !string.IsNullOrEmpty(cName) ?
var className = (currentSchema.GetSchemaName(rootSchemaIsMeaningful && currentSchema == schema) is string cName && !string.IsNullOrEmpty(cName) ?
cName :
(string.IsNullOrEmpty(className) && !string.IsNullOrEmpty(typeNameForInlineSchema) ?
(!string.IsNullOrEmpty(typeNameForInlineSchema) ?
typeNameForInlineSchema :
currentNode.GetClassName(config.StructuredMimeTypes, operation: operation, suffix: classNameSuffix, schema: currentSchema, requestBody: isRequestBody)))
.CleanupSymbolName();
Expand Down Expand Up @@ -1881,6 +1881,7 @@ private void TrimInheritedModels()
if (relatedModels.Contains(x) || classesInUse.Contains(x)) return;
if (x is CodeClass currentClass)
{
//TODO this is trimming mailboxsettingsbase when it shouldn't. Most likely because one of the indices is now broken

Check warning on line 1884 in src/Kiota.Builder/KiotaBuilder.cs

View workflow job for this annotation

GitHub Actions / Build 

Complete the task associated to this 'TODO' comment. (https://rules.sonarsource.com/csharp/RSPEC-1135)
var parents = currentClass.GetInheritanceTree(false, false);
if (parents.Any(y => classesDirectlyInUse.Contains(y))) return; // to support the inheritance recursive downcast
foreach (var baseClass in parents) // discriminator might also be in grand parent types
Expand Down
187 changes: 155 additions & 32 deletions tests/Kiota.Builder.Tests/Extensions/OpenApiSchemaExtensionsTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,87 +84,179 @@ public void LocalReferencesAreSupported()
Assert.True(mockSchema.IsReferencedSchema());
}
[Fact]
public void GetSchemaNameAllOf()
public void GetSchemaNameAllOfTitleEmpty()
{
var schema = new OpenApiSchema
{
AllOf = new List<OpenApiSchema> {
AllOf = [
new() {
Title = "microsoft.graph.entity"
},
new() {
Title = "microsoft.graph.user"
}
}
]
};
var names = schema.GetSchemaNames();
Assert.Contains("microsoft.graph.entity", names);
Assert.Contains("microsoft.graph.user", names);
Assert.Equal("microsoft.graph.user", schema.GetSchemaName());
Assert.Empty(names);
Assert.Empty(schema.GetSchemaName());
}
[Fact]
public void GetSchemaNameAllOfNested()
public void GetSchemaNameAllOfReference()
{
var schema = new OpenApiSchema
{
AllOf = new List<OpenApiSchema> {
AllOf = [
new() {
AllOf = new List<OpenApiSchema> {
Reference = new() {
Id = "microsoft.graph.entity"
}
},
new() {
Reference = new() {
Id = "microsoft.graph.user"
}
}
]
};
var names = schema.GetSchemaNames();
Assert.Contains("entity", names);
Assert.Contains("user", names);
Assert.Equal("user", schema.GetSchemaName());
}
[Fact]
public void GetSchemaNameAllOfNestedTitleEmpty()
{
var schema = new OpenApiSchema
{
AllOf = [
new() {
AllOf = [
new() {
Title = "microsoft.graph.entity"
},
new() {
Title = "microsoft.graph.user"
}
}
]
}
}
]
};
var names = schema.GetSchemaNames();
Assert.Contains("microsoft.graph.entity", names);
Assert.Contains("microsoft.graph.user", names);
Assert.Equal("microsoft.graph.user", schema.GetSchemaName());
Assert.Empty(names);
Assert.Empty(schema.GetSchemaName());
}
[Fact]
public void GetSchemaNameAnyOf()
public void GetSchemaNameAllOfNestedReference()
{
var schema = new OpenApiSchema
{
AnyOf = new List<OpenApiSchema> {
AllOf = [
new() {
AllOf = [
new() {
Reference = new() {
Id = "microsoft.graph.entity"
}
},
new() {
Reference = new() {
Id = "microsoft.graph.user"
}
}
]
}
]
};
var names = schema.GetSchemaNames();
Assert.Contains("entity", names);
Assert.Contains("user", names);
Assert.Equal("user", schema.GetSchemaName());
}
[Fact]
public void GetSchemaNameAnyOfTitleEmpty()
{
var schema = new OpenApiSchema
{
AnyOf = [
new() {
Title = "microsoft.graph.entity"
},
new() {
Title = "microsoft.graph.user"
}
}
]
};
var names = schema.GetSchemaNames();
Assert.Contains("microsoft.graph.entity", names);
Assert.Contains("microsoft.graph.user", names);
Assert.Equal("microsoft.graph.user", schema.GetSchemaName());
Assert.Empty(names);
Assert.Empty(schema.GetSchemaName());
}
[Fact]
public void GetSchemaNameOneOf()
public void GetSchemaNameAnyOfReference()
{
var schema = new OpenApiSchema
{
OneOf = new List<OpenApiSchema> {
AnyOf = [
new() {
Reference = new() {
Id = "microsoft.graph.entity"
}
},
new() {
Reference = new() {
Id = "microsoft.graph.user"
}
}
]
};
var names = schema.GetSchemaNames();
Assert.Contains("entity", names);
Assert.Contains("user", names);
Assert.Equal("user", schema.GetSchemaName());
}
[Fact]
public void GetSchemaNameOneOfTitleEmpty()
{
var schema = new OpenApiSchema
{
OneOf = [
new() {
Title = "microsoft.graph.entity"
},
new() {
Title = "microsoft.graph.user"
}
}
]
};
var names = schema.GetSchemaNames();
Assert.Contains("microsoft.graph.entity", names);
Assert.Contains("microsoft.graph.user", names);
Assert.Equal("microsoft.graph.user", schema.GetSchemaName());
Assert.Empty(names);
Assert.Empty(schema.GetSchemaName());
}
[Fact]
public void GetSchemaNameOneOfReference()
{
var schema = new OpenApiSchema
{
OneOf = [
new() {
Reference = new() {
Id = "microsoft.graph.entity"
}
},
new() {
Reference = new() {
Id = "microsoft.graph.user"
}
}
]
};
var names = schema.GetSchemaNames();
Assert.Contains("entity", names);
Assert.Contains("user", names);
Assert.Equal("user", schema.GetSchemaName());
}
[Fact]
public void GetSchemaNameItems()
public void GetSchemaNameItemsTitleEmpty()
{
var schema = new OpenApiSchema
{
Expand All @@ -174,20 +266,51 @@ public void GetSchemaNameItems()
},
};
var names = schema.GetSchemaNames();
Assert.Contains("microsoft.graph.entity", names);
Assert.Equal("microsoft.graph.entity", schema.GetSchemaName());
Assert.Empty(names);
Assert.Empty(schema.GetSchemaName());
}
[Fact]
public void GetSchemaNameItemsReference()
{
var schema = new OpenApiSchema
{
Items = new()
{
Reference = new()
{
Id = "microsoft.graph.entity"
}
},
};
var names = schema.GetSchemaNames();
Assert.Contains("entity", names);
Assert.Equal("entity", schema.GetSchemaName());
Assert.Single(names);
}
[Fact]
public void GetSchemaNameTitle()
public void GetSchemaNameTitleEmpty()
{
var schema = new OpenApiSchema
{
Title = "microsoft.graph.entity"
};
var names = schema.GetSchemaNames();
Assert.Contains("microsoft.graph.entity", names);
Assert.Equal("microsoft.graph.entity", schema.GetSchemaName());
Assert.Empty(names);
Assert.Empty(schema.GetSchemaName());
}
[Fact]
public void GetSchemaNameReference()
{
var schema = new OpenApiSchema
{
Reference = new()
{
Id = "microsoft.graph.entity"
}
};
var names = schema.GetSchemaNames();
Assert.Contains("entity", names);
Assert.Equal("entity", schema.GetSchemaName());
Assert.Single(names);
}
[Fact]
Expand Down
Loading

0 comments on commit 7576832

Please sign in to comment.