fix: Improve handling of OpenAPI tag references (#2325)

martincostello · web-flow · commit bf9954a25769 · 2025-04-15T12:25:00.000Z
* Fix OpenApiTagComparer behaviour Update `OpenApiTagComparer` to behave intuitively with `OpenApiTagReference` instances pointing to tags not defined in an `OpenApiDocument`. Contributes to #2319. * Verify tag references on serialization Verify OpenAPI tag references refer to a valid OpenAPI tag in the document on serialization. Resolves #2319.
diff --git a/src/Microsoft.OpenApi/Models/OpenApiOperation.cs b/src/Microsoft.OpenApi/Models/OpenApiOperation.cs
@@ -4,6 +4,7 @@
 using System;
 using System.Collections.Generic;
 using System.Linq;
+using Microsoft.OpenApi.Exceptions;
 using Microsoft.OpenApi.Interfaces;
 using Microsoft.OpenApi.Models.Interfaces;
 using Microsoft.OpenApi.Models.References;
@@ -86,7 +87,7 @@ public HashSet<OpenApiTagReference>? Tags
         /// <summary>
         /// REQUIRED. The list of possible responses as they are returned from executing this operation.
         /// </summary>
-        public OpenApiResponses? Responses { get; set; } = new();
+        public OpenApiResponses? Responses { get; set; } = [];
 
         /// <summary>
         /// A map of possible out-of band callbacks related to the parent operation.
@@ -182,7 +183,7 @@ private void SerializeInternal(IOpenApiWriter writer, OpenApiSpecVersion version
             // tags
             writer.WriteOptionalCollection(
                 OpenApiConstants.Tags,
-                Tags,
+                VerifyTagReferences(Tags),
                 callback);
 
             // summary
@@ -236,7 +237,7 @@ public void SerializeAsV2(IOpenApiWriter writer)
             // tags
             writer.WriteOptionalCollection(
                 OpenApiConstants.Tags,
-                Tags,
+                VerifyTagReferences(Tags),
                 (w, t) => t.SerializeAsV2(w));
 
             // summary
@@ -355,5 +356,21 @@ public void SerializeAsV2(IOpenApiWriter writer)
 
             writer.WriteEndObject();
         }
+
+        private static HashSet<OpenApiTagReference>? VerifyTagReferences(HashSet<OpenApiTagReference>? tags)
+        {
+            if (tags?.Count > 0)
+            {
+                foreach (var tag in tags)
+                {
+                    if (tag.Target is null)
+                    {
+                        throw new OpenApiException($"The OpenAPI tag reference '{tag.Reference.Id}' does reference a valid tag.");
+                    }
+                }
+            }
+
+            return tags;
+        }
     }
 }
diff --git a/src/Microsoft.OpenApi/Models/References/OpenApiTagReference.cs b/src/Microsoft.OpenApi/Models/References/OpenApiTagReference.cs
@@ -1,7 +1,6 @@
 ﻿// Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT license. 
 
-using System;
 using System.Collections.Generic;
 using System.Linq;
 using Microsoft.OpenApi.Interfaces;
diff --git a/src/Microsoft.OpenApi/OpenApiTagComparer.cs b/src/Microsoft.OpenApi/OpenApiTagComparer.cs
@@ -1,6 +1,7 @@
 using System;
 using System.Collections.Generic;
 using Microsoft.OpenApi.Models.Interfaces;
+using Microsoft.OpenApi.Models.References;
 
 namespace Microsoft.OpenApi;
 
@@ -31,6 +32,10 @@ public bool Equals(IOpenApiTag? x, IOpenApiTag? y)
         {
             return true;
         }
+        if (x is OpenApiTagReference referenceX && y is OpenApiTagReference referenceY)
+        {
+            return StringComparer.Equals(referenceX.Name ?? referenceX.Reference.Id, referenceY.Name ?? referenceY.Reference.Id);
+        }
         return StringComparer.Equals(x.Name, y.Name);
     }
 
@@ -41,5 +46,13 @@ public bool Equals(IOpenApiTag? x, IOpenApiTag? y)
     internal static readonly StringComparer StringComparer = StringComparer.Ordinal;
 
     /// <inheritdoc/>
-    public int GetHashCode(IOpenApiTag obj) => string.IsNullOrEmpty(obj?.Name) ? 0 : StringComparer.GetHashCode(obj!.Name);
+    public int GetHashCode(IOpenApiTag obj)
+    {
+        string? value = obj?.Name;
+        if (value is null && obj is OpenApiTagReference reference)
+        {
+            value = reference.Reference.Id;
+        }
+        return string.IsNullOrEmpty(value) ? 0 : StringComparer.GetHashCode(value);
+    }
 }
