sync: untracking forgets served states; the served-manifest proof repairs the shared-files record

e2e/deletion_record_test.go

@@ -203,3 +203,65 @@ func TestDeletion_SurvivesALostReport(t *testing.T) {
 		t.Errorf("the deletion never reached B: %q", got)
 	}
 }
+
+// The same with the proof from the other direction: B pulled what A served it
+// and holds it still, while the report that records the files both hold is
+// lost. A banks the served state as agreed; the record of shared files is
+// repaired with it, so a file A deletes before its watcher notices is a
+// deletion for B, not a file to fetch back (#66).
+func TestDeletion_SurvivesALostReportWhenServed(t *testing.T) {
+	a, b, gameID := pairAndTrack(t, "Lost Report Served Deletion", map[string]string{"seed.sav": "seed"})
+	testutil.SettleSync(t, gameID, a, b)
+	st := a.Daemon.Store
+	var oldBase string
+	if !testutil.WaitFor(30*time.Second, func() bool {
+		oldBase = st.GetAgreedHash(gameID, b.NodeID())
+		return oldBase != ""
+	}) {
+		t.Fatal("setup: the devices never recorded that they agree")
+	}
+
+	a.WriteSave("auto.sav", "v1")
+	time.Sleep(syncSettleWindow)
+	a.API(http.MethodPost, "/api/games/"+gameID+"/sync", nil, nil)
+	if !testutil.WaitFor(30*time.Second, func() bool { return b.ReadSave("auto.sav") == "v1" }) {
+		t.Fatal("setup: auto.sav never reached B")
+	}
+	testutil.SettleSync(t, gameID, a, b)
+
+	handedOver, err := delta.BuildManifest(a.SaveDir)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !testutil.WaitFor(30*time.Second, func() bool {
+		return st.GetAgreedHash(gameID, b.NodeID()) == handedOver.ManifestHash()
+	}) {
+		t.Fatal("setup: the report never arrived")
+	}
+	// Undone as a lost report leaves it, with only the served state to go
+	// by: the old base (which clears the push record), the old record of
+	// shared files, and what A served B under that base.
+	if err := st.SetAgreedHash(gameID, b.NodeID(), oldBase); err != nil {
+		t.Fatal(err)
+	}
+	if st.GetPushedHash(gameID, b.NodeID()) != "" {
+		t.Fatal("setup: a push is still on record")
+	}
+	if err := st.SetSyncState(gameID, b.NodeID(), []string{"seed.sav"}, nil); err != nil {
+		t.Fatal(err)
+	}
+	a.Daemon.P2P.Sync.NoteServed(gameID, b.NodeID(), handedOver)
+
+	if err := os.Remove(filepath.Join(a.SaveDir, "auto.sav")); err != nil {
+		t.Fatal(err)
+	}
+	a.API(http.MethodPost, "/api/games/"+gameID+"/sync", nil, nil) // before the watcher records the deletion
+	time.Sleep(5 * time.Second)
+
+	if got := a.ReadSave("auto.sav"); got != "" {
+		t.Errorf("the deleted file came back: %q", got)
+	}
+	if got := b.ReadSave("auto.sav"); got != "" {
+		t.Errorf("the deletion never reached B: %q", got)
+	}
+}

internal/daemon/daemon.go

