diff --git a/docs/REFACTORING.md b/docs/REFACTORING.md index 47015da..0134cf9 100644 --- a/docs/REFACTORING.md +++ b/docs/REFACTORING.md @@ -261,7 +261,7 @@ Track progress here. Mark tasks complete as they land and pass review. ### Phase 3 — Application service layer - [x] T3.1 — Create `src/app/service.go`; owns state behind mutex -- [ ] T3.2 — Add `src/app/events.go`; Event types + Observer +- [x] T3.2 — Add `src/app/events.go`; Event types + Observer - [ ] T3.3 — Add state-mutating operations to service - [ ] T3.4 — Convert `scheduler` to use service; inject Clock - [ ] T3.5 — Move display helpers to `src/app/format.go` diff --git a/src/app/events.go b/src/app/events.go new file mode 100644 index 0000000..6466ae8 --- /dev/null +++ b/src/app/events.go @@ -0,0 +1,91 @@ +package app + +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. +// +// Events replace the old single onChange callback. Instead of the scheduler +// reaching into the GUI, the Service emits typed events and the UI subscribes — +// the UI's listener becomes the one place that touches widgets. +type Event interface { + isEvent() +} + +// JobChanged signals that a job's durable config or transient runtime changed: +// created, edited, deleted, enabled/disabled, or a status transition such as a +// run starting. Observers should re-read the affected state through the Service +// (Jobs/Runtime) rather than expect a payload snapshot — that keeps the event +// small and avoids handing out stale copies. +// +// JobID identifies the affected job. A zero JobID means a broad change (for +// example a delete, or a global pause that touched every job) and observers +// should refresh their whole view. +type JobChanged struct { + JobID int +} + +// RunRecorded signals that a job run finished and produced a RunRecord. It +// carries the record by value because the record is an immutable result that +// observers append to history; there is nothing for them to re-read. +type RunRecorded struct { + Record domain.RunRecord +} + +// SchedulerStateChanged signals that the global scheduler pause state flipped. +// The UI uses it to update the pause/resume control and status text. +type SchedulerStateChanged struct { + Paused bool +} + +func (JobChanged) isEvent() {} +func (RunRecorded) isEvent() {} +func (SchedulerStateChanged) isEvent() {} + +// Observer receives events emitted by the Service. OnEvent is the single +// reaction point; the UI implements it and marshals any widget work onto the +// main thread (fyne.Do) itself — the Service knows nothing about Fyne. +type Observer interface { + OnEvent(Event) +} + +// ObserverFunc adapts a plain function to the Observer interface, so callers can +// subscribe a closure without declaring a type. +type ObserverFunc func(Event) + +// OnEvent calls the wrapped function. +func (f ObserverFunc) OnEvent(event Event) { f(event) } + +// Subscribe registers an observer to receive every subsequently emitted event. +// Registration is expected during setup, before the scheduler starts, but is +// guarded so it is safe at any time. +func (s *Service) Subscribe(observer Observer) { + s.dispatchMu.Lock() + defer s.dispatchMu.Unlock() + s.observers = append(s.observers, observer) +} + +// emit delivers an event to every registered observer. +// +// Single-threaded dispatch contract: +// - emit holds dispatchMu for the whole dispatch, so observers are never +// invoked concurrently and never overlap with each other or with Subscribe. +// Each observer sees events one at a time, in emit order. +// - emit must be called WITHOUT holding s.mu. The Service computes a state +// change under mu, releases it, then emits — so an observer is free to call +// back into read methods (Jobs/Runtime) without deadlocking on the state +// lock. +// - An observer must NOT call back into a Service method that emits (directly +// or indirectly): dispatchMu is non-reentrant, so re-entrant emission would +// deadlock. Observers react and return quickly; long or UI work is the +// observer's own responsibility to defer (e.g. fyne.Do). +func (s *Service) emit(event Event) { + s.dispatchMu.Lock() + defer s.dispatchMu.Unlock() + for _, observer := range s.observers { + observer.OnEvent(event) + } +} diff --git a/src/app/events_test.go b/src/app/events_test.go new file mode 100644 index 0000000..1c84abf --- /dev/null +++ b/src/app/events_test.go @@ -0,0 +1,59 @@ +package app + +import ( + "testing" + + "gitea.mixdep.ru/mix/gosentry/src/domain" +) + +func TestEmitDeliversToAllObserversInOrder(t *testing.T) { + svc := newTestService(nil) + + var first, second []Event + svc.Subscribe(ObserverFunc(func(e Event) { first = append(first, e) })) + svc.Subscribe(ObserverFunc(func(e Event) { second = append(second, e) })) + + svc.emit(JobChanged{JobID: 7}) + svc.emit(RunRecorded{Record: domain.RunRecord{JobID: 7, State: "Success"}}) + svc.emit(SchedulerStateChanged{Paused: true}) + + for name, got := range map[string][]Event{"first": first, "second": second} { + if len(got) != 3 { + t.Fatalf("%s observer got %d events, want 3", name, len(got)) + } + if jc, ok := got[0].(JobChanged); !ok || jc.JobID != 7 { + t.Errorf("%s event[0] = %#v, want JobChanged{JobID:7}", name, got[0]) + } + if rr, ok := got[1].(RunRecorded); !ok || rr.Record.State != "Success" { + t.Errorf("%s event[1] = %#v, want RunRecorded Success", name, got[1]) + } + if ss, ok := got[2].(SchedulerStateChanged); !ok || !ss.Paused { + t.Errorf("%s event[2] = %#v, want SchedulerStateChanged{Paused:true}", name, got[2]) + } + } +} + +func TestEmitWithNoObserversIsNoop(t *testing.T) { + svc := newTestService(nil) + // Must not panic with an empty observer list. + svc.emit(JobChanged{}) +} + +// Observers may read Service state from within OnEvent without deadlocking, +// because emit is called outside the state lock. +func TestObserverCanReadServiceState(t *testing.T) { + jobs := []domain.Job{{ID: 1, Name: "Job", Enabled: true}} + svc := newTestService(jobs) + + var sawName string + svc.Subscribe(ObserverFunc(func(Event) { + if snapshot := svc.Jobs(); len(snapshot) == 1 { + sawName = snapshot[0].Name + } + })) + + svc.emit(JobChanged{JobID: 1}) + if sawName != "Job" { + t.Errorf("observer read name = %q, want %q", sawName, "Job") + } +} diff --git a/src/app/service.go b/src/app/service.go index 196eaed..c8bda47 100644 --- a/src/app/service.go +++ b/src/app/service.go @@ -27,6 +27,12 @@ type Service struct { store *storage.Store jobs []domain.Job runtimes map[int]*domain.JobRuntime + + // observers and their guard live in events.go. dispatchMu is separate from mu + // so that emitting an event never requires (or is held under) the state lock: + // the Service must release mu before dispatching, per the locking contract. + dispatchMu sync.Mutex + observers []Observer } // NewService wires the Service to a loaded store and its jobs. It builds the