Clarify error when passing a negative int to rollback

[#174107413](https://www.pivotaltracker.com/story/show/174107413)
cloudfoundry · Oct 2, 2020 · 3e3c7fa · 3e3c7fa
1 parent 2c1edf4
commit 3e3c7fa
Show file tree

Hide file tree

Showing 4 changed files with 109 additions and 6 deletions.
diff --git a/command/flag/revision.go b/command/flag/revision.go
@@ -0,0 +1,34 @@
+package flag
+
+import (
+	"code.cloudfoundry.org/cli/types"
+	flags "github.com/jessevdk/go-flags"
+)
+
+type Revision struct {
+	types.NullInt
+}
+
+func (i *Revision) UnmarshalFlag(val string) error {
+	err := i.ParseStringValue(val)
+	if err != nil {
+		return &flags.Error{
+			Type:    flags.ErrRequired,
+			Message: "invalid argument for flag '--revision' (expected int > 0)",
+		}
+	}
+	if i.Value < 1 {
+		if i.Value == 0 && i.IsSet == false {
+			return nil
+		}
+		return &flags.Error{
+			Type:    flags.ErrRequired,
+			Message: "invalid argument for flag '--revision' (expected int > 0)",
+		}
+	}
+	return nil
+}
+
+func (i *Revision) IsValidValue(val string) error {
+	return i.UnmarshalFlag(val)
+}
diff --git a/command/flag/revision_test.go b/command/flag/revision_test.go
@@ -0,0 +1,68 @@
+package flag_test
+
+import (
+	. "code.cloudfoundry.org/cli/command/flag"
+	"code.cloudfoundry.org/cli/types"
+	flags "github.com/jessevdk/go-flags"
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+)
+
+var _ = Describe("Revision", func() {
+	var revision Revision
+
+	BeforeEach(func() {
+		revision = Revision{}
+	})
+
+	Describe("UnmarshalFlag", func() {
+		When("the empty string is provided", func() {
+			It("sets IsSet to false", func() {
+				err := revision.IsValidValue("")
+				Expect(err).ToNot(HaveOccurred())
+				Expect(revision).To(Equal(Revision{NullInt: types.NullInt{Value: 0, IsSet: false}}))
+			})
+		})
+
+		When("an invalid integer is provided", func() {
+			It("returns an error", func() {
+				err := revision.IsValidValue("abcdef")
+				Expect(err).To(MatchError(&flags.Error{
+					Type:    flags.ErrRequired,
+					Message: "invalid argument for flag '--revision' (expected int > 0)",
+				}))
+				Expect(revision).To(Equal(Revision{NullInt: types.NullInt{Value: 0, IsSet: false}}))
+			})
+		})
+
+		When("a negative integer is provided", func() {
+			It("returns an error", func() {
+				err := revision.IsValidValue("-10")
+				Expect(err).To(MatchError(&flags.Error{
+					Type:    flags.ErrRequired,
+					Message: "invalid argument for flag '--revision' (expected int > 0)",
+				}))
+				Expect(revision).To(Equal(Revision{NullInt: types.NullInt{Value: -10, IsSet: true}}))
+			})
+		})
+
+		When("0 is provided", func() {
+			It("returns an error", func() {
+				err := revision.IsValidValue("0")
+				Expect(err).To(MatchError(&flags.Error{
+					Type:    flags.ErrRequired,
+					Message: "invalid argument for flag '--revision' (expected int > 0)",
+				}))
+				Expect(revision).To(Equal(Revision{NullInt: types.NullInt{Value: 0, IsSet: true}}))
+			})
+		})
+
+		When("a valid integer is provided", func() {
+			It("stores the integer and sets IsSet to true", func() {
+				err := revision.IsValidValue("1")
+				Expect(err).ToNot(HaveOccurred())
+				Expect(revision).To(Equal(Revision{NullInt: types.NullInt{Value: 1, IsSet: true}}))
+			})
+		})
+	})
+})
diff --git a/command/v7/rollback_command.go b/command/v7/rollback_command.go
@@ -14,11 +14,11 @@ import (
 type RollbackCommand struct {
 	BaseCommand
 
-	Force           bool                 `short:"f" description:"Force rollback without confirmation"`
-	RequiredArgs    flag.AppName         `positional-args:"yes"`
-	Version         flag.PositiveInteger `long:"revision" required:"true" description:"Roll back to the given app revision"`
-	relatedCommands interface{}          `related_commands:"revisions"`
-	usage           interface{}          `usage:"CF_NAME rollback APP_NAME [--revision REVISION_NUMBER] [-f]"`
+	Force           bool          `short:"f" description:"Force rollback without confirmation"`
+	RequiredArgs    flag.AppName  `positional-args:"yes"`
+	Version         flag.Revision `long:"revision" required:"true" description:"Roll back to the given app revision"`
+	relatedCommands interface{}   `related_commands:"revisions"`
+	usage           interface{}   `usage:"CF_NAME rollback APP_NAME [--revision REVISION_NUMBER] [-f]"`
 
 	LogCacheClient sharedaction.LogCacheClient
 	Stager         shared.AppStager

diff --git a/command/v7/rollback_command_test.go b/command/v7/rollback_command_test.go
@@ -12,6 +12,7 @@ import (
 	"code.cloudfoundry.org/cli/command/v7/shared/sharedfakes"
 	"code.cloudfoundry.org/cli/command/v7/v7fakes"
 	"code.cloudfoundry.org/cli/resources"
+	"code.cloudfoundry.org/cli/types"
 	"code.cloudfoundry.org/cli/util/configv3"
 	"code.cloudfoundry.org/cli/util/ui"
 	. "github.com/onsi/ginkgo"
@@ -144,7 +145,7 @@ var _ = Describe("rollback Command", func() {
 
 	When("the first revision is set as the rollback target", func() {
 		BeforeEach(func() {
-			cmd.Version = flag.PositiveInteger{Value: 1}
+			cmd.Version = flag.Revision{NullInt: types.NullInt{Value: 1, IsSet: true}}
 		})
 
 		When("the app has at least one revision", func() {