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.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

馃敟 Add function to override form decoder setting #1100

Merged

Conversation

rockcreation7
Copy link
Contributor

@rockcreation7 rockcreation7 commented Jan 3, 2021

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

  • SetParserDecoder function map all option to form decoder, which allow you to change decoder option globally, use with type ParserConfig, ParserType.

  • Added Test_Ctx_BodyParser_WithSetParserDecoder for testing

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

  • You can do custom type in JSON format, Create custom type and related "UnmarshalJSON" method, But in form you need to register converter in decoder, I added option for supporting this feature. In order to solve problem when you need custom type for form.

  • Allow set the decoder's zeroEmpty to true, allow not skipping empty input as it will overwrite existing value. 馃悰 BodyParser does not save empty string to field聽#1084

type CustomTime time.Time

var timeConverter = func(value string) reflect.Value {
	if v, err := time.Parse("2006-01-02", value); err == nil {
		return reflect.ValueOf(v)
	}
	return reflect.Value{}
}

customType := ParserType{
		Customtype: CustomType{},
		Converter:  timeConverter,
}

SetParserDecoder(
         BodyParserConfig{
		IgnoreUnknownKeys: true,
		BodyParserType:    []ParserType{customType},
		ZeroEmpty:         true,
		SetAliasTag:       "form",
	}
)

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/

馃敟 Feature : SetBodyParserDecoder map all option to form decoder, use with BodyParserConfig, BodyParserType

馃毃 Test : Test_Ctx_BodyParser_WithSetBodyParserDecoder
@rockcreation7 rockcreation7 changed the title 馃敟 add function to overide form decoder setting 馃敟 Add function to override form decoder setting Jan 3, 2021
@rockcreation7
Copy link
Contributor Author

Any suggestion for moving forward on this PR will be appreciated, or any idea should be further investigate, wish to provide help, Thanks.

@stale
Copy link

stale bot commented May 22, 2021

馃憢 Hello. Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
Thank you for your contributions.

Copy link

@Israel-Laguan Israel-Laguan left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -244,6 +259,22 @@ var decoderPool = &sync.Pool{New: func() interface{} {
return decoder
}}

// SetBodyParserDecoder allow globally change the option of form decoder, update decoderPool
func SetBodyParserDecoder(bodyParserConfig BodyParserConfig) {
Copy link
Member

Choose a reason for hiding this comment

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

can we use the method for the initial write of the decoder pool ?
https://github.com/gofiber/fiber/blob/master/ctx.go#L274
@rockcreation7

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, I can help update the doc on this, thanks.

Please feel free to let me know anything that can help.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

or you mean I put a change to the initial write?

Copy link
Member

Choose a reason for hiding this comment

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

or you mean I put a change to the initial write?

@rockcreation7
yes exactly, with the initial set you can also use your method or ?

will merge it directly after the adjustment

@ReneWerner87 ReneWerner87 merged commit 35e38db into gofiber:master Oct 1, 2021
@ReneWerner87 ReneWerner87 linked an issue Oct 1, 2021 that may be closed by this pull request
@ReneWerner87
Copy link
Member

@rockcreation7 can you rename it a bit, because the decoder is used by the query parser and body parser

maybe something just called parser config or type

@ReneWerner87 ReneWerner87 linked an issue Oct 1, 2021 that may be closed by this pull request
@rockcreation7
Copy link
Contributor Author

Thanks, will update.

@rockcreation7
Copy link
Contributor Author

#1555

Renamed function:
SetBodyParserDecoder to SetParserDecoder

Type:
BodyParserType > ParserType
bodyParserConfig > parserConfig
BodyParserConfig > ParserConfig

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

馃悰 Can't register a custom converter 馃 How can sql NullString types be handled in c.BodyParser ?
5 participants