Adds delete operation for non-contained navigation properties only …

…if explicitly allowed via annotation (#484) * Fix convert settings text documentation * Ensure non-contained deletability set explicitly via annotation * Update test * Updates release note * PR feedback * PR suggestion; simplify conditional statements
microsoft · Feb 1, 2024 · b5e8145 · b5e8145
1 parent abe61ca
commit b5e8145
Show file tree

Hide file tree

Showing 4 changed files with 40 additions and 31 deletions.
diff --git a/src/Microsoft.OpenApi.OData.Reader/Microsoft.OpenAPI.OData.Reader.csproj b/src/Microsoft.OpenApi.OData.Reader/Microsoft.OpenAPI.OData.Reader.csproj
@@ -26,6 +26,7 @@
 - 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
 - Generates $expand query parameter for operations whose return type is a collection #481
+- Adds delete operation for non-contained navigation properties only if explicitly allowed via annotation #483
     </PackageReleaseNotes>
     <AssemblyName>Microsoft.OpenApi.OData.Reader</AssemblyName>
     <AssemblyOriginatorKeyFile>..\..\tool\Microsoft.OpenApi.OData.snk</AssemblyOriginatorKeyFile>

diff --git a/src/Microsoft.OpenApi.OData.Reader/OpenApiConvertSettings.cs b/src/Microsoft.OpenApi.OData.Reader/OpenApiConvertSettings.cs
@@ -247,7 +247,7 @@ public string PathPrefix
         /// <summary>
         /// Gets/Sets a value indicating whether or not to expand derived types to retrieve their declared navigation properties.
         /// </summary>
-        [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;
 
         /// <summary>
-        /// 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.
         /// </summary>
         public Dictionary<string, string> CustomXMLAttributesMapping { get; set; } = new();
 

diff --git 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,31 +197,26 @@ private void AddDeleteOperation(OpenApiPathItem item, NavigationPropertyRestrict
             Debug.Assert(!LastSegmentIsRefSegment);
 
             DeleteRestrictionsType navPropDeleteRestriction = restriction?.DeleteRestrictions ??
-               Context.Model.GetRecord<DeleteRestrictionsType>(NavigationProperty);
-
-            if (NavigationProperty.ContainsTarget)
-            {
-                DeleteRestrictionsType entityDeleteRestriction = Context.Model.GetRecord<DeleteRestrictionsType>(_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);
-                    }
-                }
+               Context.Model.GetRecord<DeleteRestrictionsType>(NavigationProperty);
+
+            if (!(NavigationProperty.TargetMultiplicity() != EdmMultiplicity.Many || LastSegmentIsKeySegment))
+                return;
+
+            DeleteRestrictionsType entityDeleteRestriction = Context.Model.GetRecord<DeleteRestrictionsType>(_navPropEntityType);
+            bool isDeletable = 
+                (navPropDeleteRestriction == null && entityDeleteRestriction == null) ||
+                ((entityDeleteRestriction?.IsDeletable ?? true) &&
+                (navPropDeleteRestriction?.IsDeletable ?? true));
+
+            if (NavigationProperty.ContainsTarget && isDeletable)
+            {
+                AddOperation(item, OperationType.Delete);
             }
-            else
-            {
-                if ((navPropDeleteRestriction?.IsDeletable ?? false) &&
-                    (NavigationProperty.TargetMultiplicity() != EdmMultiplicity.Many ||
-                    LastSegmentIsKeySegment))
-                {
-                    AddOperation(item, OperationType.Delete);
-                }
+            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
+                AddOperation(item, OperationType.Delete);
             }
 
             return;

diff --git 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 ? 
+                "" : 
+                @"
+<Annotation Term=""Org.OData.Capabilities.V1.DeleteRestrictions"">
+  <Record>
+    <PropertyValue Property=""Deletable"" Bool=""true"" />
+  </Record>
+</Annotation>";
+
             // 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 = @"<edmx:Edmx Version=""4.0"" xmlns:edmx=""http://docs.oasis-open.org/odata/ns/edmx"" xmlns:ags=""http://aggregator.microsoft.com/internal"">
   <edmx:DataServices>
@@ -629,11 +639,14 @@ public static IEdmModel GetEdmModel(string annotation)
       <Annotations Target=""NS.Default/Customers"">
         {0}
       </Annotations>
+      <Annotations Target=""NS.Customer/Orders"">
+        {1}
+      </Annotations>
     </Schema>
   </edmx:DataServices>
 </edmx:Edmx>";
 
-            string modelText = string.Format(template, annotation);
+            string modelText = string.Format(template, annotation, annotation2);
 
             IEdmModel model;
             IEnumerable<EdmError> errors;