watcher: changes OpenSave makes itself are not new saves

internal/delta/patch.go

@@ -9,6 +9,7 @@ import (
 	"time"
 
 	"github.com/opensave/opensave/internal/fsx"
+	"github.com/opensave/opensave/internal/owntouch"
 )
 
 // BlockSource supplies the bytes for one block index, either from the
@@ -77,7 +78,11 @@ func replaceWithRetry(tmpPath, filePath string) error {
 			delay *= 2
 		}
 		clearReadOnlyIfSet(filePath)
+		// Marked before the rename, so the watcher's event finds it already
+		// known as a change OpenSave made (owntouch).
+		owntouch.Mark(filePath)
 		if err = renameFile(tmpPath, filePath); err == nil {
+			owntouch.Settled(filePath)
 			return nil
 		}
 	}

internal/owntouch/owntouch.go

@@ -0,0 +1,137 @@
+// Package owntouch remembers which paths in save folders OpenSave itself has
+// just written or removed, so the watcher can tell those changes apart from
+// ones a game or a person made.
+//
+// Without it every change OpenSave applied — a file pulled from a peer, a
+// deletion a peer asked for, the mtimes "keep mine" touches — looked to the
+// watcher exactly like the game saving. A peer deleting files one request at
+// a time over several minutes produced a full auto-snapshot every couple of
+// seconds, each of a save that was neither the old one nor the new one, and
+// those filled the retention budget and pushed out the snapshots that mattered.
+package owntouch
+
+import (
+	"os"
+	"path/filepath"
+	"runtime"
+	"strings"
+	"sync"
+	"time"
+)
+
+// window is how long a mark counts. Long enough to cover the watcher's
+// debounce and a slow burst of events behind it; short enough that the same
+// path changed again by a game a little later is not mistaken for ours.
+const window = 2 * time.Minute
+
+var (
+	mu     sync.Mutex
+	marked = map[string]mark{}
+)
+
+// mark is when OpenSave touched a path, whether it removed it, and - once
+// OpenSave is done with a file (Settled) - the time and size it left it with.
+type mark struct {
+	at      time.Time
+	removed bool
+	settled bool
+	mod     time.Time
+	size    int64
+}
+
+func key(path string) string {
+	p := filepath.Clean(path)
+	if runtime.GOOS == "windows" {
+		p = strings.ToLower(p)
+	}
+	return p
+}
+
+// Mark records that OpenSave itself just created, wrote, removed or re-dated
+// path. Its parent folder is marked too: creating or removing an entry is an
+// event on the folder as well.
+func Mark(path string) { record(path, false) }
+
+// MarkRemoved records that OpenSave itself is about to remove path. Only a
+// path marked this way counts as OpenSave's when it is found gone: a file
+// OpenSave has just written and a game or a person then deleted is their
+// change, and has to sync and be snapshotted as one.
+func MarkRemoved(path string) { record(path, true) }
+
+// Settled records the time and size OpenSave has left a file it marked with,
+// once it is done with it (written, renamed into place, re-dated). From then on
+// the file is OpenSave's only while it still has exactly those: a game saving
+// over it a moment later - inside any slack a time comparison would need - is
+// the game's change.
+func Settled(path string) {
+	fi, err := os.Lstat(path)
+	if err != nil || fi.IsDir() {
+		return
+	}
+	k := key(path)
+	mu.Lock()
+	defer mu.Unlock()
+	if m, ok := marked[k]; ok {
+		m.settled, m.mod, m.size = true, fi.ModTime(), fi.Size()
+		marked[k] = m
+	}
+}
+
+func record(path string, removed bool) {
+	if path == "" {
+		return
+	}
+	now := time.Now()
+	mu.Lock()
+	defer mu.Unlock()
+	marked[key(path)] = mark{at: now, removed: removed}
+	marked[key(filepath.Dir(path))] = mark{at: now}
+	if len(marked) > 50000 {
+		for k, m := range marked {
+			if now.Sub(m.at) > window {
+				delete(marked, k)
+			}
+		}
+	}
+}
+
+// slack is how much later than its mark a file OpenSave wrote may be dated:
+// re-dating to now happens just after marking, and some filesystems keep
+// times to two seconds.
+const slack = 2 * time.Second
+
+// Recent reports whether the change just seen at path is one OpenSave made:
+// the path was marked within the window, and nothing has written it since.
+//
+// The second half matters as much as the first. A game often saves straight
+// after a sync — the same file OpenSave has just put there — and a mark alone
+// would take that save for the sync's for two minutes: no snapshot of it, and
+// no new version to hand to anyone. Whatever OpenSave leaves behind is dated
+// no later than its mark (a pulled file keeps the time it was written to its
+// temporary name, or is given the peer's; re-dating sets now), so a file
+// dated later was written by someone else. Once OpenSave is done with a file
+// (Settled) the test is exact: the time and size it left, or not ours - the
+// slack would take a game's save within two seconds of a pull for the pull's.
+// A path that is gone is OpenSave's
+// only if OpenSave removed it (MarkRemoved): one it wrote and someone then
+// deleted is theirs. A folder carries no save of its own, so its mark is
+// enough.
+func Recent(path string) bool {
+	mu.Lock()
+	m, ok := marked[key(path)]
+	mu.Unlock()
+	if !ok || time.Since(m.at) > window {
+		return false
+	}
+	fi, err := os.Lstat(path)
+	if err != nil {
+		return m.removed
+	}
+	if fi.IsDir() {
+		return true
+	}
+	if m.settled {
+		return fi.ModTime().Equal(m.mod) && fi.Size() == m.size
+	}
+	return !fi.ModTime().After(m.at.Add(slack))
+}

