diff --git a/data/.gitignore b/data/.gitignore index 116f5ec..f05d7a5 100644 --- a/data/.gitignore +++ b/data/.gitignore @@ -5,5 +5,6 @@ default/ dns/ interactions/ parity_mismatches/ +stats/ patterns.json settings.json diff --git a/pkg/models/models.go b/pkg/models/models.go index 4485061..783db40 100644 --- a/pkg/models/models.go +++ b/pkg/models/models.go @@ -672,6 +672,21 @@ type ErrorStats struct { Details string `json:"details,omitempty" xml:"details,omitempty"` } +// ActivityRecord is one entry in AfterTouch's local, append-only admin-UI +// activity log (e.g. an announcement banner dismissal). Local-only: written +// to plain JSON on disk, never transmitted automatically — the only way it +// leaves the operator's network is an explicitly-triggered diagnostic +// export. The same ID can recur with a new Timestamp (e.g. a dismissed +// notification shown and dismissed again later); this is a log, not a +// keyed map. Intentionally generic so it can back other admin-UI action +// kinds beyond dismissals later, not just this one feature. +type ActivityRecord struct { + Kind string `json:"kind"` + ID string `json:"id"` + Timestamp string `json:"timestamp"` + Detail map[string]interface{} `json:"detail,omitempty"` +} + // DeviceEvent represents an event that occurred on a device. type DeviceEvent struct { Type string `json:"type"` diff --git a/pkg/service/datastore/datastore.go b/pkg/service/datastore/datastore.go index 0491615..5abe8dd 100644 --- a/pkg/service/datastore/datastore.go +++ b/pkg/service/datastore/datastore.go @@ -2738,6 +2738,82 @@ func (ds *DataStore) SaveUsageStats(stats models.UsageStats) error { return ds.atomicWriteFile(path, data) } +// RecordActivity appends one entry to the local admin-UI activity log, under +// DataDir/stats/activity//, one file per event (same shape as +// SaveUsageStats/SaveErrorStats above). kind is meant to be a small, +// developer-defined constant (e.g. "notification_dismissed") used directly +// as a directory name — callers must not pass untrusted/user-supplied +// values. id may recur across calls with a new timestamp each time; this is +// an append-only log, not a keyed store. See models.ActivityRecord for the +// local-only/never-transmitted-automatically guarantee this backs. +func (ds *DataStore) RecordActivity(kind, id string, detail map[string]interface{}) error { + dir := filepath.Join(ds.DataDir, "stats", "activity", kind) + if err := ds.rootMkdirAll(dir, 0755); err != nil { + return err + } + + now := time.Now() + record := models.ActivityRecord{ + Kind: kind, + ID: id, + Timestamp: now.UTC().Format(time.RFC3339Nano), + Detail: detail, + } + + // The random suffix guards against two events for the same id landing in + // the same nanosecond (observed as flaky on coarser-resolution clocks) + // silently overwriting one another instead of both being recorded. + filename := fmt.Sprintf("%d_%d_%s.json", now.UnixNano(), rand.Int63n(1_000_000), id) //nolint:gosec + path := filepath.Join(dir, filename) + + data, err := json.MarshalIndent(record, "", " ") + if err != nil { + return err + } + + return ds.atomicWriteFile(path, data) +} + +// GetActivityRecords reads back every entry recorded via RecordActivity for +// the given kind. Unreadable or malformed files are skipped rather than +// failing the whole read — a single corrupt event shouldn't make the rest of +// the log unreadable. Returns an empty slice (not an error) when the +// directory doesn't exist yet, matching the "nothing recorded yet" case. +func (ds *DataStore) GetActivityRecords(kind string) ([]models.ActivityRecord, error) { + dir := filepath.Join(ds.DataDir, "stats", "activity", kind) + + entries, err := ds.rootReadDir(dir) + if err != nil { + if os.IsNotExist(err) { + return nil, nil + } + + return nil, err + } + + records := make([]models.ActivityRecord, 0, len(entries)) + + for _, entry := range entries { + if entry.IsDir() { + continue + } + + data, readErr := ds.rootReadFile(filepath.Join(dir, entry.Name())) + if readErr != nil { + continue + } + + var record models.ActivityRecord + if unmarshalErr := json.Unmarshal(data, &record); unmarshalErr != nil { + continue + } + + records = append(records, record) + } + + return records, nil +} + // SaveErrorStats saves error statistics to the datastore. func (ds *DataStore) SaveErrorStats(stats models.ErrorStats) error { dir := filepath.Join(ds.DataDir, "stats", "error") diff --git a/pkg/service/datastore/datastore_test.go b/pkg/service/datastore/datastore_test.go index d7f727b..e857db8 100644 --- a/pkg/service/datastore/datastore_test.go +++ b/pkg/service/datastore/datastore_test.go @@ -458,6 +458,88 @@ func TestSettingsPersistence(t *testing.T) { } } +// TestRecordActivity_EmptyKindReturnsNilNotError verifies GetActivityRecords +// for a kind that was never recorded returns an empty, non-error result — +// the "nothing recorded yet" case, not a failure. +func TestRecordActivity_EmptyKindReturnsNilNotError(t *testing.T) { + tempDir, err := os.MkdirTemp("", "activity-empty-test-*") + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(tempDir) + + ds := NewDataStore(tempDir) + + records, err := ds.GetActivityRecords("notification_dismissed") + if err != nil { + t.Fatalf("GetActivityRecords on empty kind should not error, got: %v", err) + } + if len(records) != 0 { + t.Errorf("Expected no records, got %d", len(records)) + } +} + +// TestRecordActivity_SameIDRecursWithNewTimestamp is the regression test for +// the append-only shape agreed in the #419 design: dismissing the same +// announcement twice must produce two records, not overwrite one — this is +// a log, not a keyed map. +func TestRecordActivity_SameIDRecursWithNewTimestamp(t *testing.T) { + tempDir, err := os.MkdirTemp("", "activity-recur-test-*") + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(tempDir) + + ds := NewDataStore(tempDir) + + if err := ds.RecordActivity("notification_dismissed", "admin-gate-notice", nil); err != nil { + t.Fatalf("First RecordActivity failed: %v", err) + } + if err := ds.RecordActivity("notification_dismissed", "admin-gate-notice", nil); err != nil { + t.Fatalf("Second RecordActivity failed: %v", err) + } + + records, err := ds.GetActivityRecords("notification_dismissed") + if err != nil { + t.Fatalf("GetActivityRecords failed: %v", err) + } + if len(records) != 2 { + t.Fatalf("Expected 2 records for the same recurring id, got %d: %+v", len(records), records) + } + for _, r := range records { + if r.ID != "admin-gate-notice" || r.Kind != "notification_dismissed" || r.Timestamp == "" { + t.Errorf("Unexpected record shape: %+v", r) + } + } +} + +// TestRecordActivity_DetailRoundTrips verifies the optional detail payload +// survives a write/read round trip. +func TestRecordActivity_DetailRoundTrips(t *testing.T) { + tempDir, err := os.MkdirTemp("", "activity-detail-test-*") + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(tempDir) + + ds := NewDataStore(tempDir) + + if err := ds.RecordActivity("some_kind", "some-id", map[string]interface{}{"note": "hello"}); err != nil { + t.Fatalf("RecordActivity failed: %v", err) + } + + records, err := ds.GetActivityRecords("some_kind") + if err != nil { + t.Fatalf("GetActivityRecords failed: %v", err) + } + if len(records) != 1 { + t.Fatalf("Expected 1 record, got %d", len(records)) + } + if records[0].Detail["note"] != "hello" { + t.Errorf("Expected detail to round-trip, got: %+v", records[0].Detail) + } +} + func TestMoveDeviceMigratesData(t *testing.T) { tempDir := t.TempDir() ds := NewDataStore(tempDir) diff --git a/pkg/service/handlers/handlers_activity_test.go b/pkg/service/handlers/handlers_activity_test.go new file mode 100644 index 0000000..06122b7 --- /dev/null +++ b/pkg/service/handlers/handlers_activity_test.go @@ -0,0 +1,124 @@ +package handlers + +import ( + "os" + "testing" + + "github.com/gesellix/bose-soundtouch/pkg/service/datastore" +) + +// TestIsAnnouncementDismissed_EmptyByDefault verifies a freshly-constructed +// server (no prior activity log) reports nothing as dismissed. +func TestIsAnnouncementDismissed_EmptyByDefault(t *testing.T) { + tempDir, err := os.MkdirTemp("", "dismissal-empty-test") + if err != nil { + t.Fatalf("Failed to create temp dir: %v", err) + } + defer os.RemoveAll(tempDir) + + ds := datastore.NewDataStore(tempDir) + _ = ds.Initialize() + + server := NewServer(ds, nil, "http://127.0.0.1:8000", false, false, false) + + if server.IsAnnouncementDismissed("admin-gate-notice") { + t.Error("Expected no announcement to be dismissed on a fresh install") + } +} + +// TestRecordDismissal_UpdatesCacheAndPersists is a regression test for the +// #419 design's performance requirement: after RecordDismissal, the +// in-memory cache must reflect it immediately (no disk re-read needed), and +// it must also be durably persisted via the activity log. +func TestRecordDismissal_UpdatesCacheAndPersists(t *testing.T) { + tempDir, err := os.MkdirTemp("", "dismissal-record-test") + if err != nil { + t.Fatalf("Failed to create temp dir: %v", err) + } + defer os.RemoveAll(tempDir) + + ds := datastore.NewDataStore(tempDir) + _ = ds.Initialize() + + server := NewServer(ds, nil, "http://127.0.0.1:8000", false, false, false) + + if err := server.RecordDismissal("admin-gate-notice"); err != nil { + t.Fatalf("RecordDismissal failed: %v", err) + } + + if !server.IsAnnouncementDismissed("admin-gate-notice") { + t.Error("Expected admin-gate-notice to be dismissed after RecordDismissal") + } + + records, err := ds.GetActivityRecords(activityKindNotificationDismissed) + if err != nil { + t.Fatalf("GetActivityRecords failed: %v", err) + } + if len(records) != 1 || records[0].ID != "admin-gate-notice" { + t.Errorf("Expected exactly 1 persisted dismissal record, got: %+v", records) + } +} + +// TestLoadDismissedAnnouncements_ReadsPriorHistoryAtStartup verifies a +// restarted server picks up dismissals recorded in a previous run — the +// startup scan, not just the live write-through path. +func TestLoadDismissedAnnouncements_ReadsPriorHistoryAtStartup(t *testing.T) { + tempDir, err := os.MkdirTemp("", "dismissal-startup-test") + if err != nil { + t.Fatalf("Failed to create temp dir: %v", err) + } + defer os.RemoveAll(tempDir) + + ds := datastore.NewDataStore(tempDir) + _ = ds.Initialize() + + // Simulate a dismissal recorded in a prior run, before this process's + // Server ever existed. + if err := ds.RecordActivity(activityKindNotificationDismissed, "admin-gate-notice", nil); err != nil { + t.Fatalf("Seeding activity record failed: %v", err) + } + + server := NewServer(ds, nil, "http://127.0.0.1:8000", false, false, false) + + if !server.IsAnnouncementDismissed("admin-gate-notice") { + t.Error("Expected startup scan to pick up a dismissal recorded in a prior run") + } + if server.IsAnnouncementDismissed("some-other-notice") { + t.Error("Expected an unrelated id to not be reported as dismissed") + } +} + +// TestRecordDismissal_SameIDTwiceAppendsBothKeepsCacheSane verifies dismissing +// the same announcement twice (e.g. re-shown, dismissed again) appends two +// log entries but the in-memory cache still reports it dismissed exactly +// once (a boolean check, not a count). +func TestRecordDismissal_SameIDTwiceAppendsBothKeepsCacheSane(t *testing.T) { + tempDir, err := os.MkdirTemp("", "dismissal-recur-test") + if err != nil { + t.Fatalf("Failed to create temp dir: %v", err) + } + defer os.RemoveAll(tempDir) + + ds := datastore.NewDataStore(tempDir) + _ = ds.Initialize() + + server := NewServer(ds, nil, "http://127.0.0.1:8000", false, false, false) + + if err := server.RecordDismissal("admin-gate-notice"); err != nil { + t.Fatalf("First RecordDismissal failed: %v", err) + } + if err := server.RecordDismissal("admin-gate-notice"); err != nil { + t.Fatalf("Second RecordDismissal failed: %v", err) + } + + records, err := ds.GetActivityRecords(activityKindNotificationDismissed) + if err != nil { + t.Fatalf("GetActivityRecords failed: %v", err) + } + if len(records) != 2 { + t.Errorf("Expected 2 append-only log entries for a recurring dismissal, got %d", len(records)) + } + if !server.IsAnnouncementDismissed("admin-gate-notice") { + t.Error("Expected admin-gate-notice to still be reported dismissed") + } +} diff --git a/pkg/service/handlers/server.go b/pkg/service/handlers/server.go index 9888f01..dfb8058 100644 --- a/pkg/service/handlers/server.go +++ b/pkg/service/handlers/server.go @@ -68,7 +68,8 @@ type Server struct { RepoURL string mgmtUsername string mgmtPassword string - adminAreaAuth string // "" (unset) / "enabled" / "disabled" — see datastore.Settings.AdminAreaAuth + adminAreaAuth string // "" (unset) / "enabled" / "disabled" — see datastore.Settings.AdminAreaAuth + dismissedAnnouncements map[string]time.Time // announcement id -> most recent dismissal; see RecordDismissal spotifyClientID string spotifyClientSecret string spotifyRedirectURI string @@ -318,6 +319,8 @@ func NewServer(ds *datastore.DataStore, sm *setup.Manager, serverURL string, red }, ) + s.dismissedAnnouncements = loadDismissedAnnouncements(ds) + return s } @@ -1049,6 +1052,79 @@ func (s *Server) AdminAreaAuthMode() string { return s.adminAreaAuth } +// activityKindNotificationDismissed is the datastore.RecordActivity "kind" +// used for announcement-banner dismissals (see #419 design, +// _/i419/design-admin-area-auth-gate.md). +const activityKindNotificationDismissed = "notification_dismissed" + +// loadDismissedAnnouncements scans the local activity log once at startup +// and folds it into an id -> most-recent-dismissal-timestamp map. Called +// from NewServer so the read path (IsAnnouncementDismissed) never touches +// disk — only this one, scoped, boot-time scan does, regardless of how +// large the log grows over time. Errors are logged, not fatal: a missing or +// unreadable activity log means "nothing dismissed yet", not a startup failure. +func loadDismissedAnnouncements(ds *datastore.DataStore) map[string]time.Time { + dismissed := make(map[string]time.Time) + + if ds == nil { + return dismissed + } + + records, err := ds.GetActivityRecords(activityKindNotificationDismissed) + if err != nil { + log.Printf("[Announcements] Failed to load dismissal history, treating as none: %v", err) + return dismissed + } + + for _, record := range records { + ts, parseErr := time.Parse(time.RFC3339Nano, record.Timestamp) + if parseErr != nil { + continue + } + + if existing, ok := dismissed[record.ID]; !ok || ts.After(existing) { + dismissed[record.ID] = ts + } + } + + return dismissed +} + +// RecordDismissal marks an announcement as dismissed: appends to the local +// activity log (write-through) and updates the in-memory cache immediately, +// so IsAnnouncementDismissed reflects it without re-reading disk. The same +// id can be dismissed again later (e.g. if re-shown) — each call is a new +// log entry, not an overwrite. +func (s *Server) RecordDismissal(id string) error { + if s.ds != nil { + if err := s.ds.RecordActivity(activityKindNotificationDismissed, id, nil); err != nil { + return err + } + } + + s.mu.Lock() + defer s.mu.Unlock() + + if s.dismissedAnnouncements == nil { + s.dismissedAnnouncements = make(map[string]time.Time) + } + + s.dismissedAnnouncements[id] = time.Now() + + return nil +} + +// IsAnnouncementDismissed reports whether the given announcement id has +// been dismissed, from the in-memory cache only — never touches disk. +func (s *Server) IsAnnouncementDismissed(id string) bool { + s.mu.RLock() + defer s.mu.RUnlock() + + _, ok := s.dismissedAnnouncements[id] + + return ok +} + // SetInternalPaths sets the internal paths for the server. func (s *Server) SetInternalPaths(paths []string) { s.mu.Lock()