From 85d5858d1b1796e873cfe118b75d20f92b8eea03 Mon Sep 17 00:00:00 2001 From: Vincent Biret Date: Wed, 11 Oct 2023 15:51:11 -0400 Subject: [PATCH 1/7] - adds support for enum types in query parameters --- src/Kiota.Builder/KiotaBuilder.cs | 108 +++++++++++++----- .../Kiota.Builder.Tests/KiotaBuilderTests.cs | 61 +++++++++- 2 files changed, 139 insertions(+), 30 deletions(-) diff --git a/src/Kiota.Builder/KiotaBuilder.cs b/src/Kiota.Builder/KiotaBuilder.cs index c4162b2130..dda41cd4d3 100644 --- a/src/Kiota.Builder/KiotaBuilder.cs +++ b/src/Kiota.Builder/KiotaBuilder.cs @@ -1223,7 +1223,7 @@ openApiExtension is OpenApiPrimaryErrorMessageExtension primaryErrorMessageExten }; } private const string RequestBodyPlainTextContentType = "text/plain"; - private static readonly HashSet noContentStatusCodes = new() { "201", "202", "204", "205" }; + private static readonly HashSet noContentStatusCodes = new(StringComparer.OrdinalIgnoreCase) { "201", "202", "204", "205" }; private static readonly HashSet errorStatusCodes = new(Enumerable.Range(400, 599).Select(static x => x.ToString(CultureInfo.InvariantCulture)) .Concat(new[] { FourXXError, FiveXXError }), StringComparer.OrdinalIgnoreCase); private const string FourXXError = "4XX"; @@ -1793,35 +1793,49 @@ private CodeElement AddModelDeclarationIfDoesntExist(OpenApiUrlTreeNode currentN { if (GetExistingDeclaration(currentNamespace, currentNode, declarationName) is not CodeElement existingDeclaration) // we can find it in the components { - if (schema.IsEnum()) - { - var schemaDescription = schema.Description.CleanupDescription(); - OpenApiEnumFlagsExtension? enumFlagsExtension = null; - if (schema.Extensions.TryGetValue(OpenApiEnumFlagsExtension.Name, out var rawExtension) && - rawExtension is OpenApiEnumFlagsExtension flagsExtension) - { - enumFlagsExtension = flagsExtension; - } - var newEnum = new CodeEnum - { - Name = declarationName, - Flags = enumFlagsExtension?.IsFlags ?? false, - Documentation = new() - { - Description = !string.IsNullOrEmpty(schemaDescription) || !string.IsNullOrEmpty(schema.Reference?.Id) ? - schemaDescription : // if it's a referenced component, we shouldn't use the path item description as it makes it indeterministic - currentNode.GetPathItemDescription(Constants.DefaultOpenApiLabel), - }, - Deprecation = schema.GetDeprecationInformation(), - }; - SetEnumOptions(schema, newEnum); - return currentNamespace.AddEnum(newEnum).First(); - } + if (AddEnumDeclaration(currentNode, schema, declarationName, currentNamespace) is CodeEnum enumDeclaration) + return enumDeclaration; return AddModelClass(currentNode, schema, declarationName, currentNamespace, inheritsFrom); } return existingDeclaration; } + private CodeEnum? AddEnumDeclarationIfDoesntExist(OpenApiUrlTreeNode currentNode, OpenApiSchema schema, string declarationName, CodeNamespace currentNamespace) + { + if (GetExistingDeclaration(currentNamespace, currentNode, declarationName) is not CodeEnum existingDeclaration) // we can find it in the components + { + return AddEnumDeclaration(currentNode, schema, declarationName, currentNamespace); + } + return existingDeclaration; + } + private CodeEnum? AddEnumDeclaration(OpenApiUrlTreeNode currentNode, OpenApiSchema schema, string declarationName, CodeNamespace currentNamespace) + { + if (schema.IsEnum()) + { + var schemaDescription = schema.Description.CleanupDescription(); + OpenApiEnumFlagsExtension? enumFlagsExtension = null; + if (schema.Extensions.TryGetValue(OpenApiEnumFlagsExtension.Name, out var rawExtension) && + rawExtension is OpenApiEnumFlagsExtension flagsExtension) + { + enumFlagsExtension = flagsExtension; + } + var newEnum = new CodeEnum + { + Name = declarationName, + Flags = enumFlagsExtension?.IsFlags ?? false, + Documentation = new() + { + Description = !string.IsNullOrEmpty(schemaDescription) || !string.IsNullOrEmpty(schema.Reference?.Id) ? + schemaDescription : // if it's a referenced component, we shouldn't use the path item description as it makes it indeterministic + currentNode.GetPathItemDescription(Constants.DefaultOpenApiLabel), + }, + Deprecation = schema.GetDeprecationInformation(), + }; + SetEnumOptions(schema, newEnum); + return currentNamespace.AddEnum(newEnum).First(); + } + return default; + } private static void SetEnumOptions(OpenApiSchema schema, CodeEnum target) { OpenApiEnumValuesDescriptionExtension? extensionInformation = null; @@ -2278,16 +2292,39 @@ internal static void AddSerializationMembers(CodeClass model, bool includeAdditi }, }).First(); foreach (var parameter in parameters) - AddPropertyForQueryParameter(parameter, parameterClass); + AddPropertyForQueryParameter(node, operationType, parameter, parameterClass); return parameterClass; } return null; } - private void AddPropertyForQueryParameter(OpenApiParameter parameter, CodeClass parameterClass) + private void AddPropertyForQueryParameter(OpenApiUrlTreeNode node, OperationType operationType, OpenApiParameter parameter, CodeClass parameterClass) { - var resultType = GetPrimitiveType(parameter.Schema) ?? new CodeType() + CodeType? resultType = default; + var addBackwardCompatibleParameter = false; + if (parameter.Schema.IsEnum()) + { + var schema = parameter.Schema; + var codeNamespace = schema.IsReferencedSchema() switch + { + true => GetShortestNamespace(parameterClass.GetImmediateParentOfType(), schema), // referenced schema + false => parameterClass.GetImmediateParentOfType(), // Inline schema, i.e. specific to the Operation + }; + var shortestNamespace = GetShortestNamespace(codeNamespace, schema); + var enumName = schema.GetSchemaName().CleanupSymbolName(); + if (string.IsNullOrEmpty(enumName)) + enumName = $"{operationType.ToString().ToFirstCharacterUpperCase()}{parameter.Name.ToFirstCharacterUpperCase()}QueryParameterType"; + if (AddEnumDeclarationIfDoesntExist(node, schema, enumName, shortestNamespace) is { } enumDeclaration) + { + resultType = new CodeType + { + TypeDefinition = enumDeclaration, + }; + addBackwardCompatibleParameter = true; + } + } + resultType ??= GetPrimitiveType(parameter.Schema) ?? new CodeType() { // since its a query parameter default to string if there is no schema // it also be an object type, but we'd need to create the model in that case and there's no standard on how to serialize those as query parameters @@ -2314,7 +2351,20 @@ private void AddPropertyForQueryParameter(OpenApiParameter parameter, CodeClass if (!parameterClass.ContainsPropertyWithWireName(prop.WireName)) { - parameterClass.AddProperty(prop); + if (addBackwardCompatibleParameter && config.IncludeBackwardCompatible && config.Language is GenerationLanguage.CSharp or GenerationLanguage.Go) + { //TODO remove for v2 + var modernProp = (CodeProperty)prop.Clone(); + modernProp.Name = $"{prop.Name}As{modernProp.Type.Name.ToFirstCharacterUpperCase()}"; + modernProp.SerializationName = prop.WireName; + prop.Deprecation = new($"This property is deprecated, use {modernProp.Name} instead", IsDeprecated: true); + prop.Type = GetDefaultQueryParameterType(); + prop.Type.CollectionKind = modernProp.Type.CollectionKind; + parameterClass.AddProperty(modernProp, prop); + } + else + { + parameterClass.AddProperty(prop); + } } else { diff --git a/tests/Kiota.Builder.Tests/KiotaBuilderTests.cs b/tests/Kiota.Builder.Tests/KiotaBuilderTests.cs index 2bd57761be..d789480b6d 100644 --- a/tests/Kiota.Builder.Tests/KiotaBuilderTests.cs +++ b/tests/Kiota.Builder.Tests/KiotaBuilderTests.cs @@ -3895,7 +3895,7 @@ public void MapsQueryParameterTypes(string type, string format, string expected) [OperationType.Get] = new OpenApiOperation { Parameters = new List { - new OpenApiParameter { + new() { Name = "query", In = ParameterLocation.Query, Schema = new OpenApiSchema { @@ -3924,6 +3924,65 @@ public void MapsQueryParameterTypes(string type, string format, string expected) Assert.Equal(expected, property.Type.Name); Assert.True(property.Type.AllTypes.First().IsExternal); } + [InlineData(GenerationLanguage.CSharp)] + [InlineData(GenerationLanguage.Java)] + [Theory] + public void MapsEnumQueryParameterType(GenerationLanguage generationLanguage) + { + var document = new OpenApiDocument + { + Paths = new OpenApiPaths + { + ["primitive"] = new OpenApiPathItem + { + Operations = { + [OperationType.Get] = new OpenApiOperation + { + Parameters = new List { + new() { + Name = "query", + In = ParameterLocation.Query, + Schema = new OpenApiSchema { + Type = "string", + Enum = new List { + new OpenApiString("value1"), + new OpenApiString("value2") + } + } + } + }, + Responses = new OpenApiResponses + { + ["204"] = new OpenApiResponse() + } + } + } + } + }, + }; + var mockLogger = new Mock>(); + var builder = new KiotaBuilder(mockLogger.Object, new GenerationConfiguration { ClientClassName = "Graph", ApiRootUrl = "https://localhost", Language = generationLanguage }, _httpClient); + var node = builder.CreateUriSpace(document); + var codeModel = builder.CreateSourceModel(node); + var queryParameters = codeModel.FindChildByName("primitiveRequestBuilderGetQueryParameters"); + Assert.NotNull(queryParameters); + var backwardCompatibleProperty = queryParameters.Properties.FirstOrDefault(static x => x.Name.Equals("query", StringComparison.OrdinalIgnoreCase)); + Assert.NotNull(backwardCompatibleProperty); + if (generationLanguage is GenerationLanguage.CSharp) + { + Assert.Equal("string", backwardCompatibleProperty.Type.Name); + Assert.True(backwardCompatibleProperty.Type.AllTypes.First().IsExternal); + Assert.True(backwardCompatibleProperty.Deprecation.IsDeprecated); + var property = queryParameters.Properties.FirstOrDefault(static x => x.Name.Equals("queryAsGetQueryQueryParameterType", StringComparison.OrdinalIgnoreCase)); + Assert.NotNull(property); + Assert.Equal("GetQueryQueryParameterType", property.Type.Name); + } + else + { + Assert.Equal("GetQueryQueryParameterType", backwardCompatibleProperty.Type.Name); + Assert.False(backwardCompatibleProperty.Deprecation.IsDeprecated); + } + } [InlineData(true)] [InlineData(false)] [Theory] From b624326ec7769d87e177052c97cb08851532cf29 Mon Sep 17 00:00:00 2001 From: Vincent Biret Date: Thu, 12 Oct 2023 10:53:26 -0400 Subject: [PATCH 2/7] - adds unit test for query parameters array type --- .../Kiota.Builder.Tests/KiotaBuilderTests.cs | 46 +++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/tests/Kiota.Builder.Tests/KiotaBuilderTests.cs b/tests/Kiota.Builder.Tests/KiotaBuilderTests.cs index d789480b6d..041568380c 100644 --- a/tests/Kiota.Builder.Tests/KiotaBuilderTests.cs +++ b/tests/Kiota.Builder.Tests/KiotaBuilderTests.cs @@ -3924,6 +3924,52 @@ public void MapsQueryParameterTypes(string type, string format, string expected) Assert.Equal(expected, property.Type.Name); Assert.True(property.Type.AllTypes.First().IsExternal); } + [Fact] + public void MapsQueryParameterArrayTypes() + { + var document = new OpenApiDocument + { + Paths = new OpenApiPaths + { + ["primitive"] = new OpenApiPathItem + { + Operations = { + [OperationType.Get] = new OpenApiOperation + { + Parameters = new List { + new() { + Name = "query", + In = ParameterLocation.Query, + Schema = new OpenApiSchema { + Type = "array", + Items = new OpenApiSchema { + Type = "integer", + Format = "int64" + } + } + } + }, + Responses = new OpenApiResponses + { + ["204"] = new OpenApiResponse() + } + } + } + } + }, + }; + var mockLogger = new Mock>(); + var builder = new KiotaBuilder(mockLogger.Object, new GenerationConfiguration { ClientClassName = "Graph", ApiRootUrl = "https://localhost" }, _httpClient); + var node = builder.CreateUriSpace(document); + var codeModel = builder.CreateSourceModel(node); + var queryParameters = codeModel.FindChildByName("primitiveRequestBuilderGetQueryParameters"); + Assert.NotNull(queryParameters); + var property = queryParameters.Properties.First(static x => x.Name.Equals("query", StringComparison.OrdinalIgnoreCase)); + Assert.NotNull(property); + Assert.Equal("int64", property.Type.Name); + Assert.Equal(CodeTypeBase.CodeTypeCollectionKind.Array, property.Type.CollectionKind); + Assert.True(property.Type.AllTypes.First().IsExternal); + } [InlineData(GenerationLanguage.CSharp)] [InlineData(GenerationLanguage.Java)] [Theory] From 4daac19f3a81a24c0bfe8200a8826148dbdc1511 Mon Sep 17 00:00:00 2001 From: Vincent Biret Date: Thu, 12 Oct 2023 10:54:46 -0400 Subject: [PATCH 3/7] - adds changelog entry for enum and array query parameter types --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 42728d79b5..7d523e1533 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,10 +9,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- Added support for enum query parameter types. [#2490](https://github.com/microsoft/kiota/issues/2490) - Support for primary error message in PHP [#3276](https://github.com/microsoft/kiota/issues/3276) ### Changed +- Fixed query parameters type mapping for arrays. [#3354](https://github.com/microsoft/kiota/issues/3354) - Fixed bug where base64url types would not be generated properly in Java. - Fixed bug where symbol name cleanup would not work on forward single quotes characters [#3426](https://github.com/microsoft/kiota/issues/3426). From 99f172e00f2c49b3e153478774dac13103dade38 Mon Sep 17 00:00:00 2001 From: Vincent Biret Date: Thu, 12 Oct 2023 11:25:13 -0400 Subject: [PATCH 4/7] - fixes a bug where inline enum type name would not be sanitized --- 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 dda41cd4d3..cc14e9a707 100644 --- a/src/Kiota.Builder/KiotaBuilder.cs +++ b/src/Kiota.Builder/KiotaBuilder.cs @@ -2314,7 +2314,7 @@ private void AddPropertyForQueryParameter(OpenApiUrlTreeNode node, OperationType var shortestNamespace = GetShortestNamespace(codeNamespace, schema); var enumName = schema.GetSchemaName().CleanupSymbolName(); if (string.IsNullOrEmpty(enumName)) - enumName = $"{operationType.ToString().ToFirstCharacterUpperCase()}{parameter.Name.ToFirstCharacterUpperCase()}QueryParameterType"; + enumName = $"{operationType.ToString().ToFirstCharacterUpperCase()}{parameter.Name.CleanupSymbolName().ToFirstCharacterUpperCase()}QueryParameterType"; if (AddEnumDeclarationIfDoesntExist(node, schema, enumName, shortestNamespace) is { } enumDeclaration) { resultType = new CodeType From f7ee618fded8c27d76f383bce7fe36cf6641225e Mon Sep 17 00:00:00 2001 From: Vincent Biret Date: Fri, 13 Oct 2023 10:12:09 -0400 Subject: [PATCH 5/7] Update src/Kiota.Builder/KiotaBuilder.cs Co-authored-by: Eastman --- 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 cc14e9a707..0d3b8cb576 100644 --- a/src/Kiota.Builder/KiotaBuilder.cs +++ b/src/Kiota.Builder/KiotaBuilder.cs @@ -1808,7 +1808,7 @@ private CodeElement AddModelDeclarationIfDoesntExist(OpenApiUrlTreeNode currentN } return existingDeclaration; } - private CodeEnum? AddEnumDeclaration(OpenApiUrlTreeNode currentNode, OpenApiSchema schema, string declarationName, CodeNamespace currentNamespace) + private static CodeEnum? AddEnumDeclaration(OpenApiUrlTreeNode currentNode, OpenApiSchema schema, string declarationName, CodeNamespace currentNamespace) { if (schema.IsEnum()) { From f5edbe472e2bc9f4d31c2848cd2cdf3645f8c306 Mon Sep 17 00:00:00 2001 From: Philip Gichuhi Date: Sat, 14 Oct 2023 00:27:19 +0300 Subject: [PATCH 6/7] Allow imports to be added for method parameters that are queryParams --- .../Refiners/CommonLanguageRefiner.cs | 2 +- .../Php/CodeClassDeclarationWriterTests.cs | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/src/Kiota.Builder/Refiners/CommonLanguageRefiner.cs b/src/Kiota.Builder/Refiners/CommonLanguageRefiner.cs index c3c6456271..18a5a301a9 100644 --- a/src/Kiota.Builder/Refiners/CommonLanguageRefiner.cs +++ b/src/Kiota.Builder/Refiners/CommonLanguageRefiner.cs @@ -725,7 +725,7 @@ currentClass.StartBlock is ClassDeclaration currentClassDeclaration && .Distinct(); var methodsParametersTypes = methods .SelectMany(static x => x.Parameters) - .Where(static x => x.IsOfKind(CodeParameterKind.Custom, CodeParameterKind.RequestBody, CodeParameterKind.RequestConfiguration)) + .Where(static x => x.IsOfKind(CodeParameterKind.Custom, CodeParameterKind.RequestBody, CodeParameterKind.RequestConfiguration, CodeParameterKind.QueryParameter)) .Select(static x => x.Type) .Distinct(); var indexerTypes = currentClassChildren diff --git a/tests/Kiota.Builder.Tests/Writers/Php/CodeClassDeclarationWriterTests.cs b/tests/Kiota.Builder.Tests/Writers/Php/CodeClassDeclarationWriterTests.cs index 3604589a33..64e1cba133 100644 --- a/tests/Kiota.Builder.Tests/Writers/Php/CodeClassDeclarationWriterTests.cs +++ b/tests/Kiota.Builder.Tests/Writers/Php/CodeClassDeclarationWriterTests.cs @@ -160,6 +160,9 @@ public void ExtendABaseClass() public async void AddsImportsToRequestConfigClasses() { var queryParamClass = new CodeClass { Name = "TestRequestQueryParameter", Kind = CodeClassKind.QueryParameters }; + var modelsNamespace = root.AddNamespace("Models"); + var eventStatus = new CodeEnum { Name = "EventStatus" }; + modelsNamespace.AddEnum(eventStatus); queryParamClass.AddProperty(new[] { new CodeProperty @@ -201,6 +204,20 @@ public async void AddsImportsToRequestConfigClasses() Name = "dateonly" }, }, + new CodeProperty + { + Name = "status", + Kind = CodePropertyKind.QueryParameter, + Documentation = new() + { + Description = "Filter by status", + }, + Type = new CodeType + { + Name = "EventStatus", + TypeDefinition = eventStatus + }, + } }); root.AddClass(queryParamClass); parentClass.Kind = CodeClassKind.RequestConfiguration; @@ -219,6 +236,7 @@ public async void AddsImportsToRequestConfigClasses() Assert.Contains("use DateTime;", result); Assert.Contains("use Microsoft\\Kiota\\Abstractions\\Types\\Date;", result); + Assert.Contains("use Microsoft\\Graph\\Models\\EventStatus;", result); } } From a97e9373dd679db03dfdd7977bef3c542f384a9c Mon Sep 17 00:00:00 2001 From: Philip Gichuhi Date: Sat, 14 Oct 2023 00:52:46 +0300 Subject: [PATCH 7/7] Update CHANGELOG --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7d523e1533..888209bbd0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- Fixed missing imports for method parameters that are query parameters. - Fixed query parameters type mapping for arrays. [#3354](https://github.com/microsoft/kiota/issues/3354) - Fixed bug where base64url types would not be generated properly in Java. - Fixed bug where symbol name cleanup would not work on forward single quotes characters [#3426](https://github.com/microsoft/kiota/issues/3426).