server_generatorのControllerに渡されるContextがRequestのものでない

[tsuzu]

生成されたControllerの第一引数がContextとなっているが、これはRequestからきたものではなく初期化時のものなので、わかりづらい

だが破壊的変更になるのでどうするべきか難しい

[micheam]

利用者がどのコンテキストを使うべきか。を悩まない様なシグネチャに変更したい。

api_gen/server_generator/sample/post_create_table_controller_gen.go

Lines 32 to 34 in f4e32f6

	func (p *PostCreateTableController) PostCreateTable(
	ctx context.Context, c echo.Context, req *PostCreateTableRequest,
	) (res *PostCreateTableResponse, err error) {

ctx context.Context と c.Request().Context() のどちらを使うのが正しいんだろう 🤔

補足

現状では、Handler Func にて引数から受領する context.Context が２種類存在している。

(1) 第1引数にて引き渡される ctx context.Context
(2) 第2引数 c echo.Context の内部に保持されている http.Request を経由して得られる context.Context (c.Request().Context())

(1) は server 開始時に生成された グローバルなコンテキストである一方で、 echo.Context.Request().Context() で取得できるコンテキストは リクエスト毎に生成されたものとなる。 net/http/#Request.Context

本ライブラリ の利用者が誤って、 c.Request().Context() を使うと、server 開始時に設定されたグローバルな設定値へのアクセスが失われる。一方で、 context.Context の本来の利用方針からすると、リクエスト毎にコンテキストを切りたい気がする。

A Context carries a deadline, cancelation signal, and request-scoped values across API boundaries
https://blog.golang.org/context より

[micheam]

一応、思いついた対応案をメモしておきます。
（別に無視していただいて構わない内容です）

案1: 独自Contextの提供

echo.Context と context.Context はどちらも interface であるため、
双方を実装した独自型を提供することで透過的に使用することができる。

// EchoContext is a type alias for echo.Context
type EchoContext echo.Context

// APIContext implements conetxt.Context and echo.Context
type APIContext struct {
	context.Context
	EchoContext       // <-- 別名にしないと、コンパイラに怒られる
}

package xxxx

func (g *GetXXXXController) GetXXXX(
	ctx *interfaces.APIContext, // <== 独自型に変更
	req *GetXXXXRequest,
) (res *GetXXXXResponse, err error) {

	// ctx *interfaces.APIContext は context.Context interface を満たしているので、
	// context 配下の関数にそのまま引き渡すことができる 
	childCtx, cancel := context.WithTimeout(ctx, 300*time.Millisecond)
	defer cancel() // releases resources if slowOperation completes before timeout elapses
	res, err = slowOperation(childCtx)
	// ....
}

※ 実際は 各Routesで使うためには interfaces パッケージでは実現できない（循環参照になる）ため、実装するパッケージは検討が必要そう。

Bootstrap 経由で 各種 Router に引き渡された context.Context と、
echo.Context を元に独自コンテキストを生成して Handler 関数を実行する様にすれば、
利用者はそれをそのまま context.Context としても echo.Context としても
使用できる。

func (r *Routes) GetEvents(ctx context.Context) echo.HandlerFunc {
	i := NewGetEventsController()
	return func(c echo.Context) error {
		req := new(GetEventsRequest)
		if err := c.Bind(req); err != nil {
			return c.JSON(http.StatusBadRequest, map[string]interface{}{
				"code":    http.StatusBadRequest,
				"message": "invalid request.",
			})
		}

		res, err := i.GetEvents(&interfaces.APIContext{
			Context:     ctx,
			EchoContext: c,
		}, req)
		if err != nil {
			return err
		}
		if res == nil {
			return nil
		}

		return c.JSON(http.StatusOK, res)
	}
}

labstack/echo の各種ドキュメントやIssueをみると、
この方法が推奨されている節がある？

• https://echo.labstack.com/guide/context
• Echo v5 labstack/echo#1302 (comment)

案2: Request.Context に詰める

ハンドラ関数の引数から ctx context.Context を削除し、代わりに echo.Context.Request().Context() 経由で
取得する仕様とする。

package xxxx

func (g *GetXXXXController) GetXXXX(
	c echo.Context, // <== ctx context.Context は引数から削除
	req *GetXXXXRequest,
) (res *GetXXXXResponse, err error) {
	
	ctx := c.Request().Context()
	childCtx, cancel := context.WithTimeout(ctx, 300*time.Millisecond)
	defer cancel() // releases resources if slowOperation completes before timeout elapses
	res, err = slowOperation(childCtx)
	// ....
}

Bootstrap 経由で 引き渡された context.Context がハンドラ関数内部で参照できるように、
RoutesでRequestの内部に設定しておく感じ。

func (r *Routes) GetEvents(ctx context.Context) echo.HandlerFunc {
	i := NewGetEventsController()
	return func(c echo.Context) error {
		req := new(GetEventsRequest)
		if err := c.Bind(req); err != nil {
			return c.JSON(http.StatusBadRequest, map[string]interface{}{
				"code":    http.StatusBadRequest,
				"message": "invalid request.",
			})
		}
		c.SetRequest(c.Request().WithContext(ctx)) // <== Request にコンテキストをつめてあげる
		res, err := i.GetEvents(c, req)
		if err != nil {
			return err
		}
		if res == nil {
			return nil
		}

		return c.JSON(http.StatusOK, res)
	}
}

[tsuzu]

個人的にはDIをしたい訳でもなく毎度メソッドの引数に渡す意味はないのでController側で持って仕舞えば良いと思っています(Contextではなく何らかのstructとして)

[54m]

already merged