Skip to content

Commit

Permalink
fix: anyOf/oneOf missing properties for intersection/inheritance
Browse files Browse the repository at this point in the history
  • Loading branch information
vipentti committed Dec 22, 2024
1 parent 77accd2 commit 4e18023
Show file tree
Hide file tree
Showing 5 changed files with 603 additions and 23 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

- Fixed a bug in generation when a referenced schema in an allOf was a primitive [#5701](https://github.com/microsoft/kiota/issues/5701).
- Fixed a bug where inherited error models would be missing interface declarations. [#5888](https://github.com/microsoft/kiota/issues/5888)
- Fixed a bug where oneOf/anyOf schemas with single references to inheritance or intersections would be missing properties. [#5921](https://github.com/microsoft/kiota/issues/5921)

## [1.21.0] - 2024-12-05

Expand Down
29 changes: 29 additions & 0 deletions src/Kiota.Builder/Extensions/IListExtensions.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
using System.Collections.Generic;

namespace Kiota.Builder.Extensions;

public static class IListExtensions
{
/// <summary>
/// Returns the only element of this list when it has count of exactly <c>1</c>
/// </summary>
/// <typeparam name="T">The contained item type.</typeparam>
/// <param name="items">The items.</param>
/// <returns>The only element or null.</returns>
internal static T? OnlyOneOrDefault<T>(this IList<T> items) =>
items.Count == 1 ? items[0] : default;

/// <summary>
/// Adds the provided <paramref name="values"/> to this list.
/// </summary>
/// <typeparam name="T">The contained item type.</typeparam>
/// <param name="items">The items.</param>
/// <param name="values">The values to add.</param>
internal static void AddRange<T>(this IList<T> items, IEnumerable<T> values)
{
foreach (var item in values)
{
items.Add(item);
}
}
}
60 changes: 44 additions & 16 deletions src/Kiota.Builder/Extensions/OpenApiSchemaExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -98,13 +98,7 @@ public static bool IsInherited(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);
}
}
result.TryAddProperties(schema.AnyOf.SelectMany(static x => x.Properties));
return result;
}

Expand All @@ -113,14 +107,42 @@ public static bool IsInherited(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)
result.TryAddProperties(schema.OneOf.SelectMany(static x => x.Properties));
return result;
}

internal static OpenApiSchema? MergeSingleInclusiveUnionInheritanceOrIntersectionSchemaEntries(this OpenApiSchema? schema)
{
if (schema is not null
&& schema.IsInclusiveUnion(0)
&& schema.AnyOf.OnlyOneOrDefault() is OpenApiSchema subSchema
&& (subSchema.IsInherited() || subSchema.IsIntersection()))
{
foreach (var property in subSchema.Properties)
{
result.Properties.TryAdd(property.Key, property.Value);
}
var result = new OpenApiSchema(schema);
result.AnyOf.Clear();
result.TryAddProperties(subSchema.Properties);
result.AllOf.AddRange(subSchema.AllOf);
return result;
}
return result;

return null;
}

internal static OpenApiSchema? MergeSingleExclusiveUnionInheritanceOrIntersectionSchemaEntries(this OpenApiSchema? schema)
{
if (schema is not null
&& schema.IsExclusiveUnion(0)
&& schema.OneOf.OnlyOneOrDefault() is OpenApiSchema subSchema
&& (subSchema.IsInherited() || subSchema.IsIntersection()))
{
var result = new OpenApiSchema(schema);
result.OneOf.Clear();
result.TryAddProperties(subSchema.Properties);
result.AllOf.AddRange(subSchema.AllOf);
return result;
}

return null;
}

internal static OpenApiSchema? MergeIntersectionSchemaEntries(this OpenApiSchema? schema, HashSet<OpenApiSchema>? schemasToExclude = default, bool overrideIntersection = false, Func<OpenApiSchema, bool>? filter = default)
Expand All @@ -144,11 +166,17 @@ public static bool IsInherited(this OpenApiSchema? schema)
else if (discriminator.Mapping?.Any() ?? false)
result.Discriminator.Mapping = discriminator.Mapping.ToDictionary(static x => x.Key, static x => x.Value);

foreach (var propertyToMerge in entriesToMerge.SelectMany(static x => x.Properties))
result.TryAddProperties(entriesToMerge.SelectMany(static x => x.Properties));

