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

Allow override decode semantics through decode middleware #192

Closed
smyrman opened this issue May 15, 2020 · 3 comments
Closed

Allow override decode semantics through decode middleware #192

smyrman opened this issue May 15, 2020 · 3 comments

Comments

@smyrman
Copy link

smyrman commented May 15, 2020

As shown by #191 and #115, it is not possible through the current hook system to specify override semantics because hooks only does pre-processing.

To my experience, middleware is always more powerful than hooks, because you can control the flow a lot better.

I think it would make sense to introduce a middleware for:

func (d *Decoder) decode(name string, input interface{}, outVal reflect.Value) error

Example implementation stub:

type DecodeFunc func(name string, input interface{}, outVal reflect.Value) error

type DecoderConfig struct{
   ...
   // if defined, pass in Decoder.decode to get a new DecodeFunc.
   // Fallback to call Decoder.decode directly.
   DecodeMiddleware func(DecodeFunc) DecodeFunc
}

With an example implementation for preserving time.Time as described in #191 (although this should probably be handled by default as well):

func PreseveTimeMiddlware(next DecodeFunc) DecodeFunc {
    return func(name string, input interface{}, outVal reflect.Value) error {
        inVal := reflect.ValueOf(input)
        if _, ok := input.(time.Time); ok && inVal.Type(input).AssignableTo(outVal.Type()) {
            outVal.Set(inVal)
            return nil
        }
        return next(name, input, outVal)
    }
}
@smyrman
Copy link
Author

smyrman commented May 15, 2020

PS! As for what to do with the existing hook types, and functions, they would still work, but it would make sense (to me) to document them as deprecated; they really just do half the job, and they add complexity...

@smyrman
Copy link
Author

smyrman commented May 15, 2020

Implementation wise it's probably not exactly as straight-farward as to wrap the entire Decoder.decode, but rather, a sub-set of it. The middleware must of-course run recrusivly from the inner funciton, so the bit to replace, is probably the code here:

switch outputKind {

Put the switch-case inside a new function, and pass it to the middlware func before calling.
The lower bit with the meta-data update must still happen after. Figure out how to handle the addMetaKey Boolean.

@mitchellh
Copy link
Owner

Thanks for the suggestion. I believe everything you want to do would be possible by composing decode hooks using the new #183 PR (I saw you responded there as well). In general, I'm a fan of middleware, but the aforementioned PR fits the existing "shape" of this project so I wanted to give that a shot. Let me know if that doesn't solve things for you!

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

No branches or pull requests

2 participants