Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Go] method naming #4519

Closed
NerdJeremia opened this issue Apr 19, 2024 · 8 comments · Fixed by #4547
Closed

[Go] method naming #4519

NerdJeremia opened this issue Apr 19, 2024 · 8 comments · Fixed by #4547
Assignees
Labels
generator Issues or improvements relater to generation capabilities. type:bug A broken experience
Milestone

Comments

@NerdJeremia
Copy link

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.

@baywet
Copy link
Member

baywet commented Apr 19, 2024

Hi @NerdJeremia
Thanks for using kiota and for reaching out.
So the change was introduced between 1.11 and 1.13 (inclusive).
Can I ask you to try v1.11, 1.12 so we can narrow done which change might have introduced this please?

Also can you provide the OAS description for that path segment and its parameter please ?(I'm suspecting it's type string, format guid, but I'd like to confirm that)

@baywet baywet added question Further information is requested Go Needs: Author Feedback labels Apr 19, 2024
@baywet baywet added this to the Backlog milestone Apr 19, 2024
@NerdJeremia
Copy link
Author

Hi @baywet
Thank you for the fast response.
In v1.12 its exactly the same and in v1.11 the method is named like this: client.Groups().ByGroupsIdGuid().
As for the the OAS desc it is as follows:

"/groups/{id}": {
      "get": {
        "tags": [
          "Groups"
        ],
        "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"
            }
          }

@baywet
Copy link
Member

baywet commented Apr 22, 2024

Thanks for the additional information.
So it'd be fair to say the issue was introduced by 1.13
@NerdJeremia are you seeing this issue with other languages than go? (java for instance)
@andrueastman can you have a look at this when you have a couple of minutes since you're already looking at naming issues with #4517 please?

@baywet baywet added type:bug A broken experience help wanted Issue caused by core project dependency modules or library and removed question Further information is requested Needs: Attention 👋 labels Apr 22, 2024
@NerdJeremia
Copy link
Author

NerdJeremia commented Apr 22, 2024

We are not currently using any other client languages than go so I unfortunately can't say. Yes this issue was apparently introduced in v1.13. However, versions > v1.9 renamed from ByGroupIdGuid to ByGroupsIdGuid which we think also isn't ideal naming.

@andrueastman
Copy link
Member

Hi @baywet Thank you for the fast response. In v1.12 its exactly the same and in v1.11 the method is named like this: client.Groups().ByGroupsIdGuid(). As for the the OAS desc it is as follows:

"/groups/{id}": {
      "get": {
        "tags": [
          "Groups"
        ],
        "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"
            }
          }

@NerdJeremia Just to confirm here, Any chance you can confirm if the description provided is the expected input?
Testing out on #4547 I see that using if the path is "/groups/{id}" will give the indexer method as ByIdGuid.
To get the indexer as ByGroupsIdGuid the path would need to be "/groups/{groups-id}" in original description.

@NerdJeremia
Copy link
Author

Hi @andrueastman,
for our groups endpoint the path is "/groups/{id}" and for the subresources of a group the path is "/groups/{groupId}/resourceName/{id}".
I generate the all the clients from the same open-api definition just with different versions of kiota.
However the indexer being ByGroupsIdGuid is only a problem in v1.11 and v1.10.
In the latest version we got irritated by the indexer being namen ByGroupIdIdGuid.

@andrueastman
Copy link
Member

Thanks for the extra information here on the extra path @NerdJeremia

Looks like this was caused by the path deduplication changes in #4174 that introduced the appending of -id suffix in the situations where path deduplication is performed on indexers.

Authored #4547 to resolve

@andrueastman andrueastman modified the milestones: Backlog, Kiota v1.14 Apr 25, 2024
@andrueastman andrueastman added generator Issues or improvements relater to generation capabilities. and removed help wanted Issue caused by core project dependency modules or library Needs: Attention 👋 Go labels Apr 25, 2024
@NerdJeremia
Copy link
Author

Great. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
generator Issues or improvements relater to generation capabilities. type:bug A broken experience
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants