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.

Sign up for GitHub

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

Jump to bottom

🐛 fix: Improve naming convention for Context returning functions #3193

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

🐛 fix: Improve naming convention for Context returning functions #3193

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions bind.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ func (b *Bind) RespHeader(out any) error {
// Cookie binds the requesr cookie strings into the struct, map[string]string and map[string][]string.
// NOTE: If your cookie is like key=val1,val2; they'll be binded as an slice if your map is map[string][]string. Else, it'll use last element of cookie.
func (b *Bind) Cookie(out any) error {
if err := b.returnErr(binder.CookieBinder.Bind(b.ctx.Context(), out)); err != nil {
if err := b.returnErr(binder.CookieBinder.Bind(b.ctx.RequestCtx(), out)); err != nil {
return err
}

Expand All @@ -104,7 +104,7 @@ func (b *Bind) Cookie(out any) error {

// Query binds the query string into the struct, map[string]string and map[string][]string.
func (b *Bind) Query(out any) error {
if err := b.returnErr(binder.QueryBinder.Bind(b.ctx.Context(), out)); err != nil {
if err := b.returnErr(binder.QueryBinder.Bind(b.ctx.RequestCtx(), out)); err != nil {
return err
}

Expand All @@ -131,7 +131,7 @@ func (b *Bind) XML(out any) error {

// Form binds the form into the struct, map[string]string and map[string][]string.
func (b *Bind) Form(out any) error {
if err := b.returnErr(binder.FormBinder.Bind(b.ctx.Context(), out)); err != nil {
if err := b.returnErr(binder.FormBinder.Bind(b.ctx.RequestCtx(), out)); err != nil {
return err
}

Expand All @@ -149,7 +149,7 @@ func (b *Bind) URI(out any) error {

// MultipartForm binds the multipart form into the struct, map[string]string and map[string][]string.
func (b *Bind) MultipartForm(out any) error {
if err := b.returnErr(binder.FormBinder.BindMultipart(b.ctx.Context(), out)); err != nil {
if err := b.returnErr(binder.FormBinder.BindMultipart(b.ctx.RequestCtx(), out)); err != nil {
return err
}

Expand All @@ -163,7 +163,7 @@ func (b *Bind) MultipartForm(out any) error {
// If there're no custom binder for mime type of body, it will return a ErrUnprocessableEntity error.
func (b *Bind) Body(out any) error {
// Get content-type
ctype := utils.ToLower(utils.UnsafeString(b.ctx.Context().Request.Header.ContentType()))
ctype := utils.ToLower(utils.UnsafeString(b.ctx.RequestCtx().Request.Header.ContentType()))
ctype = binder.FilterFlags(utils.ParseVendorSpecificContentType(ctype))

// Check custom binders
Expand Down
2 changes: 1 addition & 1 deletion client/client_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1572,7 +1572,7 @@ func Test_Client_SetProxyURL(t *testing.T) {
}

c.Status(resp.StatusCode())
c.Context().SetBody(resp.Body())
c.RequestCtx().SetBody(resp.Body())

return nil
})
Expand Down
22 changes: 11 additions & 11 deletions ctx.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -382,26 +382,26 @@ func (c *DefaultCtx) ClearCookie(key ...string) {
})
}

// Context returns *fasthttp.RequestCtx that carries a deadline
// RequestCtx returns *fasthttp.RequestCtx that carries a deadline
// a cancellation signal, and other values across API boundaries.
func (c *DefaultCtx) Context() *fasthttp.RequestCtx {
func (c *DefaultCtx) RequestCtx() *fasthttp.RequestCtx {
return c.fasthttp
}

// UserContext returns a context implementation that was set by
// Context returns a context implementation that was set by
// user earlier or returns a non-nil, empty context,if it was not set earlier.
func (c *DefaultCtx) UserContext() context.Context {
func (c *DefaultCtx) Context() context.Context {
ctx, ok := c.fasthttp.UserValue(userContextKey).(context.Context)
if !ok {
ctx = context.Background()
c.SetUserContext(ctx)
c.SetContext(ctx)
}

return ctx
}

// SetUserContext sets a context implementation by user.
func (c *DefaultCtx) SetUserContext(ctx context.Context) {
// SetContext sets a context implementation by user.
func (c *DefaultCtx) SetContext(ctx context.Context) {
c.fasthttp.SetUserValue(userContextKey, ctx)
}

Expand Down Expand Up @@ -1189,8 +1189,8 @@ func (c *DefaultCtx) Query(key string, defaultValue ...string) string {
// Queries()["filters[customer][name]"] == "Alice"
// Queries()["filters[status]"] == "pending"
func (c *DefaultCtx) Queries() map[string]string {
m := make(map[string]string, c.Context().QueryArgs().Len())
c.Context().QueryArgs().VisitAll(func(key, value []byte) {
m := make(map[string]string, c.RequestCtx().QueryArgs().Len())
c.RequestCtx().QueryArgs().VisitAll(func(key, value []byte) {
m[c.app.getString(key)] = c.app.getString(value)
})
return m
Expand Down Expand Up @@ -1219,7 +1219,7 @@ func (c *DefaultCtx) Queries() map[string]string {
// unknown := Query[string](c, "unknown", "default") // Returns "default" since the query parameter "unknown" is not found
func Query[V GenericType](c Ctx, key string, defaultValue ...V) V {
var v V
q := c.App().getString(c.Context().QueryArgs().Peek(key))
q := c.App().getString(c.RequestCtx().QueryArgs().Peek(key))

return genericParseType[V](q, v, defaultValue...)
}
Expand Down Expand Up @@ -1629,7 +1629,7 @@ func (c *DefaultCtx) SendFile(file string, config ...SendFile) error {
// Apply cache control header
if status != StatusNotFound && status != StatusForbidden {
if len(cacheControlValue) > 0 {
c.Context().Response.Header.Set(HeaderCacheControl, cacheControlValue)
c.RequestCtx().Response.Header.Set(HeaderCacheControl, cacheControlValue)
}

return nil
Expand Down
12 changes: 6 additions & 6 deletions ctx_interface_gen.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

32 changes: 16 additions & 16 deletions ctx_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -843,24 +843,24 @@ func Benchmark_Ctx_Body_With_Compression_Immutable(b *testing.B) {
}
}

// go test -run Test_Ctx_Context
func Test_Ctx_Context(t *testing.T) {
// go test -run Test_Ctx_RequestCtx
func Test_Ctx_RequestCtx(t *testing.T) {
t.Parallel()
app := New()
c := app.AcquireCtx(&fasthttp.RequestCtx{})

require.Equal(t, "*fasthttp.RequestCtx", fmt.Sprintf("%T", c.Context()))
require.Equal(t, "*fasthttp.RequestCtx", fmt.Sprintf("%T", c.RequestCtx()))
}

// go test -run Test_Ctx_UserContext
func Test_Ctx_UserContext(t *testing.T) {
// go test -run Test_Ctx_Context
func Test_Ctx_Context(t *testing.T) {
t.Parallel()
app := New()
c := app.AcquireCtx(&fasthttp.RequestCtx{})

t.Run("Nil_Context", func(t *testing.T) {
t.Parallel()
ctx := c.UserContext()
ctx := c.Context()
require.Equal(t, ctx, context.Background())
})
t.Run("ValueContext", func(t *testing.T) {
Expand All @@ -872,36 +872,36 @@ func Test_Ctx_UserContext(t *testing.T) {
})
}

// go test -run Test_Ctx_SetUserContext
func Test_Ctx_SetUserContext(t *testing.T) {
// go test -run Test_Ctx_SetContext
func Test_Ctx_SetContext(t *testing.T) {
t.Parallel()
app := New()
c := app.AcquireCtx(&fasthttp.RequestCtx{})

testKey := struct{}{}
testValue := "Test Value"
ctx := context.WithValue(context.Background(), testKey, testValue) //nolint: staticcheck // not needed for tests
c.SetUserContext(ctx)
require.Equal(t, testValue, c.UserContext().Value(testKey))
c.SetContext(ctx)
require.Equal(t, testValue, c.Context().Value(testKey))
}

// go test -run Test_Ctx_UserContext_Multiple_Requests
func Test_Ctx_UserContext_Multiple_Requests(t *testing.T) {
// go test -run Test_Ctx_Context_Multiple_Requests
func Test_Ctx_Context_Multiple_Requests(t *testing.T) {
t.Parallel()
testKey := struct{}{}
testValue := "foobar-value"

app := New()
app.Get("/", func(c Ctx) error {
ctx := c.UserContext()
ctx := c.Context()

if ctx.Value(testKey) != nil {
return c.SendStatus(StatusInternalServerError)
}

input := utils.CopyString(Query(c, "input", "NO_VALUE"))
ctx = context.WithValue(ctx, testKey, fmt.Sprintf("%s_%s", testValue, input)) //nolint: staticcheck // not needed for tests
c.SetUserContext(ctx)
c.SetContext(ctx)

return c.Status(StatusOK).SendString(fmt.Sprintf("resp_%s_returned", input))
})
Expand All @@ -913,7 +913,7 @@ func Test_Ctx_UserContext_Multiple_Requests(t *testing.T) {
resp, err := app.Test(httptest.NewRequest(MethodGet, fmt.Sprintf("/?input=%d", i), nil))

require.NoError(t, err, "Unexpected error from response")
require.Equal(t, StatusOK, resp.StatusCode, "context.Context returned from c.UserContext() is reused")
require.Equal(t, StatusOK, resp.StatusCode, "context.Context returned from c.Context() is reused")

b, err := io.ReadAll(resp.Body)
require.NoError(t, err, "Unexpected error from reading response body")
Expand Down Expand Up @@ -3220,7 +3220,7 @@ func Test_Ctx_SendFile_MaxAge(t *testing.T) {
// check expectation
require.NoError(t, err)
require.Equal(t, expectFileContent, c.Response().Body())
require.Equal(t, "public, max-age=100", string(c.Context().Response.Header.Peek(HeaderCacheControl)), "CacheControl Control")
require.Equal(t, "public, max-age=100", string(c.RequestCtx().Response.Header.Peek(HeaderCacheControl)), "CacheControl Control")
require.Equal(t, StatusOK, c.Response().StatusCode())
app.ReleaseCtx(c)
}
Expand Down
14 changes: 7 additions & 7 deletions docs/api/ctx.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1891,18 +1891,18 @@ app.Get("/", func(c fiber.Ctx) error {
})
```

## SetUserContext
## SetContext

Sets the user specified implementation for context interface.

```go title="Signature"
func (c Ctx) SetUserContext(ctx context.Context)
func (c Ctx) SetContext(ctx context.Context)
```

```go title="Example"
app.Get("/", func(c fiber.Ctx) error {
ctx := context.Background()
c.SetUserContext(ctx)
c.SetContext(ctx)
// Here ctx could be any context implementation

// ...
Expand Down Expand Up @@ -2005,18 +2005,18 @@ app.Get("/", func(c fiber.Ctx) error {
})
```

## UserContext
## Context

UserContext returns a context implementation that was set by user earlier
Context returns a context implementation that was set by user earlier
or returns a non-nil, empty context, if it was not set earlier.

```go title="Signature"
func (c Ctx) UserContext() context.Context
func (c Ctx) Context() context.Context
```

```go title="Example"
app.Get("/", func(c fiber.Ctx) error {
ctx := c.UserContext()
ctx := c.Context()
// ctx is context implementation set by user

// ...
Expand Down
8 changes: 4 additions & 4 deletions docs/middleware/timeout.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ There exist two distinct implementations of timeout middleware [Fiber](https://g

## New

As a `fiber.Handler` wrapper, it creates a context with `context.WithTimeout` and pass it in `UserContext`.
As a `fiber.Handler` wrapper, it creates a context with `context.WithTimeout` which is then used with `c.Context()`.

If the context passed executions (eg. DB ops, Http calls) takes longer than the given duration to return, the timeout error is set and forwarded to the centralized `ErrorHandler`.

Expand Down Expand Up @@ -38,7 +38,7 @@ func main() {
app := fiber.New()
h := func(c fiber.Ctx) error {
sleepTime, _ := time.ParseDuration(c.Params("sleepTime") + "ms")
if err := sleepWithContext(c.UserContext(), sleepTime); err != nil {
if err := sleepWithContext(c.Context(), sleepTime); err != nil {
return fmt.Errorf("%w: execution error", err)
}
return nil
Expand Down Expand Up @@ -84,7 +84,7 @@ func main() {
app := fiber.New()
h := func(c fiber.Ctx) error {
sleepTime, _ := time.ParseDuration(c.Params("sleepTime") + "ms")
if err := sleepWithContextWithCustomError(c.UserContext(), sleepTime); err != nil {
if err := sleepWithContextWithCustomError(c.Context(), sleepTime); err != nil {
return fmt.Errorf("%w: execution error", err)
}
return nil
Expand Down Expand Up @@ -116,7 +116,7 @@ func main() {
db, _ := gorm.Open(postgres.Open("postgres://localhost/foodb"), &gorm.Config{})

handler := func(ctx fiber.Ctx) error {
tran := db.WithContext(ctx.UserContext()).Begin()
tran := db.WithContext(ctx.Context()).Begin()

if tran = tran.Exec("SELECT pg_sleep(50)"); tran.Error != nil {
return tran.Error
Expand Down
6 changes: 3 additions & 3 deletions middleware/adaptor/adaptor.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ func HTTPHandlerFunc(h http.HandlerFunc) fiber.Handler {
func HTTPHandler(h http.Handler) fiber.Handler {
return func(c fiber.Ctx) error {
handler := fasthttpadaptor.NewFastHTTPHandler(h)
handler(c.Context())
handler(c.RequestCtx())
return nil
}
}
Expand All @@ -43,7 +43,7 @@ func HTTPHandler(h http.Handler) fiber.Handler {
// forServer should be set to true when the http.Request is going to be passed to a http.Handler.
func ConvertRequest(c fiber.Ctx, forServer bool) (*http.Request, error) {
var req http.Request
if err := fasthttpadaptor.ConvertRequest(c.Context(), &req, forServer); err != nil {
if err := fasthttpadaptor.ConvertRequest(c.RequestCtx(), &req, forServer); err != nil {
return nil, err //nolint:wrapcheck // This must not be wrapped
}
return &req, nil
Expand Down Expand Up @@ -108,7 +108,7 @@ func HTTPMiddleware(mw func(http.Handler) http.Handler) fiber.Handler {
c.Request().Header.Set(key, v)
}
}
CopyContextToFiberContext(r.Context(), c.Context())
CopyContextToFiberContext(r.Context(), c.RequestCtx())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Critical: Incorrect context method change.

This change appears to be an error. The r.Context() call should not be changed to r.RequestCtx() because:

  1. r is an *http.Request which has a Context() method returning context.Context
  2. *http.Request does not have a RequestCtx() method
  3. CopyContextToFiberContext expects a context.Context as its first parameter

Apply this fix:

-			CopyContextToFiberContext(r.RequestCtx(), c.RequestCtx())
+			CopyContextToFiberContext(r.Context(), c.RequestCtx())

This issue could be causing the context cancellation bug mentioned in issue #3186, where the context is reported as canceled upon creation in testing scenarios.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
CopyContextToFiberContext(r.Context(), c.RequestCtx())
CopyContextToFiberContext(r.Context(), c.RequestCtx())

})

if err := HTTPHandler(mw(nextHandler))(c); err != nil {
Expand Down
14 changes: 7 additions & 7 deletions middleware/adaptor/adaptor_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,15 +162,15 @@ func Test_HTTPMiddleware(t *testing.T) {
app := fiber.New()
app.Use(HTTPMiddleware(nethttpMW))
app.Post("/", func(c fiber.Ctx) error {
value := c.Context().Value(TestContextKey)
value := c.RequestCtx().Value(TestContextKey)
val, ok := value.(string)
if !ok {
t.Error("unexpected error on type-assertion")
}
if value != nil {
c.Set("context_okay", val)
}
value = c.Context().Value(TestContextSecondKey)
value = c.RequestCtx().Value(TestContextSecondKey)
if value != nil {
val, ok := value.(string)
if !ok {
Expand Down Expand Up @@ -316,12 +316,12 @@ func testFiberToHandlerFunc(t *testing.T, checkDefaultPort bool, app ...*fiber.A
fiberH := func(c fiber.Ctx) error {
callsCount++
require.Equal(t, expectedMethod, c.Method(), "Method")
require.Equal(t, expectedRequestURI, string(c.Context().RequestURI()), "RequestURI")
require.Equal(t, expectedContentLength, c.Context().Request.Header.ContentLength(), "ContentLength")
require.Equal(t, expectedRequestURI, string(c.RequestCtx().RequestURI()), "RequestURI")
require.Equal(t, expectedContentLength, c.RequestCtx().Request.Header.ContentLength(), "ContentLength")
require.Equal(t, expectedHost, c.Hostname(), "Host")
require.Equal(t, expectedHost, string(c.Request().Header.Host()), "Host")
require.Equal(t, "http://"+expectedHost, c.BaseURL(), "BaseURL")
require.Equal(t, expectedRemoteAddr, c.Context().RemoteAddr().String(), "RemoteAddr")
require.Equal(t, expectedRemoteAddr, c.RequestCtx().RemoteAddr().String(), "RemoteAddr")

body := string(c.Body())
require.Equal(t, expectedBody, body, "Body")
Expand Down Expand Up @@ -392,8 +392,8 @@ func Test_FiberHandler_RequestNilBody(t *testing.T) {
fiberH := func(c fiber.Ctx) error {
callsCount++
require.Equal(t, expectedMethod, c.Method(), "Method")
require.Equal(t, expectedRequestURI, string(c.Context().RequestURI()), "RequestURI")
require.Equal(t, expectedContentLength, c.Context().Request.Header.ContentLength(), "ContentLength")
require.Equal(t, expectedRequestURI, string(c.RequestCtx().RequestURI()), "RequestURI")
require.Equal(t, expectedContentLength, c.RequestCtx().Request.Header.ContentLength(), "ContentLength")

_, err := c.Write([]byte("request body is nil"))
return err
Expand Down
2 changes: 1 addition & 1 deletion middleware/cache/cache_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -894,7 +894,7 @@ func Test_Cache_MaxBytesSizes(t *testing.T) {
}))

app.Get("/*", func(c fiber.Ctx) error {
path := c.Context().URI().LastPathSegment()
path := c.RequestCtx().URI().LastPathSegment()
gaby marked this conversation as resolved.
Show resolved Hide resolved
size, err := strconv.Atoi(string(path))
require.NoError(t, err)
return c.Send(make([]byte, size))
Expand Down
Loading
Loading