mirror of
https://github.com/gesellix/Bose-SoundTouch.git
synced 2026-08-24 14:47:23 +00:00
feat(update-check): wire the Checker into the service, opt-in via env flags
Third piece of #591. --update-check-enabled/--update-check-interval (UPDATE_CHECK_ENABLED/UPDATE_CHECK_INTERVAL), default off/24h, following the same local main.go flag pattern as discovery-enabled — not pkg/config, which soundtouch-service doesn't import at all (correction to the issue's proposed location, see the design doc). Background goroutine modeled on startDeviceDiscovery: startup jitter (0-5min), skips the immediate check if the persisted last-check is still fresh, backs off retries to no sooner than 1h after a failure, logs once per newly-detected version. The decision logic (shouldCheckImmediately, shouldSkipDueToBackoff, logUpdateIfNewlyAvailable) is split into pure, directly-testable functions rather than living inline in the goroutine. Server gets a SetUpdateChecker/UpdateCheckResult pair (nil-safe) so the next two pieces (announcement, /api/setup/version) can read the current state without importing updatecheck's construction details. Manually verified against a running instance: enabled via flags, no panic, service stays responsive (jitter means the actual first check can take up to 5 minutes to fire, so this only confirms the wiring, not a live GitHub response — that's covered by the previous commit's httptest-backed unit tests). Refs #591
This commit is contained in:
@@ -9,6 +9,7 @@ import (
|
||||
"fmt"
|
||||
"io"
|
||||
"log"
|
||||
"math/rand"
|
||||
"net"
|
||||
"net/http"
|
||||
"net/url"
|
||||
@@ -32,6 +33,7 @@ import (
|
||||
"github.com/gesellix/bose-soundtouch/pkg/service/soundtouchweb"
|
||||
"github.com/gesellix/bose-soundtouch/pkg/service/spotify"
|
||||
"github.com/gesellix/bose-soundtouch/pkg/service/stockholm"
|
||||
"github.com/gesellix/bose-soundtouch/pkg/service/updatecheck"
|
||||
"github.com/go-chi/chi/v5"
|
||||
"github.com/go-chi/chi/v5/middleware"
|
||||
"github.com/urfave/cli/v2"
|
||||
@@ -314,6 +316,18 @@ func main() {
|
||||
Value: "5m",
|
||||
EnvVars: []string{"DISCOVERY_INTERVAL"},
|
||||
},
|
||||
&cli.BoolFlag{
|
||||
Name: "update-check-enabled",
|
||||
Usage: "Periodically check GitHub for a newer release (opt-in; the only network call this makes beyond speaker/provider traffic)",
|
||||
Value: false,
|
||||
EnvVars: []string{"UPDATE_CHECK_ENABLED"},
|
||||
},
|
||||
&cli.StringFlag{
|
||||
Name: "update-check-interval",
|
||||
Usage: "Update check interval",
|
||||
Value: "24h",
|
||||
EnvVars: []string{"UPDATE_CHECK_INTERVAL"},
|
||||
},
|
||||
&cli.BoolFlag{
|
||||
Name: "dns-discovery",
|
||||
Usage: "Enable DNS discovery server",
|
||||
@@ -600,6 +614,10 @@ func main() {
|
||||
|
||||
startDeviceDiscovery(server)
|
||||
|
||||
updateChecker := updatecheck.NewChecker(ds, "gesellix/Bose-SoundTouch", version)
|
||||
server.SetUpdateChecker(updateChecker)
|
||||
startUpdateCheck(updateChecker, config.updateCheckEnabled, config.updateCheckInterval)
|
||||
|
||||
var stockholmHandler *stockholm.Handler
|
||||
|
||||
if config.stockholmDir != "" {
|
||||
@@ -703,6 +721,8 @@ type serviceConfig struct {
|
||||
tlsExtraHosts []string
|
||||
discoveryEnabled bool
|
||||
discoveryInterval time.Duration
|
||||
updateCheckEnabled bool
|
||||
updateCheckInterval time.Duration
|
||||
domains []string
|
||||
spotifyClientID string
|
||||
spotifyClientSecret string
|
||||
@@ -793,6 +813,16 @@ func loadConfig(c *cli.Context) serviceConfig {
|
||||
discoveryInterval = 5 * time.Minute
|
||||
}
|
||||
|
||||
updateCheckEnabled := c.Bool("update-check-enabled")
|
||||
updateCheckIntervalStr := c.String("update-check-interval")
|
||||
|
||||
updateCheckInterval, err := time.ParseDuration(updateCheckIntervalStr)
|
||||
if err != nil {
|
||||
log.Printf("Warning: Failed to parse update check interval %s, using default 24h: %v", sanitizeLog(updateCheckIntervalStr), err)
|
||||
|
||||
updateCheckInterval = 24 * time.Hour
|
||||
}
|
||||
|
||||
spotifyClientID := c.String("spotify-client-id")
|
||||
spotifyClientSecret := c.String("spotify-client-secret")
|
||||
spotifyRedirectURI := c.String("spotify-redirect-uri")
|
||||
@@ -842,6 +872,8 @@ func loadConfig(c *cli.Context) serviceConfig {
|
||||
tlsExtraHosts: tlsExtraHosts,
|
||||
discoveryEnabled: discoveryEnabled,
|
||||
discoveryInterval: discoveryInterval,
|
||||
updateCheckEnabled: updateCheckEnabled,
|
||||
updateCheckInterval: updateCheckInterval,
|
||||
domains: domains,
|
||||
spotifyClientID: spotifyClientID,
|
||||
spotifyClientSecret: spotifyClientSecret,
|
||||
@@ -1198,6 +1230,96 @@ func startDeviceDiscovery(server *handlers.Server) {
|
||||
}()
|
||||
}
|
||||
|
||||
// startUpdateCheck runs the opt-in periodic check against GitHub Releases
|
||||
// in the background (#591, _/i591/design-update-check.md). Unlike
|
||||
// discovery, enabled/interval are fixed at startup (env-only for v1, no
|
||||
// Settings-tab control — see the design doc's answer on that trade-off),
|
||||
// so they're plain arguments, not read live from Settings each tick.
|
||||
func startUpdateCheck(checker *updatecheck.Checker, enabled bool, interval time.Duration) {
|
||||
if !enabled {
|
||||
return
|
||||
}
|
||||
|
||||
go func() {
|
||||
time.Sleep(randomJitter(5 * time.Minute))
|
||||
|
||||
lastResult := checker.LastResult()
|
||||
lastLoggedVersion := lastResult.LatestVersion
|
||||
|
||||
var lastErrorAt time.Time
|
||||
|
||||
if shouldCheckImmediately(lastResult.CheckedAt, interval, time.Now()) {
|
||||
lastErrorAt, lastLoggedVersion = runUpdateCheckTick(checker, lastLoggedVersion)
|
||||
}
|
||||
|
||||
for {
|
||||
time.Sleep(interval)
|
||||
|
||||
if shouldSkipDueToBackoff(lastErrorAt, time.Now()) {
|
||||
continue
|
||||
}
|
||||
|
||||
lastErrorAt, lastLoggedVersion = runUpdateCheckTick(checker, lastLoggedVersion)
|
||||
}
|
||||
}()
|
||||
}
|
||||
|
||||
// randomJitter returns a random duration in [0, upperBound) — the startup
|
||||
// delay so many installs restarting together (e.g. after a Docker image
|
||||
// bump) don't all hit GitHub at once. Not a security-sensitive use of
|
||||
// randomness.
|
||||
func randomJitter(upperBound time.Duration) time.Duration {
|
||||
if upperBound <= 0 {
|
||||
return 0
|
||||
}
|
||||
|
||||
return time.Duration(rand.Int63n(int64(upperBound))) //nolint:gosec
|
||||
}
|
||||
|
||||
// shouldCheckImmediately reports whether a check should run right at
|
||||
// startup (after jitter) rather than waiting a full interval: true when
|
||||
// there's no persisted last-check time, or it's stale (older than one
|
||||
// interval). Pure/testable — no sleeping.
|
||||
func shouldCheckImmediately(lastCheckedAt time.Time, interval time.Duration, now time.Time) bool {
|
||||
return lastCheckedAt.IsZero() || now.Sub(lastCheckedAt) >= interval
|
||||
}
|
||||
|
||||
// shouldSkipDueToBackoff reports whether a tick should be skipped because
|
||||
// the last attempt failed less than an hour ago — so a short
|
||||
// UPDATE_CHECK_INTERVAL doesn't hammer GitHub while it's erroring. A zero
|
||||
// lastErrorAt means "no recent failure", never skip. Pure/testable.
|
||||
func shouldSkipDueToBackoff(lastErrorAt, now time.Time) bool {
|
||||
return !lastErrorAt.IsZero() && now.Sub(lastErrorAt) < time.Hour
|
||||
}
|
||||
|
||||
// logUpdateIfNewlyAvailable logs once when result reports a version newer
|
||||
// than lastLoggedVersion, and returns the version to remember as "already
|
||||
// logged" — unchanged when there's nothing new, so a persistently-available
|
||||
// update doesn't spam the log every tick. Pure/testable.
|
||||
func logUpdateIfNewlyAvailable(result updatecheck.Result, lastLoggedVersion string) string {
|
||||
if result.Available && result.LatestVersion != "" && result.LatestVersion != lastLoggedVersion {
|
||||
log.Printf("[UpdateCheck] Update available: %s (current %s) — %s",
|
||||
result.LatestVersion, result.CurrentVersion, result.ReleaseURL)
|
||||
|
||||
return result.LatestVersion
|
||||
}
|
||||
|
||||
return lastLoggedVersion
|
||||
}
|
||||
|
||||
// runUpdateCheckTick performs one check, logs on failure, and returns the
|
||||
// updated (lastErrorAt, lastLoggedVersion) pair for the caller to carry
|
||||
// into the next iteration.
|
||||
func runUpdateCheckTick(checker *updatecheck.Checker, lastLoggedVersion string) (time.Time, string) {
|
||||
result, err := checker.CheckNow(context.Background())
|
||||
if err != nil {
|
||||
log.Printf("[UpdateCheck] check failed: %v", err)
|
||||
return time.Now(), lastLoggedVersion
|
||||
}
|
||||
|
||||
return time.Time{}, logUpdateIfNewlyAvailable(result, lastLoggedVersion)
|
||||
}
|
||||
|
||||
// newEmbeddedWebApp builds the soundtouch-player application for embedding in the
|
||||
// service router: release metadata from the build vars, the service's public
|
||||
// ServiceURL (used by Play URL for speaker-fetched stream URLs and shown in the
|
||||
|
||||
@@ -0,0 +1,112 @@
|
||||
package main
|
||||
|
||||
import (
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/gesellix/bose-soundtouch/pkg/service/updatecheck"
|
||||
)
|
||||
|
||||
func TestShouldCheckImmediately(t *testing.T) {
|
||||
now := time.Date(2026, 8, 9, 12, 0, 0, 0, time.UTC)
|
||||
interval := 24 * time.Hour
|
||||
|
||||
cases := []struct {
|
||||
name string
|
||||
lastCheckedAt time.Time
|
||||
want bool
|
||||
}{
|
||||
{"never checked", time.Time{}, true},
|
||||
{"stale (older than interval)", now.Add(-25 * time.Hour), true},
|
||||
{"exactly one interval ago", now.Add(-interval), true},
|
||||
{"recent (within interval)", now.Add(-1 * time.Hour), false},
|
||||
}
|
||||
|
||||
for _, tc := range cases {
|
||||
if got := shouldCheckImmediately(tc.lastCheckedAt, interval, now); got != tc.want {
|
||||
t.Errorf("%s: shouldCheckImmediately() = %v, want %v", tc.name, got, tc.want)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestShouldSkipDueToBackoff(t *testing.T) {
|
||||
now := time.Date(2026, 8, 9, 12, 0, 0, 0, time.UTC)
|
||||
|
||||
cases := []struct {
|
||||
name string
|
||||
lastErrorAt time.Time
|
||||
want bool
|
||||
}{
|
||||
{"no recent failure", time.Time{}, false},
|
||||
{"failed 30 minutes ago", now.Add(-30 * time.Minute), true},
|
||||
{"failed exactly 1 hour ago", now.Add(-time.Hour), false},
|
||||
{"failed 2 hours ago", now.Add(-2 * time.Hour), false},
|
||||
}
|
||||
|
||||
for _, tc := range cases {
|
||||
if got := shouldSkipDueToBackoff(tc.lastErrorAt, now); got != tc.want {
|
||||
t.Errorf("%s: shouldSkipDueToBackoff() = %v, want %v", tc.name, got, tc.want)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestLogUpdateIfNewlyAvailable(t *testing.T) {
|
||||
cases := []struct {
|
||||
name string
|
||||
result updatecheck.Result
|
||||
lastLoggedVersion string
|
||||
want string
|
||||
}{
|
||||
{
|
||||
name: "nothing available",
|
||||
result: updatecheck.Result{Available: false},
|
||||
lastLoggedVersion: "",
|
||||
want: "",
|
||||
},
|
||||
{
|
||||
name: "newly available",
|
||||
result: updatecheck.Result{Available: true, LatestVersion: "v1.1.0"},
|
||||
lastLoggedVersion: "",
|
||||
want: "v1.1.0",
|
||||
},
|
||||
{
|
||||
name: "already logged this version",
|
||||
result: updatecheck.Result{Available: true, LatestVersion: "v1.1.0"},
|
||||
lastLoggedVersion: "v1.1.0",
|
||||
want: "v1.1.0",
|
||||
},
|
||||
{
|
||||
name: "a newer version than what was logged",
|
||||
result: updatecheck.Result{Available: true, LatestVersion: "v1.2.0"},
|
||||
lastLoggedVersion: "v1.1.0",
|
||||
want: "v1.2.0",
|
||||
},
|
||||
}
|
||||
|
||||
for _, tc := range cases {
|
||||
if got := logUpdateIfNewlyAvailable(tc.result, tc.lastLoggedVersion); got != tc.want {
|
||||
t.Errorf("%s: logUpdateIfNewlyAvailable() = %q, want %q", tc.name, got, tc.want)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestRandomJitter(t *testing.T) {
|
||||
if got := randomJitter(0); got != 0 {
|
||||
t.Errorf("randomJitter(0) = %v, want 0", got)
|
||||
}
|
||||
|
||||
upperBound := 5 * time.Minute
|
||||
for i := 0; i < 20; i++ {
|
||||
got := randomJitter(upperBound)
|
||||
if got < 0 || got >= upperBound {
|
||||
t.Fatalf("randomJitter(%v) = %v, want in [0, %v)", upperBound, got, upperBound)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestStartUpdateCheck_DisabledIsANoOp(t *testing.T) {
|
||||
// Must return immediately without spawning anything that could touch a
|
||||
// nil-repo Checker or block — disabled is the default, so this path
|
||||
// runs on every install that hasn't opted in.
|
||||
startUpdateCheck(nil, false, time.Hour)
|
||||
}
|
||||
@@ -0,0 +1,34 @@
|
||||
package handlers
|
||||
|
||||
import (
|
||||
"testing"
|
||||
|
||||
"github.com/gesellix/bose-soundtouch/pkg/service/updatecheck"
|
||||
)
|
||||
|
||||
// TestUpdateCheckResult_NilCheckerIsSafe verifies the default (opt-in
|
||||
// checker never registered) returns a safe zero value rather than
|
||||
// panicking — the common case, since UPDATE_CHECK_ENABLED defaults to
|
||||
// false.
|
||||
func TestUpdateCheckResult_NilCheckerIsSafe(t *testing.T) {
|
||||
s := NewServer(nil, nil, "http://localhost", false, false, false)
|
||||
|
||||
result := s.UpdateCheckResult()
|
||||
if result.Available {
|
||||
t.Error("Expected a nil checker to report Available=false")
|
||||
}
|
||||
}
|
||||
|
||||
// TestUpdateCheckResult_ReflectsRegisteredChecker verifies SetUpdateChecker
|
||||
// wires the checker in and UpdateCheckResult reads through to it.
|
||||
func TestUpdateCheckResult_ReflectsRegisteredChecker(t *testing.T) {
|
||||
s := NewServer(nil, nil, "http://localhost", false, false, false)
|
||||
|
||||
checker := updatecheck.NewChecker(nil, "owner/repo", "v1.0.0")
|
||||
s.SetUpdateChecker(checker)
|
||||
|
||||
result := s.UpdateCheckResult()
|
||||
if result.CurrentVersion != "v1.0.0" {
|
||||
t.Errorf("Expected UpdateCheckResult to read through to the registered checker, got %+v", result)
|
||||
}
|
||||
}
|
||||
@@ -30,6 +30,7 @@ import (
|
||||
"github.com/gesellix/bose-soundtouch/pkg/service/setup"
|
||||
"github.com/gesellix/bose-soundtouch/pkg/service/spotify"
|
||||
"github.com/gesellix/bose-soundtouch/pkg/service/tts"
|
||||
"github.com/gesellix/bose-soundtouch/pkg/service/updatecheck"
|
||||
"github.com/gesellix/bose-soundtouch/pkg/ssh"
|
||||
"github.com/miekg/dns"
|
||||
)
|
||||
@@ -70,6 +71,7 @@ type Server struct {
|
||||
mgmtPassword string
|
||||
adminAreaAuth string // "" (unset) / "enabled" / "disabled" — see datastore.Settings.AdminAreaAuth
|
||||
dismissedAnnouncements map[string]time.Time // announcement id -> most recent dismissal; see RecordDismissal
|
||||
updateChecker *updatecheck.Checker // nil unless SetUpdateChecker was called (opt-in, see #591)
|
||||
spotifyClientID string
|
||||
spotifyClientSecret string
|
||||
spotifyRedirectURI string
|
||||
@@ -1126,6 +1128,30 @@ func (s *Server) IsAnnouncementDismissed(id string) bool {
|
||||
return ok
|
||||
}
|
||||
|
||||
// SetUpdateChecker registers the opt-in periodic update checker (#591). Not
|
||||
// called at all unless UPDATE_CHECK_ENABLED — leave nil otherwise;
|
||||
// UpdateCheckResult handles that safely.
|
||||
func (s *Server) SetUpdateChecker(c *updatecheck.Checker) {
|
||||
s.mu.Lock()
|
||||
defer s.mu.Unlock()
|
||||
|
||||
s.updateChecker = c
|
||||
}
|
||||
|
||||
// UpdateCheckResult returns the last known update-check result, or the
|
||||
// zero value (Available: false) if the check was never enabled.
|
||||
func (s *Server) UpdateCheckResult() updatecheck.Result {
|
||||
s.mu.RLock()
|
||||
checker := s.updateChecker
|
||||
s.mu.RUnlock()
|
||||
|
||||
if checker == nil {
|
||||
return updatecheck.Result{}
|
||||
}
|
||||
|
||||
return checker.LastResult()
|
||||
}
|
||||
|
||||
// SetInternalPaths sets the internal paths for the server.
|
||||
func (s *Server) SetInternalPaths(paths []string) {
|
||||
s.mu.Lock()
|
||||
|
||||
Reference in New Issue
Block a user