Ensures unique generation of operation ids for paths with composable …

…overloaded functions where all functions in path are overloaded (#595) * Ensure unique operation ids for composable functions with both overloaded * Update test to validate fix * Update release notes
microsoft · Oct 11, 2024 · 78f6629 · 78f6629
1 parent b919ea8
commit 78f6629
Show file tree

Hide file tree

Showing 3 changed files with 60 additions and 23 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
@@ -15,7 +15,7 @@
     <TargetFrameworks>net8.0</TargetFrameworks>
     <PackageId>Microsoft.OpenApi.OData</PackageId>
     <SignAssembly>true</SignAssembly>
-    <Version>2.0.0-preview.4</Version>
+    <Version>2.0.0-preview.5</Version>
     <Description>This package contains the codes you need to convert OData CSDL to Open API Document of Model.</Description>
     <Copyright>© Microsoft Corporation. All rights reserved.</Copyright>
     <PackageTags>Microsoft OpenApi OData EDM</PackageTags>
@@ -28,8 +28,9 @@
 		- Adds nullable to double schema conversions #581
 		- Updates tag names for actions/functions operations #585
 		- Creates unique operation ids for paths with composable overloaded functions #580
-    - Further fixes for double/decimal/float schema conversions #581
-    - Replaced integer types by number types
+        - Further fixes for double/decimal/float schema conversions #581
+        - Replaced integer types by number types
+		- Further fix for generating unique operation ids for paths with composable overloaded functions where all functions in path are overloaded #594
 	</PackageReleaseNotes>
     <AssemblyName>Microsoft.OpenApi.OData.Reader</AssemblyName>
     <AssemblyOriginatorKeyFile>..\..\tool\Microsoft.OpenApi.OData.snk</AssemblyOriginatorKeyFile>

diff --git a/src/Microsoft.OpenApi.OData.Reader/Operation/EdmOperationOperationHandler.cs b/src/Microsoft.OpenApi.OData.Reader/Operation/EdmOperationOperationHandler.cs
@@ -107,7 +107,9 @@ protected override void SetBasicInfo(OpenApiOperation operation)
                         if (opSegment.Operation is IEdmFunction function && Context.Model.IsOperationOverload(function))
                         {
                             // Hash the segment to avoid duplicate operationIds
-                            pathHash = segment.GetPathHash(Context.Settings);
+                            pathHash = string.IsNullOrEmpty(pathHash)
+                                ? opSegment.GetPathHash(Context.Settings)
+                                : (pathHash + opSegment.GetPathHash(Context.Settings)).GetHashSHA256()[..4];
                         }
 
                         identifiers.Add(segment.Identifier);

diff --git a/test/Microsoft.OpenAPI.OData.Reader.Tests/Operation/EdmFunctionOperationHandlerTests.cs b/test/Microsoft.OpenAPI.OData.Reader.Tests/Operation/EdmFunctionOperationHandlerTests.cs
@@ -310,22 +310,29 @@ public void CreateOperationForComposableOverloadEdmFunctionReturnsCorrectOperati
             customer.AddKeys(customer.AddStructuralProperty("ID", EdmPrimitiveTypeKind.Int32));
             model.AddElement(customer);
 
-            EdmFunction function = new("NS", "MyFunction", EdmCoreModel.Instance.GetString(false), true, null, false);
-            function.AddParameter("entity", new EdmEntityTypeReference(customer, false));
-            function.AddParameter("param", EdmCoreModel.Instance.GetString(false));
-            model.AddElement(function);
+            // Overloaded function 1 
+            EdmFunction function1 = new("NS", "MyFunction1", EdmCoreModel.Instance.GetString(false), true, null, false);
+            function1.AddParameter("entity", new EdmEntityTypeReference(customer, false));
+            model.AddElement(function1);
 
-            function = new EdmFunction("NS", "MyFunction", EdmCoreModel.Instance.GetString(false), true, null, false);
-            function.AddParameter("entity", new EdmEntityTypeReference(customer, false));
-            function.AddParameter("param", EdmCoreModel.Instance.GetString(false));
-            function.AddParameter("param2", EdmCoreModel.Instance.GetString(false));
-            model.AddElement(function);
+            // Overloaded function 1
+            EdmFunction function2 = new("NS", "MyFunction1", EdmCoreModel.Instance.GetString(false), true, null, false);
+            function2.AddParameter("entity", new EdmEntityTypeReference(customer, false));
+            function2.AddParameter("param", EdmCoreModel.Instance.GetString(false));
 
-            EdmFunction function2 = new("NS", "MyFunction2", EdmCoreModel.Instance.GetString(false), true, null, false);
-            function2.AddParameter("entity2", new EdmEntityTypeReference(customer, false));
-            function2.AddParameter("param3", EdmCoreModel.Instance.GetString(false));
             model.AddElement(function2);
 
+            // Overloaded function 2
+            EdmFunction function3 = new("NS", "MyFunction2", EdmCoreModel.Instance.GetString(false), true, null, false);
+            function3.AddParameter("entity2", new EdmEntityTypeReference(customer, false));
+            model.AddElement(function3);
+
+            // Overloaded function 2
+            EdmFunction function4 = new("NS", "MyFunction2", EdmCoreModel.Instance.GetString(false), true, null, false);
+            function4.AddParameter("entity2", new EdmEntityTypeReference(customer, false));
+            function4.AddParameter("param", EdmCoreModel.Instance.GetString(false));
+            model.AddElement(function4);
+
             EdmEntityContainer container = new("NS", "Default");
             EdmEntitySet customers = new(container, "Customers", customer);
             model.AddElement(container);
@@ -337,24 +344,51 @@ public void CreateOperationForComposableOverloadEdmFunctionReturnsCorrectOperati
             };
             ODataContext context = new(model, settings);
 
