diff --git a/src/Microsoft.OpenApi/Services/OpenApiUrlTreeNode.cs b/src/Microsoft.OpenApi/Services/OpenApiUrlTreeNode.cs index 5e4029c4d..0e4f5f359 100644 --- a/src/Microsoft.OpenApi/Services/OpenApiUrlTreeNode.cs +++ b/src/Microsoft.OpenApi/Services/OpenApiUrlTreeNode.cs @@ -59,7 +59,7 @@ public bool HasOperations(string label) { Utils.CheckArgumentNullOrEmpty(label); - return PathItems is not null && PathItems.TryGetValue(label, out var item) && item.Operations is not null && item.Operations.Any(); + return PathItems is not null && PathItems.TryGetValue(label, out var item) && item.Operations is { Count : > 0 }; } /// @@ -151,13 +151,6 @@ public OpenApiUrlTreeNode Attach(string path, } var segments = path.Split('/'); - if (path.EndsWith("/", StringComparison.OrdinalIgnoreCase)) - { - // Remove the last element, which is empty, and append the trailing slash to the new last element - // This is to support URLs with trailing slashes - Array.Resize(ref segments, segments.Length - 1); - segments[segments.Length - 1] += @"\"; - } return Attach(segments: segments, pathItem: pathItem, @@ -179,7 +172,8 @@ private OpenApiUrlTreeNode Attach(IEnumerable segments, string currentPath) { var segment = segments.FirstOrDefault(); - if (string.IsNullOrEmpty(segment)) + // empty segment is significant + if (segment is null) { if (PathItems.ContainsKey(label)) { @@ -190,6 +184,13 @@ private OpenApiUrlTreeNode Attach(IEnumerable segments, PathItems.Add(label, pathItem); return this; } + // special casing for '/' (root node) + if (segment.Length == 0 && currentPath.Length == 0) + { + Path = currentPath; + PathItems.Add(label, pathItem); + return this; + } // If the child segment has already been defined, then insert into it if (Children.TryGetValue(segment, out var child)) diff --git a/test/Microsoft.OpenApi.Tests/Services/OpenApiUrlTreeNodeTests.cs b/test/Microsoft.OpenApi.Tests/Services/OpenApiUrlTreeNodeTests.cs index 702b4ed12..52677751f 100644 --- a/test/Microsoft.OpenApi.Tests/Services/OpenApiUrlTreeNodeTests.cs +++ b/test/Microsoft.OpenApi.Tests/Services/OpenApiUrlTreeNodeTests.cs @@ -230,6 +230,66 @@ public void CreatePathsWithMultipleSegmentsWorks() Assert.Equal("coupes", rootNode.Children["cars"].Children["coupes"].Segment); } + [Fact] + public void CreatePathsWithTrailingSlashWorks() + { + var doc = new OpenApiDocument + { + Paths = new() + { + ["/"] = new OpenApiPathItem(), + ["/cars"] = new OpenApiPathItem(), + ["/cars/"] = new OpenApiPathItem(), + ["/cars/coupes"] = new OpenApiPathItem() + } + }; + + var label = "assets"; + var rootNode = OpenApiUrlTreeNode.Create(doc, label); + + Assert.NotNull(rootNode); + Assert.Single(rootNode.Children); + var carsNode = rootNode.Children["cars"]; + Assert.Equal("cars", carsNode.Segment); + Assert.Equal(2, carsNode.Children.Count); + var emptyNode = carsNode.Children[""]; + Assert.Empty(emptyNode.Segment); + Assert.Empty(emptyNode.Children); + var coupesNode = carsNode.Children["coupes"]; + Assert.Equal("coupes", coupesNode.Segment); + Assert.Empty(coupesNode.Children); + } + + [Fact] + public void CreatePathsWithTrailingSlashWorksInverted() + { + var doc = new OpenApiDocument + { + Paths = new() + { + ["/"] = new OpenApiPathItem(), + ["/cars/"] = new OpenApiPathItem(), + ["/cars"] = new OpenApiPathItem(), // only difference from previous test, tests that node mapping is not order specific + ["/cars/coupes"] = new OpenApiPathItem() + } + }; + + var label = "assets"; + var rootNode = OpenApiUrlTreeNode.Create(doc, label); + + Assert.NotNull(rootNode); + Assert.Single(rootNode.Children); + var carsNode = rootNode.Children["cars"]; + Assert.Equal("cars", carsNode.Segment); + Assert.Equal(2, carsNode.Children.Count); + var emptyNode = carsNode.Children[""]; + Assert.Empty(emptyNode.Segment); + Assert.Empty(emptyNode.Children); + var coupesNode = carsNode.Children["coupes"]; + Assert.Equal("coupes", coupesNode.Segment); + Assert.Empty(coupesNode.Children); + } + [Fact] public void HasOperationsWorks() { @@ -468,16 +528,16 @@ public async Task VerifyDiagramFromSampleOpenAPIAsync() await Verifier.Verify(diagram); } - public static TheoryData SupportsTrailingSlashesInPathData => new TheoryData + public static TheoryData SupportsTrailingSlashesInPathData => new TheoryData { // Path, children up to second to leaf, last expected leaf node name, expected leaf node path - { "/cars/{car-id}/build/", ["cars", "{car-id}"], @"build\", @"\cars\{car-id}\build\" }, - { "/cars/", [], @"cars\", @"\cars\" }, + { "/cars/{car-id}/build/", ["cars", "{car-id}", "build"], @"\cars\{car-id}\build\" }, + { "/cars/", ["cars"], @"\cars\" }, }; [Theory] [MemberData(nameof(SupportsTrailingSlashesInPathData))] - public void SupportsTrailingSlashesInPath(string path, string[] childrenBeforeLastNode, string expectedLeafNodeName, string expectedLeafNodePath) + public void SupportsTrailingSlashesInPath(string path, string[] childrenBeforeLastNode, string expectedLeafNodePath) { var openApiDocument = new OpenApiDocument { @@ -496,7 +556,7 @@ public void SupportsTrailingSlashesInPath(string path, string[] childrenBeforeLa secondToLeafNode = secondToLeafNode.Children[childName]; } - Assert.True(secondToLeafNode.Children.TryGetValue(expectedLeafNodeName, out var leafNode)); + Assert.True(secondToLeafNode.Children.TryGetValue(string.Empty, out var leafNode)); Assert.Equal(expectedLeafNodePath, leafNode.Path); Assert.Empty(leafNode.Children); }