Issue with paths ending with a slash about swift-openapi-generator HOT 14 CLOSED

This issue has been fixed for OpenAPIKit releases 2.5.2 and 3.0.0-alpha.7.

Thanks all for raising and investigating this!

from swift-openapi-generator.

Thanks @miguel-arrf for taking the time to dive into this one and thanks @mattpolzin for offering to take a look at a fix in OpenAPIKit—please feel free to link an issue here and we'll be more than happy to to integrate it when its ready.

from swift-openapi-generator.

Hi!

Thank you all so much for the help @czechboy0 @mattpolzin @simonjbeaumont !!

I had a fix implemented in my local repository, but it was a bit less elegant 😅!

I'm on vacations and unfortunately I have limited internet connection!

If it is okay by you @czechboy0 , I will gladly open the PR in the next (more or less) 12 hours 🙂.

Thank you for the opportunity and all the help!!

from swift-openapi-generator.

@miguel-arrf wrote:

Thank you all so much for the help @czechboy0 @mattpolzin @simonjbeaumont !!

No problem!

I'm on vacations and unfortunately I have limited internet connection!

If it is okay by you @czechboy0 , I will gladly open the PR in the next (more or less) 12 hours 🙂.

Thank you for the opportunity and all the help!!

That's fine, enjoy your vacation 😎. Ping us if you don't have the bandwidth or change your mind, and one of us will pick it up.

Be great to cover this in an update to the tests too :)

from swift-openapi-generator.

Hi!

Yup, it's exactly there that it is happening!

So, basically the issue was starting here when decoding the data with the OpenAPI.Document.self object.

My first guess was try to use the JSONDecoder instead of the YAMLDecoder, but the issue remained. I was then able to figure out where the issue is coming from. It's coming from the Path struct from OpenAPIKit, see here. What's happening is that they are spliting the path components by the / separator, meaning that all trailing / are getting ignored.

They mention the OpenAPI specification (available here). Upon reading it, I'm not entirely sure that the correct behaviour is to eliminate the last / from a Path if there are no components after it. With this, on the specification there are no cases of any endpoint ending up with a /, so I'm assuming they aren't expecting any trailing / to appear.

I will open an issue with OpenAPIKit to try to figure out what can be done! :)

Thank you so much for the help! @czechboy0

from swift-openapi-generator.

I've not heard about this problem before, but looking at the OpenAPIKit source i can confirm the problem is in how it handles these paths (splitting on forward-slash and then joining back with slash to create a string representation).

So, the bug was born of an oversight on my part. I'm on vacation without a laptop for the next week but after that I can fix the problem upstream in OpenAPIKit (especially if someone is kind enough to create me a GitHub issue to track this bug).

from swift-openapi-generator.

@miguel-arrf thanks for offering to open that issue against OpenAPIKit! I was composing my response when your response came in, but you were spot on.

from swift-openapi-generator.

it'd be good to be confident in the fix.

Shots fired. 😄 totally kidding, 100% same page about testing philosophy.

from swift-openapi-generator.

Hi @miguel-arrf, thank you for taking the time to file this great issue 👏

Just recently, @simonjbeaumont and I discussed this behavior, so I suspect it is a bug on the generator currently. We'll have to investigate where exactly the trailing slash gets stripped and figure it out from there. If you're able to, you could debug the generator yourself – which is probably easiest by cloning the repo, opening it in your IDE, and launching the CLI target with a debugger attached. We have more docs on contributing here. A good first breakpoint might be around here: https://github.com/apple/swift-openapi-generator/blob/main/Sources/_OpenAPIGeneratorCore/Translator/Operations/OperationDescription.swift#L240, where the path is parsed from the OpenAPI doc and used to generate the client code.

Now, as a temporary workaround, you could implement a ClientMiddleware that, for a set of known operation ids modifies the path to add the trailing slash before it goes out to the transport and gets sent out. That should allow you to get rid of the redirect. Once the generator is fixed, you'll be able to delete this middleware.

from swift-openapi-generator.

It's possible that the trailing slash is getting stripped in OpenAPIKit – @mattpolzin have you seen this before?

from swift-openapi-generator.

Thanks @mattpolzin for the fix!

@miguel-arrf, do you want to do the honors of opening a PR bumping our dependency to alpha 7?

from swift-openapi-generator.

PR created! ☺️ It already has the version bump to -alpha.7. I will be able to add the tests during this weekend, when I'm back from my vacations. Should I add them to the same PR @czechboy0 ?

from swift-openapi-generator.

PR created! ☺️ It already has the version bump to -alpha.7. I will be able to add the tests during this weekend, when I'm back from my vacations. Should I add them to the same PR @czechboy0 ?

Yes please, it'd be good to be confident in the fix.

from swift-openapi-generator.

it'd be good to be confident in the fix.

Shots fired. 😄 totally kidding, 100% same page about testing philosophy.

Haha, I meant that we correctly integrate the fix 😉 No shade intended.

from swift-openapi-generator.