Merge pull request #266 from FluffyEscargot/fix-mutually-exclusive-flags

Make once, cron and begin/frequency flags mutually exclusive

Author: deitch

cmd/dump.go

@@ -227,5 +227,11 @@ S3: If it is a URL of the format s3://bucketname/path then it will connect via S
 	// max-allowed-packet size
 	flags.Int("max-allowed-packet", defaultMaxAllowedPacket, "Maximum size of the buffer for client/server communication, similar to mysqldump's max_allowed_packet. 0 means to use the default size.")
 
+	cmd.MarkFlagsMutuallyExclusive("once", "cron")
+	cmd.MarkFlagsMutuallyExclusive("once", "begin")
+	cmd.MarkFlagsMutuallyExclusive("once", "frequency")
+	cmd.MarkFlagsMutuallyExclusive("cron", "begin")
+	cmd.MarkFlagsMutuallyExclusive("cron", "frequency")
+
 	return cmd, nil
 }

cmd/dump_test.go

@@ -34,12 +34,41 @@ func TestDumpCmd(t *testing.T) {
 			Compressor:       &compression.GzipCompressor{},
 			DBConn:           database.Connection{Host: "abc"},
 		}, core.TimerOptions{Frequency: defaultFrequency, Begin: defaultBegin}},
+		{"once flag", []string{"--server", "abc", "--target", "file:///foo/bar", "--once"}, "", false, core.DumpOptions{
+			Targets:          []storage.Storage{file.New(*fileTargetURL)},
+			MaxAllowedPacket: defaultMaxAllowedPacket,
+			Compressor:       &compression.GzipCompressor{},
+			DBConn:           database.Connection{Host: "abc"},
+		}, core.TimerOptions{Frequency: defaultFrequency, Begin: defaultBegin, Once: true}},
+		{"cron flag", []string{"--server", "abc", "--target", "file:///foo/bar", "--cron", "0 0 * * *"}, "", false, core.DumpOptions{
+			Targets:          []storage.Storage{file.New(*fileTargetURL)},
+			MaxAllowedPacket: defaultMaxAllowedPacket,
+			Compressor:       &compression.GzipCompressor{},
+			DBConn:           database.Connection{Host: "abc"},
+		}, core.TimerOptions{Frequency: defaultFrequency, Begin: defaultBegin, Cron: "0 0 * * *"}},
+		{"begin flag", []string{"--server", "abc", "--target", "file:///foo/bar", "--begin", "1234"}, "", false, core.DumpOptions{
+			Targets:          []storage.Storage{file.New(*fileTargetURL)},
+			MaxAllowedPacket: defaultMaxAllowedPacket,
+			Compressor:       &compression.GzipCompressor{},
+			DBConn:           database.Connection{Host: "abc"},
+		}, core.TimerOptions{Frequency: defaultFrequency, Begin: "1234"}},
+		{"frequency flag", []string{"--server", "abc", "--target", "file:///foo/bar", "--frequency", "10"}, "", false, core.DumpOptions{
+			Targets:          []storage.Storage{file.New(*fileTargetURL)},
+			MaxAllowedPacket: defaultMaxAllowedPacket,
+			Compressor:       &compression.GzipCompressor{},
+			DBConn:           database.Connection{Host: "abc"},
+		}, core.TimerOptions{Frequency: 10, Begin: defaultBegin}},
 		{"config file", []string{"--config-file", "testdata/config.yml"}, "", false, core.DumpOptions{
 			Targets:          []storage.Storage{file.New(*fileTargetURL)},
 			MaxAllowedPacket: defaultMaxAllowedPacket,
 			Compressor:       &compression.GzipCompressor{},
 			DBConn:           database.Connection{Host: "abc", Port: 3306, User: "user", Pass: "xxxx"},
 		}, core.TimerOptions{Frequency: defaultFrequency, Begin: defaultBegin}},
+		{"incompatible flags: once/cron", []string{"--server", "abc", "--target", "file:///foo/bar", "--once", "--cron", "0 0 * * *"}, "", true, core.DumpOptions{}, core.TimerOptions{}},
+		{"incompatible flags: once/begin", []string{"--server", "abc", "--target", "file:///foo/bar", "--once", "--begin", "1234"}, "", true, core.DumpOptions{}, core.TimerOptions{}},
+		{"incompatible flags: once/frequency", []string{"--server", "abc", "--target", "file:///foo/bar", "--once", "--frequency", "10"}, "", true, core.DumpOptions{}, core.TimerOptions{}},
+		{"incompatible flags: cron/begin", []string{"--server", "abc", "--target", "file:///foo/bar", "--cron", "0 0 * * *", "--begin", "1234"}, "", true, core.DumpOptions{}, core.TimerOptions{}},
+		{"incompatible flags: cron/frequency", []string{"--server", "abc", "--target", "file:///foo/bar", "--cron", "0 0 * * *", "--frequency", "10"}, "", true, core.DumpOptions{}, core.TimerOptions{}},
 	}
 
 	for _, tt := range tests {

pkg/core/timer.go

@@ -1,7 +1,6 @@
 package core
 
 import (
-	"errors"
 	"fmt"
 	"regexp"
 	"strconv"
@@ -40,25 +39,14 @@ func Timer(opts TimerOptions) (<-chan Update, error) {
 	)
 
 	now := time.Now().UTC()
-
-	// validate we do not have conflicting options
-	if opts.Once && (opts.Cron != "" || opts.Begin != "" || opts.Frequency != 0) {
-		return nil, errors.New("option 'Once' is exclusive and must not be used with Begin, Cron or Frequency")
-	}
-
-	if opts.Cron != "" && (opts.Begin != "" || opts.Frequency != 0) {
-		return nil, errors.New("option 'Cron' is exclusive and must not be used with Begin, Once or Frequency")
-	}
-
 	// parse the options to determine our delays
 	if opts.Cron != "" {
 		// calculate delay until next cron moment as defined
 		delay, err = waitForCron(opts.Cron, now)
 		if err != nil {
 			return nil, fmt.Errorf("invalid cron format '%s': %v", opts.Cron, err)
 		}
-	}
-	if opts.Begin != "" {
+	} else if opts.Begin != "" {
 		// calculate how long to wait
 		minsRe, err := regexp.Compile(`^\+([0-9]+)$`)
 		if err != nil {