Fix OpenApiTagComparer behaviour

Update `OpenApiTagComparer` to behave intuitively with `OpenApiTagReference` instances pointing to tags not defined in an `OpenApiDocument`.

Contributes to #2319.

Author: martincostello

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;

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);
+    }
 }

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)]);
+    }
 }