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.

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

Improve gin support (interface, error, middlewares) #530

Merged
merged 1 commit into from Apr 19, 2022

Conversation

zzh8829
Copy link
Contributor

@zzh8829 zzh8829 commented Feb 22, 2022

  1. Use gin.IRouter interface instead of *gin.Engine
    this will allow us to support nested router mounting using engine/routergroup/other routers
    fixes Support RouterGroup when registering handlers with gin #507
  2. switch custom middleware code to gin middleware
    the custom middleware code doesn't support context abort or c.Next() in normal gin middleware, this also fixes gin: abort request in case middleware function called abort #494 gin: included middleware cannot abort request #485
  3. add errors from validation middleware to gin context
    huge fix for debugging authentication func. before this change the nested e.Errors[] is completely hidden not surfaced anywhere.

mistermelphin pushed a commit to mistermelphin/oapi-codegen that referenced this pull request Apr 6, 2022
Use gin.IRouter interface instead of *gin.Engine
Switch custom middleware code to gin middleware

Original changes: deepmap#530
@deepmap-marcinr deepmap-marcinr merged commit 11622c2 into deepmap:master Apr 19, 2022
@ericvolp12
Copy link
Contributor

Hey, this is neat but it ended throwing off how we were constructing the gin engine, looks like now the pattern needs to be:

// Register the business logic handlers
apiRoutes := api.RegisterHandlers(r, apiServer).(*gin.RouterGroup)

r.Use(apiRoutes.Handlers...)

Instead of what was previously:

// Register the business logic handlers
r := api.RegisterHandlers(r, apiServer)

I'm gonna submit a MR to update the Gin example since this new patch breaks the example.

@jxsl13
Copy link

jxsl13 commented Apr 19, 2022

returning concrete types should be preferred over returning interfaces as concrete types that implement an interface may provide more functionality than the interface.
on the other hand requiring interfaces to be passed as parameters makes parameters more versatile.

@deepmap-marcinr
Copy link
Contributor

huh, yeah. Agreed, returning concrete types is better. I missed that. I wonder whether to roll back this change. I hate breaking existing code like this.

@ericvolp12
Copy link
Contributor

ericvolp12 commented Apr 19, 2022

We could update it to return a gin.RouterGroup instead of the IRouter but I'm not 100% sure if there are any use cases in which we'd want an IRouter over the RouterGroup. AFAIK the IRouter interface was almost pulled from Gin a while ago and it has no use other than as a wrapper interface around a RouterGroup.

ericvolp12 added a commit to ericvolp12/oapi-codegen that referenced this pull request Apr 19, 2022
The most recent release of `oapi-codegen` (v1.10.0) included changes
that broke the existing examples which were introduced in deepmap#530

This change updates the `gin` example by regening the code-generated files and
updating the router registration to work with the new generated code.
@jxsl13
Copy link

jxsl13 commented Apr 19, 2022

release a new minor version and retract the last one maybe in go.mod?

https://golangtutorial.dev/tips/retract-go-module-versions/

deepmap-marcinr pushed a commit that referenced this pull request Apr 19, 2022
This reverts commit 11622c2.

It caused a breaking change to existing users.
deepmap-marcinr pushed a commit that referenced this pull request May 4, 2022
The most recent release of `oapi-codegen` (v1.10.0) included changes
that broke the existing examples which were introduced in #530

This change updates the `gin` example by regening the code-generated files and
updating the router registration to work with the new generated code.
adrianpk pushed a commit to foorester/oapi-codegen that referenced this pull request Jan 16, 2024
Co-authored-by: Zihao Zhang <z@zeet.co>
adrianpk pushed a commit to foorester/oapi-codegen that referenced this pull request Jan 16, 2024
This reverts commit f10cd14.

It caused a breaking change to existing users.
adrianpk pushed a commit to foorester/oapi-codegen that referenced this pull request Jan 16, 2024
The most recent release of `oapi-codegen` (v1.10.0) included changes
that broke the existing examples which were introduced in deepmap#530

This change updates the `gin` example by regening the code-generated files and
updating the router registration to work with the new generated code.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support RouterGroup when registering handlers with gin
4 participants