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

Support trailing slash in URL paths #5651

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft
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
1 change: 1 addition & 0 deletions src/Kiota.Builder/CodeDOM/CodeConstant.cs
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ public CodeDocumentation Documentation
Name = $"{codeClass.Name.ToFirstCharacterLowerCase()}{RequestsMetadataSuffix}",
Kind = CodeConstantKind.RequestsMetadata,
OriginalCodeElement = codeClass,
UriTemplate = $"{codeClass.Name.ToFirstCharacterLowerCase()}{UriTemplateSuffix}",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why are we using the class name as uri template here?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mentioned it briefly in the PR description but part of the fix in the TypeScript refiner is merging multiple CodeFiles into one CodeFile.
The consequence is that a CodeFile now has multiple UriTemplates in it.
When the TypeScript writer finally gets to writing the request builder's RequestsMetadata constant, it previously only selects the first UriTemplate even if it is the wrong one.
This commit sets UriTemplate in the RequestMetadata CodeConstant so that the TypeScript writer can use it to look up the correct UriTemplate.

};
result.Documentation.DescriptionTemplate = "Metadata for all the requests in the request builder.";
if (usingsToAdd is { Length: > 0 } usingsToAddList)
Expand Down
5 changes: 5 additions & 0 deletions src/Kiota.Builder/Extensions/StringExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,11 @@ private static string NormalizeSymbolsBeforeCleanup(string original)
result = result.Replace("+", "_plus_", StringComparison.OrdinalIgnoreCase);
}

if (result.Contains('\\', StringComparison.OrdinalIgnoreCase))
{
result = result.Replace(@"\", "Slash", StringComparison.OrdinalIgnoreCase);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

from reading the unit tests this effectively inserts an additional suffix in the request builder's name.
this solution looks disruptive to me, but I'll follow up in the main comment.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I completely agree and look forward to whatever alternative we come up with!

}

return result;
}
/// <summary>
Expand Down
31 changes: 31 additions & 0 deletions src/Kiota.Builder/Refiners/TypeScriptRefiner.cs
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,7 @@ public override Task RefineAsync(CodeNamespace generatedCode, CancellationToken
GenerateReusableModelsCodeFiles(modelsNamespace);
GenerateRequestBuilderCodeFiles(modelsNamespace);
GroupReusableModelsInSingleFile(modelsNamespace);
MergeConflictingBuilderCodeFiles(modelsNamespace);
RemoveSelfReferencingUsings(generatedCode);
AddAliasToCodeFileUsings(generatedCode);
CorrectSerializerParameters(generatedCode);
Expand Down Expand Up @@ -301,6 +302,36 @@ parentNamespace.Parent is CodeNamespace parentLevelNamespace &&
CrawlTree(currentElement, AddDownwardsConstantsImports);
}

private static void MergeConflictingBuilderCodeFiles(CodeNamespace modelsNamespace)
{
if (modelsNamespace.Parent is not CodeNamespace mainNamespace) return;
var elementsToConsider = mainNamespace.Namespaces.Except([modelsNamespace]).OfType<CodeElement>().ToArray();
foreach (var element in elementsToConsider)
MergeConflictingBuilderCodeFilesForElement(element);
}
private static void MergeConflictingBuilderCodeFilesForElement(CodeElement currentElement)
{
if (currentElement is CodeNamespace currentNamespace && currentNamespace.Files.ToArray() is { Length: > 1 } codeFiles)
{
var targetFile = codeFiles.First();
foreach (var fileToMerge in codeFiles.Skip(1))
{
if (fileToMerge.Classes.Any())
targetFile.AddElements(fileToMerge.Classes.ToArray());
if (fileToMerge.Constants.Any())
targetFile.AddElements(fileToMerge.Constants.ToArray());
if (fileToMerge.Enums.Any())
targetFile.AddElements(fileToMerge.Enums.ToArray());
if (fileToMerge.Interfaces.Any())
targetFile.AddElements(fileToMerge.Interfaces.ToArray());
if (fileToMerge.Usings.Any())
targetFile.AddElements(fileToMerge.Usings.ToArray());
currentNamespace.RemoveChildElement(fileToMerge);
}
}
CrawlTree(currentElement, MergeConflictingBuilderCodeFilesForElement);
}

