Query, Header Default Value Tag Feature

[muratmirgun]

Description

This Feature provides an approach in Go to assign default values to the fields of structs. The tagHandlers function assigns these default values based on the specific type of a field. To enhance performance, we cache which fields of the structs have default values (structCache), so we don't have to repeatedly look up these fields every time. Because Handlers Query structs not change

Fixes: #2571

Type of change

Please delete options that are not relevant.

• [x] New feature (non-breaking change which adds functionality)

Checklist:

• [ ] For new functionalities I follow the inspiration of the express js framework and built them similar in usage
• [ ] I have performed a self-review of my own code
• [x] I have commented my code, particularly in hard-to-understand areas
• [x] I have made corresponding changes to the documentation - /docs/ directory for https://docs.gofiber.io/
• [x] I have added tests that prove my fix is effective or that my feature works
• [x] New and existing unit tests pass locally with my changes
• [x] If new dependencies exist, I have checked that they are really necessary and agreed with the maintainers/community (we want to have as few dependencies as possible)
• [x] I tried to make my code as fast as possible with as few allocations as possible
• [x] For new code I have written benchmarks so that they can be analyzed and improved

Commit formatting:

Use emojis on commit messages so it provides an easy way of identifying the purpose or intention of a commit. Check out the emoji cheatsheet here: https://gitmoji.carloscuesta.me/

[welcome[bot]]

Thanks for opening this pull request! 🎉 Please check out our contributing guidelines. If you need help or want to chat with us, join us on Discord https://gofiber.io/discord

[ReneWerner87]

Thx

Can you share the benchmark results How big is the impact for cases without defaults, do we have pay for that as well?

Can you put the default value handling and the tests in a separate go file (don't like big files, I'm more of a code divider)? If we have this built in for queryparser, it would probably make sense to build it in for the other parsers (body & header) as well.

Should we add a config setting so you can disable this handling to get the maximum performance if you don't need defaults or have your own handling?

[muratmirgun]

Thx

Can you share the benchmark results How big is the impact for cases without defaults, do we have pay for that as well?

Can you put the default value handling and the tests in a separate go file (don't like big files, I'm more of a code divider)? If we have this built in for queryparser, it would probably make sense to build it in for the other parsers (body & header) as well.

Should we add a config setting so you can disable this handling to get the maximum performance if you don't need defaults or have your own handling?

First, let me share the benchmark results. but These examples for ColdStart

Benchmark_Ctx_QueryParser Benchmark_Ctx_QueryParser-8 653192 1621 ns/op 856 B/op 38 allocs/op Benchmark_Ctx_QueryParser-8 751180 1611 ns/op 856 B/op 38 allocs/op Benchmark_Ctx_QueryParser-8 753249 1563 ns/op 856 B/op 38 allocs/op Benchmark_Ctx_QueryParser-8 771404 1646 ns/op 856 B/op 38 allocs/op

Benchmark_Ctx_QueryParserWithDefaultValues Benchmark_Ctx_QueryParserWithDefaultValues-8 974080 1239 ns/op 720 B/op 25 allocs/op Benchmark_Ctx_QueryParserWithDefaultValues-8 941806 1245 ns/op 720 B/op 25 allocs/op Benchmark_Ctx_QueryParserWithDefaultValues-8 963400 1220 ns/op 720 B/op 25 allocs/op Benchmark_Ctx_QueryParserWithDefaultValues-8 963571 1236 ns/op 720 B/op 25 allocs/op

Benchmarks for multiple Request Scenario:

Benchmark_Ctx_QueryParser Benchmark_Ctx_QueryParser-8 14372 78933 ns/op 42817 B/op 1900 allocs/op Benchmark_Ctx_QueryParser-8 15265 77700 ns/op 42817 B/op 1900 allocs/op Benchmark_Ctx_QueryParser-8 15346 78469 ns/op 42817 B/op 1900 allocs/op Benchmark_Ctx_QueryParser-8 15432 78051 ns/op 42817 B/op 1900 allocs/op

Benchmark_Ctx_QueryParserWithDefaultValues Benchmark_Ctx_QueryParserWithDefaultValues-8 19756 60628 ns/op 36018 B/op 1250 allocs/op Benchmark_Ctx_QueryParserWithDefaultValues-8 19581 60628 ns/op 36018 B/op 1250 allocs/op Benchmark_Ctx_QueryParserWithDefaultValues-8 19682 60993 ns/op 36017 B/op 1250 allocs/op Benchmark_Ctx_QueryParserWithDefaultValues-8 19599 60733 ns/op 36018 B/op 1250 allocs/op

With multiple

Apart from that, we can do it for all parsers, it will be logical and we can do them with a separate file logic. In the cold start logic, there is a small cost involuntarily at first until the first cache settles, but there will be no problem after the request comes to all the handlers that contain the default structure.

If we are going to integrate it into all parse structures, we can control this with the config for users who are worried about speed. If we think about the structure we're talking about, I can do that.

[ReneWerner87]

Can you share the performance without and with your function for the queryparser (i.e. the times from before vs. after the adjustment)

[muratmirgun]

Can you share the performance without and with your function for the queryparser (i.e. the times from before vs. after the adjustment)

Sorry, I forgot it. You can check in your computer from my fork

