Skip to content

Commit

Permalink
Fix: Escape paths containing the keyword "item" (#4900)
Browse files Browse the repository at this point in the history
* Fix: Escape paths containing the keyword "item"

Fixes #4814

* Add unit test for escaping "item"

* chore: code formatting

* chore: code linting

Signed-off-by: Vincent Biret <[email protected]>

* fix: type name for navigation property

Signed-off-by: Vincent Biret <[email protected]>

* fix: test data for naming convention

Signed-off-by: Vincent Biret <[email protected]>

* Move constant values to constants.

* Refactor: Move constants to OpenApiUrlTreeNodeExtensions.cs and adjust unit test

* docs: adds changelog entry for item fix

---------

Signed-off-by: Vincent Biret <[email protected]>
Co-authored-by: Vincent Biret <[email protected]>
  • Loading branch information
CasperWSchmidt and baywet authored Sep 13, 2024
1 parent 3381a4b commit 7f0cb96
Show file tree
Hide file tree
Showing 4 changed files with 96 additions and 11 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

- 📢📢📢 The Visual Studio Code extension is now generally available!. [#5368](https://github.com/microsoft/kiota/issues/5368)
- Fixed a stack overflow in the core generator caused by circular comparisons. [#5369](https://github.com/microsoft/kiota/issues/5369)
- Fixed a bug where a path segment named "item" after a single parameter path segment would derail generation. [#4814](https://github.com/microsoft/kiota/issues/4814)
- Fixed a bug where collection/array of primitive types members for union/intersection types would be ignored. [#5283](https://github.com/microsoft/kiota/issues/5283)
- Updated dependencies command and view to reflect the availability of bundles. [#5317](https://github.com/microsoft/kiota/issues/5317)
- Fixed a when generating a plugin when only an operation is selected in the root node in the extension. [#5300](https://github.com/microsoft/kiota/issues/5300)
Expand Down
4 changes: 3 additions & 1 deletion src/Kiota.Builder/Extensions/OpenApiUrlTreeNodeExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,11 @@ public static partial class OpenApiUrlTreeNodeExtensions
{
private static string GetDotIfBothNotNullOfEmpty(string x, string y) => string.IsNullOrEmpty(x) || string.IsNullOrEmpty(y) ? string.Empty : ".";
private static readonly Func<string, string> replaceSingleParameterSegmentByItem =
static x => x.IsPathSegmentWithSingleSimpleParameter() ? "item" : x;
static x => x.IsPathSegmentWithSingleSimpleParameter() ? "item" : (ReservedItemName.Equals(x, StringComparison.OrdinalIgnoreCase) ? ReservedItemNameEscaped : x);
private static readonly char[] namespaceNameSplitCharacters = ['.', '-', '$']; //$ref from OData
private const string EscapedSuffix = "Escaped";
internal const string ReservedItemName = "Item";
internal const string ReservedItemNameEscaped = $"{ReservedItemName}_{EscapedSuffix}";
internal static string GetNamespaceFromPath(this string currentPath, string prefix) =>
prefix +
((currentPath?.Contains(PathNameSeparator, StringComparison.OrdinalIgnoreCase) ?? false) ?
Expand Down
22 changes: 12 additions & 10 deletions src/Kiota.Builder/KiotaBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -618,7 +618,7 @@ private void CreateRequestBuilderClass(CodeNamespace currentNamespace, OpenApiUr
var className = currentNode.DoesNodeBelongToItemSubnamespace() ? currentNode.GetNavigationPropertyName(config.StructuredMimeTypes, ItemRequestBuilderSuffix) : currentNode.GetNavigationPropertyName(config.StructuredMimeTypes, RequestBuilderSuffix);
codeClass = targetNS.AddClass(new CodeClass
{
Name = className.CleanupSymbolName(),
Name = currentNamespace.Name.EndsWith(OpenApiUrlTreeNodeExtensions.ReservedItemNameEscaped, StringComparison.OrdinalIgnoreCase) ? className.CleanupSymbolName().Replace(OpenApiUrlTreeNodeExtensions.ReservedItemName, OpenApiUrlTreeNodeExtensions.ReservedItemNameEscaped, StringComparison.OrdinalIgnoreCase) : className.CleanupSymbolName(),
Kind = CodeClassKind.RequestBuilder,
Documentation = new()
{
Expand All @@ -630,21 +630,23 @@ private void CreateRequestBuilderClass(CodeNamespace currentNamespace, OpenApiUr
logger.LogTrace("Creating class {Class}", codeClass.Name);

// Add properties for children
foreach (var child in currentNode.Children)
foreach (var child in currentNode.Children.Select(static x => x.Value))
{
var propIdentifier = child.Value.GetNavigationPropertyName(config.StructuredMimeTypes);
var propType = child.Value.GetNavigationPropertyName(config.StructuredMimeTypes, child.Value.DoesNodeBelongToItemSubnamespace() ? ItemRequestBuilderSuffix : RequestBuilderSuffix);
var propIdentifier = child.GetNavigationPropertyName(config.StructuredMimeTypes);
var propType = child.GetNavigationPropertyName(config.StructuredMimeTypes, child.DoesNodeBelongToItemSubnamespace() ? ItemRequestBuilderSuffix : RequestBuilderSuffix);
if (child.Path.EndsWith(OpenApiUrlTreeNodeExtensions.ReservedItemName, StringComparison.OrdinalIgnoreCase))
propType = propType.Replace(OpenApiUrlTreeNodeExtensions.ReservedItemName, OpenApiUrlTreeNodeExtensions.ReservedItemNameEscaped, StringComparison.OrdinalIgnoreCase);

if (child.Value.IsPathSegmentWithSingleSimpleParameter())
if (child.IsPathSegmentWithSingleSimpleParameter())
{
var indexerParameterType = GetIndexerParameter(child.Value, currentNode);
codeClass.AddIndexer(CreateIndexer($"{propIdentifier}-indexer", propType, indexerParameterType, child.Value, currentNode));
var indexerParameterType = GetIndexerParameter(child, currentNode);
codeClass.AddIndexer(CreateIndexer($"{propIdentifier}-indexer", propType, indexerParameterType, child, currentNode));
}
else if (child.Value.IsComplexPathMultipleParameters())
CreateMethod(propIdentifier, propType, codeClass, child.Value);
else if (child.IsComplexPathMultipleParameters())
CreateMethod(propIdentifier, propType, codeClass, child);
else
{
var description = child.Value.GetPathItemDescription(Constants.DefaultOpenApiLabel).CleanupDescription();
var description = child.GetPathItemDescription(Constants.DefaultOpenApiLabel).CleanupDescription();
var prop = CreateProperty(propIdentifier, propType, kind: CodePropertyKind.RequestBuilder); // we should add the type definition here but we can't as it might not have been generated yet
if (prop is null)
{
Expand Down
80 changes: 80 additions & 0 deletions tests/Kiota.Builder.Tests/KiotaBuilderTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
using Moq;

using Xunit;
using HttpMethod = Kiota.Builder.CodeDOM.HttpMethod;

namespace Kiota.Builder.Tests;
public sealed partial class KiotaBuilderTests : IDisposable
Expand Down Expand Up @@ -229,6 +230,85 @@ await File.WriteAllTextAsync(tempFilePath, @$"openapi: 3.0.1
Assert.Null(modelsNS.FindNamespaceByName("ApiSdk.models.Specialized-Complex"));
Assert.NotNull(specializedNS.FindChildByName<CodeClass>("StorageAccount", false));
}
[Fact]
public async Task HandlesPathWithItemInNameSegment()
{
var tempFilePath = Path.Combine(Path.GetTempPath(), Path.GetTempFileName());
await File.WriteAllTextAsync(tempFilePath, @$"openapi: 3.0.1
info:
title: OData Service for namespace microsoft.graph
description: This OData service is located at https://graph.microsoft.com/v1.0
version: 1.0.1
servers:
- url: https://api.funtranslations.com
paths:
/media/item/{{id}}:
get:
parameters:
- name: id
in: path
required: true
schema:
type: string
responses:
'200':
content:
application/json:
schema:
$ref: '#/components/schemas/MediaResponseModel'
components:
schemas:
MediaResponseModel:
type: object
properties:
name:
type: string
id:
type: string
format: uuid
mediaType:
type: string
url:
type: string");
var mockLogger = new Mock<ILogger<KiotaBuilder>>();
var builder = new KiotaBuilder(mockLogger.Object, new GenerationConfiguration
{
ClientClassName = "Graph",
OpenAPIFilePath = "https://api.apis.guru/v2/specs/funtranslations.com/starwars/2.3/swagger.json"
}, _httpClient);
await using var fs = new FileStream(tempFilePath, FileMode.Open);
var document = await builder.CreateOpenApiDocumentAsync(fs);
var node = builder.CreateUriSpace(document);
builder.SetApiRootUrl();
var codeModel = builder.CreateSourceModel(node);
var rootNS = codeModel.FindNamespaceByName("ApiSdk");
Assert.NotNull(rootNS);
var mediaBuilderNs = codeModel.FindNamespaceByName("ApiSdk.media");
Assert.NotNull(mediaBuilderNs);
var mediaRequestBuilder = mediaBuilderNs.FindChildByName<CodeClass>("MediaRequestBuilder", false);
Assert.NotNull(mediaRequestBuilder);
var navigationProperty = mediaRequestBuilder.Properties.FirstOrDefault(prop =>
prop.IsOfKind(CodePropertyKind.RequestBuilder) &&
prop.Name.Equals("Item", StringComparison.OrdinalIgnoreCase));
Assert.NotNull(navigationProperty);
Assert.Equal("Item_EscapedRequestBuilder", navigationProperty.Type.Name);
var itemBuilderNs = mediaBuilderNs.FindNamespaceByName("ApiSdk.media.item_escaped");
Assert.NotNull(itemBuilderNs);
var itemRequestBuilder = itemBuilderNs.FindChildByName<CodeClass>("Item_escapedRequestBuilder", false);
Assert.NotNull(itemRequestBuilder.Indexer);
Assert.Equal("ItemItemRequestBuilder", itemRequestBuilder.Indexer.ReturnType.Name);
var nestedItemBuilderNs = itemBuilderNs.FindNamespaceByName("ApiSdk.media.item_escaped.item");
Assert.NotNull(nestedItemBuilderNs);
var nestedItemRequestBuilder = nestedItemBuilderNs.FindChildByName<CodeClass>("ItemItemRequestBuilder", false);
Assert.NotNull(nestedItemRequestBuilder);
Assert.NotNull(nestedItemRequestBuilder.Methods.FirstOrDefault(m =>
m.HttpMethod == HttpMethod.Get &&
m.IsAsync &&
m.Name.Equals("Get", StringComparison.OrdinalIgnoreCase)));
var modelsNS = codeModel.FindNamespaceByName("ApiSdk.models");
Assert.NotNull(modelsNS);
Assert.NotNull(modelsNS.FindChildByName<CodeClass>("MediaResponseModel", false));
}
private readonly HttpClient _httpClient = new();
[Fact]
public async Task ParsesEnumDescriptionsAsync()
Expand Down

0 comments on commit 7f0cb96

Please sign in to comment.