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

fix: single entry one/any of merging #5827

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Changed

- Fixed python generation in scenarios with opening/closing tags for code comments. [#5636](https://github.com/microsoft/kiota/issues/5636)
- Fixed Python error when a class inherits from a base class and implements an interface. [5637](https://github.com/microsoft/kiota/issues/5637)
- Fix anyOf/oneOf generation in TypeScript. [5353](https://github.com/microsoft/kiota/issues/5353)
- Fixed Python error when a class inherits from a base class and implements an interface. [#5637](https://github.com/microsoft/kiota/issues/5637)
- Fixed a bug where one/any schemas with single schema entries would be missing properties. [#5808](https://github.com/microsoft/kiota/issues/5808)
- Fixed anyOf/oneOf generation in TypeScript. [5353](https://github.com/microsoft/kiota/issues/5353)
- Fixed invalid code in Php caused by "*/*/" in property description. [5635](https://github.com/microsoft/kiota/issues/5635)
- Fixed TypeScript generation error when generating usings from shaken serializers. [#5634](https://github.com/microsoft/kiota/issues/5634)

Expand Down
38 changes: 34 additions & 4 deletions src/Kiota.Builder/Extensions/OpenApiSchemaExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -68,9 +68,9 @@ public static bool HasAnyProperty(this OpenApiSchema? schema)
{
return schema?.Properties is { Count: > 0 };
}
public static bool IsInclusiveUnion(this OpenApiSchema? schema)
public static bool IsInclusiveUnion(this OpenApiSchema? schema, uint exclusiveMinimumNumberOfEntries = 1)
{
return schema?.AnyOf?.Count(static x => IsSemanticallyMeaningful(x, true)) > 1;
return schema?.AnyOf?.Count(static x => IsSemanticallyMeaningful(x, true)) > exclusiveMinimumNumberOfEntries;
// so we don't consider any of object/nullable as a union type
}

Expand All @@ -89,6 +89,36 @@ public static bool IsInherited(this OpenApiSchema? schema)
return schema.MergeIntersectionSchemaEntries(schemasToExclude, true, filter);
}

internal static OpenApiSchema? MergeInclusiveUnionSchemaEntries(this OpenApiSchema? schema)
{
if (schema is null || !schema.IsInclusiveUnion(0)) return null;
var result = new OpenApiSchema(schema);
result.AnyOf.Clear();
foreach (var subSchema in schema.AnyOf)
{
foreach (var property in subSchema.Properties)
{
result.Properties.TryAdd(property.Key, property.Value);
}
}
return result;
}

internal static OpenApiSchema? MergeExclusiveUnionSchemaEntries(this OpenApiSchema? schema)
{
if (schema is null || !schema.IsExclusiveUnion(0)) return null;
var result = new OpenApiSchema(schema);
result.OneOf.Clear();
foreach (var subSchema in schema.OneOf)
{
foreach (var property in subSchema.Properties)
{
result.Properties.TryAdd(property.Key, property.Value);
}
}
return result;
}

internal static OpenApiSchema? MergeIntersectionSchemaEntries(this OpenApiSchema? schema, HashSet<OpenApiSchema>? schemasToExclude = default, bool overrideIntersection = false, Func<OpenApiSchema, bool>? filter = default)
{
if (schema is null) return null;
Expand Down Expand Up @@ -123,9 +153,9 @@ public static bool IsIntersection(this OpenApiSchema? schema)
return meaningfulSchemas?.Count(static x => !string.IsNullOrEmpty(x.Reference?.Id)) > 1 || meaningfulSchemas?.Count(static x => string.IsNullOrEmpty(x.Reference?.Id)) > 1;
}

public static bool IsExclusiveUnion(this OpenApiSchema? schema)
public static bool IsExclusiveUnion(this OpenApiSchema? schema, uint exclusiveMinimumNumberOfEntries = 1)
{
return schema?.OneOf?.Count(static x => IsSemanticallyMeaningful(x, true)) > 1;
return schema?.OneOf?.Count(static x => IsSemanticallyMeaningful(x, true)) > exclusiveMinimumNumberOfEntries;
// so we don't consider one of object/nullable as a union type
}
private static readonly HashSet<string> oDataTypes = new(StringComparer.OrdinalIgnoreCase) {
Expand Down
10 changes: 10 additions & 0 deletions src/Kiota.Builder/KiotaBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1895,6 +1895,16 @@ private CodeElement AddModelDeclarationIfDoesntExist(OpenApiUrlTreeNode currentN
// multiple allOf entries that do not translate to inheritance
return createdClass;
}
else if (schema.MergeInclusiveUnionSchemaEntries() is { } iUMergedSchema &&
AddModelClass(currentNode, iUMergedSchema, declarationName, currentNamespace, currentOperation, inheritsFrom) is CodeClass uICreatedClass)
{
return uICreatedClass;
}
else if (schema.MergeExclusiveUnionSchemaEntries() is { } eUMergedSchema &&
AddModelClass(currentNode, eUMergedSchema, declarationName, currentNamespace, currentOperation, inheritsFrom) is CodeClass uECreatedClass)
{
return uECreatedClass;
}
return AddModelClass(currentNode, schema, declarationName, currentNamespace, currentOperation, inheritsFrom);
}
return existingDeclaration;
Expand Down
4 changes: 2 additions & 2 deletions src/Kiota.Builder/Writers/CSharp/CodeMethodWriter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ private void WriteFactoryMethodBodyForUnionModel(CodeMethod codeElement, CodeCla
}
else if (propertyType.TypeDefinition is CodeClass && propertyType.IsCollection || propertyType.TypeDefinition is null || propertyType.TypeDefinition is CodeEnum)
{
var typeName = conventions.GetTypeString(propertyType, codeElement, true, false);
var typeName = conventions.GetTypeString(propertyType, codeElement, true, propertyType.TypeDefinition is CodeEnum && propertyType.CollectionKind is not CodeTypeBase.CodeTypeCollectionKind.None);
var valueVarName = $"{property.Name.ToFirstCharacterLowerCase()}Value";
writer.WriteLine($"{(includeElse ? "else " : string.Empty)}if({parseNodeParameter.Name.ToFirstCharacterLowerCase()}.{GetDeserializationMethodName(propertyType, codeElement)} is {typeName} {valueVarName})");
writer.WriteBlock(lines: $"{ResultVarName}.{property.Name.ToFirstCharacterUpperCase()} = {valueVarName};");
Expand All @@ -161,7 +161,7 @@ private void WriteFactoryMethodBodyForIntersectionModel(CodeMethod codeElement,
{
if (property.Type is CodeType propertyType)
{
var typeName = conventions.GetTypeString(propertyType, codeElement, true, false);
var typeName = conventions.GetTypeString(propertyType, codeElement, true, propertyType.TypeDefinition is CodeEnum && propertyType.CollectionKind is not CodeTypeBase.CodeTypeCollectionKind.None);
var valueVarName = $"{property.Name.ToFirstCharacterLowerCase()}Value";
writer.WriteLine($"{(includeElse ? "else " : string.Empty)}if({parseNodeParameter.Name.ToFirstCharacterLowerCase()}.{GetDeserializationMethodName(propertyType, codeElement)} is {typeName} {valueVarName})");
writer.WriteBlock(lines: $"{ResultVarName}.{property.Name.ToFirstCharacterUpperCase()} = {valueVarName};");
Expand Down
4 changes: 2 additions & 2 deletions src/Kiota.Builder/Writers/Go/CodeMethodWriter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -195,13 +195,13 @@ private void WriteFactoryMethodBodyForIntersectionModel(CodeMethod codeElement,
WriteCollectionCast(propertyTypeImportName, valueVarName, "cast", writer, isInterfaceType ? string.Empty : "*", !isInterfaceType);
valueVarName = "cast";
}
else if (propertyType.TypeDefinition is CodeClass || propertyType.TypeDefinition is CodeInterface)
else if (propertyType.TypeDefinition is CodeClass || propertyType.TypeDefinition is CodeInterface || propertyType.TypeDefinition is CodeEnum)
{
writer.StartBlock($"if {GetTypeAssertion(valueVarName, propertyTypeImportName, "cast", "ok")}; ok {{");
valueVarName = "cast";
}
writer.WriteLine($"{ResultVarName}.{property.Setter!.Name.ToFirstCharacterUpperCase()}({valueVarName})");
if (!propertyType.IsCollection && (propertyType.TypeDefinition is CodeClass || propertyType.TypeDefinition is CodeInterface))
if (!propertyType.IsCollection && (propertyType.TypeDefinition is CodeClass || propertyType.TypeDefinition is CodeInterface || propertyType.TypeDefinition is CodeEnum))
writer.CloseBlock();
writer.DecreaseIndent();
}
Expand Down
176 changes: 176 additions & 0 deletions tests/Kiota.Builder.Tests/KiotaBuilderTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8536,6 +8536,182 @@ public async Task InheritanceWithAllOfBaseClassNoAdditionalPropertiesAsync()
Assert.Equal("baseDirectoryObject", link.StartBlock.Inherits.Name);
}

[Fact]
public async Task ExclusiveUnionSingleEntriesMergingAsync()
{
var tempFilePath = Path.Combine(Path.GetTempPath(), Path.GetTempFileName());
await using var fs = await GetDocumentStreamAsync(
"""
openapi: 3.0.0
info:
title: "Generator not generating oneOf if the containing schema has type: object"
version: "1.0.0"
servers:
- url: https://mytodos.doesnotexist/
paths:
/uses-components:
post:
description: Return something
responses:
"200":
description: OK
content:
application/json:
schema:
$ref: "#/components/schemas/UsesComponents"
requestBody:
content:
application/json:
schema:
$ref: "#/components/schemas/UsesComponents"
components:
schemas:
ExampleWithSingleOneOfWithTypeObject:
type: object
oneOf:
- $ref: "#/components/schemas/Component1"
discriminator:
propertyName: objectType
ExampleWithSingleOneOfWithoutTypeObject:
oneOf:
- $ref: "#/components/schemas/Component2"
discriminator:
propertyName: objectType

UsesComponents:
type: object
properties:
component_with_single_oneof_with_type_object:
$ref: "#/components/schemas/ExampleWithSingleOneOfWithTypeObject"
component_with_single_oneof_without_type_object:
$ref: "#/components/schemas/ExampleWithSingleOneOfWithoutTypeObject"

Component1:
type: object
required:
- objectType
properties:
objectType:
type: string
one:
type: string

Component2:
type: object
required:
- objectType
properties:
objectType:
type: string
two:
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 withObjectClass = codeModel.FindChildByName<CodeClass>("ExampleWithSingleOneOfWithTypeObject");
Assert.NotNull(withObjectClass);
var oneProperty = withObjectClass.FindChildByName<CodeProperty>("one", false);
Assert.NotNull(oneProperty);

var withoutObjectClass = codeModel.FindChildByName<CodeClass>("Component2");
Assert.NotNull(withObjectClass);
var twoProperty = withoutObjectClass.FindChildByName<CodeProperty>("two", false);
Assert.NotNull(twoProperty);
}

[Fact]
public async Task InclusiveUnionSingleEntriesMergingAsync()
{
var tempFilePath = Path.Combine(Path.GetTempPath(), Path.GetTempFileName());
await using var fs = await GetDocumentStreamAsync(
"""
openapi: 3.0.0
info:
title: "Generator not generating anyOf if the containing schema has type: object"
version: "1.0.0"
servers:
- url: https://mytodos.doesnotexist/
paths:
/uses-components:
post:
description: Return something
responses:
"200":
description: OK
content:
application/json:
schema:
$ref: "#/components/schemas/UsesComponents"
requestBody:
content:
application/json:
schema:
$ref: "#/components/schemas/UsesComponents"
components:
schemas:
ExampleWithSingleOneOfWithTypeObject:
type: object
anyOf:
- $ref: "#/components/schemas/Component1"
discriminator:
propertyName: objectType
ExampleWithSingleOneOfWithoutTypeObject:
anyOf:
- $ref: "#/components/schemas/Component2"
discriminator:
propertyName: objectType

UsesComponents:
type: object
properties:
component_with_single_oneof_with_type_object:
$ref: "#/components/schemas/ExampleWithSingleOneOfWithTypeObject"
component_with_single_oneof_without_type_object:
$ref: "#/components/schemas/ExampleWithSingleOneOfWithoutTypeObject"

Component1:
type: object
required:
- objectType
properties:
objectType:
type: string
one:
type: string

Component2:
type: object
required:
- objectType
properties:
objectType:
type: string
two:
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 withObjectClass = codeModel.FindChildByName<CodeClass>("ExampleWithSingleOneOfWithTypeObject");
Assert.NotNull(withObjectClass);
var oneProperty = withObjectClass.FindChildByName<CodeProperty>("one", false);
Assert.NotNull(oneProperty);

var withoutObjectClass = codeModel.FindChildByName<CodeClass>("Component2");
Assert.NotNull(withObjectClass);
var twoProperty = withoutObjectClass.FindChildByName<CodeProperty>("two", false);
Assert.NotNull(twoProperty);
}

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