Benchmark_Ctx_QueryParser Benchmark_Ctx_QueryParser-8 679710 1629 ns/op 856 B/op 38 allocs/op Benchmark_Ctx_QueryParser-8 740930 1625 ns/op 856 B/op 38 allocs/op Benchmark_Ctx_QueryParser-8 752588 1584 ns/op 856 B/op 38 allocs/op Benchmark_Ctx_QueryParser-8 763689 1578 ns/op 856 B/op 38 allocs/op Benchmark_Ctx_QueryParser_Comma Benchmark_Ctx_QueryParser_Comma-8 665384 1764 ns/op 928 B/op 44 allocs/op Benchmark_Ctx_QueryParser_Comma-8 690904 1766 ns/op 928 B/op 44 allocs/op Benchmark_Ctx_QueryParser_Comma-8 678898 1767 ns/op 928 B/op 44 allocs/op Benchmark_Ctx_QueryParser_Comma-8 683109 1754 ns/op 928 B/op 44 allocs/op PASS ok github.com/gofiber/fiber/v2 9.871s

[gaby]

@muratmirgun Can you take a look at the ci failures

[ReneWerner87]

move the functionality to the utils folder

if you can then please add the function for all parsers and add a config setting

[muratmirgun]

move the functionality to the utils folder

if you can then please add the function for all parsers and add a config setting

Okey I am working on it! 🚀

[ReneWerner87]

just change the PR type from draft in the normal one, when you are finished with the PR

[muratmirgun]

just change the PR type from draft in the normal one, when you are finished with the PR

Yes I know thanks for remember 💯

[muratmirgun]

I saw linter results and I am working on it @efectn

[muratmirgun]

Mostly seems OK to me. Perhaps we can add more features in the future.

If all is okay four you. I merge the PR and after that, I fix cases on other PR. If there are additional improvements or issues on this side, I will follow the issuers and make the necessary improvements.

[muratmirgun]

@efectn I don't have merge access you can merge

[muratmirgun]

I have posted the fix code for int slice parser function you can check it. now test codes will not give error

[ReneWerner87]

@efectn I don't have merge access you can merge

Will look again tomorrow and merge then

[ReneWerner87]

@muratmirgun can you please look at the lint errors

2. comments

• why do we set the default values after the filling process? it should be easier beforehand and then there is no chance that something will be overwritten by faulty code
• can you add the lines for the BodyParser, ParamParser and CookieParser, it should only be a small block which is added + tests

[muratmirgun]

@ReneWerner87 Yes, I will work on the bugs and look at the additions you mentioned

[muratmirgun]

@ReneWerner87 Sorry for delay. I've been a little busy because of my job change, so I will update the PR shortly.

[ReneWerner87]

@muratmirgun did you have time to look again?

[muratmirgun]

@muratmirgun did you have time to look again?

Hi, I'll work on it today

[ReneWerner87]

@muratmirgun could you check the gosec hints ?

[rohitChaku]

Hello, while exploring the implementation of default values in Gin, I found it quite elegant. I'm wondering if we could draw inspiration from some of its constructs, particularly in terms of syntax. The logic appears to be quite similar, as seen in form_mapping.go. What are your thoughts on incorporating similar syntax into this project?

I'm relatively new to Golang and open source, so I want to ensure my suggestion aligns with the community's spirit. If there's any concern or if this isn't appropriate, please let me know, and I'll promptly remove my comment. Thanks.

[ReneWerner87]

@muratmirgun can you check and consider the last comments, thanks for your work, any help is appreciated

[ReneWerner87]

• [x] pls check the other parsers https://github.com/gofiber/fiber/pull/2699#issuecomment-1805345603
• [x] can we check https://github.com/gofiber/fiber/pull/2699#discussion_r1422480231 again -> fixed with https://github.com/gofiber/fiber/pull/2699/commits/ad89ced1a32fb033133ef1afe3c219a5cfeea702
• [x] one last lint error https://github.com/gofiber/fiber/actions/runs/7310291693/job/20152420546?pr=2699

[muratmirgun]

Hi, I am sorry for being offline lately. I have had some health problems and I have seen your articles and now I am looking at the example given. @ReneWerner87

[ReneWerner87]

Hi, I am sorry for being offline lately. I have had some health problems and I have seen your articles and now I am looking at the example given. @ReneWerner87

no problem, the pull request should be almost ready i have done the last steps with the other maintainers

health always comes first of course, hope you are well again

[ReneWerner87]

[muratmirgun]

Hi, I am sorry for being offline lately. I have had some health problems and I have seen your articles and now I am looking at the example given. @ReneWerner87

no problem, the pull request should be almost ready i have done the last steps with the other maintainers

health always comes first of course, hope you are well again

thank you very much 🙏 we will move forward again in the next issues🚀

[ReneWerner87]

@muratmirgun maybe you can help, i found out 2 important things through the review hints

1. something is not thread safe -> added parallel test execution and analyzed the problems, unfortunately I can't find the right solution if i try to correct it i get into a deadlock when running the tests because something is wrong with the mutex hirachy

2. the defaults cannot be applied deeply, which is what you would expect if you use the function from the utils directly, for example -> I have created a test that proves this and helps to correct the problem

furthermore, i noticed that the performance could be optimized even further, instead of parsing and converting the value from the default tag every time, you could keep it or create a slice with closures (instead of the structElementCache), which is later only run through with the element and sets the values

therefore the go-defaults also used the filler

however, the biggest problem is that the functionality is not thread safe

[muratmirgun]

I saw your last commits and new test cases I am looking for your comment maybe we can make a mechanism where we can separate the default parts. I've started working on it. @ReneWerner87 . I will push new methods asap