Repository: microsoft/kiota (csharp)

Issue:
[Go] method naming
We migrated from version 1.6 to version 1.13 and noticed some weird changes in the names of the methods.
In v1.6 a method was named like this:
`res, err := client.Groups().ByGroupIdGuid(idUUID).Get(ctx, nil)`

In v1.13 it is now named like this:
`res, err := client.Groups().ByGroupIdIdGuid(idUUID).Get(ctx, nil)`

I also tried v1.10 which named them like this:
`res, err := client.Groups().ByGroupsIdGuid(idUUID).Get(ctx, nil)`

The different client versions were all generated from the same open api definition.

We personally prefer the naming scheme from version 1.6 since the others are just missleading.

Pull request:
Fixes #4519

This PR is a follow up to https://github.com/microsoft/kiota/pull/4174 which deduplicates path parameters and adds the `-id` suffix to the indexers. However, in the event that the indexer ends up with the `id` suffix without the  `-`, downstream refiners will end up appending the two to result with duplicated `Id` suffix.

This PR therefore adds an attempt to cleanup the `id` suffix before appending the `-id` suffix so that an indexer like `groupId` will now be changed to `group-id` rather than `groupId-id`.

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index d29ee7d31b..5c58b0dd0e 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -16,6 +16,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 - Fixed a bug where TypeScript deserialization would fail on Uppercase properties.[#4479](https://github.com/microsoft/kiota/issues/4479)
 - Changed URI template generation to reuse templates when required templates are absent across operations.
+- Fixed path deduplication logic to avoid double `Id` suffixes in indexer names in scenarios where the `Id` suffix is already present.[#4519](https://github.com/microsoft/kiota/issues/4519) 
 - Updated reserved name providers for Java and Php so that "object" can be escaped.
 - Fixes request builder disambiguation when child nodes had different prefixes for different paths with same parameters.[#4448](https://github.com/microsoft/kiota/issues/4448) 
 - Do not generate CS8603 warnings when enabling backing store in CSharp generation.
diff --git a/src/Kiota.Builder/Extensions/OpenApiUrlTreeNodeExtensions.cs b/src/Kiota.Builder/Extensions/OpenApiUrlTreeNodeExtensions.cs
index c90136b87f..06dc242496 100644
--- a/src/Kiota.Builder/Extensions/OpenApiUrlTreeNodeExtensions.cs
+++ b/src/Kiota.Builder/Extensions/OpenApiUrlTreeNodeExtensions.cs
@@ -320,7 +320,7 @@ internal static void MergeIndexNodesAtSameLevel(this OpenApiUrlTreeNode node, IL
             node.Children.Remove(indexNode.Key);
             var oldSegmentName = indexNode.Value.Segment.Trim('{', '}').CleanupSymbolName();
             var segmentIndex = indexNode.Value.Path.Split('\\', StringSplitOptions.RemoveEmptyEntries).ToList().IndexOf(indexNode.Value.Segment);
-            var newSegmentParameterName = oldSegmentName.EndsWith("-id", StringComparison.OrdinalIgnoreCase) ? oldSegmentName : $"{{{oldSegmentName}-id}}";
+            var newSegmentParameterName = oldSegmentName.EndsWith("-id", StringComparison.OrdinalIgnoreCase) ? oldSegmentName : $"{{{oldSegmentName.TrimSuffix("id", StringComparison.OrdinalIgnoreCase)}-id}}";
             indexNode.Value.Path = indexNode.Value.Path.Replace(indexNode.Key, newSegmentParameterName, StringComparison.OrdinalIgnoreCase);
             indexNode.Value.AddDeduplicatedSegment(newSegmentParameterName);
             node.Children.Add(newSegmentParameterName, indexNode.Value);
diff --git a/src/Kiota.Builder/Extensions/StringExtensions.cs b/src/Kiota.Builder/Extensions/StringExtensions.cs
index 44738cec93..d220b073f1 100644
--- a/src/Kiota.Builder/Extensions/StringExtensions.cs
+++ b/src/Kiota.Builder/Extensions/StringExtensions.cs
@@ -290,4 +290,7 @@ public static string CleanupXMLString(this string? original)
     /// <returns></returns>
     public static bool EqualsIgnoreCase(this string? a, string? b)
         => string.Equals(a, b, StringComparison.OrdinalIgnoreCase);
+
+    public static string TrimSuffix(this string s, string suffix, StringComparison stringComparison = StringComparison.Ordinal) =>
+        !string.IsNullOrEmpty(s) && !string.IsNullOrEmpty(suffix) && s.EndsWith(suffix, stringComparison) ? s[..^suffix.Length] : s;
 }
diff --git a/src/Kiota.Builder/Writers/Go/StringExtensions.cs b/src/Kiota.Builder/Writers/Go/StringExtensions.cs
index d56a749cab..cf62649870 100644
--- a/src/Kiota.Builder/Writers/Go/StringExtensions.cs
+++ b/src/Kiota.Builder/Writers/Go/StringExtensions.cs
@@ -10,6 +10,4 @@ public static string TrimCollectionAndPointerSymbols(this string s) =>
     public static string TrimPackageReference(this string s) =>
         !string.IsNullOrEmpty(s) && s.Contains('.', StringComparison.InvariantCultureIgnoreCase) ? s.Split('.').Last() : s;
 
-    public static string TrimSuffix(this string s, string suffix) =>
-        !string.IsNullOrEmpty(s) && suffix != null && s.EndsWith(suffix, StringComparison.Ordinal) ? s[..^suffix.Length] : s;
 }


Test patch:
diff --git a/tests/Kiota.Builder.Tests/Extensions/OpenApiUrlTreeNodeExtensionsTests.cs b/tests/Kiota.Builder.Tests/Extensions/OpenApiUrlTreeNodeExtensionsTests.cs
index 99fa6e6662..e3ce0e266e 100644
--- a/tests/Kiota.Builder.Tests/Extensions/OpenApiUrlTreeNodeExtensionsTests.cs
+++ b/tests/Kiota.Builder.Tests/Extensions/OpenApiUrlTreeNodeExtensionsTests.cs
@@ -1068,9 +1068,9 @@ public void repro4085()
         Assert.Null(GetChildNodeByPath(resultNode, "{thingId}"));
         Assert.Null(GetChildNodeByPath(resultNode, "{differentThingId}"));
 
-        var differentThingId = GetChildNodeByPath(resultNode, "{differentThingId-id}");
-        Assert.Equal("\\path\\{differentThingId-id}", differentThingId.Path);
-        Assert.Equal("{+baseurl}/path/{differentThingId%2Did}", differentThingId.GetUrlTemplate());
+        var differentThingId = GetChildNodeByPath(resultNode, "{differentThing-id}");
+        Assert.Equal("\\path\\{differentThing-id}", differentThingId.Path);
+        Assert.Equal("{+baseurl}/path/{differentThing%2Did}", differentThingId.GetUrlTemplate());
     }
 
     [Theory]
diff --git a/tests/Kiota.Builder.Tests/KiotaBuilderTests.cs b/tests/Kiota.Builder.Tests/KiotaBuilderTests.cs
index 58fcb12a22..dd25c286ed 100644
--- a/tests/Kiota.Builder.Tests/KiotaBuilderTests.cs
+++ b/tests/Kiota.Builder.Tests/KiotaBuilderTests.cs
@@ -904,7 +904,7 @@ public async Task TrimsInheritanceUnusedModelsWithUnion()
         Assert.NotNull(modelsNS.FindChildByName<CodeClass>("EducationUser", false));
         Assert.Null(modelsNS.FindChildByName<CodeClass>("AuditEvent", false));
     }
