Skip to content

Commit

Permalink
Merge pull request #5827 from microsoft/fix/one-any-of-missing
Browse files Browse the repository at this point in the history
fix: single entry one/any of merging
  • Loading branch information
baywet authored Nov 28, 2024
2 parents f775663 + 5d2944e commit fcf8270
Show file tree
Hide file tree
Showing 6 changed files with 231 additions and 10 deletions.
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 a bug where discriminator property name lookup could end up in an infinite loop. [#5771](https://github.com/microsoft/kiota/issues/5771)
- 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
8 changes: 6 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,17 @@ 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)
{
if (propertyType.TypeDefinition is CodeEnum)
{
propertyTypeImportName = conventions.GetTypeString(property.Type, parentClass, false, true);
}
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

0 comments on commit fcf8270

Please sign in to comment.