From 1bdfbf8a09fa471c74de2ef829709a977d8281fd Mon Sep 17 00:00:00 2001 From: Vincent Biret Date: Wed, 1 May 2024 15:46:02 -0400 Subject: [PATCH] - fixes workspace absolute path for relative descriptions --- CHANGELOG.md | 5 ++-- .../Configuration/GenerationConfiguration.cs | 14 +++++++++-- src/Kiota.Builder/KiotaBuilder.cs | 2 +- .../Plugins/PluginsGenerationService.cs | 11 +++++--- .../BaseApiConsumerConfiguration.cs | 7 +++++- .../WorkspaceManagementService.cs | 25 ++++++++++++------- .../GenerationConfigurationTests.cs | 5 ++-- .../Plugins/PluginsGenerationServiceTests.cs | 14 ++++++----- 8 files changed, 56 insertions(+), 27 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e181101aeb..9436c70e4c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,11 +17,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Fixed a bug where TypeScript deserialization would fail on Uppercase properties.[#4479](https://github.com/microsoft/kiota/issues/4479) - Changed URI template generation to reuse templates when required templates are absent across operations. -- Fixed path deduplication logic to avoid double `Id` suffixes in indexer names in scenarios where the `Id` suffix is already present.[#4519](https://github.com/microsoft/kiota/issues/4519) +- Fixed path deduplication logic to avoid double `Id` suffixes in indexer names in scenarios where the `Id` suffix is already present.[#4519](https://github.com/microsoft/kiota/issues/4519) - Updated reserved name providers for Java and Php so that "object" can be escaped. -- Fixes request builder disambiguation when child nodes had different prefixes for different paths with same parameters.[#4448](https://github.com/microsoft/kiota/issues/4448) +- Fixes request builder disambiguation when child nodes had different prefixes for different paths with same parameters.[#4448](https://github.com/microsoft/kiota/issues/4448) - Do not generate CS8603 warnings when enabling backing store in CSharp generation. - Fixed excluding operation. [#4399](https://github.com/microsoft/kiota/issues/4399) +- Fided a bug where absolute path would be used for workspace for local descriptions. [#4582](https://github.com/microsoft/kiota/issues/4582) [#4581](https://github.com/microsoft/kiota/issues/4581) - Fixed plugin generation of `ApiManifest` type to not include the `x-ms-kiota-hash` in the generated plugin. [#4561](https://github.com/microsoft/kiota/issues/4561) ## [1.13.0] - 2024-04-04 diff --git a/src/Kiota.Builder/Configuration/GenerationConfiguration.cs b/src/Kiota.Builder/Configuration/GenerationConfiguration.cs index 79d426987c..d00d410f9b 100644 --- a/src/Kiota.Builder/Configuration/GenerationConfiguration.cs +++ b/src/Kiota.Builder/Configuration/GenerationConfiguration.cs @@ -173,11 +173,11 @@ internal void UpdateConfigurationFromLanguagesInformation(LanguagesInformation l StructuredMimeTypes = new(languageInfo.StructuredMimeTypes); } public const string KiotaHashManifestExtensionKey = "x-ms-kiota-hash"; - public ApiDependency ToApiDependency(string configurationHash, Dictionary> templatesWithOperations) + public ApiDependency ToApiDependency(string configurationHash, Dictionary> templatesWithOperations, string targetDirectory) { var dependency = new ApiDependency() { - ApiDescriptionUrl = OpenAPIFilePath, + ApiDescriptionUrl = NormalizeDescriptionLocation(targetDirectory), ApiDeploymentBaseUrl = ApiRootUrl?.EndsWith('/') ?? false ? ApiRootUrl : $"{ApiRootUrl}/", Extensions = new(), Requests = templatesWithOperations.SelectMany(static x => x.Value.Select(y => new RequestInfo { Method = y.ToUpperInvariant(), UriTemplate = x.Key.DeSanitizeUrlTemplateParameter() })).ToList(), @@ -189,6 +189,16 @@ public ApiDependency ToApiDependency(string configurationHash, Dictionary PluginTypes.Count != 0; } #pragma warning restore CA1056 diff --git a/src/Kiota.Builder/KiotaBuilder.cs b/src/Kiota.Builder/KiotaBuilder.cs index c17d1c51ca..5bcbcf7881 100644 --- a/src/Kiota.Builder/KiotaBuilder.cs +++ b/src/Kiota.Builder/KiotaBuilder.cs @@ -237,7 +237,7 @@ public async Task GeneratePluginAsync(CancellationToken cancellationToken) throw new InvalidOperationException("The OpenAPI document and the URL tree must be loaded before generating the plugins"); // generate plugin sw.Start(); - var pluginsService = new PluginsGenerationService(openApiDocument, openApiTree, config); + var pluginsService = new PluginsGenerationService(openApiDocument, openApiTree, config, Directory.GetCurrentDirectory()); await pluginsService.GenerateManifestAsync(cancellationToken).ConfigureAwait(false); StopLogAndReset(sw, $"step {++stepId} - generate plugin - took"); return stepId; diff --git a/src/Kiota.Builder/Plugins/PluginsGenerationService.cs b/src/Kiota.Builder/Plugins/PluginsGenerationService.cs index 93cc546e55..90085c7d2a 100644 --- a/src/Kiota.Builder/Plugins/PluginsGenerationService.cs +++ b/src/Kiota.Builder/Plugins/PluginsGenerationService.cs @@ -21,22 +21,25 @@ public class PluginsGenerationService private readonly OpenApiDocument OAIDocument; private readonly OpenApiUrlTreeNode TreeNode; private readonly GenerationConfiguration Configuration; + private readonly string WorkingDirectory; - public PluginsGenerationService(OpenApiDocument document, OpenApiUrlTreeNode openApiUrlTreeNode, GenerationConfiguration configuration) + public PluginsGenerationService(OpenApiDocument document, OpenApiUrlTreeNode openApiUrlTreeNode, GenerationConfiguration configuration, string workingDirectory) { ArgumentNullException.ThrowIfNull(document); ArgumentNullException.ThrowIfNull(openApiUrlTreeNode); ArgumentNullException.ThrowIfNull(configuration); + ArgumentException.ThrowIfNullOrEmpty(workingDirectory); OAIDocument = document; TreeNode = openApiUrlTreeNode; Configuration = configuration; + WorkingDirectory = workingDirectory; } private static readonly OpenAPIRuntimeComparer _openAPIRuntimeComparer = new(); private const string ManifestFileNameSuffix = ".json"; private const string DescriptionRelativePath = "./openapi.yml"; public async Task GenerateManifestAsync(CancellationToken cancellationToken = default) { - // write the decription + // write the description var descriptionFullPath = Path.Combine(Configuration.OutputPath, DescriptionRelativePath); var directory = Path.GetDirectoryName(descriptionFullPath); if (!string.IsNullOrEmpty(directory) && !Directory.Exists(directory)) @@ -67,7 +70,7 @@ public async Task GenerateManifestAsync(CancellationToken cancellationToken = de case PluginType.APIManifest: var apiManifest = new ApiManifestDocument("application"); //TODO add application name // pass empty cong hash so that its not included in this manifest. - apiManifest.ApiDependencies.AddOrReplace(Configuration.ClientClassName, Configuration.ToApiDependency(string.Empty, TreeNode?.GetRequestInfo().ToDictionary(static x => x.Key, static x => x.Value) ?? [])); + apiManifest.ApiDependencies.AddOrReplace(Configuration.ClientClassName, Configuration.ToApiDependency(string.Empty, TreeNode?.GetRequestInfo().ToDictionary(static x => x.Key, static x => x.Value) ?? [], WorkingDirectory)); apiManifest.Write(writer); break; case PluginType.OpenAI://TODO add support for OpenAI plugin type generation @@ -136,7 +139,7 @@ descriptionExtension is OpenApiDescriptionForModelExtension extension && Auth = new AnonymousAuth(), Spec = new OpenApiRuntimeSpec() { - Url = openApiDocumentPath + Url = openApiDocumentPath, }, RunForFunctions = [operation.OperationId] }); diff --git a/src/Kiota.Builder/WorkspaceManagement/BaseApiConsumerConfiguration.cs b/src/Kiota.Builder/WorkspaceManagement/BaseApiConsumerConfiguration.cs index e2a9487f5e..fa0e320a9c 100644 --- a/src/Kiota.Builder/WorkspaceManagement/BaseApiConsumerConfiguration.cs +++ b/src/Kiota.Builder/WorkspaceManagement/BaseApiConsumerConfiguration.cs @@ -38,11 +38,16 @@ private protected BaseApiConsumerConfiguration(GenerationConfiguration config) /// The output path for the generated code, related to the configuration file. /// public string OutputPath { get; set; } = string.Empty; - public void NormalizePaths(string targetDirectory) + public void NormalizeOutputPath(string targetDirectory) { if (Path.IsPathRooted(OutputPath)) OutputPath = "./" + Path.GetRelativePath(targetDirectory, OutputPath); } + public void NormalizeDescriptionLocation(string targetDirectory) + { + if (Path.IsPathRooted(DescriptionLocation) && Path.GetFullPath(DescriptionLocation).StartsWith(Path.GetFullPath(targetDirectory), StringComparison.Ordinal) && !DescriptionLocation.StartsWith("http", StringComparison.OrdinalIgnoreCase)) + DescriptionLocation = "./" + Path.GetRelativePath(targetDirectory, DescriptionLocation); + } protected void CloneBase(BaseApiConsumerConfiguration target) { ArgumentNullException.ThrowIfNull(target); diff --git a/src/Kiota.Builder/WorkspaceManagement/WorkspaceManagementService.cs b/src/Kiota.Builder/WorkspaceManagement/WorkspaceManagementService.cs index 7248cc9adb..f538b073f4 100644 --- a/src/Kiota.Builder/WorkspaceManagement/WorkspaceManagementService.cs +++ b/src/Kiota.Builder/WorkspaceManagement/WorkspaceManagementService.cs @@ -51,14 +51,16 @@ private BaseApiConsumerConfiguration UpdateConsumerConfiguration(GenerationConfi if (generationConfiguration.IsPluginConfiguration) { var generationPluginConfig = new ApiPluginConfiguration(generationConfiguration); - generationPluginConfig.NormalizePaths(WorkingDirectory); + generationPluginConfig.NormalizeOutputPath(WorkingDirectory); + generationPluginConfig.NormalizeDescriptionLocation(WorkingDirectory); wsConfig.Plugins.AddOrReplace(generationConfiguration.ClientClassName, generationPluginConfig); return generationPluginConfig; } else { var generationClientConfig = new ApiClientConfiguration(generationConfiguration); - generationClientConfig.NormalizePaths(WorkingDirectory); + generationClientConfig.NormalizeOutputPath(WorkingDirectory); + generationClientConfig.NormalizeDescriptionLocation(WorkingDirectory); wsConfig.Clients.AddOrReplace(generationConfiguration.ClientClassName, generationClientConfig); return generationClientConfig; } @@ -70,8 +72,9 @@ public async Task UpdateStateFromConfigurationAsync(GenerationConfiguration gene { var (wsConfig, manifest) = await LoadConfigurationAndManifestAsync(cancellationToken).ConfigureAwait(false); var generationClientConfig = UpdateConsumerConfiguration(generationConfiguration, wsConfig); + generationClientConfig.NormalizeDescriptionLocation(WorkingDirectory); var inputConfigurationHash = await GetConsumerConfigurationHashAsync(generationClientConfig, descriptionHash).ConfigureAwait(false); - manifest.ApiDependencies.AddOrReplace(generationConfiguration.ClientClassName, generationConfiguration.ToApiDependency(inputConfigurationHash, templatesWithOperations)); + manifest.ApiDependencies.AddOrReplace(generationConfiguration.ClientClassName, generationConfiguration.ToApiDependency(inputConfigurationHash, templatesWithOperations, WorkingDirectory)); await workspaceConfigurationStorageService.UpdateWorkspaceConfigurationAsync(wsConfig, manifest, cancellationToken).ConfigureAwait(false); if (descriptionStream != Stream.Null) await descriptionStorageService.UpdateDescriptionAsync(generationConfiguration.ClientClassName, descriptionStream, new Uri(generationConfiguration.OpenAPIFilePath).GetFileExtension(), cancellationToken).ConfigureAwait(false); @@ -116,19 +119,21 @@ public async Task ShouldGenerateAsync(GenerationConfiguration inputConfig, apiManifest.ApiDependencies.TryGetValue(inputConfig.ClientClassName, out var existingApiManifest)) { var inputClientConfig = new ApiClientConfiguration(inputConfig); - inputClientConfig.NormalizePaths(WorkingDirectory); + inputClientConfig.NormalizeOutputPath(WorkingDirectory); + inputClientConfig.NormalizeDescriptionLocation(WorkingDirectory); var inputConfigurationHash = await GetConsumerConfigurationHashAsync(inputClientConfig, descriptionHash).ConfigureAwait(false); return !clientConfigurationComparer.Equals(existingClientConfig, inputClientConfig) || - !apiDependencyComparer.Equals(inputConfig.ToApiDependency(inputConfigurationHash, []), existingApiManifest); + !apiDependencyComparer.Equals(inputConfig.ToApiDependency(inputConfigurationHash, [], WorkingDirectory), existingApiManifest); } if (wsConfig.Plugins.TryGetValue(inputConfig.ClientClassName, out var existingPluginConfig) && apiManifest.ApiDependencies.TryGetValue(inputConfig.ClientClassName, out var existingPluginApiManifest)) { var inputClientConfig = new ApiPluginConfiguration(inputConfig); - inputClientConfig.NormalizePaths(WorkingDirectory); + inputClientConfig.NormalizeOutputPath(WorkingDirectory); + inputClientConfig.NormalizeDescriptionLocation(WorkingDirectory); var inputConfigurationHash = await GetConsumerConfigurationHashAsync(inputClientConfig, descriptionHash).ConfigureAwait(false); return !pluginConfigurationComparer.Equals(existingPluginConfig, inputClientConfig) || - !apiDependencyComparer.Equals(inputConfig.ToApiDependency(inputConfigurationHash, []), existingPluginApiManifest); + !apiDependencyComparer.Equals(inputConfig.ToApiDependency(inputConfigurationHash, [], WorkingDirectory), existingPluginApiManifest); } return true; } @@ -292,7 +297,8 @@ public async Task> MigrateFromLockFileAsync(string clientNam await descriptionStorageService.UpdateDescriptionAsync(generationConfiguration.ClientClassName, ms, new Uri(generationConfiguration.OpenAPIFilePath).GetFileExtension(), cancellationToken).ConfigureAwait(false); var clientConfiguration = new ApiClientConfiguration(generationConfiguration); - clientConfiguration.NormalizePaths(WorkingDirectory); + clientConfiguration.NormalizeOutputPath(WorkingDirectory); + clientConfiguration.NormalizeDescriptionLocation(WorkingDirectory); wsConfig.Clients.Add(generationConfiguration.ClientClassName, clientConfiguration); var inputConfigurationHash = await GetConsumerConfigurationHashAsync(clientConfiguration, "migrated-pending-generate").ConfigureAwait(false); // because it's a migration, we don't want to calculate the exact hash since the description might have changed since the initial generation that created the lock file @@ -302,7 +308,8 @@ public async Task> MigrateFromLockFileAsync(string clientNam inputConfigurationHash, new Dictionary> { { MigrationPlaceholderPath, new HashSet { "GET" } } - })); + }, + WorkingDirectory)); lockManagementService.DeleteLockFile(Path.Combine(WorkingDirectory, clientConfiguration.OutputPath)); } await workspaceConfigurationStorageService.UpdateWorkspaceConfigurationAsync(wsConfig, apiManifest, cancellationToken).ConfigureAwait(false); diff --git a/tests/Kiota.Builder.Tests/Configuration/GenerationConfigurationTests.cs b/tests/Kiota.Builder.Tests/Configuration/GenerationConfigurationTests.cs index 6e3e95e9b5..268ec12326 100644 --- a/tests/Kiota.Builder.Tests/Configuration/GenerationConfigurationTests.cs +++ b/tests/Kiota.Builder.Tests/Configuration/GenerationConfigurationTests.cs @@ -1,4 +1,5 @@ using System.Collections.Generic; +using System.IO; using System.Linq; using Kiota.Builder.Configuration; using Xunit; @@ -34,7 +35,7 @@ public void ToApiDependency() }; var apiDependency = generationConfiguration.ToApiDependency("foo", new Dictionary>{ { "foo/bar", new HashSet{"GET"}} - }); + }, Path.GetTempPath()); Assert.NotNull(apiDependency); Assert.NotNull(apiDependency.Extensions); Assert.Equal("foo", apiDependency.Extensions[GenerationConfiguration.KiotaHashManifestExtensionKey].GetValue()); @@ -54,7 +55,7 @@ public void ToApiDependencyDoesNotIncludeConfigHashIfEmpty() }; var apiDependency = generationConfiguration.ToApiDependency(string.Empty, new Dictionary>{ { "foo/bar", new HashSet{"GET"}} - }); + }, Path.GetTempPath()); Assert.NotNull(apiDependency); Assert.NotNull(apiDependency.Extensions); Assert.False(apiDependency.Extensions.ContainsKey(GenerationConfiguration.KiotaHashManifestExtensionKey)); diff --git a/tests/Kiota.Builder.Tests/Plugins/PluginsGenerationServiceTests.cs b/tests/Kiota.Builder.Tests/Plugins/PluginsGenerationServiceTests.cs index 77e2b4a638..5764e672c0 100644 --- a/tests/Kiota.Builder.Tests/Plugins/PluginsGenerationServiceTests.cs +++ b/tests/Kiota.Builder.Tests/Plugins/PluginsGenerationServiceTests.cs @@ -16,9 +16,10 @@ public sealed class PluginsGenerationServiceTests : IDisposable [Fact] public void Defensive() { - Assert.Throws(() => new PluginsGenerationService(null, OpenApiUrlTreeNode.Create(), new())); - Assert.Throws(() => new PluginsGenerationService(new(), null, new())); - Assert.Throws(() => new PluginsGenerationService(new(), OpenApiUrlTreeNode.Create(), null)); + Assert.Throws(() => new PluginsGenerationService(null, OpenApiUrlTreeNode.Create(), new(), "foo")); + Assert.Throws(() => new PluginsGenerationService(new(), null, new(), "foo")); + Assert.Throws(() => new PluginsGenerationService(new(), OpenApiUrlTreeNode.Create(), null, "foo")); + Assert.Throws(() => new PluginsGenerationService(new(), OpenApiUrlTreeNode.Create(), new(), string.Empty)); } public void Dispose() @@ -43,11 +44,12 @@ public async Task GeneratesManifest() responses: '200': description: test"; - var simpleDescriptionPath = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName()) + ".yaml"; + var workingDirectory = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName()); + var simpleDescriptionPath = Path.Combine(workingDirectory) + "description.yaml"; await File.WriteAllTextAsync(simpleDescriptionPath, simpleDescriptionContent); var mockLogger = new Mock>(); var openAPIDocumentDS = new OpenApiDocumentDownloadService(_httpClient, mockLogger.Object); - var outputDirectory = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName()); + var outputDirectory = Path.Combine(workingDirectory, "output"); var generationConfiguration = new GenerationConfiguration { OutputPath = outputDirectory, @@ -60,7 +62,7 @@ public async Task GeneratesManifest() var openApiDocument = await openAPIDocumentDS.GetDocumentFromStreamAsync(openAPIDocumentStream, generationConfiguration); var urlTreeNode = OpenApiUrlTreeNode.Create(openApiDocument, Constants.DefaultOpenApiLabel); - var pluginsGenerationService = new PluginsGenerationService(openApiDocument, urlTreeNode, generationConfiguration); + var pluginsGenerationService = new PluginsGenerationService(openApiDocument, urlTreeNode, generationConfiguration, workingDirectory); await pluginsGenerationService.GenerateManifestAsync(); Assert.True(File.Exists(Path.Combine(outputDirectory, "client-microsoft.json")));