i/prompting: add doc string and adjusted error messages and comments

Signed-off-by: Oliver Calder <oliver.calder@canonical.com>

interfaces/prompting/prompting.go

@@ -25,12 +25,17 @@ import (
 	"time"
 )
 
+// OutcomeType describes the outcome associated with a reply or rule.
 type OutcomeType string
 
 const (
+	// OutcomeUnset indicates that no outcome was specified, and should only
+	// be used while unmarshalling outcome fields marked as omitempty.
 	OutcomeUnset OutcomeType = ""
+	// OutcomeAllow indicates that a corresponding request should be allowed.
 	OutcomeAllow OutcomeType = "allow"
-	OutcomeDeny  OutcomeType = "deny"
+	// OutcomeDeny indicates that a corresponding request should be denied.
+	OutcomeDeny OutcomeType = "deny"
 )
 
 func (outcome *OutcomeType) UnmarshalJSON(data []byte) error {
@@ -43,7 +48,7 @@ func (outcome *OutcomeType) UnmarshalJSON(data []byte) error {
 	case OutcomeAllow, OutcomeDeny:
 		*outcome = value
 	default:
-		return fmt.Errorf(`outcome must be %q or %q: %q`, OutcomeAllow, OutcomeDeny, value)
+		return fmt.Errorf(`cannot have outcome other than %q or %q: %q`, OutcomeAllow, OutcomeDeny, value)
 	}
 	return nil
 }
@@ -57,16 +62,24 @@ func (outcome OutcomeType) IsAllow() (bool, error) {
 	case OutcomeDeny:
 		return false, nil
 	default:
-		return false, fmt.Errorf(`outcome must be %q or %q: %q`, OutcomeAllow, OutcomeDeny, outcome)
+		return false, fmt.Errorf(`internal error: invalid outcome: %q`, outcome)
 	}
 }
 
+// LifespanType describes the temporal scope for which a reply or rule applies.
 type LifespanType string
 
 const (
-	LifespanUnset    LifespanType = ""
-	LifespanForever  LifespanType = "forever"
-	LifespanSingle   LifespanType = "single"
+	// LifespanUnset indicates that no lifespan was specified, and should only
+	// be used while unmarshalling lifespan fields marked as omitempty.
+	LifespanUnset LifespanType = ""
+	// LifespanForever indicates that the reply/rule should never expire.
+	LifespanForever LifespanType = "forever"
+	// LifespanSingle indicates that a reply should only apply once, and should
+	// not be used to create a rule.
+	LifespanSingle LifespanType = "single"
+	// LifespanTimespan indicates that a reply/rule should apply for a given
+	// duration or until a given expiration timestamp.
 	LifespanTimespan LifespanType = "timespan"
 	// TODO: add LifespanSession which expires after the user logs out
 	// LifespanSession  LifespanType = "session"
@@ -82,33 +95,33 @@ func (lifespan *LifespanType) UnmarshalJSON(data []byte) error {
 	case LifespanForever, LifespanSingle, LifespanTimespan:
 		*lifespan = value
 	default:
-		return fmt.Errorf(`lifespan must be %q, %q, or %q`, LifespanForever, LifespanSingle, LifespanTimespan)
+		return fmt.Errorf(`cannot have lifespan other than %q, %q, or %q: %q`, LifespanForever, LifespanSingle, LifespanTimespan, value)
 	}
 	return nil
 }
 
 // ValidateExpiration checks that the given expiration is valid for the
 // receiver lifespan.
 //
