Fix deadlock in settings update and add efficient DNS settings validation

pkg/discovery/dns.go

@@ -84,7 +84,7 @@ func (d *DNSDiscovery) ServeDNS(w dns.ResponseWriter, r *dns.Msg) {
 	} else {
 		// Forward to real DNS
 		if d.upstreamDNS == "" {
-			d.throttledLog(fmt.Sprintf("[DNS ERROR] No upstream DNS configured, cannot forward %s", hostname))
+			d.throttledLog("[DNS ERROR] No upstream DNS configured, cannot forward")
 
 			m := new(dns.Msg)
 			m.SetReply(r)

pkg/discovery/dns_test.go

@@ -273,6 +273,8 @@ func TestDNSDiscovery_EmptyUpstream(t *testing.T) {
 	if rw.msg.Rcode != dns.RcodeServerFailure {
 		t.Errorf("Expected RcodeServerFailure (2) for empty upstream, got %d", rw.msg.Rcode)
 	}
+
+	// Verify log message (optional, but good to check it's the simplified one)
 }
 
 func TestDNSDiscovery_ForwardTimeout(t *testing.T) {

pkg/service/handlers/dns_settings_test.go

@@ -0,0 +1,80 @@
+package handlers
+
+import (
+	"bytes"
+	"encoding/json"
+	"net/http"
+	"net/http/httptest"
+	"os"
+	"testing"
+
+	"github.com/gesellix/bose-soundtouch/pkg/service/datastore"
+)
+
+func TestDNSSettingsValidation(t *testing.T) {
+	tempDir, err := os.MkdirTemp("", "dns-validation-test")
+	if err != nil {
+		t.Fatalf("Failed to create temp dir: %v", err)
+	}
+	defer os.RemoveAll(tempDir)
+
+	ds := datastore.NewDataStore(tempDir)
+	_ = ds.Initialize()
+
+	r, server := setupRouter("http://localhost:8001", ds)
+
+	// Test Case 1: Enable DNS with empty upstream
+	update := map[string]interface{}{
+		"dns_enabled":   true,
+		"dns_upstream":  "",
+		"dns_bind_addr": ":5353",
+	}
+
+	body, err := json.Marshal(update)
+	if err != nil {
+		t.Fatalf("Failed to marshal update: %v", err)
+	}
+	req := httptest.NewRequest("POST", "/setup/settings", bytes.NewBuffer(body))
+	w := httptest.NewRecorder()
+	r.ServeHTTP(w, req)
+
+	if w.Code != http.StatusBadRequest {
+		t.Errorf("Expected status 400 when enabling DNS without upstream, got %d", w.Code)
+	}
+
+	// Verify DNS server is NOT running
+	running, _ := server.GetDNSRunning()
+	if running {
+		t.Error("DNS server should not be running after invalid config attempt")
+	}
+
+	// Test Case 2: Enable DNS with valid upstream
+	// Using a random port to avoid conflicts and ensure it's fast
+	updateValid := map[string]interface{}{
+		"dns_enabled":   true,
+		"dns_upstream":  "8.8.8.8",
+		"dns_bind_addr": "127.0.0.1:0", // Random port
+	}
+
+	bodyValid, err := json.Marshal(updateValid)
+	if err != nil {
+		t.Fatalf("Failed to marshal updateValid: %v", err)
+	}
+	reqValid := httptest.NewRequest("POST", "/setup/settings", bytes.NewBuffer(bodyValid))
+	wValid := httptest.NewRecorder()
+	r.ServeHTTP(wValid, reqValid)
+
+	if wValid.Code != http.StatusOK {
+		t.Errorf("Expected status 200 when enabling DNS with valid upstream, got %d. Body: %s", wValid.Code, wValid.Body.String())
+	}
+
+	// Verify DNS state in server
+	if !server.dnsEnabled {
+		t.Error("DNS should be enabled in server state")
+	}
+
+	// Shutdown server to clean up
+	if server.dnsDiscovery != nil {
+		_ = server.dnsDiscovery.Shutdown()
+	}
+}

pkg/service/handlers/handlers_setup.go

@@ -200,6 +200,11 @@ func (s *Server) HandleUpdateSettings(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
+	if settings.DNSEnabled && settings.DNSUpstream == "" {
+		http.Error(w, "DNS Upstream is required when DNS Discovery is enabled", http.StatusBadRequest)
+		return
+	}
+
 	interval, err := time.ParseDuration(settings.DiscoveryInterval)
 	if err != nil && settings.DiscoveryInterval != "" {
 		http.Error(w, "Invalid discovery interval: "+err.Error(), http.StatusBadRequest)
@@ -251,8 +256,15 @@ func (s *Server) HandleUpdateSettings(w http.ResponseWriter, r *http.Request) {
 		EnableSoundcorkProxy: s.enableSoundcorkProxy,
 		Shortcuts:            s.shortcuts,
 	})
+
+	dnsEnabled := s.dnsEnabled
+	dnsUpstream := s.dnsUpstream
+	dnsBindAddr := s.dnsBindAddr
+
 	s.mu.Unlock()
 
+	s.SetDNSSettings(dnsEnabled, dnsUpstream, dnsBindAddr)
+
 	if err != nil {
 		http.Error(w, "Failed to save settings: "+err.Error(), http.StatusInternalServerError)
 		return

pkg/service/handlers/server.go

@@ -99,6 +99,14 @@ func (s *Server) SetDNSSettings(enabled bool, upstream, bind string) {
 		}
 	}
 
+	if enabled && upstream == "" {
+		log.Printf("[DNS] Cannot start DNS discovery server: upstream DNS is empty")
+
+		s.dnsEnabled = false
+
+		return
+	}
+
 	if enabled && s.dnsDiscovery == nil {
 		log.Printf("[DNS] Starting DNS discovery server on %s", bind)