Skip to content

Commit

Permalink
- fixes a bug where multiple allOf entries type would not get merged …
Browse files Browse the repository at this point in the history
…if they were part of a discriminator
  • Loading branch information
baywet committed Mar 21, 2024
1 parent 0ba86dc commit 6ed0fc9
Show file tree
Hide file tree
Showing 4 changed files with 101 additions and 14 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,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)
- 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
4 changes: 3 additions & 1 deletion src/Kiota.Builder/Extensions/OpenApiSchemaExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,9 @@ public static bool IsInherited(this OpenApiSchema? schema)
{
if (schema is null) return false;
var meaningfulSchemas = schema.AllOf.FlattenSchemaIfRequired(static x => x.AllOf).Where(static x => x.IsSemanticallyMeaningful()).ToArray();
return meaningfulSchemas.Count(static x => !string.IsNullOrEmpty(x.Reference?.Id)) == 1 && meaningfulSchemas.Count(static x => string.IsNullOrEmpty(x.Reference?.Id)) == 1;
return meaningfulSchemas.Count(static x => !string.IsNullOrEmpty(x.Reference?.Id)) == 1 &&
(meaningfulSchemas.Count(static x => string.IsNullOrEmpty(x.Reference?.Id)) == 1 ||
schema.IsSemanticallyMeaningful());
}

