From 32375b922e7c383212f3a61103696c3f285b0789 Mon Sep 17 00:00:00 2001 From: Andrew Omondi Date: Thu, 16 May 2024 12:46:57 +0300 Subject: [PATCH 1/4] Cleanup operation ids --- CHANGELOG.md | 2 +- .../OpenApiUrlTreeNodeExtensions.cs | 2 +- src/Kiota.Builder/KiotaBuilder.cs | 35 ++++ .../OpenApiUrlTreeNodeExtensionsTests.cs | 2 +- .../Kiota.Builder.Tests/KiotaBuilderTests.cs | 193 +++++++++++++++++- 5 files changed, 230 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 542a68dd53..611469f436 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,7 +36,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Fixes constructor generation for nullable properties that are initialized as null in C#,Java and PHP - Fixed a bug where the hash alias in typescript wasn't being generated uniformly for similar interfaces [microsoftgraph/msgraph-beta-sdk-typescript#84](https://github.com/microsoftgraph/msgraph-beta-sdk-typescript/issues/84) - Fixes a bug where name collisions would occur in the Typescript refiner if model name also exists with the `Interface` suffix [#4382](https://github.com/microsoft/kiota/issues/4382) - +- Fixes a bug where paths without operationIds would not be included in the generated plugins and ensured operationIds are cleaned up [#4642](https://github.com/microsoft/kiota/issues/4642) ## [1.14.0] - 2024-05-02 diff --git a/src/Kiota.Builder/Extensions/OpenApiUrlTreeNodeExtensions.cs b/src/Kiota.Builder/Extensions/OpenApiUrlTreeNodeExtensions.cs index 06dc242496..220d249bd5 100644 --- a/src/Kiota.Builder/Extensions/OpenApiUrlTreeNodeExtensions.cs +++ b/src/Kiota.Builder/Extensions/OpenApiUrlTreeNodeExtensions.cs @@ -163,7 +163,7 @@ public static string GetPathItemDescription(this OpenApiUrlTreeNode currentNode, public static bool DoesNodeBelongToItemSubnamespace(this OpenApiUrlTreeNode currentNode) => currentNode.IsPathSegmentWithSingleSimpleParameter(); public static bool IsPathSegmentWithSingleSimpleParameter(this OpenApiUrlTreeNode currentNode) => currentNode?.DeduplicatedSegment().IsPathSegmentWithSingleSimpleParameter() ?? false; - private static bool IsPathSegmentWithSingleSimpleParameter(this string currentSegment) + internal static bool IsPathSegmentWithSingleSimpleParameter(this string currentSegment) { if (string.IsNullOrEmpty(currentSegment)) return false; var segmentWithoutExtension = stripExtensionForIndexersRegex().Replace(currentSegment, string.Empty); diff --git a/src/Kiota.Builder/KiotaBuilder.cs b/src/Kiota.Builder/KiotaBuilder.cs index d193292dbf..3539f1674c 100644 --- a/src/Kiota.Builder/KiotaBuilder.cs +++ b/src/Kiota.Builder/KiotaBuilder.cs @@ -8,6 +8,7 @@ using System.Linq; using System.Net.Http; using System.Runtime.CompilerServices; +using System.Text; using System.Text.RegularExpressions; using System.Threading; using System.Threading.Tasks; @@ -192,6 +193,12 @@ private static string NormalizeApiManifestPath(RequestInfo request, string? base modelNamespacePrefixToTrim = GetDeeperMostCommonNamespaceNameForModels(openApiDocument); } + // OperationId cleanup in the event that we are generating plugins + if (config.IsPluginConfiguration) + { + CleanupOperationIdForPlugins(openApiDocument); + } + // Create Uri Space of API sw.Start(); openApiTree = CreateUriSpace(openApiDocument); @@ -345,6 +352,34 @@ private static Dictionary> GetFilterPatternsFromCon .Select(static y => y!.Value)), globComparer); } + + [GeneratedRegex(@"[^a-zA-Z0-9_]+", RegexOptions.IgnoreCase | RegexOptions.Singleline, 2000)] + private static partial Regex PluginOperationIdCleanupRegex(); + internal static void CleanupOperationIdForPlugins(OpenApiDocument document) + { + if (document.Paths is null) return; + foreach (var (pathItem, operation) in document.Paths.SelectMany(path => path.Value.Operations.Select(value => new Tuple>(path.Key, value)))) + { + if (string.IsNullOrEmpty(operation.Value.OperationId)) + { + var stringBuilder = new StringBuilder(); + foreach (var segment in pathItem.Split('/')) + { + if (segment.IsPathSegmentWithSingleSimpleParameter()) + stringBuilder.Append("item"); + else if (!string.IsNullOrEmpty(segment.Trim())) + stringBuilder.Append(segment.ToLowerInvariant()); + stringBuilder.Append('_'); + } + stringBuilder.Append(operation.Key.ToString().ToLowerInvariant()); + operation.Value.OperationId = stringBuilder.ToString(); + } + else + { + operation.Value.OperationId = PluginOperationIdCleanupRegex().Replace(operation.Value.OperationId, "_");//replace non-alphanumeric characters with _ + } + } + } internal void FilterPathsByPatterns(OpenApiDocument doc) { var includePatterns = GetFilterPatternsFromConfiguration(config.IncludePatterns); diff --git a/tests/Kiota.Builder.Tests/Extensions/OpenApiUrlTreeNodeExtensionsTests.cs b/tests/Kiota.Builder.Tests/Extensions/OpenApiUrlTreeNodeExtensionsTests.cs index e3ce0e266e..718248ee2f 100644 --- a/tests/Kiota.Builder.Tests/Extensions/OpenApiUrlTreeNodeExtensionsTests.cs +++ b/tests/Kiota.Builder.Tests/Extensions/OpenApiUrlTreeNodeExtensionsTests.cs @@ -17,7 +17,7 @@ public sealed class OpenApiUrlTreeNodeExtensionsTests : IDisposable public void Defensive() { Assert.False(OpenApiUrlTreeNodeExtensions.IsComplexPathMultipleParameters(null)); - Assert.False(OpenApiUrlTreeNodeExtensions.IsPathSegmentWithSingleSimpleParameter(null)); + Assert.False(OpenApiUrlTreeNodeExtensions.IsPathSegmentWithSingleSimpleParameter((OpenApiUrlTreeNode)null)); Assert.False(OpenApiUrlTreeNodeExtensions.DoesNodeBelongToItemSubnamespace(null)); Assert.Empty(OpenApiUrlTreeNodeExtensions.GetPathItemDescription(null, null)); Assert.Empty(OpenApiUrlTreeNodeExtensions.GetPathItemDescription(null, Label)); diff --git a/tests/Kiota.Builder.Tests/KiotaBuilderTests.cs b/tests/Kiota.Builder.Tests/KiotaBuilderTests.cs index b9297956ea..1032c91c52 100644 --- a/tests/Kiota.Builder.Tests/KiotaBuilderTests.cs +++ b/tests/Kiota.Builder.Tests/KiotaBuilderTests.cs @@ -4,6 +4,7 @@ using System.Linq; using System.Net.Http; using System.Text; +using System.Text.RegularExpressions; using System.Threading; using System.Threading.Tasks; @@ -24,7 +25,7 @@ using Xunit; namespace Kiota.Builder.Tests; -public sealed class KiotaBuilderTests : IDisposable +public sealed partial class KiotaBuilderTests : IDisposable { private readonly List _tempFiles = new(); public void Dispose() @@ -8483,4 +8484,194 @@ public void SupportsIncludeFilterAndExcludeWithOperationForSpecificPath() Assert.Single(administrativeUnitItemsRS.Methods.Where(static x => x.IsOfKind(CodeMethodKind.RequestExecutor) && x.HttpMethod == Builder.CodeDOM.HttpMethod.Patch)); Assert.Empty(administrativeUnitItemsRS.Methods.Where(static x => x.IsOfKind(CodeMethodKind.RequestExecutor) && x.HttpMethod == Builder.CodeDOM.HttpMethod.Delete)); } + [Fact] + public void CleansUpOperationIdAddsMissingOperationId() + { + var myObjectSchema = new OpenApiSchema + { + Type = "object", + Properties = new Dictionary { + { + "name", new OpenApiSchema { + Type = "string", + } + } + }, + Reference = new OpenApiReference + { + Id = "myobject", + Type = ReferenceType.Schema + }, + UnresolvedReference = false, + }; + var document = new OpenApiDocument + { + Paths = new OpenApiPaths + { + ["directory/administrativeUnits"] = new OpenApiPathItem + { + Operations = { + [OperationType.Get] = new OpenApiOperation + { + Responses = new OpenApiResponses + { + ["200"] = new OpenApiResponse { + Content = { + ["application/json"] = new OpenApiMediaType { + Schema = myObjectSchema + } + } + }, + } + }, + [OperationType.Post] = new OpenApiOperation + { + RequestBody = new OpenApiRequestBody { + Content = { + ["application/json"] = new OpenApiMediaType { + Schema = myObjectSchema + } + } + }, + Responses = new OpenApiResponses + { + ["201"] = new OpenApiResponse { + Content = { + ["application/json"] = new OpenApiMediaType { + Schema = myObjectSchema + } + } + }, + } + } + } + } + }, + Components = new() + { + Schemas = new Dictionary { + { + "myobject", myObjectSchema + } + } + } + }; + KiotaBuilder.CleanupOperationIdForPlugins(document); + var operations = document.Paths.SelectMany(path => path.Value.Operations).ToList(); + foreach (var path in operations) + { + Assert.False(string.IsNullOrEmpty(path.Value.OperationId)); //Assert that the operationId is not empty + Assert.EndsWith(path.Key.ToString().ToLowerInvariant(), path.Value.OperationId);// assert that the operationId ends with the operation type + Assert.Matches(OperationIdValidationRegex(), path.Value.OperationId); // assert that the operationId is clean an matches the regex + } + Assert.Equal("directory_administrativeunits_get", operations[0].Value.OperationId); + Assert.Equal("directory_administrativeunits_post", operations[1].Value.OperationId); + } + + [Fact] + public void CleansUpOperationIdChangesOperationId() + { + var myObjectSchema = new OpenApiSchema + { + Type = "object", + Properties = new Dictionary { + { + "name", new OpenApiSchema { + Type = "string", + } + } + }, + Reference = new OpenApiReference + { + Id = "myobject", + Type = ReferenceType.Schema + }, + UnresolvedReference = false, + }; + var document = new OpenApiDocument + { + Paths = new OpenApiPaths + { + ["directory/administrativeUnits"] = new OpenApiPathItem + { + Operations = { + [OperationType.Get] = new OpenApiOperation + { + Responses = new OpenApiResponses + { + ["200"] = new OpenApiResponse { + Content = { + ["application/json"] = new OpenApiMediaType { + Schema = myObjectSchema + } + } + }, + }, + OperationId = "GetAdministrativeUnits" // Nothing wrong with this operationId + }, + [OperationType.Post] = new OpenApiOperation + { + RequestBody = new OpenApiRequestBody { + Content = { + ["application/json"] = new OpenApiMediaType { + Schema = myObjectSchema + } + } + }, + Responses = new OpenApiResponses + { + ["201"] = new OpenApiResponse { + Content = { + ["application/json"] = new OpenApiMediaType { + Schema = myObjectSchema + } + } + }, + }, + OperationId = "PostAdministrativeUnits.With201-response" // operationId should be cleaned up + } + } + }, + ["directory/adminstativeUnits/{unit-id}"] = new OpenApiPathItem + { + Operations = { + [OperationType.Get] = new OpenApiOperation + { + Responses = new OpenApiResponses + { + ["200"] = new OpenApiResponse { + Content = { + ["application/json"] = new OpenApiMediaType { + Schema = myObjectSchema + } + } + }, + }, + // OperationId is missing + }, + } + } + }, + Components = new() + { + Schemas = new Dictionary { + { + "myobject", myObjectSchema + } + } + } + }; + KiotaBuilder.CleanupOperationIdForPlugins(document); + var operations = document.Paths.SelectMany(path => path.Value.Operations).ToList(); + foreach (var path in operations) + { + Assert.False(string.IsNullOrEmpty(path.Value.OperationId)); //Assert that the operationId is not empty + Assert.Matches(OperationIdValidationRegex(), path.Value.OperationId); // assert that the operationId is clean an matches the regex + } + Assert.Equal("GetAdministrativeUnits", operations[0].Value.OperationId); + Assert.Equal("PostAdministrativeUnits_With201_response", operations[1].Value.OperationId); + Assert.Equal("directory_adminstativeunits_item_get", operations[2].Value.OperationId); + } + [GeneratedRegex(@"^[a-zA-Z0-9_]*$", RegexOptions.IgnoreCase | RegexOptions.Singleline, 2000)] + private static partial Regex OperationIdValidationRegex(); } From 5a70115e11c7b61759deac73da540ecb0a8c6a8b Mon Sep 17 00:00:00 2001 From: Andrew Omondi Date: Thu, 16 May 2024 13:26:15 +0300 Subject: [PATCH 2/4] Adds more tests --- .../Plugins/PluginsGenerationServiceTests.cs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/Kiota.Builder.Tests/Plugins/PluginsGenerationServiceTests.cs b/tests/Kiota.Builder.Tests/Plugins/PluginsGenerationServiceTests.cs index ca604039e0..04e52db82c 100644 --- a/tests/Kiota.Builder.Tests/Plugins/PluginsGenerationServiceTests.cs +++ b/tests/Kiota.Builder.Tests/Plugins/PluginsGenerationServiceTests.cs @@ -44,14 +44,13 @@ public async Task GeneratesManifest() /test: get: description: description for test path - operationId: test responses: '200': description: test /test/{id}: get: description: description for test path with id - operationId: test_WithId + operationId: test.WithId parameters: - name: id in: path @@ -79,6 +78,7 @@ public async Task GeneratesManifest() }; var (openAPIDocumentStream, _) = await openAPIDocumentDS.LoadStreamAsync(simpleDescriptionPath, generationConfiguration, null, false); var openApiDocument = await openAPIDocumentDS.GetDocumentFromStreamAsync(openAPIDocumentStream, generationConfiguration); + KiotaBuilder.CleanupOperationIdForPlugins(openApiDocument); var urlTreeNode = OpenApiUrlTreeNode.Create(openApiDocument, Constants.DefaultOpenApiLabel); var pluginsGenerationService = new PluginsGenerationService(openApiDocument, urlTreeNode, generationConfiguration, workingDirectory); @@ -95,7 +95,8 @@ public async Task GeneratesManifest() var resultingManifest = PluginManifestDocument.Load(jsonDocument.RootElement); Assert.NotNull(resultingManifest.Document); Assert.Equal(OpenApiFileName, resultingManifest.Document.Runtimes.OfType().First().Spec.Url); - Assert.Empty(resultingManifest.Problems); + Assert.Equal(2, resultingManifest.Document.Functions.Count);// all functions are generated despite missing operationIds + Assert.Empty(resultingManifest.Problems);// no problems are expected with names // Validate the v1 plugin var v1ManifestContent = await File.ReadAllTextAsync(Path.Combine(outputDirectory, OpenAIPluginFileName)); From 8a516318b9a0e3dba1ecf692c515506b88676493 Mon Sep 17 00:00:00 2001 From: Andrew Omondi Date: Thu, 16 May 2024 13:31:32 +0300 Subject: [PATCH 3/4] minor cleanup --- src/Kiota.Builder/KiotaBuilder.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Kiota.Builder/KiotaBuilder.cs b/src/Kiota.Builder/KiotaBuilder.cs index 3539f1674c..2a700ad9d8 100644 --- a/src/Kiota.Builder/KiotaBuilder.cs +++ b/src/Kiota.Builder/KiotaBuilder.cs @@ -358,12 +358,12 @@ private static Dictionary> GetFilterPatternsFromCon internal static void CleanupOperationIdForPlugins(OpenApiDocument document) { if (document.Paths is null) return; - foreach (var (pathItem, operation) in document.Paths.SelectMany(path => path.Value.Operations.Select(value => new Tuple>(path.Key, value)))) + foreach (var (pathItem, operation) in document.Paths.SelectMany(static path => path.Value.Operations.Select(value => new Tuple>(path.Key, value)))) { if (string.IsNullOrEmpty(operation.Value.OperationId)) { var stringBuilder = new StringBuilder(); - foreach (var segment in pathItem.Split('/')) + foreach (var segment in pathItem.TrimStart('/').Split('/',StringSplitOptions.RemoveEmptyEntries)) { if (segment.IsPathSegmentWithSingleSimpleParameter()) stringBuilder.Append("item"); From 6e3938952a09719dfdd9800fd4419d976377a038 Mon Sep 17 00:00:00 2001 From: Andrew Omondi Date: Thu, 16 May 2024 13:35:32 +0300 Subject: [PATCH 4/4] format --- 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 2a700ad9d8..6f65099799 100644 --- a/src/Kiota.Builder/KiotaBuilder.cs +++ b/src/Kiota.Builder/KiotaBuilder.cs @@ -363,7 +363,7 @@ internal static void CleanupOperationIdForPlugins(OpenApiDocument document) if (string.IsNullOrEmpty(operation.Value.OperationId)) { var stringBuilder = new StringBuilder(); - foreach (var segment in pathItem.TrimStart('/').Split('/',StringSplitOptions.RemoveEmptyEntries)) + foreach (var segment in pathItem.TrimStart('/').Split('/', StringSplitOptions.RemoveEmptyEntries)) { if (segment.IsPathSegmentWithSingleSimpleParameter()) stringBuilder.Append("item");