From 448af8fd37ada0e4f38379453d4739274ee72ccf Mon Sep 17 00:00:00 2001 From: Vincent Biret Date: Fri, 24 May 2024 13:22:55 -0400 Subject: [PATCH 1/4] fix: adds missing mushing of all of scenarios Signed-off-by: Vincent Biret --- src/Kiota.Builder/CodeDOM/CodeClass.cs | 8 + .../Extensions/OpenApiSchemaExtensions.cs | 33 +++- src/Kiota.Builder/KiotaBuilder.cs | 154 +++++++++--------- .../Kiota.Builder.Tests/KiotaBuilderTests.cs | 8 +- 4 files changed, 117 insertions(+), 86 deletions(-) diff --git a/src/Kiota.Builder/CodeDOM/CodeClass.cs b/src/Kiota.Builder/CodeDOM/CodeClass.cs index 41a41061c2..1e5be4b934 100644 --- a/src/Kiota.Builder/CodeDOM/CodeClass.cs +++ b/src/Kiota.Builder/CodeDOM/CodeClass.cs @@ -45,6 +45,14 @@ public CodeComposedTypeBase? OriginalComposedType { get; set; } + public string GetComponentSchemaName(CodeNamespace modelsNamespace) + { + if (Kind is not CodeClassKind.Model || + Parent is not CodeNamespace parentNamespace || + !parentNamespace.IsChildOf(modelsNamespace)) + return string.Empty; + return $"{parentNamespace.Name[(modelsNamespace.Name.Length + 1)..]}.{Name}"; + } public CodeIndexer? Indexer => InnerChildElements.Values.OfType().FirstOrDefault(static x => !x.IsLegacyIndexer); public void AddIndexer(params CodeIndexer[] indexers) { diff --git a/src/Kiota.Builder/Extensions/OpenApiSchemaExtensions.cs b/src/Kiota.Builder/Extensions/OpenApiSchemaExtensions.cs index 272c31e57d..004084d40e 100644 --- a/src/Kiota.Builder/Extensions/OpenApiSchemaExtensions.cs +++ b/src/Kiota.Builder/Extensions/OpenApiSchemaExtensions.cs @@ -84,19 +84,36 @@ public static bool IsInherited(this OpenApiSchema? schema) isRootSchemaMeaningful); } - internal static OpenApiSchema? MergeIntersectionSchemaEntries(this OpenApiSchema? schema, HashSet? schemasToExclude = default) + internal static OpenApiSchema? MergeAllOfSchemaEntries(this OpenApiSchema? schema, HashSet? schemasToExclude = default, Func? filter = default) + { + return schema.MergeIntersectionSchemaEntries(schemasToExclude, true, filter); + } + + internal static OpenApiSchema? MergeIntersectionSchemaEntries(this OpenApiSchema? schema, HashSet? schemasToExclude = default, bool overrideIntersection = false, Func? filter = default) { if (schema is null) return null; - if (!schema.IsIntersection()) return schema; + if (!schema.IsIntersection() && !overrideIntersection) return schema; var result = new OpenApiSchema(schema); result.AllOf.Clear(); var meaningfulSchemas = schema.AllOf - .Where(static x => x.IsSemanticallyMeaningful() || x.AllOf.Any()) - .Select(x => MergeIntersectionSchemaEntries(x, schemasToExclude)) + .Where(x => (x.IsSemanticallyMeaningful() || x.AllOf.Any()) && (filter == null || filter(x))) + .Select(x => MergeIntersectionSchemaEntries(x, schemasToExclude, overrideIntersection, filter)) .Where(x => x is not null && (schemasToExclude is null || !schemasToExclude.Contains(x))) .OfType() .ToArray(); - meaningfulSchemas.FlattenEmptyEntries(static x => x.AllOf).Union(meaningfulSchemas).SelectMany(static x => x.Properties).ToList().ForEach(x => result.Properties.TryAdd(x.Key, x.Value)); + var entriesToMerge = meaningfulSchemas.FlattenEmptyEntries(static x => x.AllOf).Union(meaningfulSchemas).ToArray(); + if (entriesToMerge.Select(static x => x.Discriminator).OfType().FirstOrDefault() is OpenApiDiscriminator discriminator) + if (result.Discriminator is null) + result.Discriminator = discriminator; + else if (string.IsNullOrEmpty(result.Discriminator.PropertyName) && !string.IsNullOrEmpty(discriminator.PropertyName)) + result.Discriminator.PropertyName = discriminator.PropertyName; + 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.Properties.TryAdd(propertyToMerge.Key, propertyToMerge.Value); + } return result; } @@ -225,8 +242,8 @@ internal static string GetDiscriminatorPropertyName(this OpenApiSchema schema) return oneOfDiscriminatorPropertyName; if (schema.AnyOf.Select(GetDiscriminatorPropertyName).FirstOrDefault(static x => !string.IsNullOrEmpty(x)) is string anyOfDiscriminatorPropertyName) return anyOfDiscriminatorPropertyName; - if (schema.AllOf.Any()) - return GetDiscriminatorPropertyName(schema.AllOf[^1]); + if (schema.AllOf.Select(GetDiscriminatorPropertyName).FirstOrDefault(static x => !string.IsNullOrEmpty(x)) is string allOfDiscriminatorPropertyName) + return allOfDiscriminatorPropertyName; return string.Empty; } @@ -260,7 +277,7 @@ private static IEnumerable GetAllInheritanceSchemaReferences(string curr ArgumentNullException.ThrowIfNull(inheritanceIndex); if (inheritanceIndex.TryGetValue(currentReferenceId, out var dependents)) return dependents.Keys.Union(dependents.Keys.SelectMany(x => GetAllInheritanceSchemaReferences(x, inheritanceIndex))).Distinct(StringComparer.OrdinalIgnoreCase); - return Enumerable.Empty(); + return []; } } diff --git a/src/Kiota.Builder/KiotaBuilder.cs b/src/Kiota.Builder/KiotaBuilder.cs index bb39714bf3..0955e37633 100644 --- a/src/Kiota.Builder/KiotaBuilder.cs +++ b/src/Kiota.Builder/KiotaBuilder.cs @@ -1578,7 +1578,7 @@ private string GetModelsNamespaceNameFromReferenceId(string? referenceId) private CodeType CreateModelDeclarationAndType(OpenApiUrlTreeNode currentNode, OpenApiSchema schema, OpenApiOperation? operation, CodeNamespace codeNamespace, string classNameSuffix = "", OpenApiResponse? response = default, string typeNameForInlineSchema = "", bool isRequestBody = false) { var className = string.IsNullOrEmpty(typeNameForInlineSchema) ? currentNode.GetClassName(config.StructuredMimeTypes, operation: operation, suffix: classNameSuffix, response: response, schema: schema, requestBody: isRequestBody).CleanupSymbolName() : typeNameForInlineSchema; - var codeDeclaration = AddModelDeclarationIfDoesntExist(currentNode, schema, className, codeNamespace); + var codeDeclaration = AddModelDeclarationIfDoesntExist(currentNode, operation, schema, className, codeNamespace); return new CodeType { TypeDefinition = codeDeclaration, @@ -1600,8 +1600,8 @@ private CodeClass CreateInheritedModelDeclaration(OpenApiUrlTreeNode currentNode if (rootNamespace is null) throw new InvalidOperationException("Root namespace is not set"); var shortestNamespace = string.IsNullOrEmpty(referenceId) ? codeNamespaceFromParent : rootNamespace.FindOrAddNamespace(shortestNamespaceName); - var inlineSchema = Array.Find(flattenedAllOfs, static x => !x.IsReferencedSchema()); - var referencedSchema = Array.Find(flattenedAllOfs, static x => x.IsReferencedSchema()); + var inlineSchemas = Array.FindAll(flattenedAllOfs, static x => !x.IsReferencedSchema()); + var referencedSchemas = Array.FindAll(flattenedAllOfs, static x => x.IsReferencedSchema()); var rootSchemaHasProperties = schema.HasAnyProperty(); var className = (schema.GetSchemaName(schema.IsSemanticallyMeaningful()) is string cName && !string.IsNullOrEmpty(cName) ? cName : @@ -1609,32 +1609,53 @@ private CodeClass CreateInheritedModelDeclaration(OpenApiUrlTreeNode currentNode typeNameForInlineSchema : currentNode.GetClassName(config.StructuredMimeTypes, operation: operation, suffix: classNameSuffix, schema: schema, requestBody: isRequestBody))) .CleanupSymbolName(); - var codeDeclaration = (rootSchemaHasProperties, inlineSchema, referencedSchema) switch + var codeDeclaration = (rootSchemaHasProperties, inlineSchemas, referencedSchemas) switch { // greatest parent type - (true, null, null) => AddModelDeclarationIfDoesntExist(currentNode, schema, className, shortestNamespace), + (true, { Length: 0 }, { Length: 0 }) => + AddModelDeclarationIfDoesntExist(currentNode, operation, schema, className, shortestNamespace), // inline schema + referenced schema - (false, not null, not null) => AddModelDeclarationIfDoesntExist(currentNode, inlineSchema, className, shortestNamespace, CreateInheritedModelDeclaration(currentNode, referencedSchema, operation, classNameSuffix, codeNamespace, isRequestBody, string.Empty)), + (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, null, not null) => AddModelDeclarationIfDoesntExist(currentNode, schema, className, shortestNamespace, CreateInheritedModelDeclaration(currentNode, referencedSchema, operation, classNameSuffix, codeNamespace, isRequestBody, string.Empty)), + (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, not null, null) => AddModelDeclarationIfDoesntExist(currentNode, schema, className, shortestNamespace, CreateInheritedModelDeclaration(currentNode, inlineSchema, operation, classNameSuffix, codeNamespace, isRequestBody, typeNameForInlineSchema)), + (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, null, not null) => AddModelDeclarationIfDoesntExist(currentNode, referencedSchema, className, shortestNamespace), + (false, { Length: 0 }, { Length: 1 }) => + AddModelDeclarationIfDoesntExist(currentNode, operation, referencedSchemas[0], className, shortestNamespace), // empty schema + inline schema - (false, not null, null) => AddModelDeclarationIfDoesntExist(currentNode, inlineSchema, className, shortestNamespace), + (false, { Length: 1 }, { Length: 0 }) => + AddModelDeclarationIfDoesntExist(currentNode, operation, inlineSchemas[0], className, shortestNamespace), // too much information but we can make a choice -> maps to properties + inline schema - (true, not null, not null) when inlineSchema.HasAnyProperty() => AddModelDeclarationIfDoesntExist(currentNode, schema, className, shortestNamespace, CreateInheritedModelDeclaration(currentNode, inlineSchema, operation, classNameSuffix, codeNamespace, isRequestBody, typeNameForInlineSchema)), + (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, not null, not null) when referencedSchema.HasAnyProperty() => AddModelDeclarationIfDoesntExist(currentNode, schema, className, shortestNamespace, CreateInheritedModelDeclaration(currentNode, referencedSchema, operation, classNameSuffix, codeNamespace, isRequestBody, string.Empty)), + (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 => + 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 }) => + 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 }) => + AddModelDeclarationIfDoesntExist(currentNode, operation, schema.MergeAllOfSchemaEntries()!, className, shortestNamespace), // meaningless scenario - (false, null, null) or (true, not null, not null) => throw new InvalidOperationException("invalid inheritance case"), + (false, { Length: 0 }, { Length: 0 }) => + throw new InvalidOperationException("the type does not contain any information"), }; if (codeDeclaration is not CodeClass currentClass) throw new InvalidOperationException("Inheritance is only supported for classes"); if (!currentClass.Documentation.DescriptionAvailable && - string.IsNullOrEmpty(schema.AllOf.LastOrDefault()?.Description) && - !string.IsNullOrEmpty(schema.Description)) - currentClass.Documentation.DescriptionTemplate = schema.Description.CleanupDescription(); // the last allof entry often is not a reference and doesn't have a description. + new string[] { schema.Description } + .Union(schema.AllOf + .Where(static x => x.Reference is null) + .Select(static x => x.Description)) + .FirstOrDefault(static x => !string.IsNullOrEmpty(x)) is string description) + currentClass.Documentation.DescriptionTemplate = description.CleanupDescription(); // the last allof entry often is not a reference and doesn't have a description. return currentClass; } @@ -1660,7 +1681,7 @@ private CodeTypeBase CreateComposedModelDeclaration(OpenApiUrlTreeNode currentNo var shortestNamespace = GetShortestNamespace(codeNamespace, targetSchema); return new CodeType { - TypeDefinition = AddModelDeclarationIfDoesntExist(currentNode, targetSchema, className, shortestNamespace), + TypeDefinition = AddModelDeclarationIfDoesntExist(currentNode, operation, targetSchema, className, shortestNamespace), CollectionKind = targetSchema.IsArray() ? CodeTypeBase.CodeTypeCollectionKind.Complex : default };// so we don't create unnecessary union types when anyOf was used only for nullable. } @@ -1680,7 +1701,7 @@ private CodeTypeBase CreateComposedModelDeclaration(OpenApiUrlTreeNode currentNo if (!string.IsNullOrEmpty(schema.Reference?.Id)) unionType.TargetNamespace = codeNamespace.GetRootNamespace().FindOrAddNamespace(GetModelsNamespaceNameFromReferenceId(schema.Reference.Id)); unionType.DiscriminatorInformation.DiscriminatorPropertyName = schema.GetDiscriminatorPropertyName(); - GetDiscriminatorMappings(currentNode, schema, codeNamespace, null) + GetDiscriminatorMappings(currentNode, schema, codeNamespace, null, operation) ?.ToList() .ForEach(x => unionType.DiscriminatorInformation.AddDiscriminatorMapping(x.Key, x.Value)); var membersWithNoName = 0; @@ -1699,7 +1720,7 @@ private CodeTypeBase CreateComposedModelDeclaration(OpenApiUrlTreeNode currentNo className = $"{unionType.Name}Member{++membersWithNoName}"; var declarationType = new CodeType { - TypeDefinition = AddModelDeclarationIfDoesntExist(currentNode, currentSchema, className, shortestNamespace), + TypeDefinition = AddModelDeclarationIfDoesntExist(currentNode, operation, currentSchema, className, shortestNamespace), CollectionKind = currentSchema.IsArray() ? CodeTypeBase.CodeTypeCollectionKind.Complex : default }; if (!unionType.ContainsType(declarationType)) @@ -1788,23 +1809,20 @@ private CodeNamespace GetSearchNamespace(OpenApiUrlTreeNode currentNode, CodeNam return currentNamespace; } private ConcurrentDictionary classLifecycles = new(StringComparer.OrdinalIgnoreCase); - private CodeElement AddModelDeclarationIfDoesntExist(OpenApiUrlTreeNode currentNode, OpenApiSchema schema, string declarationName, CodeNamespace currentNamespace, CodeClass? inheritsFrom = null, OpenApiSchema? parentSchemaToExcludeForIntersections = null) + private CodeElement AddModelDeclarationIfDoesntExist(OpenApiUrlTreeNode currentNode, OpenApiOperation? currentOperation, OpenApiSchema schema, string declarationName, CodeNamespace currentNamespace, CodeClass? inheritsFrom = null) { if (GetExistingDeclaration(currentNamespace, currentNode, declarationName) is not CodeElement existingDeclaration) // we can find it in the components { if (AddEnumDeclaration(currentNode, schema, declarationName, currentNamespace) is CodeEnum enumDeclaration) return enumDeclaration; - if (schema.IsIntersection() && - (parentSchemaToExcludeForIntersections is null ? - schema.MergeIntersectionSchemaEntries() : - schema.MergeIntersectionSchemaEntries([parentSchemaToExcludeForIntersections])) is OpenApiSchema mergedSchema && - AddModelDeclarationIfDoesntExist(currentNode, mergedSchema, declarationName, currentNamespace, inheritsFrom) is CodeClass createdClass) + if (schema.IsIntersection() && schema.MergeIntersectionSchemaEntries() is OpenApiSchema mergedSchema && + AddModelDeclarationIfDoesntExist(currentNode, currentOperation, mergedSchema, declarationName, currentNamespace, inheritsFrom) is CodeClass createdClass) { // multiple allOf entries that do not translate to inheritance return createdClass; } - return AddModelClass(currentNode, schema, declarationName, currentNamespace, inheritsFrom); + return AddModelClass(currentNode, schema, declarationName, currentNamespace, currentOperation, inheritsFrom); } return existingDeclaration; } @@ -1880,13 +1898,13 @@ private CodeNamespace GetShortestNamespace(CodeNamespace currentNamespace, OpenA } return currentNamespace; } - private CodeClass AddModelClass(OpenApiUrlTreeNode currentNode, OpenApiSchema schema, string declarationName, CodeNamespace currentNamespace, CodeClass? inheritsFrom = null) + private CodeClass AddModelClass(OpenApiUrlTreeNode currentNode, OpenApiSchema schema, string declarationName, CodeNamespace currentNamespace, OpenApiOperation? currentOperation, CodeClass? inheritsFrom = null) { 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); + inheritsFrom = (CodeClass)AddModelDeclarationIfDoesntExist(currentNode, currentOperation, parentSchema, parentSchema.GetSchemaName().CleanupSymbolName(), parentClassNamespace); } var newClassStub = new CodeClass { @@ -1931,17 +1949,21 @@ private CodeClass AddModelClass(OpenApiUrlTreeNode currentNode, OpenApiSchema sc } } - var mappings = GetDiscriminatorMappings(currentNode, schema, currentNamespace, newClass) + 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 AddDiscriminatorMethod(newClass, schema.GetDiscriminatorPropertyName(), mappings, static s => s); return newClass; } - private IEnumerable> GetDiscriminatorMappings(OpenApiUrlTreeNode currentNode, OpenApiSchema schema, CodeNamespace currentNamespace, CodeClass? baseClass) + private IEnumerable> GetDiscriminatorMappings(OpenApiUrlTreeNode currentNode, OpenApiSchema schema, CodeNamespace currentNamespace, CodeClass? baseClass, OpenApiOperation? currentOperation) { return schema.GetDiscriminatorMappings(inheritanceIndex) - .Select(x => KeyValuePair.Create(x.Key, GetCodeTypeForMapping(currentNode, x.Value, currentNamespace, baseClass, schema))) + .Union(baseClass is not null && modelsNamespace is not null && + (openApiDocument?.Components?.Schemas?.TryGetValue(baseClass.GetComponentSchemaName(modelsNamespace), out var componentSchema) ?? false) ? + componentSchema.GetDiscriminatorMappings(inheritanceIndex) : + []) + .Select(x => KeyValuePair.Create(x.Key, GetCodeTypeForMapping(currentNode, x.Value, currentNamespace, baseClass, currentOperation))) .Where(static x => x.Value != null) .Select(static x => KeyValuePair.Create(x.Key, x.Value!)); } @@ -2115,7 +2137,7 @@ internal static void AddDiscriminatorMethod(CodeClass newClass, string discrimin newClass.DiscriminatorInformation.DiscriminatorPropertyName = discriminatorPropertyName; newClass.AddMethod(factoryMethod); } - private CodeType? GetCodeTypeForMapping(OpenApiUrlTreeNode currentNode, string referenceId, CodeNamespace currentNamespace, CodeClass? baseClass, OpenApiSchema currentSchema) + private CodeType? GetCodeTypeForMapping(OpenApiUrlTreeNode currentNode, string referenceId, CodeNamespace currentNamespace, CodeClass? baseClass, OpenApiOperation? currentOperation) { var componentKey = referenceId?.Replace("#/components/schemas/", string.Empty, StringComparison.OrdinalIgnoreCase); if (openApiDocument == null || !openApiDocument.Components.Schemas.TryGetValue(componentKey, out var discriminatorSchema)) @@ -2123,56 +2145,40 @@ internal static void AddDiscriminatorMethod(CodeClass newClass, string discrimin logger.LogWarning("Discriminator {ComponentKey} not found in the OpenAPI document.", componentKey); return null; } - 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 && shouldInherit && !discriminatorSchema.IsInherited()) + if (CreateModelDeclarations(currentNode, discriminatorSchema, currentOperation, GetShortestNamespace(currentNamespace, discriminatorSchema), string.Empty) is not CodeType result) { - logger.LogWarning("Discriminator {ComponentKey} is not inherited from {ClassName}.", componentKey, baseClass.Name); + logger.LogWarning("Discriminator {ComponentKey} is not a valid model and points to a union type.", componentKey); return null; } - var codeClass = AddModelDeclarationIfDoesntExist(currentNode, discriminatorSchema, className, GetShortestNamespace(currentNamespace, discriminatorSchema), shouldInherit ? baseClass : null, currentSchema); - return new CodeType + if (baseClass is not null && (result.TypeDefinition is not CodeClass codeClass || codeClass.StartBlock.Inherits is null)) { - TypeDefinition = codeClass, - }; + logger.LogWarning("Discriminator {ComponentKey} is not inherited from {ClassName}.", componentKey, baseClass.Name); + return null; + } + return result; } private void CreatePropertiesForModelClass(OpenApiUrlTreeNode currentNode, OpenApiSchema schema, CodeNamespace ns, CodeClass model) { - if (CollectAllProperties(schema) is var properties && properties.Count != 0) - { - var propertiesToAdd = properties - .Select(x => + var propertiesToAdd = schema.Properties + .Select(x => + { + var propertySchema = x.Value; + var className = $"{model.Name}_{x.Key.CleanupSymbolName()}"; + var shortestNamespaceName = GetModelsNamespaceNameFromReferenceId(propertySchema.Reference?.Id); + var targetNamespace = string.IsNullOrEmpty(shortestNamespaceName) ? ns : + rootNamespace?.FindOrAddNamespace(shortestNamespaceName) ?? ns; + var definition = CreateModelDeclarations(currentNode, propertySchema, default, targetNamespace, string.Empty, typeNameForInlineSchema: className); + if (definition == null) { - var propertySchema = x.Value; - var className = $"{model.Name}_{x.Key.CleanupSymbolName()}"; - var shortestNamespaceName = GetModelsNamespaceNameFromReferenceId(propertySchema.Reference?.Id); - var targetNamespace = string.IsNullOrEmpty(shortestNamespaceName) ? ns : - rootNamespace?.FindOrAddNamespace(shortestNamespaceName) ?? ns; - var definition = CreateModelDeclarations(currentNode, propertySchema, default, targetNamespace, string.Empty, typeNameForInlineSchema: className); - if (definition == null) - { - logger.LogWarning("Omitted property {PropertyName} for model {ModelName} in API path {ApiPath}, the schema is invalid.", x.Key, model.Name, currentNode.Path); - return null; - } - return CreateProperty(x.Key, definition.Name, propertySchema: propertySchema, existingType: definition); - }) - .OfType() - .ToArray(); - if (propertiesToAdd.Length != 0) - model.AddProperty(propertiesToAdd); - } - } - private Dictionary CollectAllProperties(OpenApiSchema schema) - { - Dictionary result = schema.Properties?.ToDictionary(static x => x.Key, static x => x.Value, StringComparer.Ordinal) ?? new(StringComparer.Ordinal); - if (schema.AllOf?.Any() ?? false) - { - foreach (var supProperty in schema.AllOf.Where(static x => !x.IsReferencedSchema() && x.HasAnyProperty()).SelectMany(static x => x.Properties)) - { - result.Add(supProperty.Key, supProperty.Value); - } - } - return result; + logger.LogWarning("Omitted property {PropertyName} for model {ModelName} in API path {ApiPath}, the schema is invalid.", x.Key, model.Name, currentNode.Path); + return null; + } + return CreateProperty(x.Key, definition.Name, propertySchema: propertySchema, existingType: definition); + }) + .OfType() + .ToArray(); + if (propertiesToAdd.Length != 0) + model.AddProperty(propertiesToAdd); } private const string FieldDeserializersMethodName = "GetFieldDeserializers"; private const string SerializeMethodName = "Serialize"; diff --git a/tests/Kiota.Builder.Tests/KiotaBuilderTests.cs b/tests/Kiota.Builder.Tests/KiotaBuilderTests.cs index 7240e27a4f..932f07712b 100644 --- a/tests/Kiota.Builder.Tests/KiotaBuilderTests.cs +++ b/tests/Kiota.Builder.Tests/KiotaBuilderTests.cs @@ -3487,7 +3487,7 @@ public async Task AddsDiscriminatorMappingsAllOfImplicitWithParentHavingMappings var directoryObjectSchema = new OpenApiSchema { Type = "object", - AllOf = new List { + AllOf = [ entitySchema, new OpenApiSchema { Properties = new Dictionary { @@ -3506,7 +3506,7 @@ public async Task AddsDiscriminatorMappingsAllOfImplicitWithParentHavingMappings "@odata.type" } } - }, + ], Reference = new OpenApiReference { Id = "microsoft.graph.directoryObject", @@ -3517,7 +3517,7 @@ public async Task AddsDiscriminatorMappingsAllOfImplicitWithParentHavingMappings var userSchema = new OpenApiSchema { Type = "object", - AllOf = new List { + AllOf = [ directoryObjectSchema, new OpenApiSchema { Properties = new Dictionary { @@ -3536,7 +3536,7 @@ public async Task AddsDiscriminatorMappingsAllOfImplicitWithParentHavingMappings "@odata.type" } } - }, + ], Reference = new OpenApiReference { Id = "microsoft.graph.user", From 780bcefb6acb1b6d7180c062d1a3d1f20eba5713 Mon Sep 17 00:00:00 2001 From: Vincent Biret Date: Fri, 24 May 2024 13:27:40 -0400 Subject: [PATCH 2/4] chore: fixes formatting --- tests/Kiota.Builder.Tests/KiotaBuilderTests.cs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/Kiota.Builder.Tests/KiotaBuilderTests.cs b/tests/Kiota.Builder.Tests/KiotaBuilderTests.cs index 932f07712b..84decd6e6f 100644 --- a/tests/Kiota.Builder.Tests/KiotaBuilderTests.cs +++ b/tests/Kiota.Builder.Tests/KiotaBuilderTests.cs @@ -3489,7 +3489,8 @@ public async Task AddsDiscriminatorMappingsAllOfImplicitWithParentHavingMappings Type = "object", AllOf = [ entitySchema, - new OpenApiSchema { + new OpenApiSchema + { Properties = new Dictionary { { "tenant", new OpenApiSchema { @@ -3519,7 +3520,8 @@ public async Task AddsDiscriminatorMappingsAllOfImplicitWithParentHavingMappings Type = "object", AllOf = [ directoryObjectSchema, - new OpenApiSchema { + new OpenApiSchema + { Properties = new Dictionary { { "firstName", new OpenApiSchema { From c9086a8748495d1249794d1f087952326788abe8 Mon Sep 17 00:00:00 2001 From: Andrew Omondi Date: Tue, 28 May 2024 13:48:52 +0300 Subject: [PATCH 3/4] Update src/Kiota.Builder/KiotaBuilder.cs Co-authored-by: silaskenneth --- src/Kiota.Builder/KiotaBuilder.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Kiota.Builder/KiotaBuilder.cs b/src/Kiota.Builder/KiotaBuilder.cs index 0955e37633..f13d8238de 100644 --- a/src/Kiota.Builder/KiotaBuilder.cs +++ b/src/Kiota.Builder/KiotaBuilder.cs @@ -1816,7 +1816,7 @@ private CodeElement AddModelDeclarationIfDoesntExist(OpenApiUrlTreeNode currentN if (AddEnumDeclaration(currentNode, schema, declarationName, currentNamespace) is CodeEnum enumDeclaration) return enumDeclaration; - if (schema.IsIntersection() && schema.MergeIntersectionSchemaEntries() is OpenApiSchema mergedSchema && + if (schema.IsIntersection() && schema.MergeIntersectionSchemaEntries() is {} mergedSchema && AddModelDeclarationIfDoesntExist(currentNode, currentOperation, mergedSchema, declarationName, currentNamespace, inheritsFrom) is CodeClass createdClass) { // multiple allOf entries that do not translate to inheritance From a6feb0e78c2fa759db163c5ca4b6d5b00bf9d6d6 Mon Sep 17 00:00:00 2001 From: Andrew Omondi Date: Tue, 28 May 2024 15:34:16 +0300 Subject: [PATCH 4/4] Fixes aggressive flattening --- src/Kiota.Builder/Extensions/OpenApiSchemaExtensions.cs | 2 +- src/Kiota.Builder/KiotaBuilder.cs | 2 +- tests/Kiota.Builder.Tests/KiotaBuilderTests.cs | 3 +++ 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/Kiota.Builder/Extensions/OpenApiSchemaExtensions.cs b/src/Kiota.Builder/Extensions/OpenApiSchemaExtensions.cs index 004084d40e..375d4f6ae7 100644 --- a/src/Kiota.Builder/Extensions/OpenApiSchemaExtensions.cs +++ b/src/Kiota.Builder/Extensions/OpenApiSchemaExtensions.cs @@ -29,7 +29,7 @@ public static IEnumerable GetSchemaNames(this OpenApiSchema schema, bool internal static IEnumerable FlattenSchemaIfRequired(this IList schemas, Func> subsequentGetter) { if (schemas is null) return []; - return schemas.Count == 1 && !schemas[0].HasAnyProperty() ? + return schemas.Count == 1 && !schemas[0].HasAnyProperty() && string.IsNullOrEmpty(schemas[0].Reference?.Id) ? schemas.FlattenEmptyEntries(subsequentGetter, 1) : schemas; } diff --git a/src/Kiota.Builder/KiotaBuilder.cs b/src/Kiota.Builder/KiotaBuilder.cs index f13d8238de..ef05625247 100644 --- a/src/Kiota.Builder/KiotaBuilder.cs +++ b/src/Kiota.Builder/KiotaBuilder.cs @@ -1816,7 +1816,7 @@ private CodeElement AddModelDeclarationIfDoesntExist(OpenApiUrlTreeNode currentN if (AddEnumDeclaration(currentNode, schema, declarationName, currentNamespace) is CodeEnum enumDeclaration) return enumDeclaration; - if (schema.IsIntersection() && schema.MergeIntersectionSchemaEntries() is {} mergedSchema && + if (schema.IsIntersection() && schema.MergeIntersectionSchemaEntries() is { } mergedSchema && AddModelDeclarationIfDoesntExist(currentNode, currentOperation, mergedSchema, declarationName, currentNamespace, inheritsFrom) is CodeClass createdClass) { // multiple allOf entries that do not translate to inheritance diff --git a/tests/Kiota.Builder.Tests/KiotaBuilderTests.cs b/tests/Kiota.Builder.Tests/KiotaBuilderTests.cs index 84decd6e6f..19c2d8a249 100644 --- a/tests/Kiota.Builder.Tests/KiotaBuilderTests.cs +++ b/tests/Kiota.Builder.Tests/KiotaBuilderTests.cs @@ -7469,6 +7469,9 @@ public async Task InlineSchemaWithSingleAllOfReference() var memberClass = codeModel.FindChildByName("member"); Assert.NotNull(memberClass); Assert.Equal(2, memberClass.Properties.Count());// single prop plus additionalData + var memberProperty = memberClass.Properties.FirstOrDefault(static x => x.Name.Equals("group", StringComparison.OrdinalIgnoreCase)); + Assert.NotNull(memberProperty); + Assert.Equal("group", memberProperty.Type.Name); Assert.Null(memberClass.StartBlock.Inherits);//no base var userClass = codeModel.FindChildByName("user"); Assert.NotNull(userClass);