feat(settings): report which saved keys need a restart
Every settings update answered with a bare success, so the console could not tell a change that took effect at once from one written to the file that the running program would keep ignoring until it was restarted — the listener address, the storage paths, the Komari dashboard URL. Classify the patch instead. config.RestartRequiredKeys expands a patch into the dot-separated paths of its leaves and intersects them with the settings main reads before it starts serving, and the settings endpoint returns that list as data.restartRequired. It is a pure function of the patch, so UpdateSettings keeps its signature and nothing else has to change; the write succeeds either way, and the field only says which edits are not live. Matching is by overlap rather than equality, so a patch that names a section — replacing it, or deleting it with a null — reports the startup-only keys inside it, while a sibling subtree like webhook.endpoints is not mistaken for webhook.enabled. The result is never nil, so an update with nothing to report serializes as [] rather than null and a client can iterate it without a guard. The console reads the field in ConfigSection and warns instead of confirming, naming the keys that are pending; Settings.vue's hints for the Komari URL, the listen address and the storage paths now say which of their fields is affected rather than labelling the whole card.
This commit is contained in:
@@ -334,7 +334,7 @@ Browser WebSocket handshakes cannot carry custom headers, so `/api/system/getLog
|
||||
| `/api/server/getStatus` | GET | admin | Live server status (mirrors the Bot's `/status`). Query `?uuid=<uuid>` (or `all`). Returns `data: {<uuid>: {uuid, name, online, report}}`; `report` is `null` when the node has not reported yet. `404` for an unknown single uuid. |
|
||||
| `/api/server/exec` | POST | bot / admin | Execute a command. Body `{uuid: [<uuid>...], command}`. Dispatches a Komari task and polls until completion (or timeout). Returns `data: {taskID, results}`. |
|
||||
| `/api/settings/get` | GET | admin | `?type=global\|bot_user_config\|bot_node_config` | Returns `data: {config}`, where `config` is the selected config file's content (same layout as the JSON file). |
|
||||
| `/api/settings/set` | POST | admin | `?type=<same types>` + JSON body of partial updates, e.g. `{"system":{"debugMode":true}}` | Deep-merges the body into the selected config file, persists it, and reloads it in memory. Only the given keys change; arrays replace. See [What applies without a restart](#what-applies-without-a-restart). |
|
||||
| `/api/settings/set` | POST | admin | `?type=<same types>` + JSON body of partial updates, e.g. `{"system":{"debugMode":true}}` | Deep-merges the body into the selected config file, persists it, and reloads it in memory. Only the given keys change; arrays replace. Returns `data: {type, restartRequired}`, where `restartRequired` lists the keys the update changed that are only read at startup (see [What applies without a restart](#what-applies-without-a-restart)) — the write succeeds regardless, this only says which edits are not live yet. Always an array, empty when everything took effect. |
|
||||
| `/api/webhook/add` | POST | admin | Add an incoming webhook endpoint. Body `{name, enabled?, token?, notifyPipes?}` — only the fields given are stored, the rest start at their defaults. `409` when the name is already configured. |
|
||||
| `/api/webhook/modify` | POST | admin | Change an existing endpoint. Body `{name, ...}` — the fields given are the fields that change (same partial-update rule as `/api/settings/set`, but scoped to one endpoint). `404` for an unknown name, `400` when no other field is given. |
|
||||
| `/api/webhook/delete` | POST | admin | Remove an endpoint. Body `{name}`. `404` for an unknown name. |
|
||||
|
||||
@@ -6,6 +6,7 @@ import (
|
||||
"errors"
|
||||
"fmt"
|
||||
"os"
|
||||
"strings"
|
||||
"sync"
|
||||
|
||||
"nukumizu-backend/global"
|
||||
@@ -23,6 +24,89 @@ const (
|
||||
// accepted constants above.
|
||||
var ErrUnsupportedSettingsType = errors.New("unsupported settings type")
|
||||
|
||||
// startupOnlySettings are the config.json keys that are read once before the
|
||||
// program starts serving and never again: the listener addresses, the paths the
|
||||
// databases are opened from, and the Komari dashboard its client is built
|
||||
// against. Editing one writes the file and replaces the in-memory
|
||||
// configuration, but the running program keeps the old value, so an update that
|
||||
// touches one is reported back to the caller instead of being silently
|
||||
// accepted.
|
||||
//
|
||||
// Keep this in step with main: these are exactly the settings main reads before
|
||||
// the HTTP server comes up. Everything else — controllerMethod, networkProxy,
|
||||
// the message templates, the debug switches — is picked up at runtime.
|
||||
var startupOnlySettings = []string{
|
||||
"system.listenAddr",
|
||||
"system.listenPort",
|
||||
"webhook.enabled",
|
||||
"webhook.listenAddr",
|
||||
"webhook.listenPort",
|
||||
"komari.dashboardURL",
|
||||
"dataPath",
|
||||
"dbPath",
|
||||
}
|
||||
|
||||
// RestartRequiredKeys lists the settings in patch that only take effect at
|
||||
// startup, as dot-separated paths, in the order startupOnlySettings declares
|
||||
// them. Only config.json carries such settings; an update to one of the other
|
||||
// files always reports nothing.
|
||||
//
|
||||
// The write itself succeeds either way — this is advice for the user, not a
|
||||
// rejection. The result is never nil, so a caller can put it straight into a
|
||||
// JSON response and get [] rather than null.
|
||||
func RestartRequiredKeys(settingsType string, patch map[string]interface{}) []string {
|
||||
keys := []string{}
|
||||
if settingsType != SettingGlobal {
|
||||
return keys
|
||||
}
|
||||
|
||||
patched := patchPaths(patch)
|
||||
for _, watched := range startupOnlySettings {
|
||||
for _, path := range patched {
|
||||
if pathsOverlap(path, watched) {
|
||||
keys = append(keys, watched)
|
||||
break
|
||||
}
|
||||
}
|
||||
}
|
||||
return keys
|
||||
}
|
||||
|
||||
// patchPaths expands a nested settings patch into the dot-separated paths of its
|
||||
// leaves. An object is descended into rather than reported, so a patch that only
|
||||
// names sections still resolves to the keys it changes, and a JSON null is a
|
||||
// leaf because it deletes the key it names.
|
||||
func patchPaths(patch map[string]interface{}) []string {
|
||||
paths := []string{}
|
||||
var walk func(prefix string, node map[string]interface{})
|
||||
walk = func(prefix string, node map[string]interface{}) {
|
||||
for key, value := range node {
|
||||
path := key
|
||||
if prefix != "" {
|
||||
path = prefix + "." + key
|
||||
}
|
||||
if nested, ok := value.(map[string]interface{}); ok && nested != nil {
|
||||
walk(path, nested)
|
||||
continue
|
||||
}
|
||||
paths = append(paths, path)
|
||||
}
|
||||
}
|
||||
walk("", patch)
|
||||
return paths
|
||||
}
|
||||
|
||||
// pathsOverlap reports whether a patched path and a watched setting can affect
|
||||
// each other: they are the same key, the patch names something inside the
|
||||
// watched setting, or the patch names a section the watched setting lives in.
|
||||
// The last case matters because a patch may replace a whole section, which
|
||||
// changes every key under it.
|
||||
func pathsOverlap(patched, watched string) bool {
|
||||
return patched == watched ||
|
||||
strings.HasPrefix(patched, watched+".") ||
|
||||
strings.HasPrefix(watched, patched+".")
|
||||
}
|
||||
|
||||
// settingsLock serializes read-modify-write access to the on-disk configuration
|
||||
// files so concurrent admin edits (UpdateSettings) and the node tracker's
|
||||
// background save (SaveBotNodeConfig) cannot lose each other's updates.
|
||||
|
||||
@@ -4,6 +4,7 @@ import (
|
||||
"encoding/json"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"reflect"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
@@ -148,6 +149,162 @@ func TestUpdateSettingsReplacesArraysAndKeepsNumbers(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestRestartRequiredKeys(t *testing.T) {
|
||||
cases := []struct {
|
||||
name string
|
||||
settingsType string
|
||||
patch map[string]interface{}
|
||||
want []string
|
||||
}{
|
||||
{
|
||||
name: "a runtime switch needs no restart",
|
||||
settingsType: SettingGlobal,
|
||||
patch: map[string]interface{}{"system": map[string]interface{}{"debugMode": true}},
|
||||
want: []string{},
|
||||
},
|
||||
{
|
||||
name: "the listen port does",
|
||||
settingsType: SettingGlobal,
|
||||
patch: map[string]interface{}{"system": map[string]interface{}{"listenPort": "9090"}},
|
||||
want: []string{"system.listenPort"},
|
||||
},
|
||||
{
|
||||
name: "only the startup key of a mixed patch is reported",
|
||||
settingsType: SettingGlobal,
|
||||
patch: map[string]interface{}{
|
||||
"system": map[string]interface{}{"listenPort": "9090", "debugMode": true},
|
||||
},
|
||||
want: []string{"system.listenPort"},
|
||||
},
|
||||
{
|
||||
name: "a top-level path is reported",
|
||||
settingsType: SettingGlobal,
|
||||
patch: map[string]interface{}{"dataPath": "/srv/data"},
|
||||
want: []string{"dataPath"},
|
||||
},
|
||||
{
|
||||
name: "results follow the declared order, not the patch order",
|
||||
settingsType: SettingGlobal,
|
||||
patch: map[string]interface{}{"dbPath": "/srv/db", "dataPath": "/srv/data"},
|
||||
want: []string{"dataPath", "dbPath"},
|
||||
},
|
||||
{
|
||||
name: "deleting a startup key with null is reported",
|
||||
settingsType: SettingGlobal,
|
||||
patch: map[string]interface{}{"system": map[string]interface{}{"listenPort": nil}},
|
||||
want: []string{"system.listenPort"},
|
||||
},
|
||||
{
|
||||
name: "replacing a whole section reports the startup keys inside it",
|
||||
settingsType: SettingGlobal,
|
||||
patch: map[string]interface{}{"webhook": map[string]interface{}{"listenAddr": "127.0.0.1"}},
|
||||
want: []string{"webhook.listenAddr"},
|
||||
},
|
||||
{
|
||||
// Deleting the section resets the URL to its built-in default.
|
||||
name: "deleting a section the startup key lives in reports it",
|
||||
settingsType: SettingGlobal,
|
||||
patch: map[string]interface{}{"komari": nil},
|
||||
want: []string{"komari.dashboardURL"},
|
||||
},
|
||||
{
|
||||
name: "deleting a section reports every startup key inside it",
|
||||
settingsType: SettingGlobal,
|
||||
patch: map[string]interface{}{"webhook": nil},
|
||||
want: []string{"webhook.enabled", "webhook.listenAddr", "webhook.listenPort"},
|
||||
},
|
||||
{
|
||||
// An empty object merges nothing, so it changes no key and needs no
|
||||
// restart — surprising enough to pin.
|
||||
name: "an empty object changes nothing",
|
||||
settingsType: SettingGlobal,
|
||||
patch: map[string]interface{}{"komari": map[string]interface{}{}},
|
||||
want: []string{},
|
||||
},
|
||||
{
|
||||
// webhook.endpoints must not be mistaken for webhook.enabled.
|
||||
name: "a sibling subtree is not mistaken for the startup key",
|
||||
settingsType: SettingGlobal,
|
||||
patch: map[string]interface{}{
|
||||
"webhook": map[string]interface{}{
|
||||
"endpoints": map[string]interface{}{"example": map[string]interface{}{"enabled": true}},
|
||||
},
|
||||
},
|
||||
want: []string{},
|
||||
},
|
||||
{
|
||||
// The Komari credentials are re-read on the next login, so only the
|
||||
// dashboard URL is startup-only.
|
||||
name: "komari credentials are not startup-only",
|
||||
settingsType: SettingGlobal,
|
||||
patch: map[string]interface{}{
|
||||
"komari": map[string]interface{}{
|
||||
"account": map[string]interface{}{"username": "admin", "password": "x"},
|
||||
},
|
||||
},
|
||||
want: []string{},
|
||||
},
|
||||
{
|
||||
name: "controller settings are not startup-only",
|
||||
settingsType: SettingGlobal,
|
||||
patch: map[string]interface{}{
|
||||
"controllerMethod": map[string]interface{}{
|
||||
"telegram": map[string]interface{}{"enabled": true, "botToken": "t"},
|
||||
},
|
||||
},
|
||||
want: []string{},
|
||||
},
|
||||
{
|
||||
name: "an empty patch reports nothing",
|
||||
settingsType: SettingGlobal,
|
||||
patch: map[string]interface{}{},
|
||||
want: []string{},
|
||||
},
|
||||
{
|
||||
// Only config.json has settings that are read once at startup.
|
||||
name: "the other settings files never need a restart",
|
||||
settingsType: SettingBotUserConfig,
|
||||
patch: map[string]interface{}{"dataPath": "/srv/data"},
|
||||
want: []string{},
|
||||
},
|
||||
}
|
||||
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
got := RestartRequiredKeys(tc.settingsType, tc.patch)
|
||||
if !reflect.DeepEqual(got, tc.want) {
|
||||
t.Errorf("RestartRequiredKeys() = %v, want %v", got, tc.want)
|
||||
}
|
||||
if got == nil {
|
||||
t.Error("the result must never be nil, so it serializes as [] rather than null")
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestRestartRequiredKeysIsAdvisory pins that reporting a startup-only key does
|
||||
// not stop the write: the caller is told, the file is still updated.
|
||||
func TestRestartRequiredKeysIsAdvisory(t *testing.T) {
|
||||
writeTempConfig(t, &global.ConfigPath.Global, `{"system":{"listenPort":"8080"}}`)
|
||||
|
||||
keys := RestartRequiredKeys(SettingGlobal, map[string]interface{}{
|
||||
"system": map[string]interface{}{"listenPort": "9090"},
|
||||
})
|
||||
if len(keys) != 1 {
|
||||
t.Fatalf("expected the listen port to be reported, got %v", keys)
|
||||
}
|
||||
|
||||
if err := UpdateSettings(SettingGlobal, map[string]interface{}{
|
||||
"system": map[string]interface{}{"listenPort": "9090"},
|
||||
}); err != nil {
|
||||
t.Fatalf("UpdateSettings: %v", err)
|
||||
}
|
||||
|
||||
if got := Current().System.ListenPort; got != "9090" {
|
||||
t.Errorf("the update was not applied: listenPort = %q", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestSettingsTypeValidation(t *testing.T) {
|
||||
for _, valid := range []string{SettingGlobal, SettingBotUserConfig, SettingBotNodeConfig} {
|
||||
if !IsValidSettingsType(valid) {
|
||||
|
||||
@@ -13,6 +13,10 @@ export const serverApi = {
|
||||
|
||||
// /api/settings/get?type=… / /api/settings/set?type=…
|
||||
// get → { success, message, data: { config } }.
|
||||
// set → { success, message, data: { type, restartRequired } }, where
|
||||
// restartRequired lists the keys the update changed that are only read at
|
||||
// startup, so the caller can say which edits are not live yet. It is
|
||||
// always an array, empty when the whole update took effect.
|
||||
// `type` is one of global | bot_user_config | bot_node_config.
|
||||
// For set, pass a partial object; a JSON null value removes that key.
|
||||
export const settingsApi = {
|
||||
|
||||
@@ -133,8 +133,15 @@ async function save() {
|
||||
const patch = wrapRoot(props.section, nest(obj));
|
||||
saving.value = true;
|
||||
try {
|
||||
await settingsApi.set('global', patch);
|
||||
toast.success(`${props.section.title} saved`);
|
||||
const res = await settingsApi.set('global', patch);
|
||||
// The backend reports the keys it wrote that are only read at startup.
|
||||
// A plain "saved" would suggest those are live too.
|
||||
const pending = (res && res.data && res.data.restartRequired) || [];
|
||||
if (pending.length) {
|
||||
toast.warn(`${props.section.title} saved — restart to apply: ${pending.join(', ')}`, 7000);
|
||||
} else {
|
||||
toast.success(`${props.section.title} saved`);
|
||||
}
|
||||
emit('saved');
|
||||
} catch (e) {
|
||||
toast.error('Failed to save: ' + e.message);
|
||||
|
||||
@@ -11,7 +11,7 @@ const sections = [
|
||||
{
|
||||
id: 'system',
|
||||
title: 'System',
|
||||
hint: 'HTTP listener and global runtime switches.',
|
||||
hint: 'HTTP listener and global runtime switches. A changed listen address or port applies on restart.',
|
||||
root: ['system'],
|
||||
fields: [
|
||||
{ key: 'debugMode', type: 'bool', label: 'Debug mode', help: 'Skipped X-Timestamp checks and verbose debug logging.' },
|
||||
@@ -37,7 +37,7 @@ const sections = [
|
||||
{
|
||||
id: 'komari',
|
||||
title: 'Komari dashboard',
|
||||
hint: 'Connection the monitor reads node data from. Takes effect on restart.',
|
||||
hint: 'Connection the monitor reads node data from. The URL applies on restart; the account is re-read on the next login.',
|
||||
root: ['komari'],
|
||||
fields: [
|
||||
{ key: 'dashboardURL', type: 'text', label: 'Dashboard URL' },
|
||||
|
||||
+8
-1
@@ -47,6 +47,10 @@ func SettingsGetHandler(w http.ResponseWriter, r *http.Request) {
|
||||
// {"system": {"debugMode": true}}
|
||||
//
|
||||
// Multiple entries may be given at once; only the provided keys are changed.
|
||||
//
|
||||
// The response carries data.restartRequired: the keys the update changed that
|
||||
// are only read at startup, so the caller can say which edits are not live yet.
|
||||
// It is empty for an update that took effect in full.
|
||||
func SettingsSetHandler(w http.ResponseWriter, r *http.Request) {
|
||||
if !utils.Auth(w, r, "POST", "admin") {
|
||||
return
|
||||
@@ -80,7 +84,10 @@ func SettingsSetHandler(w http.ResponseWriter, r *http.Request) {
|
||||
return
|
||||
}
|
||||
|
||||
// The write landed either way. This only tells the caller which of the keys
|
||||
// it changed will not be live until the program is restarted.
|
||||
utils.SendSuccessResponse(w, "settings updated successfully", map[string]interface{}{
|
||||
"type": settingsType,
|
||||
"type": settingsType,
|
||||
"restartRequired": config.RestartRequiredKeys(settingsType, patch),
|
||||
})
|
||||
}
|
||||
|
||||
@@ -0,0 +1,128 @@
|
||||
package handler
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strconv"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"nukumizu-backend/config"
|
||||
"nukumizu-backend/global"
|
||||
)
|
||||
|
||||
// tempConfigFile points one of the settings paths at a throwaway file, so a
|
||||
// test can drive the settings API without touching the config files in the
|
||||
// working directory.
|
||||
func tempConfigFile(t *testing.T, field *string, content string) {
|
||||
t.Helper()
|
||||
path := filepath.Join(t.TempDir(), "config.json")
|
||||
if err := os.WriteFile(path, []byte(content), 0o644); err != nil {
|
||||
t.Fatalf("write temp config: %v", err)
|
||||
}
|
||||
original := *field
|
||||
*field = path
|
||||
t.Cleanup(func() { *field = original })
|
||||
}
|
||||
|
||||
// settingsSetRequest builds an authenticated POST for the settings endpoint.
|
||||
func settingsSetRequest(t *testing.T, settingsType, body string) *http.Request {
|
||||
t.Helper()
|
||||
req := httptest.NewRequest(
|
||||
http.MethodPost,
|
||||
"/api/settings/set?type="+settingsType,
|
||||
strings.NewReader(body),
|
||||
)
|
||||
req.Header.Set("X-Token", "test-admin-token")
|
||||
req.Header.Set("X-Timestamp", strconv.FormatInt(time.Now().Unix(), 10))
|
||||
return req
|
||||
}
|
||||
|
||||
// settingsSetResponse is the envelope /api/settings/set answers with.
|
||||
type settingsSetResponse struct {
|
||||
Success bool `json:"success"`
|
||||
Data struct {
|
||||
Type string `json:"type"`
|
||||
RestartRequired []string `json:"restartRequired"`
|
||||
} `json:"data"`
|
||||
}
|
||||
|
||||
func decodeSettingsSetResponse(t *testing.T, w *httptest.ResponseRecorder) settingsSetResponse {
|
||||
t.Helper()
|
||||
var body settingsSetResponse
|
||||
if err := json.Unmarshal(w.Body.Bytes(), &body); err != nil {
|
||||
t.Fatalf("decode response: %v; body=%s", err, w.Body.String())
|
||||
}
|
||||
return body
|
||||
}
|
||||
|
||||
// TestSettingsSetReportsStartupOnlyKeys covers the field the console reads to
|
||||
// tell the user which of their edits are not live yet.
|
||||
func TestSettingsSetReportsStartupOnlyKeys(t *testing.T) {
|
||||
setupAdminToken()
|
||||
tempConfigFile(t, &global.ConfigPath.Global, `{"system":{"debugMode":false,"listenPort":"8080"}}`)
|
||||
|
||||
w := httptest.NewRecorder()
|
||||
SettingsSetHandler(w, settingsSetRequest(t, "global",
|
||||
`{"system":{"listenPort":"9090","debugMode":true}}`))
|
||||
|
||||
if w.Code != http.StatusOK {
|
||||
t.Fatalf("status = %d, body = %s", w.Code, w.Body.String())
|
||||
}
|
||||
body := decodeSettingsSetResponse(t, w)
|
||||
if !body.Success {
|
||||
t.Fatalf("not a success envelope: %s", w.Body.String())
|
||||
}
|
||||
if body.Data.Type != "global" {
|
||||
t.Errorf("data.type = %q, want global", body.Data.Type)
|
||||
}
|
||||
if len(body.Data.RestartRequired) != 1 || body.Data.RestartRequired[0] != "system.listenPort" {
|
||||
t.Errorf("restartRequired = %v, want [system.listenPort]", body.Data.RestartRequired)
|
||||
}
|
||||
|
||||
// Being reported as startup-only must not stop the write.
|
||||
data, err := config.GetSettings(config.SettingGlobal)
|
||||
if err != nil {
|
||||
t.Fatalf("GetSettings: %v", err)
|
||||
}
|
||||
for _, want := range []string{`"listenPort": "9090"`, `"debugMode": true`} {
|
||||
if !strings.Contains(string(data), want) {
|
||||
t.Errorf("the patch was not applied, missing %s:\n%s", want, data)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TestSettingsSetRestartRequiredIsAlwaysAnArray pins the shape a client
|
||||
// iterates over: an update with nothing to report must answer [] and not null.
|
||||
func TestSettingsSetRestartRequiredIsAlwaysAnArray(t *testing.T) {
|
||||
setupAdminToken()
|
||||
tempConfigFile(t, &global.ConfigPath.Global, `{"system":{"debugMode":false}}`)
|
||||
|
||||
w := httptest.NewRecorder()
|
||||
SettingsSetHandler(w, settingsSetRequest(t, "global", `{"system":{"debugMode":true}}`))
|
||||
|
||||
if w.Code != http.StatusOK {
|
||||
t.Fatalf("status = %d, body = %s", w.Code, w.Body.String())
|
||||
}
|
||||
if !strings.Contains(w.Body.String(), `"restartRequired":[]`) {
|
||||
t.Errorf("restartRequired should serialize as an empty array: %s", w.Body.String())
|
||||
}
|
||||
}
|
||||
|
||||
// TestSettingsSetRejectsUnknownType keeps the failure path intact now that the
|
||||
// success path computes an extra field.
|
||||
func TestSettingsSetRejectsUnknownType(t *testing.T) {
|
||||
setupAdminToken()
|
||||
tempConfigFile(t, &global.ConfigPath.Global, `{}`)
|
||||
|
||||
w := httptest.NewRecorder()
|
||||
SettingsSetHandler(w, settingsSetRequest(t, "nonsense", `{}`))
|
||||
|
||||
if w.Code != http.StatusBadRequest {
|
||||
t.Errorf("status = %d, want 400", w.Code)
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user