refactor: move duration parsing into internal for #1628

backend/schema/metadataretry.go

@@ -2,14 +2,14 @@ package schema
 
 import (
 	"fmt"
-	"regexp"
 	"strconv"
 	"strings"
 	"time"
 
 	"google.golang.org/protobuf/proto"
 
 	schemapb "github.com/TBD54566975/ftl/backend/protos/xyz/block/ftl/v1/schema"
+	"github.com/TBD54566975/ftl/internal/duration"
 	"github.com/TBD54566975/ftl/internal/slices"
 )
 
@@ -60,49 +60,17 @@ func (m *MetadataRetry) ToProto() proto.Message {
 }
 
 func parseRetryDuration(str string) (time.Duration, error) {
-	// regex is more lenient than what is valid to allow for better error messages.
-	re := regexp.MustCompile(`(\d+)([a-zA-Z]+)`)
-
-	var duration time.Duration
-	previousUnitDuration := time.Duration(0)
-	for len(str) > 0 {
-		matches := re.FindStringSubmatchIndex(str)
-		if matches == nil {
-			return 0, fmt.Errorf("unable to parse retry backoff %q - expected duration in format like '1m' or '30s'", str)
-		}
-		num, err := strconv.Atoi(str[matches[2]:matches[3]])
-		if err != nil {
-			return 0, fmt.Errorf("unable to parse retry backoff text %q: %w", str, err)
-		}
-
-		unitStr := str[matches[4]:matches[5]]
-		var unitDuration time.Duration
-		switch unitStr {
-		case "d":
-			unitDuration = time.Hour * 24
-		case "h":
-			unitDuration = time.Hour
-		case "m":
-			unitDuration = time.Minute
-		case "s":
-			unitDuration = time.Second
-		default:
-			return 0, fmt.Errorf("retry has unknown unit %q - use 'd', 'h', 'm' or 's'", unitStr)
-		}
-		if previousUnitDuration != 0 && previousUnitDuration <= unitDuration {
-			return 0, fmt.Errorf("retry has unit %q out of order - units need to be ordered from largest to smallest", unitStr)
-		}
-		previousUnitDuration = unitDuration
-		duration += time.Duration(num) * unitDuration
-		str = str[matches[1]:]
+	dur, err := duration.Parse(str)
+	if err != nil {
+		return 0, fmt.Errorf("could not parse retry duration: %w", err)
 	}
-	if duration < MinBackoffLimit {
+	if dur < MinBackoffLimit {
 		return 0, fmt.Errorf("retry must have a minimum backoff of %v", MinBackoffLimitStr)
 	}
-	if duration > MaxBackoffLimit {
+	if dur > MaxBackoffLimit {
 		return 0, fmt.Errorf("retry backoff can not be larger than %v", MaxBackoffLimitStr)
 	}
-	return duration, nil
+	return dur, nil
 }
 
 type RetryParams struct {

backend/schema/validate.go

@@ -573,7 +573,7 @@ func validateVerbMetadata(scopes Scopes, module *Module, n *Verb) (merr []error)
 
 			// Validate count
 			if md.Count != nil && *md.Count <= 0 {
-				merr = append(merr, errorf(md, "verb %s: retry count must be atleast 1", n.Name))
+				merr = append(merr, errorf(md, "verb %s: retry count must be at least 1", n.Name))
 			}
 
 			// Validate parsing of durations
@@ -583,7 +583,7 @@ func validateVerbMetadata(scopes Scopes, module *Module, n *Verb) (merr []error)
 				return
 			}
 			if retryParams.MaxBackoff < retryParams.MinBackoff {
-				merr = append(merr, errorf(md, "verb %s: max backoff duration (%s) needs to be atleast as long as initial backoff (%s)", n.Name, md.MaxBackoff, md.MinBackoff))
+				merr = append(merr, errorf(md, "verb %s: max backoff duration (%s) needs to be at least as long as initial backoff (%s)", n.Name, md.MaxBackoff, md.MinBackoff))
 			}
 		case *MetadataSubscriber:
 			subErrs := validateVerbSubscriptions(module, n, md, scopes, optional.None[*Schema]())

backend/schema/validate_test.go

@@ -357,14 +357,14 @@ func TestValidate(t *testing.T) {
 			errs: []string{
 				`10:7-7: verb D: retry must have a minimum backoff`,
 				`12:7-7: verb E: retry must have a minimum backoff`,
-				`14:7-7: verb F: could not parse min backoff duration: retry has unit "m" out of order - units need to be ordered from largest to smallest`,
+				`14:7-7: verb F: could not parse min backoff duration: could not parse retry duration: duration has unit "m" out of order - units need to be ordered from largest to smallest - eg '1d3h2m'`,
 				`17:7-7: verb can not have multiple instances of retry`,
-				`19:7-7: verb H: could not parse min backoff duration: retry has unknown unit "mins" - use 'd', 'h', 'm' or 's'`,
-				`21:7-7: verb I: max backoff duration (1s) needs to be atleast as long as initial backoff (1m)`,
+				`19:7-7: verb H: could not parse min backoff duration: could not parse retry duration: duration has unknown unit "mins" - use 'd', 'h', 'm' or 's', eg '1d' or '30s'`,
+				`21:7-7: verb I: max backoff duration (1s) needs to be at least as long as initial backoff (1m)`,
 				`23:7-7: verb J: could not parse min backoff duration: retry backoff can not be larger than 1d`,
-				`25:7-7: verb K: retry count must be atleast 1`,
-				`4:7-7: verb A: could not parse min backoff duration: retry has unit "m" out of order - units need to be ordered from largest to smallest`,
-				`6:7-7: verb B: could not parse min backoff duration: retry has unit "d" out of order - units need to be ordered from largest to smallest`,
+				`25:7-7: verb K: retry count must be at least 1`,
+				`4:7-7: verb A: could not parse min backoff duration: could not parse retry duration: duration has unit "m" out of order - units need to be ordered from largest to smallest - eg '1d3h2m'`,
+				`6:7-7: verb B: could not parse min backoff duration: could not parse retry duration: duration has unit "d" out of order - units need to be ordered from largest to smallest - eg '1d3h2m'`,
 				`8:7-7: verb C: could not parse min backoff duration: retry must have a minimum backoff of 1s`,
 			},
 		},

internal/duration/duration.go

@@ -0,0 +1,49 @@
+package duration
+
+import (
+	"fmt"
+	"regexp"
+	"strconv"
+	"time"
+)
+
+func Parse(str string) (time.Duration, error) {
+	// regex is more lenient than what is valid to allow for better error messages.
+	re := regexp.MustCompile(`^(\d+)([a-zA-Z]+)`)
+
+	var duration time.Duration
+	previousUnitDuration := time.Duration(0)
+	for len(str) > 0 {
+		matches := re.FindStringSubmatchIndex(str)
+		if matches == nil {
+			return 0, fmt.Errorf("unable to parse duration %q - expected duration in format like '1m' or '30s'", str)
+		}
+		num, err := strconv.Atoi(str[matches[2]:matches[3]])
+		if err != nil {
+			return 0, fmt.Errorf("unable to parse duration %q: %w", str, err)
+		}
+
+		unitStr := str[matches[4]:matches[5]]
+		var unitDuration time.Duration
+		switch unitStr {
+		case "d":
+			unitDuration = time.Hour * 24
+		case "h":
+			unitDuration = time.Hour
+		case "m":
+			unitDuration = time.Minute
+		case "s":
+			unitDuration = time.Second
+		default:
+			return 0, fmt.Errorf("duration has unknown unit %q - use 'd', 'h', 'm' or 's', eg '1d' or '30s'", unitStr)
+		}
+		if previousUnitDuration != 0 && previousUnitDuration <= unitDuration {
+			return 0, fmt.Errorf("duration has unit %q out of order - units need to be ordered from largest to smallest - eg '1d3h2m'", unitStr)
+		}
+		previousUnitDuration = unitDuration
+		duration += time.Duration(num) * unitDuration
+		str = str[matches[1]:]
+	}
+
+	return duration, nil
+}

internal/duration/duration_test.go

@@ -0,0 +1,45 @@
+package duration
+
+import (
+	"testing"
+	"time"
+
+	"github.com/alecthomas/assert/v2"
+)
+
+func TestGoodDurations(t *testing.T) {
+	tests := []struct {
+		input    string
+		expected time.Duration
+	}{
+		{"", 0},
+		{"1d", time.Hour * 24},
+		{"1h", time.Hour},
+		{"1m", time.Minute},
+		{"1s", time.Second},
+		{"1d1h1m1s", time.Hour*24 + time.Hour + time.Minute + time.Second},
+	}
+	for _, test := range tests {
+		duration, err := Parse(test.input)
+		assert.NoError(t, err)
+		assert.Equal(t, test.expected, duration)
+	}
+}
+
+func TestBadDurations(t *testing.T) {
+	tests := []struct {
+		input string
+	}{
+		{"1"},
+		{"1x"},
+		{"1d1d"},
+		{"1d1h1m1s1"},
+		{"1s5d"},
+		{"-1s"},
+	}
+	for _, test := range tests {
+		duration, err := Parse(test.input)
+		assert.Error(t, err, test.input)
+		assert.Zero(t, duration)
+	}
+}