T2.2: Migrate scheduler to use domain.Schedule
Parse each job's schedule once on load (resetNextRuns) and on edit (RefreshSchedule) via the new parseJobSchedule helper, caching the result in a map[int]domain.Schedule keyed by job ID. prepareNextRun now looks up the cached Schedule instead of re-parsing the string on every call. Remove the nextRunTime wrapper that did the per-call parsing. Drop the three scheduler_test.go tests that duplicated coverage already in domain/schedule_test.go. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
+1
-1
@@ -255,7 +255,7 @@ Track progress here. Mark tasks complete as they land and pass review.
|
|||||||
|
|
||||||
### Phase 2 — Domain cleanup
|
### Phase 2 — Domain cleanup
|
||||||
- [x] T2.1 — Add `src/domain/schedule.go`; Schedule value object
|
- [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.3 — Split `domain.Job` (durable) from `domain.JobRuntime` (transient)
|
||||||
- [ ] T2.4 — Update `storage`: load/save Job only; move runtime init
|
- [ ] T2.4 — Update `storage`: load/save Job only; move runtime init
|
||||||
|
|
||||||
|
|||||||
+27
-22
@@ -21,20 +21,22 @@ type Scheduler struct {
|
|||||||
jobs *[]domain.Job
|
jobs *[]domain.Job
|
||||||
onChange func(domain.RunRecord)
|
onChange func(domain.RunRecord)
|
||||||
|
|
||||||
mu sync.Mutex
|
mu sync.Mutex
|
||||||
ctx context.Context
|
ctx context.Context
|
||||||
cancel context.CancelFunc
|
cancel context.CancelFunc
|
||||||
paused bool
|
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 {
|
func NewScheduler(store *storage.Store, jobs *[]domain.Job, onChange func(domain.RunRecord)) *Scheduler {
|
||||||
ctx, cancel := context.WithCancel(context.Background())
|
ctx, cancel := context.WithCancel(context.Background())
|
||||||
s := &Scheduler{
|
s := &Scheduler{
|
||||||
store: store,
|
store: store,
|
||||||
jobs: jobs,
|
jobs: jobs,
|
||||||
onChange: onChange,
|
onChange: onChange,
|
||||||
ctx: ctx,
|
ctx: ctx,
|
||||||
cancel: cancel,
|
cancel: cancel,
|
||||||
|
schedules: make(map[int]domain.Schedule),
|
||||||
}
|
}
|
||||||
s.resetNextRuns(time.Now())
|
s.resetNextRuns(time.Now())
|
||||||
return s
|
return s
|
||||||
@@ -107,6 +109,7 @@ func (s *Scheduler) RefreshSchedule(index int) {
|
|||||||
return
|
return
|
||||||
}
|
}
|
||||||
job := &(*s.jobs)[index]
|
job := &(*s.jobs)[index]
|
||||||
|
s.parseJobSchedule(job) // re-parse in case the schedule string changed
|
||||||
if !job.Enabled {
|
if !job.Enabled {
|
||||||
job.NextRun = "Paused"
|
job.NextRun = "Paused"
|
||||||
return
|
return
|
||||||
@@ -207,6 +210,7 @@ func runningOutput(job domain.Job, trigger string, started time.Time) string {
|
|||||||
func (s *Scheduler) resetNextRuns(now time.Time) {
|
func (s *Scheduler) resetNextRuns(now time.Time) {
|
||||||
for index := range *s.jobs {
|
for index := range *s.jobs {
|
||||||
job := &(*s.jobs)[index]
|
job := &(*s.jobs)[index]
|
||||||
|
s.parseJobSchedule(job) // parse once on load
|
||||||
if !job.Enabled {
|
if !job.Enabled {
|
||||||
job.NextRun = "Paused"
|
job.NextRun = "Paused"
|
||||||
continue
|
continue
|
||||||
@@ -216,24 +220,25 @@ func (s *Scheduler) resetNextRuns(now time.Time) {
|
|||||||
_ = s.store.SaveJobs(*s.jobs)
|
_ = 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) {
|
func (s *Scheduler) prepareNextRun(job *domain.Job, from time.Time) {
|
||||||
next, ok := nextRunTime(job.Schedule, from)
|
sched, ok := s.schedules[job.ID]
|
||||||
if !ok {
|
if !ok {
|
||||||
job.NextRun = "Invalid schedule"
|
job.NextRun = "Invalid schedule"
|
||||||
job.NextDue = time.Time{}
|
job.NextDue = time.Time{}
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
job.NextDue = next
|
job.NextDue = sched.Next(from)
|
||||||
job.NextRun = job.NextDue.Format("2006-01-02 15:04:05")
|
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
|
|
||||||
}
|
|
||||||
|
|||||||
@@ -8,32 +8,10 @@ import (
|
|||||||
"gitea.mixdep.ru/mix/gosentry/src/domain"
|
"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) {
|
func TestPrepareNextRunSetsDisplayString(t *testing.T) {
|
||||||
jobs := []domain.Job{{Schedule: "*/5 * * * *", Enabled: true}}
|
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)
|
from := time.Date(2026, 6, 14, 12, 3, 0, 0, time.UTC)
|
||||||
|
|
||||||
s.prepareNextRun(&jobs[0], from)
|
s.prepareNextRun(&jobs[0], from)
|
||||||
@@ -50,7 +28,9 @@ func TestPrepareNextRunSetsDisplayString(t *testing.T) {
|
|||||||
|
|
||||||
func TestPrepareNextRunSetsInvalidScheduleLabel(t *testing.T) {
|
func TestPrepareNextRunSetsInvalidScheduleLabel(t *testing.T) {
|
||||||
jobs := []domain.Job{{Schedule: "not-a-cron", Enabled: true}}
|
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())
|
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) {
|
func TestRunningOutputIncludesInvocation(t *testing.T) {
|
||||||
started := time.Date(2026, 6, 17, 23, 40, 0, 0, time.Local)
|
started := time.Date(2026, 6, 17, 23, 40, 0, 0, time.Local)
|
||||||
job := domain.Job{
|
job := domain.Job{
|
||||||
|
|||||||
Reference in New Issue
Block a user