return result;
}

internal static void TryAddProperties(this OpenApiSchema schema, IEnumerable<KeyValuePair<string, OpenApiSchema>> properties)
{
foreach (var property in properties)
{
result.Properties.TryAdd(propertyToMerge.Key, propertyToMerge.Value);
schema.Properties.TryAdd(property.Key, property.Value);
}
return result;
}

public static bool IsIntersection(this OpenApiSchema? schema)
Expand Down
20 changes: 16 additions & 4 deletions src/Kiota.Builder/KiotaBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -958,7 +958,7 @@ x.Parent is CodeIndexer ||
var parentNS = x.Parent?.Parent?.Parent as CodeNamespace;
CodeClass[] exceptions = x.Parent?.Parent is CodeClass parentClass ? [parentClass] : [];
x.TypeDefinition = parentNS?.FindChildrenByName<CodeClass>(x.Name)
.Except(exceptions)// the property method should not reference itself as a return type.
.Except(exceptions)// the property method should not reference itself as a return type.
.MinBy(shortestNamespaceOrder);
// searching down first because most request builder properties on a request builder are just sub paths on the API
if (x.TypeDefinition == null)
Expand Down Expand Up @@ -1833,14 +1833,26 @@ private CodeTypeBase CreateModelDeclarations(OpenApiUrlTreeNode currentNode, Ope
return CreateComposedModelDeclaration(currentNode, schema, operation, suffix, codeNamespace, isRequestBody, typeNameForInlineSchema);
}

// type: object with single oneOf referring to inheritance or intersection
if (schema.IsObjectType() && schema.MergeSingleExclusiveUnionInheritanceOrIntersectionSchemaEntries() is OpenApiSchema mergedExclusiveUnionSchema)
{
return CreateModelDeclarations(currentNode, mergedExclusiveUnionSchema, operation, parentElement, suffixForInlineSchema, response, typeNameForInlineSchema, isRequestBody);
}

// type: object with single anyOf referring to inheritance or intersection
if (schema.IsObjectType() && schema.MergeSingleInclusiveUnionInheritanceOrIntersectionSchemaEntries() is OpenApiSchema mergedInclusiveUnionSchema)
{
return CreateModelDeclarations(currentNode, mergedInclusiveUnionSchema, operation, parentElement, suffixForInlineSchema, response, typeNameForInlineSchema, isRequestBody);
}

if (schema.IsObjectType() || schema.HasAnyProperty() || schema.IsEnum() || !string.IsNullOrEmpty(schema.AdditionalProperties?.Type))
{
// no inheritance or union type, often empty definitions with only additional properties are used as property bags.
return CreateModelDeclarationAndType(currentNode, schema, operation, codeNamespace, suffix, response: responseValue, typeNameForInlineSchema: typeNameForInlineSchema, isRequestBody);
}

if (schema.IsArray() &&
!schema.Items.IsArray()) // Only handle collections of primitives and complex types. Otherwise, multi-dimensional arrays would be recursively unwrapped undesirably to lead to incorrect serialization types.
!schema.Items.IsArray()) // Only handle collections of primitives and complex types. Otherwise, multi-dimensional arrays would be recursively unwrapped undesirably to lead to incorrect serialization types.
{
// collections at root
return CreateCollectionModelDeclaration(currentNode, schema, operation, codeNamespace, typeNameForInlineSchema, isRequestBody);
Expand Down Expand Up @@ -2230,15 +2242,15 @@ internal static void AddDiscriminatorMethod(CodeClass newClass, string discrimin
logger.LogWarning("Discriminator {ComponentKey} not found in the OpenAPI document.", componentKey);
return null;
}
// 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.
// 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;
}
if (baseClass is not null && (result.TypeDefinition is not CodeClass codeClass || codeClass.StartBlock.Inherits is null))
{
if (!baseClass.Equals(result.TypeDefinition))// don't log warning if the discriminator points to the base type itself as this is implicitly the default case.
if (!baseClass.Equals(result.TypeDefinition))// don't log warning if the discriminator points to the base type itself as this is implicitly the default case.
logger.LogWarning("Discriminator {ComponentKey} is not inherited from {ClassName}.", componentKey, baseClass.Name);
return null;
}
Expand Down
Loading

0 comments on commit 4e18023

Please sign in to comment.