fix: codegen breaking changes in api spec

beam-community / stripity-stripe

An Elixir Library for Stripe

Other

965 stars 344 forks source link

fix: codegen breaking changes in api spec #829

Closed yordis closed 2 months ago

yordis commented 7 months ago

The existing pipeline hasn't worked since v755. I am trying to upgrade it to https://github.com/stripe/openapi/releases/tag/v756

There are some breaking changes in it, but I am not sure!

https://github.com/stripe/openapi/compare/v755...v756

yordis commented 7 months ago

@maartenvanvliet, any thoughts here?

yordis commented 7 months ago

They are two identical update operations now without taking into consideration the path value. One for the external account, and the other one for the customer

yordis commented 7 months ago

I found the issue: https://github.com/beam-community/stripity-stripe/blob/73faf6d3e4b021471db7f2768cf2fbdd69055557/lib/openapi/phases/build_modules.ex#L19

That is making assumptions that no other method_name should exists, but they will, the situation is problematic, because we need to lookup the operationId by the path value in order to know the final path that should be use here

maartenvanvliet commented 7 months ago

The https://github.com/beam-community/stripity-stripe/pull/786/files#diff-96e6fda24e580cda6be0289146011cf77e2bf6d46f3afb82dc7c628d03827ff4R2 branch has a way to override the function names for these collisions. If you find a better way I'm all for it.

yordis commented 7 months ago

I will bring it up to the Stripe repo to seek guidelines.

If you find a better way I'm all for it.

I am a bit pragmatic regarding this topic: just one module with all API Operations as if it was a gRPC service, as a direct example of the alternative. So I am not sure if I would add much value here!

I found what the GO SDK does: https://github.com/stripe/stripe-go/blob/9c48c72a38d0c6e066ea7bb49a93ae92b1c06143/bankaccount/client.go#L95-L102

yordis commented 6 months ago

@maartenvanvliet any thoughts?

maartenvanvliet commented 6 months ago

The way the Go SDK does it, will require quite some work in the generated code. Comes down to the same thing in other SDK's by making it a special case. Personally I'd lean to a different function name.

yordis commented 6 months ago

Personally I'd lean to a different function name.

I agree here; I really do not like how they are doing things at all! It comes down to how could we tackle the situation, in the worst case, static list fixing the naming situation 🤷🏻

yordis commented 2 months ago

closed in favor of https://github.com/beam-community/stripity-stripe/pull/843