[FEATURE] default values for custom types

[markbradley27]

Is there an existing feature request for this?

• [X] I have searched the existing feature requests

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

I'd like to use schema to provide default values for custom types that implement encoding.TextUnmarshaler. For example:

type MsTime time.Time

func (mt *MsTime) UnmarshalText(text []byte) error {
    str := string(text)
    if str == "" {
        now := MsTime(time.Now())
        mt = &now
        return nil
    }
    ms, err := strconv.Atoi(str)
    if err != nil {
        return fmt.Errorf("couldn't parse milliseconds string as int: %w", err)
    }
    time := MsTime(time.UnixMilli(int64(ms)))
    *mt = time
    return nil
}

type IntEnum int

const (
    IntEnumFirst  IntEnum = iota
    IntEnumSecond
    IntEnumThird
)

func (ie *IntEnum) UnmarshalText(text []byte) error {
    num, err := strconv.Atoi(string(text))
    if err != nil {
        return fmt.Errorf("couldn't parse string as int: %w", err)
    }
    switch num {
    case int(IntEnumFirst):
        *ll = IntEnumFirst
    case int(IntEnumSecond):
        *ll = IntEnumSecond
    case int(IntEnumThird):
        *ll = IntEnumThird
    default:
        return fmt.Errorf("unexpected enum: %d", num)
    }
    return nil
}

type params struct {
    Timestamp MsTime  `schema:"timestampMs,default:12345"`
    EnumValue IntEnum `schema:"intEnum,default:2`
}

If I try to rely on the default value for Timestamp, I get a helpful but disappointing error message: default option is supported only on: bool, float variants, string, unit variants types or their corresponding pointers or slices

If I try to rely on the default value for EnumValue, this line throws a panic. AFAIU, this is because schema looks up the field's type's kind, which is int, applies the built-in converter for ints, and tries to set the field to the resulting int value. The field isn't an int though, it's an IntEnum, so it panics.

Describe the solution that you would like.

I think both of these could be addressed by simply applying the default values to the incoming form data before sending it through the schema parsing pipeline (this is how I assumed the default fields were being handled, I was surprised to see they were only applied after all of the unmarshalling was completed).

Currently, it looks like form data is sent through the parsing pipeline unadulterated, and if the result of all of that is the zero value, only then is the default value from the struct tag examined, but by a snippet of code that only handles built-in converters.

Instead, the incoming form data could be scanned first, filling in any fields that have default values in the destination struct and are missing from the form data. Then when it gets shipped downstream, all of the same logic that handles custom converters and TextUnmarshallers would operate on the provided default value.

This should also address the awkwardness called out by @klajok over in https://github.com/gorilla/schema/issues/222#issuecomment-2378498127, because if you scan the form data first, you can tell the difference between a form value that was provided but happens to convert to a zero value, and a form value that wasn't provided at all.

Happy to take a stab at implementing this if nobody sees any glaring issues with this approach.

Describe alternatives you have considered.

No response

Anything else?

No response

[zak905]

Hi @markbradley27,

I have worked on the default functionality a while back, so I guess I am the most suited person to answer. Simply put, you can see from the implementation here https://github.com/gorilla/schema/blob/main/decoder.go#L110 that there are plenty of scenarios and cases to handle, some of which need information about the types, which is only available after the "decoding". If you want to detect whether a type implements https://pkg.go.dev/encoding#TextUnmarshaler, then you need type information, which is something done when "decoding". If you are convinced that it will bring an improvement still, I think the best way to demonstrate it is to provide a draft implementation.

ps: the repo maintainers are not that active nowadays. I hope it's temporary.

[markbradley27]

Check out #226. The type info is gathered in the cache, so you can retrieve it before calling Decode if you want (as I do in withDefaults).

[zak905]

Sorry for the delay. I looked at the PR, and I like the fact that the effort to set default is drastically reduced. As a side note, I noticed that there is a slight change of behavior. With the current approach, the default tag is ignored if the type is incompatible ( the behavior is tested by TestInvalidDefaultsValuesHaveNoEffect), with the new approach, the default is set, and the decoding fails as a result. This change in itself is not bad, but the error message does not really help identify that the cause of the error is the default option: schema: error converting value for "n". Maybe a adding a type check can improve things. On the overall, it looks descent.