From 18311eed836a821ad5166a841919396b4a589a8f Mon Sep 17 00:00:00 2001 From: Gavin Barron Date: Thu, 12 Nov 2026 14:01:33 -0800 Subject: [PATCH] fix: preserve cached annotations when another term is absent Keep existing target dictionaries and evaluate uncached annotations once. Cover merged record identity, absent terms, model resets, and navigation GET to count custom-option propagation. Fixes #951 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../Edm/EdmAnnotationExtensions.cs | 14 ++-- .../EdmVocabularyAnnotationExtensionsTests.cs | 57 +++++++++++++++ .../DollarCountGetOperationHandlerTests.cs | 73 +++++++++++++++++++ 3 files changed, 136 insertions(+), 8 deletions(-) diff --git a/src/Microsoft.OpenApi.OData.Reader/Edm/EdmAnnotationExtensions.cs b/src/Microsoft.OpenApi.OData.Reader/Edm/EdmAnnotationExtensions.cs index cb9c042c8..f6dd8b0fa 100644 --- a/src/Microsoft.OpenApi.OData.Reader/Edm/EdmAnnotationExtensions.cs +++ b/src/Microsoft.OpenApi.OData.Reader/Edm/EdmAnnotationExtensions.cs @@ -402,16 +402,14 @@ internal static class EdmVocabularyAnnotationExtensions T ret = (T)restriction; return ret; } - else if (createFunc() is T ret) - { - value[qualifiedName] = ret; - return ret; - } + } + else + { + // Only new targets need a dictionary; absent annotations must not evict cached records. + value = new Dictionary(); + _cachedAnnotations[target] = value; } - // It's first time to query this target, create new dictionary and restriction. - value = new Dictionary(); - _cachedAnnotations[target] = value; if (createFunc() is T newAnnotation) { value[qualifiedName] = newAnnotation; diff --git a/test/Microsoft.OpenAPI.OData.Reader.Tests/Edm/EdmVocabularyAnnotationExtensionsTests.cs b/test/Microsoft.OpenAPI.OData.Reader.Tests/Edm/EdmVocabularyAnnotationExtensionsTests.cs index 217255767..a1d03f2e9 100644 --- a/test/Microsoft.OpenAPI.OData.Reader.Tests/Edm/EdmVocabularyAnnotationExtensionsTests.cs +++ b/test/Microsoft.OpenAPI.OData.Reader.Tests/Edm/EdmVocabularyAnnotationExtensionsTests.cs @@ -21,6 +21,63 @@ public class EdmVocabularyAnnotationExtensionsTests { private static readonly EdmTerm CachedStringTerm = new EdmTerm("NS", "CachedString", EdmPrimitiveTypeKind.String); + [Theory] + [InlineData(true)] + [InlineData(false)] + public void AnnotationCachePreservesMergedRecordsAfterAbsentTermLookups(bool queryAbsentTermFirst) + { + var model = GetEdmModel(@" + + + + +"); + var target = model.EntityContainer; + if (queryAbsentTermFirst) + { + Assert.Null(model.GetRecord(target)); + } + + var readRestrictions = model.GetRecord(target); + Assert.NotNull(readRestrictions); + var inheritedRestrictions = new ReadRestrictionsType(); + inheritedRestrictions.Initialize(new EdmRecordExpression( + new EdmPropertyConstructor("CustomQueryOptions", new EdmCollectionExpression( + new EdmRecordExpression(new EdmPropertyConstructor("Name", new EdmStringConstant("includeHiddenFolders"))))))); + readRestrictions.MergePropertiesIfNull(inheritedRestrictions); + Assert.Equal("Items", model.GetString(target, "Org.OData.Core.V1.Description")); + + Assert.Null(model.GetRecord(target)); + Assert.Null(model.GetRecord(target)); + Assert.Null(model.GetString(target, "NS.UndefinedTerm")); + + var cachedRestrictions = model.GetRecord(target); + Assert.Same(readRestrictions, cachedRestrictions); + Assert.Equal("List items", cachedRestrictions.Description); + Assert.Equal("includeHiddenFolders", Assert.Single(cachedRestrictions.CustomQueryOptions).Name); + Assert.Equal("Items", model.GetString(target, "Org.OData.Core.V1.Description")); + + var secondModel = GetEdmModel(""); + Assert.Null(secondModel.GetRecord(secondModel.EntityContainer)); + + var reloadedRestrictions = model.GetRecord(target); + Assert.NotSame(readRestrictions, reloadedRestrictions); + Assert.Equal("List items", reloadedRestrictions.Description); + Assert.Null(reloadedRestrictions.CustomQueryOptions); + } + + [Fact] + public void AnnotationCacheLooksUpAbsentTermOncePerCallForNewAndExistingTargets() + { + var target = new ValueEqualAnnotatable(1, 42); + var model = GetCacheModel(); + + Assert.Null(model.GetString(target, CachedStringTerm.FullName())); + Assert.Equal(1, model.FindDeclaredVocabularyAnnotationsCallCount); + Assert.Null(model.GetString(target, CachedStringTerm.FullName())); + Assert.Equal(2, model.FindDeclaredVocabularyAnnotationsCallCount); + } + [Theory] [InlineData(true)] [InlineData(false)] diff --git a/test/Microsoft.OpenAPI.OData.Reader.Tests/Operation/DollarCountGetOperationHandlerTests.cs b/test/Microsoft.OpenAPI.OData.Reader.Tests/Operation/DollarCountGetOperationHandlerTests.cs index bf88d593d..f6dfefc7e 100644 --- a/test/Microsoft.OpenAPI.OData.Reader.Tests/Operation/DollarCountGetOperationHandlerTests.cs +++ b/test/Microsoft.OpenAPI.OData.Reader.Tests/Operation/DollarCountGetOperationHandlerTests.cs @@ -4,13 +4,17 @@ // ------------------------------------------------------------ using Microsoft.OData.Edm; +using Microsoft.OData.Edm.Csdl; using System.Linq; +using System.Xml.Linq; using Microsoft.OpenApi.OData.Edm; using Microsoft.OpenApi.OData.Tests; +using Microsoft.OpenApi.OData.Vocabulary.Capabilities; using Xunit; namespace Microsoft.OpenApi.OData.Operation.Tests { + [Collection("Vocabulary annotation cache")] public class DollarCountGetOperationHandlerTests { private readonly OpenApiDocument openApiDocument = new(); @@ -22,6 +26,75 @@ public DollarCountGetOperationHandlerTests() openApiDocument.AddComponent("ConsistencyLevel", new OpenApiParameter {Name = "ConsistencyLevel", In = ParameterLocation.Query, Schema = new OpenApiSchema {Type = JsonSchemaType.String}}); } + [Fact] + public void CreateDollarCountGetOperationPreservesCustomOptionsMergedByNavigationGet() + { + const string csdl = """ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + """; + var model = CsdlReader.Parse(XElement.Parse(csdl).CreateReader()); + var context = new ODataContext(model, new OpenApiConvertSettings()); + var me = model.EntityContainer.FindSingleton("me"); + var mailFolders = me.EntityType.DeclaredNavigationProperties().Single(); + var path = new ODataPath(new ODataNavigationSourceSegment(me), new ODataNavigationPropertySegment(mailFolders)); + var collectionOperation = new NavigationPropertyGetOperationHandler(openApiDocument).CreateOperation(context, path); + Assert.Contains(collectionOperation.Parameters, p => p.Name == "includeHiddenFolders" && p.In == ParameterLocation.Query); + + Assert.Null(model.GetRecord("NS.Default/me/mailFolders", CapabilitiesConstants.CountRestrictions)); + var countPath = new ODataPath(new ODataNavigationSourceSegment(me), + new ODataNavigationPropertySegment(mailFolders), new ODataDollarCountSegment()); + var countOperation = _operationHandler.CreateOperation(context, countPath); + + Assert.Contains(countOperation.Parameters, p => p.Name == "includeHiddenFolders" && p.In == ParameterLocation.Query); + } + [Theory] [InlineData(true, true)] [InlineData(false, true)]