Skip to content

[Proposal] Add typed getters for Context values #2770

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

Open
SebastianOsuna opened this issue Apr 21, 2025 · 8 comments
Open

[Proposal] Add typed getters for Context values #2770

SebastianOsuna opened this issue Apr 21, 2025 · 8 comments

Comments

@SebastianOsuna
Copy link

To easily access context typed values, would you consider adding typed getters?

userID := ctx.GetInt64("userId")

This of course would be a breaking change since a bunch of new functions would be added to the Context interface.

We've created an extension to handle such typed getters:

https://github.com/BacoFoods/echoext/blob/master/context.go#L12

I'd be willing to port it to the main project, but I'd like to pick your brains as to the value of this in echo before commiting to a PR.

*Example Use Case: * passing auth parameters from an auth middleware down to handlers.\

Cheers.

@aldas
Copy link
Contributor

aldas commented Apr 21, 2025

without delving into if this can be added to Context interface. Why does it not handle missing values or invalid casts as error and missing values defaulting to "zero" is problematic in cases when you need to know if value existed at all or not.

@aldas
Copy link
Contributor

aldas commented Apr 21, 2025

nb: you could create helper function for that

func ContextGet[T any](c echo.Context, key string) T {
	v, ok := c.Get(key).(T)
	if !ok {
		var t = new(T)
		return *t
	}
	return v
}

and use it in handler/middlewares

	e.GET("/", func(c echo.Context) error {
		var userID = ContextGet[int](c, "userId")
		return c.JSON(http.StatusOK, map[string]int{"value": userID})
	})

but it would make more sense if it returned an error when there is no value or cast can not be done.

@aldas
Copy link
Contributor

aldas commented Apr 21, 2025

or this

var ErrKeyNotFound = errors.New("key not found")
var ErrCanNotCastToTarget = errors.New("can not cast to target")

func ContextGet[T any](c echo.Context, key string, target *T) error {
	raw := c.Get(key)
	if raw == nil {
		return ErrKeyNotFound
	}
	v, ok := raw.(T)
	if !ok {
		return ErrCanNotCastToTarget
	}
	*target = v
	return nil
}

and used like that

		var userId int = 10
		if err := ContextGet(c, "userId", &userId); err != nil {
			return err
		}

or

		type Args struct {
			UserID int `json:"user_id"`
		}
		args := Args{
			UserID: -1, // default to -1
		}

		if err := ContextGet(c, "userId", &args.UserID); err != nil {
			return err
		}

full example:

package main

import (
	"errors"
	"github.com/labstack/echo/v4"
	"github.com/labstack/echo/v4/middleware"
	"log/slog"
	"net/http"
)

var ErrKeyNotFound = errors.New("key not found")
var ErrCanNotCastToTarget = errors.New("can not cast to target")

func ContextGet[T any](c echo.Context, key string, target *T) error {
	raw := c.Get(key)
	if raw == nil {
		return ErrKeyNotFound
	}
	v, ok := raw.(T)
	if !ok {
		return ErrCanNotCastToTarget
	}
	*target = v
	return nil
}

func main() {
	e := echo.New()
	e.Use(middleware.Logger())
	e.Use(middleware.Recover())

	e.Use(func(next echo.HandlerFunc) echo.HandlerFunc {
		return func(c echo.Context) error {
			c.Set("userId", int(123456))
			return next(c)
		}
	})

	e.GET("/", func(c echo.Context) error {
		type Args struct {
			UserID int `json:"user_id"`
		}
		args := Args{
			UserID: -1, // default to -1
		}

		if err := ContextGet(c, "userId", &args.UserID); err != nil {
			return err
		}
		
		return c.JSON(http.StatusOK, args)
	})

	if err := e.Start(":8080"); err != nil && !errors.Is(err, http.ErrServerClosed) {
		slog.Error("server start error", "error", err)
	}
}

@SebastianOsuna
Copy link
Author

@aldas Totally agree. Generics, defaults, parsing are all improvements that must be made first. This was just a quick way for us to migrate out of gin.

Just wondering if it's something worth working on as part of echo itself.

@aldas
Copy link
Contributor

aldas commented Apr 21, 2025

If I may ask off the topic question - why migrate away from Gin? it is a good library.

@SebastianOsuna
Copy link
Author

Two reasons mainly:

  • single Bind function
  • return values on ctx responses. Forgetting the return after early ctx.JSON has happened to us way too many times.

The rest is just personal preference towards echo that I couldn't justify.

@SebastianOsuna
Copy link
Author

@aldas talking about Bind, looking at one of your examples, another approach came into my mind:

type Data struct {
  Foo string `query:"foo"`
  Bar int `param:"bar"`
  Baz bool `form:"baz"`
  Qux float64 `context:"qux"` // type
}

ctx.Set("qux", 1) // notice this is an int and implies type cohersion

var reqVars Data
if err := ctx.Bind(&reqVars); err != nil {
  // ...
}

@aldas
Copy link
Contributor

aldas commented Apr 23, 2025

If you are suggesting adding support for context tag with bind - I am not immediately against. If it does not lead to complete refactor of DefaultBinder.bindData mechanics and does not hugely increase that method complexity - I would see this as acceptable change. I am cautious because that method is one of that places that situates on top 5 most complex parts of Echo and "Bind" as functionality is quite popular.

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

2 participants