From e6a5e52e113ada17088422611e5e45761914147d Mon Sep 17 00:00:00 2001 From: Irvine Sunday <40403681+irvinesunday@users.noreply.github.com> Date: Wed, 18 Aug 2021 16:33:26 +0300 Subject: [PATCH] [Fix] Generate paths for stream properties in base types of entities (#110) * Get all properties declared in type def. including base types * Update/refactor tests to validate stream props. of base types are captured * Refactor Handler class to use encapsulation to define navigation sources --- .../Edm/ODataPathProvider.cs | 4 +- .../MediaEntityGetOperationHandler.cs | 17 ++--- .../MediaEntityOperationalHandler.cs | 29 +++----- .../MediaEntityPutOperationHandler.cs | 17 ++--- .../Edm/ODataPathProviderTests.cs | 69 ++++++++++++------- .../MediaEntityGetOperationHandlerTests.cs | 4 +- .../MediaEntityPutOperationHandlerTests.cs | 4 +- 7 files changed, 69 insertions(+), 75 deletions(-) diff --git a/src/Microsoft.OpenApi.OData.Reader/Edm/ODataPathProvider.cs b/src/Microsoft.OpenApi.OData.Reader/Edm/ODataPathProvider.cs index f3be3ad..75bb41f 100644 --- a/src/Microsoft.OpenApi.OData.Reader/Edm/ODataPathProvider.cs +++ b/src/Microsoft.OpenApi.OData.Reader/Edm/ODataPathProvider.cs @@ -219,7 +219,7 @@ namespace Microsoft.OpenApi.OData.Edm Debug.Assert(currentPath != null); bool createValuePath = true; - foreach (IEdmStructuralProperty sp in entityType.DeclaredStructuralProperties()) + foreach (IEdmStructuralProperty sp in entityType.StructuralProperties()) { if (sp.Type.AsPrimitive().IsStream()) { @@ -228,7 +228,7 @@ namespace Microsoft.OpenApi.OData.Edm currentPath.Pop(); } - if (sp.Name.Equals("content", System.StringComparison.OrdinalIgnoreCase)) + if (sp.Name.Equals("content", StringComparison.OrdinalIgnoreCase)) { createValuePath = false; } diff --git a/src/Microsoft.OpenApi.OData.Reader/Operation/MediaEntityGetOperationHandler.cs b/src/Microsoft.OpenApi.OData.Reader/Operation/MediaEntityGetOperationHandler.cs index eea76de..510baa9 100644 --- a/src/Microsoft.OpenApi.OData.Reader/Operation/MediaEntityGetOperationHandler.cs +++ b/src/Microsoft.OpenApi.OData.Reader/Operation/MediaEntityGetOperationHandler.cs @@ -26,15 +26,9 @@ namespace Microsoft.OpenApi.OData.Operation protected override void SetBasicInfo(OpenApiOperation operation) { // Summary - if (IsNavigationPropertyPath) - { - operation.Summary = $"Get media content for the navigation property {NavigationProperty.Name} from {NavigationSource.Name}"; - } - else - { - IEdmEntityType entityType = EntitySet.EntityType(); - operation.Summary = $"Get media content for {entityType.Name} from {EntitySet.Name}"; - } + operation.Summary = IsNavigationPropertyPath + ? $"Get media content for the navigation property {NavigationProperty.Name} from {NavigationSource.Name}" + : $"Get media content for {NavigationSourceSegment.EntityType.Name} from {NavigationSourceSegment.Identifier}"; // Description IEdmVocabularyAnnotatable annotatableElement = GetAnnotatableElement(); @@ -73,9 +67,8 @@ namespace Microsoft.OpenApi.OData.Operation /// protected override void SetSecurity(OpenApiOperation operation) { - ReadRestrictionsType read = EntitySet != null - ? Context.Model.GetRecord(EntitySet, CapabilitiesConstants.ReadRestrictions) - : Context.Model.GetRecord(Singleton, CapabilitiesConstants.ReadRestrictions); + IEdmVocabularyAnnotatable annotatableNavigationSource = (IEdmVocabularyAnnotatable)NavigationSourceSegment.NavigationSource; + ReadRestrictionsType read = Context.Model.GetRecord(annotatableNavigationSource, CapabilitiesConstants.ReadRestrictions); if (read == null) { return; diff --git a/src/Microsoft.OpenApi.OData.Reader/Operation/MediaEntityOperationalHandler.cs b/src/Microsoft.OpenApi.OData.Reader/Operation/MediaEntityOperationalHandler.cs index 2859fc9..81de656 100644 --- a/src/Microsoft.OpenApi.OData.Reader/Operation/MediaEntityOperationalHandler.cs +++ b/src/Microsoft.OpenApi.OData.Reader/Operation/MediaEntityOperationalHandler.cs @@ -21,14 +21,9 @@ namespace Microsoft.OpenApi.OData.Operation internal abstract class MediaEntityOperationalHandler : NavigationPropertyOperationHandler { /// - /// Gets/sets the . + /// Gets/Sets the NavigationSource segment /// - protected IEdmEntitySet EntitySet { get; private set; } - - /// - /// Gets the . - /// - protected IEdmSingleton Singleton { get; private set; } + protected ODataNavigationSourceSegment NavigationSourceSegment { get; private set; } /// /// Gets/Sets flag indicating whether path is navigation property path @@ -39,13 +34,7 @@ namespace Microsoft.OpenApi.OData.Operation protected override void Initialize(ODataContext context, ODataPath path) { // The first segment will either be an EntitySet navigation source or a Singleton navigation source - ODataNavigationSourceSegment navigationSourceSegment = path.FirstSegment as ODataNavigationSourceSegment; - EntitySet = navigationSourceSegment.NavigationSource as IEdmEntitySet; - - if (EntitySet == null) - { - Singleton = navigationSourceSegment.NavigationSource as IEdmSingleton; - } + NavigationSourceSegment = path.FirstSegment as ODataNavigationSourceSegment; // Check whether path is a navigation property path IsNavigationPropertyPath = Path.Segments.Contains( @@ -67,9 +56,9 @@ namespace Microsoft.OpenApi.OData.Operation } else { - string tagIdentifier = EntitySet.Name + "." + EntitySet.EntityType().Name; + string tagIdentifier = NavigationSourceSegment.Identifier + "." + NavigationSourceSegment.EntityType.Name; - OpenApiTag tag = new OpenApiTag + OpenApiTag tag = new() { Name = tagIdentifier }; @@ -102,7 +91,7 @@ namespace Microsoft.OpenApi.OData.Operation IList items = new List { - EntitySet?.Name ?? Singleton.Name + NavigationSourceSegment.Identifier }; ODataSegment lastSegment = Path.Segments.Last(c => c is ODataStreamContentSegment || c is ODataStreamPropertySegment); @@ -112,7 +101,7 @@ namespace Microsoft.OpenApi.OData.Operation { if (!IsNavigationPropertyPath) { - string typeName = EntitySet?.EntityType().Name ?? Singleton.EntityType().Name; + string typeName = NavigationSourceSegment.EntityType.Name; items.Add(typeName); items.Add(prefix + Utils.UpperFirstChar(identifier)); } @@ -185,7 +174,7 @@ namespace Microsoft.OpenApi.OData.Operation /// The annotable element. protected IEdmVocabularyAnnotatable GetAnnotatableElement() { - IEdmEntityType entityType = EntitySet != null ? EntitySet.EntityType() : Singleton.EntityType(); + IEdmEntityType entityType = NavigationSourceSegment.EntityType; ODataSegment lastSegmentProp = Path.Segments.LastOrDefault(c => c is ODataStreamPropertySegment); if (lastSegmentProp == null) @@ -214,7 +203,7 @@ namespace Microsoft.OpenApi.OData.Operation private IEdmStructuralProperty GetStructuralProperty(IEdmEntityType entityType, string identifier) { - return entityType.DeclaredStructuralProperties().FirstOrDefault(x => x.Name.Equals(identifier)); + return entityType.StructuralProperties().FirstOrDefault(x => x.Name.Equals(identifier)); } private IEdmNavigationProperty GetNavigationProperty(IEdmEntityType entityType, string identifier) diff --git a/src/Microsoft.OpenApi.OData.Reader/Operation/MediaEntityPutOperationHandler.cs b/src/Microsoft.OpenApi.OData.Reader/Operation/MediaEntityPutOperationHandler.cs index 7d73032..6e1bfe6 100644 --- a/src/Microsoft.OpenApi.OData.Reader/Operation/MediaEntityPutOperationHandler.cs +++ b/src/Microsoft.OpenApi.OData.Reader/Operation/MediaEntityPutOperationHandler.cs @@ -26,15 +26,9 @@ namespace Microsoft.OpenApi.OData.Operation protected override void SetBasicInfo(OpenApiOperation operation) { // Summary - if (IsNavigationPropertyPath) - { - operation.Summary = $"Update media content for the navigation property {NavigationProperty.Name} in {NavigationSource.Name}"; - } - else - { - string typeName = EntitySet.EntityType().Name; - operation.Summary = $"Update media content for {typeName} in {EntitySet.Name}"; - } + operation.Summary = IsNavigationPropertyPath + ? $"Update media content for the navigation property {NavigationProperty.Name} in {NavigationSource.Name}" + : $"Update media content for {NavigationSourceSegment.EntityType.Name} in {NavigationSourceSegment.Identifier}"; // Description IEdmVocabularyAnnotatable annotatableElement = GetAnnotatableElement(); @@ -79,9 +73,8 @@ namespace Microsoft.OpenApi.OData.Operation /// protected override void SetSecurity(OpenApiOperation operation) { - UpdateRestrictionsType update = EntitySet != null - ? Context.Model.GetRecord(EntitySet, CapabilitiesConstants.UpdateRestrictions) - : Context.Model.GetRecord(Singleton, CapabilitiesConstants.UpdateRestrictions); + IEdmVocabularyAnnotatable annotatableNavigationSource = (IEdmVocabularyAnnotatable)NavigationSourceSegment.NavigationSource; + UpdateRestrictionsType update = Context.Model.GetRecord(annotatableNavigationSource, CapabilitiesConstants.UpdateRestrictions); if (update == null || update.Permissions == null) { return; diff --git a/test/Microsoft.OpenAPI.OData.Reader.Tests/Edm/ODataPathProviderTests.cs b/test/Microsoft.OpenAPI.OData.Reader.Tests/Edm/ODataPathProviderTests.cs index 00f4eb1..81307bb 100644 --- a/test/Microsoft.OpenAPI.OData.Reader.Tests/Edm/ODataPathProviderTests.cs +++ b/test/Microsoft.OpenAPI.OData.Reader.Tests/Edm/ODataPathProviderTests.cs @@ -450,41 +450,52 @@ namespace Microsoft.OpenApi.OData.Edm.Tests } [Theory] - [InlineData(true, "Logo")] - [InlineData(false, "Logo")] - [InlineData(true, "Content")] - [InlineData(false, "Content")] + [InlineData(true, "logo")] + [InlineData(false, "logo")] + [InlineData(true, "content")] + [InlineData(false, "content")] public void GetPathsWithStreamPropertyAndWithEntityHasStreamWorks(bool hasStream, string streamPropName) { // Arrange IEdmModel model = GetEdmModel(hasStream, streamPropName); ODataPathProvider provider = new ODataPathProvider(); var settings = new OpenApiConvertSettings(); + const string TodosContentPath = "/todos({id})/content"; + const string TodosValuePath = "/todos({id})/$value"; + const string TodosLogoPath = "/todos({id})/logo"; // Act - var paths = provider.GetPaths(model,settings); + var paths = provider.GetPaths(model, settings); // Assert Assert.NotNull(paths); + Assert.Contains("/catalog/content", paths.Select(p => p.GetPathItemName())); + Assert.Contains("/catalog/thumbnailPhoto", paths.Select(p => p.GetPathItemName())); + Assert.Contains("/me/photo/$value", paths.Select(p => p.GetPathItemName())); - if (hasStream && !streamPropName.Equals("Content", StringComparison.OrdinalIgnoreCase)) + if (streamPropName.Equals("logo")) { - Assert.Equal(7, paths.Count()); - Assert.Equal(new[] { "/me", "/me/photo", "/me/photo/$value", "/Todos", "/Todos({Id})", "/Todos({Id})/$value", "/Todos({Id})/Logo" }, - paths.Select(p => p.GetPathItemName())); + if (hasStream) + { + Assert.Equal(12, paths.Count()); + Assert.Contains(TodosValuePath, paths.Select(p => p.GetPathItemName())); + Assert.Contains(TodosLogoPath, paths.Select(p => p.GetPathItemName())); + Assert.DoesNotContain(TodosContentPath, paths.Select(p => p.GetPathItemName())); + } + else + { + Assert.Equal(11, paths.Count()); + Assert.Contains(TodosLogoPath, paths.Select(p => p.GetPathItemName())); + Assert.DoesNotContain(TodosContentPath, paths.Select(p => p.GetPathItemName())); + Assert.DoesNotContain(TodosValuePath, paths.Select(p => p.GetPathItemName())); + } } - else if ((hasStream && streamPropName.Equals("Content", StringComparison.OrdinalIgnoreCase)) || - (!hasStream && streamPropName.Equals("Content", StringComparison.OrdinalIgnoreCase))) + else if (streamPropName.Equals("content")) { - Assert.Equal(6, paths.Count()); - Assert.Equal(new[] { "/me", "/me/photo", "/me/photo/$value", "/Todos", "/Todos({Id})", "/Todos({Id})/Content" }, - paths.Select(p => p.GetPathItemName())); - } - else // !hasStream && !streamPropName.Equals("Content") - { - Assert.Equal(6, paths.Count()); - Assert.Equal(new[] { "/me", "/me/photo", "/me/photo/$value", "/Todos", "/Todos({Id})", "/Todos({Id})/Logo"}, - paths.Select(p => p.GetPathItemName())); + Assert.Equal(11, paths.Count()); + Assert.Contains(TodosContentPath, paths.Select(p => p.GetPathItemName())); + Assert.DoesNotContain(TodosLogoPath, paths.Select(p => p.GetPathItemName())); + Assert.DoesNotContain(TodosValuePath, paths.Select(p => p.GetPathItemName())); } } @@ -576,13 +587,13 @@ namespace Microsoft.OpenApi.OData.Edm.Tests string template = @" - + - + - + - + @@ -591,9 +602,17 @@ namespace Microsoft.OpenApi.OData.Edm.Tests + + + + + + + - + + diff --git a/test/Microsoft.OpenAPI.OData.Reader.Tests/Operation/MediaEntityGetOperationHandlerTests.cs b/test/Microsoft.OpenAPI.OData.Reader.Tests/Operation/MediaEntityGetOperationHandlerTests.cs index bec51e7..198a5ae 100644 --- a/test/Microsoft.OpenAPI.OData.Reader.Tests/Operation/MediaEntityGetOperationHandlerTests.cs +++ b/test/Microsoft.OpenAPI.OData.Reader.Tests/Operation/MediaEntityGetOperationHandlerTests.cs @@ -55,13 +55,13 @@ namespace Microsoft.OpenApi.OData.Operation.Tests Assert.NotNull(me); IEdmEntityType todo = model.SchemaElements.OfType().First(c => c.Name == "Todo"); - IEdmStructuralProperty sp = todo.DeclaredStructuralProperties().First(c => c.Name == "Logo"); + IEdmStructuralProperty sp = todo.StructuralProperties().First(c => c.Name == "Logo"); ODataPath path = new ODataPath(new ODataNavigationSourceSegment(todos), new ODataKeySegment(todos.EntityType()), new ODataStreamPropertySegment(sp.Name)); IEdmEntityType user = model.SchemaElements.OfType().First(c => c.Name == "user"); - IEdmNavigationProperty navProperty = user.DeclaredNavigationProperties().First(c => c.Name == "photo"); + IEdmNavigationProperty navProperty = user.NavigationProperties().First(c => c.Name == "photo"); ODataPath path2 = new ODataPath(new ODataNavigationSourceSegment(me), new ODataNavigationPropertySegment(navProperty), new ODataStreamContentSegment()); diff --git a/test/Microsoft.OpenAPI.OData.Reader.Tests/Operation/MediaEntityPutOperationHandlerTests.cs b/test/Microsoft.OpenAPI.OData.Reader.Tests/Operation/MediaEntityPutOperationHandlerTests.cs index 76c9e4f..7016aa5 100644 --- a/test/Microsoft.OpenAPI.OData.Reader.Tests/Operation/MediaEntityPutOperationHandlerTests.cs +++ b/test/Microsoft.OpenAPI.OData.Reader.Tests/Operation/MediaEntityPutOperationHandlerTests.cs @@ -52,13 +52,13 @@ namespace Microsoft.OpenApi.OData.Operation.Tests Assert.NotNull(todos); IEdmEntityType todo = model.SchemaElements.OfType().First(c => c.Name == "Todo"); - IEdmStructuralProperty sp = todo.DeclaredStructuralProperties().First(c => c.Name == "Logo"); + IEdmStructuralProperty sp = todo.StructuralProperties().First(c => c.Name == "Logo"); ODataPath path = new ODataPath(new ODataNavigationSourceSegment(todos), new ODataKeySegment(todos.EntityType()), new ODataStreamPropertySegment(sp.Name)); IEdmEntityType user = model.SchemaElements.OfType().First(c => c.Name == "user"); - IEdmNavigationProperty navProperty = user.DeclaredNavigationProperties().First(c => c.Name == "photo"); + IEdmNavigationProperty navProperty = user.NavigationProperties().First(c => c.Name == "photo"); ODataPath path2 = new ODataPath(new ODataNavigationSourceSegment(me), new ODataNavigationPropertySegment(navProperty), new ODataStreamContentSegment());