internal/owntouch/owntouch_test.go

@@ -0,0 +1,132 @@
+package owntouch
+
+import (
+	"os"
+	"path/filepath"
+	"testing"
+	"time"
+)
+
+func TestMarkAndRecent(t *testing.T) {
+	dir := t.TempDir()
+	file := filepath.Join(dir, "sub", "slot1.sav")
+	if Recent(file) {
+		t.Fatal("unmarked path reported as ours")
+	}
+	Mark(file)
+	if err := os.MkdirAll(filepath.Dir(file), 0o777); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.WriteFile(file, []byte("pulled"), 0o666); err != nil {
+		t.Fatal(err)
+	}
+	if !Recent(file) {
+		t.Error("marked path not reported as ours")
+	}
+	if !Recent(filepath.Dir(file)) {
+		t.Error("the parent folder of a marked path not reported as ours")
+	}
+	if Recent(filepath.Join(dir, "sub", "slot2.sav")) {
+		t.Error("a sibling of a marked path reported as ours")
+	}
+}
+
+// A game saving over a file OpenSave has just written is the game's change,
+// however soon after.
+func TestRecent_AWriteAfterTheMarkIsNotOurs(t *testing.T) {
+	dir := t.TempDir()
+	file := filepath.Join(dir, "slot1.sav")
+	Mark(file)
+	if err := os.WriteFile(file, []byte("pulled"), 0o666); err != nil {
+		t.Fatal(err)
+	}
+	if !Recent(file) {
+		t.Fatal("the file OpenSave wrote is not reported as ours")
+	}
+	// The game saves a little later.
+	later := time.Now().Add(5 * time.Second)
+	if err := os.WriteFile(file, []byte("played"), 0o666); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.Chtimes(file, later, later); err != nil {
+		t.Fatal(err)
+	}
+	if Recent(file) {
+		t.Error("a save written after OpenSave's was taken for OpenSave's")
+	}
+}
+
+// A file OpenSave has just written, then deleted by a game or a person, is
+// their deletion: it has to sync and be snapshotted. Only a path OpenSave
+// removed itself is its own when found gone.
+func TestRecent_AFileDeletedAfterOurWriteIsNotOurs(t *testing.T) {
+	dir := t.TempDir()
+	pulled := filepath.Join(dir, "pulled.sav")
+	Mark(pulled)
+	if err := os.WriteFile(pulled, []byte("from the peer"), 0o666); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.Remove(pulled); err != nil {
+		t.Fatal(err)
+	}
+	if Recent(pulled) {
+		t.Error("a file deleted by someone else after OpenSave wrote it was taken for OpenSave's")
+	}
+
+	removed := filepath.Join(dir, "removed.sav")
+	if err := os.WriteFile(removed, []byte("old"), 0o666); err != nil {
+		t.Fatal(err)
+	}
+	MarkRemoved(removed)
+	if err := os.Remove(removed); err != nil {
+		t.Fatal(err)
+	}
+	if !Recent(removed) {
+		t.Error("a file OpenSave removed was not reported as its own")
+	}
+}
+
+// A game saving over a file OpenSave has just pulled is the game's change
+// however soon it comes - inside the slack a time comparison allows too.
+func TestRecent_AWriteRightAfterASettledPullIsNotOurs(t *testing.T) {
+	file := filepath.Join(t.TempDir(), "progress.sav")
+	Mark(file)
+	if err := os.WriteFile(file, []byte("from A"), 0o666); err != nil {
+		t.Fatal(err)
+	}
+	Settled(file)
+	if !Recent(file) {
+		t.Fatal("the file OpenSave pulled is not reported as ours")
+	}
+	// Same size, a moment later: only the time tells them apart.
+	time.Sleep(20 * time.Millisecond)
+	if err := os.WriteFile(file, []byte("from B"), 0o666); err != nil {
+		t.Fatal(err)
+	}
+	if Recent(file) {
+		t.Error("a save written straight after the pull was taken for OpenSave's")
+	}
+}
+
+// A pull renames a file into place, then re-dates it to the peer's time. In
+// between, and before it is Settled again, the file is still OpenSave's.
+func TestRecent_ARedatedPullBeforeItSettlesAgainIsOurs(t *testing.T) {
+	file := filepath.Join(t.TempDir(), "auto.sav")
+	Mark(file)
+	if err := os.WriteFile(file, []byte("pulled"), 0o666); err != nil {
+		t.Fatal(err)
+	}
+	Settled(file)
+	Mark(file)
+	peers := time.Now().Add(-time.Hour)
+	if err := os.Chtimes(file, peers, peers); err != nil {
+		t.Fatal(err)
+	}
+	if !Recent(file) {
+		t.Error("a pulled file re-dated to the peer's time was taken for someone else's change")
+	}
+	Settled(file)
+	if !Recent(file) {
+		t.Error("a pulled file settled at the peer's time was taken for someone else's change")
+	}
+}

