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..6f65099799 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(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.TrimStart('/').Split('/', StringSplitOptions.RemoveEmptyEntries)) + { + 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(); } 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));