feat: tags references are now deduplicated as well

baywet · baywet · commit 763c0c1c5856 · 2025-02-25T15:09:57.000-05:00
Signed-off-by: Vincent Biret &lt;vibiret@microsoft.com&gt;
diff --git a/src/Microsoft.OpenApi/Models/OpenApiOperation.cs b/src/Microsoft.OpenApi/Models/OpenApiOperation.cs
@@ -23,11 +23,33 @@ public class OpenApiOperation : IOpenApiSerializable, IOpenApiExtensible, IOpenA
         /// </summary>
         public const bool DeprecatedDefault = false;
 
+        private HashSet<OpenApiTagReference>? _tags;
         /// <summary>
         /// A list of tags for API documentation control.
         /// Tags can be used for logical grouping of operations by resources or any other qualifier.
         /// </summary>
-        public IList<OpenApiTagReference>? Tags { get; set; } = [];
+        public ISet<OpenApiTagReference>? Tags 
+        { 
+            get
+            {
+                return _tags;
+            }
+            set
+            {
+                if (value is null)
+                {
+                    return;
+                }
+                if (value is HashSet<OpenApiTagReference> tags && tags.Comparer is OpenApiTagComparer)
+                {
+                    _tags = tags;
+                }
+                else
+                {
+                    _tags = new HashSet<OpenApiTagReference>(value, OpenApiTagComparer.Instance);
+                }
+            }
+        }
 
         /// <summary>
         /// 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<OpenApiTagReference>(operation.Tags) : null;
