-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
🔥 Feature: Add Req and Res API #2894
base: main
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -54,6 +54,8 @@ | |
fasthttp *fasthttp.RequestCtx // Reference to *fasthttp.RequestCtx | ||
bind *Bind // Default bind reference | ||
redirect *Redirect // Default redirect reference | ||
req *DefaultReq // Default request api reference | ||
res *DefaultRes // Default response api reference | ||
values [maxParams]string // Route parameter values | ||
viewBindMap sync.Map // Default view map to bind template engine | ||
method string // HTTP method | ||
|
@@ -1463,6 +1465,18 @@ | |
} | ||
} | ||
|
||
// Res returns a convenience type whose API is limited to operations | ||
// on the incoming request. | ||
func (c *DefaultCtx) Req() Req { | ||
return c.req | ||
} | ||
|
||
// Res returns a convenience type whose API is limited to operations | ||
// on the outgoing response. | ||
func (c *DefaultCtx) Res() Res { | ||
return c.res | ||
} | ||
Comment on lines
+1474
to
+1478
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Codebase verification Add test coverage for the Res() method The review comment is correct. The codebase search reveals that while there are interface and implementation definitions for
🔗 Analysis chainAdd test coverage for the Res() method While the implementation is correct and the documentation is clear, test coverage is missing for this method. Please add unit tests to ensure the method correctly returns the res field. Would you like me to help generate unit tests for this method? 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Description: Verify test coverage for Res() method
# Test: Search for test cases covering Res(). Expect: Test cases for the method.
rg --type go 'func Test.*Res\(' app/
Length of output: 83 Script: #!/bin/bash
# First, let's find the correct location of test files
fd -e go test
# Then search for any existing test coverage of Res() method
rg --type go 'func Test.*Res\('
# Also search for any usage of Res() in test files to catch potential indirect testing
rg --type go '\.Res\(' -g '*test.go'
Length of output: 2109 Script: #!/bin/bash
# Let's check ctx_test.go specifically since Res() is defined in ctx.go
cat ctx_test.go | grep -A 10 -B 10 "Res()"
# Also check for any indirect testing of the response functionality
rg --type go "c\.res\b" ctx_test.go
# Let's also look for any tests that might be using the Res type
rg --type go "type.*Res\b" -A 5
Length of output: 620 🧰 Tools🪛 GitHub Check: codecov/patch[warning] 1476-1477: ctx.go#L1476-L1477 |
||
|
||
// Route returns the matched Route struct. | ||
func (c *DefaultCtx) Route() *Route { | ||
if c.route == nil { | ||
|
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
|
@@ -35,6 +35,8 @@ func NewDefaultCtx(app *App) *DefaultCtx { | |||||||||
return &DefaultCtx{ | ||||||||||
// Set app reference | ||||||||||
app: app, | ||||||||||
req: &DefaultReq{}, | ||||||||||
res: &DefaultRes{}, | ||||||||||
Comment on lines
+38
to
+39
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix initialization of req and res fields The current initialization creates empty structs without setting the context reference. This will likely cause issues when methods try to access the context. Apply this diff to properly initialize the fields: - req: &DefaultReq{},
- res: &DefaultRes{},
+ req: &DefaultReq{ctx: c},
+ res: &DefaultRes{ctx: c}, 📝 Committable suggestion
Suggested change
|
||||||||||
} | ||||||||||
} | ||||||||||
|
||||||||||
|
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,110 @@ | ||
package fiber | ||
|
||
//go:generate ifacemaker --file req.go --struct DefaultReq --iface Req --pkg fiber --output req_interface_gen.go --not-exported true --iface-comment "Req" | ||
type DefaultReq struct { | ||
ctx *DefaultCtx | ||
} | ||
Comment on lines
+4
to
+6
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Consider adding validation for ctx field. The type DefaultReq struct {
ctx *DefaultCtx
}
+
+func NewDefaultReq(ctx *DefaultCtx) (*DefaultReq, error) {
+ if ctx == nil {
+ return nil, errors.New("context cannot be nil")
+ }
+ return &DefaultReq{ctx: ctx}, nil
+}
|
||
|
||
func (r *DefaultReq) Accepts(offers ...string) string { | ||
return r.ctx.Accepts(offers...) | ||
} | ||
|
||
func (r *DefaultReq) AcceptsCharsets(offers ...string) string { | ||
return r.ctx.AcceptsCharsets(offers...) | ||
} | ||
|
||
func (r *DefaultReq) AcceptsEncodings(offers ...string) string { | ||
return r.ctx.AcceptsEncodings(offers...) | ||
} | ||
|
||
func (r *DefaultReq) AcceptsLanguages(offers ...string) string { | ||
return r.ctx.AcceptsLanguages(offers...) | ||
} | ||
|
||
func (r *DefaultReq) BaseURL() string { | ||
return r.ctx.BaseURL() | ||
} | ||
|
||
func (r *DefaultReq) Body() []byte { | ||
return r.ctx.Body() | ||
} | ||
|
||
func (r *DefaultReq) Cookies(key string, defaultValue ...string) string { | ||
return r.ctx.Cookies(key, defaultValue...) | ||
} | ||
|
||
func (r *DefaultReq) Fresh() bool { | ||
return r.ctx.Fresh() | ||
} | ||
|
||
func (r *DefaultReq) Get(key string, defaultValue ...string) string { | ||
return r.ctx.Get(key, defaultValue...) | ||
} | ||
|
||
func (r *DefaultReq) Host() string { | ||
return r.ctx.Host() | ||
} | ||
|
||
func (r *DefaultReq) Hostname() string { | ||
return r.ctx.Hostname() | ||
} | ||
|
||
func (r *DefaultReq) IP() string { | ||
return r.ctx.IP() | ||
} | ||
|
||
func (r *DefaultReq) Is(extension string) bool { | ||
return r.ctx.Is(extension) | ||
} | ||
|
||
func (r *DefaultReq) IPs() []string { | ||
return r.ctx.IPs() | ||
} | ||
|
||
func (r *DefaultReq) Method() string { | ||
return r.ctx.Method() | ||
} | ||
|
||
func (r *DefaultReq) OriginalURL() string { | ||
return r.ctx.OriginalURL() | ||
} | ||
|
||
func (r *DefaultReq) Params(key string, defaultValue ...string) string { | ||
return r.ctx.Params(key, defaultValue...) | ||
} | ||
|
||
func (r *DefaultReq) Path() string { | ||
return r.ctx.Path() | ||
} | ||
|
||
func (r *DefaultReq) Protocol() string { | ||
return r.ctx.Protocol() | ||
} | ||
|
||
func (r *DefaultReq) Query(key string, defaultValue ...string) string { | ||
return r.ctx.Query(key, defaultValue...) | ||
} | ||
|
||
func (r *DefaultReq) Range(size int) (Range, error) { | ||
return r.ctx.Range(size) | ||
} | ||
|
||
func (r *DefaultReq) Route() *Route { | ||
return r.ctx.Route() | ||
} | ||
|
||
func (r *DefaultReq) Secure() bool { | ||
return r.ctx.Secure() | ||
} | ||
|
||
func (r *DefaultReq) Stale() bool { | ||
return r.ctx.Stale() | ||
} | ||
|
||
func (r *DefaultReq) Subdomains(offset ...int) []string { | ||
return r.ctx.Subdomains(offset...) | ||
} | ||
|
||
func (r *DefaultReq) XHR() bool { | ||
return r.ctx.XHR() | ||
} | ||
Comment on lines
+1
to
+110
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add unit tests for the request API methods. The static analysis indicates that the new request API methods lack test coverage. Comprehensive testing is essential for validating the delegation behavior. Would you like me to help generate comprehensive unit tests for these methods? 🧰 Tools🪛 GitHub Check: codecov/patch[warning] 8-9: req.go#L8-L9 [warning] 12-13: req.go#L12-L13 [warning] 16-17: req.go#L16-L17 [warning] 20-21: req.go#L20-L21 [warning] 24-25: req.go#L24-L25 [warning] 28-29: req.go#L28-L29 [warning] 32-33: req.go#L32-L33 [warning] 36-37: req.go#L36-L37 [warning] 40-41: req.go#L40-L41 [warning] 44-45: req.go#L44-L45 [warning] 48-49: req.go#L48-L49 [warning] 52-53: req.go#L52-L53 [warning] 56-57: req.go#L56-L57 [warning] 60-61: req.go#L60-L61 [warning] 64-65: req.go#L64-L65 [warning] 68-69: req.go#L68-L69 [warning] 72-73: req.go#L72-L73 [warning] 76-77: req.go#L76-L77 [warning] 80-81: req.go#L80-L81 [warning] 84-85: req.go#L84-L85 [warning] 88-89: req.go#L88-L89 [warning] 92-93: req.go#L92-L93 [warning] 96-97: req.go#L96-L97 [warning] 100-101: req.go#L100-L101 [warning] 104-105: req.go#L104-L105 [warning] 108-109: req.go#L108-L109
Comment on lines
+4
to
+110
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add unit tests for the request API methods. The static analysis indicates that the new request API methods lack test coverage. Comprehensive testing is essential for validating the delegation behavior. Would you like me to help generate comprehensive unit tests for these methods? 🧰 Tools🪛 GitHub Check: codecov/patch[warning] 8-9: req.go#L8-L9 [warning] 12-13: req.go#L12-L13 [warning] 16-17: req.go#L16-L17 [warning] 20-21: req.go#L20-L21 [warning] 24-25: req.go#L24-L25 [warning] 28-29: req.go#L28-L29 [warning] 32-33: req.go#L32-L33 [warning] 36-37: req.go#L36-L37 [warning] 40-41: req.go#L40-L41 [warning] 44-45: req.go#L44-L45 [warning] 48-49: req.go#L48-L49 [warning] 52-53: req.go#L52-L53 [warning] 56-57: req.go#L56-L57 [warning] 60-61: req.go#L60-L61 [warning] 64-65: req.go#L64-L65 [warning] 68-69: req.go#L68-L69 [warning] 72-73: req.go#L72-L73 [warning] 76-77: req.go#L76-L77 [warning] 80-81: req.go#L80-L81 [warning] 84-85: req.go#L84-L85 [warning] 88-89: req.go#L88-L89 [warning] 92-93: req.go#L92-L93 [warning] 96-97: req.go#L96-L97 [warning] 100-101: req.go#L100-L101 [warning] 104-105: req.go#L104-L105 [warning] 108-109: req.go#L108-L109 |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,100 @@ | ||
package fiber | ||
|
||
//go:generate ifacemaker --file res.go --struct DefaultRes --iface Res --pkg fiber --output res_interface_gen.go --not-exported true --iface-comment "Res" | ||
type DefaultRes struct { | ||
ctx *DefaultCtx | ||
} | ||
|
||
func (r *DefaultRes) Locals(key any, value ...any) any { | ||
return r.ctx.Locals(key, value...) | ||
} | ||
|
||
func (r *DefaultRes) Append(field string, values ...string) { | ||
r.ctx.Append(field, values...) | ||
} | ||
|
||
func (r *DefaultRes) Attachment(filename ...string) { | ||
r.ctx.Attachment(filename...) | ||
} | ||
|
||
func (r *DefaultRes) AutoFormat(body any) error { | ||
return r.ctx.AutoFormat(body) | ||
} | ||
|
||
func (r *DefaultRes) CBOR(body any, ctype ...string) error { | ||
return r.ctx.CBOR(body, ctype...) | ||
} | ||
|
||
func (r *DefaultRes) Cookie(cookie *Cookie) { | ||
r.ctx.Cookie(cookie) | ||
} | ||
|
||
func (r *DefaultRes) ClearCookie(key ...string) { | ||
r.ctx.ClearCookie(key...) | ||
} | ||
|
||
func (r *DefaultRes) Download(file string, filename ...string) error { | ||
return r.ctx.Download(file, filename...) | ||
} | ||
|
||
func (r *DefaultRes) Format(handlers ...ResFmt) error { | ||
return r.ctx.Format(handlers...) | ||
} | ||
|
||
func (r *DefaultRes) Get(key string, defaultValue ...string) string { | ||
return r.ctx.GetRespHeader(key, defaultValue...) | ||
} | ||
|
||
func (r *DefaultRes) JSON(body any, ctype ...string) error { | ||
return r.ctx.JSON(body, ctype...) | ||
} | ||
|
||
func (r *DefaultRes) JSONP(data any, callback ...string) error { | ||
return r.ctx.JSONP(data, callback...) | ||
} | ||
|
||
func (r *DefaultRes) Links(link ...string) { | ||
r.ctx.Links(link...) | ||
} | ||
|
||
func (r *DefaultRes) Location(path string) { | ||
r.ctx.Location(path) | ||
} | ||
|
||
func (r *DefaultRes) Render(name string, bind Map, layouts ...string) error { | ||
return r.ctx.Render(name, bind, layouts...) | ||
} | ||
|
||
func (r *DefaultRes) Send(body []byte) error { | ||
return r.ctx.Send(body) | ||
} | ||
|
||
func (r *DefaultRes) SendFile(file string, config ...SendFile) error { | ||
return r.ctx.SendFile(file, config...) | ||
} | ||
|
||
func (r *DefaultRes) SendStatus(status int) error { | ||
return r.ctx.SendStatus(status) | ||
} | ||
|
||
func (r *DefaultRes) Set(key, val string) { | ||
r.ctx.Set(key, val) | ||
} | ||
|
||
func (r *DefaultRes) Status(status int) Res { | ||
r.ctx.Status(status) | ||
return r | ||
} | ||
|
||
func (r *DefaultRes) Type(extension string, charset ...string) Res { | ||
r.ctx.Type(extension, charset...) | ||
return r | ||
} | ||
|
||
func (r *DefaultRes) Vary(fields ...string) { | ||
r.ctx.Vary(fields...) | ||
} | ||
|
||
func (r *DefaultRes) XML(data any) error { | ||
return r.ctx.XML(data) | ||
} | ||
Comment on lines
+4
to
+100
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add unit tests for the response API methods. The static analysis indicates that the new response API methods lack test coverage. This is critical for ensuring the delegation pattern works correctly and maintaining reliability during future changes. Would you like me to help generate comprehensive unit tests for these methods? 🧰 Tools🪛 GitHub Check: codecov/patch[warning] 8-9: res.go#L8-L9 [warning] 12-13: res.go#L12-L13 |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codebase verification
Add test coverage for the Req() method
The method needs test coverage to ensure it correctly returns the req field. Here's a suggested test structure:
ctx_test.go
that:🔗 Analysis chain
Add test coverage for the Req() method
While the implementation is correct and the documentation is clear, test coverage is missing for this method. Please add unit tests to ensure the method correctly returns the req field.
Would you like me to help generate unit tests for this method?
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
Length of output: 83
Script:
Length of output: 2112
Script:
Length of output: 233
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 1470-1471: ctx.go#L1470-L1471
Added lines #L1470 - L1471 were not covered by tests