Skip to content

Commit

Permalink
Merge pull request #4587 from microsoft/bugfix/relative-description-path
Browse files Browse the repository at this point in the history 
 - fixes workspace absolute path for relative descriptions
  • Loading branch information
@andrueastman
andrueastman authored May 2, 2024
2 parents 3ef4ec1 + 1bdfbf8 commit 0493af1
Show file tree
Hide file tree
Showing 8 changed files with 53 additions and 24 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- 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
Expand Down
14 changes: 12 additions & 2 deletions src/Kiota.Builder/Configuration/GenerationConfiguration.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, HashSet<string>> templatesWithOperations)
public ApiDependency ToApiDependency(string configurationHash, Dictionary<string, HashSet<string>> 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(),
Expand All @@ -189,6 +189,16 @@ public ApiDependency ToApiDependency(string configurationHash, Dictionary<string
}
return dependency;
}
private string NormalizeDescriptionLocation(string targetDirectory)
{
if (!string.IsNullOrEmpty(OpenAPIFilePath) &&
!string.IsNullOrEmpty(targetDirectory) &&
!OpenAPIFilePath.StartsWith("http", StringComparison.OrdinalIgnoreCase) &&
Path.IsPathRooted(OpenAPIFilePath) &&
Path.GetFullPath(OpenAPIFilePath).StartsWith(Path.GetFullPath(targetDirectory), StringComparison.Ordinal))
return "./" + Path.GetRelativePath(targetDirectory, OpenAPIFilePath);
return OpenAPIFilePath;
}
public bool IsPluginConfiguration => PluginTypes.Count != 0;
}
#pragma warning restore CA1056
Expand Down
2 changes: 1 addition & 1 deletion src/Kiota.Builder/KiotaBuilder.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ public async Task<bool> 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;
Expand Down
9 changes: 6 additions & 3 deletions src/Kiota.Builder/Plugins/PluginsGenerationService.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down Expand Up @@ -67,7 +70,7 @@ public async Task GenerateManifestAsync(CancellationToken cancellationToken = de
case PluginType.APIManifest:
var apiManifest = new ApiManifestDocument("application"); //TODO add application name

Check warning on line 71 in src/Kiota.Builder/Plugins/PluginsGenerationService.cs

View workflow job for this annotation

GitHub Actions / Build 

Complete the task associated to this 'TODO' comment. (https://rules.sonarsource.com/csharp/RSPEC-1135)
// 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

Check warning on line 76 in src/Kiota.Builder/Plugins/PluginsGenerationService.cs

View workflow job for this annotation

GitHub Actions / Build 

Complete the task associated to this 'TODO' comment. (https://rules.sonarsource.com/csharp/RSPEC-1135)
Expand Down
7 changes: 6 additions & 1 deletion src/Kiota.Builder/WorkspaceManagement/BaseApiConsumerConfiguration.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,16 @@ private protected BaseApiConsumerConfiguration(GenerationConfiguration config)
/// The output path for the generated code, related to the configuration file.
/// </summary>
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);
Expand Down
25 changes: 16 additions & 9 deletions src/Kiota.Builder/WorkspaceManagement/WorkspaceManagementService.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -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);
Expand Down Expand Up @@ -116,19 +119,21 @@ public async Task<bool> 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;
}
Expand Down Expand Up @@ -292,7 +297,8 @@ public async Task<IEnumerable<string>> 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
Expand All @@ -302,7 +308,8 @@ public async Task<IEnumerable<string>> MigrateFromLockFileAsync(string clientNam
inputConfigurationHash,
new Dictionary<string, HashSet<string>> {
{ MigrationPlaceholderPath, new HashSet<string> { "GET" } }
}));
},
WorkingDirectory));
lockManagementService.DeleteLockFile(Path.Combine(WorkingDirectory, clientConfiguration.OutputPath));
}
await workspaceConfigurationStorageService.UpdateWorkspaceConfigurationAsync(wsConfig, apiManifest, cancellationToken).ConfigureAwait(false);
Expand Down
5 changes: 3 additions & 2 deletions tests/Kiota.Builder.Tests/Configuration/GenerationConfigurationTests.cs
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System.Collections.Generic;
using System.IO;
using System.Linq;
using Kiota.Builder.Configuration;
using Xunit;
Expand Down Expand Up @@ -34,7 +35,7 @@ public void ToApiDependency()
};
var apiDependency = generationConfiguration.ToApiDependency("foo", new Dictionary<string, HashSet<string>>{
{ "foo/bar", new HashSet<string>{"GET"}}
});
}, Path.GetTempPath());
Assert.NotNull(apiDependency);
Assert.NotNull(apiDependency.Extensions);
Assert.Equal("foo", apiDependency.Extensions[GenerationConfiguration.KiotaHashManifestExtensionKey].GetValue<string>());
Expand All @@ -54,7 +55,7 @@ public void ToApiDependencyDoesNotIncludeConfigHashIfEmpty()
};
var apiDependency = generationConfiguration.ToApiDependency(string.Empty, new Dictionary<string, HashSet<string>>{
{ "foo/bar", new HashSet<string>{"GET"}}
});
}, Path.GetTempPath());
Assert.NotNull(apiDependency);
Assert.NotNull(apiDependency.Extensions);
Assert.False(apiDependency.Extensions.ContainsKey(GenerationConfiguration.KiotaHashManifestExtensionKey));
Expand Down
14 changes: 8 additions & 6 deletions tests/Kiota.Builder.Tests/Plugins/PluginsGenerationServiceTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,10 @@ public sealed class PluginsGenerationServiceTests : IDisposable
[Fact]
public void Defensive()
{
Assert.Throws<ArgumentNullException>(() => new PluginsGenerationService(null, OpenApiUrlTreeNode.Create(), new()));
Assert.Throws<ArgumentNullException>(() => new PluginsGenerationService(new(), null, new()));
Assert.Throws<ArgumentNullException>(() => new PluginsGenerationService(new(), OpenApiUrlTreeNode.Create(), null));
Assert.Throws<ArgumentNullException>(() => new PluginsGenerationService(null, OpenApiUrlTreeNode.Create(), new(), "foo"));
Assert.Throws<ArgumentNullException>(() => new PluginsGenerationService(new(), null, new(), "foo"));
Assert.Throws<ArgumentNullException>(() => new PluginsGenerationService(new(), OpenApiUrlTreeNode.Create(), null, "foo"));
Assert.Throws<ArgumentException>(() => new PluginsGenerationService(new(), OpenApiUrlTreeNode.Create(), new(), string.Empty));
}

public void Dispose()
Expand All @@ -46,11 +47,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<ILogger<PluginsGenerationService>>();
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,
Expand All @@ -63,7 +65,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, ManifestFileName)));
Expand Down

0 comments on commit 0493af1

Please sign in to comment.