diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index a4d0f3b..1be7786 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -51,6 +51,11 @@ the app icon (experimental).** gets slower the longer the app has been running. Measured on 5000 accumulated records, one History redraw went from **15.8 ms to 0.9 ms**; at the new cap the width rescan alone accounted for 1.5 ms of every redraw. +- **Max log files and max log age days now accept 0, meaning "keep + everything."** Log cleanup already supported disabling either policy; the + Settings form and the Service validator rejected the value that would have + turned it on. A config that already set either to 0 is no longer silently + rewritten back to the 100/30 defaults on load. **Jobs:** diff --git a/docs/STANDARDS.md b/docs/STANDARDS.md index a7ee553..cfb072e 100644 --- a/docs/STANDARDS.md +++ b/docs/STANDARDS.md @@ -71,6 +71,14 @@ change to their shape has to stay compatible on its own. = 0) and is overridable per job (`Job.TimeoutSeconds *int`: unset = inherit the global default, 0 = no timeout, positive = seconds). Neither zero may be normalized away on load — 0 is a value, not a missing field. +- **`Config.MaxLogFiles` and `Config.MaxLogAgeDays` of 0 mean "keep everything", + not "unset".** `runner.CleanupLogs` already treated `<= 0` as "policy + disabled"; `app.validateConfig` and the Settings form now accept 0 (only a + negative count is rejected), and `storage.loadOrCreateConfig` no longer + backfills 0 to 100 / 30 — a config written before either field existed still + picks up the default because `json.Unmarshal` leaves an absent key holding + whatever `DefaultConfig()` set, the same mechanism `DefaultTimeoutSeconds` + relies on. - **A `StartOnly` process is expected to outlive GoSentry.** The option exists to launch something and let go of it, so the runner builds that invocation on `context.Background()`, not on the application's lifecycle context: quitting diff --git a/src/app/events.go b/src/app/events.go index d21ab88..352c573 100644 --- a/src/app/events.go +++ b/src/app/events.go @@ -4,9 +4,12 @@ import "gitea.mixdep.ru/mix/gosentry/src/domain" // Event is something the Service did to its state that observers may want to // react to. It is a sealed interface: the concrete types in this file are the -// only implementations (enforced by the unexported isEvent marker), so a UI -// listener can exhaustively type-switch over them and the compiler will flag a -// new event type that a switch forgot to handle. +// only implementations (enforced by the unexported isEvent marker), so an +// Event handed to an Observer is always one of the types declared here — a +// caller outside this package cannot manufacture a new one. Go's type switch +// has no exhaustiveness check, so sealing buys that guarantee, not a +// compile-time warning when a new event type is added and a listener forgets +// to handle it; the listener still has to be updated by hand. // // Events replace the old single onChange callback. Instead of the scheduler // reaching into the GUI, the Service emits typed events and the UI subscribes — diff --git a/src/app/operations.go b/src/app/operations.go index 091f27c..006b70c 100644 --- a/src/app/operations.go +++ b/src/app/operations.go @@ -503,11 +503,13 @@ func validateConfig(config domain.Config) error { if strings.TrimSpace(config.LogsDir) == "" { return errors.New("logs directory is required") } - if config.MaxLogFiles <= 0 { - return errors.New("max log files must be a positive number") + // 0 means "keep everything" (see runner.CleanupLogs); only a negative count + // is rejected, the same three-state shape as DefaultTimeoutSeconds below. + if config.MaxLogFiles < 0 { + return errors.New("max log files must be zero (unlimited) or a positive number") } - if config.MaxLogAgeDays <= 0 { - return errors.New("max log age days must be a positive number") + if config.MaxLogAgeDays < 0 { + return errors.New("max log age days must be zero (unlimited) or a positive number") } if config.ExecutionMode != domain.ExecutionModeParallel && config.ExecutionMode != domain.ExecutionModeSequential { return errors.New("execution mode must be 'parallel' or 'sequential'") diff --git a/src/app/operations_test.go b/src/app/operations_test.go index 63ab45f..0237703 100644 --- a/src/app/operations_test.go +++ b/src/app/operations_test.go @@ -522,9 +522,9 @@ func TestUpdateSettingsPersistsAndValidates(t *testing.T) { svc := newTempService(t, nil) bad := svc.store.Config - bad.MaxLogFiles = 0 + bad.MaxLogFiles = -1 if err := svc.UpdateSettings(bad); err == nil { - t.Error("expected validation error for non-positive max log files") + t.Error("expected validation error for negative max log files") } good := svc.store.Config @@ -533,8 +533,21 @@ func TestUpdateSettingsPersistsAndValidates(t *testing.T) { if err := svc.UpdateSettings(good); err != nil { t.Fatalf("UpdateSettings: %v", err) } - if svc.Store().Config.MaxLogAgeDays != 7 || svc.Store().Config.NotifyOnFailure { - t.Errorf("config not applied: %+v", svc.Store().Config) + if svc.store.Config.MaxLogAgeDays != 7 || svc.store.Config.NotifyOnFailure { + t.Errorf("config not applied: %+v", svc.store.Config) + } + + // 0 means "keep everything" (see STANDARDS §Intentional behavior), not an + // invalid value, so it must be accepted and persisted rather than rejected + // or silently backfilled. + unlimited := svc.store.Config + unlimited.MaxLogFiles = 0 + unlimited.MaxLogAgeDays = 0 + if err := svc.UpdateSettings(unlimited); err != nil { + t.Fatalf("UpdateSettings with zero retention limits: %v", err) + } + if svc.store.Config.MaxLogFiles != 0 || svc.store.Config.MaxLogAgeDays != 0 { + t.Errorf("zero retention limits not preserved: %+v", svc.store.Config) } } @@ -549,8 +562,8 @@ func TestUpdateSettingsRejectsInvalidConfigs(t *testing.T) { {"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 }}, + {"negative max files", func(c *domain.Config) { c.MaxLogFiles = -1 }}, + {"negative max age", func(c *domain.Config) { c.MaxLogAgeDays = -1 }}, {"negative default timeout", func(c *domain.Config) { c.DefaultTimeoutSeconds = -1 }}, } for _, tc := range tests { @@ -712,12 +725,12 @@ func TestUpdateSettingsRefusesJobsFileSwitchWhileRunning(t *testing.T) { 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 { + 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 := 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) diff --git a/src/app/service.go b/src/app/service.go index 7d56f8f..2da02c3 100644 --- a/src/app/service.go +++ b/src/app/service.go @@ -204,11 +204,27 @@ func Open() (*Service, error) { return svc, nil } -// Store returns the underlying store. It is exposed so callers that still need -// resolved paths and config (the GUI, during the transition) can reach them; -// later phases narrow this surface. -func (s *Service) Store() *storage.Store { - return s.store +// Config returns a copy of the current application configuration, safe to +// call from any goroutine. UpdateSettings, SetGlobalPause, and SetJobListView +// are the only writers and all mutate store.Config under mu; copying under the +// same lock is what keeps a UI read from racing them, instead of holding onto +// the *storage.Store this used to hand out (see STANDARDS: the UI reads +// Service state through typed events and accessors, never shared mutable +// state). +func (s *Service) Config() domain.Config { + s.mu.Lock() + defer s.mu.Unlock() + return s.store.Config +} + +// Paths returns a copy of the store's resolved filesystem paths. AppDir and +// ConfigPath are fixed for the process; JobsPath, JobsDir, and LogsDir are +// re-derived under mu on every settings save (storage.Store.applyConfigPaths), +// so this copies under the same lock as Config for the same reason. +func (s *Service) Paths() storage.Paths { + s.mu.Lock() + defer s.mu.Unlock() + return s.store.Paths } // Jobs returns a copy of the durable jobs slice. Returning a copy keeps callers diff --git a/src/platform/autostart/autostart_linux.go b/src/platform/autostart/autostart_linux.go index 428d29b..6baa258 100644 --- a/src/platform/autostart/autostart_linux.go +++ b/src/platform/autostart/autostart_linux.go @@ -18,16 +18,16 @@ type linuxManager struct{} func New() Manager { return linuxManager{} } func (linuxManager) Set(enabled, startInTray bool, executablePath, iconPath string) error { - return SetAutostart(enabled, startInTray, executablePath, iconPath) + return setAutostart(enabled, startInTray, executablePath, iconPath) } func (linuxManager) Status(expectedEnabled, startInTray bool, executablePath string) (bool, string) { - return AutostartStatus(expectedEnabled, startInTray, executablePath) + return autostartStatus(expectedEnabled, startInTray, executablePath) } const autostartDesktopFileName = "gosentry.desktop" -func SetAutostart(enabled bool, startInTray bool, executablePath string, iconPath string) error { +func setAutostart(enabled bool, startInTray bool, executablePath string, iconPath string) error { desktopPath, err := autostartDesktopPath() if err != nil { return err @@ -58,7 +58,7 @@ X-GNOME-Autostart-enabled=true return nil } -func AutostartStatus(expectedEnabled bool, startInTray bool, executablePath string) (bool, string) { +func autostartStatus(expectedEnabled bool, startInTray bool, executablePath string) (bool, string) { desktopPath, err := autostartDesktopPath() if err != nil { return false, "Cannot resolve XDG autostart directory" diff --git a/src/platform/autostart/autostart_linux_test.go b/src/platform/autostart/autostart_linux_test.go index b629500..ed9f068 100644 --- a/src/platform/autostart/autostart_linux_test.go +++ b/src/platform/autostart/autostart_linux_test.go @@ -14,7 +14,7 @@ func TestLinuxAutostartStartsInTray(t *testing.T) { t.Setenv("XDG_CONFIG_HOME", t.TempDir()) executablePath := "/opt/Go Sentry/gosentry" - if err := SetAutostart(true, true, executablePath, "/opt/Go Sentry/gosentry.png"); err != nil { + if err := setAutostart(true, true, executablePath, "/opt/Go Sentry/gosentry.png"); err != nil { t.Fatalf("enable autostart: %v", err) } @@ -37,7 +37,7 @@ func TestLinuxAutostartWithoutTrayFlag(t *testing.T) { t.Setenv("XDG_CONFIG_HOME", t.TempDir()) executablePath := "/opt/Go Sentry/gosentry" - if err := SetAutostart(true, false, executablePath, ""); err != nil { + if err := setAutostart(true, false, executablePath, ""); err != nil { t.Fatalf("enable autostart: %v", err) } diff --git a/src/platform/autostart/autostart_other.go b/src/platform/autostart/autostart_other.go index 8f99fb7..8d5ed1b 100644 --- a/src/platform/autostart/autostart_other.go +++ b/src/platform/autostart/autostart_other.go @@ -10,21 +10,21 @@ type otherManager struct{} func New() Manager { return otherManager{} } func (otherManager) Set(enabled, startInTray bool, executablePath, iconPath string) error { - return SetAutostart(enabled, startInTray, executablePath, iconPath) + return setAutostart(enabled, startInTray, executablePath, iconPath) } func (otherManager) Status(expectedEnabled, startInTray bool, executablePath string) (bool, string) { - return AutostartStatus(expectedEnabled, startInTray, executablePath) + return autostartStatus(expectedEnabled, startInTray, executablePath) } -func SetAutostart(enabled bool, startInTray bool, executablePath string, iconPath string) error { +func setAutostart(enabled bool, startInTray bool, executablePath string, iconPath string) error { if !enabled { return nil } return fmt.Errorf("autostart is not implemented for this platform") } -func AutostartStatus(expectedEnabled bool, startInTray bool, executablePath string) (bool, string) { +func autostartStatus(expectedEnabled bool, startInTray bool, executablePath string) (bool, string) { if !expectedEnabled { return true, "Autostart is off" } diff --git a/src/platform/autostart/autostart_windows.go b/src/platform/autostart/autostart_windows.go index 1adbc2c..8e5bae3 100644 --- a/src/platform/autostart/autostart_windows.go +++ b/src/platform/autostart/autostart_windows.go @@ -17,17 +17,17 @@ type windowsManager struct{} func New() Manager { return windowsManager{} } func (windowsManager) Set(enabled, startInTray bool, executablePath, iconPath string) error { - return SetAutostart(enabled, startInTray, executablePath, iconPath) + return setAutostart(enabled, startInTray, executablePath, iconPath) } func (windowsManager) Status(expectedEnabled, startInTray bool, executablePath string) (bool, string) { - return AutostartStatus(expectedEnabled, startInTray, executablePath) + return autostartStatus(expectedEnabled, startInTray, executablePath) } const autostartName = "GoSentry" const startupShortcutFile = autostartName + ".lnk" -func SetAutostart(enabled bool, startInTray bool, executablePath string, iconPath string) error { +func setAutostart(enabled bool, startInTray bool, executablePath string, iconPath string) error { // Windows autostart used to write HKCU\Run values, but that approach became // brittle once paths with spaces and the "--start-in-tray" argument entered // the picture. A Startup-folder shortcut stores target path and arguments as @@ -44,7 +44,7 @@ func SetAutostart(enabled bool, startInTray bool, executablePath string, iconPat return removeIfExists(shortcutPath) } -func AutostartStatus(expectedEnabled bool, startInTray bool, executablePath string) (bool, string) { +func autostartStatus(expectedEnabled bool, startInTray bool, executablePath string) (bool, string) { shortcutPath, err := startupShortcutPath() if err != nil { return false, "Startup folder cannot be resolved" @@ -126,7 +126,7 @@ func readShortcut(shortcutPath string) (string, string, error) { // OEM code page (e.g. CP866 on Russian Windows). Without this override, // [Console]::Out.Write encodes Cyrillic and other non-ASCII characters as // OEM bytes; Go then reads them as UTF-8 and gets a different string from - // os.Executable, causing AutostartStatus to report "shortcut points to + // os.Executable, causing autostartStatus to report "shortcut points to // another executable" for any install path that contains non-ASCII chars. // New-Object System.Text.UTF8Encoding($false) is UTF-8 without BOM. script := `[Console]::OutputEncoding = New-Object System.Text.UTF8Encoding($false); $shell = New-Object -ComObject WScript.Shell; $shortcut = $shell.CreateShortcut($env:GOSENTRY_SHORTCUT_PATH); [Console]::Out.Write($shortcut.TargetPath + [Environment]::NewLine + $shortcut.Arguments)` diff --git a/src/platform/autostart/autostart_windows_test.go b/src/platform/autostart/autostart_windows_test.go index a4c7a8e..15811ff 100644 --- a/src/platform/autostart/autostart_windows_test.go +++ b/src/platform/autostart/autostart_windows_test.go @@ -134,7 +134,7 @@ func TestAutostartStatusRequiresMatchingTrayFlag(t *testing.T) { t.Fatalf("create shortcut: %v", err) } - ok, message := AutostartStatus(true, false, targetPath) + ok, message := autostartStatus(true, false, targetPath) if ok { t.Fatalf("expected problem when tray flag mismatches, got OK: %s", message) } diff --git a/src/runner/cleanup.go b/src/runner/cleanup.go index 04571f7..d839ccd 100644 --- a/src/runner/cleanup.go +++ b/src/runner/cleanup.go @@ -9,6 +9,11 @@ import ( "time" ) +// CleanupLogs enforces the count and age retention policies on the .log files +// in logsDir. maxFiles <= 0 disables the count policy and maxAgeDays <= 0 +// disables the age policy, independently — "keep everything" is a value the +// user can choose in Settings, not just an internal default (STANDARDS +// §Intentional behavior). func CleanupLogs(logsDir string, maxFiles int, maxAgeDays int) error { entries, err := os.ReadDir(logsDir) if err != nil { diff --git a/src/storage/store.go b/src/storage/store.go index a8f9891..eecb14a 100644 --- a/src/storage/store.go +++ b/src/storage/store.go @@ -144,12 +144,12 @@ func loadOrCreateConfig(paths Paths) (domain.Config, error) { if strings.TrimSpace(config.LogsDir) == "" { config.LogsDir = "logs" } - if config.MaxLogFiles <= 0 { - config.MaxLogFiles = 100 - } - if config.MaxLogAgeDays <= 0 { - config.MaxLogAgeDays = 30 - } + // MaxLogFiles and MaxLogAgeDays are deliberately not normalized: 0 means + // "keep everything" (see runner.CleanupLogs), not a missing value, so + // backfilling it here would make that choice impossible to persist. A config + // written before either field existed already carries 0 from json.Unmarshal + // leaving the DefaultConfig() value in config untouched, so old files still + // pick up 100 / 30 without an explicit backfill. if config.ExecutionMode == "" { config.ExecutionMode = domain.ExecutionModeParallel } diff --git a/src/storage/store_test.go b/src/storage/store_test.go index 07ddcae..231987b 100644 --- a/src/storage/store_test.go +++ b/src/storage/store_test.go @@ -183,6 +183,35 @@ func TestLoadOrCreateConfigCreatesDefaultsOnFirstRun(t *testing.T) { } } +// TestLoadOrCreateConfigPreservesZeroRetentionLimits verifies that 0 in +// max_log_files / max_log_age_days is read back as 0 ("keep everything"), not +// backfilled to the 100 / 30 defaults, since a config that already has the +// field set is not the "field is missing" case loadOrCreateConfig backfills. +func TestLoadOrCreateConfigPreservesZeroRetentionLimits(t *testing.T) { + dir := t.TempDir() + paths := Paths{ + AppDir: dir, + ConfigPath: filepath.Join(dir, ConfigFileName), + } + want := domain.DefaultConfig() + want.MaxLogFiles = 0 + want.MaxLogAgeDays = 0 + if err := writeJSON(paths.ConfigPath, want); err != nil { + t.Fatal(err) + } + + got, err := loadOrCreateConfig(paths) + if err != nil { + t.Fatal(err) + } + if got.MaxLogFiles != 0 { + t.Errorf("MaxLogFiles: got %d, want 0 (unlimited)", got.MaxLogFiles) + } + if got.MaxLogAgeDays != 0 { + t.Errorf("MaxLogAgeDays: got %d, want 0 (unlimited)", got.MaxLogAgeDays) + } +} + // TestLoadOrCreateJobsSeedsSampleJobsOnFirstRun verifies that a missing // jobs.json is created with the sample jobs from defaultJobs, so a new user // sees scheduled and manual execution without inventing a command. diff --git a/src/ui/jobs_view.go b/src/ui/jobs_view.go index 949f018..f309694 100644 --- a/src/ui/jobs_view.go +++ b/src/ui/jobs_view.go @@ -57,13 +57,14 @@ func newJobsView(w fyne.Window, svc *app.Service) (fyne.CanvasObject, func()) { selected = -1 } selectedFolder := allFolders - schedulerPaused := svc.Store().Config.Paused - listView := svc.Store().Config.JobListView + initialConfig := svc.Config() + schedulerPaused := initialConfig.Paused + listView := initialConfig.JobListView filteredJobs := filteredJobIndexes(jobs, selectedFolder) - dp := newDetailsPanel(job{}, &domain.JobRuntime{}, svc.Store().Config.OverlapPolicy, svc.Store().Config.DefaultTimeoutSeconds) + dp := newDetailsPanel(job{}, &domain.JobRuntime{}, initialConfig.OverlapPolicy, initialConfig.DefaultTimeoutSeconds) if selected >= 0 { - dp.update(jobs[selected], runtimeFor(selected), svc.Store().Config.OverlapPolicy, svc.Store().Config.DefaultTimeoutSeconds) + dp.update(jobs[selected], runtimeFor(selected), initialConfig.OverlapPolicy, initialConfig.DefaultTimeoutSeconds) } else { dp.clear() } @@ -76,16 +77,28 @@ func newJobsView(w fyne.Window, svc *app.Service) (fyne.CanvasObject, func()) { return } selected = index - dp.update(jobs[selected], runtimeFor(selected), svc.Store().Config.OverlapPolicy, svc.Store().Config.DefaultTimeoutSeconds) + // Overlap policy and the default timeout are global settings that can + // change from the Settings tab while this view is open, so they are + // re-read on every update rather than captured once at construction. + config := svc.Config() + dp.update(jobs[selected], runtimeFor(selected), config.OverlapPolicy, config.DefaultTimeoutSeconds) } - // list and folderSelect are declared early so closures below can reference - // them before the widget.NewList / widget.NewSelect calls assign the values. + // list, folderSelect, and applySchedulerState are declared early so closures + // below can reference them before the widgets that assign the values exist. var list *widget.List var folderSelect *widget.Select + var applySchedulerState func(bool) refreshView := func() { syncFromService() + if applySchedulerState != nil { + // The pause state is Service-owned and can change from outside this + // view (Settings has no such control today, but the event that + // reports it — SchedulerStateChanged — is consumed here rather than + // relying solely on the tap handler's own mirror). + applySchedulerState(svc.Config().Paused) + } filteredJobs = filteredJobIndexes(jobs, selectedFolder) updateDetails(selected) dp.logs.Refresh() @@ -258,26 +271,15 @@ func newJobsView(w fyne.Window, svc *app.Service) (fyne.CanvasObject, func()) { refreshView() }) - stopAllText, stopAllIcon := "Disable auto", theme.MediaPauseIcon() - if schedulerPaused { - stopAllText, stopAllIcon = "Enable auto", theme.MediaPlayIcon() - } - schedulerStateText := "Scheduler running" - if schedulerPaused { - schedulerStateText = "Scheduler paused" - } - schedulerState := widget.NewLabel(schedulerStateText) - stopAllButton := widget.NewButtonWithIcon(stopAllText, stopAllIcon, nil) - stopAllButton.OnTapped = func() { - // SetGlobalPause flips the pause flag, updates every job's next-run text, - // and emits the activity record the observer logs. Revert if the save fails. - schedulerPaused = !schedulerPaused - if err := svc.SetGlobalPause(schedulerPaused); err != nil { - schedulerPaused = !schedulerPaused - dialog.ShowError(err, w) - return - } - if schedulerPaused { + schedulerState := widget.NewLabel("") + stopAllButton := widget.NewButtonWithIcon("", nil, nil) + // applySchedulerState is the one place that draws the pause control and its + // status text from a pause value, so refreshView can drive it from whatever + // the Service reports (including a SchedulerStateChanged the general refresh + // picks up) instead of only the tap handler mirroring its own toggle. + applySchedulerState = func(paused bool) { + schedulerPaused = paused + if paused { schedulerState.SetText("Scheduler paused") stopAllButton.SetText("Enable auto") stopAllButton.SetIcon(theme.MediaPlayIcon()) @@ -286,6 +288,17 @@ func newJobsView(w fyne.Window, svc *app.Service) (fyne.CanvasObject, func()) { stopAllButton.SetText("Disable auto") stopAllButton.SetIcon(theme.MediaPauseIcon()) } + } + applySchedulerState(schedulerPaused) + stopAllButton.OnTapped = func() { + // SetGlobalPause flips the pause flag, updates every job's next-run text, + // and emits the activity record the observer logs. Revert if the save fails. + if err := svc.SetGlobalPause(!schedulerPaused); err != nil { + dialog.ShowError(err, w) + return + } + // refreshView re-derives the pause state from the Service (see + // applySchedulerState above), so it is the single place that draws it. refreshView() } pauseButton := widget.NewButtonWithIcon("Pause", theme.MediaPauseIcon(), func() { diff --git a/src/ui/mainwindow.go b/src/ui/mainwindow.go index 62311e7..e4c6a62 100644 --- a/src/ui/mainwindow.go +++ b/src/ui/mainwindow.go @@ -65,43 +65,42 @@ func newMainView(w fyne.Window, svc *app.Service) (fyne.CanvasObject, func(time. // the main thread in both cases, so the engine never mutates Fyne state off // the UI thread. This is the sole place events touch widgets. (Resolves #4.) 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.add(recorded.Record) - r := recorded.Record - if r.State == "Failed" && - (r.Trigger == "Manual" || r.Trigger == "Schedule") && + // A type switch does not get compiler-enforced exhaustiveness (see + // app.Event's doc comment) — JobChanged and SchedulerStateChanged + // intentionally fall through to the unconditional refresh() below + // without their own case, since a broad state re-read is all they need. + switch e := ev.(type) { + case app.RunRecorded: + events.add(e.Record) + if e.Record.State == "Failed" && + (e.Record.Trigger == "Manual" || e.Record.Trigger == "Schedule") && svc.ShouldNotifyOnFailure() { timing := notificationTiming{ - JobName: r.JobName, + JobName: e.Record.JobName, EmittedAt: time.Now(), } - if finished, err := time.ParseInLocation(runRecordTimeLayout, r.Time, time.Local); err == nil { + if finished, err := time.ParseInLocation(runRecordTimeLayout, e.Record.Time, time.Local); err == nil { timing.RunFinished = finished } fyne.Do(func() { timing.UIQueuedAt = time.Now() fyne.CurrentApp().SendNotification(&fyne.Notification{ Title: "GoSentry: Job Failed", - Content: r.JobName + ": " + r.Detail, + Content: e.Record.JobName + ": " + e.Record.Detail, }) timing.AfterSendAt = time.Now() - if err := appendNotificationTimingLog(svc.Store().Paths.LogsDir, timing); err != nil { + if err := appendNotificationTimingLog(svc.Paths().LogsDir, timing); err != nil { fyne.LogError("Failed to write notification timing log", err) } }) } - } - if isError { - events.add(newEvent(0, "Service", "Error", errOccurred.Err.Error())) - } - if isJobsLoaded { + case app.ErrorOccurred: + events.add(newEvent(0, "Service", "Error", e.Err.Error())) + case app.JobsLoaded: // 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 + detail := strconv.Itoa(e.Count) + " jobs from " + e.Path events.add(newEvent(0, "Service", "Jobs loaded", detail)) } refresh() diff --git a/src/ui/run.go b/src/ui/run.go index a57668d..5430f2f 100644 --- a/src/ui/run.go +++ b/src/ui/run.go @@ -70,12 +70,13 @@ func Run(startInTray bool) { a.Run() return } - keepInTray = svc.Store().Config.KeepRunningInTray + config := svc.Config() + keepInTray = config.KeepRunningInTray startHidden = resolveStartHidden(startInTray, keepInTray) applyTrayBehavior(a, w, keepInTray, false) // Apply the persisted theme before building content so the window renders in // the chosen theme from the first frame rather than flashing the default one. - applyTheme(a, svc.Store().Config.Theme) + applyTheme(a, config.Theme) content, recordStartup := newMainView(w, svc) w.SetContent(content) serveSingleInstance(instanceListener, w) diff --git a/src/ui/settings_view.go b/src/ui/settings_view.go index 142fb39..600d88b 100644 --- a/src/ui/settings_view.go +++ b/src/ui/settings_view.go @@ -27,7 +27,14 @@ var settingsCaptions = []string{ } func settingsView(w fyne.Window, svc *app.Service) fyne.CanvasObject { - store := svc.Store() + // saved mirrors the config as last persisted (or freshly loaded at + // construction); it is a local copy the closures below compare the form + // against and reassign after a successful save, rather than holding onto + // the live *storage.Store the Service owns (see app.Service.Config). + // paths never changes after construction of this view — AppDir and + // ConfigPath are fixed for the process — so it is read once, not refreshed. + saved := svc.Config() + paths := svc.Paths() // updateSaveState compares the form to the saved config and enables Save only // when something differs. It is defined below (once Save and every field // exist) but declared here so the field change handlers can reference it. @@ -36,14 +43,14 @@ func settingsView(w fyne.Window, svc *app.Service) fyne.CanvasObject { // both the initial load and the Cancel/Defaults buttons below. var loadFields func(domain.Config) startOnLogin := widget.NewCheck("Start on login", nil) - startOnLogin.SetChecked(store.Config.StartOnLogin) + startOnLogin.SetChecked(saved.StartOnLogin) minimizeToTray := widget.NewCheck("Keep running in the system tray", nil) - minimizeToTray.SetChecked(store.Config.KeepRunningInTray) + minimizeToTray.SetChecked(saved.KeepRunningInTray) autostartStatus := widget.NewLabel("") trayRestartHint := widget.NewLabel("") trayRestartHint.Truncation = fyne.TextTruncateClip refreshAutostartStatus := func() { - if settingsPendingAutostart(startOnLogin, minimizeToTray, store.Config) { + if settingsPendingAutostart(startOnLogin, minimizeToTray, saved) { autostartStatus.SetText("Pending: save settings to apply") return } @@ -67,15 +74,15 @@ func settingsView(w fyne.Window, svc *app.Service) fyne.CanvasObject { } minimizeToTray.OnChanged = func(bool) { refreshAutostartStatus() - refreshTrayRestartHint(minimizeToTray.Checked != store.Config.KeepRunningInTray) + refreshTrayRestartHint(minimizeToTray.Checked != saved.KeepRunningInTray) updateSaveState() } refreshAutostartStatus() notifications := widget.NewCheck("Show desktop notifications for failed jobs", nil) - notifications.SetChecked(store.Config.NotifyOnFailure) + notifications.SetChecked(saved.NotifyOnFailure) notifications.OnChanged = func(bool) { updateSaveState() } themeSelect := widget.NewSelect([]string{themeLabelSystem, themeLabelGoSentry}, nil) - themeSelect.SetSelected(themeLabel(store.Config.Theme)) + themeSelect.SetSelected(themeLabel(saved.Theme)) // Preview the theme the moment it is picked so the choice is visible before // saving; Save persists it. Reverting the selection reverts the preview, and // closing without saving falls back to the stored theme on next launch. @@ -87,20 +94,20 @@ func settingsView(w fyne.Window, svc *app.Service) fyne.CanvasObject { []string{string(domain.ExecutionModeParallel), string(domain.ExecutionModeSequential)}, nil, ) - executionModeSelect.SetSelected(string(store.Config.ExecutionMode)) + executionModeSelect.SetSelected(string(saved.ExecutionMode)) executionModeSelect.OnChanged = func(string) { updateSaveState() } overlapPolicySelect := widget.NewSelect( []string{string(domain.OverlapPolicySkip), string(domain.OverlapPolicyQueue)}, nil, ) - overlapPolicySelect.SetSelected(string(store.Config.OverlapPolicy)) + overlapPolicySelect.SetSelected(string(saved.OverlapPolicy)) overlapPolicySelect.OnChanged = func(string) { updateSaveState() } defaultTimeout := widget.NewEntry() defaultTimeout.SetPlaceHolder("0 = no timeout") - defaultTimeout.SetText(strconv.Itoa(store.Config.DefaultTimeoutSeconds)) + defaultTimeout.SetText(strconv.Itoa(saved.DefaultTimeoutSeconds)) defaultTimeout.OnChanged = func(string) { updateSaveState() } jobsFile := widget.NewEntry() - jobsFile.SetText(store.Config.JobsFile) + jobsFile.SetText(saved.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. @@ -108,7 +115,7 @@ func settingsView(w fyne.Window, svc *app.Service) fyne.CanvasObject { chooseJSONFile(w, jobsFile) }) logsDir := widget.NewEntry() - logsDir.SetText(store.Config.LogsDir) + logsDir.SetText(saved.LogsDir) logsDir.OnChanged = func(string) { updateSaveState() } logsDirBrowse := widget.NewButtonWithIcon("Browse", theme.FolderOpenIcon(), func() { chooseFolder(w, logsDir) @@ -118,13 +125,15 @@ func settingsView(w fyne.Window, svc *app.Service) fyne.CanvasObject { // manager. It reveals whatever the field currently holds, so an edit can be // checked before Save. logsDirOpen := widget.NewButtonWithIcon("Open", theme.FolderIcon(), func() { - openFolder(w, settingsFolderPath(store.Paths.AppDir, logsDir.Text)) + openFolder(w, settingsFolderPath(paths.AppDir, logsDir.Text)) }) maxLogFiles := widget.NewEntry() - maxLogFiles.SetText(strconv.Itoa(store.Config.MaxLogFiles)) + maxLogFiles.SetPlaceHolder("0 = unlimited") + maxLogFiles.SetText(strconv.Itoa(saved.MaxLogFiles)) maxLogFiles.OnChanged = func(string) { updateSaveState() } maxLogAgeDays := widget.NewEntry() - maxLogAgeDays.SetText(strconv.Itoa(store.Config.MaxLogAgeDays)) + maxLogAgeDays.SetPlaceHolder("0 = unlimited") + maxLogAgeDays.SetText(strconv.Itoa(saved.MaxLogAgeDays)) maxLogAgeDays.OnChanged = func(string) { updateSaveState() } // Autostart status sits on its own row beneath the checkbox (rather than // beside it) so the Application section fits within a half-width column. @@ -134,13 +143,13 @@ func settingsView(w fyne.Window, svc *app.Service) fyne.CanvasObject { saveSettings := widget.NewButtonWithIcon("Save settings", theme.DocumentSaveIcon(), func() { files, err := strconv.Atoi(strings.TrimSpace(maxLogFiles.Text)) - if err != nil || files <= 0 { - settingsStatus.SetText("Max log files must be a positive number") + if err != nil || files < 0 { + settingsStatus.SetText("Max log files must be zero (unlimited) or a positive number") return } days, err := strconv.Atoi(strings.TrimSpace(maxLogAgeDays.Text)) - if err != nil || days <= 0 { - settingsStatus.SetText("Max log age days must be a positive number") + if err != nil || days < 0 { + settingsStatus.SetText("Max log age days must be zero (unlimited) or a positive number") return } if strings.TrimSpace(jobsFile.Text) == "" { @@ -159,7 +168,7 @@ func settingsView(w fyne.Window, svc *app.Service) fyne.CanvasObject { // Build the new config from the form and hand it to the Service, which // 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 := saved config.JobsFile = strings.TrimSpace(jobsFile.Text) config.LogsDir = strings.TrimSpace(logsDir.Text) config.MaxLogFiles = files @@ -171,11 +180,16 @@ func settingsView(w fyne.Window, svc *app.Service) fyne.CanvasObject { config.OverlapPolicy = domain.OverlapPolicy(overlapPolicySelect.Selected) config.DefaultTimeoutSeconds = timeout config.Theme = themeFromLabel(themeSelect.Selected) - previousKeepInTray := store.Config.KeepRunningInTray + previousKeepInTray := saved.KeepRunningInTray if err := svc.UpdateSettings(config); err != nil { settingsStatus.SetText("Save failed: " + err.Error()) return } + // UpdateSettings may re-resolve paths (a jobs-file switch adopts a + // different directory), so pick up the fresh copy rather than assuming + // config is exactly what landed. + saved = svc.Config() + paths = svc.Paths() if err := svc.ApplyAutostart(); err != nil { refreshAutostartStatus() settingsStatus.SetText("Saved, autostart failed: " + err.Error()) @@ -198,7 +212,7 @@ func settingsView(w fyne.Window, svc *app.Service) fyne.CanvasObject { // fields compare against their canonical string form; any unparsable text // counts as a change so the user can click Save and see the validation error. updateSaveState = func() { - c := store.Config + c := saved changed := startOnLogin.Checked != c.StartOnLogin || minimizeToTray.Checked != c.KeepRunningInTray || notifications.Checked != c.NotifyOnFailure || @@ -235,17 +249,17 @@ func settingsView(w fyne.Window, svc *app.Service) fyne.CanvasObject { logsDir.SetText(c.LogsDir) maxLogFiles.SetText(strconv.Itoa(c.MaxLogFiles)) maxLogAgeDays.SetText(strconv.Itoa(c.MaxLogAgeDays)) - if settingsPendingAutostart(startOnLogin, minimizeToTray, store.Config) { + if settingsPendingAutostart(startOnLogin, minimizeToTray, saved) { autostartStatus.SetText("Pending: save settings to apply") } else { refreshAutostartStatus() } - refreshTrayRestartHint(minimizeToTray.Checked != store.Config.KeepRunningInTray) + refreshTrayRestartHint(minimizeToTray.Checked != saved.KeepRunningInTray) settingsStatus.SetText("") updateSaveState() } cancelSettings := widget.NewButtonWithIcon("Cancel", theme.CancelIcon(), func() { - loadFields(store.Config) + loadFields(saved) }) restoreDefaults := widget.NewButtonWithIcon("Defaults", theme.MediaReplayIcon(), func() { loadFields(domain.DefaultConfig()) @@ -261,7 +275,7 @@ func settingsView(w fyne.Window, svc *app.Service) fyne.CanvasObject { executionModeSelect: executionModeSelect, overlapPolicySelect: overlapPolicySelect, defaultTimeout: defaultTimeout, - configPath: store.Paths.ConfigPath, + configPath: paths.ConfigPath, jobsFile: jobsFile, jobsFileBrowse: jobsFileBrowse, logsDir: logsDir,