Don't hold the window for a game database rebuild

internal/daemon/devicesettings.go

@@ -15,6 +15,12 @@ import (
 // and a game's answer only changes when its name, App ID or folder does —
 // which makes it a different key here. A database update takes effect at the
 // next start, where adoptExclusionView carries every recorded hash over.
+//
+// First needed while the daemon starts, before the window can open, so it does
+// not wait for the database to be rebuilt: that parses the whole manifest,
+// about a minute, and is due after an update changes the index format and
+// after every weekly manifest download. The rebuild runs in the background and
+// is what the next start reads.
 
 type devSettingsCache struct {
 	loaded   bool
@@ -30,7 +36,7 @@ func (d *Daemon) deviceSettingsFor(game store.Game) []string {
 	d.devMu.Lock()
 	defer d.devMu.Unlock()
 	if !d.dev.loaded {
-		d.dev.db = d.Scanner.DeviceSettings()
+		d.dev.db = d.Scanner.DeviceSettingsNoWait()
 		d.dev.patterns = map[string][]string{}
 		d.dev.loaded = true
 	}

internal/presets/devicesettings.go

@@ -29,7 +29,21 @@ func (sc *Scanner) DeviceSettings() *DeviceSettings {
 	if sc == nil || sc.CacheFile == "" {
 		return nil
 	}
-	idx := sc.loadManifestIndex()
+	return sc.deviceSettings(sc.loadManifestIndex())
+}
+
+// DeviceSettingsNoWait is DeviceSettings for the daemon while it starts, which
+// must not wait for an index that is due a rebuild (loadManifestIndexNoWait):
+// until the rebuild is done, settings files are left out as the index on disk
+// names them, and the next start goes by the new one.
+func (sc *Scanner) DeviceSettingsNoWait() *DeviceSettings {
+	if sc == nil || sc.CacheFile == "" {
+		return nil
+	}
+	return sc.deviceSettings(sc.loadManifestIndexNoWait())
+}
+
+func (sc *Scanner) deviceSettings(idx []indexedGame) *DeviceSettings {
 	if len(idx) == 0 {
 		return nil
 	}

internal/presets/index_nowait_test.go

@@ -0,0 +1,117 @@
+package presets
+
+import (
+	"encoding/json"
+	"os"
+	"testing"
+	"time"
+)
+
+// The daemon reads the index while it starts, before the window opens. An
+// index due a rebuild is used as it is, and rebuilt behind it: the window
+// waited a minute for the rebuild at the first start after the index format
+// changed.
+func TestIndexNoWait_UsesTheIndexOnDiskAndRebuildsBehindIt(t *testing.T) {
+	sc := manifestScanner(t, `
+New Game:
+  files:
+    "<winAppData>/New/save":
+      tags: [save]
+`)
+	yamlPath, indexPath := sc.manifestPaths()
+	waitForRebuild(t)
+
+	// An index from before the manifest's last download.
+	old := []indexedGame{{Name: "Old Game", Paths: []string{"<winAppData>/Old"}}}
+	raw, _ := json.Marshal(old)
+	if err := os.WriteFile(indexPath, raw, 0o666); err != nil {
+		t.Fatal(err)
+	}
+	// The manifest downloaded half an hour ago, after the index was built.
+	// Both in the past: a filesystem with coarse timestamps can give the
+	// rebuilt index the same time as a manifest written moments before it,
+	// and it would never count as newer.
+	past := time.Now().Add(-time.Hour)
+	if err := os.Chtimes(indexPath, past, past); err != nil {
+		t.Fatal(err)
+	}
+	downloaded := time.Now().Add(-30 * time.Minute)
+	if err := os.Chtimes(yamlPath, downloaded, downloaded); err != nil {
+		t.Fatal(err)
+	}
+
+	got := sc.loadManifestIndexNoWait()
+	if len(got) != 1 || got[0].Name != "Old Game" {
+		t.Fatalf("got %+v, want the index on disk as it is", got)
+	}
+
+	// Rebuilt in the background; what the next start reads.
+	deadline := time.Now().Add(10 * time.Second)
+	for {
+		yamlInfo, err := os.Stat(yamlPath)
+		if err != nil {
+			t.Fatal(err)
+		}
+		if games, fresh := readManifestIndex(indexPath, yamlInfo); fresh {
+			if len(games) != 1 || games[0].Name != "New Game" {
+				t.Fatalf("rebuilt index: %+v", games)
+			}
+			break
+		}
+		if time.Now().After(deadline) {
+			t.Fatal("the index was not rebuilt")
+		}
+		time.Sleep(20 * time.Millisecond)
+	}
+	if got := sc.loadManifestIndexNoWait(); len(got) != 1 || got[0].Name != "New Game" {
+		t.Errorf("after the rebuild: %+v", got)
+	}
+	waitForRebuild(t)
+}
+
+// waitForRebuild waits for a background index build to be over, so the next
+// test can start one of its own.
+func waitForRebuild(t *testing.T) {
+	t.Helper()
+	deadline := time.Now().Add(10 * time.Second)
+	for rebuildInFlight.Load() {
+		if time.Now().After(deadline) {
+			t.Fatal("the background rebuild did not finish")
+		}
+		time.Sleep(10 * time.Millisecond)
+	}
+}
+
+// With no index at all, nothing is waited for either: the embedded one stands
+// in (none in a hermetic test), and the index is built behind it.
+func TestIndexNoWait_NoIndexYet(t *testing.T) {
+	sc := manifestScanner(t, `
+New Game:
+  files:
+    "<winAppData>/New/save":
+      tags: [save]
+`)
+	yamlPath, indexPath := sc.manifestPaths()
+	waitForRebuild(t)
+	downloaded := time.Now().Add(-30 * time.Minute)
+	if err := os.Chtimes(yamlPath, downloaded, downloaded); err != nil {
+		t.Fatal(err)
+	}
+	if got := sc.loadManifestIndexNoWait(); len(got) != 0 {
+		t.Fatalf("got %+v, want nothing until the index is built", got)
+	}
+	deadline := time.Now().Add(10 * time.Second)
+	for {
+		if _, err := os.Stat(indexPath); err == nil {
+			break
+		}
+		if time.Now().After(deadline) {
+			t.Fatal("the index was not built")
+		}
+		time.Sleep(20 * time.Millisecond)
+	}
+	if got := sc.loadManifestIndexNoWait(); len(got) != 1 {
+		t.Errorf("after the build: %+v", got)
+	}
+	waitForRebuild(t)
+}

internal/presets/ludusavi.go

@@ -644,23 +644,78 @@ func (sc *Scanner) loadManifestIndex() []indexedGame {
 	}
 
 	// Reuse the index when it's newer than the YAML it was built from.
-	if idxInfo, err := os.Stat(indexPath); err == nil && idxInfo.ModTime().After(yamlInfo.ModTime()) {
-		if raw, err := os.ReadFile(indexPath); err == nil {
-			var games []indexedGame
-			if json.Unmarshal(raw, &games) == nil && len(games) > 0 {
-				return games
-			}
+	if games, fresh := readManifestIndex(indexPath, yamlInfo); fresh {
+		return games
+	}
+
+	if games := rebuildManifestIndex(yamlPath, indexPath); len(games) > 0 {
+		return games
+	}
+	return embedded()
+}
+
+// loadManifestIndexNoWait is loadManifestIndex for a caller that must not wait
+// on a rebuild: the daemon while it starts, before the window can open.
+// Building the index parses the whole manifest, about a minute on an ordinary
+// PC, and it is due at the first start after the index format changes and
+// after every weekly manifest download. This takes the index on disk as it is,
+// stale or not, or the embedded one when there is none, and rebuilds it in the
+// background; the next start reads the new one.
+func (sc *Scanner) loadManifestIndexNoWait() []indexedGame {
+	yamlPath, indexPath := sc.manifestPaths()
+
+	sc.refreshManifestAsync(yamlPath)
+
+	yamlInfo, err := os.Stat(yamlPath)
+	if err != nil {
+		if sc.ManifestURL == "" {
+			return nil
 		}
+		return loadEmbeddedIndex()
 	}
+	games, fresh := readManifestIndex(indexPath, yamlInfo)
+	if !fresh && rebuildInFlight.CompareAndSwap(false, true) {
+		go func() {
+			defer rebuildInFlight.Store(false)
+			rebuildManifestIndex(yamlPath, indexPath)
+		}()
+	}
+	if len(games) > 0 {
+		return games
+	}
+	if sc.ManifestURL == "" {
+		return nil
+	}
+	return loadEmbeddedIndex()
+}
 
+// rebuildInFlight guards against overlapping background index builds.
+var rebuildInFlight atomic.Bool
+
+// readManifestIndex reads the index on disk, and says whether it is newer
+// than the manifest it was built from. Nil when there is none or it does not
+// parse.
+func readManifestIndex(indexPath string, yamlInfo os.FileInfo) (games []indexedGame, fresh bool) {
+	idxInfo, err := os.Stat(indexPath)
+	if err != nil {
+		return nil, false
+	}
+	raw, err := os.ReadFile(indexPath)
+	if err != nil || json.Unmarshal(raw, &games) != nil || len(games) == 0 {
+		return nil, false
+	}
+	return games, idxInfo.ModTime().After(yamlInfo.ModTime())
+}
+
+// rebuildManifestIndex builds the index from the manifest and saves it.
+func rebuildManifestIndex(yamlPath, indexPath string) []indexedGame {
 	games := buildManifestIndex(yamlPath)
 	if len(games) > 0 {
 		if raw, err := json.Marshal(games); err == nil {
 			_ = writeFileAtomic(indexPath, raw)
 		}
-		return games
 	}
-	return embedded()
+	return games
 }
 
 // refreshInFlight guards against overlapping background downloads.