Merge pull request #645 from microsoft/andrueastman/fixRegression

Fix duplicate operation ids in overloads that are composable.
microsoft · Jan 22, 2025 · 145b584 · 145b584
2 parents b6c9e17 + 1aa8233
commit 145b584
Show file tree

Hide file tree

Showing 3 changed files with 35 additions and 21 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,13 +15,13 @@
     <TargetFrameworks>netstandard2.0</TargetFrameworks>
     <PackageId>Microsoft.OpenApi.OData</PackageId>
     <SignAssembly>true</SignAssembly>
-    <Version>1.7.2</Version>
+    <Version>1.7.3</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>
     <RepositoryUrl>https://github.com/Microsoft/OpenAPI.NET.OData</RepositoryUrl>
     <PackageReleaseNotes>
-    - Adds action/function suffix to tag names for actions/functions operations in #642
+    - Fix regression in unique operation id generation at #462.
     </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
@@ -81,6 +81,7 @@ protected override void SetBasicInfo(OpenApiOperation operation)
                 // duplicates in entity vs entityset functions/actions
 
                 List<string> identifiers = new();
+                string pathHash = string.Empty;
                 foreach (ODataSegment segment in Path.Segments)
                 {
                     if (segment is ODataKeySegment keySegment)
@@ -101,6 +102,18 @@ protected override void SetBasicInfo(OpenApiOperation operation)
                             identifiers.Add(keySegment.Identifier);
                         }
                     }
+                    else if (segment is ODataOperationSegment opSegment) 
+                    { 
+                        if (opSegment.Operation is IEdmFunction function && Context.Model.IsOperationOverload(function)) 
+                        { 
+                            // Hash the segment to avoid duplicate operationIds 
+                            pathHash = string.IsNullOrEmpty(pathHash) 
+                                ? opSegment.GetPathHash(Context.Settings) 
+                                : (pathHash + opSegment.GetPathHash(Context.Settings)).GetHashSHA256().Substring(0, 4); 
+                        } 
+
+                        identifiers.Add(segment.Identifier); 
+                    }
                     else
                     {
                         identifiers.Add(segment.Identifier);
@@ -109,21 +122,13 @@ protected override void SetBasicInfo(OpenApiOperation operation)
 
                 string operationId = string.Join(".", identifiers);
 
-                if (EdmOperation.IsAction())
-                {
-                    operation.OperationId = operationId;
-                }
-                else
-                {
-                    if (Path.LastSegment is ODataOperationSegment operationSegment &&
-                        Context.Model.IsOperationOverload(operationSegment.Operation))
-                    {
-                        operation.OperationId = operationId + "-" + Path.LastSegment.GetPathHash(Context.Settings);
-                    }
-                    else
-                    {
-                        operation.OperationId = operationId;
-                    }
+                if (!string.IsNullOrEmpty(pathHash)) 
+                { 
+                    operation.OperationId = operationId + "-" + pathHash; 
+                } 
+                else 
+                { 
+                    operation.OperationId = operationId; 
                 }
             }
 

diff --git a/test/Microsoft.OpenAPI.OData.Reader.Tests/Operation/EdmFunctionOperationHandlerTests.cs b/test/Microsoft.OpenAPI.OData.Reader.Tests/Operation/EdmFunctionOperationHandlerTests.cs
@@ -3,6 +3,7 @@
 //  Licensed under the MIT License (MIT). See LICENSE in the repo root for license information.
 // ------------------------------------------------------------
 
+using System;
 using System.Collections.Generic;
 using System.Linq;
 using System.Xml.Linq;
@@ -378,10 +379,18 @@ public void CreateOperationForComposableOverloadEdmFunctionReturnsCorrectOperati
 
             if (enableOperationId)
             {
-                Assert.Equal("Customers.Customer.MyFunction1.MyFunction2-6b6d", operation1.OperationId);
-                Assert.Equal("Customers.Customer.MyFunction1.MyFunction2-2636", operation2.OperationId);
-                Assert.Equal("Customers.Customer.MyFunction1.MyFunction2-6b6d", operation3.OperationId);
-                Assert.Equal("Customers.Customer.MyFunction1.MyFunction2-2636", operation4.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);
+                var operationIds = new HashSet<string>(StringComparer.OrdinalIgnoreCase)
+                {
+                    operation1.OperationId, 
+                    operation2.OperationId, 
+                    operation3.OperationId, 
+                    operation4.OperationId
+                };
+                Assert.Equal(4, operationIds.Count);// All are unique as the hashset size is unchanged!
             }
             else
             {