Fixes potential duplicate operation ids of navigation property paths …

…with overloaded composable functions (#597)

src/Microsoft.OpenApi.OData.Reader/Common/EdmModelHelper.cs

@@ -6,6 +6,7 @@
 using System;
 using System.Collections.Generic;
 using System.Linq;
+using System.Security.Cryptography.X509Certificates;
 using Microsoft.OData.Edm;
 using Microsoft.OData.Edm.Csdl;
 using Microsoft.OData.Edm.Vocabularies;
@@ -93,38 +94,40 @@ internal static bool NavigationRestrictionsAllowsNavigability(
         /// Generates the operation id from a navigation property path.
         /// </summary>
         /// <param name="path">The target <see cref="ODataPath"/>.</param>
+        /// <param name="context">The OData context.</param>
         /// <param name="prefix">Optional: Identifier indicating whether it is a collection-valued non-indexed or single-valued navigation property.</param>
         /// <returns>The operation id generated from the given navigation property path.</returns>
-        internal static string GenerateNavigationPropertyPathOperationId(ODataPath path, string prefix = null)
+        internal static string GenerateNavigationPropertyPathOperationId(ODataPath path, ODataContext context, string prefix = null)
         {
-            IList<string> items = RetrieveNavigationPropertyPathsOperationIdSegments(path);
+            IList<string> items = RetrieveNavigationPropertyPathsOperationIdSegments(path, context);
 
             if (!items.Any())
                 return null;
 
-            int lastItemIndex = items.Count - 1;
+            int lastItemIndex = items[items.Count-1].StartsWith("-") ? items.Count - 2 : items.Count - 1;
 
             if (!string.IsNullOrEmpty(prefix))
             {
-                items[lastItemIndex] = prefix + Utils.UpperFirstChar(items.Last());
+                items[lastItemIndex] = prefix + Utils.UpperFirstChar(items[lastItemIndex]);
             }
             else
             {
-                items[lastItemIndex] = Utils.UpperFirstChar(items.Last());
+                items[lastItemIndex] = Utils.UpperFirstChar(items[lastItemIndex]);
             }
 
-            return string.Join(".", items);
+            return GenerateNavigationPropertyPathOperationId(items);
         }
 
         /// <summary>
         /// Generates the operation id from a complex property path.
         /// </summary>
         /// <param name="path">The target <see cref="ODataPath"/>.</param>
+        /// <param name="context">The OData context.</param>
         /// <param name="prefix">Optional: Identifier indicating whether it is a collection-valued or single-valued complex property.</param>
         /// <returns>The operation id generated from the given complex property path.</returns>
-        internal static string GenerateComplexPropertyPathOperationId(ODataPath path, string prefix = null)
+        internal static string GenerateComplexPropertyPathOperationId(ODataPath path, ODataContext context, string prefix = null)
         {
-            IList<string> items = RetrieveNavigationPropertyPathsOperationIdSegments(path);
+            IList<string> items = RetrieveNavigationPropertyPathsOperationIdSegments(path, context);
 
             if (!items.Any())
                 return null;
@@ -141,15 +144,29 @@ internal static string GenerateComplexPropertyPathOperationId(ODataPath path, st
                 items.Add(Utils.UpperFirstChar(lastSegment?.Identifier));
             }
 
-            return string.Join(".", items);
+            return GenerateNavigationPropertyPathOperationId(items);
+        }
+
+        /// <summary>
+        /// Generates a navigation property operation id from a list of string values.
+        /// </summary>
+        /// <param name="items">The list of string values.</param>
+        /// <returns>The generated navigation property operation id.</returns>
+        private static string GenerateNavigationPropertyPathOperationId(IList<string> items)
+        {
+            if (!items.Any())
+                return null;
+
+            return string.Join(".", items).Replace(".-", "-"); // Format any hashed value appropriately (this will be the last value)
         }
 
         /// <summary>
         /// Retrieves the segments of an operation id generated from a navigation property path.
         /// </summary>
         /// <param name="path">The target <see cref="ODataPath"/>.</param>
+        /// <param name="context">The OData context.</param>
         /// <returns>The segments of an operation id generated from the given navigation property path.</returns>
-        internal static IList<string> RetrieveNavigationPropertyPathsOperationIdSegments(ODataPath path)
+        internal static IList<string> RetrieveNavigationPropertyPathsOperationIdSegments(ODataPath path, ODataContext context)
         {
             Utils.CheckArgumentNull(path, nameof(path));
 
@@ -173,6 +190,8 @@ s is ODataOperationSegment ||
             Utils.CheckArgumentNull(segments, nameof(segments));
 
             string previousTypeCastSegmentId = null;
+            string pathHash = string.Empty;
+
             foreach (var segment in segments)
             {
                 if (segment is ODataNavigationPropertySegment navPropSegment)
@@ -189,25 +208,38 @@ s is ODataOperationSegment ||
                     previousTypeCastSegmentId = "As" + Utils.UpperFirstChar(schemaElement.Name);
                     items.Add(previousTypeCastSegmentId);
                 }
-                else if (segment is ODataOperationSegment operationSegment)
-                {
-                    // Navigation property generated via composable function
-                    items.Add(operationSegment.Identifier);
+                else if (segment is ODataOperationSegment operationSegment)
+                {
+                    // Navigation property generated via composable function
+                    if (operationSegment.Operation is IEdmFunction function && context.Model.IsOperationOverload(function))
+                    {
+                        // Hash the segment to avoid duplicate operationIds
+                        pathHash = string.IsNullOrEmpty(pathHash)
+                            ? operationSegment.GetPathHash(context.Settings)
+                            : (pathHash + operationSegment.GetPathHash(context.Settings)).GetHashSHA256().Substring(0,4);
+                    }
+
+                    items.Add(operationSegment.Identifier);
                 }
-                else if (segment is ODataKeySegment keySegment && keySegment.IsAlternateKey)
-                {
-                    // We'll consider alternate keys in the operation id to eliminate potential duplicates with operation id of primary path                    
-                    if (segment == segments.Last())
-                    {                        
-                        items.Add("By" + string.Join("", keySegment.Identifier.Split(',').Select(static x => Utils.UpperFirstChar(x))));
-                    }
-                    else
-                    {
-                        items.Add(keySegment.Identifier);
-                    }
+                else if (segment is ODataKeySegment keySegment && keySegment.IsAlternateKey)
+                {
+                    // We'll consider alternate keys in the operation id to eliminate potential duplicates with operation id of primary path                    
+                    if (segment == segments.Last())
+                    {                        
+                        items.Add("By" + string.Join("", keySegment.Identifier.Split(',').Select(static x => Utils.UpperFirstChar(x))));
+                    }
+                    else
+                    {
+                        items.Add(keySegment.Identifier);
+                    }
                 }
             }
 
+            if (!string.IsNullOrEmpty(pathHash))
+            {
+                items.Add("-" + pathHash);
+            }
+
             return items;
         }
 
@@ -320,9 +352,10 @@ internal static string GenerateComplexPropertyPathTagName(ODataPath path, ODataC
         /// Generates the operation id prefix from an OData type cast path.
         /// </summary>
         /// <param name="path">The target <see cref="ODataPath"/>.</param>
+        /// <param name="context">The OData context.</param>
         /// <param name="includeListOrGetPrefix">Optional: Whether to include the List or Get prefix to the generated operation id.</param>
         /// <returns>The operation id prefix generated from the OData type cast path.</returns>
-        internal static string GenerateODataTypeCastPathOperationIdPrefix(ODataPath path, bool includeListOrGetPrefix = true)
+        internal static string GenerateODataTypeCastPathOperationIdPrefix(ODataPath path, ODataContext context, bool includeListOrGetPrefix = true)
         {
             // Get the segment before the last OData type cast segment
             ODataTypeCastSegment typeCastSegment = path.Segments.OfType<ODataTypeCastSegment>()?.Last();
@@ -352,7 +385,7 @@ internal static string GenerateODataTypeCastPathOperationIdPrefix(ODataPath path
             if (secondLastSegment is ODataComplexPropertySegment complexSegment)
             {
                 string listOrGet = includeListOrGetPrefix ? (complexSegment.Property.Type.IsCollection() ? "List" : "Get") : null;
-                operationId = GenerateComplexPropertyPathOperationId(path, listOrGet);
+                operationId = GenerateComplexPropertyPathOperationId(path, context, listOrGet);
             }
             else if (secondLastSegment is ODataNavigationPropertySegment navPropSegment)
             {
@@ -362,27 +395,27 @@ internal static string GenerateODataTypeCastPathOperationIdPrefix(ODataPath path
                     prefix = navPropSegment?.NavigationProperty.TargetMultiplicity() == EdmMultiplicity.Many ? "List" : "Get";
                 }
 
-                operationId = GenerateNavigationPropertyPathOperationId(path, prefix);
+                operationId = GenerateNavigationPropertyPathOperationId(path, context, prefix);
             }
             else if (secondLastSegment is ODataKeySegment keySegment)
             {
-                if (isIndexedCollValuedNavProp)
-                {
-                    operationId = GenerateNavigationPropertyPathOperationId(path, "Get");
+                if (isIndexedCollValuedNavProp)
+                {
+                    operationId = GenerateNavigationPropertyPathOperationId(path, context, "Get");
+                }
+                else
+                {
+                    string entityTypeName = keySegment.EntityType.Name;
+                    string getPrefix = includeListOrGetPrefix ? "Get" : null;
+                    string operationName = $"{getPrefix}{Utils.UpperFirstChar(entityTypeName)}";
+                    if (keySegment.IsAlternateKey)
+                    {
+                        string alternateKeyName = string.Join("", keySegment.Identifier.Split(',').Select(static x => Utils.UpperFirstChar(x)));
+                        operationName = $"{operationName}By{alternateKeyName}";
+                    }
+                    operationId = (entitySet != null) ? entitySet.Name : singleton.Name;
+                    operationId += $".{entityTypeName}.{operationName}";
                 }
-                else
-                {
-                    string entityTypeName = keySegment.EntityType.Name;
-                    string getPrefix = includeListOrGetPrefix ? "Get" : null;
-                    string operationName = $"{getPrefix}{Utils.UpperFirstChar(entityTypeName)}";
-                    if (keySegment.IsAlternateKey)
-                    {
-                        string alternateKeyName = string.Join("", keySegment.Identifier.Split(',').Select(static x => Utils.UpperFirstChar(x)));
-                        operationName = $"{operationName}By{alternateKeyName}";
-                    }
-                    operationId = (entitySet != null) ? entitySet.Name : singleton.Name;
-                    operationId += $".{entityTypeName}.{operationName}";
-                }
             }
             else if (secondLastSegment is ODataNavigationSourceSegment)
             {

src/Microsoft.OpenApi.OData.Reader/Microsoft.OpenAPI.OData.Reader.csproj

@@ -15,17 +15,14 @@
     <TargetFrameworks>netstandard2.0</TargetFrameworks>
     <PackageId>Microsoft.OpenApi.OData</PackageId>
     <SignAssembly>true</SignAssembly>
-    <Version>1.7.0</Version>
+    <Version>1.7.1</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>
-    - Updates tag names for actions/functions operations #627
-    - Adds nullable to double schema conversion #628
-	- Updates PUT operation ID prefix from Update to Set #629
-    - Ensures unique operation IDs for composable functions #630
-	  </PackageReleaseNotes>
+    - Further fix for generating unique operation ids for navigation property paths with composable overloaded functions #596
+    </PackageReleaseNotes>
     <AssemblyName>Microsoft.OpenApi.OData.Reader</AssemblyName>
     <AssemblyOriginatorKeyFile>..\..\tool\Microsoft.OpenApi.OData.snk</AssemblyOriginatorKeyFile>
     <OutputPath Condition="'$(Configuration)|$(Platform)'=='Debug|AnyCPU'">..\..\bin\Debug\</OutputPath>

src/Microsoft.OpenApi.OData.Reader/Operation/ComplexPropertyGetOperationHandler.cs

@@ -39,7 +39,7 @@ protected override void SetBasicInfo(OpenApiOperation operation)
         if (Context.Settings.EnableOperationId)
         {
             string prefix = ComplexPropertySegment.Property.Type.IsCollection() ? "List" : "Get";
-            operation.OperationId = EdmModelHelper.GenerateComplexPropertyPathOperationId(Path, prefix);
+            operation.OperationId = EdmModelHelper.GenerateComplexPropertyPathOperationId(Path, Context, prefix);
         }
 
         // Summary and Description

src/Microsoft.OpenApi.OData.Reader/Operation/ComplexPropertyPostOperationHandler.cs

@@ -42,7 +42,7 @@ protected override void SetBasicInfo(OpenApiOperation operation)
         // OperationId
         if (Context.Settings.EnableOperationId)
         {
-            operation.OperationId = EdmModelHelper.GenerateComplexPropertyPathOperationId(Path, "Set");
+            operation.OperationId = EdmModelHelper.GenerateComplexPropertyPathOperationId(Path, Context, "Set");
         }
 
         // Summary and Description

src/Microsoft.OpenApi.OData.Reader/Operation/ComplexPropertyUpdateOperationHandler.cs

@@ -41,7 +41,7 @@ protected override void SetBasicInfo(OpenApiOperation operation)
         if (Context.Settings.EnableOperationId)
         {
             string prefix = OperationType == OperationType.Patch ? "Update" : "Set";
-            operation.OperationId = EdmModelHelper.GenerateComplexPropertyPathOperationId(Path, prefix);
+            operation.OperationId = EdmModelHelper.GenerateComplexPropertyPathOperationId(Path, Context, prefix);
         }
     }
 

src/Microsoft.OpenApi.OData.Reader/Operation/DollarCountGetOperationHandler.cs

@@ -128,17 +128,17 @@ protected override void SetBasicInfo(OpenApiOperation operation)
                 }
                 else if (SecondLastSegment is ODataNavigationPropertySegment)
                 {
-                    var navPropOpId = string.Join(".", EdmModelHelper.RetrieveNavigationPropertyPathsOperationIdSegments(Path));
+                    var navPropOpId = string.Join(".", EdmModelHelper.RetrieveNavigationPropertyPathsOperationIdSegments(Path, Context));
                     operation.OperationId = $"{navPropOpId}.GetCount-{Path.GetPathHash(Context.Settings)}";
                 }
                 else if (SecondLastSegment is ODataTypeCastSegment odataTypeCastSegment)
                 {
                     IEdmNamedElement targetStructuredType = odataTypeCastSegment.StructuredType as IEdmNamedElement;
-                    operation.OperationId = $"{EdmModelHelper.GenerateODataTypeCastPathOperationIdPrefix(Path, false)}.GetCount.As{Utils.UpperFirstChar(targetStructuredType.Name)}-{Path.GetPathHash(Context.Settings)}";
+                    operation.OperationId = $"{EdmModelHelper.GenerateODataTypeCastPathOperationIdPrefix(Path, Context, false)}.GetCount.As{Utils.UpperFirstChar(targetStructuredType.Name)}-{Path.GetPathHash(Context.Settings)}";
                 }
                 else if (SecondLastSegment is ODataComplexPropertySegment)
                 {
-                    operation.OperationId = $"{EdmModelHelper.GenerateComplexPropertyPathOperationId(Path)}.GetCount-{Path.GetPathHash(Context.Settings)}";
+                    operation.OperationId = $"{EdmModelHelper.GenerateComplexPropertyPathOperationId(Path, Context)}.GetCount-{Path.GetPathHash(Context.Settings)}";
                 }
             }
 

src/Microsoft.OpenApi.OData.Reader/Operation/NavigationPropertyOperationHandler.cs

@@ -107,7 +107,7 @@ protected override void SetExtensions(OpenApiOperation operation)
 
         internal string GetOperationId(string prefix = null)
         {            
-            return EdmModelHelper.GenerateNavigationPropertyPathOperationId(Path, prefix);
+            return EdmModelHelper.GenerateNavigationPropertyPathOperationId(Path, Context, prefix);
         }               
 
         /// <inheritdoc/>

src/Microsoft.OpenApi.OData.Reader/Operation/ODataTypeCastGetOperationHandler.cs

@@ -188,7 +188,7 @@ protected override void SetBasicInfo(OpenApiOperation operation)
 
         // OperationId
         if (Context.Settings.EnableOperationId)
-			operation.OperationId = EdmModelHelper.GenerateODataTypeCastPathOperationIdPrefix(Path) + $".As{Utils.UpperFirstChar(TargetSchemaElement.Name)}";
+			operation.OperationId = EdmModelHelper.GenerateODataTypeCastPathOperationIdPrefix(Path, Context) + $".As{Utils.UpperFirstChar(TargetSchemaElement.Name)}";
 
         base.SetBasicInfo(operation);
 	}