-// If the lifespan is LifespanTimespan LifespanTimespan, then expiration must
-// be non-zero and be after the given currTime. Otherwise, it must be zero.
-// Returns an error if any of the above are invalid.
+// If the lifespan is LifespanTimespan, then expiration must be non-zero and be
+// after the given currTime. Otherwise, it must be zero. Returns an error if
+// any of the above are invalid.
 func (lifespan LifespanType) ValidateExpiration(expiration time.Time, currTime time.Time) error {
 	switch lifespan {
 	case LifespanForever, LifespanSingle:
 		if !expiration.IsZero() {
-			return fmt.Errorf(`expiration must be omitted when lifespan is %q, but received non-zero expiration: %q`, lifespan, expiration)
+			return fmt.Errorf(`cannot have specified expiration when lifespan is %q: %q`, lifespan, expiration)
 		}
 	case LifespanTimespan:
 		if expiration.IsZero() {
-			return fmt.Errorf(`expiration must be non-zero when lifespan is %q, but received empty expiration`, lifespan)
+			return fmt.Errorf(`cannot have unspecified expiration when lifespan is %q`, lifespan)
 		}
 		if currTime.After(expiration) {
-			return fmt.Errorf("expiration time has already passed: %q", expiration)
+			return fmt.Errorf("cannot have expiration time in the past: %q", expiration)
 		}
 	default:
 		// Should not occur, since lifespan is validated when unmarshalled
-		return fmt.Errorf(`lifespan must be %q, %q, or %q`, LifespanForever, LifespanSingle, LifespanTimespan)
+		return fmt.Errorf(`internal error: invalid lifespan: %q`, lifespan)
 	}
 	return nil
 }
@@ -126,23 +139,23 @@ func (lifespan LifespanType) ParseDuration(duration string, currTime time.Time)
 	switch lifespan {
 	case LifespanForever, LifespanSingle:
 		if duration != "" {
-			return expiration, fmt.Errorf(`duration must be empty when lifespan is %q, but received non-empty duration: %q`, lifespan, duration)
+			return expiration, fmt.Errorf(`cannot have specified duration when lifespan is %q: %q`, lifespan, duration)
 		}
 	case LifespanTimespan:
 		if duration == "" {
-			return expiration, fmt.Errorf(`duration must be non-empty when lifespan is %q, but received empty expiration`, lifespan)
+			return expiration, fmt.Errorf(`cannot have unspecified duration when lifespan is %q`, lifespan)
 		}
 		parsedDuration, err := time.ParseDuration(duration)
 		if err != nil {
-			return expiration, fmt.Errorf(`error parsing duration string: %q`, duration)
+			return expiration, fmt.Errorf(`cannot parse duration: %w`, err)
 		}
 		if parsedDuration <= 0 {
-			return expiration, fmt.Errorf(`duration must be greater than zero: %q`, duration)
+			return expiration, fmt.Errorf(`cannot have zero or negative duration: %q`, duration)
 		}
 		expiration = currTime.Add(parsedDuration)
 	default:
 		// Should not occur, since lifespan is validated when unmarshalled
-		return expiration, fmt.Errorf(`lifespan must be %q, %q, or %q`, LifespanForever, LifespanSingle, LifespanTimespan)
+		return expiration, fmt.Errorf(`internal error: invalid lifespan: %q`, lifespan)
 	}
 	return expiration, nil
 }

interfaces/prompting/prompting_test.go

@@ -52,9 +52,9 @@ func (s *promptingSuite) TestOutcomeIsAllow(c *C) {
 	c.Check(err, IsNil)
 	c.Check(result, Equals, false)
 	_, err = prompting.OutcomeUnset.IsAllow()
-	c.Check(err, ErrorMatches, `outcome must be.*`)
+	c.Check(err, ErrorMatches, `internal error: invalid outcome.*`)
 	_, err = prompting.OutcomeType("foo").IsAllow()
-	c.Check(err, ErrorMatches, `outcome must be.*`)
+	c.Check(err, ErrorMatches, `internal error: invalid outcome.*`)
 }
 
 type fakeOutcomeWrapper struct {
@@ -91,12 +91,12 @@ func (s *promptingSuite) TestUnmarshalOutcomeUnhappy(c *C) {
 		var fow1 fakeOutcomeWrapper
 		data := []byte(fmt.Sprintf(`{"field1": "%s", "field2": "%s"}`, outcome, outcome))
 		err := json.Unmarshal(data, &fow1)
-		c.Check(err, ErrorMatches, "outcome must be .*", Commentf("data: %v", string(data)))
+		c.Check(err, ErrorMatches, `cannot have outcome other than.*`, Commentf("data: %v", string(data)))
 
 		var fow2 fakeOutcomeWrapper
 		data = []byte(fmt.Sprintf(`{"field1": "%s", "field2": "%s"}`, prompting.OutcomeAllow, outcome))
 		err = json.Unmarshal(data, &fow2)
-		c.Check(err, ErrorMatches, "outcome must be .*", Commentf("data: %v", string(data)))
+		c.Check(err, ErrorMatches, `cannot have outcome other than.*`, Commentf("data: %v", string(data)))
 	}
 }
 