internal/p2p/routes.go

@@ -17,6 +17,7 @@ import (
 	"github.com/go-chi/chi/v5"
 	"github.com/opensave/opensave/internal/delta"
 	"github.com/opensave/opensave/internal/logging"
+	"github.com/opensave/opensave/internal/owntouch"
 	"github.com/opensave/opensave/internal/p2p/pairing"
 	"github.com/opensave/opensave/internal/p2p/syncengine"
 	"github.com/opensave/opensave/internal/store"
@@ -829,6 +830,8 @@ func (e *Engine) handleDeleteFile(w http.ResponseWriter, r *http.Request) {
 			entry, _ = delta.FileEntryFor(full)
 		}
 		// Empty dirs only, for a folder, like rmdirSync.
+		// A change made at a peer's request, not by the game (owntouch).
+		owntouch.MarkRemoved(full)
 		if os.Remove(full) == nil && (info.IsDir() || entry.Hash != "") {
 			e.Sync.NotePeerDeletion(game.ID, body.Root, body.RelPath, entry, info.IsDir())
 		}

internal/p2p/syncengine/conflict.go

@@ -8,6 +8,7 @@ import (
 	"time"
 
 	"github.com/opensave/opensave/internal/delta"
+	"github.com/opensave/opensave/internal/owntouch"
 	"github.com/opensave/opensave/internal/snapshot"
 )
 
@@ -197,14 +198,18 @@ func (e *Engine) touchSaveMtimes(gameID, root string) {
 		return
 	}
 	if !info.IsDir() {
+		owntouch.Mark(root)
 		_ = os.Chtimes(root, now, now)
+		owntouch.Settled(root)
 		return
 	}
 	_ = filepath.Walk(root, func(path string, fi os.FileInfo, walkErr error) error {
 		if walkErr != nil || fi.IsDir() {
 			return nil
 		}
+		owntouch.Mark(path)
 		_ = os.Chtimes(path, now, now)
+		owntouch.Settled(path)
 		return nil
 	})
 }
@@ -337,6 +342,7 @@ func (e *Engine) overwriteLocalWithRemote(ctx context.Context, gameID string, pe
 		}
 		full := filepath.Join(game.SavePath, filepath.FromSlash(relPath))
 		_ = os.Chmod(full, 0o666)
+		owntouch.MarkRemoved(full)
 		_ = os.Remove(full)
 	}