internal static OpenApiSchema? MergeIntersectionSchemaEntries(this OpenApiSchema? schema)
Expand Down
18 changes: 13 additions & 5 deletions src/Kiota.Builder/KiotaBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1511,9 +1511,9 @@ private CodeType CreateModelDeclarationAndType(OpenApiUrlTreeNode currentNode, O
TypeDefinition = codeDeclaration,
};
}
private CodeType CreateInheritedModelDeclaration(OpenApiUrlTreeNode currentNode, OpenApiSchema schema, OpenApiOperation? operation, string classNameSuffix, CodeNamespace codeNamespace, bool isRequestBody)
private CodeType CreateInheritedModelDeclaration(OpenApiUrlTreeNode currentNode, OpenApiSchema schema, OpenApiOperation? operation, string classNameSuffix, CodeNamespace codeNamespace, bool isRequestBody, string typeNameForInlineSchema)
{
var allOfs = schema.AllOf.FlattenSchemaIfRequired(static x => x.AllOf);
var allOfs = (schema.IsSemanticallyMeaningful() ? new OpenApiSchema[] { schema } : []).Union(schema.AllOf.FlattenSchemaIfRequired(static x => x.AllOf));
CodeElement? codeDeclaration = null;
var className = string.Empty;
var codeNamespaceFromParent = GetShortestNamespace(codeNamespace, schema);
Expand All @@ -1524,7 +1524,9 @@ private CodeType CreateInheritedModelDeclaration(OpenApiUrlTreeNode currentNode,
var shortestNamespace = string.IsNullOrEmpty(referenceId) ? codeNamespaceFromParent : rootNamespace?.FindOrAddNamespace(shortestNamespaceName);
className = (currentSchema.GetSchemaName() is string cName && !string.IsNullOrEmpty(cName) ?
cName :
currentNode.GetClassName(config.StructuredMimeTypes, operation: operation, suffix: classNameSuffix, schema: schema, requestBody: isRequestBody))
(string.IsNullOrEmpty(className) && !string.IsNullOrEmpty(typeNameForInlineSchema) ?
typeNameForInlineSchema :
currentNode.GetClassName(config.StructuredMimeTypes, operation: operation, suffix: classNameSuffix, schema: currentSchema, requestBody: isRequestBody)))
.CleanupSymbolName();
if (shortestNamespace != null)
codeDeclaration = AddModelDeclarationIfDoesntExist(currentNode, currentSchema, className, shortestNamespace, codeDeclaration as CodeClass);
Expand Down Expand Up @@ -1635,7 +1637,7 @@ private CodeTypeBase CreateModelDeclarations(OpenApiUrlTreeNode currentNode, Ope

if (schema.IsInherited())
{
return CreateInheritedModelDeclaration(currentNode, schema, operation, suffix, codeNamespace, isRequestBody);
return CreateInheritedModelDeclaration(currentNode, schema, operation, suffix, codeNamespace, isRequestBody, typeNameForInlineSchema);
}

if (schema.IsIntersection() && schema.MergeIntersectionSchemaEntries() is OpenApiSchema mergedSchema)
Expand Down Expand Up @@ -1783,8 +1785,9 @@ private CodeNamespace GetShortestNamespace(CodeNamespace currentNamespace, OpenA
}
private CodeClass AddModelClass(OpenApiUrlTreeNode currentNode, OpenApiSchema schema, string declarationName, CodeNamespace currentNamespace, CodeClass? inheritsFrom = null)
{
if (inheritsFrom == null && schema.AllOf.FirstOrDefault(static x => x.Reference != null) is OpenApiSchema parentSchema)
if (inheritsFrom == null && schema.AllOf.Where(static x => x.Reference != null).ToArray() is { Length: 1 } referencedSchemas)
{// any non-reference would be the current class in some description styles
var parentSchema = referencedSchemas[0];
var parentClassNamespace = GetShortestNamespace(currentNamespace, parentSchema);
inheritsFrom = (CodeClass)AddModelDeclarationIfDoesntExist(currentNode, parentSchema, parentSchema.GetSchemaName().CleanupSymbolName(), parentClassNamespace);
}
Expand Down Expand Up @@ -2027,6 +2030,11 @@ internal static void AddDiscriminatorMethod(CodeClass newClass, string discrimin
}
var className = currentNode.GetClassName(config.StructuredMimeTypes, schema: discriminatorSchema).CleanupSymbolName();
var shouldInherit = discriminatorSchema.AllOf.Any(x => currentSchema.Reference?.Id.Equals(x.Reference?.Id, StringComparison.OrdinalIgnoreCase) ?? false);
if (baseClass is not null && !discriminatorSchema.IsInherited())
{
logger.LogWarning("Discriminator {ComponentKey} is not inherited from {ClassName}.", componentKey, baseClass.Name);
return null;
}
var codeClass = AddModelDeclarationIfDoesntExist(currentNode, discriminatorSchema, className, GetShortestNamespace(currentNamespace, discriminatorSchema), shouldInherit ? baseClass : null);
return new CodeType
{
Expand Down
92 changes: 84 additions & 8 deletions tests/Kiota.Builder.Tests/KiotaBuilderTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1810,7 +1810,7 @@ public void Inline_Property_Inheritance_Is_Supported()
Type = "object",
Properties = new Dictionary<string, OpenApiSchema> {
{
"info", new OpenApiSchema {
"info2", new OpenApiSchema {
Type = "object",
Properties = new Dictionary<string, OpenApiSchema> {
{
Expand Down Expand Up @@ -1855,12 +1855,12 @@ public void Inline_Property_Inheritance_Is_Supported()
var itemsNS = codeModel.FindNamespaceByName("ApiSdk.resource.item");
var responseClass = itemsNS.FindChildByName<CodeClass>("ResourceGetResponse");
var derivedResourceClass = itemsNS.FindChildByName<CodeClass>("ResourceGetResponse_derivedResource");
var derivedResourceInfoClass = itemsNS.FindChildByName<CodeClass>("ResourceGetResponse_derivedResource_info");
var derivedResourceInfoClass = itemsNS.FindChildByName<CodeClass>("ResourceGetResponse_derivedResource_info2");


Assert.NotNull(resourceClass);
Assert.NotNull(derivedResourceClass);
Assert.NotNull(derivedResourceClass.StartBlock);
Assert.NotNull(derivedResourceClass.StartBlock.Inherits);
Assert.Equal(derivedResourceClass.StartBlock.Inherits.TypeDefinition, resourceClass);
Assert.NotNull(derivedResourceInfoClass);
Assert.NotNull(responseClass);
Expand Down Expand Up @@ -7328,7 +7328,83 @@ public async Task InheritanceWithAllOfInBaseType()
Assert.NotNull(codeModel.FindChildByName<CodeClass>("Group"));
}
[Fact]
public async Task InheritanceWithAllOfWith3Parts()
public async Task InheritanceWithAllOfWith3Parts3Schema()
{
var tempFilePath = Path.Combine(Path.GetTempPath(), Path.GetTempFileName());
await using var fs = await GetDocumentStream(@"openapi: 3.0.1
info:
title: OData Service for namespace microsoft.graph
description: This OData service is located at https://graph.microsoft.com/v1.0
version: 1.0.1
servers:
- url: https://graph.microsoft.com/v1.0
paths:
/directoryObject:
get:
responses:
'200':
content:
application/json:
schema:
$ref: '#/components/schemas/microsoft.graph.directoryObject'
/group:
get:
responses:
'200':
content:
application/json:
schema:
$ref: '#/components/schemas/microsoft.graph.group'
components:
schemas:
microsoft.graph.directoryObject:
title: 'directoryObject'
required: ['@odata.type']
type: 'object'
properties:
'@odata.type':
type: 'string'
default: '#microsoft.graph.directoryObject'
discriminator:
propertyName: '@odata.type'
mapping:
'#microsoft.graph.group': '#/components/schemas/microsoft.graph.group'
microsoft.graph.groupFacet1:
title: 'group part 1'
type: 'object'
properties:
groupprop1:
type: 'string'
microsoft.graph.groupFacet2:
title: 'group part 2'
type: 'object'
properties:
groupprop2:
type: 'string'
microsoft.graph.group:
title: 'group'
allOf:
- '$ref': '#/components/schemas/microsoft.graph.directoryObject'
- '$ref': '#/components/schemas/microsoft.graph.groupFacet1'
- '$ref': '#/components/schemas/microsoft.graph.groupFacet2'");
var mockLogger = new Mock<ILogger<KiotaBuilder>>();
var builder = new KiotaBuilder(mockLogger.Object, new GenerationConfiguration { ClientClassName = "Graph", OpenAPIFilePath = tempFilePath }, _httpClient);
var document = await builder.CreateOpenApiDocumentAsync(fs);
var node = builder.CreateUriSpace(document);
var codeModel = builder.CreateSourceModel(node);
var directoryObjectClass = codeModel.FindChildByName<CodeClass>("DirectoryObject");
Assert.NotNull(directoryObjectClass);
var resultClass = codeModel.FindChildByName<CodeClass>("Group");
Assert.NotNull(resultClass);
Assert.Equal(4, resultClass.Properties.Count());
Assert.Null(resultClass.StartBlock.Inherits);
Assert.Single(resultClass.Properties.Where(static x => x.Kind is CodePropertyKind.AdditionalData));
Assert.Single(resultClass.Properties.Where(static x => x.Name.Equals("oDataType", StringComparison.OrdinalIgnoreCase)));
Assert.Single(resultClass.Properties.Where(static x => x.Name.Equals("groupprop1", StringComparison.OrdinalIgnoreCase)));
Assert.Single(resultClass.Properties.Where(static x => x.Name.Equals("groupprop2", StringComparison.OrdinalIgnoreCase)));
}
[Fact]
public async Task InheritanceWithAllOfWith3Parts1Schema2Inline()
{
var tempFilePath = Path.Combine(Path.GetTempPath(), Path.GetTempFileName());
await using var fs = await GetDocumentStream(@"openapi: 3.0.1
Expand All @@ -7343,7 +7419,6 @@ public async Task InheritanceWithAllOfWith3Parts()
get:
responses:
'200':
description: Example response
content:
application/json:
schema:
Expand All @@ -7361,7 +7436,6 @@ public async Task InheritanceWithAllOfWith3Parts()
discriminator:
propertyName: '@odata.type'
mapping:
'#microsoft.graph.user': '#/components/schemas/microsoft.graph.user'
'#microsoft.graph.group': '#/components/schemas/microsoft.graph.group'
microsoft.graph.group:
allOf:
Expand All @@ -7383,9 +7457,11 @@ public async Task InheritanceWithAllOfWith3Parts()
var codeModel = builder.CreateSourceModel(node);
var resultClass = codeModel.FindChildByName<CodeClass>("Group");
Assert.NotNull(resultClass);
Assert.Equal("directoryObject", resultClass.StartBlock.Inherits?.Name, StringComparer.OrdinalIgnoreCase);
Assert.Equal(2, resultClass.Properties.Count());
Assert.Single(resultClass.Properties.Where(x => x.Name.Equals("groupprop1", StringComparison.OrdinalIgnoreCase)));
Assert.Single(resultClass.Properties.Where(x => x.Name.Equals("groupprop2", StringComparison.OrdinalIgnoreCase)));
Assert.Empty(resultClass.Properties.Where(static x => x.Name.Equals("oDataType", StringComparison.OrdinalIgnoreCase)));
Assert.Single(resultClass.Properties.Where(static x => x.Name.Equals("groupprop1", StringComparison.OrdinalIgnoreCase)));
Assert.Single(resultClass.Properties.Where(static x => x.Name.Equals("groupprop2", StringComparison.OrdinalIgnoreCase)));
}
[Fact]
public async Task EnumsWithNullableDoesNotResultInInlineType()
Expand Down

0 comments on commit 6ed0fc9

Please sign in to comment.