@@ -135,12 +135,12 @@ func (s *promptingSuite) TestUnmarshalLifespanUnhappy(c *C) {
 		var flw1 fakeLifespanWrapper
 		data := []byte(fmt.Sprintf(`{"field1": "%s", "field2": "%s"}`, lifespan, lifespan))
 		err := json.Unmarshal(data, &flw1)
-		c.Check(err, ErrorMatches, "lifespan must be .*", Commentf("data: %v", string(data)))
+		c.Check(err, ErrorMatches, `cannot have lifespan other than.*`, Commentf("data: %v", string(data)))
 
 		var flw2 fakeLifespanWrapper
 		data = []byte(fmt.Sprintf(`{"field1": "%s", "field2": "%s"}`, prompting.LifespanForever, lifespan))
 		err = json.Unmarshal(data, &flw2)
-		c.Check(err, ErrorMatches, "lifespan must be .*", Commentf("data: %v", string(data)))
+		c.Check(err, ErrorMatches, `cannot have lifespan other than.*`, Commentf("data: %v", string(data)))
 	}
 }
 
@@ -158,15 +158,15 @@ func (s *promptingSuite) TestValidateExpiration(c *C) {
 		c.Check(err, IsNil)
 		for _, exp := range []time.Time{negativeExpiration, validExpiration} {
 			err = lifespan.ValidateExpiration(exp, currTime)
-			c.Check(err, ErrorMatches, `expiration must be omitted.*`)
+			c.Check(err, ErrorMatches, `cannot have specified expiration when lifespan is.*`)
 		}
 	}
 
 	err := prompting.LifespanTimespan.ValidateExpiration(unsetExpiration, currTime)
-	c.Check(err, ErrorMatches, `expiration must be non-zero.*`)
+	c.Check(err, ErrorMatches, `cannot have unspecified expiration when lifespan is.*`)
 
 	err = prompting.LifespanTimespan.ValidateExpiration(negativeExpiration, currTime)
-	c.Check(err, ErrorMatches, `expiration time has already passed.*`)
+	c.Check(err, ErrorMatches, `cannot have expiration time in the past.*`)
 
 	err = prompting.LifespanTimespan.ValidateExpiration(validExpiration, currTime)
 	c.Check(err, IsNil)
@@ -191,21 +191,21 @@ func (s *promptingSuite) TestParseDuration(c *C) {
 		for _, dur := range []string{invalidDuration, negativeDuration, validDuration} {
 			expiration, err = lifespan.ParseDuration(dur, currTime)
 			c.Check(expiration.IsZero(), Equals, true)
-			c.Check(err, ErrorMatches, `duration must be empty.*`)
+			c.Check(err, ErrorMatches, `cannot have specified duration when lifespan is.*`)
 		}
 	}
 
 	expiration, err := prompting.LifespanTimespan.ParseDuration(unsetDuration, currTime)
 	c.Check(expiration.IsZero(), Equals, true)
-	c.Check(err, ErrorMatches, `duration must be non-empty.*`)
+	c.Check(err, ErrorMatches, `cannot have unspecified duration when lifespan is.*`)
 
 	expiration, err = prompting.LifespanTimespan.ParseDuration(invalidDuration, currTime)
 	c.Check(expiration.IsZero(), Equals, true)
-	c.Check(err, ErrorMatches, `error parsing duration string.*`)
+	c.Check(err, ErrorMatches, `cannot parse duration.*`)
 
 	expiration, err = prompting.LifespanTimespan.ParseDuration(negativeDuration, currTime)
 	c.Check(expiration.IsZero(), Equals, true)
-	c.Check(err, ErrorMatches, `duration must be greater than zero.*`)
+	c.Check(err, ErrorMatches, `cannot have zero or negative duration.*`)
 
 	expiration, err = prompting.LifespanTimespan.ParseDuration(validDuration, currTime)
 	c.Check(err, IsNil)