From a50d1137a383c101f6585400421b68ec587480f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hakan=20=C4=B0SMA=C4=B0L?= <93117749+Hakanbaban53@users.noreply.github.com> Date: Wed, 1 Jul 2026 17:05:10 +0300 Subject: [PATCH] fs/rc: add ParseOptions and CheckParamsUsed unified options helpers --- fs/rc/context.go | 168 ++++++++++++++---------------------------- fs/rc/context_test.go | 124 +++++++++++++++++++++++++++++++ 2 files changed, 181 insertions(+), 111 deletions(-) create mode 100644 fs/rc/context_test.go diff --git a/fs/rc/context.go b/fs/rc/context.go index 3f68c0c4f..b9d441a68 100644 --- a/fs/rc/context.go +++ b/fs/rc/context.go @@ -2,40 +2,16 @@ package rc import ( "context" + "fmt" "reflect" - "sync" + "sort" + "strings" "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 { @@ -48,118 +24,88 @@ func isMap(v any) bool { 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 { +// hasOption checks if any options are present in the params +func hasOption(in Params, key string, opt any) bool { + if _, ok := in[key]; ok { return true } - initConfigOptions() - for k, v := range in { - if configOptionsMap[k] { - if isMap(v) { - continue - } + items, err := configstruct.Items(opt) + if err != nil { + return false + } + for _, item := range items { + if v, ok := in[item.Name]; ok && !isMap(v) { 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 +// ParseOptions sets opt from the flat parameters in whose names match opt's +// fields, then overlays the nested block under key if present. Every key it +// consumes is deleted from in. Values in the nested block take precedence over +// flat ones. opt must be a pointer to a struct of configstruct-settable fields. +func ParseOptions(in Params, key string, opt any) error { + items, err := configstruct.Items(opt) + if err != nil { + return err } - initFilterOptions() - for k, v := range in { - if filterOptionsMap[k] { - if isMap(v) { - continue - } - return true + flat := make(map[string]any) + for _, item := range items { + if v, ok := in[item.Name]; ok && !isMap(v) { + flat[item.Name] = v } } - return false + if len(flat) > 0 { + if err := configstruct.SetAny(flat, opt); err != nil { + return err + } + for k := range flat { + delete(in, k) + } + } + if err := in.GetStructMissingOK(key, opt); err != nil { + return err + } + delete(in, key) + return nil +} + +// CheckParamsUsed returns an error if any parameters remain in the map. +// It formats them as a list of sorted keys for determinism. +func CheckParamsUsed(in Params) error { + if len(in) == 0 { + return nil + } + keys := make([]string, 0, len(in)) + for k := range in { + keys = append(keys, k) + } + sort.Strings(keys) + return fmt.Errorf("unknown parameters: %s", strings.Join(keys, ", ")) } // 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) { + if !hasOption(in, "_config", &fs.ConfigInfo{}) { 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 + if err := ParseOptions(in, "_config", ci); err != nil { + return ctx, err } 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) { + if !hasOption(in, "_filter", &filter.Opt) { 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 + if err := ParseOptions(in, "_filter", &opt); err != nil { + return ctx, err } fi, err := filter.NewFilter(&opt) if err != nil { diff --git a/fs/rc/context_test.go b/fs/rc/context_test.go new file mode 100644 index 000000000..ee9bce36e --- /dev/null +++ b/fs/rc/context_test.go @@ -0,0 +1,124 @@ +package rc + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +type dummyOptions struct { + StringOpt string `config:"string_opt"` + IntOpt int `config:"int_opt"` + BoolOpt bool `config:"bool_opt"` + SliceOpt []string `config:"slice_opt"` +} + +func TestParseOptions(t *testing.T) { + t.Run("FlatOnly", func(t *testing.T) { + in := Params{ + "string_opt": "hello", + "int_opt": 42, + "bool_opt": true, + "slice_opt": []any{"a", "b"}, + } + var opt dummyOptions + err := ParseOptions(in, "dummy", &opt) + require.NoError(t, err) + assert.Equal(t, "hello", opt.StringOpt) + assert.Equal(t, 42, opt.IntOpt) + assert.True(t, opt.BoolOpt) + assert.Equal(t, []string{"a", "b"}, opt.SliceOpt) + assert.Empty(t, in) + }) + + t.Run("NestedOnly", func(t *testing.T) { + in := Params{ + "dummy": Params{ + "StringOpt": "nested_hello", + "IntOpt": 100, + "BoolOpt": false, + "SliceOpt": []string{"x", "y"}, + }, + } + var opt dummyOptions + err := ParseOptions(in, "dummy", &opt) + require.NoError(t, err) + assert.Equal(t, "nested_hello", opt.StringOpt) + assert.Equal(t, 100, opt.IntOpt) + assert.False(t, opt.BoolOpt) + assert.Equal(t, []string{"x", "y"}, opt.SliceOpt) + assert.Empty(t, in) + }) + + t.Run("BothPrecedence", func(t *testing.T) { + in := Params{ + "string_opt": "flat_value", + "int_opt": 42, + "dummy": Params{ + "StringOpt": "nested_value", + }, + } + var opt dummyOptions + err := ParseOptions(in, "dummy", &opt) + require.NoError(t, err) + assert.Equal(t, "nested_value", opt.StringOpt) // Nested takes precedence + assert.Equal(t, 42, opt.IntOpt) // Flat is still parsed + assert.Empty(t, in) + }) + + t.Run("MapSkip", func(t *testing.T) { + in := Params{ + "string_opt": Params{"nested_key": "some_value"}, // string_opt matches but value is a map, should be skipped + "int_opt": 42, + } + var opt dummyOptions + err := ParseOptions(in, "dummy", &opt) + require.NoError(t, err) + assert.Equal(t, "", opt.StringOpt) // skipped + assert.Equal(t, 42, opt.IntOpt) + assert.Len(t, in, 1) + assert.Contains(t, in, "string_opt") + }) + + t.Run("NullErrors", func(t *testing.T) { + in := Params{ + "string_opt": nil, // string_opt is nil, configstruct should return error + } + var opt dummyOptions + err := ParseOptions(in, "dummy", &opt) + assert.Error(t, err) + assert.Contains(t, err.Error(), "interpreting as string failed") + }) + + t.Run("UnknownLeftBehind", func(t *testing.T) { + in := Params{ + "string_opt": "value", + "unknown": "leftover", + } + var opt dummyOptions + err := ParseOptions(in, "dummy", &opt) + require.NoError(t, err) + assert.Equal(t, "value", opt.StringOpt) + assert.Len(t, in, 1) + assert.Equal(t, "leftover", in["unknown"]) + }) +} + +func TestCheckParamsUsed(t *testing.T) { + t.Run("Empty", func(t *testing.T) { + in := Params{} + err := CheckParamsUsed(in) + assert.NoError(t, err) + }) + + t.Run("Leftovers", func(t *testing.T) { + in := Params{ + "z": "last", + "a": "first", + } + err := CheckParamsUsed(in) + assert.Error(t, err) + assert.Equal(t, "unknown parameters: a, z", err.Error()) + }) +}