diff --git a/internal/core/validate.go b/internal/core/validate.go index 4b8f170cb3..9741f3f99c 100644 --- a/internal/core/validate.go +++ b/internal/core/validate.go @@ -21,8 +21,8 @@ type CommandValidateFunc func(ctx context.Context, cmd *Command, cmdArgs interfa type ArgSpecValidateFunc func(argSpec *ArgSpec, value interface{}) error type OneOfGroupManager struct { - Groups map[string][]string // Contient les noms des arguments pour chaque groupe - RequiredGroups map[string]bool // Indique si un groupe est requis + Groups map[string][]string + RequiredGroups map[string]bool } // DefaultCommandValidateFunc is the default validation function for commands. @@ -74,9 +74,6 @@ func validateArgValues(cmd *Command, cmdArgs interface{}) error { // Returns nil otherwise. // TODO refactor this method which uses a mix of reflect and string arrays func validateRequiredArgs(cmd *Command, cmdArgs interface{}, rawArgs args.RawArgs) error { - - oneOfManager := NewOneOfGroupManager(cmd) - for _, arg := range cmd.ArgSpecs { if !arg.Required || arg.OneOfGroup != "" { continue @@ -103,14 +100,24 @@ func validateRequiredArgs(cmd *Command, cmdArgs interface{}, rawArgs args.RawArg } } } - - if err := oneOfManager.ValidateOneOfGroups(rawArgs); err != nil { + if err := validateOneOfRequiredArgs(cmd, rawArgs); err != nil { return err } return nil } +func validateOneOfRequiredArgs(cmd *Command, rawArgs args.RawArgs) error { + oneOfManager := NewOneOfGroupManager(cmd) + if err := oneOfManager.ValidateUniqueOneOfGroups(rawArgs); err != nil { + return err + } + if err := oneOfManager.ValidateRequiredOneOfGroups(rawArgs); err != nil { + return err + } + return nil +} + func ValidateNoConflict(cmd *Command, rawArgs args.RawArgs) error { for _, arg1 := range cmd.ArgSpecs { for _, arg2 := range cmd.ArgSpecs { @@ -281,7 +288,22 @@ func NewOneOfGroupManager(cmd *Command) *OneOfGroupManager { return manager } -func (m *OneOfGroupManager) ValidateOneOfGroups(rawArgs args.RawArgs) error { +func (m *OneOfGroupManager) ValidateUniqueOneOfGroups(rawArgs args.RawArgs) error { + for _, groupArgs := range m.Groups { + existingArg := "" + for _, argName := range groupArgs { + if rawArgs.ExistsArgByName(argName) { + if existingArg != "" { + return fmt.Errorf("arguments '%s' and '%s' are mutually exclusive", existingArg, argName) + } + existingArg = argName + } + } + } + return nil +} + +func (m *OneOfGroupManager) ValidateRequiredOneOfGroups(rawArgs args.RawArgs) error { for group, required := range m.RequiredGroups { if required { found := false diff --git a/internal/core/validate_test.go b/internal/core/validate_test.go index c71ac921fb..571b1f33d2 100644 --- a/internal/core/validate_test.go +++ b/internal/core/validate_test.go @@ -477,7 +477,7 @@ func TestNewOneOfGroupManager(t *testing.T) { } } -func TestValidateOneOfGroups(t *testing.T) { +func TestValidateRequiredOneOfGroups(t *testing.T) { tests := []struct { name string setupManager func() *core.OneOfGroupManager @@ -522,7 +522,7 @@ func TestValidateOneOfGroups(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { manager := tt.setupManager() - err := manager.ValidateOneOfGroups(tt.rawArgs) + err := manager.ValidateRequiredOneOfGroups(tt.rawArgs) if tt.expectedError == "" { assert.NoError(t, err, "Expected no error, got %v", err) @@ -532,3 +532,254 @@ func TestValidateOneOfGroups(t *testing.T) { }) } } + +func TestValidateUniqueOneOfGroups(t *testing.T) { + tests := []struct { + name string + setupManager func() *core.OneOfGroupManager + rawArgs args.RawArgs + expectedError string + }{ + { + name: "Required group satisfied with first argument", + setupManager: func() *core.OneOfGroupManager { + return &core.OneOfGroupManager{ + Groups: map[string][]string{"group1": {"a", "b"}}, + } + }, + rawArgs: []string{"a=true"}, + expectedError: "", + }, + { + name: "No arguments passed", + setupManager: func() *core.OneOfGroupManager { + return &core.OneOfGroupManager{ + Groups: map[string][]string{"group1": {"a", "b"}}, + } + }, + rawArgs: []string{}, + expectedError: "", + }, + { + name: "Multiple groups, all satisfied", + setupManager: func() *core.OneOfGroupManager { + return &core.OneOfGroupManager{ + Groups: map[string][]string{ + "group1": {"a", "b"}, + "group2": {"c", "d"}, + }, + } + }, + rawArgs: []string{"a=true", "c=true"}, + expectedError: "", + }, + { + name: "Multiple groups, one satisfied", + setupManager: func() *core.OneOfGroupManager { + return &core.OneOfGroupManager{ + Groups: map[string][]string{ + "group1": {"a", "b"}, + "group2": {"c", "d"}, + }, + } + }, + rawArgs: []string{"a=true"}, + expectedError: "", + }, + { + name: "Multiple groups, not exclusive argument for groups 2", + setupManager: func() *core.OneOfGroupManager { + return &core.OneOfGroupManager{ + Groups: map[string][]string{ + "group1": {"a", "b"}, + "group2": {"c", "d"}, + }, + } + }, + rawArgs: []string{"a=true", "c=true", "d=true"}, + expectedError: "arguments 'c' and 'd' are mutually exclusive", + }, + { + name: "Multiple groups, not exclusive argument for groups 1", + setupManager: func() *core.OneOfGroupManager { + return &core.OneOfGroupManager{ + Groups: map[string][]string{ + "group1": {"a", "b"}, + "group2": {"c", "d"}, + }, + } + }, + rawArgs: []string{"a=true", "b=true", "c=true"}, + expectedError: "arguments 'a' and 'b' are mutually exclusive", + }, + { + name: "One group, not exclusive argument for groups 1", + setupManager: func() *core.OneOfGroupManager { + return &core.OneOfGroupManager{ + Groups: map[string][]string{ + "group1": {"a", "b", "c", "d"}, + }, + } + }, + rawArgs: []string{"a=true", "d=true"}, + expectedError: "arguments 'a' and 'd' are mutually exclusive", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + manager := tt.setupManager() + err := manager.ValidateUniqueOneOfGroups(tt.rawArgs) + if tt.expectedError == "" { + assert.NoError(t, err, "Expected no error, got %v", err) + } else { + assert.EqualError(t, err, tt.expectedError, fmt.Sprintf("Expected error message '%s', got '%v'", tt.expectedError, err)) + } + }) + } +} + +func Test_ValidateOneOf(t *testing.T) { + t.Run("Validate OneOf", core.Test(&core.TestConfig{ + Commands: core.NewCommands(&core.Command{ + Namespace: "oneof", + ArgsType: reflect.TypeOf(args.RawArgs{}), + AllowAnonymousClient: true, + Run: func(_ context.Context, _ interface{}) (i interface{}, e error) { + return &core.SuccessResult{}, nil + }, + ArgSpecs: core.ArgSpecs{ + { + Name: "a", + OneOfGroup: "groups1", + }, + { + Name: "b", + OneOfGroup: "groups1", + }, + }, + }), + Cmd: "scw oneof a=yo", + Check: core.TestCheckCombine( + core.TestCheckExitCode(0), + ), + })) + t.Run("Required group satisfied", func(t *testing.T) { + core.Test(&core.TestConfig{ + Commands: core.NewCommands(&core.Command{ + Namespace: "oneof", + ArgsType: reflect.TypeOf(args.RawArgs{}), + AllowAnonymousClient: true, + Run: func(_ context.Context, _ interface{}) (i interface{}, e error) { + return &core.SuccessResult{}, nil + }, + ArgSpecs: core.ArgSpecs{ + { + Name: "a", + OneOfGroup: "group1", + Required: true, + }, + { + Name: "b", + OneOfGroup: "group1", + Required: true, + }, + }, + }), + Cmd: "scw oneof b=yo", + Check: core.TestCheckCombine( + core.TestCheckExitCode(0), + ), + })(t) + }) + + t.Run("Required group not satisfied", func(t *testing.T) { + core.Test(&core.TestConfig{ + Commands: core.NewCommands(&core.Command{ + Namespace: "oneof", + ArgsType: reflect.TypeOf(args.RawArgs{}), + AllowAnonymousClient: true, + Run: func(_ context.Context, _ interface{}) (i interface{}, e error) { + return &core.SuccessResult{}, nil + }, + ArgSpecs: core.ArgSpecs{ + { + Name: "a", + OneOfGroup: "group1", + Required: true, + }, + { + Name: "b", + OneOfGroup: "group1", + Required: true, + }, + }, + }), + Cmd: "scw oneof c=yo", + Check: core.TestCheckCombine( + core.TestCheckExitCode(1), + core.TestCheckError(fmt.Errorf("at least one argument from the 'group1' group is required")), + ), + })(t) + }) + + t.Run("Arguments are mutually exclusive", func(t *testing.T) { + core.Test(&core.TestConfig{ + Commands: core.NewCommands(&core.Command{ + Namespace: "oneof", + ArgsType: reflect.TypeOf(args.RawArgs{}), + AllowAnonymousClient: true, + Run: func(_ context.Context, _ interface{}) (i interface{}, e error) { + return &core.SuccessResult{}, nil + }, + ArgSpecs: core.ArgSpecs{ + { + Name: "a", + OneOfGroup: "group1", + }, + { + Name: "b", + OneOfGroup: "group1", + }, + }, + }), + Cmd: "scw oneof a=yo b=no", + Check: core.TestCheckCombine( + core.TestCheckExitCode(1), + core.TestCheckError(fmt.Errorf("arguments 'a' and 'b' are mutually exclusive")), + ), + })(t) + }) + + t.Run("Arguments are mutually exclusive with 3 arguments", func(t *testing.T) { + core.Test(&core.TestConfig{ + Commands: core.NewCommands(&core.Command{ + Namespace: "oneof", + ArgsType: reflect.TypeOf(args.RawArgs{}), + AllowAnonymousClient: true, + Run: func(_ context.Context, _ interface{}) (i interface{}, e error) { + return &core.SuccessResult{}, nil + }, + ArgSpecs: core.ArgSpecs{ + { + Name: "a", + OneOfGroup: "group1", + }, + { + Name: "b", + OneOfGroup: "group1", + }, + { + Name: "c", + OneOfGroup: "group1", + }, + }, + }), + Cmd: "scw oneof a=yo c=no", + Check: core.TestCheckCombine( + core.TestCheckExitCode(1), + core.TestCheckError(fmt.Errorf("arguments 'a' and 'c' are mutually exclusive")), + ), + })(t) + }) +}