diff --git a/test/Microsoft.OpenApi.Hidi.Tests/Services/OpenApiFilterServiceTests.cs b/test/Microsoft.OpenApi.Hidi.Tests/Services/OpenApiFilterServiceTests.cs
@@ -244,7 +244,7 @@ public async Task CopiesOverAllReferencedComponentsToTheSubsetDocumentCorrectly(
             var openApiOperationTags = doc?.Paths["/items"].Operations?[HttpMethod.Get].Tags?.ToArray();
             Assert.NotNull(openApiOperationTags);
             Assert.Single(openApiOperationTags);
-            Assert.True(openApiOperationTags[0].UnresolvedReference);
+            Assert.NotNull(openApiOperationTags[0].Target);
 
             var predicate = OpenApiFilterService.CreatePredicate(operationIds: operationIds);
             if (doc is not null)
@@ -271,7 +271,7 @@ public async Task CopiesOverAllReferencedComponentsToTheSubsetDocumentCorrectly(
                 var trimmedOpenApiOperationTags = subsetOpenApiDocument.Paths?["/items"].Operations?[HttpMethod.Get].Tags?.ToArray();
                 Assert.NotNull(trimmedOpenApiOperationTags);
                 Assert.Single(trimmedOpenApiOperationTags);
-                Assert.True(trimmedOpenApiOperationTags[0].UnresolvedReference);
+                Assert.NotNull(trimmedOpenApiOperationTags[0].Target);
 
                 // Finally try to write the trimmed document as v3 document
                 var outputStringWriter = new StringWriter(CultureInfo.InvariantCulture);
@@ -302,7 +302,8 @@ public void ReturnsPathParametersOnSlicingBasedOnOperationIdsOrTags(string? oper
             // Assert
             foreach (var pathItem in subsetOpenApiDocument.Paths)
             {
-                Assert.True(pathItem.Value.Parameters!.Count != 0);
+                Assert.NotNull(pathItem.Value.Parameters);
+                Assert.NotEmpty(pathItem.Value.Parameters);
                 Assert.Single(pathItem.Value.Parameters!);
             }
         }
diff --git a/test/Microsoft.OpenApi.Hidi.Tests/UtilityFiles/docWithReusableHeadersAndExamples.yaml b/test/Microsoft.OpenApi.Hidi.Tests/UtilityFiles/docWithReusableHeadersAndExamples.yaml
@@ -81,3 +81,5 @@ components:
       value:
         name: "New Item"
 
+tags:
+  - name: list.items
diff --git a/test/Microsoft.OpenApi.Readers.Tests/V31Tests/OpenApiDocumentTests.ParseDocumentWith31PropertiesWorks.verified.txt b/test/Microsoft.OpenApi.Readers.Tests/V31Tests/OpenApiDocumentTests.ParseDocumentWith31PropertiesWorks.verified.txt
@@ -99,6 +99,8 @@ components:
       in: header
 security:
   - api_key: [ ]
+tags:
+  - name: pets
 webhooks:
   newPetAlert:
     post:
diff --git a/test/Microsoft.OpenApi.Readers.Tests/V31Tests/Samples/OpenApiDocument/documentWith31Properties.yaml b/test/Microsoft.OpenApi.Readers.Tests/V31Tests/Samples/OpenApiDocument/documentWith31Properties.yaml
@@ -122,5 +122,8 @@ components:
         ExtraInfo:
           type: string
 
+tags:
+  - name: pets
+
 security:
   - api_key: []
diff --git a/test/Microsoft.OpenApi.Tests/Models/OpenApiOperationTests.cs b/test/Microsoft.OpenApi.Tests/Models/OpenApiOperationTests.cs
@@ -4,6 +4,7 @@
 using System.Collections.Generic;
 using System.Text.Json.Nodes;
 using System.Threading.Tasks;
+using Microsoft.OpenApi.Exceptions;
 using Microsoft.OpenApi.Extensions;
 using Microsoft.OpenApi.Models;
 using Microsoft.OpenApi.Models.Interfaces;
@@ -859,5 +860,34 @@ public void OpenApiOperationCopyConstructorWithAnnotationsSucceeds()
 
             Assert.NotEqual(baseOperation.Metadata["key1"], actualOperation.Metadata["key1"]);
         }
+
+        [Theory]
+        [InlineData(OpenApiSpecVersion.OpenApi2_0)]
+        [InlineData(OpenApiSpecVersion.OpenApi3_0)]
+        [InlineData(OpenApiSpecVersion.OpenApi3_1)]
+        public async Task SerializeAsJsonAsyncThrowsIfTagReferenceIsUnresolved(OpenApiSpecVersion version)
+        {
+            var document = new OpenApiDocument()
+            {
+                Tags =
+                [
+                    new() { Name = "one" },
+                    new() { Name = "three" }
+                ]
+            };
+
+            var operation = new OpenApiOperation()
+            {
+                Tags =
+                [
+                    new OpenApiTagReference("one", document),
+                    new OpenApiTagReference("two", document),
+                    new OpenApiTagReference("three", document)
+                ]
+            };
+
+            var exception = await Assert.ThrowsAsync<OpenApiException>(() => operation.SerializeAsJsonAsync(version));
+            Assert.Equal("The OpenAPI tag reference 'two' does reference a valid tag.", exception.Message);
+        }
     }
 }
