feat(config): notify reload hooks after a settings update
Build / ubuntu-latest (push) Failing after 3m8s
Build / windows-latest (push) Canceled after 4m3s

A settings update replaced the in-memory configuration but nothing told the code
that derives state from it. The logger is the first such consumer: its debug
flag is captured once at startup by postLog.SetDebugMode, so toggling
system.debugMode at runtime changed what config.IsDebugMode() reported while
DEBUG lines stayed filtered by the value from boot.

Add a hook registry to the config package. OnReload registers a callback and
every writer of config.json runs the registered hooks with the configuration now
in effect. It lives in config so packages that already depend on it (the logger,
and later the controller manager) can react without config importing them back,
which would be an import cycle.

Hooks run after settingsLock is released, not inside it. They will rebuild
controllers once those support reloading, which can wait on a network call, and
settingsLock is also held by the node tracker's background save — running a hook
under the lock would stall node registration behind a settings edit.
runSettingsUpdate now owns that lock/notify sequence for all four writer paths
(UpdateSettings and the three webhook endpoint helpers).

A panicking hook is logged and skipped: by then the new configuration is on disk
and published, so reporting the write as failed would be a lie, and the hooks
registered after the broken one still need to run.
This commit is contained in:
2026-09-28 23:10:33 +08:00
parent ac5587d0c8
commit d9a9918e21
5 changed files with 280 additions and 38 deletions
+73
View File
@@ -0,0 +1,73 @@
package config
import (
"fmt"
"sync"
"nukumizu-backend/postLog"
)
// reloadHooks are the callbacks run after the global configuration has been
// reloaded, i.e. after every settings update that touches config.json.
//
// They exist so that packages which already depend on config — the controller
// manager, the logger — can react to an update without config importing them,
// which would be an import cycle. Everything a hook needs is passed in.
var (
reloadHooksMu sync.Mutex
reloadHooks []func(*Config)
)
// OnReload registers a hook to run after every reload of the global
// configuration, receiving the configuration now in effect. Hooks run in
// registration order.
//
// Register once, at startup, before the first settings update can arrive: a
// hook registered later has already missed the updates that came before it, and
// the configuration it would have seen is not replayed.
//
// A hook runs on the goroutine serving /api/settings/set, so it must not block
// for long. It may be called concurrently by two overlapping updates.
func OnReload(hook func(*Config)) {
reloadHooksMu.Lock()
defer reloadHooksMu.Unlock()
reloadHooks = append(reloadHooks, hook)
}
// notifyReload runs every registered hook with cfg. A nil cfg is the signal
// that the update touched one of the other settings files, which have no
// hook-visible reload, and it is ignored.
//
// A panicking hook is logged and skipped rather than allowed to unwind through
// UpdateSettings: by the time hooks run the new configuration is already on
// disk and published in memory, so reporting the write as failed would be a
// lie, and the hooks registered after the broken one must still run.
func notifyReload(cfg *Config) {
if cfg == nil {
return
}
// Copy under the lock, then run outside it: a hook is free to register
// another hook without deadlocking.
reloadHooksMu.Lock()
hooks := make([]func(*Config), len(reloadHooks))
copy(hooks, reloadHooks)
reloadHooksMu.Unlock()
for _, hook := range hooks {
runReloadHook(hook, cfg)
}
}
// runReloadHook runs one hook, isolating a panic to that hook. Recovering in a
// separate function rather than inline is deliberate: a deferred recover placed
// in the loop body would not run until notifyReload itself returned, which
// would abandon the remaining hooks.
func runReloadHook(hook func(*Config), cfg *Config) {
defer func() {
if r := recover(); r != nil {
postLog.Error(fmt.Sprintf("Configuration reload hook panicked: %v", r))
}
}()
hook(cfg)
}
+138
View File
@@ -0,0 +1,138 @@
package config
import (
"testing"
"nukumizu-backend/global"
)
// swapReloadHooks replaces the registered reload hooks for the duration of a
// test and restores the previous set afterwards, so the package-level registry
// cannot leak into another test.
func swapReloadHooks(t *testing.T, hooks ...func(*Config)) {
t.Helper()
reloadHooksMu.Lock()
original := reloadHooks
reloadHooks = hooks
reloadHooksMu.Unlock()
t.Cleanup(func() {
reloadHooksMu.Lock()
reloadHooks = original
reloadHooksMu.Unlock()
})
}
func TestReloadHookRunsOnlyForGlobalSettings(t *testing.T) {
writeTempConfig(t, &global.ConfigPath.Global, `{"system":{"debugMode":false}}`)
writeTempConfig(t, &global.ConfigPath.BotUserConfig, `{}`)
writeTempConfig(t, &global.ConfigPath.BotNodeConfig, `{}`)
var seen []bool
swapReloadHooks(t, func(cfg *Config) {
seen = append(seen, cfg.System.DebugMode)
})
// A config.json update runs the hook, with the values just written.
if err := UpdateSettings(SettingGlobal, map[string]interface{}{
"system": map[string]interface{}{"debugMode": true},
}); err != nil {
t.Fatalf("UpdateSettings(global): %v", err)
}
if len(seen) != 1 {
t.Fatalf("hook ran %d times for a config.json update, want 1", len(seen))
}
if !seen[0] {
t.Error("hook received a configuration without the updated debugMode")
}
// The other two files reload a singleton that callers read at the point of
// use, so they have nothing to notify.
for _, settingsType := range []string{SettingBotUserConfig, SettingBotNodeConfig} {
if err := UpdateSettings(settingsType, map[string]interface{}{
"unused": map[string]interface{}{"event_reply": true},
}); err != nil {
t.Fatalf("UpdateSettings(%s): %v", settingsType, err)
}
}
if len(seen) != 1 {
t.Errorf("hook ran %d times after updates to the other settings files, want 1", len(seen))
}
}
func TestReloadHookIsolatesPanic(t *testing.T) {
writeTempConfig(t, &global.ConfigPath.Global, `{"system":{"debugMode":false}}`)
reached := false
swapReloadHooks(t,
func(*Config) { panic("hook under test") },
func(*Config) { reached = true },
)
// The configuration is already on disk and published by the time hooks run,
// so a broken hook must not turn a successful write into a failed request.
if err := UpdateSettings(SettingGlobal, map[string]interface{}{
"system": map[string]interface{}{"debugMode": true},
}); err != nil {
t.Fatalf("a panicking hook must not fail the settings write: %v", err)
}
if !reached {
t.Error("a panicking hook stopped the hooks registered after it")
}
// The write itself must still have landed.
cfg := Current()
if cfg == nil || !cfg.System.DebugMode {
t.Error("the settings write did not take effect")
}
}
// TestReloadHookSeesEveryWriterPath pins the invariant that each writer of
// config.json notifies, not just /api/settings/set: the webhook endpoint
// helpers go through the same channel.
func TestReloadHookSeesEveryWriterPath(t *testing.T) {
writeTempConfig(t, &global.ConfigPath.Global, `{
"webhook": { "endpoints": {} }
}`)
var seen int
swapReloadHooks(t, func(*Config) { seen++ })
if err := AddWebhookEndpoint("example", map[string]interface{}{
"enabled": true,
"token": "secret",
"notifyPipes": []interface{}{"ntfy"},
}); err != nil {
t.Fatalf("AddWebhookEndpoint: %v", err)
}
if seen != 1 {
t.Errorf("hook ran %d times after adding an endpoint, want 1", seen)
}
if err := ModifyWebhookEndpoint("example", map[string]interface{}{
"enabled": false,
}); err != nil {
t.Fatalf("ModifyWebhookEndpoint: %v", err)
}
if seen != 2 {
t.Errorf("hook ran %d times after modifying an endpoint, want 2", seen)
}
if err := DeleteWebhookEndpoint("example"); err != nil {
t.Fatalf("DeleteWebhookEndpoint: %v", err)
}
if seen != 3 {
t.Errorf("hook ran %d times after deleting an endpoint, want 3", seen)
}
// A rejected write changes nothing, so it must not notify either.
if err := DeleteWebhookEndpoint("ghost"); err == nil {
t.Error("deleting an unknown endpoint should fail")
}
if seen != 3 {
t.Errorf("hook ran for a rejected write (%d notifications, want 3)", seen)
}
}
+43 -16
View File
@@ -82,19 +82,45 @@ func GetSettings(settingsType string) ([]byte, error) {
// written the matching in-memory singleton is reloaded so runtime code observes // written the matching in-memory singleton is reloaded so runtime code observes
// the new values. // the new values.
func UpdateSettings(settingsType string, patch map[string]interface{}) error { func UpdateSettings(settingsType string, patch map[string]interface{}) error {
settingsLock.Lock() return runSettingsUpdate(func() (*Config, error) {
defer settingsLock.Unlock() return updateSettingsLocked(settingsType, patch)
})
}
return updateSettingsLocked(settingsType, patch) // runSettingsUpdate runs fn under settingsLock and then, once the lock is
// released, runs the reload hooks with whatever configuration fn reports (nil
// when the update did not touch config.json).
//
// The hooks deliberately run outside settingsLock. A hook rebuilds controllers,
// which can wait on a network call, while settingsLock is also held by the node
// tracker's background save (SaveBotNodeConfig); holding it across a hook would
// stall node registration behind an unrelated settings edit.
func runSettingsUpdate(fn func() (*Config, error)) error {
cfg, err := func() (*Config, error) {
settingsLock.Lock()
defer settingsLock.Unlock()
return fn()
}()
if err != nil {
return err
}
notifyReload(cfg)
return nil
} }
// updateSettingsLocked is UpdateSettings without the locking, for callers that // updateSettingsLocked is UpdateSettings without the locking, for callers that
// need to inspect the loaded configuration and write in one critical section // need to inspect the loaded configuration and write in one critical section
// (see the incoming webhook endpoint helpers). Callers must hold settingsLock. // (see the incoming webhook endpoint helpers). Callers must hold settingsLock.
func updateSettingsLocked(settingsType string, patch map[string]interface{}) error { //
// It returns the freshly loaded global configuration, or nil when the settings
// type is one of the other files. The caller is responsible for handing that
// value to notifyReload once settingsLock is released — which runSettingsUpdate
// does for every writer.
func updateSettingsLocked(settingsType string, patch map[string]interface{}) (*Config, error) {
path, err := settingsPath(settingsType) path, err := settingsPath(settingsType)
if err != nil { if err != nil {
return err return nil, err
} }
// Start from whatever is already on disk so nothing is dropped. A missing or // Start from whatever is already on disk so nothing is dropped. A missing or
@@ -104,23 +130,23 @@ func updateSettingsLocked(settingsType string, patch map[string]interface{}) err
if err == nil { if err == nil {
if len(bytes.TrimSpace(data)) > 0 { if len(bytes.TrimSpace(data)) > 0 {
if err := json.Unmarshal(data, &current); err != nil { if err := json.Unmarshal(data, &current); err != nil {
return fmt.Errorf("failed to parse existing %s settings file %s: %w", settingsType, path, err) return nil, fmt.Errorf("failed to parse existing %s settings file %s: %w", settingsType, path, err)
} }
} }
} else if !os.IsNotExist(err) { } else if !os.IsNotExist(err) {
return fmt.Errorf("failed to read existing %s settings file %s: %w", settingsType, path, err) return nil, fmt.Errorf("failed to read existing %s settings file %s: %w", settingsType, path, err)
} }
deepMergeSettings(current, patch) deepMergeSettings(current, patch)
data, err = json.MarshalIndent(current, "", " ") data, err = json.MarshalIndent(current, "", " ")
if err != nil { if err != nil {
return fmt.Errorf("failed to marshal %s settings: %w", settingsType, err) return nil, fmt.Errorf("failed to marshal %s settings: %w", settingsType, err)
} }
data = append(data, '\n') data = append(data, '\n')
if err := os.WriteFile(path, data, 0o644); err != nil { if err := os.WriteFile(path, data, 0o644); err != nil {
return fmt.Errorf("failed to write %s settings file %s: %w", settingsType, path, err) return nil, fmt.Errorf("failed to write %s settings file %s: %w", settingsType, path, err)
} }
return reloadSettings(settingsType, path) return reloadSettings(settingsType, path)
@@ -151,18 +177,19 @@ func deepMergeSettings(dst, src map[string]interface{}) {
} }
// reloadSettings refreshes the in-memory singleton for the given settings type // reloadSettings refreshes the in-memory singleton for the given settings type
// so the running program observes the values just persisted to disk. // so the running program observes the values just persisted to disk. Only
func reloadSettings(settingsType, path string) error { // config.json has a hook-visible reload, so for SettingGlobal it returns the
// configuration now in effect and for the other types it returns nil.
func reloadSettings(settingsType, path string) (*Config, error) {
switch settingsType { switch settingsType {
case SettingGlobal: case SettingGlobal:
_, err := LoadGlobalConfig(path) return LoadGlobalConfig(path)
return err
case SettingBotUserConfig: case SettingBotUserConfig:
_, err := LoadBotUserConfig(path) _, err := LoadBotUserConfig(path)
return err return nil, err
case SettingBotNodeConfig: case SettingBotNodeConfig:
return LoadBotNodeConfig(path) return nil, LoadBotNodeConfig(path)
default: default:
return ErrUnsupportedSettingsType return nil, ErrUnsupportedSettingsType
} }
} }
+18 -22
View File
@@ -60,13 +60,12 @@ func AddWebhookEndpoint(name string, fields map[string]interface{}) error {
return err return err
} }
settingsLock.Lock() return runSettingsUpdate(func() (*Config, error) {
defer settingsLock.Unlock() if _, exists := webhookEndpoint(name); exists {
return nil, fmt.Errorf("%w: %s", ErrWebhookEndpointExists, name)
if _, exists := webhookEndpoint(name); exists { }
return fmt.Errorf("%w: %s", ErrWebhookEndpointExists, name) return updateSettingsLocked(SettingGlobal, webhookEndpointsPatch(name, patch))
} })
return updateSettingsLocked(SettingGlobal, webhookEndpointsPatch(name, patch))
} }
// ModifyWebhookEndpoint updates an existing incoming webhook endpoint. Only the // ModifyWebhookEndpoint updates an existing incoming webhook endpoint. Only the
@@ -84,27 +83,24 @@ func ModifyWebhookEndpoint(name string, fields map[string]interface{}) error {
return fmt.Errorf("%w: no fields to update", ErrWebhookEndpointInvalid) return fmt.Errorf("%w: no fields to update", ErrWebhookEndpointInvalid)
} }
settingsLock.Lock() return runSettingsUpdate(func() (*Config, error) {
defer settingsLock.Unlock() if _, exists := webhookEndpoint(name); !exists {
return nil, fmt.Errorf("%w: %s", ErrWebhookEndpointNotFound, name)
if _, exists := webhookEndpoint(name); !exists { }
return fmt.Errorf("%w: %s", ErrWebhookEndpointNotFound, name) return updateSettingsLocked(SettingGlobal, webhookEndpointsPatch(name, patch))
} })
return updateSettingsLocked(SettingGlobal, webhookEndpointsPatch(name, patch))
} }
// DeleteWebhookEndpoint removes the incoming webhook endpoint registered under // DeleteWebhookEndpoint removes the incoming webhook endpoint registered under
// name. The endpoint stops accepting requests as soon as the configuration is // name. The endpoint stops accepting requests as soon as the configuration is
// reloaded. // reloaded.
func DeleteWebhookEndpoint(name string) error { func DeleteWebhookEndpoint(name string) error {
settingsLock.Lock() return runSettingsUpdate(func() (*Config, error) {
defer settingsLock.Unlock() if _, exists := webhookEndpoint(name); !exists {
return nil, fmt.Errorf("%w: %s", ErrWebhookEndpointNotFound, name)
if _, exists := webhookEndpoint(name); !exists { }
return fmt.Errorf("%w: %s", ErrWebhookEndpointNotFound, name) return updateSettingsLocked(SettingGlobal, webhookEndpointDeletePatch(name))
} })
return updateSettingsLocked(SettingGlobal, webhookEndpointDeletePatch(name))
} }
// webhookEndpoint returns the named endpoint held by the loaded configuration. // webhookEndpoint returns the named endpoint held by the loaded configuration.
+8
View File
@@ -58,6 +58,14 @@ func main() {
postLog.SetDebugMode(cfg.System.DebugMode) postLog.SetDebugMode(cfg.System.DebugMode)
postLog.InitLogBroadcaster() postLog.InitLogBroadcaster()
// The logger's debug flag is a process-wide setting rather than something
// read at every log call, so a settings update has to push the new value
// into it. This is the first link in the reload hook chain; the controllers
// join it as they gain reload support.
config.OnReload(func(updated *config.Config) {
postLog.SetDebugMode(updated.System.DebugMode)
})
dbPath := cfg.DBPath dbPath := cfg.DBPath
if err := postLog.InitLogsDatabase(fmt.Sprintf("%s/log.db", dbPath)); err != nil { if err := postLog.InitLogsDatabase(fmt.Sprintf("%s/log.db", dbPath)); err != nil {