mapstructure icon indicating copy to clipboard operation
mapstructure copied to clipboard

Nested map[string]interface{} not decoded

Open bep opened this issue 6 years ago • 7 comments

The example below prints:

map[V1:v1 M2:map[V2:v2]]
V1
v1
M2
map[V2:v2]
[VS1_1 VS1_2]
VS1_1
VS1_2
[map[VMS:vms]]
map[VMS:vms]

The first map[string]interface{} is traversed, but the last (map[V2:v2]) seems to be just set as-is without any decoding of its elements. The same is the case for maps inside slices.

package main

import (
	"fmt"
	"log"

	"reflect"

	"github.com/mitchellh/mapstructure"
)

type T struct {
	M1 map[string]interface{}
	S1 []interface{}
	S2 []interface{}
}

func main() {
	dest := &T{}

	hook := func(t1 reflect.Type, t2 reflect.Type, v interface{}) (interface{}, error) {
		fmt.Printf("%v\n", v)
		return v, nil
	}

	input := map[string]interface{}{
		"M1": map[string]interface{}{
			"V1": "v1",
			"M2": map[string]interface{}{
				"V2": "v2",
			},
		},
		"S1": []interface{}{"VS1_1", "VS1_2"},
		"S2": []interface{}{map[string]interface{}{
			"VMS": "vms",
		}},
	}

	conf := &mapstructure.DecoderConfig{
		DecodeHook:       hook,
		ErrorUnused:      false,
		WeaklyTypedInput: false,
		Metadata:         nil,
		Result:           dest,
		ZeroFields:       true,
	}

	dec, err := mapstructure.NewDecoder(conf)
	if err != nil {
		log.Fatal(err)
	}
	err = dec.Decode(input)
	if err != nil {
		log.Fatal(err)
	}

}

bep avatar May 20 '18 09:05 bep

@bep Thank you for raising this issue. I am also facing this issue.

opan avatar May 21 '18 07:05 opan

Internally when M1 gets passed to decodeMapFromMap, the input is map[V1:v1 M2:map[V2:v2]] but the valElemType is set to type interface{}.

// valElemType is of Type interface{}
currentVal := reflect.Indirect(reflect.New(valElemType))` 
if err := d.decode(fieldName, v, currentVal); err != nil {
	errors = appendErrors(errors, err)
	continue
}

As we range over the map the decode method (above) receives interface{} as outval; subsequently stuffing the data into an interface{} (within the decode method):

inputKind := getKind(outVal)
switch inputKind {
// ...
case reflect.Interface:
	err = d.decodeBasic(name, input, outVal)

Similar story for decodeSlice; currentField is of Type interface{}.

Given the type you supplied, this appears to be the intended functionality of this package.

For decodeMapFromMap one could set valElemType inside the range loop to the type of the inner v: valElemType = reflect.ValueOf(v).Type()

Which would return:

map[M2:map[V2:v2] V1:v1]
V1
v1
M2
map[V2:v2]
V2 <---------
v2 <---------
[VS1_1 VS1_2]
VS1_1
VS1_2
[map[VMS:vms]]
map[VMS:vms]

But, this would break existing functionality (and tests).

mfridman avatar May 29 '18 16:05 mfridman

Given the type you supplied, this appears to be the intended functionality of this package.

As there are no tests for my use case, I think it is safe to assume that. So I'm no calling this a bug, more of a feature request. I would call this surprising behaviour, and it behaves differently than other relevant reflection libraries, e.g. the json package in the standard library. I try to avoid the interface{} type whenever possible, but it has its uses.

bep avatar May 29 '18 16:05 bep

Thanks, I'm trying to understand the use case for this one. If the data being received contains a mixture of types, e.g., string and map[string]interface{} then based on the current implementation we can only store that in interface{}.

Is the issue you're facing the result of DecodeHookFunc not acting on the nested items?

Also, is this related to #123 (I should probably go read that more closely).

mfridman avatar May 29 '18 16:05 mfridman

Is the issue you're facing the result of DecodeHookFunc not acting on the nested items?

DecodeHookFunc never sees these nested items.

bep avatar May 29 '18 21:05 bep

I'm wondering if anyone has found any possible workarounds for this.

geoah avatar Sep 05 '18 20:09 geoah

Looking back at this, I think we could consider adding an option to basically always deeply traverse interface{}. Mapstructure has always taken the approach where if direct assignment just works, then we use it. Since interface{} can be assigned to interface{} we don't need to decode deeper.

I'd be open to an option on the DecoderConfig to continue traversing into interfaces if someone wants to take that on!

mitchellh avatar Nov 26 '20 23:11 mitchellh