gin-gonic / gin

Gin is a HTTP web framework written in Go (Golang). It features a Martini-like API with much better performance -- up to 40 times faster. If you need smashing performance, get yourself some Gin.
https://gin-gonic.com/
MIT License
77.93k stars 7.97k forks source link

gin.Context is not a interface, how to extend it? #1123

Open nelsonken opened 6 years ago

nelsonken commented 6 years ago

gin.Context is not a interface, how to extend it?

ycdesu commented 6 years ago

What's your scenario? I think context is the core part of gin that coordinates other components, like controls the middlewares execution, and model binding...etc. It'll make the system more complicated if you change the default behavior of such core components.

oantoro commented 6 years ago

do you mean you want to share service for all request handlers globally?

potterdai commented 6 years ago

@ycavatars Interface makes it possible to extend context, for example, you can add UserID field to context, and call context.UserID directly instead of using context.GetString("UserID").

NarHakobyan commented 6 years ago

any news? :)

ww9 commented 5 years ago

Here's some input about supporting custom Context in Gin. I'm not very experienced so take it with a grain of salt.

What I expected

type MyContext struct {
    *gin.Context

    // Value set in my custom middleware
    User *AuthenticatedUser
}

func indexHandler (c *MyContext) {
    fmt.Print(c.User)
}

Advantages:

✅ c.User is typed ✅ no typecasting from interface{} ✅ no map lookup ✅ wont compile if dev mistypes c.User ✅ is available to all handlers, per request, without extra work ✅ editor autocompletion at all times

What Gin and almost all libs including standard lib provides

Instead Gin, and most libs, force me to do something along the lines of:

func indexHandler (c *Context) {
    user, ok =: c.Get("user").(*AuthenticatedUser)
    if ok {
        fmt.Print(user)
    } else {
        // handle case when "user" key was not found somehow
    }
}

❌ bloat ❌ typecast from interface{} ❌ map lookup ❌ "user" key prone to mistyping* ❌ editors wont autocomplete until you finished casting it

*Note that we are not supposed to use strings as keys in stuff like that, which alleviates some of the problems. Although there's code on Gin's frontpage using string as key (https://i.imgur.com/Kdbr9rg.png), perhaps for simplicity sake. This part of https://golang.org/pkg/context/#WithValue provides some reasoning on why not using string is good practice:

The provided key must be comparable and should not be of type string or any other built-in type to avoid collisions between packages using context. Users of WithValue should define their own types for keys. To avoid allocating when assigning to an interface{}, context keys often have concrete type struct{}. Alternatively, exported context key variables' static type should be a pointer or interface.

Mitigating Context map problems

There are ways to mitigate some of the bloat. One is encapsulating the typecasting logic inside helper functions (aka shove the bloat somewhere only we know), typically contained inside an App struct so we don't pollute global scope:

func (app *App) User(c *gin.Context) *AuthenticatedUser {
    if user, ok =: c.Get("user").(*AuthenticatedUser); ok {
        return user
    }
    return &AuthenticatedUser{} // Or perhaps nil
}

So now our handlers look something along the lines of:

func (app *App) indexHandler (c *gin.Context) {
    fmt.Print(app.User(c))
}

To me as a lib user, Context's map[interface{}]interface{} provided by standard library, Gin and most other libs is a bit disappointing. From my brief evaluation of alternatives so far, gorilla mux, echo and gocraft/web seems to tackle this problem in different ways:

I'm going to study the problem further.

ww9 commented 5 years ago

After some careful consideration I decided to go with the helper() solution described in Mitigating Context map problems. Some considerations:

unlikezy commented 5 years ago

One bad thing of gin.Context is that it's hard to cooperate with standard Context. For example, to add user's KV to gin.Context:

func UserHandler(gc *gin.Context) {
        ctx := context.WithValue(gc, "user_defined_key", "user_value")
        //or context.WithTimeout   context.WithDeadline  etc
        UserFunc1(ctx)
}

func UserFunc1(ctx context.Context) {
       //do something with user_defined_key
       v, ok := ctx.Value("user_defined_key").(string)
       //.....

       //we can never get gin.Context back anyway...
}

so, I modify the gin.Context, to make it more compatible with standard context: https://github.com/unlikezy/gin Above example could be done by this:

func UserHandler(gc *gin.Context) {
        ctx := context.WithValue(gc, "user_defined_key", "user_value")
         UserFunc1(ctx)
}

func UserFunc1(ctx context.Context) {
       //do something with user_defined_key
       v, ok := ctx.Value("user_defined_key").(string)
       //.....

       gc := gin.MustGinContext(ctx)
       //to do things with gin.Context as need
}
LaysDragon commented 4 years ago

I encounter same problem ,I need to get gin.Context work with OpenTracing client packet which work with context.Context,but cannot get gin.Context back after process...hope I can simply get gin.Context from context.Context,at least provide some method can merge value data from context.Context to gin.Context.

        span, ctx := opentracing.StartSpanFromContext(c, opName)
        // c.with
        defer span.Finish()
        span.LogFields(
            log.String("event", "soft error"),
            log.String("type", "cache timeout"),
            log.Int("waited.millis", 1500))
        h(ctx.(*gin.Context))//can get the context back to gin.context since it is actually valueCtx struct type
LaysDragon commented 4 years ago

well fine,it looks like almost impossible to copy all value from standard context since valueCtx is private recursive structure...especially the key is private type in opentracing...I can't even use the key manually outside that package. I hope there is any way to pass regular context's Values though gin.context.

LaysDragon commented 4 years ago

ok I found that I can pass context though Request's ctx ,not pretty but it should work

jarrodhroberson commented 3 years ago

One approach that I learned when researching heterogeneous type-safe map implementations in Java was to put the method for converting the type as a method on the Key. This would kill two birds with one stone, 1) you would have a unique type as a key and not a string as is recommended. 2) you have the key already since you are passing it in, just use the same instance to convert the interface{} to the correct type. That is basically what all the implementations in Java ended up doing.

