fix(odata): handle mixed annotation cache key collisions on v2

src/Microsoft.OpenApi.OData.Reader/Edm/EdmAnnotationExtensions.cs

@@ -390,7 +390,7 @@
 
                 if (_cachedAnnotations == null)
                 {
-                    _cachedAnnotations = new Dictionary<IEdmVocabularyAnnotatable, IDictionary<string, object>>();
+                    _cachedAnnotations = new Dictionary<IEdmVocabularyAnnotatable, IDictionary<string, object>>(new AnnotationTargetComparer());
                 }
 
                 object? restriction;
@@ -421,6 +421,32 @@
             }
         }
 
+        private sealed class AnnotationTargetComparer : IEqualityComparer<IEdmVocabularyAnnotatable>
+        {
+            public bool Equals(IEdmVocabularyAnnotatable? x, IEdmVocabularyAnnotatable? y)
+            {
+                // EdmTargetPath.Equals dereferences a failed cast when a hash collision involves another target kind.
+                if (x is EdmTargetPath xPath)
+                {
+                    return y is EdmTargetPath yPath && StringComparer.Ordinal.Equals(xPath.Path, yPath.Path);
+                }
+
+                if (y is EdmTargetPath)
+                {
+                    return false;
+                }
+
+                return EqualityComparer<IEdmVocabularyAnnotatable>.Default.Equals(x, y);
+            }
+
+            public int GetHashCode(IEdmVocabularyAnnotatable target)
+            {
+                return target is EdmTargetPath path
+                    ? StringComparer.Ordinal.GetHashCode(path.Path)
+                    : EqualityComparer<IEdmVocabularyAnnotatable>.Default.GetHashCode(target);
+            }
+        }
+
         private static bool? GetBoolean(this IEdmModel model, IEdmVocabularyAnnotatable target, IEdmTerm term)
         {
             Debug.Assert(model != null);
@@ -456,7 +482,7 @@
             return null;
         }
 
         private static IEnumerable<string>? GetCollection(this IEdmModel model, IEdmVocabularyAnnotatable target, IEdmTerm term)
         {
             Debug.Assert(model != null);
             Debug.Assert(target != null);

test/Microsoft.OpenAPI.OData.Reader.Tests/Edm/EdmVocabularyAnnotationExtensionsTests.cs

@@ -3,20 +3,148 @@
 //  Licensed under the MIT License (MIT). See LICENSE in the repo root for license information.
 // ------------------------------------------------------------
 
+using System;
 using System.Collections.Generic;
 using System.Linq;
-using System.Runtime.InteropServices;
 using System.Xml.Linq;
 using Microsoft.OData.Edm;
 using Microsoft.OData.Edm.Csdl;
 using Microsoft.OData.Edm.Validation;
+using Microsoft.OData.Edm.Vocabularies;
 using Microsoft.OpenApi.OData.Vocabulary.Capabilities;
+using Moq;
 using Xunit;
 
 namespace Microsoft.OpenApi.OData.Edm.Tests
 {
+    [Collection("Vocabulary annotation cache")]
     public class EdmVocabularyAnnotationExtensionsTests
     {
+        private static readonly EdmTerm CachedStringTerm = new EdmTerm("NS", "CachedString", EdmPrimitiveTypeKind.String);
+
+        [Theory]
+        [InlineData(true)]
+        [InlineData(false)]
+        public void AnnotationCacheHandlesMixedTargetHashCollisions(bool pathFirst)
+        {
+            var path = CreateTargetPath();
+            var other = new ValueEqualAnnotatable(1, path.GetHashCode());
+            Assert.Equal(path.GetHashCode(), other.GetHashCode());
+            var model = GetCacheModel(
+                new EdmVocabularyAnnotation(path, CachedStringTerm, new EdmStringConstant("path")),
+                new EdmVocabularyAnnotation(other, CachedStringTerm, new EdmStringConstant("other")));
+
+            IEdmVocabularyAnnotatable first = pathFirst ? path : other;
+            IEdmVocabularyAnnotatable second = pathFirst ? other : path;
+            Assert.Equal(pathFirst ? "path" : "other", model.Object.GetString(first, CachedStringTerm.FullName()));
+            Assert.Equal(pathFirst ? "other" : "path", model.Object.GetString(second, CachedStringTerm.FullName()));
+            // Revisit both keys so a stored path is compared with a nonpath in either insertion order.
+            Assert.Equal("other", model.Object.GetString(other, CachedStringTerm.FullName()));
+            Assert.Equal("path", model.Object.GetString(path, CachedStringTerm.FullName()));
+            model.Verify(m => m.FindDeclaredVocabularyAnnotations(It.IsAny<IEdmVocabularyAnnotatable>()), Times.Exactly(2));
+        }
+
+        [Fact]
+        public void AnnotationCacheSharesEquivalentPathsUsingOrdinalEquality()
+        {
+            var path = CreateTargetPath();
+            var equivalentPath = CreateTargetPath();
+            var differentPath = CreateTargetPath("items");
+            Assert.NotSame(path, equivalentPath);
+            Assert.Equal(path.Path, equivalentPath.Path);
+            var model = GetCacheModel(
+                new EdmVocabularyAnnotation(path, CachedStringTerm, new EdmStringConstant("original")),
+                new EdmVocabularyAnnotation(differentPath, CachedStringTerm, new EdmStringConstant("different")));
+
+            Assert.Equal("original", model.Object.GetString(path, CachedStringTerm.FullName()));
+            Assert.Equal("original", model.Object.GetString(equivalentPath, CachedStringTerm.FullName()));
+            Assert.Equal("different", model.Object.GetString(differentPath, CachedStringTerm.FullName()));
+            model.Verify(m => m.FindDeclaredVocabularyAnnotations(It.IsAny<IEdmVocabularyAnnotatable>()), Times.Exactly(2));
+        }
+
+        [Fact]
+        public void AnnotationCachePreservesNonpathValueEqualityAndHashing()
+        {
+            var target = new ValueEqualAnnotatable(1, 42);
+            var equivalentTarget = new ValueEqualAnnotatable(1, 42);
+            var differentTarget = new ValueEqualAnnotatable(2, 42);
+            var model = GetCacheModel(
+                new EdmVocabularyAnnotation(target, CachedStringTerm, new EdmStringConstant("original")),
+                new EdmVocabularyAnnotation(differentTarget, CachedStringTerm, new EdmStringConstant("different")));
+
+            Assert.Equal("original", model.Object.GetString(target, CachedStringTerm.FullName()));
+            Assert.Equal("original", model.Object.GetString(equivalentTarget, CachedStringTerm.FullName()));
+            Assert.Equal("different", model.Object.GetString(differentTarget, CachedStringTerm.FullName()));
+            model.Verify(m => m.FindDeclaredVocabularyAnnotations(It.IsAny<IEdmVocabularyAnnotatable>()), Times.Exactly(2));
+        }
+
+        [Fact]
+        public void AnnotationCachePreservesNonpathReferenceEquality()
+        {
+            var target = new EdmEntityContainer("NS", "Default");
+            var differentTarget = new EdmEntityContainer("NS", "Default");
+            var model = GetCacheModel(
+                new EdmVocabularyAnnotation(target, CachedStringTerm, new EdmStringConstant("original")),
+                new EdmVocabularyAnnotation(differentTarget, CachedStringTerm, new EdmStringConstant("different")));
+
+            Assert.Equal("original", model.Object.GetString(target, CachedStringTerm.FullName()));
+            Assert.Equal("different", model.Object.GetString(differentTarget, CachedStringTerm.FullName()));
+            Assert.Equal("original", model.Object.GetString(target, CachedStringTerm.FullName()));
+            model.Verify(m => m.FindDeclaredVocabularyAnnotations(It.IsAny<IEdmVocabularyAnnotatable>()), Times.Exactly(2));
+        }
+
+        [Theory]
+        [InlineData(true)]
+        [InlineData(false)]
+        public void AnnotationCacheInvalidatesBothTargetKindsWhenModelChanges(bool pathTarget)
+        {
+            IEdmVocabularyAnnotatable target = pathTarget
+                ? CreateTargetPath()
+                : new ValueEqualAnnotatable(1, 42);
+            var firstModel = GetCacheModel(new EdmVocabularyAnnotation(target, CachedStringTerm, new EdmStringConstant("first")));
+            var secondModel = GetCacheModel(new EdmVocabularyAnnotation(target, CachedStringTerm, new EdmStringConstant("second")));
+
+            Assert.Equal("first", firstModel.Object.GetString(target, CachedStringTerm.FullName()));
+            Assert.Equal("second", secondModel.Object.GetString(target, CachedStringTerm.FullName()));
+            Assert.Equal("first", firstModel.Object.GetString(target, CachedStringTerm.FullName()));
+            firstModel.Verify(m => m.FindDeclaredVocabularyAnnotations(It.IsAny<IEdmVocabularyAnnotatable>()), Times.Exactly(2));
+            secondModel.Verify(m => m.FindDeclaredVocabularyAnnotations(It.IsAny<IEdmVocabularyAnnotatable>()), Times.Once());
+        }
+
+        private static Mock<IEdmModel> GetCacheModel(params IEdmVocabularyAnnotation[] annotations)
+        {
+            var model = new Mock<IEdmModel>();
+            model.Setup(m => m.DirectValueAnnotationsManager).Returns(new EdmModel().DirectValueAnnotationsManager);
+            model.Setup(m => m.FindDeclaredTerm(CachedStringTerm.FullName())).Returns(CachedStringTerm);
+            // Keep the model lookup reference-based to isolate the annotation cache's key comparison.
+            model.Setup(m => m.FindDeclaredVocabularyAnnotations(It.IsAny<IEdmVocabularyAnnotatable>()))
+                .Returns((IEdmVocabularyAnnotatable target) => annotations.Where(a => ReferenceEquals(a.Target, target)));
+            return model;
+        }
+
+        private static EdmTargetPath CreateTargetPath(string entitySetName = "Items")
+        {
+            var container = new EdmEntityContainer("NS", "Default");
+            var entitySet = container.AddEntitySet(entitySetName, new EdmEntityType("NS", "Item"));
+            return new EdmTargetPath(container, entitySet);
+        }
+
+        private sealed class ValueEqualAnnotatable : IEdmVocabularyAnnotatable
+        {
+            private readonly int value;
+            private readonly int hashCode;
+
+            public ValueEqualAnnotatable(int value, int hashCode)
+            {
+                this.value = value;
+                this.hashCode = hashCode;
+            }
+
+            public override bool Equals(object obj) => obj is ValueEqualAnnotatable other && value == other.value;
+
+            public override int GetHashCode() => hashCode;
+        }
+
         [Fact]
         public void GetStringWorksForString()
         {
@@ -255,4 +383,9 @@ private IEdmModel GetEdmModel(string annotation)
             return model;
         }
     }
+
+    [CollectionDefinition("Vocabulary annotation cache", DisableParallelization = true)]
+    public class VocabularyAnnotationCacheCollection
+    {
+    }
 }