From 10954c6161d9b2e557e7a84e8f7ca401e65c314d Mon Sep 17 00:00:00 2001 From: Tobias Gesellchen Date: Sun, 10 May 2026 22:29:58 +0200 Subject: [PATCH] feat(setup): XML migration honors per-field URL overrides MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds applyURLOverrides — a tiny helper that, given a PrivateCfg and the migration options map, copies any non-empty marge_url / stats_url / sw_update_url / bmx_url value into the matching PrivateCfg field. The helper runs after applyProxyOptions in both the read path (GetMigrationSummary's planned-config preview) and the write path (migrateViaXML's actual XML upload), so the planned diff and the file the migration writes both reflect what the user typed. Precedence: a literal *_url override wins over the legacy self/proxied/original mode set on the same field, because the user picked a URL and the migration honors it verbatim. Empty/missing overrides leave the field unchanged. The legacy mode handling stays in place for API back-compat — only the UI is moving away from it. Tests cover the helper directly, the override-vs-mode precedence rule, and a full GetMigrationSummary round-trip that verifies the override shows up in the rendered PlannedConfig XML. This is the data-layer half of the upcoming unified per-field URL editor in the Plan card; no UI changes here. Co-Authored-By: Claude Opus 4.7 (1M context) --- pkg/service/setup/setup.go | 42 +++++++++ pkg/service/setup/url_overrides_test.go | 110 ++++++++++++++++++++++++ 2 files changed, 152 insertions(+) create mode 100644 pkg/service/setup/url_overrides_test.go diff --git a/pkg/service/setup/setup.go b/pkg/service/setup/setup.go index 1772ada..31424fe 100644 --- a/pkg/service/setup/setup.go +++ b/pkg/service/setup/setup.go @@ -312,6 +312,12 @@ func (m *Manager) GetMigrationSummary(deviceIP, targetURL, proxyURL string, opti } } } + + // Per-field literal URL overrides win over both the canonical + // derivation and any self/proxied/original mode applied above — + // the user picked a URL, so the planned preview reflects exactly + // what the XML migration will write. + applyURLOverrides(&plannedCfg, options) // Note: CurrentConfig is set by checkCurrentConfig in all cases (success or failure) xmlContent, err := xml.MarshalIndent(plannedCfg, "", " ") @@ -697,6 +703,38 @@ func (m *Manager) applyProxyOptions(plannedCfg *PrivateCfg, proxyURL string, opt } } +// applyURLOverrides applies per-field literal URL overrides from the +// migration options map (marge_url / stats_url / sw_update_url / +// bmx_url) on top of an already-populated PrivateCfg. Empty or missing +// entries leave the field unchanged. +// +// These overrides win over the legacy "self/proxied/original" semantic +// applied by applyProxyOptions: if the user picked a literal URL, the +// migration honors it verbatim. The XML and Telnet write paths and +// the GetMigrationSummary read path all call this so the planned +// preview matches what migration actually writes. +func applyURLOverrides(cfg *PrivateCfg, options map[string]string) { + if cfg == nil || options == nil { + return + } + + if v := options["marge_url"]; v != "" { + cfg.MargeServerUrl = v + } + + if v := options["stats_url"]; v != "" { + cfg.StatsServerUrl = v + } + + if v := options["sw_update_url"]; v != "" { + cfg.SwUpdateUrl = v + } + + if v := options["bmx_url"]; v != "" { + cfg.BmxRegistryUrl = v + } +} + // checkRemoteServices checks for remote services files on the device func (m *Manager) checkRemoteServices(summary *MigrationSummary, deviceIP string) { client := m.NewSSH(deviceIP) @@ -898,6 +936,10 @@ func (m *Manager) migrateViaXML(deviceIP, targetURL, proxyURL string, options ma } } + // Per-field literal URL overrides take precedence over the + // proxy/original modes applied above — see applyURLOverrides. + applyURLOverrides(&cfg, options) + xmlContent, err := xml.MarshalIndent(cfg, "", " ") if err != nil { return logs, fmt.Errorf("failed to marshal XML: %w", err) diff --git a/pkg/service/setup/url_overrides_test.go b/pkg/service/setup/url_overrides_test.go new file mode 100644 index 0000000..b738bb5 --- /dev/null +++ b/pkg/service/setup/url_overrides_test.go @@ -0,0 +1,110 @@ +package setup + +import ( + "errors" + "strings" + "testing" +) + +func TestApplyURLOverrides_NilSafety(t *testing.T) { + // Should not panic on nil cfg or nil options. + applyURLOverrides(nil, map[string]string{"marge_url": "x"}) + + cfg := &PrivateCfg{} + applyURLOverrides(cfg, nil) +} + +func TestApplyURLOverrides_EmptyValueIsIgnored(t *testing.T) { + cfg := &PrivateCfg{ + MargeServerUrl: "http://example:8000", + } + applyURLOverrides(cfg, map[string]string{"marge_url": ""}) + + if cfg.MargeServerUrl != "http://example:8000" { + t.Errorf("MargeServerUrl was overwritten by empty override: %q", cfg.MargeServerUrl) + } +} + +func TestApplyURLOverrides_AllFour(t *testing.T) { + cfg := &PrivateCfg{ + MargeServerUrl: "default-marge", + StatsServerUrl: "default-stats", + SwUpdateUrl: "default-sw", + BmxRegistryUrl: "default-bmx", + } + + applyURLOverrides(cfg, map[string]string{ + "marge_url": "http://example:8000/marge", + "stats_url": "http://example:8000", + "sw_update_url": "http://example:8000/updates/soundtouch", + "bmx_url": "http://example:8000/bmx/registry/v1/services", + }) + + if cfg.MargeServerUrl != "http://example:8000/marge" { + t.Errorf("MargeServerUrl = %q", cfg.MargeServerUrl) + } + + if cfg.StatsServerUrl != "http://example:8000" { + t.Errorf("StatsServerUrl = %q", cfg.StatsServerUrl) + } + + if cfg.SwUpdateUrl != "http://example:8000/updates/soundtouch" { + t.Errorf("SwUpdateUrl = %q", cfg.SwUpdateUrl) + } + + if cfg.BmxRegistryUrl != "http://example:8000/bmx/registry/v1/services" { + t.Errorf("BmxRegistryUrl = %q", cfg.BmxRegistryUrl) + } +} + +// TestApplyURLOverrides_OverridesProxiedMode locks in the precedence +// rule: a literal *_url override wins over a self/proxied/original +// mode set on the same field. This is the load-bearing behaviour for +// the unified per-field URL editor in the Plan card — the user picked +// a URL and the migration honors it verbatim. +func TestApplyURLOverrides_OverridesProxiedMode(t *testing.T) { + m := &Manager{} + + cfg := &PrivateCfg{ + MargeServerUrl: "http://example:8000", // canonical default + } + + currentCfg := &PrivateCfg{ + MargeServerUrl: "https://streaming.bose.com", + } + + options := map[string]string{ + "marge": "proxied", // legacy mode + "marge_url": "http://example:8000/marge", + } + + m.applyProxyOptions(cfg, "http://proxy:8000", options, currentCfg) + applyURLOverrides(cfg, options) + + if cfg.MargeServerUrl != "http://example:8000/marge" { + t.Errorf("MargeServerUrl = %q, want literal override (not the /proxy/… form)", cfg.MargeServerUrl) + } +} + +// TestGetMigrationSummary_HonorsURLOverridesInPlannedConfig drives the +// PlannedConfig diff back from a real GetMigrationSummary to confirm +// the user's per-field URL overrides reach the planned XML the UI +// shows — closing the loop between the Plan card editor and the +// preview pane. +func TestGetMigrationSummary_HonorsURLOverridesInPlannedConfig(t *testing.T) { + m, host, cleanup := telnetSummaryEnv(t, nil, &fakeTelnet{dialErr: errors.New("not the focus of this test")}) + defer cleanup() + + options := map[string]string{ + "marge_url": "http://example:8000/marge", + } + + summary, err := m.GetMigrationSummary(host, "http://example:8000", "", options) + if err != nil { + t.Fatalf("GetMigrationSummary: %v", err) + } + + if !strings.Contains(summary.PlannedConfig, "http://example:8000/marge") { + t.Errorf("PlannedConfig should reflect marge_url override:\n%s", summary.PlannedConfig) + } +}