-    private static async Task<Stream> GetDocumentStream(string document)
+    internal static async Task<Stream> GetDocumentStream(string document)
     {
         var ms = new MemoryStream();
         await using var tw = new StreamWriter(ms, Encoding.UTF8, leaveOpen: true);
diff --git a/tests/Kiota.Builder.Tests/Refiners/GoLanguageRefinerTests.cs b/tests/Kiota.Builder.Tests/Refiners/GoLanguageRefinerTests.cs
index 9857df54d6..170c31dafc 100644
--- a/tests/Kiota.Builder.Tests/Refiners/GoLanguageRefinerTests.cs
+++ b/tests/Kiota.Builder.Tests/Refiners/GoLanguageRefinerTests.cs
@@ -1,10 +1,13 @@
 ﻿using System;
+using System.IO;
 using System.Linq;
+using System.Net.Http;
 using System.Threading.Tasks;
 using Kiota.Builder.CodeDOM;
 using Kiota.Builder.Configuration;
 using Kiota.Builder.Refiners;
-
+using Microsoft.Extensions.Logging;
+using Moq;
 using Xunit;
 
 namespace Kiota.Builder.Tests.Refiners;
@@ -439,6 +442,86 @@ public async Task SupportsTypeSpecificOverrideIndexers()
         Assert.NotNull(model.Methods.SingleOrDefault(static x => x.IsOfKind(CodeMethodKind.IndexerBackwardCompatibility) && x.Name.Equals("ByIdInteger") && x.OriginalIndexer != null && x.OriginalIndexer.IndexParameter.Type.Name.Equals("Integer", StringComparison.OrdinalIgnoreCase)));
         Assert.NotNull(model.Methods.SingleOrDefault(static x => x.IsOfKind(CodeMethodKind.IndexerBackwardCompatibility) && x.Name.Equals("ById") && x.OriginalIndexer != null && x.OriginalIndexer.IndexParameter.Type.Name.Equals("string", StringComparison.OrdinalIgnoreCase)));
     }
