From ec8bbb2f86c040610cc55d90d3673654b7c44b37 Mon Sep 17 00:00:00 2001 From: Tobias Gesellchen Date: Sat, 21 Feb 2026 00:28:29 +0100 Subject: [PATCH] Lint: cleanup --- cmd/soundtouch-service/main.go | 2 + pkg/service/handlers/handlers_mgmt.go | 58 +++++++++++----- pkg/service/handlers/server.go | 4 +- pkg/service/spotify/service.go | 97 ++++++++++++++++++--------- pkg/service/spotify/service_test.go | 12 ++-- 5 files changed, 118 insertions(+), 55 deletions(-) diff --git a/cmd/soundtouch-service/main.go b/cmd/soundtouch-service/main.go index 7a53c69..e50cecd 100644 --- a/cmd/soundtouch-service/main.go +++ b/cmd/soundtouch-service/main.go @@ -230,10 +230,12 @@ func main() { config.dataDir, ) server.SetSpotifyService(spotifyService) + clientIDPrefix := config.spotifyClientID if len(clientIDPrefix) > 8 { clientIDPrefix = clientIDPrefix[:8] } + log.Printf("Spotify service initialized (client ID: %s...)", clientIDPrefix) } diff --git a/pkg/service/handlers/handlers_mgmt.go b/pkg/service/handlers/handlers_mgmt.go index d1a5c37..2e4f270 100644 --- a/pkg/service/handlers/handlers_mgmt.go +++ b/pkg/service/handlers/handlers_mgmt.go @@ -27,6 +27,7 @@ func (s *Server) HandleMgmtListSpeakers(w http.ResponseWriter, r *http.Request) allDevices, err := s.ds.ListAllDevices() if err != nil { log.Printf("[Mgmt] Failed to list devices: %v", err) + allDevices = nil } @@ -38,7 +39,8 @@ func (s *Server) HandleMgmtListSpeakers(w http.ResponseWriter, r *http.Request) } speakers := make([]speaker, 0, len(allDevices)) - for _, d := range allDevices { + for i := range allDevices { + d := &allDevices[i] speakers = append(speakers, speaker{ IPAddress: d.IPAddress, Name: d.Name, @@ -48,9 +50,12 @@ func (s *Server) HandleMgmtListSpeakers(w http.ResponseWriter, r *http.Request) } w.Header().Set("Content-Type", "application/json") - _ = json.NewEncoder(w).Encode(map[string]interface{}{ + + if err := json.NewEncoder(w).Encode(map[string]interface{}{ "speakers": speakers, - }) + }); err != nil { + log.Printf("[Mgmt] Failed to encode speakers: %v", err) + } } // HandleMgmtDeviceEvents returns events for a device (currently a placeholder). @@ -80,9 +85,11 @@ func (s *Server) HandleMgmtDeviceEvents(w http.ResponseWriter, r *http.Request) }) } - _ = json.NewEncoder(w).Encode(map[string]interface{}{ + if err := json.NewEncoder(w).Encode(map[string]interface{}{ "events": result, - }) + }); err != nil { + log.Printf("[Mgmt] Failed to encode events: %v", err) + } } // HandleMgmtSpotifyInit starts the Spotify OAuth flow by returning an authorization URL. @@ -101,9 +108,12 @@ func (s *Server) HandleMgmtSpotifyInit(w http.ResponseWriter, _ *http.Request) { w.Header().Set("Content-Type", "application/json") enc := json.NewEncoder(w) enc.SetEscapeHTML(false) - _ = enc.Encode(map[string]string{ + + if err := enc.Encode(map[string]string{ "redirectUrl": redirectURL, - }) + }); err != nil { + log.Printf("[Mgmt] Failed to encode redirect URL: %v", err) + } } // HandleMgmtSpotifyCallback is the browser OAuth callback from Spotify. @@ -118,6 +128,7 @@ func (s *Server) HandleMgmtSpotifyCallback(w http.ResponseWriter, r *http.Reques w.Header().Set("Content-Type", "text/html") w.WriteHeader(http.StatusServiceUnavailable) _, _ = w.Write([]byte(`

Error

Spotify integration not configured

`)) + return } @@ -125,6 +136,7 @@ func (s *Server) HandleMgmtSpotifyCallback(w http.ResponseWriter, r *http.Reques w.Header().Set("Content-Type", "text/html") w.WriteHeader(http.StatusBadRequest) _, _ = w.Write([]byte(`

Spotify Authorization Failed

Error: ` + errMsg + `

`)) + return } @@ -133,6 +145,7 @@ func (s *Server) HandleMgmtSpotifyCallback(w http.ResponseWriter, r *http.Reques w.Header().Set("Content-Type", "text/html") w.WriteHeader(http.StatusBadRequest) _, _ = w.Write([]byte(`

Missing authorization code

`)) + return } @@ -141,6 +154,7 @@ func (s *Server) HandleMgmtSpotifyCallback(w http.ResponseWriter, r *http.Reques w.Header().Set("Content-Type", "text/html") w.WriteHeader(http.StatusInternalServerError) _, _ = w.Write([]byte(`

Error

Token exchange failed

`)) + return } @@ -170,6 +184,7 @@ func (s *Server) HandleMgmtSpotifyConfirm(w http.ResponseWriter, r *http.Request if err := svc.ExchangeCodeAndStore(code); err != nil { log.Printf("[Mgmt] Spotify confirm failed: %v", err) http.Error(w, `{"error":"token exchange failed"}`, http.StatusInternalServerError) + return } @@ -192,9 +207,12 @@ func (s *Server) HandleMgmtSpotifyAccounts(w http.ResponseWriter, _ *http.Reques accounts := svc.GetAccounts() w.Header().Set("Content-Type", "application/json") - _ = json.NewEncoder(w).Encode(map[string]interface{}{ + + if err := json.NewEncoder(w).Encode(map[string]interface{}{ "accounts": accounts, - }) + }); err != nil { + log.Printf("[Mgmt] Failed to encode accounts: %v", err) + } } // HandleMgmtSpotifyToken returns a fresh Spotify access token and username. @@ -212,14 +230,18 @@ func (s *Server) HandleMgmtSpotifyToken(w http.ResponseWriter, _ *http.Request) if err != nil { log.Printf("[Mgmt] Spotify token error: %v", err) http.Error(w, `{"error":"no token available"}`, http.StatusInternalServerError) + return } w.Header().Set("Content-Type", "application/json") - _ = json.NewEncoder(w).Encode(map[string]string{ + + if err := json.NewEncoder(w).Encode(map[string]string{ "access_token": accessToken, "username": username, - }) + }); err != nil { + log.Printf("[Mgmt] Failed to encode token: %v", err) + } } // HandleMgmtSpotifyEntity resolves a Spotify URI to name and image URL. @@ -239,24 +261,28 @@ func (s *Server) HandleMgmtSpotifyEntity(w http.ResponseWriter, r *http.Request) return } - var req struct { + var request struct { URI string `json:"uri"` } - if err := json.Unmarshal(body, &req); err != nil || req.URI == "" { + if unmarshalErr := json.Unmarshal(body, &request); unmarshalErr != nil || request.URI == "" { http.Error(w, `{"error":"missing or invalid uri"}`, http.StatusBadRequest) return } - name, imageURL, err := svc.ResolveEntity(req.URI) + name, imageURL, err := svc.ResolveEntity(request.URI) if err != nil { log.Printf("[Mgmt] Spotify entity resolve error: %v", err) http.Error(w, `{"error":"entity resolution failed"}`, http.StatusInternalServerError) + return } w.Header().Set("Content-Type", "application/json") - _ = json.NewEncoder(w).Encode(map[string]string{ + + if err := json.NewEncoder(w).Encode(map[string]string{ "name": name, "imageUrl": imageURL, - }) + }); err != nil { + log.Printf("[Mgmt] Failed to encode entity: %v", err) + } } diff --git a/pkg/service/handlers/server.go b/pkg/service/handlers/server.go index a2f688e..1c28d2e 100644 --- a/pkg/service/handlers/server.go +++ b/pkg/service/handlers/server.go @@ -47,7 +47,7 @@ type Server struct { spotifyClientSecret string spotifyRedirectURI string baseURL string - spotifyService *spotify.SpotifyService + spotifyService *spotify.Service } // NewServer creates a new SoundTouch service server. @@ -251,7 +251,7 @@ func (s *Server) SetBaseURL(baseURL string) { } // SetSpotifyService sets the Spotify OAuth service. -func (s *Server) SetSpotifyService(ss *spotify.SpotifyService) { +func (s *Server) SetSpotifyService(ss *spotify.Service) { s.mu.Lock() defer s.mu.Unlock() diff --git a/pkg/service/spotify/service.go b/pkg/service/spotify/service.go index 2aad5e0..680e303 100644 --- a/pkg/service/spotify/service.go +++ b/pkg/service/spotify/service.go @@ -27,8 +27,8 @@ const ( SpotifyScopes = "streaming user-read-private user-read-email user-read-playback-state user-modify-playback-state" ) -// SpotifyAccount represents a stored Spotify account with tokens. -type SpotifyAccount struct { +// Account represents a stored Spotify account with tokens. +type Account struct { UserID string `json:"user_id"` DisplayName string `json:"display_name"` Email string `json:"email"` @@ -37,51 +37,53 @@ type SpotifyAccount struct { ExpiresAt int64 `json:"expires_at"` } -// SpotifyService manages Spotify OAuth flow and token lifecycle. -type SpotifyService struct { +// Service manages Spotify OAuth flow and token lifecycle. +type Service struct { clientID string clientSecret string redirectURI string dataDir string mu sync.RWMutex - accounts map[string]*SpotifyAccount + accounts map[string]*Account // Overridable URLs for testing tokenURL string apiBase string } -// NewSpotifyService creates a new SpotifyService and loads any persisted accounts. -func NewSpotifyService(clientID, clientSecret, redirectURI, dataDir string) *SpotifyService { - s := &SpotifyService{ +// NewSpotifyService creates a new Service and loads any persisted accounts. +func NewSpotifyService(clientID, clientSecret, redirectURI, dataDir string) *Service { + s := &Service{ clientID: clientID, clientSecret: clientSecret, redirectURI: redirectURI, dataDir: dataDir, - accounts: make(map[string]*SpotifyAccount), + accounts: make(map[string]*Account), tokenURL: SpotifyTokenURL, apiBase: SpotifyAPIBase, } if err := s.load(); err != nil { log.Printf("[Spotify] Failed to load accounts: %v", err) } + return s } // BuildAuthorizeURL constructs the Spotify OAuth authorization URL. -func (s *SpotifyService) BuildAuthorizeURL() string { +func (s *Service) BuildAuthorizeURL() string { params := url.Values{ "client_id": {s.clientID}, "response_type": {"code"}, "redirect_uri": {s.redirectURI}, "scope": {SpotifyScopes}, } + return SpotifyAuthorizeURL + "?" + params.Encode() } // ExchangeCodeAndStore exchanges an authorization code for tokens, // fetches the user profile, and stores the account. -func (s *SpotifyService) ExchangeCodeAndStore(code string) error { +func (s *Service) ExchangeCodeAndStore(code string) error { // Exchange code for tokens tokenResp, err := s.exchangeCode(code) if err != nil { @@ -90,6 +92,7 @@ func (s *SpotifyService) ExchangeCodeAndStore(code string) error { accessToken, _ := tokenResp["access_token"].(string) refreshToken, _ := tokenResp["refresh_token"].(string) + expiresIn, _ := tokenResp["expires_in"].(float64) if expiresIn == 0 { expiresIn = 3600 @@ -105,7 +108,7 @@ func (s *SpotifyService) ExchangeCodeAndStore(code string) error { displayName, _ := profile["display_name"].(string) email, _ := profile["email"].(string) - account := &SpotifyAccount{ + account := &Account{ UserID: userID, DisplayName: displayName, Email: email, @@ -123,10 +126,11 @@ func (s *SpotifyService) ExchangeCodeAndStore(code string) error { } log.Printf("[Spotify] Account linked: %s (%s)", displayName, userID) + return nil } -func (s *SpotifyService) exchangeCode(code string) (map[string]interface{}, error) { +func (s *Service) exchangeCode(code string) (map[string]interface{}, error) { data := url.Values{ "grant_type": {"authorization_code"}, "code": {code}, @@ -137,6 +141,7 @@ func (s *SpotifyService) exchangeCode(code string) (map[string]interface{}, erro if err != nil { return nil, err } + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") req.SetBasicAuth(s.clientID, s.clientSecret) @@ -144,7 +149,10 @@ func (s *SpotifyService) exchangeCode(code string) (map[string]interface{}, erro if err != nil { return nil, fmt.Errorf("token request: %w", err) } - defer resp.Body.Close() + + defer func() { + _ = resp.Body.Close() + }() body, err := io.ReadAll(resp.Body) if err != nil { @@ -159,21 +167,26 @@ func (s *SpotifyService) exchangeCode(code string) (map[string]interface{}, erro if err := json.Unmarshal(body, &result); err != nil { return nil, fmt.Errorf("parse response: %w", err) } + return result, nil } -func (s *SpotifyService) getUserProfile(accessToken string) (map[string]interface{}, error) { +func (s *Service) getUserProfile(accessToken string) (map[string]interface{}, error) { req, err := http.NewRequest(http.MethodGet, s.apiBase+"/me", nil) if err != nil { return nil, err } + req.Header.Set("Authorization", "Bearer "+accessToken) resp, err := http.DefaultClient.Do(req) if err != nil { return nil, fmt.Errorf("profile request: %w", err) } - defer resp.Body.Close() + + defer func() { + _ = resp.Body.Close() + }() body, err := io.ReadAll(resp.Body) if err != nil { @@ -188,11 +201,12 @@ func (s *SpotifyService) getUserProfile(accessToken string) (map[string]interfac if err := json.Unmarshal(body, &result); err != nil { return nil, fmt.Errorf("parse profile: %w", err) } + return result, nil } // RefreshAccessToken refreshes the access token for the given account. -func (s *SpotifyService) RefreshAccessToken(account *SpotifyAccount) error { +func (s *Service) RefreshAccessToken(account *Account) error { data := url.Values{ "grant_type": {"refresh_token"}, "refresh_token": {account.RefreshToken}, @@ -202,6 +216,7 @@ func (s *SpotifyService) RefreshAccessToken(account *SpotifyAccount) error { if err != nil { return err } + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") req.SetBasicAuth(s.clientID, s.clientSecret) @@ -209,7 +224,10 @@ func (s *SpotifyService) RefreshAccessToken(account *SpotifyAccount) error { if err != nil { return fmt.Errorf("refresh request: %w", err) } - defer resp.Body.Close() + + defer func() { + _ = resp.Body.Close() + }() body, err := io.ReadAll(resp.Body) if err != nil { @@ -227,10 +245,12 @@ func (s *SpotifyService) RefreshAccessToken(account *SpotifyAccount) error { s.mu.Lock() account.AccessToken, _ = result["access_token"].(string) + expiresIn, _ := result["expires_in"].(float64) if expiresIn == 0 { expiresIn = 3600 } + account.ExpiresAt = time.Now().Unix() + int64(expiresIn) if newRefresh, ok := result["refresh_token"].(string); ok && newRefresh != "" { account.RefreshToken = newRefresh @@ -245,19 +265,21 @@ func (s *SpotifyService) RefreshAccessToken(account *SpotifyAccount) error { } // GetFreshToken returns a valid access token and username, refreshing if needed. -func (s *SpotifyService) GetFreshToken() (accessToken, username string, err error) { +func (s *Service) GetFreshToken() (accessToken, username string, err error) { s.mu.RLock() + if len(s.accounts) == 0 { s.mu.RUnlock() return "", "", fmt.Errorf("no Spotify accounts linked") } // Get the first account - var account *SpotifyAccount + var account *Account for _, a := range s.accounts { account = a break } + s.mu.RUnlock() // Check if token needs refresh (expired or within 60s of expiry) @@ -269,17 +291,18 @@ func (s *SpotifyService) GetFreshToken() (accessToken, username string, err erro s.mu.RLock() defer s.mu.RUnlock() + return account.AccessToken, account.UserID, nil } // GetAccounts returns a copy of all accounts with tokens stripped for API responses. -func (s *SpotifyService) GetAccounts() []SpotifyAccount { +func (s *Service) GetAccounts() []Account { s.mu.RLock() defer s.mu.RUnlock() - result := make([]SpotifyAccount, 0, len(s.accounts)) + result := make([]Account, 0, len(s.accounts)) for _, a := range s.accounts { - result = append(result, SpotifyAccount{ + result = append(result, Account{ UserID: a.UserID, DisplayName: a.DisplayName, Email: a.Email, @@ -287,11 +310,12 @@ func (s *SpotifyService) GetAccounts() []SpotifyAccount { // AccessToken and RefreshToken deliberately omitted }) } + return result } // ResolveEntity resolves a Spotify URI to a name and image URL. -func (s *SpotifyService) ResolveEntity(uri string) (name, imageURL string, err error) { +func (s *Service) ResolveEntity(uri string) (name, imageURL string, err error) { entityType, entityID, err := parseSpotifyURI(uri) if err != nil { return "", "", err @@ -303,17 +327,22 @@ func (s *SpotifyService) ResolveEntity(uri string) (name, imageURL string, err e } apiURL := fmt.Sprintf("%s/%s/%s", s.apiBase, entityType, entityID) + req, err := http.NewRequest(http.MethodGet, apiURL, nil) if err != nil { return "", "", err } + req.Header.Set("Authorization", "Bearer "+accessToken) resp, err := http.DefaultClient.Do(req) if err != nil { return "", "", fmt.Errorf("API request: %w", err) } - defer resp.Body.Close() + + defer func() { + _ = resp.Body.Close() + }() body, err := io.ReadAll(resp.Body) if err != nil { @@ -321,10 +350,11 @@ func (s *SpotifyService) ResolveEntity(uri string) (name, imageURL string, err e } if resp.StatusCode == http.StatusNotFound { - return "", "", fmt.Errorf("Spotify entity not found") + return "", "", fmt.Errorf("spotify entity not found") } + if resp.StatusCode != http.StatusOK { - return "", "", fmt.Errorf("Spotify API error (%d): %s", resp.StatusCode, string(body)) + return "", "", fmt.Errorf("spotify API error (%d): %s", resp.StatusCode, string(body)) } var data map[string]interface{} @@ -361,6 +391,7 @@ func extractImageURL(data map[string]interface{}, entityType string) string { return url } } + return "" } @@ -391,12 +422,14 @@ func parseSpotifyURI(uri string) (entityType, entityID string, err error) { } // save persists accounts to disk as JSON. -func (s *SpotifyService) save() error { +func (s *Service) save() error { s.mu.RLock() - data := make(map[string]*SpotifyAccount, len(s.accounts)) + + data := make(map[string]*Account, len(s.accounts)) for k, v := range s.accounts { data[k] = v } + s.mu.RUnlock() dir := filepath.Join(s.dataDir, "spotify") @@ -418,7 +451,7 @@ func (s *SpotifyService) save() error { } // load reads persisted accounts from disk. -func (s *SpotifyService) load() error { +func (s *Service) load() error { path := filepath.Join(s.dataDir, "spotify", "accounts.json") jsonData, err := os.ReadFile(path) @@ -426,10 +459,11 @@ func (s *SpotifyService) load() error { if os.IsNotExist(err) { return nil // No accounts file yet, not an error } + return fmt.Errorf("read file: %w", err) } - var accounts map[string]*SpotifyAccount + var accounts map[string]*Account if err := json.Unmarshal(jsonData, &accounts); err != nil { return fmt.Errorf("unmarshal accounts: %w", err) } @@ -439,5 +473,6 @@ func (s *SpotifyService) load() error { s.mu.Unlock() log.Printf("[Spotify] Loaded %d account(s)", len(accounts)) + return nil } diff --git a/pkg/service/spotify/service_test.go b/pkg/service/spotify/service_test.go index 788d9db..4309614 100644 --- a/pkg/service/spotify/service_test.go +++ b/pkg/service/spotify/service_test.go @@ -38,7 +38,7 @@ func TestGetAccountsStripsTokens(t *testing.T) { // Manually add an account with tokens svc.mu.Lock() - svc.accounts["user1"] = &SpotifyAccount{ + svc.accounts["user1"] = &Account{ UserID: "user1", DisplayName: "Test User", Email: "test@example.com", @@ -107,7 +107,7 @@ func TestGetFreshTokenRefreshesExpired(t *testing.T) { // Add an account with an expired token svc.mu.Lock() - svc.accounts["user1"] = &SpotifyAccount{ + svc.accounts["user1"] = &Account{ UserID: "user1", DisplayName: "Test User", AccessToken: "old-expired-token", @@ -213,7 +213,7 @@ func TestResolveEntityFetchesFromAPI(t *testing.T) { // Add a non-expired account svc.mu.Lock() - svc.accounts["user1"] = &SpotifyAccount{ + svc.accounts["user1"] = &Account{ UserID: "user1", AccessToken: "fresh-token", RefreshToken: "refresh", @@ -252,7 +252,7 @@ func TestSaveAndLoad(t *testing.T) { // Create and populate svc := NewSpotifyService("cid", "csecret", "http://localhost/cb", dir) svc.mu.Lock() - svc.accounts["user1"] = &SpotifyAccount{ + svc.accounts["user1"] = &Account{ UserID: "user1", DisplayName: "Test User", Email: "test@example.com", @@ -260,7 +260,7 @@ func TestSaveAndLoad(t *testing.T) { RefreshToken: "rt", ExpiresAt: 1234567890, } - svc.accounts["user2"] = &SpotifyAccount{ + svc.accounts["user2"] = &Account{ UserID: "user2", DisplayName: "User Two", Email: "two@example.com", @@ -405,7 +405,7 @@ func TestGetFreshTokenNotExpired(t *testing.T) { svc := NewSpotifyService("cid", "csecret", "http://localhost/cb", t.TempDir()) svc.mu.Lock() - svc.accounts["user1"] = &SpotifyAccount{ + svc.accounts["user1"] = &Account{ UserID: "user1", AccessToken: "valid-token", RefreshToken: "rt",