@@ -1081,6 +1081,7 @@ func (d *Daemon) UntrackGame(gameID string) error {
 	// same ID and would inherit all of it — and a stale lineage reads a file
 	// that went missing while untracked as a deletion to propagate.
 	_ = d.Store.ForgetGameSyncState(gameID)
+	d.P2P.Sync.ForgetServed(gameID)
 	d.P2P.ClearPendingResync(gameID)
 	d.P2P.NotifyUntrack(gameID)
 	return nil
@@ -1238,6 +1239,7 @@ func (d *Daemon) untrackFromPeer(gameID string) {
 	// And everything it agreed with any peer, for the reason given in
 	// UntrackGame: the ID comes back, and this must not come back with it.
 	_ = d.Store.ForgetGameSyncState(gameID)
+	d.P2P.Sync.ForgetServed(gameID)
 	d.P2P.ClearPendingResync(gameID)
 	d.Log.Log("info", fmt.Sprintf("game %q untracked on a paired device", gameID))
 }

internal/p2p/syncengine/engine.go

@@ -605,8 +605,19 @@ func (e *Engine) SyncWithPeer(ctx context.Context, gameID string, peer Peer) (Re
 	// Matched on the peer's save as served, before this side's exclusion rules
 	// filtered it, which is how it was recorded; banked in today's filtered
 	// terms. Only against the base it was served under (served.go).
+	//
+	// The record of which files both hold is repaired with it, as the push
+	// proof above does and for the same reason: without it, a file the peer
+	// took from what was served and this device deleted before the watcher
+	// noted it read as one the peer had and this device never did, and was
+	// fetched back. And not while the game is held as emptied, for the same
+	// reason as above. Safe since untracking forgets what was served
+	// (ForgetServed): a re-tracked game no longer banks a state served before.
 	if remoteHash := remoteData.Manifest.ManifestHash(); agreedHash != remoteHash &&
 		e.servedUnderBase(gameID, peer.ID, unfilteredRemote.ManifestHash(), storedBase) {
+		if _, held, err := e.Store.GetDeletionHold(gameID); err == nil && !held {
+			e.persistLineage(gameID, peer.ID, remoteData.Manifest, remoteData.Manifest)
+		}
 		_ = e.Store.SetAgreedHash(gameID, peer.ID, remoteHash)
 		agreedHash = remoteHash
 	}

internal/p2p/syncengine/served.go

@@ -1,6 +1,7 @@
 package syncengine
 
 import (
+	"strings"
 	"time"
 
 	"github.com/opensave/opensave/internal/delta"
@@ -71,6 +72,23 @@ func (e *Engine) NoteServed(gameID, peerID string, m delta.Manifest) {
 	e.served[key] = list
 }
 
+// ForgetServed drops every served state recorded for a game, with any peer.
+// Called when the game is untracked, with the rest of what it agreed with its
+// peers (store.ForgetGameSyncState). The records live in memory, so they
+// outlived the untrack: a game tracked again within the hour could bank a
+// state served before it was untracked as agreed, against a base that the
+// untrack had wiped.
+func (e *Engine) ForgetServed(gameID string) {
+	prefix := servedKey(gameID, "")
+	e.servedMu.Lock()
+	defer e.servedMu.Unlock()
+	for key := range e.served {
+		if strings.HasPrefix(key, prefix) {
+			delete(e.served, key)
+		}
+	}
+}
+
 // servedUnderBase reports whether this device served hash to the peer while
 // base was the agreed state — and so whether a peer holding hash holds a state
 // this device held since that agreement.

internal/p2p/syncengine/served_forget_test.go

@@ -0,0 +1,34 @@
+package syncengine
+
+import (
+	"testing"
+
+	"github.com/opensave/opensave/internal/delta"
+)
+
+// Untracking a game forgets what this device served for it, so a re-track
+// cannot bank a state served before the untrack; other games keep theirs.
+func TestForgetServed_ForgetsOnlyThatGame(t *testing.T) {
+	env := setupEngine(t)
+	write(t, env.localDir, "save.dat", "v1")
+	m, err := delta.BuildManifest(env.localDir)
+	if err != nil {
+		t.Fatal(err)
+	}
+	e := env.engine
+	e.NoteServed("game1", env.peer.ID, m)
+	e.NoteServed("game10", env.peer.ID, m)
+	base := e.Store.GetAgreedHash("game1", env.peer.ID)
+	if !e.servedUnderBase("game1", env.peer.ID, m.ManifestHash(), base) {
+		t.Fatal("setup: the served state was not recorded")
+	}
+
+	e.ForgetServed("game1")
+
+	if e.servedUnderBase("game1", env.peer.ID, m.ManifestHash(), base) {
+		t.Error("a state served before the untrack is still remembered")
+	}
+	if !e.servedUnderBase("game10", env.peer.ID, m.ManifestHash(), e.Store.GetAgreedHash("game10", env.peer.ID)) {
+		t.Error("forgetting game1 also forgot game10")
+	}
+}