From 8430caab9fa907250b1060d73e901b77a8298a3a Mon Sep 17 00:00:00 2001 From: Vincent Biret Date: Wed, 27 Sep 2023 11:10:33 -0400 Subject: [PATCH] - adds a parameter to exclude backward compatible types --- .../Configuration/GenerationConfiguration.cs | 9 +++++ src/Kiota.Builder/KiotaBuilder.cs | 6 ++-- src/Kiota.Builder/Lock/KiotaLock.cs | 9 +++++ src/Kiota.Builder/Lock/KiotaLockComparer.cs | 17 +++++----- src/Kiota.Builder/Refiners/CSharpRefiner.cs | 33 ++++++++++--------- .../Writers/Go/CodeMethodWriter.cs | 2 +- .../Handlers/KiotaGenerationCommandHandler.cs | 7 ++++ src/kiota/KiotaHost.cs | 7 ++-- .../Kiota.Builder.Tests/KiotaBuilderTests.cs | 28 +++++++++++----- 9 files changed, 79 insertions(+), 39 deletions(-) diff --git a/src/Kiota.Builder/Configuration/GenerationConfiguration.cs b/src/Kiota.Builder/Configuration/GenerationConfiguration.cs index 655d807a05..810ebb5181 100644 --- a/src/Kiota.Builder/Configuration/GenerationConfiguration.cs +++ b/src/Kiota.Builder/Configuration/GenerationConfiguration.cs @@ -43,6 +43,14 @@ public bool UsesBackingStore { get; set; } + public bool ExcludeBackwardCompatible + { + get; set; + } + public bool IncludeBackwardCompatible + { + get => !ExcludeBackwardCompatible; + } public bool IncludeAdditionalData { get; set; } = true; public HashSet Serializers { @@ -118,6 +126,7 @@ public object Clone() { return new GenerationConfiguration { + ExcludeBackwardCompatible = ExcludeBackwardCompatible, OpenAPIFilePath = OpenAPIFilePath, OutputPath = OutputPath, ClientClassName = ClientClassName, diff --git a/src/Kiota.Builder/KiotaBuilder.cs b/src/Kiota.Builder/KiotaBuilder.cs index 95db1dbf32..e3e1c1c2db 100644 --- a/src/Kiota.Builder/KiotaBuilder.cs +++ b/src/Kiota.Builder/KiotaBuilder.cs @@ -1113,7 +1113,7 @@ private CodeIndexer[] CreateIndexer(string childIdentifier, string childType, Co IndexParameter = parameter, }}; - if (!"string".Equals(parameter.Type.Name, StringComparison.OrdinalIgnoreCase)) + if (!"string".Equals(parameter.Type.Name, StringComparison.OrdinalIgnoreCase) && config.IncludeBackwardCompatible) { // adding a second indexer for the string version of the parameter so we keep backward compatibility //TODO remove for v2 var backCompatibleValue = (CodeIndexer)result[0].Clone(); @@ -1271,7 +1271,7 @@ codeType.TypeDefinition is CodeClass codeClass && { var suffix = $"{operationType}Response"; var modelType = CreateModelDeclarations(currentNode, schema, operation, parentClass, suffix); - if (modelType is not null && config.Language is GenerationLanguage.CSharp or GenerationLanguage.Go && modelType.Name.EndsWith(suffix, StringComparison.Ordinal)) + if (modelType is not null && config.IncludeBackwardCompatible && config.Language is GenerationLanguage.CSharp or GenerationLanguage.Go && modelType.Name.EndsWith(suffix, StringComparison.Ordinal)) { //TODO remove for v2 var obsoleteTypeName = modelType.Name[..^suffix.Length] + "Response"; if (modelType is CodeType codeType && @@ -1393,7 +1393,7 @@ private void CreateOperationMethods(OpenApiUrlTreeNode currentNode, OperationTyp }; executorMethod.AddParameter(cancellationParam);// Add cancellation token parameter - if (returnTypes.Item2 is not null) + if (returnTypes.Item2 is not null && config.IncludeBackwardCompatible) { //TODO remove for v2 var additionalExecutorMethod = (CodeMethod)executorMethod.Clone(); additionalExecutorMethod.ReturnType = returnTypes.Item2; diff --git a/src/Kiota.Builder/Lock/KiotaLock.cs b/src/Kiota.Builder/Lock/KiotaLock.cs index cfdd5b32f0..099d781477 100644 --- a/src/Kiota.Builder/Lock/KiotaLock.cs +++ b/src/Kiota.Builder/Lock/KiotaLock.cs @@ -45,6 +45,13 @@ public bool UsesBackingStore get; set; } /// + /// Whether backward compatible code was excluded for this client. + /// + public bool ExcludeBackwardCompatible + { + get; set; + } + /// /// Whether additional data was used for this client. /// public bool IncludeAdditionalData @@ -89,6 +96,7 @@ public void UpdateGenerationConfigurationFromLock(GenerationConfiguration config if (Enum.TryParse(Language, out var parsedLanguage)) config.Language = parsedLanguage; config.UsesBackingStore = UsesBackingStore; + config.ExcludeBackwardCompatible = ExcludeBackwardCompatible; config.IncludeAdditionalData = IncludeAdditionalData; config.Serializers = Serializers; config.Deserializers = Deserializers; @@ -115,6 +123,7 @@ public KiotaLock(GenerationConfiguration config) ClientClassName = config.ClientClassName; ClientNamespaceName = config.ClientNamespaceName; UsesBackingStore = config.UsesBackingStore; + ExcludeBackwardCompatible = config.ExcludeBackwardCompatible; IncludeAdditionalData = config.IncludeAdditionalData; Serializers = config.Serializers; Deserializers = config.Deserializers; diff --git a/src/Kiota.Builder/Lock/KiotaLockComparer.cs b/src/Kiota.Builder/Lock/KiotaLockComparer.cs index a5e20cde06..d4e564b776 100644 --- a/src/Kiota.Builder/Lock/KiotaLockComparer.cs +++ b/src/Kiota.Builder/Lock/KiotaLockComparer.cs @@ -21,14 +21,15 @@ public int GetHashCode([DisallowNull] KiotaLock obj) { if (obj == null) return 0; return - _stringIEnumerableDeepComparer.GetHashCode(obj.DisabledValidationRules?.Order(StringComparer.OrdinalIgnoreCase) ?? Enumerable.Empty()) * 47 + - obj.KiotaVersion.GetHashCode(StringComparison.OrdinalIgnoreCase) * 43 + - obj.LockFileVersion.GetHashCode(StringComparison.OrdinalIgnoreCase) * 41 + - (string.IsNullOrEmpty(obj.DescriptionLocation) ? 0 : obj.DescriptionLocation.GetHashCode(StringComparison.OrdinalIgnoreCase)) * 37 + - (string.IsNullOrEmpty(obj.DescriptionHash) ? 0 : obj.DescriptionHash.GetHashCode(StringComparison.OrdinalIgnoreCase)) * 31 + - (string.IsNullOrEmpty(obj.ClientClassName) ? 0 : obj.ClientClassName.GetHashCode(StringComparison.OrdinalIgnoreCase)) * 29 + - (string.IsNullOrEmpty(obj.ClientNamespaceName) ? 0 : obj.ClientNamespaceName.GetHashCode(StringComparison.OrdinalIgnoreCase)) * 23 + - (string.IsNullOrEmpty(obj.Language) ? 0 : obj.Language.GetHashCode(StringComparison.OrdinalIgnoreCase)) * 19 + + _stringIEnumerableDeepComparer.GetHashCode(obj.DisabledValidationRules?.Order(StringComparer.OrdinalIgnoreCase) ?? Enumerable.Empty()) * 53 + + obj.KiotaVersion.GetHashCode(StringComparison.OrdinalIgnoreCase) * 47 + + obj.LockFileVersion.GetHashCode(StringComparison.OrdinalIgnoreCase) * 43 + + (string.IsNullOrEmpty(obj.DescriptionLocation) ? 0 : obj.DescriptionLocation.GetHashCode(StringComparison.OrdinalIgnoreCase)) * 41 + + (string.IsNullOrEmpty(obj.DescriptionHash) ? 0 : obj.DescriptionHash.GetHashCode(StringComparison.OrdinalIgnoreCase)) * 37 + + (string.IsNullOrEmpty(obj.ClientClassName) ? 0 : obj.ClientClassName.GetHashCode(StringComparison.OrdinalIgnoreCase)) * 31 + + (string.IsNullOrEmpty(obj.ClientNamespaceName) ? 0 : obj.ClientNamespaceName.GetHashCode(StringComparison.OrdinalIgnoreCase)) * 29 + + (string.IsNullOrEmpty(obj.Language) ? 0 : obj.Language.GetHashCode(StringComparison.OrdinalIgnoreCase)) * 23 + + obj.ExcludeBackwardCompatible.GetHashCode() * 19 + obj.UsesBackingStore.GetHashCode() * 17 + obj.IncludeAdditionalData.GetHashCode() * 13 + _stringIEnumerableDeepComparer.GetHashCode(obj.Serializers?.Order(StringComparer.OrdinalIgnoreCase) ?? Enumerable.Empty()) * 11 + diff --git a/src/Kiota.Builder/Refiners/CSharpRefiner.cs b/src/Kiota.Builder/Refiners/CSharpRefiner.cs index 7472c4c922..94265f40c8 100644 --- a/src/Kiota.Builder/Refiners/CSharpRefiner.cs +++ b/src/Kiota.Builder/Refiners/CSharpRefiner.cs @@ -31,22 +31,23 @@ public override Task Refine(CodeNamespace generatedCode, CancellationToken cance IsExternal = true } }); - //TODO uncomment on the next major version - // RemoveRequestConfigurationClasses(generatedCode, - // new CodeUsing - // { - // Name = "RequestConfiguration", - // Declaration = new CodeType - // { - // Name = AbstractionsNamespaceName, - // IsExternal = true - // } - // }, - // new CodeType - // { - // Name = "DefaultQueryParameters", - // IsExternal = true, - // }); + //TODO remove the condition for v2 + if (_configuration.ExcludeBackwardCompatible) + RemoveRequestConfigurationClasses(generatedCode, + new CodeUsing + { + Name = "RequestConfiguration", + Declaration = new CodeType + { + Name = AbstractionsNamespaceName, + IsExternal = true + } + }, + new CodeType + { + Name = "DefaultQueryParameters", + IsExternal = true, + }); AddDefaultImports(generatedCode, defaultUsingEvaluators); MoveClassesWithNamespaceNamesUnderNamespace(generatedCode); ConvertUnionTypesToWrapper(generatedCode, diff --git a/src/Kiota.Builder/Writers/Go/CodeMethodWriter.cs b/src/Kiota.Builder/Writers/Go/CodeMethodWriter.cs index 804e9b33ce..73364bc8fb 100644 --- a/src/Kiota.Builder/Writers/Go/CodeMethodWriter.cs +++ b/src/Kiota.Builder/Writers/Go/CodeMethodWriter.cs @@ -42,7 +42,7 @@ public override void WriteCodeElement(CodeMethod codeElement, LanguageWriter wri case CodeMethodKind.RequestGenerator when !codeElement.IsOverload: WriteRequestGeneratorBody(codeElement, requestParams, writer, parentClass); break; - case CodeMethodKind.RequestExecutor when !codeElement.IsOverload || (codeElement.Deprecation?.IsDeprecated ?? false): //TODO remove deprecration condition for v2 + case CodeMethodKind.RequestExecutor when !codeElement.IsOverload || (codeElement.Deprecation?.IsDeprecated ?? false): //TODO remove deprecation condition for v2 WriteRequestExecutorBody(codeElement, requestParams, returnType, parentClass, writer); break; case CodeMethodKind.Getter: diff --git a/src/kiota/Handlers/KiotaGenerationCommandHandler.cs b/src/kiota/Handlers/KiotaGenerationCommandHandler.cs index 5976c0c455..c94e794751 100644 --- a/src/kiota/Handlers/KiotaGenerationCommandHandler.cs +++ b/src/kiota/Handlers/KiotaGenerationCommandHandler.cs @@ -71,6 +71,7 @@ public override async Task InvokeAsync(InvocationContext context) string openapi = context.ParseResult.GetValueForOption(DescriptionOption) ?? string.Empty; string manifest = context.ParseResult.GetValueForOption(ManifestOption) ?? string.Empty; bool backingStore = context.ParseResult.GetValueForOption(BackingStoreOption); + bool excludeBackwardCompatible = context.ParseResult.GetValueForOption(ExcludeBackwardCompatibleOption); bool clearCache = context.ParseResult.GetValueForOption(ClearCacheOption); bool includeAdditionalData = context.ParseResult.GetValueForOption(AdditionalDataOption); string className = context.ParseResult.GetValueForOption(ClassOption) ?? string.Empty; @@ -89,6 +90,7 @@ public override async Task InvokeAsync(InvocationContext context) AssignIfNotNullOrEmpty(className, (c, s) => c.ClientClassName = s); AssignIfNotNullOrEmpty(namespaceName, (c, s) => c.ClientNamespaceName = s); Configuration.Generation.UsesBackingStore = backingStore; + Configuration.Generation.ExcludeBackwardCompatible = excludeBackwardCompatible; Configuration.Generation.IncludeAdditionalData = includeAdditionalData; Configuration.Generation.Language = language; if (serializer.Any()) @@ -167,4 +169,9 @@ public required Option ManifestOption { get; init; } + public required Option ExcludeBackwardCompatibleOption + { + get; + set; + } } diff --git a/src/kiota/KiotaHost.cs b/src/kiota/KiotaHost.cs index 3b60f467c9..66e2e4f37d 100644 --- a/src/kiota/KiotaHost.cs +++ b/src/kiota/KiotaHost.cs @@ -3,7 +3,6 @@ using System.CommandLine; using System.CommandLine.Parsing; using System.Linq; -using System.Reflection; using System.Text.RegularExpressions; using kiota.Handlers; using kiota.Rpc; @@ -11,7 +10,6 @@ using Kiota.Builder.Configuration; using Kiota.Builder.Validation; using Microsoft.Extensions.Logging; -using Microsoft.OpenApi.Validations; namespace kiota; public static class KiotaHost @@ -344,6 +342,9 @@ private static Command GetGenerateCommand() var backingStoreOption = new Option("--backing-store", () => defaultConfiguration.UsesBackingStore, "Enables backing store for models."); backingStoreOption.AddAlias("-b"); + var excludeBackwardCompatible = new Option("--exclude-backward-compatible", () => defaultConfiguration.ExcludeBackwardCompatible, "Excludes backward compatible and obsolete assets from the generated result. Should be used for new clients."); + excludeBackwardCompatible.AddAlias("--ebc"); + var additionalDataOption = new Option("--additional-data", () => defaultConfiguration.IncludeAdditionalData, "Will include the 'AdditionalData' property for models."); additionalDataOption.AddAlias("--ad"); @@ -384,6 +385,7 @@ private static Command GetGenerateCommand() namespaceOption, logLevelOption, backingStoreOption, + excludeBackwardCompatible, additionalDataOption, serializerOption, deserializerOption, @@ -404,6 +406,7 @@ private static Command GetGenerateCommand() NamespaceOption = namespaceOption, LogLevelOption = logLevelOption, BackingStoreOption = backingStoreOption, + ExcludeBackwardCompatibleOption = excludeBackwardCompatible, AdditionalDataOption = additionalDataOption, SerializerOption = serializerOption, DeserializerOption = deserializerOption, diff --git a/tests/Kiota.Builder.Tests/KiotaBuilderTests.cs b/tests/Kiota.Builder.Tests/KiotaBuilderTests.cs index 3c7d8a0543..4f92463e05 100644 --- a/tests/Kiota.Builder.Tests/KiotaBuilderTests.cs +++ b/tests/Kiota.Builder.Tests/KiotaBuilderTests.cs @@ -4636,8 +4636,10 @@ public void AcceptVendorsTypes(string contentType) Assert.NotNull(executorMethod); Assert.Equal("myobject", executorMethod.ReturnType.Name); } - [Fact] - public void ModelsUseDescriptionWhenAvailable() + [Theory] + [InlineData(true)] + [InlineData(false)] + public void ModelsUseDescriptionWhenAvailable(bool excludeBackwardCompatible) { var document = new OpenApiDocument { @@ -4677,7 +4679,7 @@ public void ModelsUseDescriptionWhenAvailable() } }; var mockLogger = new Mock>(); - var builder = new KiotaBuilder(mockLogger.Object, new GenerationConfiguration { ClientClassName = "TestClient", ClientNamespaceName = "TestSdk", ApiRootUrl = "https://localhost" }, _httpClient); + var builder = new KiotaBuilder(mockLogger.Object, new GenerationConfiguration { ClientClassName = "TestClient", ClientNamespaceName = "TestSdk", ApiRootUrl = "https://localhost", ExcludeBackwardCompatible = excludeBackwardCompatible }, _httpClient); var node = builder.CreateUriSpace(document); var codeModel = builder.CreateSourceModel(node); var modelsSubNS = codeModel.FindNamespaceByName("TestSdk.answer"); @@ -4687,17 +4689,25 @@ public void ModelsUseDescriptionWhenAvailable() Assert.Equal("some description", responseClass.Documentation.Description); var obsoleteResponseClass = modelsSubNS.FindChildByName("AnswerResponse", false); - Assert.NotNull(obsoleteResponseClass); - Assert.Equal("some description", obsoleteResponseClass.Documentation.Description); - Assert.True(obsoleteResponseClass.Deprecation.IsDeprecated); + if (excludeBackwardCompatible) + Assert.Null(obsoleteResponseClass); + else + { + Assert.NotNull(obsoleteResponseClass); + Assert.Equal("some description", obsoleteResponseClass.Documentation.Description); + Assert.True(obsoleteResponseClass.Deprecation.IsDeprecated); + } - var requestBuilderClass = modelsSubNS.Classes.FirstOrDefault(c => c.IsOfKind(CodeClassKind.RequestBuilder)); + var requestBuilderClass = modelsSubNS.Classes.FirstOrDefault(static c => c.IsOfKind(CodeClassKind.RequestBuilder)); Assert.NotNull(requestBuilderClass); Assert.Equal("some path item description", requestBuilderClass.Documentation.Description); - Assert.Equal(2, requestBuilderClass.Methods.Where(static x => x.Kind is CodeMethodKind.RequestExecutor).Count()); + if (excludeBackwardCompatible) + Assert.Single(requestBuilderClass.Methods.Where(static x => x.Kind is CodeMethodKind.RequestExecutor)); + else + Assert.Equal(2, requestBuilderClass.Methods.Where(static x => x.Kind is CodeMethodKind.RequestExecutor).Count()); - var responseProperty = codeModel.FindNamespaceByName("TestSdk").Classes.SelectMany(c => c.Properties).FirstOrDefault(p => p.Kind == CodePropertyKind.RequestBuilder); + var responseProperty = codeModel.FindNamespaceByName("TestSdk").Classes.SelectMany(c => c.Properties).FirstOrDefault(static p => p.Kind == CodePropertyKind.RequestBuilder); Assert.NotNull(responseProperty); Assert.Equal("some path item description", responseProperty.Documentation.Description); }