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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FEATURE] default values for custom types #225

Open
1 task done
markbradley27 opened this issue Nov 8, 2024 · 2 comments · May be fixed by #226
Open
1 task done

[FEATURE] default values for custom types #225

markbradley27 opened this issue Nov 8, 2024 · 2 comments · May be fixed by #226

Comments

@markbradley27
Copy link

markbradley27 commented Nov 8, 2024

Is there an existing feature request for this?

  • 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 #222 (comment), 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
Copy link
Contributor

zak905 commented Nov 10, 2024

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
Copy link
Author

markbradley27 commented Nov 12, 2024

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).

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