-            ODataPath path = new(new ODataNavigationSourceSegment(customers),
+            ODataPath path1 = new(new ODataNavigationSourceSegment(customers),
                 new ODataKeySegment(customer),
-                new ODataOperationSegment(function),
-                new ODataOperationSegment(function2));
+                new ODataOperationSegment(function1),
+                new ODataOperationSegment(function3));
+
+            ODataPath path2 = new(new ODataNavigationSourceSegment(customers),
+                new ODataKeySegment(customer),
+                new ODataOperationSegment(function1),
+                new ODataOperationSegment(function4));
+
+            ODataPath path3 = new(new ODataNavigationSourceSegment(customers),
+                new ODataKeySegment(customer),
+                new ODataOperationSegment(function2),
+                new ODataOperationSegment(function3));
+
+            ODataPath path4 = new(new ODataNavigationSourceSegment(customers),
+                new ODataKeySegment(customer),
+                new ODataOperationSegment(function2),
+                new ODataOperationSegment(function4));
 
             // Act
-            var operation = _operationHandler.CreateOperation(context, path);
+            var operation1 = _operationHandler.CreateOperation(context, path1);
+            var operation2 = _operationHandler.CreateOperation(context, path2);
+            var operation3 = _operationHandler.CreateOperation(context, path3);
+            var operation4 = _operationHandler.CreateOperation(context, path4);
 
             // Assert
-            Assert.NotNull(operation);
+            Assert.NotNull(operation1);
+            Assert.NotNull(operation2);
+            Assert.NotNull(operation3);
+            Assert.NotNull(operation4);
 
             if (enableOperationId)
             {
-                Assert.Equal("Customers.Customer.MyFunction.MyFunction2-df74", operation.OperationId);
+                Assert.Equal("Customers.Customer.MyFunction1.MyFunction2-c53d", operation1.OperationId);
+                Assert.Equal("Customers.Customer.MyFunction1.MyFunction2-4d93", operation2.OperationId);
+                Assert.Equal("Customers.Customer.MyFunction1.MyFunction2-a2b2", operation3.OperationId);
+                Assert.Equal("Customers.Customer.MyFunction1.MyFunction2-7bea", operation4.OperationId);
             }
             else
             {
-                Assert.Null(operation.OperationId);
+                Assert.Null(operation1.OperationId);
+                Assert.Null(operation2.OperationId);
+                Assert.Null(operation3.OperationId);
+                Assert.Null(operation4.OperationId);
             }
         }