+
+    [Fact]
+    public async Task ValidatesNamingOfRequestBuilderDoesNotRepeatIdCharacter()
+    {
+        var tempFilePath = Path.Combine(Path.GetTempPath(), Path.GetTempFileName());
+        await using var fs = await KiotaBuilderTests.GetDocumentStream(@"openapi: 3.0.1
+info:
+  title: OData Service for namespace microsoft.graph
+  description: This OData service is located at https://graph.microsoft.com/v1.0
+  version: 1.0.1
+servers:
+  - url: https://graph.microsoft.com/v1.0
+paths:
+  /groups/{id}:
+    get:
+      summary: Get Group by Id
+      operationId: groups_GetById
+      parameters:
+        - name: id
+          in: path
+          description: The id of the group
+          required : true
+          schema :
+            type: string
+            format: uuid
+      responses:
+        '200':
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/microsoft.graph.directoryObject'
+  /groups/{groupId}/member/{id}:
+    get:
+      summary: Get member by Id
+      operationId: member_GetById
+      parameters:
+        - name: groupId
+          in: path
+          description: The id of the group
+          required : true
+          schema :
+            type: string
+            format: uuid
+        - name: id
+          in: path
+          description: The id of the member
+          required : true
+          schema :
+            type: string
+            format: uuid
+      responses:
+        '200':
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/microsoft.graph.directoryObject'
+components:
+  schemas:
+    microsoft.graph.directoryObject:
+      title: directoryObject
+      type: object
+      properties:
+        deletedDateTime:
+          type: string
+          pattern: '^[0-9]{4,}-(0[1-9]|1[012])-(0[1-9]|[12][0-9]|3[01])T([01][0-9]|2[0-3]):[0-5][0-9]:[0-5][0-9]([.][0-9]{1,12})?(Z|[+-][0-9][0-9]:[0-9][0-9])$'
+          format: date-time
+          nullable: true");
+        var mockLogger = new Mock<ILogger<KiotaBuilder>>();
+        var builder = new KiotaBuilder(mockLogger.Object, new GenerationConfiguration { ClientClassName = "Graph", OpenAPIFilePath = tempFilePath }, new HttpClient());
+        var document = await builder.CreateOpenApiDocumentAsync(fs);
+        var node = builder.CreateUriSpace(document);
+        var codeModel = builder.CreateSourceModel(node);
+        await ILanguageRefiner.Refine(new GenerationConfiguration { Language = GenerationLanguage.Go }, codeModel);
+        var requestBuilder = codeModel.FindChildByName<CodeClass>("groupsRequestBuilder");
+        var indexerMethods = requestBuilder.Methods.Where(static method =>
+            method.IsOfKind(CodeMethodKind.IndexerBackwardCompatibility)).ToArray();
+        Assert.Equal(2, indexerMethods.Length);
+        Assert.Equal("ByGroupId", indexerMethods[0].Name);
+        Assert.Equal("ByGroupIdGuid", indexerMethods[1].Name);
+    }
     [Fact]
     public async Task AddsExceptionInheritanceOnErrorClasses()
     {