gorilla / schema

Package gorilla/schema fills a struct with form values.
https://gorilla.github.io
BSD 3-Clause "New" or "Revised" License
1.39k stars 231 forks source link

[feature] default values for scheme struct fields #182

Closed zak905 closed 8 months ago

zak905 commented 3 years ago

Is your feature request related to a problem? Please describe.

in case values are not set for a non required value, it would be nice to have the possibility to set a default value using a tag, for example schemaDefault or just default

Describe the solution you'd like

type Person struct {
    Name  string `schema:"name,required"`
    Phone string `schema:"phone" schemaDefault:"+1234567890"`
}

update (12-03-2022)

the design has been slightly changed to use a tag option rather than a standalone tag

type Person struct {
    Name  string `schema:"name,required"`
    Phone string `schema:"phone,default=+1234567890"`
}
zak905 commented 3 years ago

Here a fist attempt: https://github.com/zak905/schema/commit/66f64fe87c96b3d214096829e63dfbc1e4340e20

I made the following assumptions:

Let me know if have any comments and if you would be interested in adding this to upstream.

zak905 commented 3 years ago

I added some improvements. Now, the default values are set for nested structs (or pointer to struct) fields: https://github.com/zak905/schema/commit/36c9802c2068b86e8babaa00525900653590754c

zak905 commented 3 years ago

PR submitted. Looking forward to hear the community's feedback

stale[bot] commented 2 years ago

This issue has been automatically marked as stale because it hasn't seen a recent update. It'll be automatically closed in a few days.

zak905 commented 2 years ago

hey stale bot, please remove the stale tag

DavidLarsKetch commented 2 years ago

Really interesting feature. Two things I'm curious about:

zak905 commented 2 years ago

Hi @DavidLarsKetch,

Thanks for the feedback.

what are some use cases you considered for this feature? For decoding from HTML-based POSTs, a client can readily provide default values for a form input (e.g., ). For encoding, I can see how this provides an extra convenience, but just as well could see how a constructor (e.g., func NewPerson() Person) would be a preferable path in a codebase.

schema is meant for form values, but can be also be used to parse a request's query parameters as a struct.

decoder.Decode(params, r.URL.Query())

Either case, this PR aims to make decoding more convenient. Yes it's possible to provide values in forms, but in case the process is to be handled backend side. It think it makes it not only simpler, but also self-explanatory. You can just checkout the struct and see which values has defaults. Instead of having to if-else every struct type to check and set default values, you could have for example a generic parameters struct of type interface{} that you decode in a middleware and put in the context. You could then cast to your struct, for example, based on the endpoint.

what led to introducing a new struct tag? I see you considered parsing the tag read by this package (schema), but opted for adding default. Were there any limitations in the codebase preventing doing, e.g., schema:"phone,default=+12334555"?

If I remember well, there are at least two reasons. The first is the additional parsing, and the second is the order of options. The current tag options parsing puts all options in an array in the order they are provided, and therefore you have to loop a second time to find the default option. So I guess a tag requires less work, but I don't mind changing if the option alternative is more acceptable semantically.

DavidLarsKetch commented 2 years ago

I was following with the use case pretty well and agree, prima facie, that there's a clarity to signalling a default value, but got a bit hung up on this line:

you could have for example a generic parameters struct of type interface{} that you decode in a middleware and put in the context. You could then cast to your struct, for example, based on the endpoint. I'm unclear on the need for a middleware here, or, is this just an alternative pattern?

With regards to the struct tag, from my experience, I have not used a package exposing more than 1 struct tag and instead am familiar with packages that expose 1 struct tag and use a separator like , or ; (e.g., json:foobar,omitempty). For example, while unlikely, if someone were to use this package alongside schema (with your changes in #183) to supply filling in default values, the struct tags would collide.

Looking at #183 more specifically, I noticed you omit supporting default values for structs, pointers, and slices. The first two I have a hard time envisioning how that would work, but a default slice of strings, for example, seems less of a challenge.

zak905 commented 2 years ago

I'm unclear on the need for a middleware here, or, is this just an alternative pattern?

No there is no necessity for middleware, it just happens that this is the way I am using it in some projects, so I was just giving an example.

It should not be complicated changing from a tag to a tag option, I admit I was a bit lazy on this one:) `schema:"phone,default=+12334555" sounds like more golang style indeed.

If we are to introduce slices, then I guess we have to support also slices for all primitive types, not only strings. How would we go about delimiters? I think a comma , would be confusing since comma is used to separate options : schema:"list,default=1,2,4"

aigoya commented 2 years ago

I create my own defaultvalue:"value" tag and before bind I set that default value tag in the url. Values with reflect. It's look ugly, but It works pretty well.

zak905 commented 2 years ago

cool, so we all agree that a tag option is less cumbersome than a standalone tag

zak905 commented 2 years ago

Hi @DavidLarsKetch,

as per our latest discussion, I made the following changes:

Once we finalize the above, I can update the README

zak905 commented 8 months ago

Done in #183 and #209