Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

- fixes workspace absolute path for relative descriptions #4587

Merged
merged 1 commit into from
May 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
14 changes: 12 additions & 2 deletions src/Kiota.Builder/Configuration/GenerationConfiguration.cs
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
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@
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 Expand Up @@ -1012,7 +1012,7 @@

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

Check warning on line 1015 in src/Kiota.Builder/KiotaBuilder.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)
var backCompatibleValue = (CodeIndexer)result[0].Clone();
backCompatibleValue.Name += "-string";
backCompatibleValue.IndexParameter.Type = DefaultIndexerParameterType;
Expand Down Expand Up @@ -1166,7 +1166,7 @@
var suffix = $"{operationType}Response";
var modelType = CreateModelDeclarations(currentNode, schema, operation, parentClass, suffix);
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

Check warning on line 1169 in src/Kiota.Builder/KiotaBuilder.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)
var obsoleteTypeName = modelType.Name[..^suffix.Length] + "Response";
if (modelType is CodeType codeType &&
codeType.TypeDefinition is CodeClass codeClass)
Expand Down Expand Up @@ -1299,7 +1299,7 @@
executorMethod.AddParameter(cancellationParam);// Add cancellation token parameter

if (returnTypes.Item2 is not null && config.IncludeBackwardCompatible)
{ //TODO remove for v2

Check warning on line 1302 in src/Kiota.Builder/KiotaBuilder.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)
var additionalExecutorMethod = (CodeMethod)executorMethod.Clone();
additionalExecutorMethod.ReturnType = returnTypes.Item2;
additionalExecutorMethod.OriginalMethod = executorMethod;
Expand Down Expand Up @@ -2308,7 +2308,7 @@
if (!parameterClass.ContainsPropertyWithWireName(prop.WireName))
{
if (addBackwardCompatibleParameter && config.IncludeBackwardCompatible && config.Language is GenerationLanguage.CSharp or GenerationLanguage.Go)
{ //TODO remove for v2

Check warning on line 2311 in src/Kiota.Builder/KiotaBuilder.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)
var modernProp = (CodeProperty)prop.Clone();
modernProp.Name = $"{prop.Name}As{modernProp.Type.Name.ToFirstCharacterUpperCase()}";
modernProp.SerializationName = prop.WireName;
Expand Down
11 changes: 7 additions & 4 deletions src/Kiota.Builder/Plugins/PluginsGenerationService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,22 +21,25 @@
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 @@ -65,12 +68,12 @@
pluginDocument.Write(writer);
break;
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)
// intentional drop to the default case
default:
throw new NotImplementedException($"The {pluginType} plugin is not implemented.");
Expand Down Expand Up @@ -103,7 +106,7 @@
{
SchemaVersion = "v2",
NameForHuman = OAIDocument.Info?.Title.CleanupXMLString(),
// TODO name for model ???

Check warning on line 109 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)
DescriptionForHuman = descriptionForHuman,
DescriptionForModel = descriptionForModel,
ContactEmail = OAIDocument.Info?.Contact?.Email,
Expand Down Expand Up @@ -136,7 +139,7 @@
Auth = new AnonymousAuth(),
Spec = new OpenApiRuntimeSpec()
{
Url = openApiDocumentPath
Url = openApiDocumentPath,
},
RunForFunctions = [operation.OperationId]
});
Expand All @@ -144,7 +147,7 @@
.Union(pathItem.Parameters.Where(static x => x.In is ParameterLocation.Path))
.Where(static x => x.Schema?.Type is not null && scalarTypes.Contains(x.Schema.Type))
.ToArray();
//TODO add request body

Check warning on line 150 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)

functions.Add(new Function
{
Expand Down
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
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
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
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,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 @@ -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<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 @@ -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")));
Expand Down
Loading