mirror of
https://github.com/gesellix/Bose-SoundTouch.git
synced 2026-08-18 00:26:29 +00:00
fix(datastore): real account wins over "default" placeholder in dedup
ListAllDevices used to let an entry under accounts/default/devices/<id> replace the real-account entry for the same physical device whenever the default-side DeviceInfo.xml had a non-empty <name>. The consistency check then reported the device under "account default" even while the speaker was happily POST/PUT'ing to its actually-paired account — the operator saw "preset slot 1 present on speaker but missing from service" for slots that very obviously did exist, just under the real account they couldn't see. The dedup now treats "default" as a fallback placeholder: sorts it to the back of the iteration, and never lets it replace a real-account entry. A default-only device (fresh discovery, never paired) is still returned exactly as before. Also adds an orphan-detection finding in the consistency check that walks accounts/default/devices/ directly and flags entries whose deviceID is also paired under a real account, with a copy-pasteable rm -rf hint. We don't delete automatically — destructive filesystem actions need explicit operator consent (CLAUDE.md). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Sonnet 4.6
parent
97238eb07a
commit
e6954eed60
@@ -618,19 +618,47 @@ func (ds *DataStore) ListAllDevices() ([]models.ServiceDeviceInfo, error) {
|
||||
}
|
||||
|
||||
devices := []models.ServiceDeviceInfo{}
|
||||
seenIDs := make(map[string]bool)
|
||||
|
||||
type seenEntry struct {
|
||||
index int
|
||||
account string
|
||||
}
|
||||
|
||||
seenIDs := make(map[string]seenEntry)
|
||||
|
||||
for _, dir := range dirs {
|
||||
accounts, err := os.ReadDir(dir)
|
||||
entries, err := os.ReadDir(dir)
|
||||
if err != nil {
|
||||
continue
|
||||
}
|
||||
|
||||
for _, acc := range accounts {
|
||||
if !acc.IsDir() {
|
||||
continue
|
||||
// Sort "default" to the back so a real-account entry always
|
||||
// wins the first-seen race. "default" exists as a pre-pair
|
||||
// placeholder; once the speaker pairs with a real account it
|
||||
// becomes orphan state. Sort returns the same list for tests
|
||||
// because os.ReadDir is already sorted, and digits sort before
|
||||
// "default" in ASCII — but be defensive.
|
||||
accounts := make([]os.DirEntry, 0, len(entries))
|
||||
for _, e := range entries {
|
||||
if e.IsDir() {
|
||||
accounts = append(accounts, e)
|
||||
}
|
||||
}
|
||||
|
||||
sort.SliceStable(accounts, func(i, j int) bool {
|
||||
ai, aj := accounts[i].Name(), accounts[j].Name()
|
||||
if ai == accountIDDefault && aj != accountIDDefault {
|
||||
return false
|
||||
}
|
||||
|
||||
if aj == accountIDDefault && ai != accountIDDefault {
|
||||
return true
|
||||
}
|
||||
|
||||
return ai < aj
|
||||
})
|
||||
|
||||
for _, acc := range accounts {
|
||||
accDevices := ds.listDevicesInAccount(dir, acc.Name())
|
||||
for i := range accDevices {
|
||||
info := accDevices[i]
|
||||
@@ -641,21 +669,28 @@ func (ds *DataStore) ListAllDevices() ([]models.ServiceDeviceInfo, error) {
|
||||
key = info.IPAddress
|
||||
}
|
||||
|
||||
if !seenIDs[key] || info.Name != "" {
|
||||
if seenIDs[key] && info.Name != "" {
|
||||
// Replace previous empty-named entry with one that has a name
|
||||
for j := range devices {
|
||||
existing := &devices[j]
|
||||
if (existing.DeviceID != "" && existing.DeviceID == info.DeviceID) ||
|
||||
(existing.IPAddress != "" && existing.IPAddress == info.IPAddress) {
|
||||
devices[j] = info
|
||||
break
|
||||
}
|
||||
}
|
||||
} else if !seenIDs[key] {
|
||||
devices = append(devices, info)
|
||||
seenIDs[key] = true
|
||||
}
|
||||
entry, alreadySeen := seenIDs[key]
|
||||
if !alreadySeen {
|
||||
devices = append(devices, info)
|
||||
seenIDs[key] = seenEntry{index: len(devices) - 1, account: info.AccountID}
|
||||
|
||||
continue
|
||||
}
|
||||
|
||||
// Don't let a "default" entry replace a real-account
|
||||
// one. The speaker pairs to a real account; any
|
||||
// remaining "default" record is orphan state from
|
||||
// before pairing.
|
||||
if info.AccountID == accountIDDefault {
|
||||
continue
|
||||
}
|
||||
|
||||
// Allow real-account → real-account replacement only
|
||||
// when the prior was the placeholder "default" or had
|
||||
// no name (less authoritative).
|
||||
if entry.account == accountIDDefault || (devices[entry.index].Name == "" && info.Name != "") {
|
||||
devices[entry.index] = info
|
||||
seenIDs[key] = seenEntry{index: entry.index, account: info.AccountID}
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -664,6 +699,13 @@ func (ds *DataStore) ListAllDevices() ([]models.ServiceDeviceInfo, error) {
|
||||
return devices, nil
|
||||
}
|
||||
|
||||
// accountIDDefault is the placeholder account id assigned to records
|
||||
// that exist before a speaker has been paired with a real Marge
|
||||
// account. Treated as fallback in ListAllDevices and skipped in
|
||||
// consistency checks when a real-account entry exists for the same
|
||||
// device.
|
||||
const accountIDDefault = "default"
|
||||
|
||||
func (ds *DataStore) getPossibleDataDirs() []string {
|
||||
dirs := []string{}
|
||||
// Check primary data directory
|
||||
|
||||
@@ -0,0 +1,107 @@
|
||||
package datastore
|
||||
|
||||
import (
|
||||
"os"
|
||||
"path/filepath"
|
||||
"testing"
|
||||
)
|
||||
|
||||
// TestListAllDevices_RealAccountWinsOverDefault reproduces the operator's
|
||||
// observation: the same physical device exists on disk under both
|
||||
// accounts/default/devices/<id>/ (leftover pre-pair state) and
|
||||
// accounts/<real>/devices/<id>/ (active pairing). The dedupe loop used to
|
||||
// let "default" replace the real-account entry whenever the default
|
||||
// directory's DeviceInfo.xml had a non-empty <name>, which made
|
||||
// consistency checks report the device under "default" while the speaker
|
||||
// was happily POST/PUT'ing to the real account.
|
||||
//
|
||||
// "default" must be treated as a fallback placeholder, never as
|
||||
// authoritative when a real account also has the device.
|
||||
func TestListAllDevices_RealAccountWinsOverDefault(t *testing.T) {
|
||||
tempDir, err := os.MkdirTemp("", "datastore-default-orphan-*")
|
||||
if err != nil {
|
||||
t.Fatalf("tempdir: %v", err)
|
||||
}
|
||||
defer func() { _ = os.RemoveAll(tempDir) }()
|
||||
|
||||
deviceID := "AABBCCDDEEFF"
|
||||
|
||||
// Default-account entry: stale pre-pair state with a name.
|
||||
defaultDir := filepath.Join(tempDir, "accounts", "default", "devices", deviceID)
|
||||
if err := os.MkdirAll(defaultDir, 0755); err != nil {
|
||||
t.Fatalf("mkdir default: %v", err)
|
||||
}
|
||||
|
||||
if err := os.WriteFile(filepath.Join(defaultDir, "DeviceInfo.xml"), []byte(`<?xml version="1.0"?>
|
||||
<info deviceID="`+deviceID+`"><name>Discovered Device</name></info>`), 0644); err != nil {
|
||||
t.Fatalf("write default info: %v", err)
|
||||
}
|
||||
|
||||
// Real-account entry: the speaker is paired here.
|
||||
realDir := filepath.Join(tempDir, "accounts", "1111111", "devices", deviceID)
|
||||
if err := os.MkdirAll(realDir, 0755); err != nil {
|
||||
t.Fatalf("mkdir real: %v", err)
|
||||
}
|
||||
|
||||
if err := os.WriteFile(filepath.Join(realDir, "DeviceInfo.xml"), []byte(`<?xml version="1.0"?>
|
||||
<info deviceID="`+deviceID+`"><name>Living Room SoundTouch</name></info>`), 0644); err != nil {
|
||||
t.Fatalf("write real info: %v", err)
|
||||
}
|
||||
|
||||
ds := NewDataStore(tempDir)
|
||||
|
||||
devices, err := ds.ListAllDevices()
|
||||
if err != nil {
|
||||
t.Fatalf("ListAllDevices: %v", err)
|
||||
}
|
||||
|
||||
if len(devices) != 1 {
|
||||
t.Fatalf("expected exactly 1 deduped device, got %d: %+v", len(devices), devices)
|
||||
}
|
||||
|
||||
if devices[0].AccountID != "1111111" {
|
||||
t.Errorf("expected real account 1111111 to win dedup, got AccountID=%q (default leaked through)", devices[0].AccountID)
|
||||
}
|
||||
|
||||
if devices[0].Name != "Living Room SoundTouch" {
|
||||
t.Errorf("expected real-account Name to survive dedup, got %q", devices[0].Name)
|
||||
}
|
||||
}
|
||||
|
||||
// TestListAllDevices_DefaultOnlyDeviceStillSeen confirms the dedup
|
||||
// preference is one-way: a device that *only* exists under "default"
|
||||
// (e.g. fresh discovery, never paired) is still returned. We just
|
||||
// refuse to let "default" override a real account.
|
||||
func TestListAllDevices_DefaultOnlyDeviceStillSeen(t *testing.T) {
|
||||
tempDir, err := os.MkdirTemp("", "datastore-default-only-*")
|
||||
if err != nil {
|
||||
t.Fatalf("tempdir: %v", err)
|
||||
}
|
||||
defer func() { _ = os.RemoveAll(tempDir) }()
|
||||
|
||||
deviceID := "112233445566"
|
||||
defaultDir := filepath.Join(tempDir, "accounts", "default", "devices", deviceID)
|
||||
if err := os.MkdirAll(defaultDir, 0755); err != nil {
|
||||
t.Fatalf("mkdir: %v", err)
|
||||
}
|
||||
|
||||
if err := os.WriteFile(filepath.Join(defaultDir, "DeviceInfo.xml"), []byte(`<?xml version="1.0"?>
|
||||
<info deviceID="`+deviceID+`"><name>Newly Discovered</name></info>`), 0644); err != nil {
|
||||
t.Fatalf("write: %v", err)
|
||||
}
|
||||
|
||||
ds := NewDataStore(tempDir)
|
||||
|
||||
devices, err := ds.ListAllDevices()
|
||||
if err != nil {
|
||||
t.Fatalf("ListAllDevices: %v", err)
|
||||
}
|
||||
|
||||
if len(devices) != 1 {
|
||||
t.Fatalf("expected 1 device, got %d", len(devices))
|
||||
}
|
||||
|
||||
if devices[0].AccountID != "default" {
|
||||
t.Errorf("expected default-only device to be returned with AccountID=default, got %q", devices[0].AccountID)
|
||||
}
|
||||
}
|
||||
@@ -4,6 +4,7 @@ import (
|
||||
"context"
|
||||
"encoding/xml"
|
||||
"fmt"
|
||||
"os"
|
||||
"time"
|
||||
|
||||
"github.com/gesellix/bose-soundtouch/pkg/models"
|
||||
@@ -84,6 +85,12 @@ func runPresetsConsistencyCheck(ds *datastore.DataStore) []Finding {
|
||||
|
||||
var findings []Finding
|
||||
|
||||
// Surface orphan "default"-account entries for devices that are
|
||||
// also paired under a real account. The speaker pairs to one
|
||||
// account at a time; the leftover "default" record from the
|
||||
// pre-pair phase is stale state the operator can safely remove.
|
||||
findings = append(findings, detectOrphanDefaultEntries(ds, devices)...)
|
||||
|
||||
for i := range devices {
|
||||
dev := &devices[i]
|
||||
if dev.AccountID == "" || dev.DeviceID == "" {
|
||||
@@ -96,6 +103,48 @@ func runPresetsConsistencyCheck(ds *datastore.DataStore) []Finding {
|
||||
return findings
|
||||
}
|
||||
|
||||
// detectOrphanDefaultEntries walks the on-disk account directories
|
||||
// directly (not through ListAllDevices, which already dedupes "default"
|
||||
// out) and flags any device that exists under "default" *and* under a
|
||||
// real account. The fix is to delete accounts/default/devices/<id>/ —
|
||||
// we don't do it automatically because filesystem deletions need
|
||||
// explicit operator consent (CLAUDE.md "destructive actions" rule).
|
||||
func detectOrphanDefaultEntries(ds *datastore.DataStore, paired []models.ServiceDeviceInfo) []Finding {
|
||||
pairedReal := map[string]string{} // deviceID -> real accountID
|
||||
|
||||
for i := range paired {
|
||||
if paired[i].AccountID != "" && paired[i].AccountID != "default" && paired[i].DeviceID != "" {
|
||||
pairedReal[paired[i].DeviceID] = paired[i].AccountID
|
||||
}
|
||||
}
|
||||
|
||||
defaultDevicesDir := ds.AccountDevicesDir("default")
|
||||
|
||||
entries, err := os.ReadDir(defaultDevicesDir)
|
||||
if err != nil {
|
||||
return nil
|
||||
}
|
||||
|
||||
var findings []Finding
|
||||
|
||||
for _, e := range entries {
|
||||
if !e.IsDir() {
|
||||
continue
|
||||
}
|
||||
|
||||
deviceID := e.Name()
|
||||
if realAccount, ok := pairedReal[deviceID]; ok {
|
||||
findings = append(findings, Finding{
|
||||
Severity: SeverityWarning,
|
||||
Target: Target{Account: "default", Device: deviceID},
|
||||
Message: "Orphan \"default\" account entry: device " + deviceID + " is also paired under account " + realAccount + ". The default entry is leftover state from before pairing and is masked from consistency checks. Safe to delete: rm -rf <data-dir>/accounts/default/devices/" + deviceID,
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
return findings
|
||||
}
|
||||
|
||||
func checkOneDeviceConsistency(ds *datastore.DataStore, account, deviceID, ipAddress string) []Finding {
|
||||
target := Target{Account: account, Device: deviceID}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user