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.

Sign up for GitHub

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

Jump to bottom

Trailing slash in URL is dropped during generation #4291

Open
mderriey opened this issue Mar 5, 2024 · 11 comments · May be fixed by #5651
Open

Trailing slash in URL is dropped during generation #4291

mderriey opened this issue Mar 5, 2024 · 11 comments · May be fixed by #5651
Labels
help wanted Issue caused by core project dependency modules or library type:bug A broken experience WIP
Milestone
Kiota v1.21

Comments

@mderriey
Copy link

mderriey commented Mar 5, 2024 •
edited
Loading

Hi there

Problem

We're working with an API which endpoints contain trailing slashes, for example {+baseUrl}/api/v1/app/{app_id}/msg/.

During the client generation, the trailing slash is dropped, and the generated client doesn't work as expected as all requests return a 404.

Repro

For reference, here's a link to the OpenAPI document used here: https://github.com/svix/svix-webhooks/blob/8d32e47e0484f5d0839bce364d8700d2c7457937/openapi.json#L7779

  1. Generate the .NET client 
    kiota generate `
  --openapi https://raw.githubusercontent.com/svix/svix-webhooks/main/openapi.json `
  --output .\TrailingSlashDroppedIssue4291 `
  --language CSharp `
  --class-name SvixClient `
  --namespace-name SvixApiClient `
  --exclude-backward-compatible true `
  --serializer Microsoft.Kiota.Serialization.Json.JsonSerializationWriterFactory `
  --deserializer Microsoft.Kiota.Serialization.Json.JsonParseNodeFactory `
  --structured-mime-types application/json `
  --include-path '/api/v1/app/{app_id}/msg/#POST'
  2. See the missing trailing slashes in the constructor definitions in the TrailingSlashDroppedIssue4291\Api\V1\App\Item\Msg\MsgRequestBuilder.cs file 
    public class MsgRequestBuilder : BaseRequestBuilder {
    public MsgRequestBuilder(Dictionary<string, object> pathParameters, IRequestAdapter requestAdapter) : base(requestAdapter, "{+baseurl}/api/v1/app/{app_id}/msg{?with_content*}", pathParameters) {
    }
    public MsgRequestBuilder(string rawUrl, IRequestAdapter requestAdapter) : base(requestAdapter, "{+baseurl}/api/v1/app/{app_id}/msg{?with_content*}", rawUrl) {
    }
}

Expected result

Kiota shouldn't manipulate paths defined in the OpenAPI document, or should provide an option to opt out of this behavior.
@github-project-automation github-project-automation bot moved this to Todo in Kiota Mar 5, 2024
@baywet
Copy link
Member

baywet commented Mar 5, 2024

Hi @mderriey,
Thanks for using kiota and for reaching out.
Let me start by saying that while requiring trailing slashes is supported according to RFCs 9110 and 3986, it's a bit unusual from a REST API design perspective, see this stack overflow post. This is not an excuse, just adding some perspective here.

Kiota generates the URI templates here and additional unit tests can be added here

The challenge is that kiota relies on OpenAPI.net to parse the description and build the tree of the API. And that process kind of relies on trimming the trailing slashes to work as you can see here and there. In fact it's removing the trailing slash

From kiota's perspective, the trailing slash doesn't exist, and adding it systematically would be wrong in the same sense it's wrong to remove it.

What needs to happen it:

  1. OpenAPI.net needs to updated to keep the trailing slash without derailing the control logic
  2. Once the patch is released and pulled in kiota, the slash should appear "magically"

Would you be willing to work on a pull request for openapi.net?

@baywet baywet added type:bug A broken experience help wanted Issue caused by core project dependency modules or library dependencies Pull requests that update a dependency file blocked This work can't be done until an external dependent work is done. labels Mar 5, 2024
@baywet baywet added this to the Backlog milestone Mar 5, 2024
@mderriey mderriey mentioned this issue Mar 6, 2024
Closed
@mderriey

This comment was marked as outdated.

Sign in to view
@mderriey
Copy link
Author

mderriey commented Mar 6, 2024 •
edited
Loading

My bad, please ignore my last message, after writing a test in the Kiota codebase, I can see now how it comes from OpenApiUrlTreeNode, which you originally pointed to. And I also realised what I thought was the leading slash is actually the trailing slash as it works segment by segment.

Let me open an issue in the OpenAPI.NET repo, thanks again.

@mderriey mderriey mentioned this issue Mar 6, 2024
Closed
@sevein sevein mentioned this issue Jul 23, 2024
Closed
@jlarmstrongiv
Copy link

We just fixed trailing slashes in the OpenAPI.NET repo via microsoft/OpenAPI.NET#1835, which was released in 1.6.22.

Can we update the OpenAPI.NET dependency to see if it adds trailing slash support in Kiota?

@jlarmstrongiv
Copy link

It looks like the OpenAPI.NET dependency was already updated by dependabot https://github.com/microsoft/kiota/blob/main/src/Kiota.Builder/Kiota.Builder.csproj#L47 once the latest preview includes it, I’ll try it out.

@baywet baywet modified the milestones: Backlog, Kiota v1.19, Kiota v1.20 Oct 2, 2024
@baywet
Copy link
Member

baywet commented Oct 2, 2024

Thanks for the follow up @jlarmstrongiv !
Since we have a minor release this week, we won't have a preview release.
You should be able to try it with 1.19, and if more changes are required, we'll be more than happy to review any incoming PR for 1.20

@baywet baywet removed dependencies Pull requests that update a dependency file blocked This work can't be done until an external dependent work is done. labels Oct 2, 2024
@sevein sevein mentioned this issue Oct 17, 2024
Merged
@sevein
Copy link
Contributor

sevein commented Oct 17, 2024

I've just tried kiota v1.19.1, and I can confirm that the issue is resolved for me. I had been using an appendTrailingSlashHandler middleware as a workaround, and I've now tested that my client works as expected with the new client code generated by Kiota v1.19.1, without the need for that middleware.

@baywet
Copy link
Member

baywet commented Oct 17, 2024

Thanks for confirming! Closing.

@baywet baywet closed this as completed Oct 17, 2024
@github-project-automation github-project-automation bot moved this from Todo 📃 to Done ✔️ in Kiota Oct 17, 2024
@jlarmstrongiv
Copy link

Could we re-open this issue? Trailing slashes are still broken in TypeScript

@sevein
Copy link
Contributor

sevein commented Oct 18, 2024

Oh no! I only tested with a Go client.

@baywet baywet reopened this Oct 18, 2024
@github-project-automation github-project-automation bot moved this from Done ✔️ to In Progress 🚧 in Kiota Oct 18, 2024
@baywet
Copy link
Member

baywet commented Oct 18, 2024

@jlarmstrongiv thanks for letting us know! Do you have more context to share around this issue?

@chelkyl chelkyl linked a pull request Oct 21, 2024 that will close this issue
Draft
@samwelkanda samwelkanda modified the milestones: Kiota v1.20, Kiota v1.21 Nov 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
help wanted Issue caused by core project dependency modules or library type:bug A broken experience WIP
Projects
Kiota
Status: In Progress 🚧
Milestone
Kiota v1.21
Development

Successfully merging a pull request may close this issue.

Support trailing slash in URL paths chelkyl/kiota
5 participants
@sevein @mderriey @baywet @jlarmstrongiv @samwelkanda