Skip to content

Commit

Permalink
Merge pull request #4670 from microsoft/andrueastman/operationIds
Browse files Browse the repository at this point in the history
Fixes missing functions when operationIds aren't present in the input openApi doc
  • Loading branch information
andrueastman authored May 16, 2024
2 parents 2f2d203 + 6e39389 commit b242eaf
Show file tree
Hide file tree
Showing 6 changed files with 234 additions and 7 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
35 changes: 35 additions & 0 deletions src/Kiota.Builder/KiotaBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -345,6 +352,34 @@ private static Dictionary<Glob, HashSet<OperationType>> 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<string, KeyValuePair<OperationType, OpenApiOperation>>(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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
193 changes: 192 additions & 1 deletion tests/Kiota.Builder.Tests/KiotaBuilderTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -24,7 +25,7 @@
using Xunit;

namespace Kiota.Builder.Tests;
public sealed class KiotaBuilderTests : IDisposable
public sealed partial class KiotaBuilderTests : IDisposable
{
private readonly List<string> _tempFiles = new();
public void Dispose()
Expand Down Expand Up @@ -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<string, OpenApiSchema> {
{
"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<string, OpenApiSchema> {
{
"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<string, OpenApiSchema> {
{
"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<string, OpenApiSchema> {
{
"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();
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand All @@ -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<OpenApiRuntime>().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));
Expand Down

0 comments on commit b242eaf

Please sign in to comment.