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

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

Open
nelsonken opened this issue Sep 29, 2017 · 18 comments
Open

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

nelsonken opened this issue Sep 29, 2017 · 18 comments

Comments

@nelsonken
Copy link

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

@ycdesu
Copy link

ycdesu commented Nov 17, 2017

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
Copy link

oantoro commented Jan 2, 2018

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

@potterdai
Copy link

potterdai commented Feb 4, 2018

@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
Copy link

any news? :)

@ww9
Copy link

ww9 commented Nov 15, 2018

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:

  • echo - Allows for declaring a custom Context struct but requires one to typecast from default Context to it in EVERY handler. I don't see much gain here.

  • Extending Gorilla Mux Tutorial (Pt. 1) - Tutorial on how to use Custom context on Gorilla mux. But as far as I understood, it doesn't support request scoped values. So it's not much different than declaring handlers as methods of a struct and putting data like Db connection in that struct to be accessible from all handlers. I could be wrong, but that's what it looks.

  • HTTP Request Contexts & Go (2004) - Suggests some interesting alternatives but in the end settles with helper() methods to encapsulate typecasting.

I'm going to study the problem further.

@ww9
Copy link

ww9 commented Nov 17, 2018

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

  • Injecting dependencies (like Db connection) in handlers is done via App struct fields which is not as ideal as the more functional approach of passing them via parameters when building each handler.

  • I should think about making the App struct and their handlers testable. Perhaps not all routes but I'm sure I'll want to test some routes and in those tests I'll want to pass fake dependencies such as a fake (or lightweight SQLite) database connection.

  • Accessing request scoped data like authenticated user will still require writing a helper function that executes a map lookup and typecast from interface{} every time it is accessed. But since this bloat is encapsulated, I can live with that. For now.

@unlikezy
Copy link

unlikezy commented Apr 28, 2019

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
Copy link

LaysDragon commented Mar 6, 2020

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
Copy link

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
Copy link

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

@jarrodhroberson
Copy link

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
Copy link

Pitasi commented Mar 28, 2022

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
Copy link

jarrodhroberson commented Mar 30, 2022

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
Copy link

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
Copy link

hamza72x commented Dec 9, 2022

*2023?

*almost

@YuZongYangHi
Copy link

YuZongYangHi commented Aug 31, 2023

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
Copy link

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
Copy link
Contributor

flc1125 commented May 10, 2024

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. :-(

#3963 I think that should solve some of the problem.

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

No branches or pull requests