amadan
schedule: fold Validate+Spec into one parse in UpsertSchedule
UpsertSchedule called sc.Validate() (which itself parses every/cron via s.Spec() to check Next isn't zero) and then called sc.Spec() again to get the Spec for normalisation — two parses of the same text per call. Add a private validate() that does the check-and-parse once and returns the Spec; Validate() now just discards it, keeping its public signature and non-mutating, error-only contract unchanged. UpsertSchedule calls validate() directly instead of Validate()+Spec().
1 file changed,
+16
−9
internal/schedule/declaration.go+16 −9
diff --git a/internal/schedule/declaration.go b/internal/schedule/declaration.go| index fcd24423c..e4432f2be 100644 |
| --- a/internal/schedule/declaration.go |
| +++ b/internal/schedule/declaration.go |
| @@ -95,23 +95,33 @@ func (s Schedule) Spec() (Spec, error) { |
| // ParseSpec reads back) is UpsertSchedule's job, done on its own addressable |
| // copy just before storing. |
| func (s Schedule) Validate() error { |
| + _, err := s.validate() |
| + return err |
| +} |
| + |
| +// validate is Validate's work, plus the parsed Spec — UpsertSchedule needs |
| +// that Spec to normalise every/cron text, and calling Validate then Spec |
| +// separately parsed the every/cron string twice for one call. Kept private |
| +// so Validate's public signature (and its non-mutating, error-only |
| +// contract) does not change. |
| +func (s Schedule) validate() (Spec, error) { |
| if !ValidName(s.Name) { |
| - return fmt.Errorf("name %q: want [a-z0-9][a-z0-9-]{0,31}", s.Name) |
| + return Spec{}, fmt.Errorf("name %q: want [a-z0-9][a-z0-9-]{0,31}", s.Name) |
| } |
| if err := ValidPath(s.Path); err != nil { |
| - return err |
| + return Spec{}, err |
| } |
| sp, err := s.Spec() |
| if err != nil { |
| - return err |
| + return Spec{}, err |
| } |
| // A cron expression can parse and still match nothing ("0 0 30 2 *"): |
| // its Next is the zero time, which a box must never store as a due |
| // instant. Refuse it here, where the member can read the answer. |
| if sp.Next(time.Now()).IsZero() { |
| - return errors.New("this expression never fires (no matching minute within the next 5 years)") |
| + return Spec{}, errors.New("this expression never fires (no matching minute within the next 5 years)") |
| } |
| - return nil |
| + return sp, nil |
| } |
| // Declaration is apps/<acct>/<app>/schedules.json (spec §4.1). |
| @@ -202,10 +212,7 @@ func modify(ctx context.Context, s release.Store, acct, app string, now time.Tim |
| // UpsertSchedule validates sc, normalises its every/cron text to what |
| // ParseSpec reads back, then replaces-or-appends it by name. |
| func UpsertSchedule(ctx context.Context, s release.Store, acct, app string, sc Schedule, now time.Time) (Declaration, error) { |
| - if err := sc.Validate(); err != nil { |
| - return Declaration{}, err |
| - } |
| - sp, err := sc.Spec() // Validate already confirmed this succeeds |
| + sp, err := sc.validate() // one parse of every/cron, not Validate then Spec |
| if err != nil { |
| return Declaration{}, err |
| } |