diff --git a/base/fill/struct.go b/base/fill/struct.go index 721199e8b..6e79c9f5f 100644 --- a/base/fill/struct.go +++ b/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. diff --git a/base/fill/struct_test.go b/base/fill/struct_test.go index be3c5c705..c902bfb20 100644 --- a/base/fill/struct_test.go +++ b/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"]) + } +} diff --git a/lib/file.go b/lib/file.go index 2583a45e5..db04f3433 100644 --- a/lib/file.go +++ b/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