Skip to content

🔥 feat: Add All method to Bind #3373

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

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
57 changes: 57 additions & 0 deletions bind.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package fiber

import (
"reflect"

"github.com/gofiber/fiber/v3/binder"
"github.com/gofiber/utils/v2"
)
Expand Down Expand Up @@ -273,3 +275,58 @@
// No suitable content type found
return ErrUnprocessableEntity
}

func (b *Bind) All(out any) error {
Copy link
Member

Choose a reason for hiding this comment

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

Add function to docs

outVal := reflect.ValueOf(out)
if outVal.Kind() != reflect.Ptr || outVal.Elem().Kind() != reflect.Struct {
return ErrUnprocessableEntity
}

outElem := outVal.Elem()

// Precedence: URL Params -> Body -> Query -> Headers -> Cookies
sources := []func(any) error{
b.URI,
b.Body,
b.Query,
b.Header,
b.Cookie,
}

tempStruct := reflect.New(outElem.Type()).Interface()
Copy link
Preview

Copilot AI Apr 21, 2025

Choose a reason for hiding this comment

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

The temporary struct (tempStruct) is created once outside the loop and then reused for each binding source. This may cause previous binding values to carry over into subsequent bindings. Consider reinitializing tempStruct inside the loop for each source to ensure isolated binding.

Copilot is powered by AI, so mistakes are possible. Review output carefully before use.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The purpose of tempStruct is to act as a temporary holder for each source binding (such as URI, Body, Query, etc.) and merge it into the final outElem struct.


// TODO: Support custom precedence with an optional binding_source tag
// TODO: Create WithOverrideEmptyValues
// Bind from each source, but only update unset fields
for _, bindFunc := range sources {

Check failure on line 301 in bind.go

View workflow job for this annotation

GitHub Actions / lint

empty-lines: extra empty line at the start of a block (revive)

if err := bindFunc(tempStruct); err != nil {
return err
}

tempStructVal := reflect.ValueOf(tempStruct).Elem()
mergeStruct(outElem, tempStructVal)
}

return nil
}
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

⚠️ Potential issue

Handle “no body” and similar benign errors so All doesn’t abort early

b.Body() (and the other single‑source helpers) can legitimately return
ErrUnprocessableEntity, ErrEmptyBody, etc. when the incoming request
simply doesn’t contain data for that source.
All should treat those as non‑fatal and continue with the next source;
otherwise a perfectly valid request that, say, only has query params will
fail the whole bind.

-		if err := bindFunc(tempStruct); err != nil {
-			return err
-		}
+		if err := bindFunc(tempStruct); err != nil {
+			// Skip “nothing to bind”‑style errors and continue.
+			if errors.Is(err, ErrUnprocessableEntity) ||
+				errors.Is(err, binder.ErrEmptyBody) {
+				continue
+			}
+			return err
+		}

(You may need to import errors and, if available, the concrete “empty body”
error from the binder package.)

🧰 Tools
🪛 GitHub Check: lint

[failure] 301-301:
empty-lines: extra empty line at the start of a block (revive)

🪛 GitHub Actions: golangci-lint

[error] 301-301: golangci-lint (revive): extra empty line at the start of a block (empty-lines)


func mergeStruct(dst, src reflect.Value) {
dstFields := dst.NumField()
for i := 0; i < dstFields; i++ {
dstField := dst.Field(i)
srcField := src.Field(i)

// Skip if the destination field is already set
if isZero(dstField.Interface()) {
Copy link
Preview

Copilot AI Apr 21, 2025

Choose a reason for hiding this comment

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

Using isZero to decide whether to update a field might unintentionally overwrite valid zero values (e.g., 0 for an int). Reevaluate the logic for what constitutes an 'unset' field, possibly by using pointer types or an explicit flag.

Suggested change
if isZero(dstField.Interface()) {
if (dstField.Kind() == reflect.Ptr && dstField.IsNil()) || (dstField.Kind() != reflect.Ptr && isZero(dstField.Interface())) {

Copilot is powered by AI, so mistakes are possible. Review output carefully before use.

if dstField.CanSet() && srcField.IsValid() {
dstField.Set(srcField)
}
}
}
}

func isZero(value any) bool {
v := reflect.ValueOf(value)
return v.IsZero()
}
49 changes: 49 additions & 0 deletions bind_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1885,3 +1885,52 @@
testDecodeParser(MIMEApplicationForm, "body_param=body_param")
testDecodeParser(MIMEMultipartForm+`;boundary="b"`, "--b\r\nContent-Disposition: form-data; name=\"body_param\"\r\n\r\nbody_param\r\n--b--")
}

func TestBind_All(t *testing.T) {
Copy link
Member

Choose a reason for hiding this comment

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

Can you create some benchmarks

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure

type User struct {

Check failure on line 1890 in bind_test.go

View workflow job for this annotation

GitHub Actions / lint

fieldalignment: struct with 80 pointer bytes could be 64 (govet)
ID int `param:"id" query:"id" json:"id" form:"id"`
Name string `query:"name" json:"name" form:"name"`
Email string `json:"email" form:"email"`
Role string `header:"x-user-role"`
SessionID string `json:"SessionID" cookie:"session_id"`
Avatar *multipart.FileHeader `form:"avatar"`
}
app := New()
c := app.AcquireCtx(&fasthttp.RequestCtx{})
tests := []struct {

Check failure on line 1900 in bind_test.go

View workflow job for this annotation

GitHub Actions / lint

fieldalignment: struct with 40 pointer bytes could be 32 (govet)
name string
bind *Bind
out any
wantErr bool
}{
{
name: "Invalid output type",
bind: &Bind{},
out: 123,
wantErr: true,
},
{
// Validate this test case
name: "Successful binding",
bind: &Bind{
ctx: c,
},
out: &User{
ID: 10,
Name: "Alice",
Email: "[email protected]",
Role: "admin",
SessionID: "abc123",
},
wantErr: false,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if err := tt.bind.All(tt.out); (err != nil) != tt.wantErr {
t.Errorf("Bind.All() error = %v, wantErr %v", err, tt.wantErr)
}
})
}
}
Loading