private static CodeFile? GenerateModelCodeFile(CodeInterface codeInterface, CodeNamespace codeNamespace)
{
var functions = GetSerializationAndFactoryFunctions(codeInterface, codeNamespace).ToArray();
Expand Down
16 changes: 14 additions & 2 deletions src/Kiota.Builder/Writers/TypeScript/CodeConstantWriter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,20 @@ private void WriteRequestsMetadataConstant(CodeConstant codeElement, LanguageWri
.OrderBy(static x => x.Name, StringComparer.OrdinalIgnoreCase)
.ToArray() is not { Length: > 0 } executorMethods)
return;
var uriTemplateConstant = codeElement.Parent is CodeFile parentFile && parentFile.Constants.FirstOrDefault(static x => x.Kind is CodeConstantKind.UriTemplate) is CodeConstant tplct ?
tplct : throw new InvalidOperationException("Couldn't find the associated uri template constant for the requests metadata constant");
CodeConstant? uriTemplateConstant = null;
if (codeElement.Parent is CodeFile parentFile)
{
var uriTemplates = parentFile.Constants.Where(static x => x.Kind is CodeConstantKind.UriTemplate).ToArray();
var uriTemplate = uriTemplates.Length == 1
? uriTemplates.First()
: uriTemplates.FirstOrDefault(x => x.Name == codeElement.UriTemplate);
if (uriTemplate is CodeConstant tplct)
{
uriTemplateConstant = tplct;
}
}
if (uriTemplateConstant == null)
throw new InvalidOperationException("Couldn't find the associated uri template constant for the requests metadata constant");
writer.StartBlock($"export const {codeElement.Name.ToFirstCharacterUpperCase()}: RequestsMetadata = {{");
foreach (var executorMethod in executorMethods)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ public void NormalizeNameSpaceName()
Assert.Equal("Toto", "toto".NormalizeNameSpaceName("-"));
Assert.Equal("Microsoft_Graph_Message_Content", "microsoft.Graph.Message.Content".NormalizeNameSpaceName("_"));
}
[InlineData("\" !#$%&'()*+,./:;<=>?@[]\\^`{}|~-", "plus")]
[InlineData("\" !#$%&'()*+,./:;<=>?@[]^`{}|~-", "plus")]
[InlineData("unchanged", "unchanged")]
[InlineData("@odata.changed", "OdataChanged")]
[InlineData("specialLast@", "specialLast")]
Expand All @@ -98,6 +98,9 @@ public void NormalizeNameSpaceName()
[InlineData("-1-", "minus_1")]
[InlineData("-1-1", "minus_11")]
[InlineData("-", "minus")]
[InlineData("\\", "Slash")]
[InlineData("Param\\", "ParamSlash")]
[InlineData("Param\\RequestBuilder", "ParamSlashRequestBuilder")]
[Theory]
public void CleansUpSymbolNames(string input, string expected)
{
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,132 @@
namespace Kiota.Builder.Tests.OpenApiSampleFiles;

public static class TrailingSlashSampleYml
{
/**
* An OpenAPI 3.0.0 sample document with trailing slashes on some paths.
*/
public static readonly string OpenApiYaml = @"
openapi: 3.0.0
info:
title: Sample API
description: A sample API that uses trailing slashes.
version: 1.0.0
servers:
- url: https://api.example.com/v1
paths:
/foo:
get:
summary: Get foo
description: Returns foo.
responses:
'200':
description: foo
content:
text/plain:
schema:
type: string
/foo/:
get:
summary: Get foo slash
description: Returns foo slash.
responses:
'200':
description: foo slash
content:
text/plain:
schema:
type: string
/message/{id}:
get:
summary: Get a Message
description: Returns a single Message object.
parameters:
- name: id
in: path
required: true
schema:
type: string
responses:
'200':
description: A Message object
content:
application/json:
schema:
$ref: '#/components/schemas/Message'
/message/{id}/:
get:
summary: Get replies to a Message
description: Returns a list of Message object replies for a Message.
parameters:
- name: id
in: path
required: true
schema:
type: string
responses:
'200':
description: A list of Message objects
content:
application/json:
schema:
type: array
items:
$ref: '#/components/schemas/Message'
/bucket/{name}/:
get:
summary: List items in a bucket
description: Returns a list of BucketFiles in a bucket.
parameters:
- name: name
in: path
required: true
schema:
type: string
responses:
'200':
description: A list of BucketFile objects
content:
application/json:
schema:
type: array
items:
$ref: '#/components/schemas/BucketFile'
/bucket/{name}/{id}:
get:
summary: Get a bucket item
description: Returns a single BucketFile object.
parameters:
- name: name
in: path
required: true
schema:
type: string
- name: id
in: path
required: true
schema:
type: string
responses:
'200':
description: A BucketFile object
content:
application/json:
schema:
$ref: '#/components/schemas/BucketFile'
components:
schemas:
Message:
type: object
properties:
Guid:
type: string
required:
- Guid
BucketFile:
type: object
properties:
Guid:
type: string
required:
- Guid";
}
Original file line number Diff line number Diff line change
Expand Up @@ -961,6 +961,55 @@ public async Task ParsesAndRefinesUnionOfPrimitiveValuesAsync()
var unionType = modelCodeFile.GetChildElements().Where(x => x is CodeFunction function && TypeScriptRefiner.GetOriginalComposedType(function.OriginalLocalMethod.ReturnType) is not null).ToList();
Assert.True(unionType.Count > 0);
}
[Fact]
public async Task ParsesAndRefinesPathsWithTrailingSlashAsync()
{
var generationConfiguration = new GenerationConfiguration { Language = GenerationLanguage.TypeScript };
var tempFilePath = Path.Combine(Path.GetTempPath(), Path.GetTempFileName());
await File.WriteAllTextAsync(tempFilePath, TrailingSlashSampleYml.OpenApiYaml);
var mockLogger = new Mock<ILogger<KiotaBuilder>>();
var builder = new KiotaBuilder(mockLogger.Object, new GenerationConfiguration { ClientClassName = "Testing", Serializers = ["none"], Deserializers = ["none"] }, _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);
await ILanguageRefiner.RefineAsync(generationConfiguration, rootNS);
Assert.NotNull(rootNS);

var fooNS = rootNS.FindNamespaceByName("ApiSdk.foo");
Assert.NotNull(fooNS);
var fooCodeFile = fooNS.FindChildByName<CodeFile>("fooRequestBuilder", false);
Assert.NotNull(fooCodeFile);
var fooRequestBuilder = fooCodeFile.FindChildByName<CodeInterface>("fooRequestBuilder", false);
var fooSlashRequestBuilder = fooCodeFile.FindChildByName<CodeInterface>("fooSlashRequestBuilder", false);
Assert.NotNull(fooRequestBuilder);
Assert.NotNull(fooSlashRequestBuilder);

var messageNS = rootNS.FindNamespaceByName("ApiSdk.message");
Assert.NotNull(messageNS);
var messageCodeFile = messageNS.FindChildByName<CodeFile>("messageRequestBuilder", false);
Assert.NotNull(messageCodeFile);
var messageRequestBuilder = messageCodeFile.FindChildByName<CodeInterface>("messageRequestBuilder", false);
Assert.NotNull(messageRequestBuilder);
var messageWithIdSlashMethod = messageRequestBuilder.FindChildByName<CodeMethod>("withIdSlash", false);
var messageByIdMethod = messageRequestBuilder.FindChildByName<CodeMethod>("byId", false);
Assert.NotNull(messageWithIdSlashMethod);
Assert.NotNull(messageByIdMethod);

var bucketNS = rootNS.FindNamespaceByName("ApiSdk.bucket");
Assert.NotNull(bucketNS);
var bucketItemNS = bucketNS.FindChildByName<CodeNamespace>("ApiSdk.bucket.item", false);
Assert.NotNull(bucketItemNS);
var bucketItemCodeFile = bucketItemNS.FindChildByName<CodeFile>("WithNameItemRequestBuilder", false);
Assert.NotNull(bucketItemCodeFile);
var bucketWithNameItemRequestBuilder = bucketItemCodeFile.FindChildByName<CodeInterface>("WithNameItemRequestBuilder", false);
var bucketWithNameSlashRequestBuilder = bucketItemCodeFile.FindChildByName<CodeInterface>("WithNameSlashRequestBuilder", false);
Assert.NotNull(bucketWithNameItemRequestBuilder);
Assert.NotNull(bucketWithNameSlashRequestBuilder);
}

