Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Always inherit from base class (don't squash) when oneOf ref'd via discriminator #5123

Merged
merged 12 commits into from
Aug 13, 2024
4 changes: 2 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Changed

- Reverts modification of responses in output openApi file when generating plugings [#4945](https://github.com/microsoft/kiota/issues/4945)
- Fixed an issue where models would be missing when they had no properties and a single allOf entry. [#5014](https://github.com/microsoft/kiota/issues/5014)
- Reverts modification of responses in output openApi file when generating plugins [#4945](https://github.com/microsoft/kiota/issues/4945)

## [1.17.0] - 2024-08-09

Expand Down Expand Up @@ -1414,4 +1415,3 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Added

- Initial GitHub release

50 changes: 30 additions & 20 deletions src/Kiota.Builder/KiotaBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1599,14 +1599,14 @@ private CodeType CreateModelDeclarationAndType(OpenApiUrlTreeNode currentNode, O
TypeDefinition = codeDeclaration,
};
}
private CodeType CreateInheritedModelDeclarationAndType(OpenApiUrlTreeNode currentNode, OpenApiSchema schema, OpenApiOperation? operation, string classNameSuffix, CodeNamespace codeNamespace, bool isRequestBody, string typeNameForInlineSchema)
private CodeType CreateInheritedModelDeclarationAndType(OpenApiUrlTreeNode currentNode, OpenApiSchema schema, OpenApiOperation? operation, string classNameSuffix, CodeNamespace codeNamespace, bool isRequestBody, string typeNameForInlineSchema, bool isViaDiscriminator = false)
{
return new CodeType
{
TypeDefinition = CreateInheritedModelDeclaration(currentNode, schema, operation, classNameSuffix, codeNamespace, isRequestBody, typeNameForInlineSchema),
TypeDefinition = CreateInheritedModelDeclaration(currentNode, schema, operation, classNameSuffix, codeNamespace, isRequestBody, typeNameForInlineSchema, isViaDiscriminator),
};
}
private CodeClass CreateInheritedModelDeclaration(OpenApiUrlTreeNode currentNode, OpenApiSchema schema, OpenApiOperation? operation, string classNameSuffix, CodeNamespace codeNamespace, bool isRequestBody, string typeNameForInlineSchema)
private CodeClass CreateInheritedModelDeclaration(OpenApiUrlTreeNode currentNode, OpenApiSchema schema, OpenApiOperation? operation, string classNameSuffix, CodeNamespace codeNamespace, bool isRequestBody, string typeNameForInlineSchema, bool isViaDiscriminator = false)
{
var flattenedAllOfs = schema.AllOf.FlattenSchemaIfRequired(static x => x.AllOf).ToArray();
var referenceId = schema.Reference?.Id;
Expand All @@ -1624,43 +1624,49 @@ private CodeClass CreateInheritedModelDeclaration(OpenApiUrlTreeNode currentNode
typeNameForInlineSchema :
currentNode.GetClassName(config.StructuredMimeTypes, operation: operation, suffix: classNameSuffix, schema: schema, requestBody: isRequestBody)))
.CleanupSymbolName();
var codeDeclaration = (rootSchemaHasProperties, inlineSchemas, referencedSchemas) switch
var codeDeclaration = (rootSchemaHasProperties, inlineSchemas, referencedSchemas, isViaDiscriminator) switch
{
// greatest parent type
(true, { Length: 0 }, { Length: 0 }) =>
(true, { Length: 0 }, { Length: 0 }, _) =>
AddModelDeclarationIfDoesntExist(currentNode, operation, schema, className, shortestNamespace),
// inline schema + referenced schema
(false, { Length: > 0 }, { Length: 1 }) =>
(false, { Length: > 0 }, { Length: 1 }, _) =>
AddModelDeclarationIfDoesntExist(currentNode, operation, schema.MergeAllOfSchemaEntries([.. referencedSchemas], static x => x.Reference is null)!, className, shortestNamespace, CreateInheritedModelDeclaration(currentNode, referencedSchemas[0], operation, classNameSuffix, codeNamespace, isRequestBody, string.Empty)),
// properties + referenced schema
(true, { Length: 0 }, { Length: 1 }) =>
(true, { Length: 0 }, { Length: 1 }, _) =>
AddModelDeclarationIfDoesntExist(currentNode, operation, schema, className, shortestNamespace, CreateInheritedModelDeclaration(currentNode, referencedSchemas[0], operation, classNameSuffix, codeNamespace, isRequestBody, string.Empty)),
// properties + inline schema
(true, { Length: 1 }, { Length: 0 }) =>
(true, { Length: 1 }, { Length: 0 }, _) =>
AddModelDeclarationIfDoesntExist(currentNode, operation, schema, className, shortestNamespace, CreateInheritedModelDeclaration(currentNode, inlineSchemas[0], operation, classNameSuffix, codeNamespace, isRequestBody, typeNameForInlineSchema)),
// empty schema + referenced schema
(false, { Length: 0 }, { Length: 1 }) =>
(false, { Length: 0 }, { Length: 1 }, false) =>
AddModelDeclarationIfDoesntExist(currentNode, operation, referencedSchemas[0], className, shortestNamespace),
// empty schema + inline schema
(false, { Length: 1 }, { Length: 0 }) =>
(false, { Length: 1 }, { Length: 0 }, false) =>
AddModelDeclarationIfDoesntExist(currentNode, operation, inlineSchemas[0], className, shortestNamespace),
// empty schema + referenced schema and referenced by oneOf discriminator
(false, { Length: 0 }, { Length: 1 }, true) =>
AddModelDeclarationIfDoesntExist(currentNode, operation, schema, className, shortestNamespace, CreateInheritedModelDeclaration(currentNode, referencedSchemas[0], operation, classNameSuffix, codeNamespace, isRequestBody, string.Empty)),
// empty schema + inline schema and referenced by oneOf discriminator
(false, { Length: 1 }, { Length: 0 }, true) =>
AddModelDeclarationIfDoesntExist(currentNode, operation, schema, className, shortestNamespace, CreateInheritedModelDeclaration(currentNode, inlineSchemas[0], operation, classNameSuffix, codeNamespace, isRequestBody, typeNameForInlineSchema)),
// too much information but we can make a choice -> maps to properties + inline schema
(true, { Length: 1 }, { Length: 1 }) when inlineSchemas[0].HasAnyProperty() && !referencedSchemas[0].HasAnyProperty() =>
(true, { Length: 1 }, { Length: 1 }, _) when inlineSchemas[0].HasAnyProperty() && !referencedSchemas[0].HasAnyProperty() =>
AddModelDeclarationIfDoesntExist(currentNode, operation, schema, className, shortestNamespace, CreateInheritedModelDeclaration(currentNode, inlineSchemas[0], operation, classNameSuffix, codeNamespace, isRequestBody, typeNameForInlineSchema)),
// too much information but we can make a choice -> maps to properties + referenced schema
(true, { Length: 1 }, { Length: 1 }) when referencedSchemas[0].HasAnyProperty() && !inlineSchemas[0].HasAnyProperty() =>
(true, { Length: 1 }, { Length: 1 }, _) when referencedSchemas[0].HasAnyProperty() && !inlineSchemas[0].HasAnyProperty() =>
AddModelDeclarationIfDoesntExist(currentNode, operation, schema, className, shortestNamespace, CreateInheritedModelDeclaration(currentNode, referencedSchemas[0], operation, classNameSuffix, codeNamespace, isRequestBody, string.Empty)),
// too much information but we can merge root + inline schema
(true, { Length: 1 }, { Length: 1 }) when referencedSchemas[0].HasAnyProperty() && inlineSchemas[0].HasAnyProperty() && schema.MergeAllOfSchemaEntries([.. referencedSchemas]) is { } mergedSchema =>
(true, { Length: 1 }, { Length: 1 }, _) when referencedSchemas[0].HasAnyProperty() && inlineSchemas[0].HasAnyProperty() && schema.MergeAllOfSchemaEntries([.. referencedSchemas]) is { } mergedSchema =>
AddModelDeclarationIfDoesntExist(currentNode, operation, mergedSchema, className, shortestNamespace, CreateInheritedModelDeclaration(currentNode, referencedSchemas[0], operation, classNameSuffix, codeNamespace, isRequestBody, string.Empty)),
// none of the allOf entries have properties, it's a grandparent schema
(true, { Length: 1 }, { Length: 1 }) =>
(true, { Length: 1 }, { Length: 1 }, _) =>
AddModelDeclarationIfDoesntExist(currentNode, operation, schema, className, shortestNamespace),
// too many entries, we mush everything together
(_, { Length: > 1 }, { Length: > 1 }) or (_, { Length: 0 or 1 }, { Length: > 1 }) or (_, { Length: > 1 }, { Length: 0 or 1 }) =>
(_, { Length: > 1 }, { Length: > 1 }, _) or (_, { Length: 0 or 1 }, { Length: > 1 }, _) or (_, { Length: > 1 }, { Length: 0 or 1 }, _) =>
AddModelDeclarationIfDoesntExist(currentNode, operation, schema.MergeAllOfSchemaEntries()!, className, shortestNamespace),
// meaningless scenario
(false, { Length: 0 }, { Length: 0 }) =>
(false, { Length: 0 }, { Length: 0 }, _) =>
throw new InvalidOperationException($"The type does not contain any information Path: {currentNode.Path}, Reference Id: {referenceId}"),
};
if (codeDeclaration is not CodeClass currentClass) throw new InvalidOperationException("Inheritance is only supported for classes");
Expand Down Expand Up @@ -1735,15 +1741,15 @@ private CodeTypeBase CreateComposedModelDeclaration(OpenApiUrlTreeNode currentNo
className = $"{unionType.Name}Member{++membersWithNoName}";
var declarationType = new CodeType
{
TypeDefinition = AddModelDeclarationIfDoesntExist(currentNode, operation, currentSchema, className, shortestNamespace),
TypeDefinition = AddModelDeclarationIfDoesntExist(currentNode, operation, currentSchema, className, shortestNamespace, null),
CollectionKind = currentSchema.IsArray() ? CodeTypeBase.CodeTypeCollectionKind.Complex : default
};
if (!unionType.ContainsType(declarationType))
unionType.AddType(declarationType);
}
return unionType;
}
private CodeTypeBase CreateModelDeclarations(OpenApiUrlTreeNode currentNode, OpenApiSchema schema, OpenApiOperation? operation, CodeElement parentElement, string suffixForInlineSchema, OpenApiResponse? response = default, string typeNameForInlineSchema = "", bool isRequestBody = false)
private CodeTypeBase CreateModelDeclarations(OpenApiUrlTreeNode currentNode, OpenApiSchema schema, OpenApiOperation? operation, CodeElement parentElement, string suffixForInlineSchema, OpenApiResponse? response = default, string typeNameForInlineSchema = "", bool isRequestBody = false, bool isViaDiscriminator = false)
{
var (codeNamespace, responseValue, suffix) = schema.IsReferencedSchema() switch
{
Expand All @@ -1760,7 +1766,8 @@ private CodeTypeBase CreateModelDeclarations(OpenApiUrlTreeNode currentNode, Ope

if (schema.IsInherited())
{
return CreateInheritedModelDeclarationAndType(currentNode, schema, operation, suffix, codeNamespace, isRequestBody, typeNameForInlineSchema);
// Pass isViaDiscriminator so that we can handle the special case where this model was referenced by a discriminator and we always want to generate a base class.
return CreateInheritedModelDeclarationAndType(currentNode, schema, operation, suffix, codeNamespace, isRequestBody, typeNameForInlineSchema, isViaDiscriminator);
}

if (schema.IsIntersection() && schema.MergeIntersectionSchemaEntries() is OpenApiSchema mergedSchema)
Expand Down Expand Up @@ -1964,6 +1971,7 @@ private CodeClass AddModelClass(OpenApiUrlTreeNode currentNode, OpenApiSchema sc
}
}

// Recurse into the discriminator & generate its referenced types
var mappings = GetDiscriminatorMappings(currentNode, schema, currentNamespace, newClass, currentOperation)
.Where(x => x.Value is { TypeDefinition: CodeClass definition } &&
definition.DerivesFrom(newClass)); // only the mappings that derive from the current class
Expand All @@ -1973,6 +1981,7 @@ private CodeClass AddModelClass(OpenApiUrlTreeNode currentNode, OpenApiSchema sc
}
private IEnumerable<KeyValuePair<string, CodeType>> GetDiscriminatorMappings(OpenApiUrlTreeNode currentNode, OpenApiSchema schema, CodeNamespace currentNamespace, CodeClass? baseClass, OpenApiOperation? currentOperation)
{
// Generate types that this discriminator references
return schema.GetDiscriminatorMappings(inheritanceIndex)
.Union(baseClass is not null && modelsNamespace is not null &&
(openApiDocument?.Components?.Schemas?.TryGetValue(baseClass.GetComponentSchemaName(modelsNamespace), out var componentSchema) ?? false) ?
Expand Down Expand Up @@ -2160,7 +2169,8 @@ internal static void AddDiscriminatorMethod(CodeClass newClass, string discrimin
logger.LogWarning("Discriminator {ComponentKey} not found in the OpenAPI document.", componentKey);
return null;
}
if (CreateModelDeclarations(currentNode, discriminatorSchema, currentOperation, GetShortestNamespace(currentNamespace, discriminatorSchema), string.Empty) is not CodeType result)
// Call CreateModelDeclarations with isViaDiscriminator=true. This is for a special case where we always generate a base class when types are referenced via a oneOf discriminator.
if (CreateModelDeclarations(currentNode, discriminatorSchema, currentOperation, GetShortestNamespace(currentNamespace, discriminatorSchema), string.Empty, null, string.Empty, false, true) is not CodeType result)
{
logger.LogWarning("Discriminator {ComponentKey} is not a valid model and points to a union type.", componentKey);
return null;
Expand Down
74 changes: 74 additions & 0 deletions tests/Kiota.Builder.Tests/KiotaBuilderTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7840,6 +7840,80 @@ public async Task InheritanceWithAllOfWith3Parts3SchemaChildClass()
Assert.Single(groupClass.Properties.Where(static x => x.Name.Equals("facetprop1", StringComparison.OrdinalIgnoreCase)));
Assert.Single(groupClass.Properties.Where(static x => x.Name.Equals("facetprop2", StringComparison.OrdinalIgnoreCase)));
}

[Fact]
public async Task InheritanceWithAllOfBaseClassNoAdditionalProperties()
{
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.directoryResult'
components:
schemas:
microsoft.graph.directoryResult:
required: ['fstype']
oneOf:
- $ref: '#/components/schemas/microsoft.graph.file'
- $ref: '#/components/schemas/microsoft.graph.folder'
- $ref: '#/components/schemas/microsoft.graph.link'
properties:
fstype:
type: string
discriminator:
propertyName: 'fstype'
mapping:
'file': '#/components/schemas/microsoft.graph.file'
'folder': '#/components/schemas/microsoft.graph.folder'
'link': '#/components/schemas/microsoft.graph.link'
microsoft.graph.baseDirectoryObject:
properties:
path:
type: string
microsoft.graph.file:
allOf:
- '$ref': '#/components/schemas/microsoft.graph.baseDirectoryObject'
microsoft.graph.folder:
allOf:
- '$ref': '#/components/schemas/microsoft.graph.baseDirectoryObject'
microsoft.graph.link:
allOf:
- '$ref': '#/components/schemas/microsoft.graph.baseDirectoryObject'
properties:
target:
type: string");
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);

// Verify that all three classes referenced by the discriminator inherit from baseDirectoryObject
var folder = codeModel.FindChildByName<CodeClass>("Folder");
Assert.NotNull(folder.StartBlock.Inherits);
Assert.Equal("baseDirectoryObject", folder.StartBlock.Inherits.Name);

var file = codeModel.FindChildByName<CodeClass>("File");
Assert.NotNull(file.StartBlock.Inherits);
Assert.Equal("baseDirectoryObject", file.StartBlock.Inherits.Name);

var link = codeModel.FindChildByName<CodeClass>("Link");
Assert.NotNull(link.StartBlock.Inherits);
Assert.Equal("baseDirectoryObject", link.StartBlock.Inherits.Name);
}

[Fact]
public async Task NestedIntersectionTypeAllOf()
{
Expand Down
Loading