diff --git a/docs/content/rc.md b/docs/content/rc.md index 85b4f9a04..49cb5e58f 100644 --- a/docs/content/rc.md +++ b/docs/content/rc.md @@ -404,6 +404,8 @@ This shows: If you wish to set config (the equivalent of the global flags) for the duration of an rc call only then pass in the `_config` parameter. +Alternatively, you can pass config options flat at the top level of the parameter map. The option names are the same as their CLI flags without `--` and with `-` replaced by `_` (e.g. `transfers` instead of `Transfers` inside `_config`). + This should be in the same format as the `main` key returned by [options/get](#options-get). @@ -425,12 +427,26 @@ parameter, you would pass this parameter in your JSON blob. "_config":{"CheckSum": true} ``` +Or pass it flat at the top level: + +```json +"checksum": true +``` + If using `rclone rc` this could be passed as ```console rclone rc sync/sync ... _config='{"CheckSum": true}' ``` +Or simply flat: + +```console +rclone rc sync/sync ... checksum=true +``` + +If both flat parameters and `_config` are supplied, the parameters in the legacy `_config` block will take precedence. + Any config parameters you don't set will inherit the global defaults which were set with command line flags or environment variables. @@ -443,6 +459,13 @@ setting the equivalent of `--buffer-size` in string or integer format. "_config":{"BufferSize": 44040192} ``` +Or flat: + +```json +"buffer_size": "42M" +"buffer_size": 44040192 +``` + If you wish to check the `_config` assignment has worked properly then calling `options/local` will show what the value got set to. @@ -451,6 +474,8 @@ calling `options/local` will show what the value got set to. If you wish to set filters for the duration of an rc call only then pass in the `_filter` parameter. +Alternatively, you can pass filter options flat at the top level of the parameter map. The option names are the same as their CLI flags without `--` and with `-` replaced by `_` (e.g. `exclude` instead of `Exclude` inside `_filter`). + This should be in the same format as the `filter` key returned by [options/get](#options-get). @@ -477,12 +502,26 @@ you would pass this parameter in your JSON blob. "_filter":{"MaxSize":"1M", "IncludeRule":["a","b"], "MaxAge":"42s"} ``` +Or pass them flat at the top level: + +```json +"max_size":"1M", "include":["a","b"], "max_age":"42s" +``` + If using `rclone rc` this could be passed as ```console rclone rc ... _filter='{"MaxSize":"1M", "IncludeRule":["a","b"], "MaxAge":"42s"}' ``` +Or simply flat: + +```console +rclone rc ... max_size=1M include="a,b" max_age=42s +``` + +If both flat parameters and `_filter` are supplied, the parameters in the legacy `_filter` block will take precedence. + Any filter parameters you don't set will inherit the global defaults which were set with command line flags or environment variables. @@ -495,6 +534,12 @@ setting the equivalent of `--buffer-size` in string or integer format. "_filter":{"MinSize": 44040192} ``` +Or flat: + +```json +"min_size": "42M" +``` + If you wish to check the `_filter` assignment has worked properly then calling `options/local` will show what the value got set to. diff --git a/fs/config/configstruct/configstruct.go b/fs/config/configstruct/configstruct.go index 545f7d166..4fa95fe1c 100644 --- a/fs/config/configstruct/configstruct.go +++ b/fs/config/configstruct/configstruct.go @@ -133,6 +133,19 @@ func InterfaceToString(in any) (strValue string, err error) { if do, ok := in.(fmt.Stringer); ok { strValue = do.String() } else { + // Check if it is a slice or array + val := reflect.ValueOf(in) + if val.Kind() == reflect.Slice || val.Kind() == reflect.Array { + strSlice := make([]string, val.Len()) + for i := 0; i < val.Len(); i++ { + strValue, err = InterfaceToString(val.Index(i).Interface()) + if err != nil { + return "", err + } + strSlice[i] = strValue + } + return InterfaceToString(strSlice) + } err = errors.New("don't know how to convert this") } } @@ -262,14 +275,35 @@ func Set(config configmap.Getter, opt any) (err error) { // setIfSameType set aPtr with b if they are the same type or returns false. func setIfSameType(aPtr any, b any) bool { + if b == nil { + return false + } aVal := reflect.ValueOf(aPtr).Elem() bVal := reflect.ValueOf(b) - if aVal.Type() != bVal.Type() { - return false + if aVal.Type() == bVal.Type() { + aVal.Set(bVal) + return true } - aVal.Set(bVal) - return true + + // Special case: if target is []string and source is a slice or array + if aVal.Type() == reflect.TypeOf([]string(nil)) && (bVal.Kind() == reflect.Slice || bVal.Kind() == reflect.Array) { + strSlice := make([]string, bVal.Len()) + ok := true + for i := 0; i < bVal.Len(); i++ { + var err error + strSlice[i], err = InterfaceToString(bVal.Index(i).Interface()) + if err != nil { + ok = false + break + } + } + if ok { + aVal.Set(reflect.ValueOf(strSlice)) + return true + } + } + return false } // SetAny interprets the field names in defaults and looks up config diff --git a/fs/config/configstruct/configstruct_test.go b/fs/config/configstruct/configstruct_test.go index fb0337220..79ebffbfc 100644 --- a/fs/config/configstruct/configstruct_test.go +++ b/fs/config/configstruct/configstruct_test.go @@ -209,6 +209,45 @@ func TestSetAnyFull(t *testing.T) { assert.Equal(t, want, in) } +func TestSetAnyWithSlice(t *testing.T) { + type ConfWithSlice struct { + ExcludeRule []string `config:"exclude"` + } + + // 1. Test []any (JSON array of strings) + in := &ConfWithSlice{ + ExcludeRule: []string{"original"}, + } + m := map[string]any{ + "exclude": []any{"foo", "bar"}, + } + err := configstruct.SetAny(m, in) + require.NoError(t, err) + assert.Equal(t, []string{"foo", "bar"}, in.ExcludeRule) + + // 2. Test []string directly + in2 := &ConfWithSlice{ + ExcludeRule: []string{"original"}, + } + m2 := map[string]any{ + "exclude": []string{"hello", "world"}, + } + err = configstruct.SetAny(m2, in2) + require.NoError(t, err) + assert.Equal(t, []string{"hello", "world"}, in2.ExcludeRule) + + // 3. Test []any with mixed convertible types (e.g. ints/floats/bools) + in3 := &ConfWithSlice{ + ExcludeRule: []string{"original"}, + } + m3 := map[string]any{ + "exclude": []any{"foo", 123, true, 45.6}, + } + err = configstruct.SetAny(m3, in3) + require.NoError(t, err) + assert.Equal(t, []string{"foo", "123", "true", "45.6"}, in3.ExcludeRule) +} + func TestStringToInterface(t *testing.T) { item := struct{ A int }{2} for _, test := range []struct { @@ -288,6 +327,13 @@ func TestInterfaceToString(t *testing.T) { {[]string{"hello", "world"}, `hello,world`, ""}, {[]string{"hello", "", "world"}, `hello,,world`, ""}, {[]string{`hello, world`, `goodbye, world!`}, `"hello, world","goodbye, world!"`, ""}, + {[]any{"hello", "world"}, `hello,world`, ""}, + {[]any{123, 456}, `123,456`, ""}, + {[]any{"hello", 45.6, true}, `hello,45.6,true`, ""}, + {[2]string{"a", "b"}, `a,b`, ""}, + {[2]any{"x", 12}, `x,12`, ""}, + {fs.SpaceSepList{"a", "b"}, `a b`, ""}, + {fs.CommaSepList{"x", "y"}, `x,y`, ""}, {time.Second, "1s", ""}, {61 * time.Second, "1m1s", ""}, {fs.Mebi, "1Mi", ""}, diff --git a/fs/rc/context.go b/fs/rc/context.go new file mode 100644 index 000000000..3f68c0c4f --- /dev/null +++ b/fs/rc/context.go @@ -0,0 +1,170 @@ +package rc + +import ( + "context" + "reflect" + "sync" + + "github.com/rclone/rclone/fs" + "github.com/rclone/rclone/fs/config/configstruct" + "github.com/rclone/rclone/fs/filter" +) + +var ( + configOptionsOnce sync.Once + configOptionsMap map[string]bool + + filterOptionsOnce sync.Once + filterOptionsMap map[string]bool +) + +func initConfigOptions() { + configOptionsOnce.Do(func() { + configOptionsMap = make(map[string]bool, len(fs.ConfigOptionsInfo)) + for _, opt := range fs.ConfigOptionsInfo { + configOptionsMap[opt.Name] = true + } + }) +} + +func initFilterOptions() { + filterOptionsOnce.Do(func() { + filterOptionsMap = make(map[string]bool, len(filter.OptionsInfo)) + for _, opt := range filter.OptionsInfo { + filterOptionsMap[opt.Name] = true + } + }) +} + +// isMap returns true if v's underlying type is a map +func isMap(v any) bool { + if v == nil { + return false + } + t := reflect.TypeOf(v) + for t.Kind() == reflect.Pointer { + t = t.Elem() + } + return t.Kind() == reflect.Map +} + +// hasConfigOption checks if any config options are present in the params +func hasConfigOption(in Params) bool { + if _, ok := in["_config"]; ok { + return true + } + initConfigOptions() + for k, v := range in { + if configOptionsMap[k] { + if isMap(v) { + continue + } + return true + } + } + return false +} + +// hasFilterOption checks if any filter options are present in the params +func hasFilterOption(in Params) bool { + if _, ok := in["_filter"]; ok { + return true + } + initFilterOptions() + for k, v := range in { + if filterOptionsMap[k] { + if isMap(v) { + continue + } + return true + } + } + return false +} + +// AddConfig parses any config options from the parameters and returns a new context with the configuration. +func AddConfig(ctx context.Context, in Params) (context.Context, error) { + if !hasConfigOption(in) { + return ctx, nil + } + ctx, ci := fs.AddConfig(ctx) + + // Extract the genuine flat config options + initConfigOptions() + flatConfig := make(map[string]any) + for k, v := range in { + if configOptionsMap[k] { + if isMap(v) { + continue + } + flatConfig[k] = v + } + } + + if len(flatConfig) > 0 { + err := configstruct.SetAny(flatConfig, ci) + if err != nil { + return ctx, err + } + // Remove the consumed flat options from the input params + for k := range flatConfig { + delete(in, k) + } + } + + if _, ok := in["_config"]; ok { + err := in.GetStruct("_config", ci) + if err != nil { + return ctx, err + } + delete(in, "_config") // remove the parameter + } + return ctx, nil +} + +// AddFilter parses any filter options from the parameters and returns a new context with the filter. +func AddFilter(ctx context.Context, in Params) (context.Context, error) { + if !hasFilterOption(in) { + return ctx, nil + } + // Copy of the current filter options + opt := filter.GetConfig(ctx).Opt + + // Extract the genuine flat filter options + initFilterOptions() + flatFilter := make(map[string]any) + for k, v := range in { + if filterOptionsMap[k] { + if isMap(v) { + continue + } + flatFilter[k] = v + } + } + + if len(flatFilter) > 0 { + err := configstruct.SetAny(flatFilter, &opt) + if err != nil { + return ctx, err + } + // Remove the consumed flat options from the input params + for k := range flatFilter { + delete(in, k) + } + } + + if _, ok := in["_filter"]; ok { + // Update the options from the parameter + err := in.GetStruct("_filter", &opt) + if err != nil { + return ctx, err + } + delete(in, "_filter") // remove the parameter + } + fi, err := filter.NewFilter(&opt) + if err != nil { + return ctx, err + } + ctx = filter.ReplaceConfig(ctx, fi) + return ctx, nil +} diff --git a/fs/rc/jobs/job.go b/fs/rc/jobs/job.go index 2ba939210..e1acfae4c 100644 --- a/fs/rc/jobs/job.go +++ b/fs/rc/jobs/job.go @@ -19,7 +19,6 @@ import ( "github.com/rclone/rclone/fs" "github.com/rclone/rclone/fs/accounting" "github.com/rclone/rclone/fs/cache" - "github.com/rclone/rclone/fs/filter" "github.com/rclone/rclone/fs/rc" "golang.org/x/sync/errgroup" ) @@ -242,41 +241,6 @@ func getAsync(ctx context.Context, in rc.Params) (context.Context, bool, error) return ctx, isAsync, nil } -// See if _config is set and if so adjust ctx to include it -func getConfig(ctx context.Context, in rc.Params) (context.Context, error) { - if _, ok := in["_config"]; !ok { - return ctx, nil - } - ctx, ci := fs.AddConfig(ctx) - err := in.GetStruct("_config", ci) - if err != nil { - return ctx, err - } - delete(in, "_config") // remove the parameter - return ctx, nil -} - -// See if _filter is set and if so adjust ctx to include it -func getFilter(ctx context.Context, in rc.Params) (context.Context, error) { - if _, ok := in["_filter"]; !ok { - return ctx, nil - } - // Copy of the current filter options - opt := filter.GetConfig(ctx).Opt - // Update the options from the parameter - err := in.GetStruct("_filter", &opt) - if err != nil { - return ctx, err - } - fi, err := filter.NewFilter(&opt) - if err != nil { - return ctx, err - } - ctx = filter.ReplaceConfig(ctx, fi) - delete(in, "_filter") // remove the parameter - return ctx, nil -} - type jobKeyType struct{} // Key for adding jobs to ctx @@ -292,12 +256,12 @@ func (jobs *Jobs) NewJob(ctx context.Context, fn rc.Func, in rc.Params) (job *Jo return nil, nil, err } - ctx, err = getConfig(ctx, in) + ctx, err = rc.AddConfig(ctx, in) if err != nil { return nil, nil, err } - ctx, err = getFilter(ctx, in) + ctx, err = rc.AddFilter(ctx, in) if err != nil { return nil, nil, err } diff --git a/fs/rc/jobs/job_test.go b/fs/rc/jobs/job_test.go index eaf95aead..02f171860 100644 --- a/fs/rc/jobs/job_test.go +++ b/fs/rc/jobs/job_test.go @@ -352,6 +352,141 @@ func TestExecuteJobWithFilter(t *testing.T) { assert.Equal(t, true, called) } +func TestExecuteJobWithFlatConfig(t *testing.T) { + ctx := context.Background() + jobID.Store(0) + called := false + jobFn := func(ctx context.Context, in rc.Params) (rc.Params, error) { + ci := fs.GetConfig(ctx) + assert.Equal(t, 42*fs.Mebi, ci.BufferSize) + called = true + return nil, nil + } + _, _, err := NewJob(ctx, jobFn, rc.Params{ + "buffer_size": "42M", + }) + require.NoError(t, err) + assert.Equal(t, true, called) + + // Test that legacy _config overrides flat parameter + jobID.Store(0) + called = false + jobFn2 := func(ctx context.Context, in rc.Params) (rc.Params, error) { + ci := fs.GetConfig(ctx) + assert.Equal(t, 10*fs.Mebi, ci.BufferSize) + called = true + return nil, nil + } + _, _, err = NewJob(ctx, jobFn2, rc.Params{ + "buffer_size": "42M", + "_config": rc.Params{ + "BufferSize": "10M", + }, + }) + require.NoError(t, err) + assert.Equal(t, true, called) +} + +func TestExecuteJobWithFlatFilter(t *testing.T) { + ctx := context.Background() + called := false + jobID.Store(0) + jobFn := func(ctx context.Context, in rc.Params) (rc.Params, error) { + fi := filter.GetConfig(ctx) + assert.Equal(t, fs.SizeSuffix(1024), fi.Opt.MaxSize) + assert.Equal(t, []string{"a", "b", "c"}, fi.Opt.IncludeRule) + called = true + return nil, nil + } + _, _, err := NewJob(ctx, jobFn, rc.Params{ + "include": []string{"a", "b", "c"}, + "max_size": "1k", + }) + require.NoError(t, err) + assert.Equal(t, true, called) + + // Test that legacy _filter overrides flat parameter + called = false + jobID.Store(0) + jobFn2 := func(ctx context.Context, in rc.Params) (rc.Params, error) { + fi := filter.GetConfig(ctx) + assert.Equal(t, fs.SizeSuffix(2048), fi.Opt.MaxSize) + assert.Equal(t, []string{"x", "y"}, fi.Opt.IncludeRule) + called = true + return nil, nil + } + _, _, err = NewJob(ctx, jobFn2, rc.Params{ + "include": []string{"a", "b", "c"}, + "max_size": "1k", + "_filter": rc.Params{ + "IncludeRule": []string{"x", "y"}, + "MaxSize": "2k", + }, + }) + require.NoError(t, err) + assert.Equal(t, true, called) +} + +// A null-valued flat config/filter param must produce a clean +// error, not panic the rc handler. +func TestExecuteJobWithFlatConfigNull(t *testing.T) { + ctx := context.Background() + jobID.Store(0) + called := false + jobFn := func(ctx context.Context, in rc.Params) (rc.Params, error) { + called = true + return nil, nil + } + var err error + require.NotPanics(t, func() { + _, _, err = NewJob(ctx, jobFn, rc.Params{ + "buffer_size": nil, + }) + }) + assert.Error(t, err) + assert.False(t, called) +} + +// Flat config/filter options should be consumed and removed from +// the params (like _config and _filter are), so they don't leak into the +// command's parameter map. +func TestExecuteJobFlatParamsRemoved(t *testing.T) { + ctx := context.Background() + jobID.Store(0) + var got rc.Params + jobFn := func(ctx context.Context, in rc.Params) (rc.Params, error) { + got = in + return nil, nil + } + _, _, err := NewJob(ctx, jobFn, rc.Params{ + "buffer_size": "42M", + "max_size": "1k", + "include": []string{"a"}, + }) + require.NoError(t, err) + _, ok := got["buffer_size"] + assert.False(t, ok, "flat config option buffer_size should have been removed from in") + _, ok = got["max_size"] + assert.False(t, ok, "flat filter option max_size should have been removed from in") + _, ok = got["include"] + assert.False(t, ok, "flat filter option include should have been removed from in") +} + +// options/set with a "filter" block is a valid, documented call, but +// the flat-parameter feature treats the top-level "filter" key (a registered +// filter option name) as the --filter option and fails trying to parse the +// block map as a string, breaking the call. +func TestExecuteJobOptionsSetFilterBlock(t *testing.T) { + ctx := context.Background() + jobID.Store(0) + call := rc.Calls.Get("options/set") + require.NotNil(t, call) + _, _, err := NewJob(ctx, call.Fn, rc.Params{ + "filter": rc.Params{"MaxSize": "1M"}, + }) + require.NoError(t, err) +} + func TestExecuteJobWithGroup(t *testing.T) { ctx := context.Background() jobID.Store(0)