T2.1: Add domain.Schedule value object

Introduce src/domain/schedule.go with a Schedule value object that centralizes schedule parsing and validation: Parse, Validate, and Next(time.Time). It owns the cron parser and @every handling, moved out of the scheduler. The scheduler's nextRunTime is kept as a thin wrapper delegating to domain.Parse for now (T2.2 will parse once on load/edit). Add unit tests covering invalid specs, @every intervals, five-field cron, cron descriptors, whitespace trimming, the zero-value Next, and String. Mark T2.1 complete in REFACTORING.md. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-18 22:55:20 +03:00
parent 9938164c7a
commit 0f17782174
4 changed files with 195 additions and 21 deletions
@@ -254,7 +254,7 @@ Track progress here. Mark tasks complete as they land and pass review.
 - [x] T1.8 — Delete empty `src/core`; build + test both platforms

 ### Phase 2 — Domain cleanup
- [ ] 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
 - [ ] T2.3 — Split `domain.Job` (durable) from `domain.JobRuntime` (transient)
 - [ ] T2.4 — Update `storage`: load/save Job only; move runtime init
@@ -0,0 +1,87 @@
+package domain
+
+import (
+	"fmt"
+	"strings"
+	"time"
+
+	"github.com/robfig/cron/v3"
+)
+
+// cronParser accepts standard five-field cron expressions (minute, hour, day of
+// month, month, day of week) plus descriptors such as "@daily". It is the single
+// source of truth for what GoSentry considers a valid cron schedule.
+var cronParser = cron.NewParser(cron.Minute | cron.Hour | cron.Dom | cron.Month | cron.Dow | cron.Descriptor)
+
+// everyPrefix marks the "@every <duration>" form, which is kept alongside cron
+// because it is convenient for quick tests and for simple intervals that are
+// awkward to express as five fields.
+const everyPrefix = "@every "
+
+// Schedule is a parsed, validated job schedule. It supports two forms:
+//
+//   - "@every <duration>" intervals (e.g. "@every 10s"), and
+//   - standard five-field cron expressions (e.g. "*/5 * * * *").
+//
+// Parsing once and reusing the value avoids re-validating and re-parsing the
+// same string on every scheduler tick. A zero Schedule is invalid; its Next
+// method returns the zero time.
+type Schedule struct {
+	raw   string
+	every time.Duration // > 0 when the schedule is an "@every" interval
+	cron  cron.Schedule  // non-nil when the schedule is a cron expression
+}
+
+// Parse validates spec and returns a reusable Schedule. It returns an error
+// describing why the schedule is unusable, which callers can surface to the user.
+func Parse(spec string) (Schedule, error) {
+	trimmed := strings.TrimSpace(spec)
+	if trimmed == "" {
+		return Schedule{}, fmt.Errorf("schedule is empty")
+	}
+	if strings.HasPrefix(trimmed, everyPrefix) {
+		interval, err := time.ParseDuration(strings.TrimSpace(strings.TrimPrefix(trimmed, everyPrefix)))
+		if err != nil {
+			return Schedule{}, fmt.Errorf("invalid %q duration: %w", strings.TrimSpace(everyPrefix), err)
+		}
+		if interval <= 0 {
+			return Schedule{}, fmt.Errorf("%q duration must be positive, got %s", strings.TrimSpace(everyPrefix), interval)
+		}
+		return Schedule{raw: trimmed, every: interval}, nil
+	}
+	// robfig/cron handles edge cases such as ranges, steps, and day-of-week names,
+	// keeping GoSentry compatible with the mental model users know from Unix cron.
+	parsed, err := cronParser.Parse(trimmed)
+	if err != nil {
+		return Schedule{}, fmt.Errorf("invalid cron expression: %w", err)
+	}
+	return Schedule{raw: trimmed, cron: parsed}, nil
+}
+
+// Validate reports whether spec is a usable schedule string. It is a convenience
+// wrapper around Parse for callers (such as form validation) that only need the
+// yes/no answer and the error message.
+func Validate(spec string) error {
+	_, err := Parse(spec)
+	return err
+}
+
+// Next returns the next time the schedule fires strictly after from. For an
+// "@every" interval this is from plus the interval; for a cron expression it is
+// the cron library's next matching time. A zero (unparsed) Schedule returns the
+// zero time.
+func (s Schedule) Next(from time.Time) time.Time {
+	switch {
+	case s.every > 0:
+		return from.Add(s.every)
+	case s.cron != nil:
+		return s.cron.Next(from)
+	default:
+		return time.Time{}
+	}
+}
+
+// String returns the original, trimmed schedule specification.
+func (s Schedule) String() string {
+	return s.raw
+}
@@ -0,0 +1,103 @@
+package domain
+
+import (
+	"testing"
+	"time"
+)
+
+func TestParseRejectsInvalidSchedules(t *testing.T) {
+	cases := []struct {
+		spec string
+		desc string
+	}{
+		{"", "empty string"},
+		{"   ", "whitespace only"},
+		{"@every", "bare @every without duration"},
+		{"@every ", "@every with trailing space but no 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"},
+		{"* * * *", "too few cron fields"},
+	}
+	for _, tc := range cases {
+		if _, err := Parse(tc.spec); err == nil {
+			t.Errorf("Parse(%q) [%s]: expected error, got nil", tc.spec, tc.desc)
+		}
+		if err := Validate(tc.spec); err == nil {
+			t.Errorf("Validate(%q) [%s]: expected error, got nil", tc.spec, tc.desc)
+		}
+	}
+}
+
+func TestParseEveryInterval(t *testing.T) {
+	from := time.Date(2026, 6, 14, 12, 0, 0, 0, time.UTC)
+	s, err := Parse("@every 10s")
+	if err != nil {
+		t.Fatalf("Parse(@every 10s): unexpected error: %v", err)
+	}
+	if got, want := s.Next(from), from.Add(10*time.Second); !got.Equal(want) {
+		t.Fatalf("Next: got %s, want %s", got, want)
+	}
+}
+
+func TestParseEveryTrimsSurroundingWhitespace(t *testing.T) {
+	from := time.Date(2026, 6, 14, 12, 0, 0, 0, time.UTC)
+	s, err := Parse("  @every 90m  ")
+	if err != nil {
+		t.Fatalf("Parse: unexpected error: %v", err)
+	}
+	if got, want := s.Next(from), from.Add(90*time.Minute); !got.Equal(want) {
+		t.Fatalf("Next: got %s, want %s", got, want)
+	}
+}
+
+func TestParseCronExpression(t *testing.T) {
+	from := time.Date(2026, 6, 14, 12, 3, 0, 0, time.UTC)
+	s, err := Parse("*/5 * * * *")
+	if err != nil {
+		t.Fatalf("Parse(*/5 * * * *): unexpected error: %v", err)
+	}
+	want := time.Date(2026, 6, 14, 12, 5, 0, 0, time.UTC)
+	if got := s.Next(from); !got.Equal(want) {
+		t.Fatalf("Next: got %s, want %s", got, want)
+	}
+}
+
+func TestParseCronDescriptor(t *testing.T) {
+	from := time.Date(2026, 6, 14, 12, 3, 0, 0, time.UTC)
+	s, err := Parse("@daily")
+	if err != nil {
+		t.Fatalf("Parse(@daily): unexpected error: %v", err)
+	}
+	want := time.Date(2026, 6, 15, 0, 0, 0, 0, time.UTC)
+	if got := s.Next(from); !got.Equal(want) {
+		t.Fatalf("Next: got %s, want %s", got, want)
+	}
+}
+
+func TestValidateAcceptsValidSchedules(t *testing.T) {
+	for _, spec := range []string{"@every 1s", "*/5 * * * *", "0 9 * * 1", "@hourly"} {
+		if err := Validate(spec); err != nil {
+			t.Errorf("Validate(%q): unexpected error: %v", spec, err)
+		}
+	}
+}
+
+func TestZeroScheduleNextIsZero(t *testing.T) {
+	var s Schedule
+	if got := s.Next(time.Now()); !got.IsZero() {
+		t.Fatalf("zero Schedule Next: got %s, want zero time", got)
+	}
+}
+
+func TestStringReturnsTrimmedSpec(t *testing.T) {
+	s, err := Parse("  */5 * * * *  ")
+	if err != nil {
+		t.Fatalf("Parse: unexpected error: %v", err)
+	}
+	if got, want := s.String(), "*/5 * * * *"; got != want {
+		t.Fatalf("String: got %q, want %q", got, want)
+	}
+}
@@ -10,11 +10,8 @@ import (
 	"gitea.mixdep.ru/mix/gosentry/src/domain"
 	"gitea.mixdep.ru/mix/gosentry/src/storage"
 	"gitea.mixdep.ru/mix/gosentry/src/runner"
-	"github.com/robfig/cron/v3"
 )

