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 description:
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()
     {