oapi-codegen icon indicating copy to clipboard operation
oapi-codegen copied to clipboard

Published 20 hours ago • verified publisher icon deepmap

feat: allow round trip middleware per operation

Open pseudo-su opened this issue 4 years ago • 9 comments

Reference implementation to cater to the use-cases I described in #150

Notes

Fixes https://github.com/deepmap/oapi-codegen/issues/150

pseudo-su avatar Mar 16 '20 10:03 pseudo-su

This is exactly the use case I have, so I would love to see a complete example in your pseudo-su /golang-service-template repo

StevenACoffman avatar Mar 17 '20 17:03 StevenACoffman

@StevenACoffman I haven't forgotten about this, you make a good point about examples... I might actually add some more tests cases to this PR that would also help somewhat in capturing examples.

I have a few open PRs for oapi-codegen that may or may not get merged. I was trying to avoid it because I'm lazy but I might merge them all into my forked repo and add a branch to pseudo-su /golang-service-template with examples.

My preference was to get things merged (where possible) before spending too much time messing about with a temporary fork.

pseudo-su avatar Apr 01 '20 06:04 pseudo-su

@deepmap-marcinr This one is ready too!

StevenACoffman avatar Apr 01 '20 14:04 StevenACoffman

@pseudo-su Hmmm, I think your rebase you need to regenerate because now there's some test failures.

StevenACoffman avatar Apr 01 '20 22:04 StevenACoffman

I need to fix some things and add those example/tests I mentioned, working on it now

pseudo-su avatar Apr 01 '20 22:04 pseudo-su

Ok @StevenACoffman three main things changed and this should be good now.

  • joinMiddleware had an issue when passing an empty list of middleware to join (picked up by the new tests from master 🎉 ).
  • I've added tests in internal/test/client/client_test.go that have scenarios where WithSharedRoundTripMiddleware and WithRoundTripMiddleware are used with a mock "interceptor".
  • I fixed the spec in internal/test/client/client.yaml because the responses were missing the content field inbetween the status code and the content-type

EG it was this

responses:
  200:
    application/json:

instead of

responses:
  200:
    content:
      application/json:

pseudo-su avatar Apr 02 '20 00:04 pseudo-su

Thanks, yes, this works for us in our client. Nice improvement! @deepmap-marcinr Ready for you!

StevenACoffman avatar Apr 02 '20 16:04 StevenACoffman

What is the status of this PR? Is there any other way to set custom behavior?

@StevenACoffman @pseudo-su

batazor avatar Jan 03 '24 17:01 batazor

@oapi-codegen any reason why this wasn't approved/merged (aside from the current merge conflicts)?

DAcodedBEAT avatar Jun 13 '24 19:06 DAcodedBEAT