From 2c07180c3961c5f538a7538d2b4641d496ed5373 Mon Sep 17 00:00:00 2001 From: Simon J Mudd Date: Sat, 28 Mar 2026 16:48:06 +0100 Subject: [PATCH] Refactor app --- app/app.go | 129 ++++++----------------------- app/collector.go | 125 ++++++++++++++++++++++++++++ app/collector_test.go | 168 ++++++++++++++++++++++++++++++++++++++ app/signalhandler.go | 26 ++++++ app/signalhandler_test.go | 37 +++++++++ 5 files changed, 380 insertions(+), 105 deletions(-) create mode 100644 app/collector.go create mode 100644 app/collector_test.go create mode 100644 app/signalhandler.go create mode 100644 app/signalhandler_test.go diff --git a/app/app.go b/app/app.go index 4a0981e..9fc3271 100644 --- a/app/app.go +++ b/app/app.go @@ -5,9 +5,6 @@ import ( "database/sql" "errors" "fmt" - "os" - "os/signal" - "syscall" "time" "github.com/sjmudd/anonymiser" @@ -18,7 +15,6 @@ import ( "github.com/sjmudd/ps-top/global" "github.com/sjmudd/ps-top/log" "github.com/sjmudd/ps-top/model/filter" - "github.com/sjmudd/ps-top/pstable" "github.com/sjmudd/ps-top/setupinstruments" "github.com/sjmudd/ps-top/utils" "github.com/sjmudd/ps-top/view" @@ -39,19 +35,10 @@ type App struct { db *sql.DB // connection to MySQL display *display.Display // display displays the information to the screen finished bool // has the app finished? - sigChan chan os.Signal // signal handler channel - waiter *wait.Waiter // for handling waits between collecting metrics help bool // show help (during runtime) - fileinfolatency pstable.Tabler // file i/o latency information - tableiolatency pstable.Tabler // table i/o latency information - tableioops pstable.Tabler // table i/o operations information - tablelocklatency pstable.Tabler // table lock information - mutexlatency pstable.Tabler // mutex latency information - stageslatency pstable.Tabler // stages latency information - memory pstable.Tabler // memory usage information - users pstable.Tabler // user information - currentTabler pstable.Tabler // current data being collected - currentView view.View // holds the view we are currently using + collector *DBCollector // owns all tablers and collection logic + signalHandler *SignalHandler // handles signals + waiter *wait.Waiter // for handling waits between collecting metrics setupInstruments *setupinstruments.SetupInstruments // for setting up and restoring performance_schema configuration. } @@ -111,95 +98,36 @@ func NewApp( app.waiter = wait.NewWaiter() app.waiter.SetWaitInterval(time.Second * time.Duration(settings.Interval)) - // setup to their initial types/values - log.Println("app.NewApp: Setting up models") - - app.fileinfolatency = pstable.NewTabler(pstable.FileIoLatency, app.config, app.db) - temptableiolatency := pstable.NewTabler(pstable.TableIoLatency, app.config, app.db) // shared backend/metrics - app.tableiolatency = temptableiolatency - app.tableioops = pstable.NewTableIoOps(temptableiolatency) - app.tablelocklatency = pstable.NewTabler(pstable.TableLockLatency, app.config, app.db) - app.mutexlatency = pstable.NewTabler(pstable.MutexLatency, app.config, app.db) - app.stageslatency = pstable.NewTabler(pstable.StagesLatency, app.config, app.db) - app.memory = pstable.NewTabler(pstable.MemoryUsage, app.config, app.db) - app.users = pstable.NewTabler(pstable.UserLatency, app.config, app.db) + // Create DBCollector to manage all data collection (replaces individual tabler fields) + log.Println("app.NewApp: Setting up models via DBCollector") + app.collector = NewDBCollector(app.config, app.db) - log.Println("app.NewApp: model setup complete") - - app.resetDBStatistics() + // Create signal handler + app.signalHandler = NewSignalHandler() + // Setup view (using collector's currentView field) var viewErr error - app.currentView, viewErr = view.SetupAndValidate(settings.ViewName, app.db) // if empty will use the default + app.collector.currentView, viewErr = view.SetupAndValidate(settings.ViewName, app.db) // if empty will use the default if viewErr != nil { return nil, fmt.Errorf("app.NewApp: %w", viewErr) } - app.UpdateCurrentTabler() + app.collector.UpdateCurrentTabler() + + // Initial collection and reset to establish baseline + log.Println("app.NewApp: Initial collection and reset") + app.collector.CollectAll() + app.collector.ResetAll() log.Println("app.NewApp() finishes") return app, nil } -// UpdateCurrentTabler updates the current tabler to use -func (app *App) UpdateCurrentTabler() { - switch app.currentView.Get() { - case view.ViewLatency: - app.currentTabler = app.tableiolatency - case view.ViewOps: - app.currentTabler = app.tableioops - case view.ViewIO: - app.currentTabler = app.fileinfolatency - case view.ViewLocks: - app.currentTabler = app.tablelocklatency - case view.ViewUsers: - app.currentTabler = app.users - case view.ViewMutex: - app.currentTabler = app.mutexlatency - case view.ViewStages: - app.currentTabler = app.stageslatency - case view.ViewMemory: - app.currentTabler = app.memory - } -} - -// CollectAll collects all the stats together in one go -func (app *App) collectAll() { - log.Println("app.collectAll() start") - app.fileinfolatency.Collect() - app.tablelocklatency.Collect() - app.tableiolatency.Collect() - app.users.Collect() - app.stageslatency.Collect() - app.mutexlatency.Collect() - app.memory.Collect() - log.Println("app.collectAll() finished") -} - -// resetDBStatistics does a fresh collection of data and then updates the initial values based on that. -func (app *App) resetDBStatistics() { - log.Println("app.resetDBStatistcs()") - app.collectAll() - app.resetStatistics() -} - -func (app *App) resetStatistics() { - start := time.Now() - app.fileinfolatency.ResetStatistics() - app.tablelocklatency.ResetStatistics() - app.tableiolatency.ResetStatistics() - app.users.ResetStatistics() - app.stageslatency.ResetStatistics() - app.mutexlatency.ResetStatistics() - app.memory.ResetStatistics() - - log.Println("app.resetStatistics() took", time.Since(start)) -} - // Collect the data we are looking at. func (app *App) Collect() { log.Println("app.Collect()") start := time.Now() - app.currentTabler.Collect() + app.collector.Collect() app.waiter.CollectedNow() log.Println("app.Collect() took", time.Since(start)) } @@ -209,22 +137,22 @@ func (app *App) Display() { if app.help { app.display.Display(display.Help) } else { - app.display.Display(app.currentTabler) + app.display.Display(app.collector.CurrentTabler()) } } // change to the previous display mode func (app *App) displayPrevious() { - app.currentView.SetPrev() - app.UpdateCurrentTabler() + app.collector.currentView.SetPrev() + app.collector.UpdateCurrentTabler() app.display.Clear() app.Display() } // change to the next display mode func (app *App) displayNext() { - app.currentView.SetNext() - app.UpdateCurrentTabler() + app.collector.currentView.SetNext() + app.collector.UpdateCurrentTabler() app.display.Clear() app.Display() } @@ -245,13 +173,11 @@ func (app *App) Run() { log.Println("app.Run()") - // set up signal handling and event channel once - app.setupSignalHandler() eventChan := app.display.EventChan() for !app.finished { select { - case sig := <-app.sigChan: + case sig := <-app.signalHandler.Channel(): log.Println("Caught signal: ", sig) app.finished = true case <-app.waiter.WaitUntilNextPeriod(): @@ -264,13 +190,6 @@ func (app *App) Run() { } } -// setupSignalHandler initializes the signal channel and registers for -// SIGINT and SIGTERM. Extracted to reduce complexity in Run(). -func (app *App) setupSignalHandler() { - app.sigChan = make(chan os.Signal, 10) // 10 entries - signal.Notify(app.sigChan, syscall.SIGINT, syscall.SIGTERM) -} - // collectAndDisplay runs a collection and then updates the display. // Extracted to keep the Run loop concise. func (app *App) collectAndDisplay() { @@ -304,7 +223,7 @@ func (app *App) handleInputEvent(inputEvent event.Event) bool { app.config.SetWantRelativeStats(!app.config.WantRelativeStats()) app.Display() case event.EventResetStatistics: - app.resetDBStatistics() + app.collector.ResetAll() app.Display() case event.EventResizeScreen: width, height := inputEvent.Width, inputEvent.Height diff --git a/app/collector.go b/app/collector.go new file mode 100644 index 0000000..61296f4 --- /dev/null +++ b/app/collector.go @@ -0,0 +1,125 @@ +package app + +import ( + "database/sql" + + "github.com/sjmudd/ps-top/config" + "github.com/sjmudd/ps-top/pstable" + "github.com/sjmudd/ps-top/view" +) + +// DBCollector owns all the Tabler instances and coordinates data collection. +// It knows nothing about display, signals, or event loops - only database collection. +type DBCollector struct { + config *config.Config + db *sql.DB + fileInfoLatency pstable.Tabler + tableIoLatency pstable.Tabler + tableIoOps pstable.Tabler + tableLockLatency pstable.Tabler + mutexLatency pstable.Tabler + stagesLatency pstable.Tabler + memoryUsage pstable.Tabler + userLatency pstable.Tabler + currentTabler pstable.Tabler + currentView view.View +} + +// NewDBCollector creates and initializes all tablers. +func NewDBCollector(cfg *config.Config, db *sql.DB) *DBCollector { + dc := &DBCollector{ + config: cfg, + db: db, + } + + // Initialize all tablers (exact same logic as original app.NewApp lines 117-125) + dc.fileInfoLatency = pstable.NewTabler(pstable.FileIoLatency, cfg, db) + tempTableIoLatency := pstable.NewTabler(pstable.TableIoLatency, cfg, db) + dc.tableIoLatency = tempTableIoLatency + dc.tableIoOps = pstable.NewTableIoOps(tempTableIoLatency) + dc.tableLockLatency = pstable.NewTabler(pstable.TableLockLatency, cfg, db) + dc.mutexLatency = pstable.NewTabler(pstable.MutexLatency, cfg, db) + dc.stagesLatency = pstable.NewTabler(pstable.StagesLatency, cfg, db) + dc.memoryUsage = pstable.NewTabler(pstable.MemoryUsage, cfg, db) + dc.userLatency = pstable.NewTabler(pstable.UserLatency, cfg, db) + + return dc +} + +// UpdateCurrentTabler sets currentTabler based on currentView. +// Extracted from App.UpdateCurrentTabler. +func (dc *DBCollector) UpdateCurrentTabler() { + switch dc.currentView.Get() { + case view.ViewLatency: + dc.currentTabler = dc.tableIoLatency + case view.ViewOps: + dc.currentTabler = dc.tableIoOps + case view.ViewIO: + dc.currentTabler = dc.fileInfoLatency + case view.ViewLocks: + dc.currentTabler = dc.tableLockLatency + case view.ViewUsers: + dc.currentTabler = dc.userLatency + case view.ViewMutex: + dc.currentTabler = dc.mutexLatency + case view.ViewStages: + dc.currentTabler = dc.stagesLatency + case view.ViewMemory: + dc.currentTabler = dc.memoryUsage + } +} + +// Collect collects data for the current tabler. +// Extracted from App.Collect. +func (dc *DBCollector) Collect() { + dc.currentTabler.Collect() +} + +// CollectAll collects data for all tablers. +// Extracted from App.collectAll. +func (dc *DBCollector) CollectAll() { + dc.fileInfoLatency.Collect() + dc.tableLockLatency.Collect() + dc.tableIoLatency.Collect() + dc.userLatency.Collect() + dc.stagesLatency.Collect() + dc.mutexLatency.Collect() + dc.memoryUsage.Collect() +} + +// ResetAll resets statistics on all tablers. +// Extracted from App.resetStatistics. +func (dc *DBCollector) ResetAll() { + dc.fileInfoLatency.ResetStatistics() + dc.tableLockLatency.ResetStatistics() + dc.tableIoLatency.ResetStatistics() + dc.userLatency.ResetStatistics() + dc.stagesLatency.ResetStatistics() + dc.mutexLatency.ResetStatistics() + dc.memoryUsage.ResetStatistics() +} + +// SetView updates the current view and refreshes currentTabler. +func (dc *DBCollector) SetView(v view.View) { + dc.currentView = v + dc.UpdateCurrentTabler() +} + +// CurrentTabler returns the currently selected tabler (for display). +func (dc *DBCollector) CurrentTabler() pstable.Tabler { + return dc.currentTabler +} + +// CurrentView returns the current view code. +func (dc *DBCollector) CurrentView() view.View { + return dc.currentView +} + +// SetViewByName sets the view by name (convenience wrapper). +func (dc *DBCollector) SetViewByName(name string) error { + if err := dc.currentView.SetByName(name); err != nil { + return err + } + dc.UpdateCurrentTabler() + return nil +} diff --git a/app/collector_test.go b/app/collector_test.go new file mode 100644 index 0000000..30a1d13 --- /dev/null +++ b/app/collector_test.go @@ -0,0 +1,168 @@ +package app + +import ( + "testing" + "time" + + "github.com/sjmudd/ps-top/view" +) + +// mockTabler is a minimal implementation of pstable.Tabler for testing. +type mockTabler struct { + name string + coll bool + reset bool + desc string + head string + rows []string + total string + empty string + haveRel bool + wantRel bool +} + +func (m *mockTabler) Collect() { m.coll = true } +func (m *mockTabler) ResetStatistics() { m.reset = true } +func (m *mockTabler) Description() string { return m.desc } +func (m *mockTabler) HaveRelativeStats() bool { return m.haveRel } +func (m *mockTabler) Headings() string { return m.head } +func (m *mockTabler) FirstCollectTime() time.Time { return time.Time{} } +func (m *mockTabler) LastCollectTime() time.Time { return time.Time{} } +func (m *mockTabler) RowContent() []string { return m.rows } +func (m *mockTabler) TotalRowContent() string { return m.total } +func (m *mockTabler) EmptyRowContent() string { return m.empty } +func (m *mockTabler) WantRelativeStats() bool { return m.wantRel } + +// TestDBCollector_NewDBCollector verifies that NewDBCollector creates all 8 tablers. +// Since NewDBCollector actually calls pstable.NewTabler which needs a real DB, +// this test constructs a DBCollector manually with mock tablers to verify structure. +func TestDBCollector_Structure(t *testing.T) { + dc := &DBCollector{ + fileInfoLatency: &mockTabler{name: "fileInfoLatency"}, + tableIoLatency: &mockTabler{name: "tableIoLatency"}, + tableIoOps: &mockTabler{name: "tableIoOps"}, + tableLockLatency: &mockTabler{name: "tableLockLatency"}, + mutexLatency: &mockTabler{name: "mutexLatency"}, + stagesLatency: &mockTabler{name: "stagesLatency"}, + memoryUsage: &mockTabler{name: "memoryUsage"}, + userLatency: &mockTabler{name: "userLatency"}, + } + if dc.fileInfoLatency == nil { + t.Error("fileInfoLatency is nil") + } + if dc.tableIoLatency == nil { + t.Error("tableIoLatency is nil") + } + if dc.tableIoOps == nil { + t.Error("tableIoOps is nil") + } + if dc.tableLockLatency == nil { + t.Error("tableLockLatency is nil") + } + if dc.mutexLatency == nil { + t.Error("mutexLatency is nil") + } + if dc.stagesLatency == nil { + t.Error("stagesLatency is nil") + } + if dc.memoryUsage == nil { + t.Error("memoryUsage is nil") + } + if dc.userLatency == nil { + t.Error("userLatency is nil") + } +} + +// TestDBCollector_Collect tests that Collect delegates to currentTabler. +func TestDBCollector_Collect(t *testing.T) { + mt := &mockTabler{} + dc := &DBCollector{ + currentTabler: mt, + } + dc.Collect() + if !mt.coll { + t.Error("Collect did not delegate to currentTabler") + } +} + +// TestDBCollector_CollectAll tests that CollectAll calls Collect on all 8 tablers. +func TestDBCollector_CollectAll(t *testing.T) { + mocks := []*mockTabler{ + {name: "fileInfoLatency"}, + {name: "tableLockLatency"}, + {name: "tableIoLatency"}, + {name: "userLatency"}, + {name: "stagesLatency"}, + {name: "mutexLatency"}, + {name: "memoryUsage"}, + } + dc := &DBCollector{ + fileInfoLatency: mocks[0], + tableLockLatency: mocks[1], + tableIoLatency: mocks[2], + userLatency: mocks[3], + stagesLatency: mocks[4], + mutexLatency: mocks[5], + memoryUsage: mocks[6], + } + dc.CollectAll() + for i, m := range mocks { + if !m.coll { + t.Errorf("mockTabler %d (%s) was not collected", i, m.name) + } + } +} + +// TestDBCollector_ResetAll tests that ResetAll calls ResetStatistics on all 8 tablers. +func TestDBCollector_ResetAll(t *testing.T) { + mocks := []*mockTabler{ + {name: "fileInfoLatency"}, + {name: "tableLockLatency"}, + {name: "tableIoLatency"}, + {name: "userLatency"}, + {name: "stagesLatency"}, + {name: "mutexLatency"}, + {name: "memoryUsage"}, + } + dc := &DBCollector{ + fileInfoLatency: mocks[0], + tableLockLatency: mocks[1], + tableIoLatency: mocks[2], + userLatency: mocks[3], + stagesLatency: mocks[4], + mutexLatency: mocks[5], + memoryUsage: mocks[6], + } + dc.ResetAll() + for i, m := range mocks { + if !m.reset { + t.Errorf("mockTabler %d (%s) was not reset", i, m.name) + } + } +} + +// TestDBCollector_Accessors tests CurrentTabler and CurrentView getters. +func TestDBCollector_Accessors(t *testing.T) { + mt := &mockTabler{} + fv := view.View{} // zero value + dc := &DBCollector{ + currentTabler: mt, + currentView: fv, + } + if dc.CurrentTabler() != mt { + t.Error("CurrentTabler did not return correct tabler") + } + if dc.CurrentView() != fv { + t.Error("CurrentView did not return correct view") + } +} + +// TestDBCollector_SetView tests that SetView updates currentView and calls UpdateCurrentTabler. +func TestDBCollector_SetView(t *testing.T) { + t.Skip("Requires view.View with valid manager; tested in integration") +} + +// TestDBCollector_SetViewByName tests SetViewByName; integration scope. +func TestDBCollector_SetViewByName(t *testing.T) { + t.Skip("Requires view.View with valid manager; tested in integration") +} diff --git a/app/signalhandler.go b/app/signalhandler.go new file mode 100644 index 0000000..c03f530 --- /dev/null +++ b/app/signalhandler.go @@ -0,0 +1,26 @@ +package app + +import ( + "os" + "os/signal" + "syscall" +) + +// SignalHandler manages signal handling for the application. +type SignalHandler struct { + sigChan chan os.Signal +} + +// NewSignalHandler creates a new signal handler listening for SIGINT and SIGTERM. +func NewSignalHandler() *SignalHandler { + sh := &SignalHandler{ + sigChan: make(chan os.Signal, 10), + } + signal.Notify(sh.sigChan, syscall.SIGINT, syscall.SIGTERM) + return sh +} + +// Channel returns the signal channel (read-only to caller). +func (sh *SignalHandler) Channel() <-chan os.Signal { + return sh.sigChan +} diff --git a/app/signalhandler_test.go b/app/signalhandler_test.go new file mode 100644 index 0000000..b12fb68 --- /dev/null +++ b/app/signalhandler_test.go @@ -0,0 +1,37 @@ +package app + +import ( + "testing" +) + +// TestSignalHandler_New verifies that NewSignalHandler returns a non-nil handler. +func TestSignalHandler_New(t *testing.T) { + sh := NewSignalHandler() + if sh == nil { + t.Fatal("NewSignalHandler returned nil") + } +} + +// TestSignalHandler_Channel verifies that Channel() returns a non-nil channel. +func TestSignalHandler_Channel(t *testing.T) { + sh := NewSignalHandler() + ch := sh.Channel() + if ch == nil { + t.Fatal("Channel() returned nil") + } +} + +// TestSignalHandler_MultipleHandlers verifies that multiple handlers can be created +// without interfering (each has its own channel). +func TestSignalHandler_MultipleHandlers(t *testing.T) { + sh1 := NewSignalHandler() + sh2 := NewSignalHandler() + if sh1 == nil || sh2 == nil { + t.Fatal("Failed to create handlers") + } + ch1 := sh1.Channel() + ch2 := sh2.Channel() + if ch1 == ch2 { + t.Error("Handlers should have separate channels") + } +}