+            Tags = operation.Tags != null ? new HashSet<OpenApiTagReference>(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
@@ -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
@@ -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.
 /// </summary>
-internal sealed class OpenApiTagComparer : IEqualityComparer<OpenApiTag>
+internal sealed class OpenApiTagComparer : IEqualityComparer<IOpenApiTag>
 {
     private static readonly Lazy<OpenApiTagComparer> _lazyInstance = new(() => new OpenApiTagComparer());
     /// <summary>
@@ -18,7 +18,7 @@ internal sealed class OpenApiTagComparer : IEqualityComparer<OpenApiTag>
     internal static OpenApiTagComparer Instance { get => _lazyInstance.Value; }
 
     /// <inheritdoc/>
-    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;
 
     /// <inheritdoc/>
-    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
@@ -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<OpenApiTagReference>(tags, OpenApiTagComparer.Instance);
+                        }
+                    }
                 },
                 {
                     "summary",
diff --git 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<OpenApiTagReference>(tags, OpenApiTagComparer.Instance);
+                        }
+                    }
                 },
                 {
                     "summary",
diff --git 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<OpenApiTagReference>(tags, OpenApiTagComparer.Instance);
+                        }
+                    }
                 },
                 {
                     "summary", (o, n, _) =>
diff --git a/src/Microsoft.OpenApi/Services/OpenApiFilterService.cs b/src/Microsoft.OpenApi/Services/OpenApiFilterService.cs
@@ -363,11 +363,11 @@ private static void ValidateFilters(IDictionary<string, List<string>> 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
@@ -316,7 +316,7 @@ public virtual void Visit(ISet<OpenApiTag> openApiTags)
         /// <summary>
         /// Visits list of <see cref="OpenApiTagReference"/>
         /// </summary>
-        public virtual void Visit(IList<OpenApiTagReference> openApiTags)
+        public virtual void Visit(ISet<OpenApiTagReference> openApiTags)
         {
         }
 
diff --git a/src/Microsoft.OpenApi/Services/OpenApiWalker.cs b/src/Microsoft.OpenApi/Services/OpenApiWalker.cs
@@ -84,7 +84,7 @@ internal void Walk(ISet<OpenApiTag> tags)
         /// <summary>
         /// Visits list of <see cref="OpenApiTagReference"/> and child objects
         /// </summary>
-        internal void Walk(IList<OpenApiTagReference> tags)
+        internal void Walk(ISet<OpenApiTagReference> tags)
         {
             if (tags == null)
             {
@@ -96,9 +96,10 @@ internal void Walk(IList<OpenApiTagReference> 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
@@ -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<OpenApiTagReference> {new OpenApiTagReference("reports.Functions", document)};
+            document.Paths[getTeamsActivityByDatePath].Operations[OperationType.Get].Tags = new HashSet<OpenApiTagReference> {new OpenApiTagReference("reports.Functions", document)};
+            document.Paths[usersPath].Operations[OperationType.Get].Tags = new HashSet<OpenApiTagReference> {new OpenApiTagReference("users.user", document)};
+            document.Paths[usersByIdPath].Operations[OperationType.Get].Tags = new HashSet<OpenApiTagReference> {new OpenApiTagReference("users.user", document)};
+            document.Paths[usersByIdPath].Operations[OperationType.Patch].Tags = new HashSet<OpenApiTagReference> {new OpenApiTagReference("users.user", document)};
+            document.Paths[messagesByIdPath].Operations[OperationType.Get].Tags = new HashSet<OpenApiTagReference> {new OpenApiTagReference("users.message", document)};
+            document.Paths[administrativeUnitRestorePath].Operations[OperationType.Post].Tags = new HashSet<OpenApiTagReference> {new OpenApiTagReference("administrativeUnits.Actions", document)};
+            document.Paths[logoPath].Operations[OperationType.Put].Tags = new HashSet<OpenApiTagReference> {new OpenApiTagReference("applications.application", document)};
+            document.Paths[securityProfilesPath].Operations[OperationType.Get].Tags = new HashSet<OpenApiTagReference> {new OpenApiTagReference("security.hostSecurityProfile", document)};
+            document.Paths[communicationsCallsKeepAlivePath].Operations[OperationType.Post].Tags = new HashSet<OpenApiTagReference> {new OpenApiTagReference("communications.Actions", document)};
+            document.Paths[eventsDeltaPath].Operations[OperationType.Get].Tags = new HashSet<OpenApiTagReference> {new OpenApiTagReference("groups.Functions", document)};
+            document.Paths[refPath].Operations[OperationType.Get].Tags = new HashSet<OpenApiTagReference> {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
@@ -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<OpenApiTag> { new() { Name = "user" } }
+            };
+            // Act
+            var expectedOp = new OpenApiOperation
+            {
+                Tags = new HashSet<OpenApiTagReference>
+                {
+                    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<JsonObject>(parsedJson);
+            var tags = Assert.IsType<JsonArray>(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
@@ -724,7 +724,7 @@ public async Task ParseModifiedPetStoreDocumentWithTagAndSecurityShouldSucceed()
                         {
                             [OperationType.Get] = new OpenApiOperation
                             {
-                                Tags = new List<OpenApiTagReference>
+                                Tags = new HashSet<OpenApiTagReference>
                                     {
                                         tagReference1,
                                         tagReference2
@@ -812,7 +812,7 @@ public async Task ParseModifiedPetStoreDocumentWithTagAndSecurityShouldSucceed()
                             },
                             [OperationType.Post] = new OpenApiOperation
                             {
-                                Tags = new List<OpenApiTagReference>
+                                Tags = new HashSet<OpenApiTagReference>
                                     {
                                         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
diff --git a/test/Microsoft.OpenApi.Tests/Models/OpenApiOperationTests.cs b/test/Microsoft.OpenApi.Tests/Models/OpenApiOperationTests.cs
diff --git a/test/Microsoft.OpenApi.Tests/PublicApi/PublicApi.approved.txt b/test/Microsoft.OpenApi.Tests/PublicApi/PublicApi.approved.txt
diff --git a/test/Microsoft.OpenApi.Tests/Walkers/WalkerLocationTests.cs b/test/Microsoft.OpenApi.Tests/Walkers/WalkerLocationTests.cs

Original file line number	Diff line number	Diff line change
`@@ -21,7 +21,7 @@ public override OpenApiTag Target`
`21`	`21`	`{`
`22`	`22`	`get`
`23`	`23`	`{`
`24`		`- return Reference.HostDocument?.Tags.FirstOrDefault(t => OpenApiTagComparer.StringComparer.Equals(t.Name, Reference.Id));`
	`24`	`+ return Reference.HostDocument?.Tags?.FirstOrDefault(t => OpenApiTagComparer.StringComparer.Equals(t.Name, Reference.Id));`
`25`	`25`	`}`
`26`	`26`	`}`
`27`	`27`
Original file line number	Diff line number	Diff line change
`@@ -23,10 +23,12 @@ internal static partial class OpenApiV2Deserializer`
`23`	`23`	`new()`
`24`	`24`	`{`
`25`	`25`	`{`
`26`		`- "tags", (o, n, doc) => o.Tags = n.CreateSimpleList(`
`27`		`- (valueNode, doc) =>`
`28`		`- LoadTagByReference(`
`29`		`- valueNode.GetScalarValue(), doc), doc)`
	`26`	`+ "tags", (o, n, doc) => {`
	`27`	`+ if (n.CreateSimpleList((valueNode, doc) => LoadTagByReference(valueNode.GetScalarValue(), doc), doc) is {Count: > 0} tags)`
	`28`	`+ {`
	`29`	`+ o.Tags = new HashSet<OpenApiTagReference>(tags, OpenApiTagComparer.Instance);`
	`30`	`+ }`
	`31`	`+ }`
`30`	`32`	`},`
`31`	`33`	`{`
`32`	`34`	`"summary",`
Original file line number	Diff line number	Diff line change
`@@ -363,11 +363,11 @@ private static void ValidateFilters(IDictionary<string, List<string>> requestUrl`
`363`	`363`	`if (tagsArray.Length == 1)`
`364`	`364`	`{`
`365`	`365`	`var regex = new Regex(tagsArray[0]);`
`366`		`- return (_, _, operation) => operation.Tags.Any(tag => regex.IsMatch(tag.Name));`
	`366`	`+ return (_, _, operation) => operation.Tags?.Any(tag => regex.IsMatch(tag.Name)) ?? false;`
`367`	`367`	`}`
`368`	`368`	`else`
`369`	`369`	`{`
`370`		`- return (_, _, operation) => operation.Tags.Any(tag => tagsArray.Contains(tag.Name));`
	`370`	`+ return (_, _, operation) => operation.Tags?.Any(tag => tagsArray.Contains(tag.Name)) ?? false;`
`371`	`371`	`}`
`372`	`372`	`}`
`373`	`373`
Original file line number	Diff line number	Diff line change
`@@ -316,7 +316,7 @@ public virtual void Visit(ISet<OpenApiTag> openApiTags)`
`316`	`316`	`/// <summary>`
`317`	`317`	`/// Visits list of <see cref="OpenApiTagReference"/>`
`318`	`318`	`/// </summary>`
`319`		`- public virtual void Visit(IList<OpenApiTagReference> openApiTags)`
	`319`	`+ public virtual void Visit(ISet<OpenApiTagReference> openApiTags)`
`320`	`320`	`{`
`321`	`321`	`}`
`322`	`322`
Original file line number	Diff line number	Diff line change
`@@ -84,7 +84,7 @@ internal void Walk(ISet<OpenApiTag> tags)`
`84`	`84`	`/// <summary>`
`85`	`85`	`/// Visits list of <see cref="OpenApiTagReference"/> and child objects`
`86`	`86`	`/// </summary>`
`87`		`- internal void Walk(IList<OpenApiTagReference> tags)`
	`87`	`+ internal void Walk(ISet<OpenApiTagReference> tags)`
`88`	`88`	`{`
`89`	`89`	`if (tags == null)`
`90`	`90`	`{`
`@@ -96,9 +96,10 @@ internal void Walk(IList<OpenApiTagReference> tags)`
`96`	`96`	`// Visit tags`
`97`	`97`	`if (tags != null)`
`98`	`98`	`{`
`99`		`- for (var i = 0; i < tags.Count; i++)`
	`99`	`+ var referencesAsArray = tags.ToArray();`
	`100`	`+ for (var i = 0; i < referencesAsArray.Length; i++)`
`100`	`101`	`{`
`101`		`- Walk(i.ToString(), () => Walk(tags[i]));`
	`102`	`+ Walk(i.ToString(), () => Walk(referencesAsArray[i]));`
`102`	`103`	`}`
`103`	`104`	`}`
`104`	`105`	`}`