From afeed588ae7ec5a6c4b4e7d6e567cba41f526a8d Mon Sep 17 00:00:00 2001 From: Vincent Biret Date: Mon, 18 Sep 2023 15:29:20 -0400 Subject: [PATCH 1/2] - maps 4xx and 5xx to default when absent --- CHANGELOG.md | 1 + src/Kiota.Builder/CodeDOM/CodeMethod.cs | 5 + src/Kiota.Builder/KiotaBuilder.cs | 53 +++++---- .../Kiota.Builder.Tests/KiotaBuilderTests.cs | 109 ++++++++++++++++++ 4 files changed, 148 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 73622037a2..703427eec2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Localhost based descriptions are not cached anymore to facilitate development workflows. [#3316](https://github.com/microsoft/kiota/issues/3316) - Changed parameter order in with_url method body to match the signature of RequestBuilder constructor in Python. [#3328](https://github.com/microsoft/kiota/issues/3328 - Removed redundant undefined qualifier in TypeScript for properties. [#3244](https://github.com/microsoft/kiota/issues/3244) +- The default status code response is now used as 4XX and 5XX when those class responses are not provided in the description. [#3245](https://github.com/microsoft/kiota/issues/3245) ## [1.6.1] - 2023-09-11 diff --git a/src/Kiota.Builder/CodeDOM/CodeMethod.cs b/src/Kiota.Builder/CodeDOM/CodeMethod.cs index f8db7511cc..1d2cc952db 100644 --- a/src/Kiota.Builder/CodeDOM/CodeMethod.cs +++ b/src/Kiota.Builder/CodeDOM/CodeMethod.cs @@ -263,6 +263,11 @@ public IOrderedEnumerable> ErrorMappings return errorMappings.OrderBy(static x => x.Key); } } + public bool HasErrorMappingCode(string code) + { + ArgumentException.ThrowIfNullOrEmpty(code); + return errorMappings.ContainsKey(code); + } public DeprecationInformation? Deprecation { diff --git a/src/Kiota.Builder/KiotaBuilder.cs b/src/Kiota.Builder/KiotaBuilder.cs index a8fc56d995..9c64aa8cb9 100644 --- a/src/Kiota.Builder/KiotaBuilder.cs +++ b/src/Kiota.Builder/KiotaBuilder.cs @@ -1224,31 +1224,44 @@ openApiExtension is OpenApiPrimaryErrorMessageExtension primaryErrorMessageExten private const string RequestBodyPlainTextContentType = "text/plain"; private static readonly HashSet noContentStatusCodes = new() { "201", "202", "204", "205" }; private static readonly HashSet errorStatusCodes = new(Enumerable.Range(400, 599).Select(static x => x.ToString(CultureInfo.InvariantCulture)) - .Concat(new[] { "4XX", "5XX" }), StringComparer.OrdinalIgnoreCase); - + .Concat(new[] { FourXXError, FiveXXError }), StringComparer.OrdinalIgnoreCase); + private const string FourXXError = "4XX"; + private const string FiveXXError = "5XX"; private void AddErrorMappingsForExecutorMethod(OpenApiUrlTreeNode currentNode, OpenApiOperation operation, CodeMethod executorMethod) { foreach (var response in operation.Responses.Where(x => errorStatusCodes.Contains(x.Key))) { - var errorCode = response.Key.ToUpperInvariant(); - var errorSchema = response.Value.GetResponseSchema(config.StructuredMimeTypes); - if (errorSchema != null && modelsNamespace != null) - { - var parentElement = string.IsNullOrEmpty(response.Value.Reference?.Id) && string.IsNullOrEmpty(errorSchema.Reference?.Id) - ? (CodeElement)executorMethod - : modelsNamespace; - var errorType = CreateModelDeclarations(currentNode, errorSchema, operation, parentElement, $"{errorCode}Error", response: response.Value); - if (errorType is CodeType codeType && - codeType.TypeDefinition is CodeClass codeClass && - !codeClass.IsErrorDefinition) - { - codeClass.IsErrorDefinition = true; - } - if (errorType is null) - logger.LogWarning("Could not create error type for {Error} in {Operation}", errorCode, operation.OperationId); - else - executorMethod.AddErrorMapping(errorCode, errorType); + if (response.Value.GetResponseSchema(config.StructuredMimeTypes) is { } schema) + { + AddErrorMappingToExecutorMethod(currentNode, operation, executorMethod, schema, response.Value, response.Key.ToUpperInvariant()); + } + } + if (operation.Responses.TryGetValue("default", out var defaultResponse) && defaultResponse.GetResponseSchema(config.StructuredMimeTypes) is { } errorSchema) + { + if (!executorMethod.HasErrorMappingCode(FourXXError)) + AddErrorMappingToExecutorMethod(currentNode, operation, executorMethod, errorSchema, defaultResponse, FourXXError); + if (!executorMethod.HasErrorMappingCode(FiveXXError)) + AddErrorMappingToExecutorMethod(currentNode, operation, executorMethod, errorSchema, defaultResponse, FiveXXError); + } + } + private void AddErrorMappingToExecutorMethod(OpenApiUrlTreeNode currentNode, OpenApiOperation operation, CodeMethod executorMethod, OpenApiSchema errorSchema, OpenApiResponse response, string errorCode) + { + if (modelsNamespace != null) + { + var parentElement = string.IsNullOrEmpty(response.Reference?.Id) && string.IsNullOrEmpty(errorSchema.Reference?.Id) + ? (CodeElement)executorMethod + : modelsNamespace; + var errorType = CreateModelDeclarations(currentNode, errorSchema, operation, parentElement, $"{errorCode}Error", response: response); + if (errorType is CodeType codeType && + codeType.TypeDefinition is CodeClass codeClass && + !codeClass.IsErrorDefinition) + { + codeClass.IsErrorDefinition = true; } + if (errorType is null) + logger.LogWarning("Could not create error type for {Error} in {Operation}", errorCode, operation.OperationId); + else + executorMethod.AddErrorMapping(errorCode, errorType); } } private CodeTypeBase? GetExecutorMethodReturnType(OpenApiUrlTreeNode currentNode, OpenApiSchema? schema, OpenApiOperation operation, CodeClass parentClass) diff --git a/tests/Kiota.Builder.Tests/KiotaBuilderTests.cs b/tests/Kiota.Builder.Tests/KiotaBuilderTests.cs index 4fdb78f5c1..08857a7638 100644 --- a/tests/Kiota.Builder.Tests/KiotaBuilderTests.cs +++ b/tests/Kiota.Builder.Tests/KiotaBuilderTests.cs @@ -2089,6 +2089,115 @@ public void DoesntAddSuffixesToErrorTypesWhenComponents() Assert.Null(codeModel.FindChildByName("tasks5XXError")); } [Fact] + public void UsesDefaultAs4XXAnd5XXWhenAbsent() + { + var errorSchema = new OpenApiSchema + { + Type = "object", + Properties = new Dictionary { + { + "errorId", new OpenApiSchema { + Type = "string" + } + } + }, + Reference = new OpenApiReference + { + Id = "microsoft.graph.error", + Type = ReferenceType.Schema + }, + UnresolvedReference = false + }; + var errorResponse = new OpenApiResponse + { + Content = + { + ["application/json"] = new OpenApiMediaType + { + Schema = errorSchema + } + }, + Reference = new OpenApiReference + { + Id = "microsoft.graph.error", + Type = ReferenceType.Response + }, + UnresolvedReference = false + }; + var document = new OpenApiDocument + { + Paths = new OpenApiPaths + { + ["tasks"] = new OpenApiPathItem + { + Operations = { + [OperationType.Get] = new OpenApiOperation + { + Responses = new OpenApiResponses + { + ["200"] = new OpenApiResponse + { + Content = + { + ["application/json"] = new OpenApiMediaType + { + Schema = new OpenApiSchema + { + Type = "object", + Properties = new Dictionary { + { + "progress", new OpenApiSchema{ + Type = "string", + } + } + } + } + } + } + }, + ["default"] = errorResponse, + ["401"] = errorResponse + } + } + } + } + }, + Components = new OpenApiComponents + { + Schemas = new Dictionary { + { + "microsoft.graph.error", errorSchema + } + }, + Responses = new Dictionary { + { + "microsoft.graph.error", errorResponse + } + } + }, + }; + var mockLogger = new Mock>(); + var builder = new KiotaBuilder(mockLogger.Object, new GenerationConfiguration { ClientClassName = "Graph", ApiRootUrl = "https://localhost" }, _httpClient); + builder.SetOpenApiDocument(document); + var node = builder.CreateUriSpace(document); + var codeModel = builder.CreateSourceModel(node); + var executorMethod = codeModel.FindChildByName("get"); + Assert.NotNull(executorMethod); + Assert.NotEmpty(executorMethod.ErrorMappings); + var keys = executorMethod.ErrorMappings.Select(static x => x.Key).ToHashSet(); + Assert.Contains("4XX", keys); + Assert.Contains("401", keys); + Assert.Contains("5XX", keys); + var errorType = codeModel.FindChildByName("Error"); + Assert.NotNull(errorType); + Assert.True(errorType.IsErrorDefinition); + Assert.NotNull(errorType.FindChildByName("errorId")); + + Assert.Null(codeModel.FindChildByName("tasks401Error")); + Assert.Null(codeModel.FindChildByName("tasks4XXError")); + Assert.Null(codeModel.FindChildByName("tasks5XXError")); + } + [Fact] public void DoesntAddPropertyHolderOnNonAdditionalModels() { var weatherForecastSchema = new OpenApiSchema From 4debdf7f0fc5513b34f9787d9ab705dc594514b4 Mon Sep 17 00:00:00 2001 From: Vincent Biret Date: Tue, 19 Sep 2023 08:48:01 -0400 Subject: [PATCH 2/2] - fixes a bug where identifying the primary error message would lead to a stack overflow --- .../Writers/ProprietableBlockExtensions.cs | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/src/Kiota.Builder/Writers/ProprietableBlockExtensions.cs b/src/Kiota.Builder/Writers/ProprietableBlockExtensions.cs index 6c71045564..8500c1cea7 100644 --- a/src/Kiota.Builder/Writers/ProprietableBlockExtensions.cs +++ b/src/Kiota.Builder/Writers/ProprietableBlockExtensions.cs @@ -1,4 +1,5 @@ using System; +using System.Collections.Generic; using System.Linq; using Kiota.Builder.CodeDOM; @@ -12,8 +13,14 @@ internal static class ProprietableBlockExtensions internal static string GetPrimaryMessageCodePath(this ProprietableBlock block, Func propertyNameNormalization, Func methodNameNormalization, - string pathSegment = ".") where TBlockKind : Enum where TBlockDeclaration : ProprietableBlockDeclaration, new() + string pathSegment = ".", + HashSet? visitedElements = default) where TBlockKind : Enum where TBlockDeclaration : ProprietableBlockDeclaration, new() { + visitedElements ??= new(); + if (visitedElements.Contains(block)) + return string.Empty; + else + visitedElements.Add(block); if (block is CodeInterface currentInterface) { if (currentInterface.Methods.FirstOrDefault(static x => isGetterMethod(x) && x.AccessedProperty is not null && isPrimaryErrorMessage(x.AccessedProperty)) is CodeMethod primaryErrorMessageMethod) @@ -22,10 +29,10 @@ internal static string GetPrimaryMessageCodePath( return propertyNameNormalization(primaryErrorMessageProperty); else if (currentInterface.Methods .Where(isGetterMethod) - .Select(x => new { Value = x.ReturnType is CodeType codeType && codeType.TypeDefinition is CodeInterface codeInterface && codeInterface.GetPrimaryMessageCodePath(propertyNameNormalization, methodNameNormalization, pathSegment) is string segment && !string.IsNullOrEmpty(segment) ? $"{methodNameNormalization(x)}{pathSegment}{segment}" : string.Empty, IsMethod = true }) + .Select(x => new { Value = x.ReturnType is CodeType codeType && codeType.TypeDefinition is CodeInterface codeInterface && codeInterface.GetPrimaryMessageCodePath(propertyNameNormalization, methodNameNormalization, pathSegment, visitedElements) is string segment && !string.IsNullOrEmpty(segment) ? $"{methodNameNormalization(x)}{pathSegment}{segment}" : string.Empty, IsMethod = true }) .Union(currentInterface.Properties .Where(isCustomProperty) - .Select(x => new { Value = x.Type is CodeType codeType && codeType.TypeDefinition is CodeInterface codeInterface && codeInterface.GetPrimaryMessageCodePath(propertyNameNormalization, methodNameNormalization, pathSegment) is string segment && !string.IsNullOrEmpty(segment) ? $"{propertyNameNormalization(x)}{pathSegment}{segment}" : string.Empty, IsMethod = false })) + .Select(x => new { Value = x.Type is CodeType codeType && codeType.TypeDefinition is CodeInterface codeInterface && codeInterface.GetPrimaryMessageCodePath(propertyNameNormalization, methodNameNormalization, pathSegment, visitedElements) is string segment && !string.IsNullOrEmpty(segment) ? $"{propertyNameNormalization(x)}{pathSegment}{segment}" : string.Empty, IsMethod = false })) .OrderBy(static x => x.IsMethod) .ThenBy(static x => x.Value, StringComparer.OrdinalIgnoreCase) .FirstOrDefault(static x => !string.IsNullOrEmpty(x.Value)) is { } primaryMessageCodePath) @@ -40,10 +47,10 @@ internal static string GetPrimaryMessageCodePath( return propertyNameNormalization(primaryErrorMessageProperty); else if (currentClass.Methods .Where(isGetterMethod) - .Select(x => new { Value = x.ReturnType is CodeType codeType && codeType.TypeDefinition is CodeClass codeClass && codeClass.GetPrimaryMessageCodePath(propertyNameNormalization, methodNameNormalization, pathSegment) is string segment && !string.IsNullOrEmpty(segment) ? $"{methodNameNormalization(x)}{pathSegment}{segment}" : string.Empty, IsMethod = true }) + .Select(x => new { Value = x.ReturnType is CodeType codeType && codeType.TypeDefinition is CodeClass codeClass && codeClass.GetPrimaryMessageCodePath(propertyNameNormalization, methodNameNormalization, pathSegment, visitedElements) is string segment && !string.IsNullOrEmpty(segment) ? $"{methodNameNormalization(x)}{pathSegment}{segment}" : string.Empty, IsMethod = true }) .Union(currentClass.Properties .Where(isCustomProperty) - .Select(x => new { Value = x.Type is CodeType codeType && codeType.TypeDefinition is CodeClass codeClass && codeClass.GetPrimaryMessageCodePath(propertyNameNormalization, methodNameNormalization, pathSegment) is string segment && !string.IsNullOrEmpty(segment) ? $"{propertyNameNormalization(x)}{pathSegment}{segment}" : string.Empty, IsMethod = false })) + .Select(x => new { Value = x.Type is CodeType codeType && codeType.TypeDefinition is CodeClass codeClass && codeClass.GetPrimaryMessageCodePath(propertyNameNormalization, methodNameNormalization, pathSegment, visitedElements) is string segment && !string.IsNullOrEmpty(segment) ? $"{propertyNameNormalization(x)}{pathSegment}{segment}" : string.Empty, IsMethod = false })) .OrderBy(static x => x.IsMethod) .ThenBy(static x => x.Value, StringComparer.OrdinalIgnoreCase) .FirstOrDefault(static x => !string.IsNullOrEmpty(x.Value)) is { } primaryMessageCodePath)