From 88f9806a1eabd3d34d7006b9aa455e3c09176065 Mon Sep 17 00:00:00 2001 From: Irvine Sunday Date: Wed, 31 Jan 2024 16:25:28 +0300 Subject: [PATCH 1/6] Fix convert settings text documentation --- src/Microsoft.OpenApi.OData.Reader/OpenApiConvertSettings.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Microsoft.OpenApi.OData.Reader/OpenApiConvertSettings.cs b/src/Microsoft.OpenApi.OData.Reader/OpenApiConvertSettings.cs index 7445d5e2..47bc6d4b 100644 --- a/src/Microsoft.OpenApi.OData.Reader/OpenApiConvertSettings.cs +++ b/src/Microsoft.OpenApi.OData.Reader/OpenApiConvertSettings.cs @@ -247,7 +247,7 @@ public string PathPrefix /// /// Gets/Sets a value indicating whether or not to expand derived types to retrieve their declared navigation properties. /// - [Obsolete("Use RetrieveDerivedTypesProperties to Get or Set the value.")] + [Obsolete("Use GenerateDerivedTypesProperties to Get or Set the value.")] public bool ExpandDerivedTypesNavigationProperties { get => GenerateDerivedTypesProperties; @@ -299,7 +299,7 @@ public bool ExpandDerivedTypesNavigationProperties public bool RequireRestrictionAnnotationsToGenerateComplexPropertyPaths { get; set; } = true; /// - /// Gets/sets a dictionary containing a mapping of custom atttribute names and extension names. + /// Gets/sets a dictionary containing a mapping of custom attribute names and extension names. /// public Dictionary CustomXMLAttributesMapping { get; set; } = new(); From e7b184fe97f05189a051401e478bf35c224a4755 Mon Sep 17 00:00:00 2001 From: Irvine Sunday Date: Wed, 31 Jan 2024 16:27:56 +0300 Subject: [PATCH 2/6] Ensure non-contained deletability set explicitly via annotation --- .../NavigationPropertyPathItemHandler.cs | 35 ++++++++++--------- 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/src/Microsoft.OpenApi.OData.Reader/PathItem/NavigationPropertyPathItemHandler.cs b/src/Microsoft.OpenApi.OData.Reader/PathItem/NavigationPropertyPathItemHandler.cs index a95743e1..b9673974 100644 --- a/src/Microsoft.OpenApi.OData.Reader/PathItem/NavigationPropertyPathItemHandler.cs +++ b/src/Microsoft.OpenApi.OData.Reader/PathItem/NavigationPropertyPathItemHandler.cs @@ -1,4 +1,4 @@ -// ------------------------------------------------------------ +// ------------------------------------------------------------ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License (MIT). See LICENSE in the repo root for license information. // ------------------------------------------------------------ @@ -197,27 +197,28 @@ private void AddDeleteOperation(OpenApiPathItem item, NavigationPropertyRestrict Debug.Assert(!LastSegmentIsRefSegment); DeleteRestrictionsType navPropDeleteRestriction = restriction?.DeleteRestrictions ?? - Context.Model.GetRecord(NavigationProperty); - + Context.Model.GetRecord(NavigationProperty); + if (NavigationProperty.ContainsTarget) { - DeleteRestrictionsType entityDeleteRestriction = Context.Model.GetRecord(_navPropEntityType); - bool isDeletableDefault = navPropDeleteRestriction == null && entityDeleteRestriction == null; - - if (isDeletableDefault || - ((entityDeleteRestriction?.IsDeletable ?? true) && - (navPropDeleteRestriction?.IsDeletable ?? true))) - { - if (NavigationProperty.TargetMultiplicity() != EdmMultiplicity.Many || LastSegmentIsKeySegment) - { - AddOperation(item, OperationType.Delete); - } - } + DeleteRestrictionsType entityDeleteRestriction = Context.Model.GetRecord(_navPropEntityType); + bool isDeletableDefault = navPropDeleteRestriction == null && entityDeleteRestriction == null; + + if ((isDeletableDefault || + ((entityDeleteRestriction?.IsDeletable ?? true) && + (navPropDeleteRestriction?.IsDeletable ?? true))) && + (NavigationProperty.TargetMultiplicity() != EdmMultiplicity.Many || + LastSegmentIsKeySegment)) + { + AddOperation(item, OperationType.Delete); + } } else { - if ((navPropDeleteRestriction?.IsDeletable ?? false) && - (NavigationProperty.TargetMultiplicity() != EdmMultiplicity.Many || + // Add delete operation for non-contained nav. props only if explicitly set to true via annotation + // Note: Use Deletable and not IsDeletable + if ((navPropDeleteRestriction?.Deletable ?? false) && + (NavigationProperty.TargetMultiplicity() != EdmMultiplicity.Many || LastSegmentIsKeySegment)) { AddOperation(item, OperationType.Delete); From 50780a49bbd08204f592c3fa01ff2f8f8d8d55db Mon Sep 17 00:00:00 2001 From: Irvine Sunday Date: Wed, 31 Jan 2024 16:28:31 +0300 Subject: [PATCH 3/6] Update test --- .../NavigationPropertyPathItemHandlerTests.cs | 21 +++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/test/Microsoft.OpenAPI.OData.Reader.Tests/PathItem/NavigationPropertyPathItemHandlerTests.cs b/test/Microsoft.OpenAPI.OData.Reader.Tests/PathItem/NavigationPropertyPathItemHandlerTests.cs index e06ce16d..66aedac9 100644 --- a/test/Microsoft.OpenAPI.OData.Reader.Tests/PathItem/NavigationPropertyPathItemHandlerTests.cs +++ b/test/Microsoft.OpenAPI.OData.Reader.Tests/PathItem/NavigationPropertyPathItemHandlerTests.cs @@ -61,12 +61,22 @@ public void CreatePathItemThrowsForNonNavigationPropertyPath() [Theory] [InlineData(true, true, new OperationType[] { OperationType.Get, OperationType.Patch, OperationType.Delete })] [InlineData(true, false, new OperationType[] { OperationType.Get, OperationType.Post })] - [InlineData(false, true, new OperationType[] { OperationType.Get })] + [InlineData(false, true, new OperationType[] { OperationType.Get, OperationType.Delete })] // Deletablity explicitly set via annotation [InlineData(false, false, new OperationType[] { OperationType.Get})] public void CreateCollectionNavigationPropertyPathItemReturnsCorrectPathItem(bool containment, bool keySegment, OperationType[] expected) { + string annotation = + containment ? + "" : + @" + + + + +"; + // Arrange - IEdmModel model = GetEdmModel(""); + IEdmModel model = GetEdmModel(annotation: "", annotation2: annotation); ODataContext context = new ODataContext(model); IEdmEntitySet entitySet = model.EntityContainer.FindEntitySet("Customers"); Assert.NotNull(entitySet); // guard @@ -591,7 +601,7 @@ public void CreateNavigationPropertyPathItemAddsCustomAttributeValuesToPathExten Assert.Equal("true", isHiddenValue); } - public static IEdmModel GetEdmModel(string annotation) + public static IEdmModel GetEdmModel(string annotation, string annotation2 = "") { const string template = @" @@ -629,11 +639,14 @@ public static IEdmModel GetEdmModel(string annotation) {0} + + {1} + "; - string modelText = string.Format(template, annotation); + string modelText = string.Format(template, annotation, annotation2); IEdmModel model; IEnumerable errors; From 317d3767d16541ed95bb21baf02d20933e64606b Mon Sep 17 00:00:00 2001 From: Irvine Sunday Date: Wed, 31 Jan 2024 16:30:49 +0300 Subject: [PATCH 4/6] Updates release note --- .../Microsoft.OpenAPI.OData.Reader.csproj | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Microsoft.OpenApi.OData.Reader/Microsoft.OpenAPI.OData.Reader.csproj b/src/Microsoft.OpenApi.OData.Reader/Microsoft.OpenAPI.OData.Reader.csproj index fab1671d..153b01e6 100644 --- a/src/Microsoft.OpenApi.OData.Reader/Microsoft.OpenAPI.OData.Reader.csproj +++ b/src/Microsoft.OpenApi.OData.Reader/Microsoft.OpenAPI.OData.Reader.csproj @@ -15,7 +15,7 @@ netstandard2.0 Microsoft.OpenApi.OData true - 1.6.0-preview.4 + 1.6.0-preview.5 This package contains the codes you need to convert OData CSDL to Open API Document of Model. © Microsoft Corporation. All rights reserved. Microsoft OpenApi OData EDM @@ -25,6 +25,7 @@ - Updates the format of the request body schema of a collection of complex property #423 - Adds a delete operation and a required @id query parameter to collection-valued navigation property paths with $ref #453 - Fixes inconsistency of nullability of schemas of properties that are a collection of structured types #467 +- Adds delete operation for non-contained navigation properties only if explicitly allowed via annotation #483 Microsoft.OpenApi.OData.Reader ..\..\tool\Microsoft.OpenApi.OData.snk From b3b0831c083590a35360012c0363501e957558e4 Mon Sep 17 00:00:00 2001 From: Irvine Sunday Date: Thu, 1 Feb 2024 12:44:16 +0300 Subject: [PATCH 5/6] PR feedback --- .../PathItem/NavigationPropertyPathItemHandler.cs | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/src/Microsoft.OpenApi.OData.Reader/PathItem/NavigationPropertyPathItemHandler.cs b/src/Microsoft.OpenApi.OData.Reader/PathItem/NavigationPropertyPathItemHandler.cs index b9673974..9595f4ee 100644 --- a/src/Microsoft.OpenApi.OData.Reader/PathItem/NavigationPropertyPathItemHandler.cs +++ b/src/Microsoft.OpenApi.OData.Reader/PathItem/NavigationPropertyPathItemHandler.cs @@ -213,16 +213,13 @@ private void AddDeleteOperation(OpenApiPathItem item, NavigationPropertyRestrict AddOperation(item, OperationType.Delete); } } - else - { - // Add delete operation for non-contained nav. props only if explicitly set to true via annotation - // Note: Use Deletable and not IsDeletable - if ((navPropDeleteRestriction?.Deletable ?? false) && + else if ((navPropDeleteRestriction?.Deletable ?? false) && (NavigationProperty.TargetMultiplicity() != EdmMultiplicity.Many || - LastSegmentIsKeySegment)) - { - AddOperation(item, OperationType.Delete); - } + LastSegmentIsKeySegment)) + { + // Add delete operation for non-contained nav. props only if explicitly set to true via annotation + // Note: Use Deletable and not IsDeletable + AddOperation(item, OperationType.Delete); } return; From 0ddf800e67a0dd4cdc72dc519874143989b7a323 Mon Sep 17 00:00:00 2001 From: Irvine Sunday Date: Thu, 1 Feb 2024 17:22:33 +0300 Subject: [PATCH 6/6] PR suggestion; simplify conditional statements --- .../NavigationPropertyPathItemHandler.cs | 27 +++++++++---------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/src/Microsoft.OpenApi.OData.Reader/PathItem/NavigationPropertyPathItemHandler.cs b/src/Microsoft.OpenApi.OData.Reader/PathItem/NavigationPropertyPathItemHandler.cs index 9595f4ee..37084b61 100644 --- a/src/Microsoft.OpenApi.OData.Reader/PathItem/NavigationPropertyPathItemHandler.cs +++ b/src/Microsoft.OpenApi.OData.Reader/PathItem/NavigationPropertyPathItemHandler.cs @@ -199,26 +199,23 @@ private void AddDeleteOperation(OpenApiPathItem item, NavigationPropertyRestrict DeleteRestrictionsType navPropDeleteRestriction = restriction?.DeleteRestrictions ?? Context.Model.GetRecord(NavigationProperty); - if (NavigationProperty.ContainsTarget) - { - DeleteRestrictionsType entityDeleteRestriction = Context.Model.GetRecord(_navPropEntityType); - bool isDeletableDefault = navPropDeleteRestriction == null && entityDeleteRestriction == null; + if (!(NavigationProperty.TargetMultiplicity() != EdmMultiplicity.Many || LastSegmentIsKeySegment)) + return; - if ((isDeletableDefault || + DeleteRestrictionsType entityDeleteRestriction = Context.Model.GetRecord(_navPropEntityType); + bool isDeletable = + (navPropDeleteRestriction == null && entityDeleteRestriction == null) || ((entityDeleteRestriction?.IsDeletable ?? true) && - (navPropDeleteRestriction?.IsDeletable ?? true))) && - (NavigationProperty.TargetMultiplicity() != EdmMultiplicity.Many || - LastSegmentIsKeySegment)) - { - AddOperation(item, OperationType.Delete); - } + (navPropDeleteRestriction?.IsDeletable ?? true)); + + if (NavigationProperty.ContainsTarget && isDeletable) + { + AddOperation(item, OperationType.Delete); } - else if ((navPropDeleteRestriction?.Deletable ?? false) && - (NavigationProperty.TargetMultiplicity() != EdmMultiplicity.Many || - LastSegmentIsKeySegment)) + else if (navPropDeleteRestriction?.Deletable ?? false) { // Add delete operation for non-contained nav. props only if explicitly set to true via annotation - // Note: Use Deletable and not IsDeletable + // Note: Use Deletable and NOT IsDeletable AddOperation(item, OperationType.Delete); }