-var cronParser = cron.NewParser(cron.Minute | cron.Hour | cron.Dom | cron.Month | cron.Dow | cron.Descriptor)
-
 // Scheduler owns the timing loop for jobs that are currently loaded in the GUI.
 // It receives a pointer to the jobs slice because the GUI edits the same slice;
 // this keeps the early architecture simple while storage and scheduling are
@@ -230,24 +227,11 @@ func (s *Scheduler) prepareNextRun(job *domain.Job, from time.Time) {
 	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) {
-	schedule = strings.TrimSpace(schedule)
-	if schedule == "" {
-		return time.Time{}, false
-	}
-	if strings.HasPrefix(schedule, "@every ") {
-		// @every is kept alongside cron because it is convenient for quick tests
-		// and for simple intervals that are awkward to express as five fields.
-		interval, err := time.ParseDuration(strings.TrimSpace(strings.TrimPrefix(schedule, "@every ")))
-		if err != nil || interval <= 0 {
-			return time.Time{}, false
-		}
-		return from.Add(interval), true
-	}
-	// Standard five-field cron keeps GoSentry compatible with the mental model
-	// users already know from Unix cron, while robfig/cron handles edge cases
-	// such as ranges, steps, and day-of-week names.
-	parsed, err := cronParser.Parse(schedule)
+	parsed, err := domain.Parse(schedule)
 	if err != nil {
 		return time.Time{}, false
 	}