Skip to content

Commit 4439340

Browse files
committed
fix: tree node has the wrong structure because of trailing slashes
Signed-off-by: Vincent Biret <[email protected]>
1 parent 9791eb6 commit 4439340

File tree

2 files changed

+75
-14
lines changed

2 files changed

+75
-14
lines changed

src/Microsoft.OpenApi/Services/OpenApiUrlTreeNode.cs

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ public bool HasOperations(string label)
5959
{
6060
Utils.CheckArgumentNullOrEmpty(label);
6161

62-
return PathItems is not null && PathItems.TryGetValue(label, out var item) && item.Operations is not null && item.Operations.Any();
62+
return PathItems is not null && PathItems.TryGetValue(label, out var item) && item.Operations is { Count : > 0 };
6363
}
6464

6565
/// <summary>
@@ -151,13 +151,6 @@ public OpenApiUrlTreeNode Attach(string path,
151151
}
152152

153153
var segments = path.Split('/');
154-
if (path.EndsWith("/", StringComparison.OrdinalIgnoreCase))
155-
{
156-
// Remove the last element, which is empty, and append the trailing slash to the new last element
157-
// This is to support URLs with trailing slashes
158-
Array.Resize(ref segments, segments.Length - 1);
159-
segments[segments.Length - 1] += @"\";
160-
}
161154

162155
return Attach(segments: segments,
163156
pathItem: pathItem,
@@ -179,7 +172,8 @@ private OpenApiUrlTreeNode Attach(IEnumerable<string> segments,
179172
string currentPath)
180173
{
181174
var segment = segments.FirstOrDefault();
182-
if (string.IsNullOrEmpty(segment))
175+
// empty segment is significant
176+
if (segment is null)
183177
{
184178
if (PathItems.ContainsKey(label))
185179
{
@@ -190,6 +184,13 @@ private OpenApiUrlTreeNode Attach(IEnumerable<string> segments,
190184
PathItems.Add(label, pathItem);
191185
return this;
192186
}
187+
// special casing for '/' (root node)
188+
if (segment.Length == 0 && currentPath.Length == 0)
189+
{
190+
Path = currentPath;
191+
PathItems.Add(label, pathItem);
192+
return this;
193+
}
193194

194195
// If the child segment has already been defined, then insert into it
195196
if (Children.TryGetValue(segment, out var child))

test/Microsoft.OpenApi.Tests/Services/OpenApiUrlTreeNodeTests.cs

Lines changed: 65 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -230,6 +230,66 @@ public void CreatePathsWithMultipleSegmentsWorks()
230230
Assert.Equal("coupes", rootNode.Children["cars"].Children["coupes"].Segment);
231231
}
232232

233+
[Fact]
234+
public void CreatePathsWithTrailingSlashWorks()
235+
{
236+
var doc = new OpenApiDocument
237+
{
238+
Paths = new()
239+
{
240+
["/"] = new OpenApiPathItem(),
241+
["/cars"] = new OpenApiPathItem(),
242+
["/cars/"] = new OpenApiPathItem(),
243+
["/cars/coupes"] = new OpenApiPathItem()
244+
}
245+
};
246+
247+
var label = "assets";
248+
var rootNode = OpenApiUrlTreeNode.Create(doc, label);
249+
250+
Assert.NotNull(rootNode);
251+
Assert.Single(rootNode.Children);
252+
var carsNode = rootNode.Children["cars"];
253+
Assert.Equal("cars", carsNode.Segment);
254+
Assert.Equal(2, carsNode.Children.Count);
255+
var emptyNode = carsNode.Children[""];
256+
Assert.Empty(emptyNode.Segment);
257+
Assert.Empty(emptyNode.Children);
258+
var coupesNode = carsNode.Children["coupes"];
259+
Assert.Equal("coupes", coupesNode.Segment);
260+
Assert.Empty(coupesNode.Children);
261+
}
262+
263+
[Fact]
264+
public void CreatePathsWithTrailingSlashWorksInverted()
265+
{
266+
var doc = new OpenApiDocument
267+
{
268+
Paths = new()
269+
{
270+
["/"] = new OpenApiPathItem(),
271+
["/cars/"] = new OpenApiPathItem(),
272+
["/cars"] = new OpenApiPathItem(), // only difference from previous test, tests that node mapping is not order specific
273+
["/cars/coupes"] = new OpenApiPathItem()
274+
}
275+
};
276+
277+
var label = "assets";
278+
var rootNode = OpenApiUrlTreeNode.Create(doc, label);
279+
280+
Assert.NotNull(rootNode);
281+
Assert.Single(rootNode.Children);
282+
var carsNode = rootNode.Children["cars"];
283+
Assert.Equal("cars", carsNode.Segment);
284+
Assert.Equal(2, carsNode.Children.Count);
285+
var emptyNode = carsNode.Children[""];
286+
Assert.Empty(emptyNode.Segment);
287+
Assert.Empty(emptyNode.Children);
288+
var coupesNode = carsNode.Children["coupes"];
289+
Assert.Equal("coupes", coupesNode.Segment);
290+
Assert.Empty(coupesNode.Children);
291+
}
292+
233293
[Fact]
234294
public void HasOperationsWorks()
235295
{
@@ -468,16 +528,16 @@ public async Task VerifyDiagramFromSampleOpenAPIAsync()
468528
await Verifier.Verify(diagram);
469529
}
470530

471-
public static TheoryData<string, string[], string, string> SupportsTrailingSlashesInPathData => new TheoryData<string, string[], string, string>
531+
public static TheoryData<string, string[], string> SupportsTrailingSlashesInPathData => new TheoryData<string, string[], string>
472532
{
473533
// Path, children up to second to leaf, last expected leaf node name, expected leaf node path
474-
{ "/cars/{car-id}/build/", ["cars", "{car-id}"], @"build\", @"\cars\{car-id}\build\" },
475-
{ "/cars/", [], @"cars\", @"\cars\" },
534+
{ "/cars/{car-id}/build/", ["cars", "{car-id}", "build"], @"\cars\{car-id}\build\" },
535+
{ "/cars/", ["cars"], @"\cars\" },
476536
};
477537

478538
[Theory]
479539
[MemberData(nameof(SupportsTrailingSlashesInPathData))]
480-
public void SupportsTrailingSlashesInPath(string path, string[] childrenBeforeLastNode, string expectedLeafNodeName, string expectedLeafNodePath)
540+
public void SupportsTrailingSlashesInPath(string path, string[] childrenBeforeLastNode, string expectedLeafNodePath)
481541
{
482542
var openApiDocument = new OpenApiDocument
483543
{
@@ -496,7 +556,7 @@ public void SupportsTrailingSlashesInPath(string path, string[] childrenBeforeLa
496556
secondToLeafNode = secondToLeafNode.Children[childName];
497557
}
498558

499-
Assert.True(secondToLeafNode.Children.TryGetValue(expectedLeafNodeName, out var leafNode));
559+
Assert.True(secondToLeafNode.Children.TryGetValue(string.Empty, out var leafNode));
500560
Assert.Equal(expectedLeafNodePath, leafNode.Path);
501561
Assert.Empty(leafNode.Children);
502562
}

0 commit comments

Comments
 (0)