diff --git a/docs/REFACTORING.md b/docs/REFACTORING.md index 083e3ad..243d1da 100644 --- a/docs/REFACTORING.md +++ b/docs/REFACTORING.md @@ -255,7 +255,7 @@ Track progress here. Mark tasks complete as they land and pass review. ### Phase 2 — Domain cleanup - [x] T2.1 — Add `src/domain/schedule.go`; Schedule value object -- [ ] T2.2 — Migrate `scheduler` to use Schedule +- [x] T2.2 — Migrate `scheduler` to use Schedule - [ ] T2.3 — Split `domain.Job` (durable) from `domain.JobRuntime` (transient) - [ ] T2.4 — Update `storage`: load/save Job only; move runtime init diff --git a/src/scheduler/scheduler.go b/src/scheduler/scheduler.go index 5de2c30..f192fc8 100644 --- a/src/scheduler/scheduler.go +++ b/src/scheduler/scheduler.go @@ -21,20 +21,22 @@ type Scheduler struct { jobs *[]domain.Job onChange func(domain.RunRecord) - mu sync.Mutex - ctx context.Context - cancel context.CancelFunc - paused bool + mu sync.Mutex + ctx context.Context + cancel context.CancelFunc + paused bool + schedules map[int]domain.Schedule // parsed once per job on load/edit } func NewScheduler(store *storage.Store, jobs *[]domain.Job, onChange func(domain.RunRecord)) *Scheduler { ctx, cancel := context.WithCancel(context.Background()) s := &Scheduler{ - store: store, - jobs: jobs, - onChange: onChange, - ctx: ctx, - cancel: cancel, + store: store, + jobs: jobs, + onChange: onChange, + ctx: ctx, + cancel: cancel, + schedules: make(map[int]domain.Schedule), } s.resetNextRuns(time.Now()) return s @@ -107,6 +109,7 @@ func (s *Scheduler) RefreshSchedule(index int) { return } job := &(*s.jobs)[index] + s.parseJobSchedule(job) // re-parse in case the schedule string changed if !job.Enabled { job.NextRun = "Paused" return @@ -207,6 +210,7 @@ func runningOutput(job domain.Job, trigger string, started time.Time) string { func (s *Scheduler) resetNextRuns(now time.Time) { for index := range *s.jobs { job := &(*s.jobs)[index] + s.parseJobSchedule(job) // parse once on load if !job.Enabled { job.NextRun = "Paused" continue @@ -216,24 +220,25 @@ func (s *Scheduler) resetNextRuns(now time.Time) { _ = s.store.SaveJobs(*s.jobs) } +// parseJobSchedule caches a parsed domain.Schedule for the job. Invalid +// schedule strings are silently dropped from the cache so prepareNextRun can +// distinguish them from valid ones. +func (s *Scheduler) parseJobSchedule(job *domain.Job) { + sched, err := domain.Parse(job.Schedule) + if err != nil { + delete(s.schedules, job.ID) + return + } + s.schedules[job.ID] = sched +} + func (s *Scheduler) prepareNextRun(job *domain.Job, from time.Time) { - next, ok := nextRunTime(job.Schedule, from) + sched, ok := s.schedules[job.ID] if !ok { job.NextRun = "Invalid schedule" job.NextDue = time.Time{} return } - job.NextDue = next + job.NextDue = sched.Next(from) job.NextRun = job.NextDue.Format("2006-01-02 15:04:05") } - -// nextRunTime is a thin wrapper over domain.Schedule kept for the scheduler's -// existing call sites. It parses the schedule on every call for now; T2.2 -// replaces this with a Schedule parsed once on load/edit. -func nextRunTime(schedule string, from time.Time) (time.Time, bool) { - parsed, err := domain.Parse(schedule) - if err != nil { - return time.Time{}, false - } - return parsed.Next(from), true -} diff --git a/src/scheduler/scheduler_test.go b/src/scheduler/scheduler_test.go index a10fe6c..6daec2d 100644 --- a/src/scheduler/scheduler_test.go +++ b/src/scheduler/scheduler_test.go @@ -8,32 +8,10 @@ import ( "gitea.mixdep.ru/mix/gosentry/src/domain" ) -func TestNextRunTimeRejectsInvalidSchedules(t *testing.T) { - from := time.Date(2026, 6, 14, 12, 0, 0, 0, time.UTC) - cases := []struct { - schedule string - desc string - }{ - {"", "empty string"}, - {" ", "whitespace only"}, - {"@every", "bare @every without duration"}, - {"@every xyz", "invalid @every duration string"}, - {"@every -1s", "negative @every duration"}, - {"@every 0s", "zero @every duration"}, - {"not-a-cron", "invalid cron expression"}, - {"60 * * * *", "cron minute out of range"}, - } - for _, tc := range cases { - _, ok := nextRunTime(tc.schedule, from) - if ok { - t.Errorf("nextRunTime(%q) [%s]: expected false, got true", tc.schedule, tc.desc) - } - } -} - func TestPrepareNextRunSetsDisplayString(t *testing.T) { jobs := []domain.Job{{Schedule: "*/5 * * * *", Enabled: true}} - s := &Scheduler{jobs: &jobs} + s := &Scheduler{jobs: &jobs, schedules: make(map[int]domain.Schedule)} + s.parseJobSchedule(&jobs[0]) from := time.Date(2026, 6, 14, 12, 3, 0, 0, time.UTC) s.prepareNextRun(&jobs[0], from) @@ -50,7 +28,9 @@ func TestPrepareNextRunSetsDisplayString(t *testing.T) { func TestPrepareNextRunSetsInvalidScheduleLabel(t *testing.T) { jobs := []domain.Job{{Schedule: "not-a-cron", Enabled: true}} - s := &Scheduler{jobs: &jobs} + s := &Scheduler{jobs: &jobs, schedules: make(map[int]domain.Schedule)} + // parseJobSchedule will drop the invalid spec, so schedules map stays empty. + s.parseJobSchedule(&jobs[0]) s.prepareNextRun(&jobs[0], time.Now()) @@ -62,29 +42,6 @@ func TestPrepareNextRunSetsInvalidScheduleLabel(t *testing.T) { } } -func TestNextRunTimeSupportsEvery(t *testing.T) { - from := time.Date(2026, 6, 14, 12, 0, 0, 0, time.UTC) - next, ok := nextRunTime("@every 10s", from) - if !ok { - t.Fatal("expected @every schedule to parse") - } - if want := from.Add(10 * time.Second); !next.Equal(want) { - t.Fatalf("expected %s, got %s", want, next) - } -} - -func TestNextRunTimeSupportsCron(t *testing.T) { - from := time.Date(2026, 6, 14, 12, 3, 0, 0, time.UTC) - next, ok := nextRunTime("*/5 * * * *", from) - if !ok { - t.Fatal("expected cron schedule to parse") - } - want := time.Date(2026, 6, 14, 12, 5, 0, 0, time.UTC) - if !next.Equal(want) { - t.Fatalf("expected %s, got %s", want, next) - } -} - func TestRunningOutputIncludesInvocation(t *testing.T) { started := time.Date(2026, 6, 17, 23, 40, 0, 0, time.Local) job := domain.Job{