diff options
| -rw-r--r-- | cmd/mobius-hotline-server/main.go | 70 | ||||
| -rw-r--r-- | internal/mobius/api_test.go | 18 | ||||
| -rw-r--r-- | internal/mobius/ban.go | 4 | ||||
| -rw-r--r-- | internal/mobius/ban_test.go | 16 | ||||
| -rw-r--r-- | internal/mobius/redis_ban_test.go | 18 | ||||
| -rw-r--r-- | internal/mobius/reload.go | 19 | ||||
| -rw-r--r-- | internal/mobius/threaded_news.go | 4 | ||||
| -rw-r--r-- | internal/mobius/threaded_news_test.go | 8 |
8 files changed, 90 insertions, 67 deletions
diff --git a/cmd/mobius-hotline-server/main.go b/cmd/mobius-hotline-server/main.go index 117cb2a..ba7924d 100644 --- a/cmd/mobius-hotline-server/main.go +++ b/cmd/mobius-hotline-server/main.go @@ -109,11 +109,17 @@ func main() { os.Exit(1) } - srv.MessageBoard, err = mobius.NewFlatNews(path.Join(*configDir, "MessageBoard.txt")) + // reloaders collects the storage backends whose state is reloaded on SIGHUP or via the + // reload API endpoint. + var reloaders []namedReloader + + messageBoard, err := mobius.NewFlatNews(path.Join(*configDir, "MessageBoard.txt")) if err != nil { slogger.Error("Error loading message board", "err", err) os.Exit(1) } + srv.MessageBoard = messageBoard + reloaders = append(reloaders, namedReloader{"message board", messageBoard}) // Initialize ban list - use Redis if configured, otherwise use file-based storage if *redisAddr != "" { @@ -133,18 +139,23 @@ func main() { srv.BanList = mobius.NewRedisBanMgr(redisClient, slogger) slogger.Debug("Using Redis for ban management", "addr", *redisAddr) } else { - srv.BanList, err = mobius.NewBanFile(path.Join(*configDir, "Banlist.yaml")) + banFile, err := mobius.NewBanFile(path.Join(*configDir, "Banlist.yaml")) if err != nil { slogger.Error("Error loading ban list", "err", err) os.Exit(1) } + srv.BanList = banFile + // The Redis-backed ban list needs no reload, so only the file-backed one registers. + reloaders = append(reloaders, namedReloader{"ban list", banFile}) } - srv.ThreadedNewsMgr, err = mobius.NewThreadedNewsYAML(path.Join(*configDir, "ThreadedNews.yaml")) + threadedNews, err := mobius.NewThreadedNewsYAML(path.Join(*configDir, "ThreadedNews.yaml")) if err != nil { slogger.Error("Error loading news", "err", err) os.Exit(1) } + srv.ThreadedNewsMgr = threadedNews + reloaders = append(reloaders, namedReloader{"threaded news", threadedNews}) srv.AccountManager, err = mobius.NewYAMLAccountManager(path.Join(*configDir, "Users/")) if err != nil { @@ -152,48 +163,35 @@ func main() { os.Exit(1) } - srv.Agreement, err = mobius.NewAgreement(*configDir, "\r") + agreement, err := mobius.NewAgreement(*configDir, "\r") if err != nil { slogger.Error("Error loading agreement", "err", err) os.Exit(1) } + srv.Agreement = agreement + reloaders = append(reloaders, namedReloader{"agreement", agreement}) - bannerPath := path.Join(*configDir, config.BannerFile) - banner, err := os.ReadFile(bannerPath) - if err != nil { + // On reload failure, the previous banner is kept because SetBanner is only called on success. + reloadBanner := mobius.ReloaderFunc(func() error { + banner, err := os.ReadFile(path.Join(*configDir, config.BannerFile)) + if err != nil { + return err + } + srv.SetBanner(banner) + return nil + }) + if err := reloadBanner.Reload(); err != nil { slogger.Error("Error loading banner", "err", err) os.Exit(1) } - srv.SetBanner(banner) + reloaders = append(reloaders, namedReloader{"banner", reloadBanner}) reloadFunc := func() { - if err := srv.MessageBoard.(*mobius.FlatNews).Reload(); err != nil { - slogger.Error("Error reloading news", "err", err) - } - - // Only reload ban list if using file-based storage (Redis doesn't need reload) - if banFile, ok := srv.BanList.(*mobius.BanFile); ok { - if err := banFile.Load(); err != nil { - slogger.Error("Error reloading ban list", "err", err) + for _, item := range reloaders { + if err := item.reloader.Reload(); err != nil { + slogger.Error("Error reloading "+item.name, "err", err) } } - - if err := srv.ThreadedNewsMgr.(*mobius.ThreadedNewsYAML).Load(); err != nil { - slogger.Error("Error reloading threaded news list", "err", err) - } - - if err := srv.Agreement.(*mobius.Agreement).Reload(); err != nil { - slogger.Error("Error reloading agreement", "err", err) - } - - // Let's try to reload the banner. On failure, keep serving the previous banner. - bannerPath := path.Join(*configDir, config.BannerFile) - banner, err := os.ReadFile(bannerPath) - if err != nil { - slogger.Error("Error reloading banner", "err", err) - } else { - srv.SetBanner(banner) - } } if *apiAddr != "" { @@ -248,6 +246,12 @@ func main() { slogger.Info("Server shut down") } +// namedReloader pairs a Reloader with a human-readable name for reload error logging. +type namedReloader struct { + name string + reloader mobius.Reloader +} + // findConfigPath searches for an existing config directory from the predefined search order. // Returns the first directory that exists, or falls back to "config" as the default. func findConfigPath() string { diff --git a/internal/mobius/api_test.go b/internal/mobius/api_test.go index ca1e207..a5ff7ab 100644 --- a/internal/mobius/api_test.go +++ b/internal/mobius/api_test.go @@ -35,15 +35,15 @@ type mockBanMgr struct { unbanNicknameArg string // Allow injecting errors - addErr error - banUsernameErr error - banNicknameErr error - unbanIPErr error - unbanUsernameErr error - unbanNicknameErr error - listIPsErr error - listUsernamesErr error - listNicknamesErr error + addErr error + banUsernameErr error + banNicknameErr error + unbanIPErr error + unbanUsernameErr error + unbanNicknameErr error + listIPsErr error + listUsernamesErr error + listNicknamesErr error } func (m *mockBanMgr) Add(ip string, _ *time.Time) error { diff --git a/internal/mobius/ban.go b/internal/mobius/ban.go index 6ccb9fe..e156677 100644 --- a/internal/mobius/ban.go +++ b/internal/mobius/ban.go @@ -29,7 +29,7 @@ func NewBanFile(path string) (*BanFile, error) { bannedNicks: make(map[string]bool), } - err := bf.Load() + err := bf.Reload() if err != nil { return nil, fmt.Errorf("load ban file: %w", err) } @@ -43,7 +43,7 @@ type BanFileData struct { BannedNicks map[string]bool `yaml:"bannedNicks"` } -func (bf *BanFile) Load() error { +func (bf *BanFile) Reload() error { bf.Lock() defer bf.Unlock() diff --git a/internal/mobius/ban_test.go b/internal/mobius/ban_test.go index c7535ac..d59b757 100644 --- a/internal/mobius/ban_test.go +++ b/internal/mobius/ban_test.go @@ -102,7 +102,7 @@ func TestAdd(t *testing.T) { // Load the file to check its contents. loadedBanFile := &BanFile{filePath: tmpFilePath} - err = loadedBanFile.Load() + err = loadedBanFile.Reload() assert.NoError(t, err, "Load() error") assert.Equal(t, tt.expect, loadedBanFile.banList, "Ban list does not match") }) @@ -178,7 +178,7 @@ func TestBanFile_UsernameBanning(t *testing.T) { // Persist and reload. bf2 := &BanFile{filePath: bf.filePath} - require.NoError(t, bf2.Load()) + require.NoError(t, bf2.Reload()) assert.True(t, bf2.IsUsernameBanned("baduser")) // Unban. @@ -187,7 +187,7 @@ func TestBanFile_UsernameBanning(t *testing.T) { // Verify unban persists. bf3 := &BanFile{filePath: bf.filePath} - require.NoError(t, bf3.Load()) + require.NoError(t, bf3.Reload()) assert.False(t, bf3.IsUsernameBanned("baduser")) } @@ -201,7 +201,7 @@ func TestBanFile_NicknameBanning(t *testing.T) { // Persist and reload. bf2 := &BanFile{filePath: bf.filePath} - require.NoError(t, bf2.Load()) + require.NoError(t, bf2.Reload()) assert.True(t, bf2.IsNicknameBanned("troll")) // Unban. @@ -210,7 +210,7 @@ func TestBanFile_NicknameBanning(t *testing.T) { // Verify unban persists. bf3 := &BanFile{filePath: bf.filePath} - require.NoError(t, bf3.Load()) + require.NoError(t, bf3.Reload()) assert.False(t, bf3.IsNicknameBanned("troll")) } @@ -227,7 +227,7 @@ func TestBanFile_UnbanIP(t *testing.T) { // Verify unban persists. bf2 := &BanFile{filePath: bf.filePath} - require.NoError(t, bf2.Load()) + require.NoError(t, bf2.Reload()) banned, _ = bf2.IsBanned("10.0.0.1") assert.False(t, banned) } @@ -267,7 +267,7 @@ func TestBanFile_PermanentBanViaAdd(t *testing.T) { // Verify persistence. bf2 := &BanFile{filePath: bf.filePath} - require.NoError(t, bf2.Load()) + require.NoError(t, bf2.Reload()) banned, until = bf2.IsBanned("172.16.0.1") assert.True(t, banned) assert.Nil(t, until) @@ -328,7 +328,7 @@ func TestBanFile_NewFormatPersistence(t *testing.T) { // Reload into a fresh BanFile. bf2 := &BanFile{filePath: bf.filePath} - require.NoError(t, bf2.Load()) + require.NoError(t, bf2.Reload()) // Verify IPs. banned, until := bf2.IsBanned("10.0.0.1") diff --git a/internal/mobius/redis_ban_test.go b/internal/mobius/redis_ban_test.go index 6cd3078..2ca04be 100644 --- a/internal/mobius/redis_ban_test.go +++ b/internal/mobius/redis_ban_test.go @@ -128,19 +128,19 @@ func TestRedisBanMgr_TemporalBans(t *testing.T) { permanentIP := "192.168.1.105" err := banMgr.Add(permanentIP, nil) assert.NoError(t, err) - + // Add temporary ban temporaryIP := "192.168.1.106" expiration := time.Now().Add(1 * time.Hour) err = banMgr.Add(temporaryIP, &expiration) assert.NoError(t, err) - + // List should include both ips, err := banMgr.ListBannedIPs() assert.NoError(t, err) assert.Contains(t, ips, permanentIP) assert.Contains(t, ips, temporaryIP) - + // Cleanup err = banMgr.UnbanIP(permanentIP) assert.NoError(t, err) @@ -198,25 +198,25 @@ func TestRedisBanMgr_UserAndNicknameBans(t *testing.T) { t.Run(tt.name, func(t *testing.T) { // Initially not banned assert.False(t, tt.isBannedFunc(tt.value)) - + // Ban the item err := tt.banFunc(tt.value) assert.NoError(t, err) - + // Should be banned assert.True(t, tt.isBannedFunc(tt.value)) - + // Should appear in list items, err := tt.listFunc() assert.NoError(t, err) assert.Contains(t, items, tt.value) - + // Unban the item err = tt.unbanFunc(tt.value) assert.NoError(t, err) - + // Should not be banned assert.False(t, tt.isBannedFunc(tt.value)) }) } -}
\ No newline at end of file +} diff --git a/internal/mobius/reload.go b/internal/mobius/reload.go new file mode 100644 index 0000000..e9f5768 --- /dev/null +++ b/internal/mobius/reload.go @@ -0,0 +1,19 @@ +package mobius + +// Reloader is implemented by storage backends that can reload their state from disk, e.g. in +// response to SIGHUP or the reload API endpoint. +type Reloader interface { + Reload() error +} + +// ReloaderFunc adapts a plain func to the Reloader interface. +type ReloaderFunc func() error + +func (f ReloaderFunc) Reload() error { return f() } + +var ( + _ Reloader = (*FlatNews)(nil) + _ Reloader = (*BanFile)(nil) + _ Reloader = (*ThreadedNewsYAML)(nil) + _ Reloader = (*Agreement)(nil) +) diff --git a/internal/mobius/threaded_news.go b/internal/mobius/threaded_news.go index 41ae2cb..57b63b0 100644 --- a/internal/mobius/threaded_news.go +++ b/internal/mobius/threaded_news.go @@ -24,7 +24,7 @@ type ThreadedNewsYAML struct { func NewThreadedNewsYAML(filePath string) (*ThreadedNewsYAML, error) { tn := &ThreadedNewsYAML{filePath: filePath} - err := tn.Load() + err := tn.Reload() return tn, err } @@ -259,7 +259,7 @@ func (n *ThreadedNewsYAML) ListArticles(newsPath []string) (hotline.NewsArtListD return cat.GetNewsArtListData() } -func (n *ThreadedNewsYAML) Load() error { +func (n *ThreadedNewsYAML) Reload() error { n.mu.Lock() defer n.mu.Unlock() diff --git a/internal/mobius/threaded_news_test.go b/internal/mobius/threaded_news_test.go index 032e57f..32b77be 100644 --- a/internal/mobius/threaded_news_test.go +++ b/internal/mobius/threaded_news_test.go @@ -168,7 +168,7 @@ func TestThreadedNewsYAML_PostArticle(t *testing.T) { assert.Equal(t, "Content here", art.Data) // Verify persistence by reloading. - require.NoError(t, tn.Load()) + require.NoError(t, tn.Reload()) art = tn.GetArticle([]string{"Fresh"}, 1) require.NotNil(t, art) assert.Equal(t, "First Post", art.Title) @@ -252,7 +252,7 @@ func TestThreadedNewsYAML_DeleteArticle(t *testing.T) { assert.Nil(t, art) // Verify persistence. - require.NoError(t, tn.Load()) + require.NoError(t, tn.Reload()) art = tn.GetArticle([]string{"General"}, 1) assert.Nil(t, art) } @@ -283,7 +283,7 @@ func TestThreadedNewsYAML_CreateGrouping(t *testing.T) { assert.Equal(t, hotline.NewsBundle, item.Type) // Verify persistence. - require.NoError(t, tn.Load()) + require.NoError(t, tn.Reload()) item = tn.NewsItem([]string{"NewBundle"}) assert.Equal(t, "NewBundle", item.Name) }) @@ -320,7 +320,7 @@ func TestThreadedNewsYAML_DeleteNewsItem(t *testing.T) { } // Verify persistence. - require.NoError(t, tn.Load()) + require.NoError(t, tn.Reload()) cats = tn.GetCategories(nil) for _, c := range cats { assert.NotEqual(t, "General", c.Name) |