✨ v3 (feature): request binding

[trim21]

Please provide enough information so that others can review your pull request:

related to https://github.com/gofiber/fiber/issues/1821 and https://github.com/gofiber/fiber/issues/2002

Explain the details for making this change. What existing problem does the pull request solve?

this PR add a function decorator style bind method to provide high speed and low alloc request binding.

this PR add a high performance and low alloc binding implement.

some changes:

1. data[]=john&data[]=doe won't bind to query:"data", only bind to query:"data[]" and can't bind query like data[0]=john&data[1]=doe
2. you can't used multiple tag together, only one of them will work.
3. function decorators will panic if any misused happened (eg req is not a struct type). I think there is no meaning to return a error because developer must change their code instead of handling there errors.

TODO:

• [x] document
• [x] better error message
• [x] bind request body
• [x] validate
• [x] form
• [x] multipart
• [x] Add some of old methods like GetHeaders etc.
• [ ] Pointer support for struct fields.
• [ ] Add more tests and benchmarks.
• [ ] Better nested binding behavior like current binding
  • [x] Support nested.and.age, nested.age queries
    • [x] Compiler support.
    • [x] Decoder support.
  • [ ] Support data.0.name data.0.users.0.name and queries
    • [x] Compiler support.
    • [ ] Decoder support.
  • [ ] Support data[0][name], nested[and][age] queries (we can convert dots to square brackets or the opposite)

TODO for Extra Enhancements:

• [ ] Multipart file binding.
• [ ] Support map binding
• [ ] Default support.

[trim21]

Do we need to support multiples content type automatically binding like the old way? I'm thinking this may cause handle unmarshal some struct with with unexpected content type.

Like a struct with only XML or no tag at all may unmarshal json if client send that content-type header. 🤔

[trim21]

shoule we hide the original unmarshal message from encoding.TextUnmarshaler.UnmarshalText or strconv.Parse{Int|Uint|Bool}?

[efectn]

I reviewed this a bit today and this way seems good to me but i have a little question. Why don't we support map binding? I think we can add map support when checking the type in compiler.go

[efectn]

And we don't need default validator i think. Playground validator has too much and big sized indirect dependencies (https://github.com/gin-gonic/gin/issues/3165). But we can support custom validators by an struct validator interface.

[trim21]

I wrap a interface {Struct(s any) error} in production code, it's only used in example_test.go, and go mod won't fetch test packages by default.

[trim21]

Why don't we support map binding

I think we can add old methods like ctx.GetRespHeaders back for this use case.

[trim21]

This PR only add JSON and XML body type, The multipart and form body need a new decoder, I'll add them in the next PR.

[trim21]

Btw we can try to implement form, multipart form in this pr

I add form and multipart support, multipart files are not handled.

[ReneWerner87]

@Trim21 can you update this again, will look at it next week, unfortunately had to do with the other issues and pull requests until now

thank you in any case already for the invested time

[efectn]

Hey @Trim21 can you check the discord

[ReneWerner87]

thank you for your work

this pull request is more complex than expected and i will probably need even longer for the review i have already made some comments which you can please have a look at

my first impression is that we need a very good documentation here and please give me some examples that will help me to fully understand the possibilities of this customization

due to the fact that there are many possibilities here, we need to manage the consumer well so that there are few queries as there are many types and levels of developers

[ReneWerner87]

and in any case a lot of unittests to make sure with the mass of code lines that there are no or few errors in the logic and that we don't have to make any bugfix releases.

[trim21]

and in any case a lot of unittests to make sure with the mass of code lines that there are no or few errors in the logic and that we don't have to make any bugfix releases.

I'll add a testable example

[trim21]

nested binding

can you explain this? like binding ?user.first_name=john&user.last_name=doe to User struct { FirstName string; LastName string; } ?

[trim21]

i have already made some comments which you can please have a look at

@ReneWerner87 did you forget to finally submit review comments in github ui? I can't see any review comments except 2 comments your directly replied.

[ReneWerner87]

Yes sorry

[trim21]

and in any case a lot of unittests to make sure with the mass of code lines that there are no or few errors in the logic and that we don't have to make any bugfix releases.

I saw my PR again and looks like current all supported types are coveraged by tests already

[efectn]

I preapred a list of tasks. What do you think @trim21

• Map binding (will check it asap)
• Multipart file binding
• Better nested binding behavior like current binding
• Specific binding methods like current binding of v3
• More tests and benchmarks (i can do it)

[trim21]

I'm woking on multipart file binding now.

Why do we add a map binding insteading of adding a util function FasthttpArgsToMap(interface{VisitAll})map[string]...?

[ReneWerner87]

I'm woking on multipart file binding now.

Why do we add a map binding insteading of adding a util function FasthttpArgsToMap(interface{VisitAll})map[string]...?

sure good idea

or maybe we use just https://github.com/savsgio/dictpool

can you test both ?

[trim21]

sure good idea

or maybe we use just https://github.com/savsgio/dictpool

can you test both ?

I don't think github.com/savsgio/dictpool fit this case ...

[trim21]

• Specific binding methods like current binding of v3

currently binding is controlled by struct tag, there is no need to add old API like Query, Params.

[efectn]

I'm woking on multipart file binding now.

Why do we add a map binding insteading of adding a util function FasthttpArgsToMap(interface{VisitAll})map[string]...?

I think it would be more useful to adding map binding instead of an util. Otherwise we have to add some methods to shortify getting them.

• Specific binding methods like current binding of v3

currently binding is controlled by struct tag, there is no need to add old API like Query, Params.

Oh, right. Controlling by struct tag seems ok

[ReneWerner87]

thanks for the patience with my review and the work you put in here ❤️

please update the branch again and just let me know when the pull request is ready for further review

[trim21]

Got little busy these days, will update it soon

[efectn]

I think we should add GetHeaders, GetRespHeaders, GetParams back. They are useful for something needs map. Should we add them into the ctx or Bind (like Bind().GetHeaders())? @ReneWerner87 @trim21

[efectn]

Hi @trim21. I added simple nested binding support. Can you review my last commit?

I also plan to add support for slices as soon as possible.

[FGYFFFF]

Hello~ The idea of implementing v3's bind is really great! I have an idea I'd like to discuss.

	type Req struct {
		F1  *string `param:"f1" query:"f1" header:"f1"`
	}

the binding order is: "query"、 "param" 、"header".

If the 'query' missed the bind, the field will not be decoded. So is it possible to continue trying to bind the "param" parameter after not binding to the query parameter. This is a "best effort delivery" concept.

In addition, I think this 'pr' is very meaningful, if necessary I can also participate in

[efectn]

Hello~ The idea of implementing v3's bind is really great! I have an idea I'd like to discuss.

	type Req struct {
		F1  *string `param:"f1" query:"f1" header:"f1"`
	}

the binding order is: "query"、 "param" 、"header".

If the 'query' missed the bind, the field will not be decoded. So is it possible to continue trying to bind the "param" parameter after not binding to the query parameter. This is a "best effort delivery" concept.

In addition, I think this 'pr' is very meaningful, if necessary I can also participate in

Hi @FGYFFFF. You can help to complete this PR. I don't have much time nowadays so i couldn't find time to finish the PR. Feel free to help us <3

[FGYFFFF]

@trim21 Can you give me access to a push code? I want to add support for the "float" type.