diff --git a/base/fill/path_value.go b/base/fill/path_value.go new file mode 100644 index 000000000..0087088c4 --- /dev/null +++ b/base/fill/path_value.go @@ -0,0 +1,183 @@ +package fill + +import ( + "fmt" + "reflect" + "strconv" + "strings" +) + +var ( + // ErrNotFound is returned when a field isn't found + ErrNotFound = fmt.Errorf("not found") +) + +// CoercionError represents an error when a value cannot be converted from one type to another +type CoercionError struct { + From string + To string +} + +// Error displays the text version of the CoercionError +func (c *CoercionError) Error() string { + return fmt.Sprintf("could not coerce from %s to %s", c.From, c.To) +} + +// SetPathValue sets a value on a the output struct, accessed using the path of dot-separated fields +func SetPathValue(path string, val interface{}, output interface{}) error { + target := reflect.ValueOf(output) + steps := strings.Split(path, ".") + // Find target place to assign to, field is a non-empty string only if target is a map. + target, field, err := findTargetAtPath(steps, target) + if err != nil { + if err == ErrNotFound { + return fmt.Errorf("path: \"%s\" not found", path) + } + return err + } + if field == "" { + // Convert val into the correct type, parsing ints and bools and so forth. + val, err = coerceToTargetType(val, target) + if err != nil { + if cerr, ok := err.(*CoercionError); ok { + return fmt.Errorf("invalid type for path \"%s\": expected %s, got %s", + path, cerr.To, cerr.From) + } + return err + } + return putValueToPlace(val, target) + } + // A map with a field name to assign to. + // TODO: Only works for map[string]string, not map's with a struct for a value. + target.SetMapIndex(reflect.ValueOf(field), reflect.ValueOf(val)) + return nil +} + +// GetPathValue gets a value from the input struct, accessed using the path of dot-separated fields +func GetPathValue(path string, input interface{}) (interface{}, error) { + target := reflect.ValueOf(input) + steps := strings.Split(path, ".") + // Find target place to assign to, field is a non-empty string only if target is a map. + target, field, err := findTargetAtPath(steps, target) + if err != nil { + if err == ErrNotFound { + return nil, fmt.Errorf("path: \"%s\" not found", path) + } + return nil, err + } + if field == "" { + return target.Interface(), nil + } + lookup := target.MapIndex(reflect.ValueOf(field)) + if lookup.IsValid() { + return lookup.Interface(), nil + } + return nil, fmt.Errorf("invalid path: \"%s\"", path) +} + +func findTargetAtPath(steps []string, place reflect.Value) (reflect.Value, string, error) { + if len(steps) == 0 { + return place, "", nil + } + // Get current step of the path, dispatch on its type. + s := steps[0] + rest := steps[1:] + if place.Kind() == reflect.Struct { + field := getFieldCaseInsensitive(place, s) + if !field.IsValid() { + return place, "", ErrNotFound + } + return findTargetAtPath(rest, field) + } else if place.Kind() == reflect.Ptr { + var inner reflect.Value + if place.IsNil() { + alloc := reflect.New(place.Type().Elem()) + place.Set(alloc) + inner = alloc.Elem() + } else { + inner = place.Elem() + } + return findTargetAtPath(steps, inner) + } else if place.Kind() == reflect.Map { + if place.IsNil() { + place.Set(reflect.MakeMap(place.Type())) + } + // TODO: Handle case where `rest` has more steps and `val` is a struct: more + // recursive is needed. + return place, s, nil + } else if place.Kind() == reflect.Slice { + num, err := coerceToInt(s) + if err != nil { + return place, "", err + } + if num >= place.Len() { + return place, "", fmt.Errorf("index outside of range: %d, len is %d", num, place.Len()) + } + elem := place.Index(num) + return findTargetAtPath(rest, elem) + } else { + return place, "", fmt.Errorf("cannot set field of type %s", place.Kind()) + } +} + +func coerceToTargetType(val interface{}, place reflect.Value) (interface{}, error) { + switch place.Kind() { + case reflect.Bool: + str, ok := val.(string) + if ok { + str = strings.ToLower(str) + if str == "true" { + return true, nil + } else if str == "false" { + return false, nil + } else { + return nil, fmt.Errorf("could not parse value to bool") + } + } + b, ok := val.(bool) + if ok { + return b, nil + } + return nil, &CoercionError{To: "bool", From: reflect.TypeOf(val).Name()} + case reflect.Int: + num, ok := val.(int) + if ok { + return num, nil + } + str, ok := val.(string) + if ok { + parsed, err := strconv.ParseInt(str, 10, 32) + return int(parsed), err + } + return nil, &CoercionError{To: "int", From: reflect.TypeOf(val).Name()} + case reflect.Ptr: + alloc := reflect.New(place.Type().Elem()) + return coerceToTargetType(val, alloc.Elem()) + case reflect.String: + str, ok := val.(string) + if ok { + return str, nil + } + return nil, &CoercionError{To: "string", From: reflect.TypeOf(val).Name()} + default: + return nil, fmt.Errorf("unknown kind: %s", place.Kind()) + } +} + +func coerceToInt(val interface{}) (int, error) { + num, ok := val.(int) + if ok { + return num, nil + } + str, ok := val.(string) + if ok { + parsed, err := strconv.ParseInt(str, 10, 32) + return int(parsed), err + } + return -1, &CoercionError{To: "int", From: reflect.TypeOf(val).Name()} +} + +func getFieldCaseInsensitive(place reflect.Value, name string) reflect.Value { + name = strings.ToLower(name) + return place.FieldByNameFunc(func(s string) bool { return strings.ToLower(s) == name }) +} diff --git a/base/fill/path_value_test.go b/base/fill/path_value_test.go new file mode 100644 index 000000000..de3fd06e2 --- /dev/null +++ b/base/fill/path_value_test.go @@ -0,0 +1,122 @@ +package fill + +import ( + "testing" +) + +func TestFillPathValue(t *testing.T) { + c := Collection{} + err := SetPathValue("name", "Alice", &c) + if err != nil { + panic(err) + } + if c.Name != "Alice" { + t.Errorf("expected: s.Name should be \"Alice\"") + } + + c = Collection{} + err = SetPathValue("age", 42, &c) + if err != nil { + panic(err) + } + if c.Age != 42 { + t.Errorf("expected: s.Age should be 42") + } + + c = Collection{} + err = SetPathValue("age", "56", &c) + if err != nil { + panic(err) + } + if c.Age != 56 { + t.Errorf("expected: s.Age should be 56") + } + + c = Collection{} + err = SetPathValue("ison", "true", &c) + if err != nil { + panic(err) + } + if !c.IsOn { + t.Errorf("expected: s.IsOn should be true") + } + + c = Collection{} + err = SetPathValue("ison", true, &c) + if err != nil { + panic(err) + } + if !c.IsOn { + t.Errorf("expected: s.IsOn should be true") + } + + c = Collection{} + err = SetPathValue("ptr", 123, &c) + if err != nil { + panic(err) + } + if *(c.Ptr) != 123 { + t.Errorf("expected: s.Ptr should be 123") + } + + c = Collection{} + err = SetPathValue("not_found", "missing", &c) + expect := "path: \"not_found\" not found" + if err == nil { + t.Fatalf("expected: error \"%s\", got no error", expect) + } + if err.Error() != expect { + t.Errorf("expected: error should be \"%s\", got \"%s\"", expect, err.Error()) + } + + c = Collection{} + err = SetPathValue("sub.num", 7, &c) + if err != nil { + panic(err) + } + if c.Sub.Num != 7 { + t.Errorf("expected: s.Sub.Num should be 7") + } + + c = Collection{} + err = SetPathValue("dict.cat", "meow", &c) + if err != nil { + panic(err) + } + if c.Dict["cat"] != "meow" { + t.Errorf("expected: s.Dict[\"cat\"] should be \"meow\"") + } + + // Don't allocate a new map. + err = SetPathValue("dict.dog", "bark", &c) + if err != nil { + panic(err) + } + if c.Dict["cat"] != "meow" { + t.Errorf("expected: s.Dict[\"cat\"] should be \"meow\"") + } + if c.Dict["dog"] != "bark" { + t.Errorf("expected: s.Dict[\"dog\"] should be \"bark\"") + } + + s := &SubElement{} + err = SetPathValue("things.eel", "zap", &s) + if err != nil { + panic(err) + } + if (*s.Things)["eel"] != "zap" { + t.Errorf("expected: c.Things[\"eel\"] should be \"zap\"") + } + + // Don't allocate a new map. + err = SetPathValue("things.frog", "ribbit", &s) + if err != nil { + panic(err) + } + if (*s.Things)["eel"] != "zap" { + t.Errorf("expected: c.Things[\"eel\"] should be \"zap\"") + } + if (*s.Things)["frog"] != "ribbit" { + t.Errorf("expected: c.Things[\"frog\"] should be \"ribbit\"") + } +} diff --git a/cmd/config.go b/cmd/config.go index 5373f7715..8be6d37b5 100644 --- a/cmd/config.go +++ b/cmd/config.go @@ -6,7 +6,6 @@ import ( "os" "strings" - "github.com/ghodss/yaml" "github.com/qri-io/ioes" "github.com/qri-io/qri/config" "github.com/qri-io/qri/lib" @@ -179,8 +178,9 @@ func (o *ConfigOptions) Set(args []string) (err error) { profileChanged := false for i := 0; i < len(args)-1; i = i + 2 { - var value interface{} path := strings.ToLower(args[i]) + value := args[i+1] + if ip[path] { ErrExit(o.ErrOut, fmt.Errorf("cannot set path %s", path)) } @@ -196,10 +196,6 @@ func (o *ConfigOptions) Set(args []string) (err error) { } profileChanged = true } else { - if err = yaml.Unmarshal([]byte(args[i+1]), &value); err != nil { - return err - } - if err = lib.Config.Set(path, value); err != nil { return err } diff --git a/config/config.go b/config/config.go index de0602375..4322177f4 100644 --- a/config/config.go +++ b/config/config.go @@ -6,10 +6,6 @@ import ( "encoding/json" "fmt" "io/ioutil" - "math" - "reflect" - "strconv" - "strings" "github.com/ghodss/yaml" "github.com/qri-io/jsonschema" @@ -129,102 +125,12 @@ func (cfg Config) WriteToFile(path string) error { // Get a config value with case.insensitive.dot.separated.paths func (cfg Config) Get(path string) (interface{}, error) { - v, err := cfg.path(path) - if err != nil { - return nil, err - } - return v.Interface(), nil + return fill.GetPathValue(path, cfg) } // Set a config value with case.insensitive.dot.separated.paths func (cfg *Config) Set(path string, value interface{}) error { - v, err := cfg.path(path) - if err != nil { - return err - } - - rv := reflect.ValueOf(value) - if rv.Kind() != v.Kind() { - // we have one caveat: since json automatically casts all numbers to float64 - // we need to check if we have a float value trying to be cast to an integer value - // if it can be cast as an integer without loss of value, we should allow it - if rv.Kind() == reflect.Float64 && v.Kind() == reflect.Int { - _, fraction := math.Modf(rv.Float()) - if fraction == 0 { - v.SetInt(int64(rv.Float())) - return nil - } - } - return fmt.Errorf("invalid type for config path %s, expected: %s, got: %s", path, v.Kind().String(), rv.Kind().String()) - } - - // if we can't address this value check to see if it's a map - if !v.CanAddr() { - parent, base := splitBase(path) - if v, _ := cfg.path(parent); v.Kind() == reflect.Map { - v.SetMapIndex(reflect.ValueOf(base), rv) - return nil - } - } - - // how to make sure a float doesn't have a decimal, can it be cast to an int - switch v.Kind() { - case reflect.Int: - v.SetInt(rv.Int()) - case reflect.String: - v.SetString(rv.String()) - case reflect.Bool: - v.SetBool(rv.Bool()) - } - - return nil -} - -func splitBase(path string) (string, string) { - components := strings.Split(path, ".") - if len(components) > 0 { - return strings.Join(components[0:len(components)-1], "."), components[len(components)-1] - } - return "", "" -} - -func (cfg Config) path(path string) (elem reflect.Value, err error) { - elem = reflect.ValueOf(cfg) - - for _, sel := range strings.Split(path, ".") { - sel = strings.ToLower(sel) - - if elem.Kind() == reflect.Ptr { - elem = elem.Elem() - } - - switch elem.Kind() { - case reflect.Struct: - elem = elem.FieldByNameFunc(func(str string) bool { - return strings.ToLower(str) == sel - }) - case reflect.Slice: - index, err := strconv.Atoi(sel) - if err != nil { - return elem, fmt.Errorf("invalid index value: %s", sel) - } - elem = elem.Index(index) - case reflect.Map: - for _, key := range elem.MapKeys() { - // we only support strings as keys - if strings.ToLower(key.String()) == sel { - return elem.MapIndex(key), nil - } - } - return elem, fmt.Errorf("invalid config path: %s", path) - } - - if elem.Kind() == reflect.Invalid { - return elem, fmt.Errorf("invalid config path: %s", path) - } - } - - return elem, nil + return fill.SetPathValue(path, value, cfg) } // ImmutablePaths returns a map of paths that should never be modified @@ -340,6 +246,9 @@ func (cfg *Config) Copy() *Config { if cfg.RPC != nil { res.RPC = cfg.RPC.Copy() } + if cfg.Remotes != nil { + res.Remotes = cfg.Remotes.Copy() + } if cfg.Logging != nil { res.Logging = cfg.Logging.Copy() } diff --git a/config/config_test.go b/config/config_test.go index 18d81783f..695f8936c 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -95,12 +95,12 @@ func TestConfigGet(t *testing.T) { expect interface{} err string }{ - {"foo", nil, "invalid config path: foo"}, + {"foo", nil, "path: \"foo\" not found"}, {"p2p.enabled", true, ""}, - {"p2p.QriBootstrapAddrs.foo", nil, "invalid index value: foo"}, + {"p2p.QriBootstrapAddrs.foo", nil, "strconv.ParseInt: parsing \"foo\": invalid syntax"}, {"p2p.QriBootstrapAddrs.0", cfg.P2P.QriBootstrapAddrs[0], ""}, {"logging.levels.qriapi", cfg.Logging.Levels["qriapi"], ""}, - {"logging.levels.foo", nil, "invalid config path: logging.levels.foo"}, + {"logging.levels.foo", nil, "invalid path: \"logging.levels.foo\""}, } for i, c := range cases { @@ -123,10 +123,10 @@ func TestConfigSet(t *testing.T) { value interface{} err string }{ - {"foo", nil, "invalid config path: foo"}, + {"foo", nil, "path: \"foo\" not found"}, {"p2p.enabled", false, ""}, {"p2p.qribootstrapaddrs.0", "wahoo", ""}, - {"p2p.qribootstrapaddrs.0", false, "invalid type for config path p2p.qribootstrapaddrs.0, expected: string, got: bool"}, + {"p2p.qribootstrapaddrs.0", false, "invalid type for path \"p2p.qribootstrapaddrs.0\": expected string, got bool"}, {"logging.levels.qriapi", "debug", ""}, } @@ -134,13 +134,13 @@ func TestConfigSet(t *testing.T) { cfg := DefaultConfigForTesting() err := cfg.Set(c.path, c.value) - if !(err == nil && c.err == "" || err != nil && err.Error() == c.err) { - t.Errorf("case %d error mismatch. expected: %s, got: %s", i, c.err, err) - continue - } - - if c.err != "" { + if err != nil { + if err.Error() != c.err { + t.Errorf("case %d error mismatch. expected: %s, got: %s", i, c.err, err) + } continue + } else if c.err != "" { + t.Errorf("case %d error mismatch. expected: %s, got: %s", i, c.err, err) } got, err := cfg.Get(c.path) diff --git a/config/remotes.go b/config/remotes.go index a7e04724c..5d7983ce6 100644 --- a/config/remotes.go +++ b/config/remotes.go @@ -22,3 +22,12 @@ func (r *Remotes) Get(name string) (string, bool) { addr, ok := (*r)[name] return addr, ok } + +// Copy creates a copy of a Remotes struct +func (r *Remotes) Copy() *Remotes { + c := make(map[string]string) + for k, v := range *r { + c[k] = v + } + return (*Remotes)(&c) +} diff --git a/lib/datasets_test.go b/lib/datasets_test.go index 99440bd4f..d1498bda7 100644 --- a/lib/datasets_test.go +++ b/lib/datasets_test.go @@ -457,23 +457,23 @@ func TestDatasetRequestsGet(t *testing.T) { {"body with limit and offfset", &GetParams{Path: "peer/movies", Selector: "body", Format: "json", - Limit: 5, Offset: 0, All: false}, bodyToString(moviesBody[:5])}, + Limit: 5, Offset: 0, All: false}, bodyToString(moviesBody[:5])}, {"body with invalid limit and offset", &GetParams{Path: "peer/movies", Selector: "body", Format: "json", - Limit: -5, Offset: -100, All: false}, "invalid limit / offset settings"}, + Limit: -5, Offset: -100, All: false}, "invalid limit / offset settings"}, {"body with all flag ignores invalid limit and offset", &GetParams{Path: "peer/movies", Selector: "body", Format: "json", - Limit: -5, Offset: -100, All: true}, bodyToString(moviesBody)}, + Limit: -5, Offset: -100, All: true}, bodyToString(moviesBody)}, {"body with all flag", &GetParams{Path: "peer/movies", Selector: "body", Format: "json", - Limit: 0, Offset: 0, All: true}, bodyToString(moviesBody)}, + Limit: 0, Offset: 0, All: true}, bodyToString(moviesBody)}, {"body with limit and non-zero offset", &GetParams{Path: "peer/movies", Selector: "body", Format: "json", - Limit: 2, Offset: 10, All: false}, bodyToString(moviesBody[10:12])}, + Limit: 2, Offset: 10, All: false}, bodyToString(moviesBody[10:12])}, } req := NewDatasetRequests(node, nil) diff --git a/lib/remote.go b/lib/remote.go index ca5ec1460..0e771d98f 100644 --- a/lib/remote.go +++ b/lib/remote.go @@ -54,8 +54,10 @@ func (r *RemoteRequests) PushToRemote(p *PushParams, out *bool) error { return err } - // TODO(dlong): Resolve remote name from p.RemoteName instead of using registry's location. - location := Config.Registry.Location + location, found := Config.Remotes.Get(p.RemoteName) + if !found { + return fmt.Errorf("remote name \"%s\" not found", p.RemoteName) + } data, err := json.Marshal(dinfo) if err != nil {