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

- fixes a regression with path deduplication #3904

Merged
merged 2 commits into from
Dec 13, 2023
Merged
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 CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Changed

- Fixed a regression introduced by [#3760](https://github.com/microsoft/kiota/issues/3760) in 1.9.0 and its previews where indexer parameter name would be wrong leading to invalid URLs. [#3901](https://github.com/microsoft/kiota/issues/3901)
- Fixed a bug in the vscode extension where the "Paste API Manifest" button would not be able to parse the manifest.
- Enhance the way Enums are expressed in Typescript. [#2105](https://github.com/microsoft/kiota/issues/2105)

Expand Down
24 changes: 19 additions & 5 deletions src/Kiota.Builder/Extensions/OpenApiUrlTreeNodeExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -73,11 +73,11 @@ public static IEnumerable<OpenApiParameter> GetPathParametersForCurrentSegment(t
if (node != null &&
node.IsComplexPathMultipleParameters())
if (node.PathItems.TryGetValue(Constants.DefaultOpenApiLabel, out var pathItem))
return GetParametersForPathItem(pathItem, node.Segment);
return GetParametersForPathItem(pathItem, node.DeduplicatedSegment());
else if (node.Children.Any())
return node.Children
.Where(static x => x.Value.PathItems.ContainsKey(Constants.DefaultOpenApiLabel))
.SelectMany(x => GetParametersForPathItem(x.Value.PathItems[Constants.DefaultOpenApiLabel], node.Segment))
.SelectMany(x => GetParametersForPathItem(x.Value.PathItems[Constants.DefaultOpenApiLabel], node.DeduplicatedSegment()))
.Distinct();
return Enumerable.Empty<OpenApiParameter>();
}
Expand Down Expand Up @@ -110,7 +110,7 @@ private static string GetSegmentName(this OpenApiUrlTreeNode currentNode, Struct
responseClassName :
((requestBody ? operation?.GetRequestSchema(structuredMimeTypes) : operation?.GetResponseSchema(structuredMimeTypes))?.Reference?.GetClassName() is string requestClassName && !string.IsNullOrEmpty(requestClassName) ?
requestClassName :
CleanupParametersFromPath(currentNode.Segment)?.ReplaceValueIdentifier()));
CleanupParametersFromPath(currentNode.DeduplicatedSegment())?.ReplaceValueIdentifier()));
if (!string.IsNullOrEmpty(rawClassName) && string.IsNullOrEmpty(referenceName))
{
if (stripExtensionForIndexersTestRegex().IsMatch(rawClassName)) // {id}.json is considered as indexer
Expand Down Expand Up @@ -159,7 +159,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?.Segment.IsPathSegmentWithSingleSimpleParameter() ?? false;
currentNode?.DeduplicatedSegment().IsPathSegmentWithSingleSimpleParameter() ?? false;
private static bool IsPathSegmentWithSingleSimpleParameter(this string currentSegment)
{
if (string.IsNullOrEmpty(currentSegment)) return false;
Expand All @@ -180,7 +180,7 @@ private static bool IsPathSegmentWithNumberOfParameters(this string currentSegme
[GeneratedRegex(@"\{\w+\}\.(?:json|yaml|yml|csv|txt)$", RegexOptions.Singleline, 500)]
private static partial Regex stripExtensionForIndexersTestRegex(); // so {param-name}.json is considered as indexer
public static bool IsComplexPathMultipleParameters(this OpenApiUrlTreeNode currentNode) =>
(currentNode?.Segment?.IsPathSegmentWithNumberOfParameters(static x => x.Any()) ?? false) && !currentNode.IsPathSegmentWithSingleSimpleParameter();
(currentNode?.DeduplicatedSegment()?.IsPathSegmentWithNumberOfParameters(static x => x.Any()) ?? false) && !currentNode.IsPathSegmentWithSingleSimpleParameter();
public static string GetUrlTemplate(this OpenApiUrlTreeNode currentNode)
{
ArgumentNullException.ThrowIfNull(currentNode);
Expand Down Expand Up @@ -244,4 +244,18 @@ public static string SanitizeParameterNameForCodeSymbols(this string original, s
if (string.IsNullOrEmpty(original)) return original;
return removePctEncodedCharacters().Replace(original.ToCamelCase('-', '.', '~').SanitizeParameterNameForUrlTemplate(), replaceEncodedCharactersWith);
}
private const string DeduplicatedSegmentKey = "x-ms-kiota-deduplicatedSegment";
public static string DeduplicatedSegment(this OpenApiUrlTreeNode currentNode)
{
if (currentNode is null) return string.Empty;
if (currentNode.AdditionalData.TryGetValue(DeduplicatedSegmentKey, out var deduplicatedSegment) && deduplicatedSegment.FirstOrDefault() is string deduplicatedSegmentString)
return deduplicatedSegmentString;
return currentNode.Segment;
}
public static void AddDeduplicatedSegment(this OpenApiUrlTreeNode openApiUrlTreeNode, string newName)
{
ArgumentNullException.ThrowIfNull(openApiUrlTreeNode);
ArgumentException.ThrowIfNullOrEmpty(newName);
openApiUrlTreeNode.AdditionalData.Add(DeduplicatedSegmentKey, [newName]);
}
}
15 changes: 8 additions & 7 deletions src/Kiota.Builder/KiotaBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -552,6 +552,7 @@ private void MergeIndexNodesAtSameLevel(OpenApiUrlTreeNode node)
node.Children.Remove(indexNode.Key);
var newSegmentParameterName = $"{{{node.Segment.CleanupSymbolName()}-id}}";
indexNode.Value.Path = indexNode.Value.Path.Replace(indexNode.Key, newSegmentParameterName, StringComparison.OrdinalIgnoreCase);
indexNode.Value.AddDeduplicatedSegment(newSegmentParameterName);
node.Children.Add(newSegmentParameterName, indexNode.Value);
CopyNodeIntoOtherNode(indexNode.Value, indexNode.Value, indexNode.Key, newSegmentParameterName);
foreach (var child in indexNodes.Except(new[] { indexNode }))
Expand Down Expand Up @@ -727,7 +728,7 @@ private void CreateRequestBuilderClass(CodeNamespace currentNamespace, OpenApiUr

if (child.Value.IsPathSegmentWithSingleSimpleParameter())
{
var indexerParameterType = GetIndexerParameterType(child.Value, currentNode);
var indexerParameterType = GetIndexerParameter(child.Value, currentNode);
codeClass.AddIndexer(CreateIndexer($"{propIdentifier}-indexer", propType, indexerParameterType, child.Value, currentNode));
}
else if (child.Value.IsComplexPathMultipleParameters())
Expand Down Expand Up @@ -1076,7 +1077,7 @@ private IEnumerable<CodeType> GetUnmappedTypeDefinitions(CodeElement codeElement
}
private static CodeType DefaultIndexerParameterType => new() { Name = "string", IsExternal = true };
private const char OpenAPIUrlTreeNodePathSeparator = '\\';
private CodeParameter GetIndexerParameterType(OpenApiUrlTreeNode currentNode, OpenApiUrlTreeNode parentNode)
private CodeParameter GetIndexerParameter(OpenApiUrlTreeNode currentNode, OpenApiUrlTreeNode parentNode)
{
var parameterName = string.Join(OpenAPIUrlTreeNodePathSeparator, currentNode.Path.Split(OpenAPIUrlTreeNodePathSeparator, StringSplitOptions.RemoveEmptyEntries)
.Skip(parentNode.Path.Count(static x => x == OpenAPIUrlTreeNodePathSeparator)))
Expand All @@ -1095,11 +1096,12 @@ private CodeParameter GetIndexerParameterType(OpenApiUrlTreeNode currentNode, Op
_ => GetPrimitiveType(parameter.Schema),
} ?? DefaultIndexerParameterType;
type.IsNullable = false;
var segment = currentNode.DeduplicatedSegment();
var result = new CodeParameter
{
Type = type,
SerializationName = currentNode.Segment.SanitizeParameterNameForUrlTemplate(),
Name = currentNode.Segment.CleanupSymbolName(),
SerializationName = segment.SanitizeParameterNameForUrlTemplate(),
Name = segment.CleanupSymbolName(),
Documentation = new()
{
Description = parameter?.Description.CleanupDescription() is string description && !string.IsNullOrEmpty(description) ? description : "Unique identifier of the item",
Expand Down Expand Up @@ -1127,15 +1129,14 @@ private static IDictionary<string, OpenApiPathItem> GetPathItems(OpenApiUrlTreeN
private CodeIndexer[] CreateIndexer(string childIdentifier, string childType, CodeParameter parameter, OpenApiUrlTreeNode currentNode, OpenApiUrlTreeNode parentNode)
{
logger.LogTrace("Creating indexer {Name}", childIdentifier);
var result = new List<CodeIndexer> { new CodeIndexer
{
var result = new List<CodeIndexer> { new() {
Name = childIdentifier,
Documentation = new()
{
Description = currentNode.GetPathItemDescription(Constants.DefaultOpenApiLabel, $"Gets an item from the {currentNode.GetNodeNamespaceFromPath(config.ClientNamespaceName)} collection"),
},
ReturnType = new CodeType { Name = childType },
PathSegment = parentNode.GetNodeNamespaceFromPath(string.Empty).Split('.').Last(),
PathSegment = parentNode.GetNodeNamespaceFromPath(string.Empty).Split('.')[^1],
Deprecation = currentNode.GetDeprecationInformation(),
IndexParameter = parameter,
}};
Expand Down
3 changes: 2 additions & 1 deletion src/kiota/Handlers/KiotaShowCommandHandler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using System.Threading;
using System.Threading.Tasks;
using Kiota.Builder;
using Kiota.Builder.Extensions;
using Microsoft.Extensions.Logging;
using Microsoft.OpenApi.Services;

Expand Down Expand Up @@ -119,7 +120,7 @@ public override async Task<int> InvokeAsync(InvocationContext context)
private const string Space = " ";
private static void RenderNode(OpenApiUrlTreeNode node, uint maxDepth, StringBuilder builder, string indent = "", int nodeDepth = 0)
{
builder.AppendLine(node.Segment);
builder.AppendLine(node.DeduplicatedSegment());

var children = node.Children;
var numberOfChildren = children.Count;
Expand Down
3 changes: 2 additions & 1 deletion src/kiota/Rpc/Server.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
using System.Threading.Tasks;
using Kiota.Builder;
using Kiota.Builder.Configuration;
using Kiota.Builder.Extensions;
using Kiota.Builder.Lock;
using Kiota.Builder.Logging;
using Kiota.Generated;
Expand Down Expand Up @@ -218,7 +219,7 @@ private static PathItem ConvertOpenApiUrlTreeNodeToPathItem(OpenApiUrlTreeNode n
.OrderByDescending(static x => x.isOperation)
.ThenBy(static x => x.segment, StringComparer.OrdinalIgnoreCase)
.ToArray();
return new PathItem(node.Path, node.Segment, children, filteredPaths.Count == 0 || Array.Exists(children, static x => x.isOperation) && children.Where(static x => x.isOperation).All(static x => x.selected));
return new PathItem(node.Path, node.DeduplicatedSegment(), children, filteredPaths.Count == 0 || Array.Exists(children, static x => x.isOperation) && children.Where(static x => x.isOperation).All(static x => x.selected));
}
protected static string GetAbsolutePath(string source)
{
Expand Down
9 changes: 7 additions & 2 deletions tests/Kiota.Builder.Tests/KiotaBuilderTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -932,8 +932,8 @@ await File.WriteAllTextAsync(tempFilePath, @"openapi: 3.0.1
var builder = new KiotaBuilder(mockLogger.Object, new GenerationConfiguration { ClientClassName = "Graph", OpenAPIFilePath = tempFilePath }, _httpClient);
var treeNode = await builder.GetUrlTreeNodeAsync(new CancellationToken());
Assert.NotNull(treeNode);
Assert.Equal("/", treeNode.Segment);
Assert.Equal("enumeration", treeNode.Children.First().Value.Segment);
Assert.Equal("/", treeNode.DeduplicatedSegment());
Assert.Equal("enumeration", treeNode.Children.First().Value.DeduplicatedSegment());

_tempFiles.Add(tempFilePath);
}
Expand Down Expand Up @@ -6520,6 +6520,11 @@ public void SinglePathParametersAreDeduplicated()
var builder = new KiotaBuilder(mockLogger, new GenerationConfiguration { ClientClassName = "Graph", ApiRootUrl = "https://localhost" }, _httpClient);
var node = builder.CreateUriSpace(document);
var codeModel = builder.CreateSourceModel(node);
var usersRB = codeModel.FindNamespaceByName("ApiSdk.users").FindChildByName<CodeClass>("UsersRequestBuilder", false);
Assert.NotNull(usersRB);
var usersIndexer = usersRB.Indexer;
Assert.NotNull(usersIndexer);
Assert.Equal("users%2Did", usersIndexer.IndexParameter.SerializationName);
var managerRB = codeModel.FindNamespaceByName("ApiSdk.users.item.manager").FindChildByName<CodeClass>("ManagerRequestBuilder", false);
Assert.NotNull(managerRB);
var managerUrlTemplate = managerRB.FindChildByName<CodeProperty>("UrlTemplate", false);
Expand Down