From 73214a0fdd06903902cb195f8a3aa8bbe4ce571f Mon Sep 17 00:00:00 2001 From: Vincent Biret Date: Mon, 18 Sep 2023 15:29:20 -0400 Subject: [PATCH] - maps 4xx and 5xx to default when absent --- CHANGELOG.md | 2 + src/Kiota.Builder/CodeDOM/CodeMethod.cs | 5 + src/Kiota.Builder/KiotaBuilder.cs | 53 +++++---- .../Kiota.Builder.Tests/KiotaBuilderTests.cs | 109 ++++++++++++++++++ 4 files changed, 149 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f80ffb049a..068f047538 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- 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 ### Changed 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