diff --git a/src/Microsoft.OpenApi.OData.Reader/Edm/EdmAnnotationExtensions.cs b/src/Microsoft.OpenApi.OData.Reader/Edm/EdmAnnotationExtensions.cs index cb9c042c..f6dd8b0f 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 21725576..a1d03f2e 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 bf88d593..f6dfefc7 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)]