[Fact]
public void GetOriginalComposedType_ReturnsNull_WhenElementIsNull()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ public void NamesDontDiffer_DoesntWriteEnumMember()
}

[Theory]
[InlineData("\\", "BackSlash")]
[InlineData("\\", "Slash")]
[InlineData("?", "QuestionMark")]
[InlineData("$", "Dollar")]
[InlineData("~", "Tilde")]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -477,6 +477,33 @@ public void WritesRequestExecutorBody()
Assert.Contains("403: createError403FromDiscriminatorValue as ParsableFactory<Parsable>", result);
}
[Fact]
public void WritesRequestsMetadataWithCorrectUriTemplate()
{
parentClass.Kind = CodeClassKind.RequestBuilder;
method.Kind = CodeMethodKind.RequestExecutor;
method.HttpMethod = HttpMethod.Get;
AddRequestProperties();
AddRequestBodyParameters();
var constant = CodeConstant.FromRequestBuilderToRequestsMetadata(parentClass);
var codeFile = root.TryAddCodeFile("foo", constant);
codeFile.AddElements(new CodeConstant
{
Name = "firstAndWrongUriTemplate",
Kind = CodeConstantKind.UriTemplate,
UriTemplate = "{+baseurl}/foo/"
});
codeFile.AddElements(new CodeConstant
{
Name = "parentClassUriTemplate",
Kind = CodeConstantKind.UriTemplate,
UriTemplate = "{+baseurl}/foo"
});
writer.Write(constant);
var result = tw.ToString();
Assert.Contains("uriTemplate: ParentClassUriTemplate", result);
AssertExtensions.CurlyBracesAreClosed(result);
}
[Fact]
public void WritesIndexer()
{
AddRequestProperties();
Expand Down
Loading