diff --git a/README.md b/README.md index 728651b..cc29bed 100644 --- a/README.md +++ b/README.md @@ -63,7 +63,7 @@ portable application: moving the program folder also moves its configuration. ```json { - "jobs_dir": ".", + "jobs_file": "jobs.json", "logs_dir": "logs", "max_log_files": 100, "max_log_age_days": 30, @@ -91,10 +91,15 @@ portable application: moving the program folder also moves its configuration. } ``` -`jobs_dir` is the directory GoSentry reads `jobs.json` from. The default `"."` -means the same folder as the executable. An absolute path can be used when jobs +`jobs_file` is the file GoSentry reads job definitions from, file name included, +so the file can be named anything. The default `"jobs.json"` is relative and +resolves to the executable's folder. An absolute path can be used when jobs should live elsewhere, such as a shared network drive. +A `gosentry.json` from an earlier version that carries `jobs_dir` instead keeps +working: the directory is combined with `jobs.json` on load, and the file is +rewritten with `jobs_file`. + `logs_dir` is relative to the program folder when it does not start with a drive letter or `/`. @@ -132,9 +137,20 @@ Standard 5-field cron expressions: 5. Use **Pause** on a single job to suspend it without deleting it. 6. Use **Pause all** as a global stop switch for all scheduled runs. 7. Open **History** to see past runs, their trigger (`Manual`, `Schedule`, or `UI`), state, and log file. -8. Open **Settings** to change storage directories, log cleanup limits, queue behavior, and notifications. +8. Open **Settings** to change the storage paths, log cleanup limits, queue behavior, and notifications. -Changing `jobs_dir` in Settings saves the current job list to the new directory. +The **Jobs file** row picks the file itself: **Browse** lists `.json` files, and +a path can also be typed to name a file that does not exist yet. What Save does +depends on whether that file is already there: + +- **The file exists** — its jobs are loaded and replace the current list, so + selecting a jobs file switches to it (another machine's file, a shared one on + a network drive). History records how many jobs were loaded and from where. +- **The file does not exist** — the current jobs are written to it, which is how + the jobs file is renamed or moved somewhere else. + +Switching to a different jobs file is refused while a job is running, because +loading a new list discards the run state of the old one. The **Start on login** checkbox shows an `OK` or `Problem` status. Saving with it enabled writes an autostart entry using the current executable path. diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 195867d..1062a2d 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -51,7 +51,7 @@ flowchart LR runner -->|"execute command"| shell runner -->|"write stdout/stderr log"| logs runner -->|"RunRecord"| svc - svc -->|"emit JobChanged / RunRecorded / ErrorOccurred"| ui + svc -->|"emit JobChanged / RunRecorded / JobsLoaded / ErrorOccurred"| ui ui -->|"display jobs, history, status"| user ui -->|"SetAutostart, AutostartStatus"| autostart @@ -75,6 +75,14 @@ flowchart LR `Event`. The UI's observer receives the event and refreshes the relevant widget on the main thread via `fyne.Do`. + `UpdateSettings` has one extra step: when the configured jobs file changes + and a file already exists at the new path, that file is authoritative. The + Service loads it, calls `adoptJobsLocked` to rebuild the jobs slice, runtime + map, schedule cache, next-run times, and log-seeded statistics around it, and + emits `JobsLoaded` plus a broad `JobChanged`. A path with no file behind it + receives the current jobs instead. Adoption drops all runtime state, so it is + refused while a job is running. + 3. Scheduled run: `scheduler.Scheduler` fires a tick every second. On each tick it calls `Service.RunDue(now)`. The Service checks which enabled, non-paused jobs are diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 807f048..dad3b91 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -2,6 +2,48 @@ All notable GoSentry changes are recorded in this file. +## 0.15.0 - 2026-07-26 + +**Settings points at the jobs file itself, not the folder holding it.** + +**Settings:** + +- The **Jobs directory** row is now a **Jobs file** row. Browse opens a file + picker filtered to `.json` instead of a folder picker, so the job list can + live under any file name — `team-jobs.json`, one file per machine, a file + shared over a network drive — rather than a fixed `jobs.json` per folder. The + field stays editable, which is how a file that does not exist yet is named. +- **Selecting an existing jobs file now loads it.** Previously the current job + list was written over whatever was at the new path, which made it impossible + to switch to an existing jobs file — its contents were destroyed on Save. Now + an existing file wins: its jobs are loaded, normalized, and replace the loaded + list, with runtimes, parsed schedules, next-run times, and log-seeded + statistics rebuilt around them. A path with no file behind it still receives + the current jobs (and its folder is created), which is how the jobs file is + renamed or relocated. History records `Jobs loaded — N jobs from `, + since the switch happens without a prompt. +- Switching to a different jobs file is refused while a job is running: adoption + discards every runtime, and a run finishing afterwards would write its result + onto whichever job inherited its ID. Settings unrelated to the jobs file still + save normally during a run. +- Saving a path with no file name (a trailing separator, `.`, `..`) is rejected + with "jobs file must include a file name" instead of failing later with an + opaque OS error. + +**Configuration:** + +- `Config.JobsDir` / `jobs_dir` is replaced by `Config.JobsFile` / `jobs_file`, + which holds the full path including the file name; the default is + `"jobs.json"`, resolved against the program folder as before. `Paths.JobsDir` + is now derived from the configured file so job saves still create the folder. +- A `gosentry.json` written by an earlier version is migrated on load: its + `jobs_dir` is joined with `jobs.json`, which is the exact file that version + used, and the retired key is dropped when the config is rewritten. +- New `app.JobsLoaded{Path, Count}` event, emitted when a selected jobs file + replaces the job list; the UI turns it into the History entry. New + `storage.LoadJobsFile`, which reads and normalizes a jobs file and reports a + missing one as "not found" instead of seeding it the way startup does. + ## 0.14.0 - 2026-07-26 **Compact job list view, "no timeout" at both timeout levels, and an Open diff --git a/docs/STANDARDS.md b/docs/STANDARDS.md index b9d17f3..84fa3ee 100644 --- a/docs/STANDARDS.md +++ b/docs/STANDARDS.md @@ -29,12 +29,24 @@ change to their shape has to stay compatible on its own. one helper that every consumer shares (`JobListView.IsCompact`, `ui.themeFor`), and is normalized before being written back, so the file never gains a value no reader understands. +- A renamed key keeps the old field on `Config` (tagged `omitempty`) purely so + it can still be read. `storage.loadOrCreateConfig` converts it to the new + field and clears it, so the retired key disappears on the next save. See + `Config.JobsDir` → `Config.JobsFile`. Where the new field has a non-empty + default, clear that default before unmarshalling, or "the file omits it" and + "the file sets it" become indistinguishable and the conversion never runs. - Each of the three gets a test: the default in `storage`, the normalization in `domain`, and a round-trip through the real config file in `app`. ## Intentional behavior (not bugs) - `RunNow` is allowed during global pause and for disabled jobs. +- Selecting a jobs file that already exists **loads** it: its jobs replace the + in-memory list, which is the only way the user can switch between job lists. A + path with no file behind it receives the current jobs (rename/relocate). The + switch is refused while a job is running, because adoption drops every runtime + and a finishing run would then write its result onto whichever job inherited + its ID. - Sequential mode runs jobs FIFO by order in `jobs.json`. - Scheduler tick is 1s — sub-second `@every` intervals are not supported. - Command timeout defaults to no timeout globally (`Config.DefaultTimeoutSeconds` diff --git a/docs/TESTS.md b/docs/TESTS.md index 9524c4f..f88e887 100644 --- a/docs/TESTS.md +++ b/docs/TESTS.md @@ -130,6 +130,11 @@ Tests all mutating operations on the Service, scheduler integration, and setting |------|---------| | `TestUpdateSettingsPersistsAndValidates` | Verifies that `UpdateSettings` persists a valid config and rewrites autostart if needed. | | `TestUpdateSettingsRejectsInvalidConfigs` | Verifies that `UpdateSettings` returns validation errors without persisting. | +| `TestHasFileName` | Verifies the jobs-file path check: a file name passes; a trailing separator, `.`, and `..` do not. | +| `TestUpdateSettingsWritesJobsToTheNewFile` | Verifies that changing `JobsFile` re-resolves `Paths.JobsPath` and writes the loaded jobs to the new file, creating its folder. | +| `TestUpdateSettingsAdoptsExistingJobsFile` | Verifies that selecting a jobs file that already exists replaces the job list with its contents, rebuilds runtimes, and emits `JobsLoaded`. | +| `TestUpdateSettingsKeepsJobsWhenTheNewFileIsMissing` | Verifies that a path with no file behind it receives the current jobs instead (the rename/relocate case). | +| `TestUpdateSettingsRefusesJobsFileSwitchWhileRunning` | Verifies that switching the jobs file is refused (and not persisted) while a job runs, while unrelated settings still save. | | `TestPrependLogCapsActivityList` | Verifies that the activity log never grows beyond its maximum cap. | --- @@ -204,6 +209,9 @@ Tests JSON round-tripping and default generation. | `TestConfigRoundTrip` | Verifies that settings saved to JSON are reloaded with identical field values. | | `TestNormalizeJobsFillsDefaults` | Verifies that `normalizeJobs` assigns sequential IDs and sets default name, schedule, and command for jobs missing those fields. | | `TestLoadOrCreateConfigCreatesDefaultsOnFirstRun` | Verifies that a missing config file is created with sane defaults and a sample job. | +| `TestLoadOrCreateConfigMigratesJobsDir` | Verifies that a pre-0.15 `jobs_dir` becomes `jobs_file` pointing at the same `jobs.json`, and that the retired key is not written back. | +| `TestLoadJobsFileReportsMissingWithoutCreating` | Verifies that `LoadJobsFile` reports a missing file as not-found without creating or seeding it, and normalizes the jobs it does load. | +| `TestApplyConfigPathsDerivesJobsDir` | Verifies that the configured jobs file resolves against the program folder and that `Paths.JobsDir` is derived from it. | | `TestJobsJSONDoesNotPersistRuntimeNoise` | Verifies that `jobs.json` does not persist runtime state (LastRun, NextRun, etc.). Only durable job fields are stored. | --- diff --git a/src/app/events.go b/src/app/events.go index 5fe3ac9..d21ab88 100644 --- a/src/app/events.go +++ b/src/app/events.go @@ -41,6 +41,17 @@ type SchedulerStateChanged struct { Paused bool } +// JobsLoaded signals that the whole job list was replaced by the contents of a +// jobs file the user selected in Settings. It carries the path and job count +// because that is what the user needs to see confirmed — the switch happens +// without a prompt, and the previous list is no longer on screen to compare +// against. Observers that render jobs should re-read them through the Service; +// a broad JobChanged is emitted alongside for exactly that. +type JobsLoaded struct { + Path string + Count int +} + // ErrorOccurred signals a background error that could not be returned to a // caller — typically a failed save or cleanup after an async run. The UI // surfaces it in the History tab so the user is not silently left with @@ -50,6 +61,7 @@ type ErrorOccurred struct { } func (JobChanged) isEvent() {} +func (JobsLoaded) isEvent() {} func (RunRecorded) isEvent() {} func (SchedulerStateChanged) isEvent() {} func (ErrorOccurred) isEvent() {} diff --git a/src/app/operations.go b/src/app/operations.go index 6e0eb56..928ca19 100644 --- a/src/app/operations.go +++ b/src/app/operations.go @@ -3,11 +3,13 @@ package app import ( "errors" "fmt" + "path/filepath" "strings" "time" "gitea.mixdep.ru/mix/gosentry/src/domain" "gitea.mixdep.ru/mix/gosentry/src/runner" + "gitea.mixdep.ru/mix/gosentry/src/storage" ) // maxJobLogs bounds the in-memory activity list kept per job. The full history @@ -224,30 +226,71 @@ func (s *Service) ShouldNotifyOnFailure() bool { } // UpdateSettings validates and persists a new application configuration. The -// loaded jobs are re-saved because the jobs directory may have changed, and log +// loaded jobs are re-saved because the jobs file may have changed, and log // cleanup runs so a tightened retention policy takes effect immediately. +// +// Pointing the config at a different jobs file that already exists adopts that +// file: its jobs replace the loaded ones, which is the only way the user can +// switch between job lists. A path with no file there yet receives the current +// jobs instead, which is how the jobs file is renamed or relocated. Adoption +// discards all runtime state, so it is refused while a job is running. func (s *Service) UpdateSettings(config domain.Config) error { if err := validateConfig(config); err != nil { return err } + // The path is stored exactly as it is resolved, so a hand-typed value with + // stray spaces cannot make the saved setting and the file in use disagree. + config.JobsFile = strings.TrimSpace(config.JobsFile) s.mu.Lock() + jobsPath := storage.ResolveConfiguredPath(s.store.Paths.AppDir, config.JobsFile) + switching := jobsPath != s.store.Paths.JobsPath + if switching && s.anyRunningLocked() { + s.mu.Unlock() + return errors.New("cannot change the jobs file while a job is running") + } + // Read the new file before anything is written, so a file that cannot be + // parsed leaves both the config and the current jobs untouched. + var adopted []domain.Job + if switching { + jobs, found, err := storage.LoadJobsFile(jobsPath) + if err != nil { + s.mu.Unlock() + return fmt.Errorf("read jobs file %s: %w", jobsPath, err) + } + if found { + adopted = jobs + } + } + s.store.Config = config if err := s.store.SaveConfig(); err != nil { s.mu.Unlock() return err } + if adopted != nil { + s.adoptJobsLocked(adopted) + } // SaveConfig re-resolved the paths from the new config, so SaveJobs writes to - // the (possibly new) jobs directory and cleanup targets the new logs dir. + // the (possibly new) jobs file and cleanup targets the new logs dir. Adopted + // jobs are written back too, which persists the IDs and defaults that + // normalization filled in, exactly as loading them at startup would. if err := s.store.SaveJobs(s.jobs); err != nil { s.mu.Unlock() return err } + loaded := len(s.jobs) logsDir := s.store.Paths.LogsDir maxFiles := s.store.Config.MaxLogFiles maxAge := s.store.Config.MaxLogAgeDays s.mu.Unlock() + if adopted != nil { + // A broad JobChanged redraws the job list; JobsLoaded tells the user in + // History which file those jobs came from, since nothing was asked. + s.emit(JobsLoaded{Path: jobsPath, Count: loaded}) + s.emit(JobChanged{}) + } return runner.CleanupLogs(logsDir, maxFiles, maxAge) } @@ -394,10 +437,31 @@ func validateJob(job domain.Job) error { return nil } +// hasFileName reports whether a path ends in something that can be a file name. +// It is a syntax check only — an existing directory whose name looks like a file +// name still passes, and fails at write time — but it catches the shapes a user +// types when they mean a folder: a trailing separator, "." and "..". +func hasFileName(path string) bool { + if strings.HasSuffix(path, "/") || strings.HasSuffix(path, string(filepath.Separator)) { + return false + } + switch filepath.Base(path) { + case ".", "..", string(filepath.Separator): + return false + } + return true +} + // validateConfig rejects settings that would break persistence or cleanup. func validateConfig(config domain.Config) error { - if strings.TrimSpace(config.JobsDir) == "" { - return errors.New("jobs directory is required") + jobsFile := strings.TrimSpace(config.JobsFile) + if jobsFile == "" { + return errors.New("jobs file is required") + } + // A path that names only a folder would be written to as if it were a file + // and fail later with an opaque OS error, so require a file name here. + if !hasFileName(jobsFile) { + return errors.New("jobs file must include a file name") } if strings.TrimSpace(config.LogsDir) == "" { return errors.New("logs directory is required") diff --git a/src/app/operations_test.go b/src/app/operations_test.go index d45d1b4..07b0862 100644 --- a/src/app/operations_test.go +++ b/src/app/operations_test.go @@ -27,7 +27,7 @@ func newTempService(t *testing.T, jobs []domain.Job) *Service { JobsPath: filepath.Join(dir, "jobs.json"), LogsDir: filepath.Join(dir, "logs"), }, - Config: domain.Config{JobsDir: ".", LogsDir: "logs", MaxLogFiles: 100, MaxLogAgeDays: 30, ExecutionMode: domain.ExecutionModeParallel, OverlapPolicy: domain.OverlapPolicySkip, DefaultTimeoutSeconds: 30}, + Config: domain.Config{JobsFile: "jobs.json", LogsDir: "logs", MaxLogFiles: 100, MaxLogAgeDays: 30, ExecutionMode: domain.ExecutionModeParallel, OverlapPolicy: domain.OverlapPolicySkip, DefaultTimeoutSeconds: 30}, } return NewService(store, jobs) } @@ -527,7 +527,8 @@ func TestUpdateSettingsRejectsInvalidConfigs(t *testing.T) { name string mutate func(c *domain.Config) }{ - {"missing jobs dir", func(c *domain.Config) { c.JobsDir = " " }}, + {"missing jobs file", func(c *domain.Config) { c.JobsFile = " " }}, + {"jobs file without a file name", func(c *domain.Config) { c.JobsFile = "jobs" + string(filepath.Separator) }}, {"missing logs dir", func(c *domain.Config) { c.LogsDir = "" }}, {"non-positive max files", func(c *domain.Config) { c.MaxLogFiles = 0 }}, {"non-positive max age", func(c *domain.Config) { c.MaxLogAgeDays = -1 }}, @@ -544,6 +545,169 @@ func TestUpdateSettingsRejectsInvalidConfigs(t *testing.T) { } } +func TestHasFileName(t *testing.T) { + tests := []struct { + path string + want bool + }{ + {"jobs.json", true}, + {filepath.Join("data", "team.json"), true}, + {"jobs" + string(filepath.Separator), false}, + {"data/", false}, + {".", false}, + {"..", false}, + {string(filepath.Separator), false}, + } + for _, tc := range tests { + if got := hasFileName(tc.path); got != tc.want { + t.Errorf("hasFileName(%q) = %v, want %v", tc.path, got, tc.want) + } + } +} + +// Renaming or relocating the jobs file writes the loaded jobs to the new path, +// which is what makes the Settings change take effect without a restart. +func TestUpdateSettingsWritesJobsToTheNewFile(t *testing.T) { + svc := newTempService(t, []domain.Job{{ID: 1, Name: "Kept", Schedule: "@every 1m", Command: "echo hi", Enabled: true}}) + + config := svc.store.Config + config.JobsFile = filepath.Join("data", "team-jobs.json") + if err := svc.UpdateSettings(config); err != nil { + t.Fatalf("UpdateSettings: %v", err) + } + + moved := filepath.Join(svc.store.Paths.AppDir, "data", "team-jobs.json") + if svc.store.Paths.JobsPath != moved { + t.Errorf("JobsPath: got %q, want %q", svc.store.Paths.JobsPath, moved) + } + data, err := os.ReadFile(moved) + if err != nil { + t.Fatalf("read moved jobs file: %v", err) + } + var file domain.JobsFile + if err := json.Unmarshal(data, &file); err != nil { + t.Fatalf("unmarshal moved jobs file: %v", err) + } + if len(file.Jobs) != 1 || file.Jobs[0].Name != "Kept" { + t.Errorf("moved jobs file: got %+v, want the single 'Kept' job", file.Jobs) + } +} + +// Pointing Settings at a jobs file that already exists must adopt that file: +// its jobs replace the loaded ones instead of being overwritten by them. This is +// the only way the user can switch between job lists, so the file's contents +// win, the job list is rebuilt around them, and History is told where they came +// from. +func TestUpdateSettingsAdoptsExistingJobsFile(t *testing.T) { + svc := newTempService(t, []domain.Job{{ID: 1, Name: "Local", Schedule: "@every 1m", Command: "echo local", Enabled: true}}) + rec := &recorder{} + svc.Subscribe(rec) + + shared := filepath.Join(svc.store.Paths.AppDir, "shared.json") + existing := domain.JobsFile{Jobs: []domain.Job{ + {ID: 4, Name: "Adopted", Schedule: "@every 5m", Command: "echo adopted", Enabled: true}, + {Name: "Needs an ID", Schedule: "@every 9m", Command: "echo second", Enabled: false}, + }} + data, err := json.Marshal(existing) + if err != nil { + t.Fatal(err) + } + if err := os.WriteFile(shared, data, 0o644); err != nil { + t.Fatal(err) + } + + config := svc.store.Config + config.JobsFile = shared + if err := svc.UpdateSettings(config); err != nil { + t.Fatalf("UpdateSettings: %v", err) + } + + jobs := svc.Jobs() + if len(jobs) != 2 || jobs[0].Name != "Adopted" { + t.Fatalf("jobs after adoption: got %+v, want the two jobs from the selected file", jobs) + } + // The adopted jobs must be fully live, not just listed: runtime and parsed + // schedule are rebuilt for the IDs the file brought (including the one + // normalization had to assign). + for _, job := range jobs { + if svc.Runtime(job.ID) == nil { + t.Errorf("job %d (%q) has no runtime after adoption", job.ID, job.Name) + } + } + if svc.Runtime(1) != nil { + t.Error("runtime of the replaced job should be gone") + } + + var loaded []JobsLoaded + for _, e := range rec.events { + if jl, ok := e.(JobsLoaded); ok { + loaded = append(loaded, jl) + } + } + if len(loaded) != 1 || loaded[0].Path != shared || loaded[0].Count != 2 { + t.Errorf("JobsLoaded events: got %+v, want one for %q with 2 jobs", loaded, shared) + } +} + +// A path with no file behind it is the "rename or relocate" case: the current +// jobs are written there rather than an empty list being adopted. +func TestUpdateSettingsKeepsJobsWhenTheNewFileIsMissing(t *testing.T) { + svc := newTempService(t, []domain.Job{{ID: 1, Name: "Local", Schedule: "@every 1m", Command: "echo local", Enabled: true}}) + + config := svc.store.Config + config.JobsFile = filepath.Join("moved", "jobs.json") + if err := svc.UpdateSettings(config); err != nil { + t.Fatalf("UpdateSettings: %v", err) + } + + jobs := svc.Jobs() + if len(jobs) != 1 || jobs[0].Name != "Local" { + t.Fatalf("jobs after the move: got %+v, want the original job", jobs) + } + if _, err := os.Stat(filepath.Join(svc.store.Paths.AppDir, "moved", "jobs.json")); err != nil { + t.Errorf("jobs should have been written to the new path: %v", err) + } +} + +// Adoption throws away every runtime, including the state of a run in flight, +// and a finishing run would then write its result onto whichever job inherited +// its ID. Refusing the switch is what keeps that from happening. +func TestUpdateSettingsRefusesJobsFileSwitchWhileRunning(t *testing.T) { + svc := newTempService(t, []domain.Job{{ID: 1, Name: "Long", Schedule: "@every 1h", Command: "echo long", Enabled: true}}) + entered := make(chan int, 1) + release := make(chan struct{}) + svc.runJob = func(_ context.Context, job *domain.Job, _ string, _ string, _ time.Duration) (domain.RunRecord, error) { + entered <- job.ID + <-release + return domain.RunRecord{Time: "t", JobID: job.ID, JobName: job.Name, State: "Success"}, nil + } + done := completions(svc) + + if err := svc.RunNow(1); err != nil { + t.Fatalf("RunNow: %v", err) + } + <-entered + + config := svc.store.Config + config.JobsFile = filepath.Join("elsewhere", "jobs.json") + if err := svc.UpdateSettings(config); err == nil { + t.Error("expected the jobs-file switch to be refused while a job is running") + } + if svc.Store().Config.JobsFile == config.JobsFile { + t.Error("the refused switch must not have been persisted") + } + + // A setting that does not touch the jobs file still saves during a run. + unrelated := svc.Store().Config + unrelated.NotifyOnFailure = !unrelated.NotifyOnFailure + if err := svc.UpdateSettings(unrelated); err != nil { + t.Errorf("unrelated setting should still save during a run: %v", err) + } + + close(release) + waitRecord(t, done) +} + func TestPrependLogCapsActivityList(t *testing.T) { runtime := &domain.JobRuntime{} for i := 0; i < maxJobLogs+10; i++ { diff --git a/src/app/service.go b/src/app/service.go index ea83e9a..00fa6f1 100644 --- a/src/app/service.go +++ b/src/app/service.go @@ -69,28 +69,38 @@ type Service struct { // store is the Service's sole channel to persistence. func NewService(store *storage.Store, jobs []domain.Job) *Service { s := &Service{ - store: store, - jobs: jobs, - runtimes: domain.NewRuntimes(jobs), - schedules: make(map[int]domain.Schedule, len(jobs)), - runJob: runner.RunJob, - ctx: context.Background(), - paused: store.Config.Paused, + store: store, + runJob: runner.RunJob, + ctx: context.Background(), + paused: store.Config.Paused, } - // Parse every schedule once, then compute each job's first next-run so the - // Service is ready to schedule the moment it exists — mirroring the old - // scheduler's reset-on-construction. No lock is needed: construction is - // single-threaded, before Start launches the timing loop. + // No lock is needed here: construction is single-threaded, before Start + // launches the timing loop. + s.adoptJobsLocked(jobs) + return s +} + +// adoptJobsLocked makes jobs the Service's durable state and rebuilds everything +// derived from it: the runtime map, the parsed-schedule cache, each job's first +// next-run — so the Service is ready to schedule the moment it exists, mirroring +// the old scheduler's reset-on-construction — and the statistics seeded from +// existing log files, so the details panel shows accumulated run history +// immediately rather than only runs since this process started. +// +// It backs both construction and a Settings change that points at a different +// jobs file. The caller must hold mu. +func (s *Service) adoptJobsLocked(jobs []domain.Job) { + s.jobs = jobs + s.runtimes = domain.NewRuntimes(jobs) + s.schedules = make(map[int]domain.Schedule, len(jobs)) + now := time.Now() for index := range s.jobs { job := &s.jobs[index] s.parseScheduleLocked(job) s.refreshNextRunFromLocked(job, s.runtimes[job.ID], now) } - // Seed execution-time statistics from existing log files so the details panel - // shows accumulated run history immediately after a restart, not just runs - // since this process started. - for id, seed := range runner.SeedStats(store.Paths.LogsDir, jobs, store.Config.MaxLogFiles) { + for id, seed := range runner.SeedStats(s.store.Paths.LogsDir, s.jobs, s.store.Config.MaxLogFiles) { runtime := s.runtimes[id] if runtime == nil { continue @@ -102,7 +112,6 @@ func NewService(store *storage.Store, jobs []domain.Job) *Service { runtime.MaxDurationMS = seed.MaxDurationMS runtime.TimedRunCount = seed.TimedRunCount } - return s } // Start begins scheduling with the real wall clock. It is the production entry diff --git a/src/app/version.go b/src/app/version.go index fb85cb6..ae525ed 100644 --- a/src/app/version.go +++ b/src/app/version.go @@ -3,4 +3,4 @@ package app // Version is the application version shown in the GUI and used by build // scripts in artifact names. It is a var rather than a const so release builds // can override it with Go ldflags when CI tags a build. -var Version = "0.14.0" +var Version = "0.15.0" diff --git a/src/domain/config.go b/src/domain/config.go index f093c3f..b03fafe 100644 --- a/src/domain/config.go +++ b/src/domain/config.go @@ -64,7 +64,15 @@ const ( // application-level choices: where to read jobs from, where to write logs, and // how the desktop shell should behave. type Config struct { - JobsDir string `json:"jobs_dir"` + // JobsFile is the full path of the JSON file holding the job definitions, + // file name included, so the user can keep jobs under any name they like. A + // relative path is resolved against the program folder. + JobsFile string `json:"jobs_file"` + // JobsDir is the pre-0.15 setting that named only the directory, with the + // file name fixed to jobs.json. It is still read so an older gosentry.json + // keeps working: storage.loadOrCreateConfig turns it into JobsFile and + // clears it, so the field disappears from the file on the next save. + JobsDir string `json:"jobs_dir,omitempty"` LogsDir string `json:"logs_dir"` MaxLogFiles int `json:"max_log_files"` MaxLogAgeDays int `json:"max_log_age_days"` @@ -93,7 +101,7 @@ type Config struct { // offers to restore via its "Defaults" button. func DefaultConfig() Config { return Config{ - JobsDir: ".", + JobsFile: "jobs.json", LogsDir: "logs", MaxLogFiles: 100, MaxLogAgeDays: 30, diff --git a/src/storage/paths.go b/src/storage/paths.go index fc64057..2c30aef 100644 --- a/src/storage/paths.go +++ b/src/storage/paths.go @@ -9,9 +9,11 @@ const ( // The config file stays beside the executable so the portable build behaves // predictably: moving the program folder moves its settings with it. ConfigFileName = "gosentry.json" - // Jobs are kept in a separate JSON file because the user can choose a - // different jobs directory, while application settings remain local to the - // installed/copied program. + // Jobs are kept in a separate JSON file because the user can point the + // configuration at any jobs file they like, while application settings + // remain local to the installed/copied program. This is only the default + // name, used before the config is read and when an older config that named + // just a directory is migrated. JobsFileName = "jobs.json" ) @@ -23,10 +25,13 @@ type Paths struct { ExecutablePath string AppDir string ConfigPath string - JobsDir string - JobsPath string - LogsDir string - DesktopIcon string + // JobsDir is the directory containing JobsPath. It is derived from the + // configured jobs file, never configured on its own, and exists so writers + // can create the folder before saving. + JobsDir string + JobsPath string + LogsDir string + DesktopIcon string } func ResolvePaths() (Paths, error) { diff --git a/src/storage/store.go b/src/storage/store.go index 91a049d..0e6f627 100644 --- a/src/storage/store.go +++ b/src/storage/store.go @@ -78,14 +78,25 @@ func loadOrCreateConfig(paths Paths) (domain.Config, error) { if err != nil { return domain.Config{}, err } + // Clearing the default first keeps "the file sets jobs_file" distinguishable + // from "the file omits it", which the jobs_dir migration below depends on. + // The fallbacks restore a value in either case. + config.JobsFile = "" if err := json.Unmarshal(data, &config); err != nil { return domain.Config{}, err } - if strings.TrimSpace(config.JobsDir) == "" { + // A config written before the setting named a file carries jobs_dir instead + // of jobs_file. Keep its meaning by appending the fixed name that version + // used, then drop the old key so the file is rewritten in the current shape. + if strings.TrimSpace(config.JobsFile) == "" && strings.TrimSpace(config.JobsDir) != "" { + config.JobsFile = filepath.Join(config.JobsDir, JobsFileName) + } + config.JobsDir = "" + if strings.TrimSpace(config.JobsFile) == "" { // Empty paths are treated as missing values rather than intentional root // directories. This avoids accidentally writing jobs to unexpected places. - config.JobsDir = "." + config.JobsFile = JobsFileName } if strings.TrimSpace(config.LogsDir) == "" { config.LogsDir = "logs" @@ -112,24 +123,39 @@ func loadOrCreateConfig(paths Paths) (domain.Config, error) { return config, nil } -func loadOrCreateJobs(path string) ([]domain.Job, error) { - if _, err := os.Stat(path); errors.Is(err, os.ErrNotExist) { - // Seed harmless sample jobs so a new user can immediately see scheduled - // and manual execution without inventing a command. - jobs := defaultJobs() - normalizeJobs(jobs) - return jobs, writeJSON(path, domain.JobsFile{Jobs: jobs}) - } - +// LoadJobsFile reads and normalizes the job definitions at path. The bool +// reports whether the file was there: a missing file is not an error but the +// answer to "is this file already a jobs file?", which is what the Settings tab +// needs when the user points the application at a different jobs file. +func LoadJobsFile(path string) ([]domain.Job, bool, error) { data, err := os.ReadFile(path) + if errors.Is(err, os.ErrNotExist) { + return nil, false, nil + } if err != nil { - return nil, err + return nil, false, err } var file domain.JobsFile if err := json.Unmarshal(data, &file); err != nil { + return nil, false, err + } + normalizeJobs(file.Jobs) + return file.Jobs, true, nil +} + +func loadOrCreateJobs(path string) ([]domain.Job, error) { + jobs, found, err := LoadJobsFile(path) + if err != nil { return nil, err } - return file.Jobs, nil + if found { + return jobs, nil + } + // Seed harmless sample jobs so a new user can immediately see scheduled + // and manual execution without inventing a command. + jobs = defaultJobs() + normalizeJobs(jobs) + return jobs, writeJSON(path, domain.JobsFile{Jobs: jobs}) } func normalizeJobs(jobs []domain.Job) { @@ -162,28 +188,26 @@ func normalizeJobs(jobs []domain.Job) { } } -func resolveJobsDir(appDir string, jobsDir string) string { - return ResolveConfiguredDir(appDir, jobsDir) -} - -// ResolveConfiguredDir turns a directory from the config into the absolute -// path the application will actually use. It is exported so callers outside -// storage — the settings tab, which opens the configured logs folder — apply -// the same rule to a path the user has typed but not yet saved. -func ResolveConfiguredDir(appDir string, dir string) string { - if filepath.IsAbs(dir) { - return dir +// ResolveConfiguredPath turns a file or directory path from the config into the +// absolute path the application will actually use. It is exported so callers +// outside storage — the settings tab, which opens the configured logs folder — +// apply the same rule to a path the user has typed but not yet saved. +func ResolveConfiguredPath(appDir string, path string) string { + if filepath.IsAbs(path) { + return path } // Relative paths are resolved against the executable directory, not the // process working directory. This matches ResolvePaths and keeps shortcuts, // Explorer launches, and terminal launches consistent. - return filepath.Clean(filepath.Join(appDir, dir)) + return filepath.Clean(filepath.Join(appDir, path)) } func (s *Store) applyConfigPaths() { - s.Paths.JobsDir = ResolveConfiguredDir(s.Paths.AppDir, s.Config.JobsDir) - s.Paths.JobsPath = filepath.Join(s.Paths.JobsDir, JobsFileName) - s.Paths.LogsDir = ResolveConfiguredDir(s.Paths.AppDir, s.Config.LogsDir) + // The jobs file is configured as a whole path; its directory is derived so + // SaveJobs can create the folder when the user points at a new location. + s.Paths.JobsPath = ResolveConfiguredPath(s.Paths.AppDir, s.Config.JobsFile) + s.Paths.JobsDir = filepath.Dir(s.Paths.JobsPath) + s.Paths.LogsDir = ResolveConfiguredPath(s.Paths.AppDir, s.Config.LogsDir) } func writeJSON(path string, value any) error { diff --git a/src/storage/store_test.go b/src/storage/store_test.go index ff1f568..7563a7b 100644 --- a/src/storage/store_test.go +++ b/src/storage/store_test.go @@ -77,7 +77,7 @@ func TestConfigRoundTrip(t *testing.T) { } want := domain.Config{ - JobsDir: "/custom/jobs", + JobsFile: "/custom/jobs/team.json", LogsDir: "/custom/logs", MaxLogFiles: 50, MaxLogAgeDays: 14, @@ -94,8 +94,8 @@ func TestConfigRoundTrip(t *testing.T) { t.Fatal(err) } - if got.JobsDir != want.JobsDir { - t.Errorf("JobsDir: got %q, want %q", got.JobsDir, want.JobsDir) + if got.JobsFile != want.JobsFile { + t.Errorf("JobsFile: got %q, want %q", got.JobsFile, want.JobsFile) } if got.LogsDir != want.LogsDir { t.Errorf("LogsDir: got %q, want %q", got.LogsDir, want.LogsDir) @@ -156,8 +156,8 @@ func TestLoadOrCreateConfigCreatesDefaultsOnFirstRun(t *testing.T) { if err != nil { t.Fatal(err) } - if got.JobsDir != "." { - t.Errorf("default JobsDir = %q, want '.'", got.JobsDir) + if got.JobsFile != "jobs.json" { + t.Errorf("default JobsFile = %q, want 'jobs.json'", got.JobsFile) } if got.LogsDir != "logs" { t.Errorf("default LogsDir = %q, want 'logs'", got.LogsDir) @@ -207,6 +207,108 @@ func TestLoadOrCreateConfigKeepsZeroTimeoutOnReload(t *testing.T) { } } +// TestLoadOrCreateConfigMigratesJobsDir covers a gosentry.json written before +// the setting named a file: the old jobs_dir keeps pointing at the same jobs +// file, and the retired key is dropped so it is not written back. +func TestLoadOrCreateConfigMigratesJobsDir(t *testing.T) { + dir := t.TempDir() + paths := Paths{ + AppDir: dir, + ConfigPath: filepath.Join(dir, ConfigFileName), + } + legacy := map[string]any{ + "jobs_dir": filepath.Join(dir, "shared"), + "logs_dir": "logs", + "max_log_files": 100, + "max_log_age_days": 30, + } + if err := writeJSON(paths.ConfigPath, legacy); err != nil { + t.Fatal(err) + } + + got, err := loadOrCreateConfig(paths) + if err != nil { + t.Fatal(err) + } + want := filepath.Join(dir, "shared", JobsFileName) + if got.JobsFile != want { + t.Errorf("migrated JobsFile: got %q, want %q", got.JobsFile, want) + } + if got.JobsDir != "" { + t.Errorf("legacy JobsDir should be cleared, got %q", got.JobsDir) + } + + // The migrated config must not carry the retired key once it is saved. + store := &Store{Paths: paths, Config: got} + if err := store.SaveConfig(); err != nil { + t.Fatal(err) + } + data, err := os.ReadFile(paths.ConfigPath) + if err != nil { + t.Fatal(err) + } + if strings.Contains(string(data), "jobs_dir") { + t.Errorf("saved config should not contain jobs_dir:\n%s", data) + } +} + +// TestLoadJobsFileReportsMissingWithoutCreating covers the loader the Settings +// tab uses to decide between adopting a jobs file and writing the current jobs +// to it: a missing file is reported as "not found" rather than an error, and — +// unlike the startup path — is not seeded with sample jobs. +func TestLoadJobsFileReportsMissingWithoutCreating(t *testing.T) { + dir := t.TempDir() + missing := filepath.Join(dir, "nothing-here.json") + + jobs, found, err := LoadJobsFile(missing) + if err != nil { + t.Fatalf("missing file should not be an error: %v", err) + } + if found || jobs != nil { + t.Errorf("missing file: got found=%v jobs=%+v, want false/nil", found, jobs) + } + if _, err := os.Stat(missing); !os.IsNotExist(err) { + t.Error("LoadJobsFile must not create the file it was asked about") + } + + // An existing file comes back normalized, so a hand-written jobs file gains + // its IDs and defaults before the application adopts it. + path := filepath.Join(dir, "hand-written.json") + if err := writeJSON(path, domain.JobsFile{Jobs: []domain.Job{{Name: "No ID"}}}); err != nil { + t.Fatal(err) + } + jobs, found, err = LoadJobsFile(path) + if err != nil { + t.Fatal(err) + } + if !found || len(jobs) != 1 { + t.Fatalf("existing file: got found=%v jobs=%+v, want true and one job", found, jobs) + } + if jobs[0].ID != 1 || jobs[0].Schedule == "" || jobs[0].Command == "" { + t.Errorf("loaded job should be normalized, got %+v", jobs[0]) + } +} + +// TestApplyConfigPathsDerivesJobsDir checks that the jobs file drives both +// resolved paths: relative values resolve against the program folder, and the +// containing directory comes from the file name the user chose. +func TestApplyConfigPathsDerivesJobsDir(t *testing.T) { + dir := t.TempDir() + store := &Store{ + Paths: Paths{AppDir: dir}, + Config: domain.Config{JobsFile: filepath.Join("shared", "team.json"), LogsDir: "logs"}, + } + + store.applyConfigPaths() + + if want := filepath.Join(dir, "shared", "team.json"); store.Paths.JobsPath != want { + t.Errorf("JobsPath: got %q, want %q", store.Paths.JobsPath, want) + } + if want := filepath.Join(dir, "shared"); store.Paths.JobsDir != want { + t.Errorf("JobsDir: got %q, want %q", store.Paths.JobsDir, want) + } +} + // TestJobTimeoutRoundTripsThreeStates pins the on-disk encoding that keeps // "inherit" and "no timeout" distinguishable: nil is omitted entirely, while an // explicit 0 is written and read back as a set value. diff --git a/src/ui/mainwindow.go b/src/ui/mainwindow.go index 286bf51..fa7160a 100644 --- a/src/ui/mainwindow.go +++ b/src/ui/mainwindow.go @@ -1,6 +1,7 @@ package ui import ( + "strconv" "time" "gitea.mixdep.ru/mix/gosentry/assets" @@ -64,6 +65,7 @@ func newMainView(w fyne.Window, svc *app.Service) (fyne.CanvasObject, func(time. svc.Subscribe(app.ObserverFunc(func(ev app.Event) { recorded, isRecorded := ev.(app.RunRecorded) errOccurred, isError := ev.(app.ErrorOccurred) + jobsLoaded, isJobsLoaded := ev.(app.JobsLoaded) fyne.Do(func() { if isRecorded { events = append(events, recorded.Record) @@ -80,6 +82,12 @@ func newMainView(w fyne.Window, svc *app.Service) (fyne.CanvasObject, func(time. if isError { events = append(events, newEvent(0, "Service", "Error", errOccurred.Err.Error())) } + if isJobsLoaded { + // Selecting an existing jobs file replaces the job list without a + // prompt, so History carries the receipt: how many jobs, from where. + detail := strconv.Itoa(jobsLoaded.Count) + " jobs from " + jobsLoaded.Path + events = append(events, newEvent(0, "Service", "Jobs loaded", detail)) + } refresh() }) })) diff --git a/src/ui/mainwindow_test.go b/src/ui/mainwindow_test.go index 926dc87..01806e6 100644 --- a/src/ui/mainwindow_test.go +++ b/src/ui/mainwindow_test.go @@ -27,7 +27,7 @@ func newTestStore(t *testing.T) *storage.Store { LogsDir: filepath.Join(dir, "logs"), }, Config: domain.Config{ - JobsDir: ".", + JobsFile: "jobs.json", LogsDir: "logs", MaxLogFiles: 100, MaxLogAgeDays: 30, diff --git a/src/ui/settings_view.go b/src/ui/settings_view.go index 4a2bf45..af88f36 100644 --- a/src/ui/settings_view.go +++ b/src/ui/settings_view.go @@ -18,6 +18,7 @@ import ( "fyne.io/fyne/v2/canvas" "fyne.io/fyne/v2/container" "fyne.io/fyne/v2/dialog" + fynestorage "fyne.io/fyne/v2/storage" "fyne.io/fyne/v2/theme" "fyne.io/fyne/v2/widget" ) @@ -94,11 +95,13 @@ func settingsView(w fyne.Window, svc *app.Service) fyne.CanvasObject { defaultTimeout.SetPlaceHolder("0 = no timeout") defaultTimeout.SetText(strconv.Itoa(store.Config.DefaultTimeoutSeconds)) defaultTimeout.OnChanged = func(string) { updateSaveState() } - jobsDir := widget.NewEntry() - jobsDir.SetText(store.Config.JobsDir) - jobsDir.OnChanged = func(string) { updateSaveState() } - jobsDirBrowse := widget.NewButtonWithIcon("Browse", theme.FolderOpenIcon(), func() { - chooseFolder(w, jobsDir) + jobsFile := widget.NewEntry() + jobsFile.SetText(store.Config.JobsFile) + jobsFile.OnChanged = func(string) { updateSaveState() } + // The picker only offers existing files; a jobs file that does not exist yet + // is entered by typing its path, which Save then creates. + jobsFileBrowse := widget.NewButtonWithIcon("Browse", theme.FileIcon(), func() { + chooseJSONFile(w, jobsFile) }) logsDir := widget.NewEntry() logsDir.SetText(store.Config.LogsDir) @@ -136,8 +139,8 @@ func settingsView(w fyne.Window, svc *app.Service) fyne.CanvasObject { settingsStatus.SetText("Max log age days must be a positive number") return } - if strings.TrimSpace(jobsDir.Text) == "" { - settingsStatus.SetText("Jobs directory is required") + if strings.TrimSpace(jobsFile.Text) == "" { + settingsStatus.SetText("Jobs file is required") return } if strings.TrimSpace(logsDir.Text) == "" { @@ -153,7 +156,7 @@ func settingsView(w fyne.Window, svc *app.Service) fyne.CanvasObject { // validates it, persists config and jobs to the (possibly new) directory, // and runs log cleanup so tightened retention limits take effect at once. config := store.Config - config.JobsDir = strings.TrimSpace(jobsDir.Text) + config.JobsFile = strings.TrimSpace(jobsFile.Text) config.LogsDir = strings.TrimSpace(logsDir.Text) config.MaxLogFiles = files config.MaxLogAgeDays = days @@ -191,7 +194,7 @@ func settingsView(w fyne.Window, svc *app.Service) fyne.CanvasObject { executionModeSelect.Selected != string(c.ExecutionMode) || overlapPolicySelect.Selected != string(c.OverlapPolicy) || strings.TrimSpace(defaultTimeout.Text) != strconv.Itoa(c.DefaultTimeoutSeconds) || - strings.TrimSpace(jobsDir.Text) != c.JobsDir || + strings.TrimSpace(jobsFile.Text) != c.JobsFile || strings.TrimSpace(logsDir.Text) != c.LogsDir || strings.TrimSpace(maxLogFiles.Text) != strconv.Itoa(c.MaxLogFiles) || strings.TrimSpace(maxLogAgeDays.Text) != strconv.Itoa(c.MaxLogAgeDays) || @@ -217,7 +220,7 @@ func settingsView(w fyne.Window, svc *app.Service) fyne.CanvasObject { executionModeSelect.SetSelected(string(c.ExecutionMode)) overlapPolicySelect.SetSelected(string(c.OverlapPolicy)) defaultTimeout.SetText(strconv.Itoa(c.DefaultTimeoutSeconds)) - jobsDir.SetText(c.JobsDir) + jobsFile.SetText(c.JobsFile) logsDir.SetText(c.LogsDir) maxLogFiles.SetText(strconv.Itoa(c.MaxLogFiles)) maxLogAgeDays.SetText(strconv.Itoa(c.MaxLogAgeDays)) @@ -268,8 +271,8 @@ func settingsView(w fyne.Window, svc *app.Service) fyne.CanvasObject { container.NewVBox( widget.NewLabelWithStyle("Storage", fyne.TextAlignLeading, fyne.TextStyle{Bold: true}), settingsRow("Config JSON", widget.NewLabel(store.Paths.ConfigPath)), - settingsRow("Jobs directory", container.NewBorder(nil, nil, nil, jobsDirBrowse, jobsDir)), - // Browse stays rightmost so it lines up with the Jobs directory row + settingsRow("Jobs file", container.NewBorder(nil, nil, nil, jobsFileBrowse, jobsFile)), + // Browse stays rightmost so it lines up with the Jobs file row // above it; Open sits between it and the path it opens. settingsRow("Logs directory", container.NewBorder(nil, nil, nil, container.NewHBox(logsDirOpen, logsDirBrowse), logsDir)), settingsRow("Max log files", maxLogFiles), @@ -358,6 +361,21 @@ func chooseFile(w fyne.Window, target *widget.Entry) { fileDialog.Show() } +// chooseJSONFile is chooseFile restricted to .json files, used for the jobs +// file so the picker does not list every file in the folder. The entry stays +// editable, which is how a path to a file that does not exist yet is entered. +func chooseJSONFile(w fyne.Window, target *widget.Entry) { + fileDialog := dialog.NewFileOpen(func(uri fyne.URIReadCloser, err error) { + if err != nil || uri == nil { + return + } + target.SetText(uri.URI().Path()) + }, w) + fileDialog.SetFilter(fynestorage.NewExtensionFileFilter([]string{".json"})) + fileDialog.Resize(fyne.NewSize(900, 640)) + fileDialog.Show() +} + func chooseFolder(w fyne.Window, target *widget.Entry) { folderDialog := dialog.NewFolderOpen(func(uri fyne.ListableURI, err error) { if err != nil || uri == nil { @@ -380,7 +398,7 @@ func settingsFolderPath(appDir string, text string) string { if trimmed == "" { return "" } - return storage.ResolveConfiguredDir(appDir, trimmed) + return storage.ResolveConfiguredPath(appDir, trimmed) } // openFolder reveals dir in the desktop file manager. A folder that is not set