Use one duration parser and fix the --older-than months example (closes #123)
check / check (pull_request) Failing after 0s
check / check (pull_request) Failing after 0s
Two functions named parseDuration existed with different grammars. --older-than and --keep-newer-than both already parsed through the one in internal/vaultik; the richer copy in internal/cli/duration.go was reachable only from its own test. Kept the live-path parser and deleted the unused one, so no flag's accepted grammar changes. README documented 6m as the months example for --older-than, but m is minutes, so that command deleted every snapshot older than six minutes; corrected it to 6mo and put both flags' help on one example list stating m is minutes and mo is months. The parser now rejects negatives it used to accept (-5h) or silently made positive (-5d). Model: opus-4-8
This commit is contained in:
@@ -1,126 +0,0 @@
|
||||
package cli
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"fmt"
|
||||
"regexp"
|
||||
"strconv"
|
||||
"strings"
|
||||
"time"
|
||||
)
|
||||
|
||||
// Approximate lengths of the extended calendar units accepted by
|
||||
// parseDuration.
|
||||
const (
|
||||
durationDay = 24 * time.Hour
|
||||
durationWeek = 7 * durationDay
|
||||
durationMonth = 30 * durationDay
|
||||
durationYear = 365 * durationDay
|
||||
)
|
||||
|
||||
var (
|
||||
errNegativeDuration = errors.New("negative durations are not supported")
|
||||
errInvalidDuration = errors.New("invalid duration format")
|
||||
errUnknownTimeUnit = errors.New("unknown time unit")
|
||||
)
|
||||
|
||||
// parseDuration parses duration strings. Supports standard Go duration format
|
||||
// (e.g., "3h30m", "1h45m30s") as well as extended units:
|
||||
// - d: days (e.g., "30d", "7d")
|
||||
// - w: weeks (e.g., "2w", "4w")
|
||||
// - mo: months (30 days) (e.g., "6mo", "1mo")
|
||||
// - y: years (365 days) (e.g., "1y", "2y")
|
||||
//
|
||||
// Can combine units: "1y6mo", "2w3d", "1d12h30m"
|
||||
func parseDuration(s string) (time.Duration, error) {
|
||||
// First try standard Go duration parsing
|
||||
d, err := time.ParseDuration(s)
|
||||
if err == nil {
|
||||
return d, nil
|
||||
}
|
||||
|
||||
// Extended duration parsing
|
||||
// Check for negative values
|
||||
if strings.HasPrefix(strings.TrimSpace(s), "-") {
|
||||
return 0, errNegativeDuration
|
||||
}
|
||||
|
||||
// Pattern matches: number + unit, repeated
|
||||
re := regexp.MustCompile(`(\d+(?:\.\d+)?)\s*([a-zA-Z]+)`)
|
||||
matches := re.FindAllStringSubmatch(s, -1)
|
||||
|
||||
if len(matches) == 0 {
|
||||
return 0, fmt.Errorf("%w: %q", errInvalidDuration, s)
|
||||
}
|
||||
|
||||
var total time.Duration
|
||||
|
||||
for _, match := range matches {
|
||||
valueStr := match[1]
|
||||
unit := strings.ToLower(match[2])
|
||||
|
||||
value, err := strconv.ParseFloat(valueStr, 64)
|
||||
if err != nil {
|
||||
return 0, fmt.Errorf("invalid number %q: %w", valueStr, err)
|
||||
}
|
||||
|
||||
d, err := durationForUnit(value, unit)
|
||||
if err != nil {
|
||||
return 0, err
|
||||
}
|
||||
|
||||
total += d
|
||||
}
|
||||
|
||||
return total, nil
|
||||
}
|
||||
|
||||
// durationForUnit converts a value with a (case-normalized) unit suffix
|
||||
// into a time.Duration, accepting Go's standard units plus the extended
|
||||
// calendar units.
|
||||
func durationForUnit(value float64, unit string) (time.Duration, error) {
|
||||
switch unit {
|
||||
// Standard time units
|
||||
case "ns", "nanosecond", "nanoseconds":
|
||||
return time.Duration(value), nil
|
||||
case "us", "µs", "microsecond", "microseconds":
|
||||
return time.Duration(value * float64(time.Microsecond)), nil
|
||||
case "ms", "millisecond", "milliseconds":
|
||||
return time.Duration(value * float64(time.Millisecond)), nil
|
||||
case "s", "sec", "second", "seconds":
|
||||
return time.Duration(value * float64(time.Second)), nil
|
||||
case "m", "min", "minute", "minutes":
|
||||
return time.Duration(value * float64(time.Minute)), nil
|
||||
case "h", "hr", "hour", "hours":
|
||||
return time.Duration(value * float64(time.Hour)), nil
|
||||
// Extended units
|
||||
case "d", "day", "days":
|
||||
return time.Duration(value * float64(durationDay)), nil
|
||||
case "w", "week", "weeks":
|
||||
return time.Duration(value * float64(durationWeek)), nil
|
||||
case "mo", "month", "months":
|
||||
// Using 30 days as approximation
|
||||
return time.Duration(value * float64(durationMonth)), nil
|
||||
case "y", "year", "years":
|
||||
// Using 365 days as approximation
|
||||
return time.Duration(value * float64(durationYear)), nil
|
||||
default:
|
||||
// Try parsing as standard Go duration unit
|
||||
testStr := "1" + unit
|
||||
|
||||
_, err := time.ParseDuration(testStr)
|
||||
if err != nil {
|
||||
return 0, fmt.Errorf("%w: %q", errUnknownTimeUnit, unit)
|
||||
}
|
||||
|
||||
// It's a valid Go duration unit, parse the full value
|
||||
fullStr := fmt.Sprintf("%g%s", value, unit)
|
||||
|
||||
d, err := time.ParseDuration(fullStr)
|
||||
if err != nil {
|
||||
return 0, fmt.Errorf("invalid duration %q: %w", fullStr, err)
|
||||
}
|
||||
|
||||
return d, nil
|
||||
}
|
||||
}
|
||||
@@ -1,299 +0,0 @@
|
||||
package cli //nolint:testpackage // needs access to unexported parseDuration
|
||||
|
||||
import (
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
type parseDurationCase struct {
|
||||
name string
|
||||
input string
|
||||
expected time.Duration
|
||||
wantErr bool
|
||||
}
|
||||
|
||||
// runParseDurationCases executes a table of parseDuration cases as
|
||||
// parallel subtests.
|
||||
func runParseDurationCases(t *testing.T, tests []parseDurationCase) {
|
||||
t.Helper()
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
got, err := parseDuration(tt.input)
|
||||
|
||||
if tt.wantErr {
|
||||
require.Error(t, err, "expected error for input %q", tt.input)
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
require.NoError(t, err, "unexpected error for input %q", tt.input)
|
||||
assert.Equal(t, tt.expected, got, "duration mismatch for input %q", tt.input)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestParseDurationStandard(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
runParseDurationCases(t, []parseDurationCase{
|
||||
{
|
||||
name: "standard seconds",
|
||||
input: "30s",
|
||||
expected: 30 * time.Second,
|
||||
},
|
||||
{
|
||||
name: "standard minutes",
|
||||
input: "45m",
|
||||
expected: 45 * time.Minute,
|
||||
},
|
||||
{
|
||||
name: "standard hours",
|
||||
input: "2h",
|
||||
expected: 2 * time.Hour,
|
||||
},
|
||||
{
|
||||
name: "standard combined",
|
||||
input: "3h30m",
|
||||
expected: 3*time.Hour + 30*time.Minute,
|
||||
},
|
||||
{
|
||||
name: "standard complex",
|
||||
input: "1h45m30s",
|
||||
expected: 1*time.Hour + 45*time.Minute + 30*time.Second,
|
||||
},
|
||||
{
|
||||
name: "standard with milliseconds",
|
||||
input: "1s500ms",
|
||||
expected: 1*time.Second + 500*time.Millisecond,
|
||||
},
|
||||
})
|
||||
}
|
||||
|
||||
func TestParseDurationExtendedUnits(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
runParseDurationCases(t, []parseDurationCase{
|
||||
// Extended units - days
|
||||
{
|
||||
name: "single day",
|
||||
input: "1d",
|
||||
expected: 24 * time.Hour,
|
||||
},
|
||||
{
|
||||
name: "multiple days",
|
||||
input: "7d",
|
||||
expected: 7 * 24 * time.Hour,
|
||||
},
|
||||
{
|
||||
name: "fractional days",
|
||||
input: "1.5d",
|
||||
expected: 36 * time.Hour,
|
||||
},
|
||||
{
|
||||
name: "days spelled out",
|
||||
input: "3days",
|
||||
expected: 3 * 24 * time.Hour,
|
||||
},
|
||||
// Extended units - weeks
|
||||
{
|
||||
name: "single week",
|
||||
input: "1w",
|
||||
expected: 7 * 24 * time.Hour,
|
||||
},
|
||||
{
|
||||
name: "multiple weeks",
|
||||
input: "4w",
|
||||
expected: 4 * 7 * 24 * time.Hour,
|
||||
},
|
||||
{
|
||||
name: "weeks spelled out",
|
||||
input: "2weeks",
|
||||
expected: 2 * 7 * 24 * time.Hour,
|
||||
},
|
||||
// Extended units - months
|
||||
{
|
||||
name: "single month",
|
||||
input: "1mo",
|
||||
expected: 30 * 24 * time.Hour,
|
||||
},
|
||||
{
|
||||
name: "multiple months",
|
||||
input: "6mo",
|
||||
expected: 6 * 30 * 24 * time.Hour,
|
||||
},
|
||||
{
|
||||
name: "months spelled out",
|
||||
input: "3months",
|
||||
expected: 3 * 30 * 24 * time.Hour,
|
||||
},
|
||||
// Extended units - years
|
||||
{
|
||||
name: "single year",
|
||||
input: "1y",
|
||||
expected: 365 * 24 * time.Hour,
|
||||
},
|
||||
{
|
||||
name: "multiple years",
|
||||
input: "2y",
|
||||
expected: 2 * 365 * 24 * time.Hour,
|
||||
},
|
||||
{
|
||||
name: "years spelled out",
|
||||
input: "1year",
|
||||
expected: 365 * 24 * time.Hour,
|
||||
},
|
||||
})
|
||||
}
|
||||
|
||||
func TestParseDurationCombinedAndErrors(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
runParseDurationCases(t, []parseDurationCase{
|
||||
// Combined extended units
|
||||
{
|
||||
name: "weeks and days",
|
||||
input: "2w3d",
|
||||
expected: 2*7*24*time.Hour + 3*24*time.Hour,
|
||||
},
|
||||
{
|
||||
name: "years and months",
|
||||
input: "1y6mo",
|
||||
expected: 365*24*time.Hour + 6*30*24*time.Hour,
|
||||
},
|
||||
{
|
||||
name: "days and hours",
|
||||
input: "1d12h",
|
||||
expected: 24*time.Hour + 12*time.Hour,
|
||||
},
|
||||
{
|
||||
name: "complex combination",
|
||||
input: "1y2mo3w4d5h6m7s",
|
||||
expected: 365*24*time.Hour + 2*30*24*time.Hour +
|
||||
3*7*24*time.Hour + 4*24*time.Hour +
|
||||
5*time.Hour + 6*time.Minute + 7*time.Second,
|
||||
},
|
||||
{
|
||||
name: "with spaces",
|
||||
input: "1d 12h 30m",
|
||||
expected: 24*time.Hour + 12*time.Hour + 30*time.Minute,
|
||||
},
|
||||
// Edge cases
|
||||
{
|
||||
name: "zero duration",
|
||||
input: "0s",
|
||||
expected: 0,
|
||||
},
|
||||
{
|
||||
name: "large duration",
|
||||
input: "10y",
|
||||
expected: 10 * 365 * 24 * time.Hour,
|
||||
},
|
||||
// Error cases
|
||||
{
|
||||
name: "empty string",
|
||||
input: "",
|
||||
wantErr: true,
|
||||
},
|
||||
{
|
||||
name: "invalid format",
|
||||
input: "abc",
|
||||
wantErr: true,
|
||||
},
|
||||
{
|
||||
name: "unknown unit",
|
||||
input: "5x",
|
||||
wantErr: true,
|
||||
},
|
||||
{
|
||||
name: "invalid number",
|
||||
input: "xyzd",
|
||||
wantErr: true,
|
||||
},
|
||||
{
|
||||
name: "negative not supported",
|
||||
input: "-5d",
|
||||
wantErr: true,
|
||||
},
|
||||
})
|
||||
}
|
||||
|
||||
func TestParseDurationSpecialCases(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
// Test that standard Go durations work exactly as expected
|
||||
standardDurations := []string{
|
||||
"300ms",
|
||||
"1.5h",
|
||||
"2h45m",
|
||||
"72h",
|
||||
"1us",
|
||||
"1µs",
|
||||
"1ns",
|
||||
}
|
||||
|
||||
for _, d := range standardDurations {
|
||||
expected, err := time.ParseDuration(d)
|
||||
require.NoError(t, err)
|
||||
|
||||
got, err := parseDuration(d)
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, expected, got, "standard duration %q should parse identically", d)
|
||||
}
|
||||
}
|
||||
|
||||
func TestParseDurationRealWorldExamples(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
// Test real-world snapshot purge scenarios
|
||||
tests := []struct {
|
||||
description string
|
||||
input string
|
||||
olderThan time.Duration
|
||||
}{
|
||||
{
|
||||
description: "keep snapshots from last 30 days",
|
||||
input: "30d",
|
||||
olderThan: 30 * 24 * time.Hour,
|
||||
},
|
||||
{
|
||||
description: "keep snapshots from last 6 months",
|
||||
input: "6mo",
|
||||
olderThan: 6 * 30 * 24 * time.Hour,
|
||||
},
|
||||
{
|
||||
description: "keep snapshots from last year",
|
||||
input: "1y",
|
||||
olderThan: 365 * 24 * time.Hour,
|
||||
},
|
||||
{
|
||||
description: "keep snapshots from last week and a half",
|
||||
input: "1w3d",
|
||||
olderThan: 10 * 24 * time.Hour,
|
||||
},
|
||||
{
|
||||
description: "keep snapshots from last 90 days",
|
||||
input: "90d",
|
||||
olderThan: 90 * 24 * time.Hour,
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.description, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
got, err := parseDuration(tt.input)
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, tt.olderThan, got)
|
||||
|
||||
// Verify the duration makes sense for snapshot purging
|
||||
assert.Greater(t, got, time.Hour,
|
||||
"snapshot purge duration should be at least an hour")
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -141,7 +141,8 @@ specifying a path using --config or by setting VAULTIK_CONFIG to a path.`,
|
||||
"orphaned blobs")
|
||||
cmd.Flags().StringVar(&opts.KeepNewerThan, "keep-newer-than", "",
|
||||
"With --prune: keep snapshots newer than this duration "+
|
||||
"(e.g. 4w, 30d, 6mo) instead of only the latest")
|
||||
"(e.g. 30d, 4w, 6mo, 1y; m is minutes, mo is months) "+
|
||||
"instead of only the latest")
|
||||
|
||||
return cmd
|
||||
}
|
||||
@@ -204,7 +205,8 @@ restrict the operation to specific snapshot names.`,
|
||||
cmd.Flags().BoolVar(&opts.KeepLatest, "keep-latest", false,
|
||||
"Keep only the latest snapshot of each name")
|
||||
cmd.Flags().StringVar(&opts.OlderThan, "older-than", "",
|
||||
"Remove snapshots older than duration (e.g., 30d, 6m, 1y)")
|
||||
"Remove snapshots older than duration "+
|
||||
"(e.g. 30d, 4w, 6mo, 1y; m is minutes, mo is months)")
|
||||
cmd.Flags().BoolVar(&opts.Force, "force", false, "Skip confirmation prompt")
|
||||
cmd.Flags().StringArrayVar(&opts.Names, "snapshot", nil,
|
||||
"Restrict to snapshots with these names (repeat for multiple)")
|
||||
|
||||
Reference in New Issue
Block a user