Skip to content
This repository has been archived by the owner on Jul 22, 2024. It is now read-only.

Encode: DecodeHook not called for struct fields #166

Open
corani opened this issue Jul 2, 2019 · 8 comments
Open

Encode: DecodeHook not called for struct fields #166

corani opened this issue Jul 2, 2019 · 8 comments

Comments

@corani
Copy link

corani commented Jul 2, 2019

I use the DecodeHook to translate the values of certain fields. This works great when "decoding" from map -> struct, but doesn't work the other way around.

When I "encode" from struct -> map, the DecodeHook is called only once, for the whole struct. I would expect it to be called for each field as well.

I've currently hacked it into my decoderHook function, by iterating over the fields and recursively calling itself. But this is rather tedious and has the disadvantage that I can't change the types of fields.

@camdencheek
Copy link
Contributor

Hi @corani -- I suspect the issue you're having is related to #124. The crux of the issue is that when encoding from struct -> map[string]interface{}, after the first round of decoding, the receiver type is just an interface{}, and since the fields of the struct can be directly assigned to an interface{}, the fields of the struct are copied over as-is.

To my knowledge, there is no non-manual way around this right now, but I ran into this same issue which led me to write #183. If this is still relevant to you, I'd be happy to take feedback on that PR on whether that fix works for your case.

@camdencheek
Copy link
Contributor

Interestingly, it looks like this behavior has changed between v1.3.3 and v1.4.0, and this appears to no longer be an issue. I don't immediately see which change did it, but the following code works how I'd expect it to now.

package main

import (
	"fmt"

	"github.com/mitchellh/mapstructure"
)

type A struct {
	Btype *B
}

type B struct {
	Bval string
}

func main() {
	a := A{
		Btype: &B{
			Bval: "test",
		},
	}

	var m map[string]interface{}
	if err := mapstructure.Decode(a, &m); err != nil {
		panic(err)
	}

	fmt.Printf("%#v\n", m)
}

Output for v1.3.3:

map[string]interface {}{"Btype":(*main.B)(0xc000098560)}

Output for v1.4.0:

map[string]interface {}{"Btype":map[string]interface {}{"Bval":"test"}}

@MeteorSis
Copy link

Hi @camdencheek I just tested your code on v1.4.1. It doesn't work recursively :(
Output for v1.4.0:
map[string]interface {}{"Btype":map[string]interface {}{"Bval":"test"}}
Output for v1.4.1:
map[string]interface {}{"Btype":(*main.B)(0xc000012580)}

@voodoo-dn
Copy link

@MeteorSis The same for me after switching to v1.4.1

@jefchien
Copy link

jefchien commented Sep 9, 2022

Did a bit of testing and it turns out that decode is called once for the base struct and once for each nested struct when decoding into a map[string]interface{}

err := d.decode(keyName, x.Interface(), reflect.Indirect(addrVal))

It doesn't however do this for other struct fields. Like if I wanted to have a hook that used encoding.TextMarshaler to convert a struct key in a map to a string. Like so

type TestConfig struct {
	Map map[TestID]string `mapstructure:"map,omitempty"`
}

type TestID struct {
	part1 string
	part2 string
}

func (tID TestID) MarshalText() (text []byte, err error) {
	return []byte(tID.part1 + "_" + tID.part2), nil
}

func textMarshalerHookFunc() mapstructure.DecodeHookFuncValue {
	return func(from reflect.Value, _ reflect.Value) (interface{}, error) {
		marshaler, ok := from.Interface().(encoding.TextMarshaler)
		if !ok {
			return from.Interface(), nil
		}
		out, err := marshaler.MarshalText()
		if err != nil {
			return nil, err
		}
		return string(out), nil
	}
}

This would not work because the struct field kind is not a struct, so will not be decoded again. See https://go.dev/play/p/0YBJAW-iCtP

Another interesting thing is that if there is an embedded struct and decode is called again, if the decode hook func converts the type of the struct into a string like the above TestID, it'll break the decoding.

type TestConfig struct {
	ID TestID `mapstructure:"test_id"`
}

This is because it'll try to decode the newly converted string into a map and fail.

return fmt.Errorf("'%s' expected a map, got '%s'", name, dataVal.Kind())

@daniel-cohen
Copy link

@jefchien Did you ever find a workaround for the second case (encoding an embedded struct to a string) ?
I can see you implemented your own "mapstructure" encoder here: https://github.com/open-telemetry/opentelemetry-collector/blob/e55d22aec61d7b9787b88448ec4a2ae8fb5eefd6/confmap/internal/mapstructure/encoder.go#L109).

@jefchien
Copy link

There isn't currently a workaround using the mapstructure package. Like you said, I just ended up writing a separate encoder that would check the type after each encode/decode.

@dminOf
Copy link

dminOf commented May 30, 2024

ok, I was able to get this to work -- for anyone that is still looking for a solution for this

  1. Upgrade to the currently maintained version of mapstructure: https://github.com/go-viper/mapstructure

  2. Use the encoder implemented by opentlemetry-collector: https://github.com/open-telemetry/opentelemetry-collector/blob/e55d22aec61d7b9787b88448ec4a2ae8fb5eefd6/confmap/internal/mapstructure/encoder.go

	var a any
	var err error
	
        // New is part of the encoder -- you may put it in a different package --   encodeConsts is the mapstructure encoder
	ms := New(&EncoderConfig{		EncodeHook: encodeConsts	})

       // Pass your struct that you want to encode
	a, err = ms.Encode(p)
	if err != nil {
		logger.Error("Error decoding to map[string]interface{}: %v", err)
		return
	}

        // will work recursively as expected:
	m = a.(map[string]interface{})

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

No branches or pull requests

7 participants