feat(fill.Struct): Support interface{} fields, require map string keys

base/fill/struct.go

@@ -125,8 +125,9 @@ func putValueToPlace(val interface{}, place reflect.Value, collector *ErrorColle
 			return
 		}
 		// Struct must be assigned from a map.
-		component, err := toStringMap(val)
-		if collector.Add(err) {
+		component := toStringMap(val)
+		if component == nil {
+			collector.Add(fmt.Errorf("could not convert to map[string]"))
 			return
 		}
 		// Recursion to handle sub-component.
@@ -136,21 +137,36 @@ func putValueToPlace(val interface{}, place reflect.Value, collector *ErrorColle
 			// If map is nil, nothing more to do.
 			return
 		}
-		m, ok := val.(map[string]interface{})
-		if !ok {
-			collector.Add(&FieldError{Want: "map", Got: reflect.TypeOf(val).Name(), Val: val})
+		ms, ok := val.(map[string]interface{})
+		if ok {
+			// Special case map[string]string, convert values to strings.
+			if place.Type().Elem() == reflect.TypeOf(strObj) {
+				strmap := make(map[string]string)
+				for k, v := range ms {
+					strmap[k] = fmt.Sprintf("%s", v)
+				}
+				place.Set(reflect.ValueOf(strmap))
+				return
+			}
+			place.Set(reflect.ValueOf(ms))
 			return
 		}
-		// Special case map[string]string, convert values to strings.
-		if place.Type().Elem() == reflect.TypeOf(strObj) {
-			strmap := make(map[string]string)
-			for k, v := range m {
-				strmap[k] = fmt.Sprintf("%s", v)
+		mi, ok := val.(map[interface{}]interface{})
+		if ok {
+			// Special case map[string]string, convert values to strings.
+			if place.Type().Elem() == reflect.TypeOf(strObj) {
+				strmap := make(map[string]string)
+				for k, v := range mi {
+					strmap[fmt.Sprintf("%s", k)] = fmt.Sprintf("%s", v)
+				}
+				place.Set(reflect.ValueOf(strmap))
+				return
 			}
-			place.Set(reflect.ValueOf(strmap))
+			place.Set(reflect.ValueOf(ensureMapsHaveStringKeys(mi)))
 			return
 		}
-		place.Set(reflect.ValueOf(m))
+		// Error due to not being able to convert.
+		collector.Add(&FieldError{Want: "map", Got: reflect.TypeOf(val).Name(), Val: val})
 		return
 	case reflect.Slice:
 		if val == nil {
@@ -298,6 +314,14 @@ func putValueToUnit(val interface{}, place reflect.Value) error {
 			return nil
 		}
 		return &FieldError{Want: "bool", Got: reflect.TypeOf(val).Name(), Val: val}
+	case reflect.Interface:
+		imap, ok := val.(map[interface{}]interface{})
+		if ok {
+			place.Set(reflect.ValueOf(ensureMapsHaveStringKeys(imap)))
+			return nil
+		}
+		place.Set(reflect.ValueOf(val))
+		return nil
 	default:
 		return fmt.Errorf("unknown kind %s", place.Kind())
 	}
@@ -306,20 +330,36 @@ func putValueToUnit(val interface{}, place reflect.Value) error {
 // toStringMap converts the input to a map[string] if able. This is needed because, while JSON
 // correctly deserializes sub structures to map[string], YAML instead deserializes to
 // map[interface{}]interface{}, so we need to manually convert this case to map[string].
-func toStringMap(val interface{}) (map[string]interface{}, error) {
-	m, ok := val.(map[string]interface{})
+func toStringMap(obj interface{}) map[string]interface{} {
+	m, ok := obj.(map[string]interface{})
 	if ok {
-		return m, nil
+		return m
 	}
-	imap, ok := val.(map[interface{}]interface{})
+	imap, ok := obj.(map[interface{}]interface{})
 	if ok {
-		convert := make(map[string]interface{})
-		for k, v := range imap {
-			convert[fmt.Sprintf("%v", k)] = v
+		return ensureMapsHaveStringKeys(imap)
+	}
+	return nil
+}
+
+// ensureMapsHaveStringKeys will recursively convert map's key to be strings. This will allow us
+// to serialize back into JSON.
+func ensureMapsHaveStringKeys(imap map[interface{}]interface{}) map[string]interface{} {
+	build := make(map[string]interface{})
+	for k, v := range imap {
+		switch x := v.(type) {
+		case map[interface{}]interface{}:
+			v = ensureMapsHaveStringKeys(x)
+		case []interface{}:
+			for i, elem := range x {
+				if inner, ok := elem.(map[interface{}]interface{}); ok {
+					x[i] = ensureMapsHaveStringKeys(inner)
+				}
+			}
 		}
-		return convert, nil
+		build[fmt.Sprintf("%s", k)] = v
 	}
-	return nil, fmt.Errorf("could not convert to map[string]")
+	return build
 }
 
 // getArbitrarySetter returns a ArbitrarySetter if the target implements it.

base/fill/struct_test.go

@@ -253,6 +253,7 @@ type Collection struct {
 type SubElement struct {
 	Num    int
 	Things *map[string]string
+	Any    interface{}
 }
 
 func (c *Collection) SetArbitrary(key string, val interface{}) error {
@@ -727,3 +728,178 @@ func TestFillPointerToMap(t *testing.T) {
 		t.Errorf("expected: s.Things[\"b\"] should be \"banana\"")
 	}
 }
+
+func TestFillInterface(t *testing.T) {
+	// Any can be a float
+	jsonData := `{
+  "Any": 123.4
+}`
+	data := make(map[string]interface{})
+	err := json.Unmarshal([]byte(jsonData), &data)
+	if err != nil {
+		panic(err)
+	}
+
+	var s SubElement
+	err = Struct(data, &s)
+	if err != nil {
+		panic(err)
+	}
+
+	if s.Any != 123.4 {
+		t.Errorf("expected: s.Any should be 123, got %v of %s", s.Any, reflect.TypeOf(s.Any))
+	}
+
+	// Any can be a string
+	jsonData = `{
+  "Any": "abc"
+}`
+	data = make(map[string]interface{})
+	err = json.Unmarshal([]byte(jsonData), &data)
+	if err != nil {
+		panic(err)
+	}
+
+	s = SubElement{}
+	err = Struct(data, &s)
+	if err != nil {
+		panic(err)
+	}
+
+	if s.Any != "abc" {
+		t.Errorf("expected: s.Any should be \"abc\", got %v of %s", s.Any, reflect.TypeOf(s.Any))
+	}
+}
+
+func TestFillYamlFields(t *testing.T) {
+	yamlData := `
+name: abc
+age: 42
+xpos: 3.51
+big: 1234567890123456
+`
+	data := make(map[string]interface{})
+	err := yaml.Unmarshal([]byte(yamlData), &data)
+	if err != nil {
+		panic(err)
+	}
+
+	var c Collection
+	err = Struct(data, &c)
+	if err != nil {
+		panic(err)
+	}
+
+	if c.Name != "abc" {
+		t.Errorf("expected: c.Name should be \"abc\", got: %s", c.Name)
+	}
+	if c.Age != 42 {
+		t.Errorf("expected: c.Name should be 42, got: %d", c.Age)
+	}
+	if c.Xpos != 3.51 {
+		t.Errorf("expected: c.Xpos should be 3.51, got: %f", c.Xpos)
+	}
+	if c.Big != 1234567890123456 {
+		t.Errorf("expected: c.Big should be 1234567890123456, got: %d", c.Big)
+	}
+}
+
+func TestFillYamlMap(t *testing.T) {
+	yamlData := `
+name: more
+dict:
+  a: apple
+  b: banana
+sub:
+  num: 7
+  things:
+    c: cat
+    d: dog
+`
+	data := make(map[string]interface{})
+	err := yaml.Unmarshal([]byte(yamlData), &data)
+	if err != nil {
+		panic(err)
+	}
+
+	var c Collection
+	err = Struct(data, &c)
+	if err != nil {
+		panic(err)
+	}
+
+	if c.Name != "more" {
+		t.Errorf("expected: c.Name should be \"abc\", got: %s", c.Name)
+	}
+	if len(c.Dict) != 2 {
+		t.Errorf("expected: len(c.Dict) should be 2, got: %d", len(c.Dict))
+	}
+	if c.Dict["a"] != "apple" {
+		t.Errorf("expected: c.Dict[\"a\"] should be \"apple\", got: %s", c.Dict["a"])
+	}
+	if c.Dict["b"] != "banana" {
+		t.Errorf("expected: c.Dict[\"b\"] should be \"banana\", got: %s", c.Dict["b"])
+	}
+
+	if c.Sub.Num != 7 {
+		t.Errorf("expected: c.Sub.Num should be 7, got: %d", c.Sub.Num)
+	}
+	if len(*c.Sub.Things) != 2 {
+		t.Errorf("expected: len(c.Sub.Things) should be 2, got: %d", len(*c.Sub.Things))
+	}
+	if (*c.Sub.Things)["c"] != "cat" {
+		t.Errorf("expected: c.Sub.Things[\"c\"] should be \"cat\", got: %s", (*c.Sub.Things)["c"])
+	}
+	if (*c.Sub.Things)["d"] != "dog" {
+		t.Errorf("expected: c.Sub.Things[\"d\"] should be \"dog\", got: %s", (*c.Sub.Things)["d"])
+	}
+}
+
+func TestFillYamlMapsHaveStringKeys(t *testing.T) {
+	yamlData := `
+name: schema
+sub:
+  any:
+    type: object
+    inner:
+      e: eel
+      f: frog
+`
+	data := make(map[string]interface{})
+	err := yaml.Unmarshal([]byte(yamlData), &data)
+	if err != nil {
+		panic(err)
+	}
+
+	var c Collection
+	err = Struct(data, &c)
+	if err != nil {
+		panic(err)
+	}
+
+	if c.Name != "schema" {
+		t.Errorf("expected: c.Name should be \"schema\", got: %s", c.Name)
+	}
+	m, ok := c.Sub.Any.(map[string]interface{})
+	if !ok {
+		t.Fatalf("expected: c.Sub.Any should be a map[string]interface{}, got: %s",
+			reflect.TypeOf(c.Sub.Any))
+	}
+	if m["type"] != "object" {
+		t.Errorf("expected: c.Sub.Any[\"type\"] should be \"object\", got: %d", m["type"])
+	}
+
+	// This is asserting that maps within data structures also have string keys, despite
+	// the fact that YAML deserialized this, and YAML always uses interface{} keys.
+	m, ok = m["inner"].(map[string]interface{})
+	if !ok {
+		t.Fatalf("expected: Inner should be a map[string]interface{}, got: %s",
+			reflect.TypeOf(m["inner"]))
+	}
+	if m["e"] != "eel" {
+		t.Errorf("expected: c.Sub.Any[\"e\"] should be \"eel\", got: %d", m["e"])
+	}
+	if m["f"] != "frog" {
+		t.Errorf("expected: c.Sub.Any[\"f\"] should be \"frog\", got: %d", m["f"])
+	}
+}

lib/file.go

@@ -86,18 +86,11 @@ func readSingleFile(path string) (*dataset.Dataset, string, error) {
 				return nil, "", err
 			}
 
-			fieldsMapIface := make(map[interface{}]interface{})
-			if err = yaml.Unmarshal(data, fieldsMapIface); err != nil {
+			fields := make(map[string]interface{})
+			if err = yaml.Unmarshal(data, fields); err != nil {
 				return nil, "", err
 			}
 
-			// TODO (b5): slow. the yaml package we use unmarshals to map[interface{}]interface{}
-			// (because that's what the yaml spec says you should do), so we have to do this extra
-			// recurse/convert step. It shouldn't be *too* big a deal since most use cases don't inline
-			// full dataset bodies into yaml files, but that's not an assumption we should rely on
-			// long term
-			fields := toMapIface(fieldsMapIface)
-
 			kind, err := fillDatasetOrComponent(fields, path, &ds)
 			return &ds, kind, err
 
@@ -143,27 +136,6 @@ func readSingleFile(path string) (*dataset.Dataset, string, error) {
 	}
 }
 
-func toMapIface(i map[interface{}]interface{}) map[string]interface{} {
-	mapi := map[string]interface{}{}
-	for ikey, val := range i {
-		switch x := val.(type) {
-		case map[interface{}]interface{}:
-			val = toMapIface(x)
-		case []interface{}:
-			for i, v := range x {
-				if mapi, ok := v.(map[interface{}]interface{}); ok {
-					x[i] = toMapIface(mapi)
-				}
-			}
-		}
-
-		if key, ok := ikey.(string); ok {
-			mapi[key] = val
-		}
-	}
-	return mapi
-}
-
 func fillDatasetOrComponent(fields map[string]interface{}, path string, ds *dataset.Dataset) (string, error) {
 	var target interface{}
 	target = ds