diff --git a/test/Microsoft.OpenApi.Tests/OpenApiTagComparerTests.cs b/test/Microsoft.OpenApi.Tests/OpenApiTagComparerTests.cs
@@ -1,11 +1,15 @@
+﻿using System.Collections.Generic;
+using System.Linq;
 using Microsoft.OpenApi.Models;
+using Microsoft.OpenApi.Models.References;
 using Xunit;
 
 namespace Microsoft.OpenApi.Tests;
 
 public class OpenApiTagComparerTests
 {
     private readonly OpenApiTagComparer _comparer = OpenApiTagComparer.Instance;
+
     [Fact]
     public void Defensive()
     {
@@ -16,13 +20,15 @@ public void Defensive()
         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()
     {
@@ -37,4 +43,112 @@ public void DifferentCasingAreNotEquals()
         var openApiTag2 = new OpenApiTag { Name = "TAG" };
         Assert.False(_comparer.Equals(openApiTag1, openApiTag2));
     }
+
+    [Fact]
+    public void WorksCorrectlyWithHashSetOfTags()
+    {
+        var tags = new HashSet<OpenApiTag>(_comparer)
+        {
+            new() { Name = "one" },
+            new() { Name = "two" },
+            new() { Name = "two" },
+            new() { Name = "three" }
+        };
+
+        Assert.Equal(["one", "two", "three"], [.. tags.Select(t => t.Name)]);
+    }
+
+    [Fact]
+    public void SameReferenceInstanceAreEqual()
+    {
+        var openApiTag = new OpenApiTagReference("tag");
+        Assert.True(_comparer.Equals(openApiTag, openApiTag));
+    }
+
+    [Fact]
+    public void SameReferenceIdsAreEqual()
+    {
+        var openApiTag1 = new OpenApiTagReference("tag");
+        var openApiTag2 = new OpenApiTagReference("tag");
+        Assert.True(_comparer.Equals(openApiTag1, openApiTag2));
+    }
+
+    [Fact]
+    public void SameReferenceIdAreEqualWithValidTagReferences()
+    {
+        var document = new OpenApiDocument
+        {
+            Tags = [new() { Name = "tag" }]
+        };
+
+        var openApiTag1 = new OpenApiTagReference("tag", document);
+        var openApiTag2 = new OpenApiTagReference("tag", document);
+        Assert.True(_comparer.Equals(openApiTag1, openApiTag2));
+    }
+
+    [Fact]
+    public void DifferentReferenceIdAreNotEqualWithValidTagReferences()
+    {
+        var document = new OpenApiDocument
+        {
+            Tags =
+            [
+                new() { Name = "one" },
+                new() { Name = "two" },
+            ]
+        };
+
+        var openApiTag1 = new OpenApiTagReference("one", document);
+        var openApiTag2 = new OpenApiTagReference("two", document);
+        Assert.False(_comparer.Equals(openApiTag1, openApiTag2));
+    }
+
+    [Fact]
+    public void DifferentCasingReferenceIdsAreNotEqual()
+    {
+        var openApiTag1 = new OpenApiTagReference("tag");
+        var openApiTag2 = new OpenApiTagReference("TAG");
+        Assert.False(_comparer.Equals(openApiTag1, openApiTag2));
+    }
+
+    [Fact] // See https://github.com/microsoft/OpenAPI.NET/issues/2319
+    public void WorksCorrectlyWithHashSetOfReferences()
+    {
+        // The document intentionally does not contain the actual tags
+        var document = new OpenApiDocument();
+
+        var tags = new HashSet<OpenApiTagReference>(_comparer)
+        {
+            new("one", document),
+            new("two", document),
+            new("two", document),
+            new("three", document)
+        };
+
+        Assert.Equal(["one", "two", "three"], [..tags.Select(t => t.Reference.Id)]);
+    }
+
+    [Fact]
+    public void WorksCorrectlyWithHashSetOfReferencesToValidTags()
+    {
+        var document = new OpenApiDocument
+        {
+            Tags =
+            [
+                new() { Name = "one" },
+                new() { Name = "two" },
+                new() { Name = "three" }
+            ]
+        };
+
+        var tags = new HashSet<OpenApiTagReference>(_comparer)
+        {
+            new("one", document),
+            new("two", document),
+            new("two", document),
+            new("three", document)
+        };
+
+        Assert.Equal(["one", "two", "three"], [.. tags.Select(t => t.Reference.Id)]);
+    }
 }
