Fixes potential duplicate operation ids of navigation property paths with overloaded composable functions (#597)

* Fixes generation of navigation property op ids with overloaded composable fxns

* Adds validation unit test

* Updates release note

* chore: adds string comparison

Co-authored-by: Andrew Omondi <[email protected]>

---------

Co-authored-by: Vincent Biret <[email protected]>
Co-authored-by: Andrew Omondi <[email protected]>

Author: 3 people

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

@@ -26,6 +26,7 @@
 	- 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
     <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/Microsoft.OpenAPI.OData.Reader.csproj

@@ -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/ComplexPropertyGetOperationHandler.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/ComplexPropertyPostOperationHandler.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/DollarCountGetOperationHandler.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/NavigationPropertyOperationHandler.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);
 	}

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

@@ -131,6 +131,70 @@ public void CreateNavigationGetOperationViaComposableFunctionReturnsCorrectOpera
             Assert.Contains(operation.Parameters, x => x.Name == "path");
         }
 
+        [Fact]
+        public void CreateNavigationGetOperationViaOverloadedComposableFunctionReturnsCorrectOperation()
+        {
+            // Arrange
+            IEdmModel model = EdmModelHelper.GraphBetaModel;
+            ODataContext context = new(model, new OpenApiConvertSettings()
+            {
+                EnableOperationId = true
+            });
+
+            IEdmEntitySet drives = model.EntityContainer.FindEntitySet("drives");
+            IEdmEntityType drive = model.SchemaElements.OfType<IEdmEntityType>().First(c => c.Name == "drive");
+            IEdmNavigationProperty items = drive.DeclaredNavigationProperties().First(c => c.Name == "items");
+            IEdmEntityType driveItem = model.SchemaElements.OfType<IEdmEntityType>().First(c => c.Name == "driveItem");
+            IEdmNavigationProperty workbook = driveItem.DeclaredNavigationProperties().First(c => c.Name == "workbook");
+            IEdmEntityType workbookEntity = model.SchemaElements.OfType<IEdmEntityType>().First(c => c.Name == "workbook");
+            IEdmNavigationProperty worksheets = workbookEntity.DeclaredNavigationProperties().First(c => c.Name == "worksheets");
+            IEdmEntityType workbookWorksheet = model.SchemaElements.OfType<IEdmEntityType>().First(c => c.Name == "workbookWorksheet");
+            IEdmOperation usedRangeWithParams = model.SchemaElements.OfType<IEdmOperation>().First(f => f.Name == "usedRange" && f.Parameters.Any(x => x.Name.Equals("valuesOnly")));
+            IEdmOperation usedRange = model.SchemaElements.OfType<IEdmOperation>().First(f => f.Name == "usedRange" && f.Parameters.Count() == 1);
+            IEdmEntityType workbookRange = model.SchemaElements.OfType<IEdmEntityType>().First(c => c.Name == "workbookRange");
+            IEdmNavigationProperty format = workbookRange.DeclaredNavigationProperties().First(c => c.Name == "format");
+
+
+            ODataPath path1 = new(new ODataNavigationSourceSegment(drives),
+                new ODataKeySegment(drive),
+                new ODataNavigationPropertySegment(items),
+                new ODataKeySegment(driveItem),
+                new ODataNavigationPropertySegment(workbook),
+                new ODataNavigationPropertySegment(worksheets),
+                new ODataKeySegment(workbookWorksheet),
+                new ODataOperationSegment(usedRangeWithParams),
+                new ODataNavigationPropertySegment(format));
+
+            ODataPath path2 = new(new ODataNavigationSourceSegment(drives),
+                new ODataKeySegment(drive),
+                new ODataNavigationPropertySegment(items),
+                new ODataKeySegment(driveItem),
+                new ODataNavigationPropertySegment(workbook),
+                new ODataNavigationPropertySegment(worksheets),
+                new ODataKeySegment(workbookWorksheet),
+                new ODataOperationSegment(usedRange),
+                new ODataNavigationPropertySegment(format));
+
+            // Act
+            var operation1 = _operationHandler.CreateOperation(context, path1);
+            var operation2 = _operationHandler.CreateOperation(context, path2);
+
+            // Assert
+            Assert.NotNull(operation1);
+            Assert.NotNull(operation2);
+
+            Assert.Equal("drives.items.workbook.worksheets.usedRange.GetFormat-206d", operation1.OperationId);
+            Assert.Equal("drives.items.workbook.worksheets.usedRange.GetFormat-ec2c", operation2.OperationId);
+
+            Assert.NotNull(operation1.Parameters);
+            Assert.Equal(6, operation1.Parameters.Count);
+            Assert.Contains(operation1.Parameters, x => x.Name == "valuesOnly");
+
+            Assert.NotNull(operation2.Parameters);
+            Assert.Equal(5, operation2.Parameters.Count);
+            Assert.DoesNotContain(operation2.Parameters, x => x.Name == "valuesOnly");
+        }
+
         [Theory]
         [InlineData(true)]
         [InlineData(false)]