OpenAPI.NET icon indicating copy to clipboard operation
OpenAPI.NET copied to clipboard

Support trailing slashes in OpenAPI paths

Open mderriey opened this issue 1 year ago • 4 comments

Coming from https://github.com/microsoft/kiota/issues/4291

Is your feature request related to a problem? Please describe. We're working with an API which endpoints end with trailing slashes, for example /api/v1/app/{app_id}/msg/. See the OpenAPI document at https://github.com/svix/svix-webhooks/blob/8d32e47e0484f5d0839bce364d8700d2c7457937/openapi.json#L7779.

We're using Kiota to generate a .NET client for this API, and the generated client doesn't contain the trailing slashes (see the issue at the top for more details).

While we agree it's not common to have trailing slashes, apparently it's supported according to the relevant RFCs.

Describe the solution you'd like OpenAPI.NET should respect the OpenAPI path when building out the OpenApiUrlTreeNode tree.

Describe alternatives you've considered N/A.

Additional context I've forked the repo and pushed a branch which shows how the trailing slash is removed, see https://github.com/microsoft/OpenAPI.NET/compare/vnext...mderriey:OpenAPI.NET:mderriey/support-trailing-slashes?expand=1.

I haven't opened a PR yet because it might be too much noise given it's not actionable as-is.

mderriey avatar Mar 06 '24 09:03 mderriey

Hi @mderriey, Thanks for creating this issue and for starting the work for a PR. We'll be happy to review your PR once it's ready! Make sure to add plenty of test cases! We want to be sure extraneous nodes don't appear magically or nodes don't disappear because of that change.

baywet avatar Mar 06 '24 14:03 baywet

Hey @baywet,

To be clear, the "work" I started is really just a showcase of how the trailing slash gets eaten away when building a URL tree node.

If you'd like me to continue that work, I'm going to need some guidance as to which approach we should take.

If we find the path ends with a slash, maybe we exclude the last slash from the split, so the last segment we process has the trailing slash?

Open to suggestions here.

mderriey avatar Mar 06 '24 14:03 mderriey

@maggiekimani1 can you help guide the contributions here please? (currently traveling for the next 10 days, it'll probably be faster this way)

baywet avatar Mar 06 '24 14:03 baywet

@mderriey Thank you for raising this and it is awesome that you have made an attempt at a PR. My first reaction to this problem is one of mild terror. You are absolutely correct that we should support this scenario. However, we need to make sure that trailing slashes do not create child nodes. We effectively want to special case this so that the trailing slash is part of the prior path segment identifier. We may need to add a new property to the tree node to indicate that it is a leaf node with a trailing slash. I'll try and spend some time to look at your PR and provide feedback.

darrelmiller avatar Mar 06 '24 19:03 darrelmiller

FWIW, @typespec/http has recently added support for including the trailing slash (https://github.com/microsoft/typespec/issues/3350).

sevein avatar Jul 23 '24 10:07 sevein

Supporting trailing slashes would allow me to write clients for APIs like https://docs.bunny.net/reference/get_-storagezonename-path-

jlarmstrongiv avatar Sep 08 '24 20:09 jlarmstrongiv

If someone wants to pick up the work on https://github.com/microsoft/OpenAPI.NET/pull/1584, please go ahead, I don't have the capacity anymore.

Apologies for leaving things unfinished.

The changes requested in the latest comment (https://github.com/microsoft/OpenAPI.NET/pull/1584#discussion_r1568802613) should be easy to implement, I think most of them are suggestions already.

Thanks in advance.

mderriey avatar Sep 12 '24 10:09 mderriey