From 93c468ebd9ee30b0cb32a583821d8abe3d017b18 Mon Sep 17 00:00:00 2001 From: Vincent Biret Date: Tue, 25 Feb 2025 14:31:48 -0500 Subject: [PATCH 1/6] feat: deduplicates tags at the document level Signed-off-by: Vincent Biret --- .../Models/OpenApiDocument.cs | 26 +++++++++- .../Models/References/OpenApiTagReference.cs | 2 +- src/Microsoft.OpenApi/OpenApiTagComparer.cs | 47 +++++++++++++++++++ .../Reader/V2/OpenApiDocumentDeserializer.cs | 2 +- .../Reader/V3/OpenApiDocumentDeserializer.cs | 3 +- .../Reader/V31/OpenApiDocumentDeserializer.cs | 3 +- .../Services/OpenApiVisitorBase.cs | 2 +- .../Services/OpenApiWalker.cs | 10 ++-- .../UtilityFiles/OpenApiDocumentMock.cs | 2 +- .../V3Tests/OpenApiDocumentTests.cs | 2 +- .../Models/OpenApiDocumentTests.cs | 39 ++++++++++++++- .../Models/OpenApiOperationTests.cs | 2 +- .../OpenApiTagComparerTests.cs | 40 ++++++++++++++++ .../PublicApi/PublicApi.approved.txt | 4 +- .../Visitors/InheritanceTests.cs | 4 +- .../Walkers/WalkerLocationTests.cs | 4 +- 16 files changed, 171 insertions(+), 21 deletions(-) create mode 100644 src/Microsoft.OpenApi/OpenApiTagComparer.cs create mode 100644 test/Microsoft.OpenApi.Tests/OpenApiTagComparerTests.cs diff --git a/src/Microsoft.OpenApi/Models/OpenApiDocument.cs b/src/Microsoft.OpenApi/Models/OpenApiDocument.cs index 7820a6f8e..35c78c78b 100644 --- a/src/Microsoft.OpenApi/Models/OpenApiDocument.cs +++ b/src/Microsoft.OpenApi/Models/OpenApiDocument.cs @@ -76,10 +76,32 @@ public void RegisterComponents() public IList? SecurityRequirements { get; set; } = new List(); + private HashSet? _tags; /// /// A list of tags used by the specification with additional metadata. /// - public IList? Tags { get; set; } = new List(); + public ISet? Tags + { + get + { + return _tags; + } + set + { + if (value is null) + { + return; + } + if (value is HashSet tags && tags.Comparer is OpenApiTagComparer) + { + _tags = tags; + } + else + { + _tags = new HashSet(value, OpenApiTagComparer.Instance); + } + } + } /// /// Additional external documentation. @@ -123,7 +145,7 @@ public OpenApiDocument(OpenApiDocument? document) Webhooks = document?.Webhooks != null ? new Dictionary(document.Webhooks) : null; Components = document?.Components != null ? new(document?.Components) : null; SecurityRequirements = document?.SecurityRequirements != null ? new List(document.SecurityRequirements) : null; - Tags = document?.Tags != null ? new List(document.Tags) : null; + Tags = document?.Tags != null ? new HashSet(document.Tags, OpenApiTagComparer.Instance) : null; ExternalDocs = document?.ExternalDocs != null ? new(document?.ExternalDocs) : null; Extensions = document?.Extensions != null ? new Dictionary(document.Extensions) : null; Annotations = document?.Annotations != null ? new Dictionary(document.Annotations) : null; diff --git a/src/Microsoft.OpenApi/Models/References/OpenApiTagReference.cs b/src/Microsoft.OpenApi/Models/References/OpenApiTagReference.cs index 6f218fc13..22b1c7a47 100644 --- a/src/Microsoft.OpenApi/Models/References/OpenApiTagReference.cs +++ b/src/Microsoft.OpenApi/Models/References/OpenApiTagReference.cs @@ -21,7 +21,7 @@ public override OpenApiTag Target { get { - return Reference.HostDocument?.Tags.FirstOrDefault(t => StringComparer.Ordinal.Equals(t.Name, Reference.Id)); + return Reference.HostDocument?.Tags.FirstOrDefault(t => OpenApiTagComparer.StringComparer.Equals(t.Name, Reference.Id)); } } diff --git a/src/Microsoft.OpenApi/OpenApiTagComparer.cs b/src/Microsoft.OpenApi/OpenApiTagComparer.cs new file mode 100644 index 000000000..543b57829 --- /dev/null +++ b/src/Microsoft.OpenApi/OpenApiTagComparer.cs @@ -0,0 +1,47 @@ +using System; +using System.Collections.Generic; +using Microsoft.OpenApi.Models; + +namespace Microsoft.OpenApi; + +#nullable enable +/// +/// This comparer is used to maintain a globally unique list of tags encountered +/// in a particular OpenAPI document. +/// +internal sealed class OpenApiTagComparer : IEqualityComparer +{ + private static readonly Lazy _lazyInstance = new(() => new OpenApiTagComparer()); + /// + /// Default instance for the comparer. + /// + internal static OpenApiTagComparer Instance { get => _lazyInstance.Value; } + + /// + public bool Equals(OpenApiTag? x, OpenApiTag? y) + { + if (x is null && y is null) + { + return true; + } + if (x is null || y is null) + { + return false; + } + if (ReferenceEquals(x, y)) + { + return true; + } + return StringComparer.Equals(x.Name, y.Name); + } + + // Tag comparisons are case-sensitive by default. Although the OpenAPI specification + // only outlines case sensitivity for property names, we extend this principle to + // property values for tag names as well. + // See https://spec.openapis.org/oas/v3.1.0#format. + internal static readonly StringComparer StringComparer = StringComparer.Ordinal; + + /// + public int GetHashCode(OpenApiTag obj) => obj?.Name is null ? 0 : StringComparer.GetHashCode(obj.Name); +} +#nullable restore diff --git a/src/Microsoft.OpenApi/Reader/V2/OpenApiDocumentDeserializer.cs b/src/Microsoft.OpenApi/Reader/V2/OpenApiDocumentDeserializer.cs index 0aa2b8093..81f1e2829 100644 --- a/src/Microsoft.OpenApi/Reader/V2/OpenApiDocumentDeserializer.cs +++ b/src/Microsoft.OpenApi/Reader/V2/OpenApiDocumentDeserializer.cs @@ -104,7 +104,7 @@ internal static partial class OpenApiV2Deserializer } }, {"security", (o, n, _) => o.SecurityRequirements = n.CreateList(LoadSecurityRequirement, o)}, - {"tags", (o, n, _) => o.Tags = n.CreateList(LoadTag, o)}, + {"tags", (o, n, _) => o.Tags = new HashSet(n.CreateList(LoadTag, o), OpenApiTagComparer.Instance)}, {"externalDocs", (o, n, _) => o.ExternalDocs = LoadExternalDocs(n, o)} }; diff --git a/src/Microsoft.OpenApi/Reader/V3/OpenApiDocumentDeserializer.cs b/src/Microsoft.OpenApi/Reader/V3/OpenApiDocumentDeserializer.cs index c07d28d46..ff18f758b 100644 --- a/src/Microsoft.OpenApi/Reader/V3/OpenApiDocumentDeserializer.cs +++ b/src/Microsoft.OpenApi/Reader/V3/OpenApiDocumentDeserializer.cs @@ -2,6 +2,7 @@ // Licensed under the MIT license. using System; +using System.Collections.Generic; using Microsoft.OpenApi.Extensions; using Microsoft.OpenApi.Models; using Microsoft.OpenApi.Reader.ParseNodes; @@ -26,7 +27,7 @@ internal static partial class OpenApiV3Deserializer {"servers", (o, n, _) => o.Servers = n.CreateList(LoadServer, o)}, {"paths", (o, n, _) => o.Paths = LoadPaths(n, o)}, {"components", (o, n, _) => o.Components = LoadComponents(n, o)}, - {"tags", (o, n, _) => o.Tags = n.CreateList(LoadTag, o) }, + {"tags", (o, n, _) => o.Tags = new HashSet(n.CreateList(LoadTag, o), OpenApiTagComparer.Instance) }, {"externalDocs", (o, n, _) => o.ExternalDocs = LoadExternalDocs(n, o)}, {"security", (o, n, _) => o.SecurityRequirements = n.CreateList(LoadSecurityRequirement, o)} }; diff --git a/src/Microsoft.OpenApi/Reader/V31/OpenApiDocumentDeserializer.cs b/src/Microsoft.OpenApi/Reader/V31/OpenApiDocumentDeserializer.cs index 4f3a05fcc..90ec89dce 100644 --- a/src/Microsoft.OpenApi/Reader/V31/OpenApiDocumentDeserializer.cs +++ b/src/Microsoft.OpenApi/Reader/V31/OpenApiDocumentDeserializer.cs @@ -1,4 +1,5 @@ using System; +using System.Collections.Generic; using Microsoft.OpenApi.Extensions; using Microsoft.OpenApi.Models; using Microsoft.OpenApi.Reader.ParseNodes; @@ -24,7 +25,7 @@ internal static partial class OpenApiV31Deserializer {"paths", (o, n, _) => o.Paths = LoadPaths(n, o)}, {"webhooks", (o, n, _) => o.Webhooks = n.CreateMap(LoadPathItem, o)}, {"components", (o, n, _) => o.Components = LoadComponents(n, o)}, - {"tags", (o, n, _) => o.Tags = n.CreateList(LoadTag, o) }, + {"tags", (o, n, _) => o.Tags = new HashSet(n.CreateList(LoadTag, o), OpenApiTagComparer.Instance) }, {"externalDocs", (o, n, _) => o.ExternalDocs = LoadExternalDocs(n, o)}, {"security", (o, n, _) => o.SecurityRequirements = n.CreateList(LoadSecurityRequirement, o)} }; diff --git a/src/Microsoft.OpenApi/Services/OpenApiVisitorBase.cs b/src/Microsoft.OpenApi/Services/OpenApiVisitorBase.cs index 254528b41..45c3e3a73 100644 --- a/src/Microsoft.OpenApi/Services/OpenApiVisitorBase.cs +++ b/src/Microsoft.OpenApi/Services/OpenApiVisitorBase.cs @@ -309,7 +309,7 @@ public virtual void Visit(IOpenApiExample example) /// /// Visits list of /// - public virtual void Visit(IList openApiTags) + public virtual void Visit(ISet openApiTags) { } diff --git a/src/Microsoft.OpenApi/Services/OpenApiWalker.cs b/src/Microsoft.OpenApi/Services/OpenApiWalker.cs index ae4430067..bf34d5668 100644 --- a/src/Microsoft.OpenApi/Services/OpenApiWalker.cs +++ b/src/Microsoft.OpenApi/Services/OpenApiWalker.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Linq; using System.Text.Json.Nodes; using Microsoft.OpenApi.Any; using Microsoft.OpenApi.Extensions; @@ -60,7 +61,7 @@ public void Walk(OpenApiDocument doc) /// /// Visits list of and child objects /// - internal void Walk(IList tags) + internal void Walk(ISet tags) { if (tags == null) { @@ -72,9 +73,10 @@ internal void Walk(IList tags) // Visit tags if (tags != null) { - for (var i = 0; i < tags.Count; i++) + var tagsAsArray = tags.ToArray(); + for (var i = 0; i < tagsAsArray.Length; i++) { - Walk(i.ToString(), () => Walk(tags[i])); + Walk(i.ToString(), () => Walk(tagsAsArray[i])); } } } @@ -1213,7 +1215,7 @@ internal void Walk(IOpenApiElement element) case OpenApiServer e: Walk(e); break; case OpenApiServerVariable e: Walk(e); break; case OpenApiTag e: Walk(e); break; - case IList e: Walk(e); break; + case ISet e: Walk(e); break; case IOpenApiExtensible e: Walk(e); break; case IOpenApiExtension e: Walk(e); break; } diff --git a/test/Microsoft.OpenApi.Hidi.Tests/UtilityFiles/OpenApiDocumentMock.cs b/test/Microsoft.OpenApi.Hidi.Tests/UtilityFiles/OpenApiDocumentMock.cs index 3f81c71a3..1bdcd2463 100644 --- a/test/Microsoft.OpenApi.Hidi.Tests/UtilityFiles/OpenApiDocumentMock.cs +++ b/test/Microsoft.OpenApi.Hidi.Tests/UtilityFiles/OpenApiDocumentMock.cs @@ -629,7 +629,7 @@ public static OpenApiDocument CreateOpenApiDocument() } } }, - Tags = new List + Tags = new HashSet { new() { diff --git a/test/Microsoft.OpenApi.Readers.Tests/V3Tests/OpenApiDocumentTests.cs b/test/Microsoft.OpenApi.Readers.Tests/V3Tests/OpenApiDocumentTests.cs index 83daf329d..79dca5e9b 100644 --- a/test/Microsoft.OpenApi.Readers.Tests/V3Tests/OpenApiDocumentTests.cs +++ b/test/Microsoft.OpenApi.Readers.Tests/V3Tests/OpenApiDocumentTests.cs @@ -1000,7 +1000,7 @@ public async Task ParseModifiedPetStoreDocumentWithTagAndSecurityShouldSucceed() } }, Components = components, - Tags = new List + Tags = new HashSet { new OpenApiTag { diff --git a/test/Microsoft.OpenApi.Tests/Models/OpenApiDocumentTests.cs b/test/Microsoft.OpenApi.Tests/Models/OpenApiDocumentTests.cs index 3716a0b32..35e52cc59 100644 --- a/test/Microsoft.OpenApi.Tests/Models/OpenApiDocumentTests.cs +++ b/test/Microsoft.OpenApi.Tests/Models/OpenApiDocumentTests.cs @@ -2078,7 +2078,7 @@ public async Task SerializeDocumentTagsWithMultipleExtensionsWorks() Version = "1.0.0" }, Paths = new OpenApiPaths(), - Tags = new List + Tags = new HashSet { new OpenApiTag { @@ -2102,5 +2102,42 @@ public async Task SerializeDocumentTagsWithMultipleExtensionsWorks() var actual = await doc.SerializeAsJsonAsync(OpenApiSpecVersion.OpenApi3_0); Assert.Equal(expected.MakeLineBreaksEnvironmentNeutral(), actual.MakeLineBreaksEnvironmentNeutral()); } + [Fact] + public void DeduplicatesTags() + { + var document = new OpenApiDocument + { + Tags = new HashSet + { + new OpenApiTag + { + Name = "tag1", + Extensions = new Dictionary + { + ["x-tag1"] = new OpenApiAny("tag1") + } + }, + new OpenApiTag + { + Name = "tag2", + Extensions = new Dictionary + { + ["x-tag2"] = new OpenApiAny("tag2") + } + }, + new OpenApiTag + { + Name = "tag1", + Extensions = new Dictionary + { + ["x-tag1"] = new OpenApiAny("tag1") + } + } + } + }; + Assert.Equal(2, document.Tags.Count); + Assert.Contains(document.Tags, t => t.Name == "tag1"); + Assert.Contains(document.Tags, t => t.Name == "tag2"); + } } } diff --git a/test/Microsoft.OpenApi.Tests/Models/OpenApiOperationTests.cs b/test/Microsoft.OpenApi.Tests/Models/OpenApiOperationTests.cs index af22284b9..8b5ab31f3 100644 --- a/test/Microsoft.OpenApi.Tests/Models/OpenApiOperationTests.cs +++ b/test/Microsoft.OpenApi.Tests/Models/OpenApiOperationTests.cs @@ -91,7 +91,7 @@ public class OpenApiOperationTests { Tags = new List { - new OpenApiTagReference("tagId1", new OpenApiDocument{ Tags = new List() { new OpenApiTag{Name = "tagId1"}} }) + new OpenApiTagReference("tagId1", new OpenApiDocument{ Tags = new HashSet() { new OpenApiTag{Name = "tagId1"}} }) }, Summary = "summary1", Description = "operationDescription", diff --git a/test/Microsoft.OpenApi.Tests/OpenApiTagComparerTests.cs b/test/Microsoft.OpenApi.Tests/OpenApiTagComparerTests.cs new file mode 100644 index 000000000..9ea0c498c --- /dev/null +++ b/test/Microsoft.OpenApi.Tests/OpenApiTagComparerTests.cs @@ -0,0 +1,40 @@ +using Microsoft.OpenApi.Models; +using Xunit; + +namespace Microsoft.OpenApi.Tests; + +public class OpenApiTagComparerTests +{ + private readonly OpenApiTagComparer _comparer = OpenApiTagComparer.Instance; + [Fact] + public void Defensive() + { + Assert.NotNull(_comparer); + + Assert.True(_comparer.Equals(null, null)); + Assert.False(_comparer.Equals(null, new OpenApiTag())); + Assert.Equal(0, _comparer.GetHashCode(null)); + Assert.Equal(0, _comparer.GetHashCode(new OpenApiTag())); + } + [Fact] + public void SameNamesAreEqual() + { + var openApiTag1 = new OpenApiTag { Name = "tag" }; + var openApiTag2 = new OpenApiTag { Name = "tag" }; + Assert.True(_comparer.Equals(openApiTag1, openApiTag2)); + } + [Fact] + public void SameInstanceAreEqual() + { + var openApiTag = new OpenApiTag { Name = "tag" }; + Assert.True(_comparer.Equals(openApiTag, openApiTag)); + } + + [Fact] + public void DifferentCasingAreNotEquals() + { + var openApiTag1 = new OpenApiTag { Name = "tag" }; + var openApiTag2 = new OpenApiTag { Name = "TAG" }; + Assert.False(_comparer.Equals(openApiTag1, openApiTag2)); + } +} diff --git a/test/Microsoft.OpenApi.Tests/PublicApi/PublicApi.approved.txt b/test/Microsoft.OpenApi.Tests/PublicApi/PublicApi.approved.txt index ca8c30327..9a03b8a62 100644 --- a/test/Microsoft.OpenApi.Tests/PublicApi/PublicApi.approved.txt +++ b/test/Microsoft.OpenApi.Tests/PublicApi/PublicApi.approved.txt @@ -722,7 +722,7 @@ namespace Microsoft.OpenApi.Models public Microsoft.OpenApi.Models.OpenApiPaths Paths { get; set; } public System.Collections.Generic.IList? SecurityRequirements { get; set; } public System.Collections.Generic.IList? Servers { get; set; } - public System.Collections.Generic.IList? Tags { get; set; } + public System.Collections.Generic.ISet? Tags { get; set; } public System.Collections.Generic.IDictionary? Webhooks { get; set; } public Microsoft.OpenApi.Services.OpenApiWorkspace? Workspace { get; set; } public bool AddComponent(string id, T componentToRegister) { } @@ -1664,8 +1664,8 @@ namespace Microsoft.OpenApi.Services public virtual void Visit(System.Collections.Generic.IList parameters) { } public virtual void Visit(System.Collections.Generic.IList openApiSecurityRequirements) { } public virtual void Visit(System.Collections.Generic.IList servers) { } - public virtual void Visit(System.Collections.Generic.IList openApiTags) { } public virtual void Visit(System.Collections.Generic.IList openApiTags) { } + public virtual void Visit(System.Collections.Generic.ISet openApiTags) { } public virtual void Visit(System.Text.Json.Nodes.JsonNode node) { } } public class OpenApiWalker diff --git a/test/Microsoft.OpenApi.Tests/Visitors/InheritanceTests.cs b/test/Microsoft.OpenApi.Tests/Visitors/InheritanceTests.cs index 581f2998a..44febe633 100644 --- a/test/Microsoft.OpenApi.Tests/Visitors/InheritanceTests.cs +++ b/test/Microsoft.OpenApi.Tests/Visitors/InheritanceTests.cs @@ -53,7 +53,7 @@ public void ExpectedVirtualsInvolved() visitor.Visit(default(OpenApiSecurityRequirement)); visitor.Visit(default(IOpenApiSecurityScheme)); visitor.Visit(default(IOpenApiExample)); - visitor.Visit(default(IList)); + visitor.Visit(default(ISet)); visitor.Visit(default(IList)); visitor.Visit(default(IOpenApiExtensible)); visitor.Visit(default(IOpenApiExtension)); @@ -292,7 +292,7 @@ public override void Visit(IOpenApiExample example) base.Visit(example); } - public override void Visit(IList openApiTags) + public override void Visit(ISet openApiTags) { EncodeCall(); base.Visit(openApiTags); diff --git a/test/Microsoft.OpenApi.Tests/Walkers/WalkerLocationTests.cs b/test/Microsoft.OpenApi.Tests/Walkers/WalkerLocationTests.cs index ce45186c6..cd6ea989a 100644 --- a/test/Microsoft.OpenApi.Tests/Walkers/WalkerLocationTests.cs +++ b/test/Microsoft.OpenApi.Tests/Walkers/WalkerLocationTests.cs @@ -42,7 +42,7 @@ public void LocateTopLevelArrayItems() new(), new() }, - Tags = new List + Tags = new HashSet { new() } @@ -305,7 +305,7 @@ public override void Visit(IOpenApiSchema schema) Locations.Add(this.PathString); } - public override void Visit(IList openApiTags) + public override void Visit(ISet openApiTags) { Locations.Add(this.PathString); } From 96e071f7ea9df5b7820d4461637cf7f40eabc5c3 Mon Sep 17 00:00:00 2001 From: Vincent Biret Date: Tue, 25 Feb 2025 14:39:47 -0500 Subject: [PATCH 2/6] chore: removes extraneous tags locations Signed-off-by: Vincent Biret --- test/Microsoft.OpenApi.Tests/Walkers/WalkerLocationTests.cs | 3 --- 1 file changed, 3 deletions(-) diff --git a/test/Microsoft.OpenApi.Tests/Walkers/WalkerLocationTests.cs b/test/Microsoft.OpenApi.Tests/Walkers/WalkerLocationTests.cs index cd6ea989a..ad8d91b23 100644 --- a/test/Microsoft.OpenApi.Tests/Walkers/WalkerLocationTests.cs +++ b/test/Microsoft.OpenApi.Tests/Walkers/WalkerLocationTests.cs @@ -28,7 +28,6 @@ public void LocateTopLevelObjects() "#/info", "#/servers", "#/paths", - "#/tags" }, locator.Locations); } @@ -109,7 +108,6 @@ public void LocatePathOperationContentSchema() "#/paths/~1test/get/responses/200/content/application~1json", "#/paths/~1test/get/responses/200/content/application~1json/schema", "#/paths/~1test/get/tags", - "#/tags", }, locator.Locations); @@ -152,7 +150,6 @@ public void WalkDOMWithCycles() "#/components", "#/components/schemas/loopy", "#/components/schemas/loopy/properties/name", - "#/tags" }, locator.Locations); } From 512f75c326b1f647ccfb449aa937681d64e8148e Mon Sep 17 00:00:00 2001 From: Vincent Biret Date: Tue, 25 Feb 2025 14:40:02 -0500 Subject: [PATCH 3/6] chore: fixes unit test setup unable to find implementation Signed-off-by: Vincent Biret --- .../V3Tests/OpenApiOperationTests.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/Microsoft.OpenApi.Readers.Tests/V3Tests/OpenApiOperationTests.cs b/test/Microsoft.OpenApi.Readers.Tests/V3Tests/OpenApiOperationTests.cs index 1dd24a128..f43a86d00 100644 --- a/test/Microsoft.OpenApi.Readers.Tests/V3Tests/OpenApiOperationTests.cs +++ b/test/Microsoft.OpenApi.Readers.Tests/V3Tests/OpenApiOperationTests.cs @@ -1,6 +1,7 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT license. +using System.Collections.Generic; using System.IO; using System.Linq; using System.Threading.Tasks; @@ -35,7 +36,7 @@ public async Task ParseOperationWithParameterWithNoLocationShouldSucceed() { var openApiDocument = new OpenApiDocument { - Tags = { new OpenApiTag() { Name = "user" } } + Tags = new HashSet { new() { Name = "user" } } }; // Act var operation = await OpenApiModelFactory.LoadAsync(Path.Combine(SampleFolderPath, "operationWithParameterWithNoLocation.json"), OpenApiSpecVersion.OpenApi3_0, openApiDocument); From 7b7be4af0cbaf18f12f6569bf6854df78b8b3a9b Mon Sep 17 00:00:00 2001 From: Vincent Biret Date: Tue, 25 Feb 2025 14:40:20 -0500 Subject: [PATCH 4/6] chore; fixes parsing to avoid unnecessary allocations Signed-off-by: Vincent Biret --- src/Microsoft.OpenApi/Reader/V2/OpenApiDocumentDeserializer.cs | 2 +- src/Microsoft.OpenApi/Reader/V3/OpenApiDocumentDeserializer.cs | 2 +- src/Microsoft.OpenApi/Reader/V31/OpenApiDocumentDeserializer.cs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Microsoft.OpenApi/Reader/V2/OpenApiDocumentDeserializer.cs b/src/Microsoft.OpenApi/Reader/V2/OpenApiDocumentDeserializer.cs index 81f1e2829..7e13578f3 100644 --- a/src/Microsoft.OpenApi/Reader/V2/OpenApiDocumentDeserializer.cs +++ b/src/Microsoft.OpenApi/Reader/V2/OpenApiDocumentDeserializer.cs @@ -104,7 +104,7 @@ internal static partial class OpenApiV2Deserializer } }, {"security", (o, n, _) => o.SecurityRequirements = n.CreateList(LoadSecurityRequirement, o)}, - {"tags", (o, n, _) => o.Tags = new HashSet(n.CreateList(LoadTag, o), OpenApiTagComparer.Instance)}, + {"tags", (o, n, _) => { if (n.CreateList(LoadTag, o) is {Count:> 0} tags) {o.Tags = new HashSet(tags, OpenApiTagComparer.Instance); } } }, {"externalDocs", (o, n, _) => o.ExternalDocs = LoadExternalDocs(n, o)} }; diff --git a/src/Microsoft.OpenApi/Reader/V3/OpenApiDocumentDeserializer.cs b/src/Microsoft.OpenApi/Reader/V3/OpenApiDocumentDeserializer.cs index ff18f758b..f6ca536c4 100644 --- a/src/Microsoft.OpenApi/Reader/V3/OpenApiDocumentDeserializer.cs +++ b/src/Microsoft.OpenApi/Reader/V3/OpenApiDocumentDeserializer.cs @@ -27,7 +27,7 @@ internal static partial class OpenApiV3Deserializer {"servers", (o, n, _) => o.Servers = n.CreateList(LoadServer, o)}, {"paths", (o, n, _) => o.Paths = LoadPaths(n, o)}, {"components", (o, n, _) => o.Components = LoadComponents(n, o)}, - {"tags", (o, n, _) => o.Tags = new HashSet(n.CreateList(LoadTag, o), OpenApiTagComparer.Instance) }, + {"tags", (o, n, _) => { if (n.CreateList(LoadTag, o) is {Count:> 0} tags) {o.Tags = new HashSet(tags, OpenApiTagComparer.Instance); } } }, {"externalDocs", (o, n, _) => o.ExternalDocs = LoadExternalDocs(n, o)}, {"security", (o, n, _) => o.SecurityRequirements = n.CreateList(LoadSecurityRequirement, o)} }; diff --git a/src/Microsoft.OpenApi/Reader/V31/OpenApiDocumentDeserializer.cs b/src/Microsoft.OpenApi/Reader/V31/OpenApiDocumentDeserializer.cs index 90ec89dce..f16ac31cc 100644 --- a/src/Microsoft.OpenApi/Reader/V31/OpenApiDocumentDeserializer.cs +++ b/src/Microsoft.OpenApi/Reader/V31/OpenApiDocumentDeserializer.cs @@ -25,7 +25,7 @@ internal static partial class OpenApiV31Deserializer {"paths", (o, n, _) => o.Paths = LoadPaths(n, o)}, {"webhooks", (o, n, _) => o.Webhooks = n.CreateMap(LoadPathItem, o)}, {"components", (o, n, _) => o.Components = LoadComponents(n, o)}, - {"tags", (o, n, _) => o.Tags = new HashSet(n.CreateList(LoadTag, o), OpenApiTagComparer.Instance) }, + {"tags", (o, n, _) => { if (n.CreateList(LoadTag, o) is {Count:> 0} tags) {o.Tags = new HashSet(tags, OpenApiTagComparer.Instance); } } }, {"externalDocs", (o, n, _) => o.ExternalDocs = LoadExternalDocs(n, o)}, {"security", (o, n, _) => o.SecurityRequirements = n.CreateList(LoadSecurityRequirement, o)} }; From 763c0c1c5856a0ed56128b0ab8ce4b3a29ed193a Mon Sep 17 00:00:00 2001 From: Vincent Biret Date: Tue, 25 Feb 2025 15:09:57 -0500 Subject: [PATCH 5/6] feat: tags references are now deduplicated as well Signed-off-by: Vincent Biret --- .../Models/OpenApiOperation.cs | 26 +++++++++- .../Models/References/OpenApiTagReference.cs | 2 +- src/Microsoft.OpenApi/OpenApiTagComparer.cs | 8 +-- .../Reader/V2/OpenApiOperationDeserializer.cs | 10 ++-- .../Reader/V3/OpenApiOperationDeserializer.cs | 11 ++-- .../V31/OpenApiOperationDeserializer.cs | 10 ++-- .../Services/OpenApiFilterService.cs | 4 +- .../Services/OpenApiVisitorBase.cs | 2 +- .../Services/OpenApiWalker.cs | 7 +-- .../UtilityFiles/OpenApiDocumentMock.cs | 24 ++++----- .../V2Tests/OpenApiOperationTests.cs | 43 +++++++++++++++ .../V3Tests/OpenApiDocumentTests.cs | 15 ++---- .../V3Tests/OpenApiOperationTests.cs | 52 +++++++++++++++++-- .../Models/OpenApiOperationTests.cs | 2 +- .../PublicApi/PublicApi.approved.txt | 4 +- .../Walkers/WalkerLocationTests.cs | 3 +- 16 files changed, 166 insertions(+), 57 deletions(-) diff --git a/src/Microsoft.OpenApi/Models/OpenApiOperation.cs b/src/Microsoft.OpenApi/Models/OpenApiOperation.cs index 3acbd05ab..0ad61ec27 100644 --- a/src/Microsoft.OpenApi/Models/OpenApiOperation.cs +++ b/src/Microsoft.OpenApi/Models/OpenApiOperation.cs @@ -23,11 +23,33 @@ public class OpenApiOperation : IOpenApiSerializable, IOpenApiExtensible, IOpenA /// public const bool DeprecatedDefault = false; + private HashSet? _tags; /// /// A list of tags for API documentation control. /// Tags can be used for logical grouping of operations by resources or any other qualifier. /// - public IList? Tags { get; set; } = []; + public ISet? Tags + { + get + { + return _tags; + } + set + { + if (value is null) + { + return; + } + if (value is HashSet tags && tags.Comparer is OpenApiTagComparer) + { + _tags = tags; + } + else + { + _tags = new HashSet(value, OpenApiTagComparer.Instance); + } + } + } /// /// A short summary of what the operation does. @@ -123,7 +145,7 @@ public OpenApiOperation() { } public OpenApiOperation(OpenApiOperation operation) { Utils.CheckArgumentNull(operation); - Tags = operation.Tags != null ? new List(operation.Tags) : null; + Tags = operation.Tags != null ? new HashSet(operation.Tags) : null; Summary = operation.Summary ?? Summary; Description = operation.Description ?? Description; ExternalDocs = operation.ExternalDocs != null ? new(operation.ExternalDocs) : null; diff --git a/src/Microsoft.OpenApi/Models/References/OpenApiTagReference.cs b/src/Microsoft.OpenApi/Models/References/OpenApiTagReference.cs index 22b1c7a47..019d4c367 100644 --- a/src/Microsoft.OpenApi/Models/References/OpenApiTagReference.cs +++ b/src/Microsoft.OpenApi/Models/References/OpenApiTagReference.cs @@ -21,7 +21,7 @@ public override OpenApiTag Target { get { - return Reference.HostDocument?.Tags.FirstOrDefault(t => OpenApiTagComparer.StringComparer.Equals(t.Name, Reference.Id)); + return Reference.HostDocument?.Tags?.FirstOrDefault(t => OpenApiTagComparer.StringComparer.Equals(t.Name, Reference.Id)); } } diff --git a/src/Microsoft.OpenApi/OpenApiTagComparer.cs b/src/Microsoft.OpenApi/OpenApiTagComparer.cs index 543b57829..6652dd5ba 100644 --- a/src/Microsoft.OpenApi/OpenApiTagComparer.cs +++ b/src/Microsoft.OpenApi/OpenApiTagComparer.cs @@ -1,6 +1,6 @@ using System; using System.Collections.Generic; -using Microsoft.OpenApi.Models; +using Microsoft.OpenApi.Models.Interfaces; namespace Microsoft.OpenApi; @@ -9,7 +9,7 @@ namespace Microsoft.OpenApi; /// This comparer is used to maintain a globally unique list of tags encountered /// in a particular OpenAPI document. /// -internal sealed class OpenApiTagComparer : IEqualityComparer +internal sealed class OpenApiTagComparer : IEqualityComparer { private static readonly Lazy _lazyInstance = new(() => new OpenApiTagComparer()); /// @@ -18,7 +18,7 @@ internal sealed class OpenApiTagComparer : IEqualityComparer internal static OpenApiTagComparer Instance { get => _lazyInstance.Value; } /// - public bool Equals(OpenApiTag? x, OpenApiTag? y) + public bool Equals(IOpenApiTag? x, IOpenApiTag? y) { if (x is null && y is null) { @@ -42,6 +42,6 @@ public bool Equals(OpenApiTag? x, OpenApiTag? y) internal static readonly StringComparer StringComparer = StringComparer.Ordinal; /// - public int GetHashCode(OpenApiTag obj) => obj?.Name is null ? 0 : StringComparer.GetHashCode(obj.Name); + public int GetHashCode(IOpenApiTag obj) => string.IsNullOrEmpty(obj?.Name) ? 0 : StringComparer.GetHashCode(obj!.Name); } #nullable restore diff --git a/src/Microsoft.OpenApi/Reader/V2/OpenApiOperationDeserializer.cs b/src/Microsoft.OpenApi/Reader/V2/OpenApiOperationDeserializer.cs index 37c95c9b2..c2f6ca204 100644 --- a/src/Microsoft.OpenApi/Reader/V2/OpenApiOperationDeserializer.cs +++ b/src/Microsoft.OpenApi/Reader/V2/OpenApiOperationDeserializer.cs @@ -23,10 +23,12 @@ internal static partial class OpenApiV2Deserializer new() { { - "tags", (o, n, doc) => o.Tags = n.CreateSimpleList( - (valueNode, doc) => - LoadTagByReference( - valueNode.GetScalarValue(), doc), doc) + "tags", (o, n, doc) => { + if (n.CreateSimpleList((valueNode, doc) => LoadTagByReference(valueNode.GetScalarValue(), doc), doc) is {Count: > 0} tags) + { + o.Tags = new HashSet(tags, OpenApiTagComparer.Instance); + } + } }, { "summary", diff --git a/src/Microsoft.OpenApi/Reader/V3/OpenApiOperationDeserializer.cs b/src/Microsoft.OpenApi/Reader/V3/OpenApiOperationDeserializer.cs index e9712da98..9fca4d14b 100644 --- a/src/Microsoft.OpenApi/Reader/V3/OpenApiOperationDeserializer.cs +++ b/src/Microsoft.OpenApi/Reader/V3/OpenApiOperationDeserializer.cs @@ -2,6 +2,7 @@ // Licensed under the MIT license. using System; +using System.Collections.Generic; using Microsoft.OpenApi.Extensions; using Microsoft.OpenApi.Models; using Microsoft.OpenApi.Models.References; @@ -19,10 +20,12 @@ internal static partial class OpenApiV3Deserializer new() { { - "tags", (o, n, doc) => o.Tags = n.CreateSimpleList( - (valueNode, doc) => - LoadTagByReference( - valueNode.GetScalarValue(), doc), doc) + "tags", (o, n, doc) => { + if (n.CreateSimpleList((valueNode, doc) => LoadTagByReference(valueNode.GetScalarValue(), doc), doc) is {Count: > 0} tags) + { + o.Tags = new HashSet(tags, OpenApiTagComparer.Instance); + } + } }, { "summary", diff --git a/src/Microsoft.OpenApi/Reader/V31/OpenApiOperationDeserializer.cs b/src/Microsoft.OpenApi/Reader/V31/OpenApiOperationDeserializer.cs index cb44bb438..d969cca36 100644 --- a/src/Microsoft.OpenApi/Reader/V31/OpenApiOperationDeserializer.cs +++ b/src/Microsoft.OpenApi/Reader/V31/OpenApiOperationDeserializer.cs @@ -1,4 +1,5 @@ using System; +using System.Collections.Generic; using Microsoft.OpenApi.Extensions; using Microsoft.OpenApi.Models; using Microsoft.OpenApi.Models.References; @@ -16,9 +17,12 @@ internal static partial class OpenApiV31Deserializer new() { { - "tags", (o, n, doc) => o.Tags = n.CreateSimpleList( - (valueNode, doc) => - LoadTagByReference(valueNode.GetScalarValue(), doc), doc) + "tags", (o, n, doc) => { + if (n.CreateSimpleList((valueNode, doc) => LoadTagByReference(valueNode.GetScalarValue(), doc), doc) is {Count: > 0} tags) + { + o.Tags = new HashSet(tags, OpenApiTagComparer.Instance); + } + } }, { "summary", (o, n, _) => diff --git a/src/Microsoft.OpenApi/Services/OpenApiFilterService.cs b/src/Microsoft.OpenApi/Services/OpenApiFilterService.cs index 20fa54839..5f439fe94 100644 --- a/src/Microsoft.OpenApi/Services/OpenApiFilterService.cs +++ b/src/Microsoft.OpenApi/Services/OpenApiFilterService.cs @@ -363,11 +363,11 @@ private static void ValidateFilters(IDictionary> requestUrl if (tagsArray.Length == 1) { var regex = new Regex(tagsArray[0]); - return (_, _, operation) => operation.Tags.Any(tag => regex.IsMatch(tag.Name)); + return (_, _, operation) => operation.Tags?.Any(tag => regex.IsMatch(tag.Name)) ?? false; } else { - return (_, _, operation) => operation.Tags.Any(tag => tagsArray.Contains(tag.Name)); + return (_, _, operation) => operation.Tags?.Any(tag => tagsArray.Contains(tag.Name)) ?? false; } } diff --git a/src/Microsoft.OpenApi/Services/OpenApiVisitorBase.cs b/src/Microsoft.OpenApi/Services/OpenApiVisitorBase.cs index 45c3e3a73..e4420c3c3 100644 --- a/src/Microsoft.OpenApi/Services/OpenApiVisitorBase.cs +++ b/src/Microsoft.OpenApi/Services/OpenApiVisitorBase.cs @@ -316,7 +316,7 @@ public virtual void Visit(ISet openApiTags) /// /// Visits list of /// - public virtual void Visit(IList openApiTags) + public virtual void Visit(ISet openApiTags) { } diff --git a/src/Microsoft.OpenApi/Services/OpenApiWalker.cs b/src/Microsoft.OpenApi/Services/OpenApiWalker.cs index bf34d5668..68e3133d6 100644 --- a/src/Microsoft.OpenApi/Services/OpenApiWalker.cs +++ b/src/Microsoft.OpenApi/Services/OpenApiWalker.cs @@ -84,7 +84,7 @@ internal void Walk(ISet tags) /// /// Visits list of and child objects /// - internal void Walk(IList tags) + internal void Walk(ISet tags) { if (tags == null) { @@ -96,9 +96,10 @@ internal void Walk(IList tags) // Visit tags if (tags != null) { - for (var i = 0; i < tags.Count; i++) + var referencesAsArray = tags.ToArray(); + for (var i = 0; i < referencesAsArray.Length; i++) { - Walk(i.ToString(), () => Walk(tags[i])); + Walk(i.ToString(), () => Walk(referencesAsArray[i])); } } } diff --git a/test/Microsoft.OpenApi.Hidi.Tests/UtilityFiles/OpenApiDocumentMock.cs b/test/Microsoft.OpenApi.Hidi.Tests/UtilityFiles/OpenApiDocumentMock.cs index 1bdcd2463..0da220427 100644 --- a/test/Microsoft.OpenApi.Hidi.Tests/UtilityFiles/OpenApiDocumentMock.cs +++ b/test/Microsoft.OpenApi.Hidi.Tests/UtilityFiles/OpenApiDocumentMock.cs @@ -678,18 +678,18 @@ public static OpenApiDocument CreateOpenApiDocument() } } }; - document.Paths[getTeamsActivityByPeriodPath].Operations[OperationType.Get].Tags!.Add(new OpenApiTagReference("reports.Functions", document)); - document.Paths[getTeamsActivityByDatePath].Operations[OperationType.Get].Tags!.Add(new OpenApiTagReference("reports.Functions", document)); - document.Paths[usersPath].Operations[OperationType.Get].Tags!.Add(new OpenApiTagReference("users.user", document)); - document.Paths[usersByIdPath].Operations[OperationType.Get].Tags!.Add(new OpenApiTagReference("users.user", document)); - document.Paths[usersByIdPath].Operations[OperationType.Patch].Tags!.Add(new OpenApiTagReference("users.user", document)); - document.Paths[messagesByIdPath].Operations[OperationType.Get].Tags!.Add(new OpenApiTagReference("users.message", document)); - document.Paths[administrativeUnitRestorePath].Operations[OperationType.Post].Tags!.Add(new OpenApiTagReference("administrativeUnits.Actions", document)); - document.Paths[logoPath].Operations[OperationType.Put].Tags!.Add(new OpenApiTagReference("applications.application", document)); - document.Paths[securityProfilesPath].Operations[OperationType.Get].Tags!.Add(new OpenApiTagReference("security.hostSecurityProfile", document)); - document.Paths[communicationsCallsKeepAlivePath].Operations[OperationType.Post].Tags!.Add(new OpenApiTagReference("communications.Actions", document)); - document.Paths[eventsDeltaPath].Operations[OperationType.Get].Tags!.Add(new OpenApiTagReference("groups.Functions", document)); - document.Paths[refPath].Operations[OperationType.Get].Tags!.Add(new OpenApiTagReference("applications.directoryObject", document)); + document.Paths[getTeamsActivityByPeriodPath].Operations[OperationType.Get].Tags = new HashSet {new OpenApiTagReference("reports.Functions", document)}; + document.Paths[getTeamsActivityByDatePath].Operations[OperationType.Get].Tags = new HashSet {new OpenApiTagReference("reports.Functions", document)}; + document.Paths[usersPath].Operations[OperationType.Get].Tags = new HashSet {new OpenApiTagReference("users.user", document)}; + document.Paths[usersByIdPath].Operations[OperationType.Get].Tags = new HashSet {new OpenApiTagReference("users.user", document)}; + document.Paths[usersByIdPath].Operations[OperationType.Patch].Tags = new HashSet {new OpenApiTagReference("users.user", document)}; + document.Paths[messagesByIdPath].Operations[OperationType.Get].Tags = new HashSet {new OpenApiTagReference("users.message", document)}; + document.Paths[administrativeUnitRestorePath].Operations[OperationType.Post].Tags = new HashSet {new OpenApiTagReference("administrativeUnits.Actions", document)}; + document.Paths[logoPath].Operations[OperationType.Put].Tags = new HashSet {new OpenApiTagReference("applications.application", document)}; + document.Paths[securityProfilesPath].Operations[OperationType.Get].Tags = new HashSet {new OpenApiTagReference("security.hostSecurityProfile", document)}; + document.Paths[communicationsCallsKeepAlivePath].Operations[OperationType.Post].Tags = new HashSet {new OpenApiTagReference("communications.Actions", document)}; + document.Paths[eventsDeltaPath].Operations[OperationType.Get].Tags = new HashSet {new OpenApiTagReference("groups.Functions", document)}; + document.Paths[refPath].Operations[OperationType.Get].Tags = new HashSet {new OpenApiTagReference("applications.directoryObject", document)}; ((OpenApiSchema)document.Paths[usersPath].Operations[OperationType.Get].Responses!["200"].Content[applicationJsonMediaType].Schema!.Properties["value"]).Items = new OpenApiSchemaReference("microsoft.graph.user", document); document.Paths[usersByIdPath].Operations[OperationType.Get].Responses!["200"].Content[applicationJsonMediaType].Schema = new OpenApiSchemaReference("microsoft.graph.user", document); document.Paths[messagesByIdPath].Operations[OperationType.Get].Responses!["200"].Content[applicationJsonMediaType].Schema = new OpenApiSchemaReference("microsoft.graph.message", document); diff --git a/test/Microsoft.OpenApi.Readers.Tests/V2Tests/OpenApiOperationTests.cs b/test/Microsoft.OpenApi.Readers.Tests/V2Tests/OpenApiOperationTests.cs index 13339332a..00c4faaeb 100644 --- a/test/Microsoft.OpenApi.Readers.Tests/V2Tests/OpenApiOperationTests.cs +++ b/test/Microsoft.OpenApi.Readers.Tests/V2Tests/OpenApiOperationTests.cs @@ -603,5 +603,48 @@ public async Task SerializesBodyReferencesWorks() """; Assert.True(JsonNode.DeepEquals(JsonNode.Parse(expected), JsonNode.Parse(actual))); } + [Fact] + public void DeduplicatesTagReferences() + { + + var openApiDocument = new OpenApiDocument + { + Tags = new HashSet { new() { Name = "user" } } + }; + // Act + var expectedOp = new OpenApiOperation + { + Tags = new HashSet + { + new OpenApiTagReference("user", openApiDocument), + new OpenApiTagReference("user", openApiDocument), + }, + Summary = "Logs user into the system", + Description = "", + OperationId = "loginUser", + Parameters = + { + new OpenApiParameter + { + Name = "password", + Description = "The password for login in clear text", + In = ParameterLocation.Query, + Required = true, + Schema = new OpenApiSchema() + { + Type = JsonSchemaType.String + } + } + } + }; + using var textWriter = new StringWriter(); + var writer = new OpenApiJsonWriter(textWriter); + expectedOp.SerializeAsV2(writer); + var result = textWriter.ToString(); + var parsedJson = JsonNode.Parse(result); + var operationObject = Assert.IsType(parsedJson); + var tags = Assert.IsType(operationObject["tags"]); + Assert.Single(tags); + } } } diff --git a/test/Microsoft.OpenApi.Readers.Tests/V3Tests/OpenApiDocumentTests.cs b/test/Microsoft.OpenApi.Readers.Tests/V3Tests/OpenApiDocumentTests.cs index 79dca5e9b..5359162bf 100644 --- a/test/Microsoft.OpenApi.Readers.Tests/V3Tests/OpenApiDocumentTests.cs +++ b/test/Microsoft.OpenApi.Readers.Tests/V3Tests/OpenApiDocumentTests.cs @@ -724,7 +724,7 @@ public async Task ParseModifiedPetStoreDocumentWithTagAndSecurityShouldSucceed() { [OperationType.Get] = new OpenApiOperation { - Tags = new List + Tags = new HashSet { tagReference1, tagReference2 @@ -812,7 +812,7 @@ public async Task ParseModifiedPetStoreDocumentWithTagAndSecurityShouldSucceed() }, [OperationType.Post] = new OpenApiOperation { - Tags = new List + Tags = new HashSet { tagReference1, tagReference2 @@ -1032,15 +1032,8 @@ public async Task ParseModifiedPetStoreDocumentWithTagAndSecurityShouldSucceed() actual.Document.Should().BeEquivalentTo(expected, options => options .IgnoringCyclicReferences() - .Excluding(x => x.Paths["/pets"].Operations[OperationType.Get].Tags[0].Reference) - .Excluding(x => x.Paths["/pets"].Operations[OperationType.Get].Tags[0].Reference.HostDocument) - .Excluding(x => x.Paths["/pets"].Operations[OperationType.Get].Tags[0].Target) - .Excluding(x => x.Paths["/pets"].Operations[OperationType.Post].Tags[0].Reference.HostDocument) - .Excluding(x => x.Paths["/pets"].Operations[OperationType.Post].Tags[0].Target) - .Excluding(x => x.Paths["/pets"].Operations[OperationType.Get].Tags[1].Reference.HostDocument) - .Excluding(x => x.Paths["/pets"].Operations[OperationType.Get].Tags[1].Target) - .Excluding(x => x.Paths["/pets"].Operations[OperationType.Post].Tags[1].Reference.HostDocument) - .Excluding(x => x.Paths["/pets"].Operations[OperationType.Post].Tags[1].Target) + .Excluding(x => x.Paths["/pets"].Operations[OperationType.Get].Tags) + .Excluding(x => x.Paths["/pets"].Operations[OperationType.Post].Tags) .Excluding(x => x.Workspace) .Excluding(y => y.BaseUri)); diff --git a/test/Microsoft.OpenApi.Readers.Tests/V3Tests/OpenApiOperationTests.cs b/test/Microsoft.OpenApi.Readers.Tests/V3Tests/OpenApiOperationTests.cs index f43a86d00..c7a2317dc 100644 --- a/test/Microsoft.OpenApi.Readers.Tests/V3Tests/OpenApiOperationTests.cs +++ b/test/Microsoft.OpenApi.Readers.Tests/V3Tests/OpenApiOperationTests.cs @@ -4,11 +4,13 @@ using System.Collections.Generic; using System.IO; using System.Linq; +using System.Text.Json.Nodes; using System.Threading.Tasks; using FluentAssertions; using Microsoft.OpenApi.Models; using Microsoft.OpenApi.Models.References; using Microsoft.OpenApi.Reader; +using Microsoft.OpenApi.Writers; using Xunit; namespace Microsoft.OpenApi.Readers.Tests.V3Tests @@ -42,7 +44,7 @@ public async Task ParseOperationWithParameterWithNoLocationShouldSucceed() var operation = await OpenApiModelFactory.LoadAsync(Path.Combine(SampleFolderPath, "operationWithParameterWithNoLocation.json"), OpenApiSpecVersion.OpenApi3_0, openApiDocument); var expectedOp = new OpenApiOperation { - Tags = + Tags = new HashSet() { new OpenApiTagReference("user", openApiDocument) }, @@ -78,10 +80,50 @@ public async Task ParseOperationWithParameterWithNoLocationShouldSucceed() // Assert expectedOp.Should().BeEquivalentTo(operation, options => - options.Excluding(x => x.Tags[0].Reference.HostDocument) - .Excluding(x => x.Tags[0].Reference) - .Excluding(x => x.Tags[0].Target) - .Excluding(x => x.Tags[0].Extensions)); + options.Excluding(x => x.Tags)); + } + [Fact] + public void DeduplicatesTagReferences() + { + + var openApiDocument = new OpenApiDocument + { + Tags = new HashSet { new() { Name = "user" } } + }; + // Act + var expectedOp = new OpenApiOperation + { + Tags = new HashSet() + { + new OpenApiTagReference("user", openApiDocument), + new OpenApiTagReference("user", openApiDocument), + }, + Summary = "Logs user into the system", + Description = "", + OperationId = "loginUser", + Parameters = + { + new OpenApiParameter + { + Name = "password", + Description = "The password for login in clear text", + In = ParameterLocation.Query, + Required = true, + Schema = new OpenApiSchema() + { + Type = JsonSchemaType.String + } + } + } + }; + using var textWriter = new StringWriter(); + var writer = new OpenApiJsonWriter(textWriter); + expectedOp.SerializeAsV3(writer); + var result = textWriter.ToString(); + var parsedJson = JsonNode.Parse(result); + var operationObject = Assert.IsType(parsedJson); + var tags = Assert.IsType(operationObject["tags"]); + Assert.Single(tags); } } } diff --git a/test/Microsoft.OpenApi.Tests/Models/OpenApiOperationTests.cs b/test/Microsoft.OpenApi.Tests/Models/OpenApiOperationTests.cs index 8b5ab31f3..31a26f1be 100644 --- a/test/Microsoft.OpenApi.Tests/Models/OpenApiOperationTests.cs +++ b/test/Microsoft.OpenApi.Tests/Models/OpenApiOperationTests.cs @@ -89,7 +89,7 @@ public class OpenApiOperationTests private static OpenApiOperation _advancedOperationWithTagsAndSecurity => new() { - Tags = new List + Tags = new HashSet { new OpenApiTagReference("tagId1", new OpenApiDocument{ Tags = new HashSet() { new OpenApiTag{Name = "tagId1"}} }) }, diff --git a/test/Microsoft.OpenApi.Tests/PublicApi/PublicApi.approved.txt b/test/Microsoft.OpenApi.Tests/PublicApi/PublicApi.approved.txt index 9a03b8a62..6686f1446 100644 --- a/test/Microsoft.OpenApi.Tests/PublicApi/PublicApi.approved.txt +++ b/test/Microsoft.OpenApi.Tests/PublicApi/PublicApi.approved.txt @@ -914,7 +914,7 @@ namespace Microsoft.OpenApi.Models public System.Collections.Generic.IList? Security { get; set; } public System.Collections.Generic.IList? Servers { get; set; } public string? Summary { get; set; } - public System.Collections.Generic.IList? Tags { get; set; } + public System.Collections.Generic.ISet? Tags { get; set; } public void SerializeAsV2(Microsoft.OpenApi.Writers.IOpenApiWriter writer) { } public void SerializeAsV3(Microsoft.OpenApi.Writers.IOpenApiWriter writer) { } public void SerializeAsV31(Microsoft.OpenApi.Writers.IOpenApiWriter writer) { } @@ -1664,8 +1664,8 @@ namespace Microsoft.OpenApi.Services public virtual void Visit(System.Collections.Generic.IList parameters) { } public virtual void Visit(System.Collections.Generic.IList openApiSecurityRequirements) { } public virtual void Visit(System.Collections.Generic.IList servers) { } - public virtual void Visit(System.Collections.Generic.IList openApiTags) { } public virtual void Visit(System.Collections.Generic.ISet openApiTags) { } + public virtual void Visit(System.Collections.Generic.ISet openApiTags) { } public virtual void Visit(System.Text.Json.Nodes.JsonNode node) { } } public class OpenApiWalker diff --git a/test/Microsoft.OpenApi.Tests/Walkers/WalkerLocationTests.cs b/test/Microsoft.OpenApi.Tests/Walkers/WalkerLocationTests.cs index ad8d91b23..a2f66e0c8 100644 --- a/test/Microsoft.OpenApi.Tests/Walkers/WalkerLocationTests.cs +++ b/test/Microsoft.OpenApi.Tests/Walkers/WalkerLocationTests.cs @@ -107,7 +107,6 @@ public void LocatePathOperationContentSchema() "#/paths/~1test/get/responses/200/content", "#/paths/~1test/get/responses/200/content/application~1json", "#/paths/~1test/get/responses/200/content/application~1json/schema", - "#/paths/~1test/get/tags", }, locator.Locations); @@ -316,7 +315,7 @@ public override void Visit(OpenApiServer server) { Locations.Add(this.PathString); } - public override void Visit(IList openApiTags) + public override void Visit(ISet openApiTags) { Locations.Add(this.PathString); } From 89973835822377e6c80b0cae2e253ac99e550635 Mon Sep 17 00:00:00 2001 From: Vincent Biret Date: Tue, 25 Feb 2025 15:17:44 -0500 Subject: [PATCH 6/6] chore: linting Signed-off-by: Vincent Biret --- src/Microsoft.OpenApi/Models/OpenApiDocument.cs | 11 +++-------- src/Microsoft.OpenApi/Models/OpenApiOperation.cs | 11 +++-------- 2 files changed, 6 insertions(+), 16 deletions(-) diff --git a/src/Microsoft.OpenApi/Models/OpenApiDocument.cs b/src/Microsoft.OpenApi/Models/OpenApiDocument.cs index 35c78c78b..3504a92f7 100644 --- a/src/Microsoft.OpenApi/Models/OpenApiDocument.cs +++ b/src/Microsoft.OpenApi/Models/OpenApiDocument.cs @@ -92,14 +92,9 @@ public ISet? Tags { return; } - if (value is HashSet tags && tags.Comparer is OpenApiTagComparer) - { - _tags = tags; - } - else - { - _tags = new HashSet(value, OpenApiTagComparer.Instance); - } + _tags = value is HashSet tags && tags.Comparer is OpenApiTagComparer ? + tags : + new HashSet(value, OpenApiTagComparer.Instance); } } diff --git a/src/Microsoft.OpenApi/Models/OpenApiOperation.cs b/src/Microsoft.OpenApi/Models/OpenApiOperation.cs index 0ad61ec27..1e10f640c 100644 --- a/src/Microsoft.OpenApi/Models/OpenApiOperation.cs +++ b/src/Microsoft.OpenApi/Models/OpenApiOperation.cs @@ -40,14 +40,9 @@ public ISet? Tags { return; } - if (value is HashSet tags && tags.Comparer is OpenApiTagComparer) - { - _tags = tags; - } - else - { - _tags = new HashSet(value, OpenApiTagComparer.Instance); - } + _tags = value is HashSet tags && tags.Comparer is OpenApiTagComparer ? + tags : + new HashSet(value, OpenApiTagComparer.Instance); } }