Unfortunately that does very little good with *gin.Context because .Set() uses a string as the key. :-(

Pitasi commented 2 years ago

Just brainstormig: now that we have generics it would be nice to have gin.Context as a type constraint and implement our custom context types like

type MyContext struct {
  gin.BaseContext // this itself satisfy the constraint
  MyUser *User
}

then I could register that type somewhere when initialising the gin.Engine and it would be passed around instead of the current gin.Context. (Not 100% sure how to do this in Go 1.18)

Of course every requests starts with a fresh context so it's my (the gin user) responsibility to fill that MyUser field somehow (i.e. in a middleware)


btw the helper function can be a little more generic now:

func GetValue[T any](c *gin.Context, key string) T {
    v, ok := c.Get(key)
    if !ok {
        panic("key not found in context")
    }
    return v.(T)
}
jarrodhroberson commented 2 years ago

A way to do what @Pitasi is suggesting and configure the custom stuff is to provide a Factory or Provider function would provide instances of a gin.Context compliant implementation. This would obviously just override a default context provider that just created gin.Context just like it does now.

func ContextProvider(func() gin.Context) {}

Or something similar, then we could register a custom Context that was a gin.Context with additional functionality without breaking anything.

This kind of refactoring should not break anything because it is just adding a new method and not removing anything and if the default behavior stays the same it would continue to work with existing code. A default Provider could even be used when no custom one was needed.

jarrodhroberson commented 1 year ago

Just brainstormig: now that we have generics it would be nice to have gin.Context as a type constraint and implement our custom context types like

type MyContext struct {
  gin.BaseContext // this itself satisfy the constraint
  MyUser *User
}

then I could register that type somewhere when initialising the gin.Engine and it would be passed around instead of the current gin.Context. (Not 100% sure how to do this in Go 1.18)

Of course every requests starts with a fresh context so it's my (the gin user) responsibility to fill that MyUser field somehow (i.e. in a middleware)

btw the helper function can be a little more generic now:

func GetValue[T any](c *gin.Context, key string) T {
  v, ok := c.Get(key)
  if !ok {
      panic("key not found in context")
  }
  return v.(T)
}

That is what Producer/Consumer pattern is for. You do not just provide the type you provide a Producer[T] that can generate a default instance for you that you can customize. Then no code really has to change and you do not have to deal with providing a customized instance anywhere "everytime".

hamza72x commented 1 year ago

*2023?

*almost

YuZongYangHi commented 1 year ago
func handleFunc(handler func(extender *base.ContextExtender)) func(*gin.Context) {
    return func(c *gin.Context) {
        customContext := base.ContextExtender{Context: c}
        handler(&customContext)
    }
}

func Role(c *base.ContextExtender) {
    c.List()
    c.JSON(200, "OK")
}

func NewRouter() *gin.Engine {
    router := gin.Default()

    v1 := router.Group("/openapi/v1")
    v1.Use()
    v1.Use(AuthenticationMiddleware())
    {
        v1.GET("/rbac/role", handleFunc(Role))
    }

    return router
}
junderhill commented 4 months ago

The option to provide a custom context producer would be incredibly useful. I’ve recently been trying to use a request logger that has additional log attributes appended to it throughout the request pipeline. The issue is that with middleware we only have access to *gin.Context - If this could be some custom struct that is composed of gin.Context plus additional properties it would have solved all the issues.

flc1125 commented 4 months ago

One approach that I learned when researching heterogeneous type-safe map implementations in Java was to put the method for converting the type as a method on the Key. This would kill two birds with one stone, 1) you would have a unique type as a key and not a string as is recommended. 2) you have the key already since you are passing it in, just use the same instance to convert the interface{} to the correct type. That is basically what all the implementations in Java ended up doing.

Unfortunately that does very little good with *gin.Context because .Set() uses a string as the key. :-(

https://github.com/gin-gonic/gin/pull/3963 I think that should solve some of the problem.