echo icon indicating copy to clipboard operation
echo copied to clipboard

[Proposal] Add typed getters for Context values

Open SebastianOsuna opened this issue 9 months ago • 8 comments

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.

SebastianOsuna avatar Apr 21 '25 17:04 SebastianOsuna

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 avatar Apr 21 '25 17:04 aldas

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 avatar Apr 21 '25 18:04 aldas

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)
	}
}

aldas avatar Apr 21 '25 18:04 aldas

@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.

SebastianOsuna avatar Apr 21 '25 18:04 SebastianOsuna

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

aldas avatar Apr 21 '25 18:04 aldas

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 avatar Apr 21 '25 18:04 SebastianOsuna

@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 {
  // ...
}

SebastianOsuna avatar Apr 22 '25 22:04 SebastianOsuna

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.

aldas avatar Apr 23 '25 14:04 aldas