A backup folder that is not connected is not written to

internal/api/routes.go

@@ -10,6 +10,7 @@ import (
 	"time"
 
 	"github.com/go-chi/chi/v5"
+	"github.com/opensave/opensave/internal/cloud"
 	"github.com/opensave/opensave/internal/daemon"
 	"github.com/opensave/opensave/internal/p2p/syncengine"
 	"github.com/opensave/opensave/internal/snapshot"
@@ -293,6 +294,14 @@ func (s *Server) applyCloudPatch(patch *cloudSyncPatch) error {
 			cfg.CustomClientSecrets[k] = v
 		}
 	}
+	// A local folder chosen now is the one meant: marked as the backup folder,
+	// so that later, a share that is not connected is told apart from it
+	// (cloud.MarkLocalFolder).
+	if cfg.Provider == "local" && (patch.URL != nil || patch.Provider != nil) {
+		if err := cloud.MarkLocalFolder(cfg.URL); err != nil {
+			return err
+		}
+	}
 	return s.Daemon.Store.UpdateCloudConfig(cfg)
 }
 

internal/cloud/cloud_test.go

@@ -60,7 +60,7 @@ func writeTempZip(t *testing.T, content string) string {
 
 func TestLocalFolderRoundTrip(t *testing.T) {
 	svc, s := newTestService(t)
-	destDir := t.TempDir()
+	destDir := markedDir(t)
 	setCloudConfig(t, s, func(c *store.CloudConfig) {
 		c.Enabled = true
 		c.Provider = "local"
@@ -274,7 +274,7 @@ func TestGoogleDriveProviderWithTokenRefresh(t *testing.T) {
 // stay, older ones are deleted, other games/branches are untouched.
 func TestPruneGameBranch(t *testing.T) {
 	svc, s := newTestService(t)
-	destDir := t.TempDir()
+	destDir := markedDir(t)
 	setCloudConfig(t, s, func(c *store.CloudConfig) {
 		c.Enabled = true
 		c.Provider = "local"
@@ -789,7 +789,7 @@ func TestUploadIsDoneWithTheArchiveWhenItSaysSo(t *testing.T) {
 	setCloudConfig(t, s, func(c *store.CloudConfig) {
 		c.Enabled = true
 		c.Provider = "local"
-		c.URL = t.TempDir()
+		c.URL = markedDir(t)
 	})
 	src := writeTempZip(t, "zip bytes")
 	var removeErr error
@@ -861,13 +861,16 @@ func folderNames(t *testing.T, dir string) []string {
 	}
 	var names []string
 	for _, e := range entries {
+		if e.Name() == folderMarker {
+			continue // what makes it the backup folder, not a copy's leftover
+		}
 		names = append(names, e.Name())
 	}
 	return names
 }
 
 func TestALocalCopyInProgressIsNotListed(t *testing.T) {
-	dir := t.TempDir()
+	dir := markedDir(t)
 	p := localFolder{cfg: store.CloudConfig{URL: dir}}
 	const name = "game__main__snap_1.zip"
 	body := []byte(strings.Repeat("archive bytes ", 4096))
@@ -908,7 +911,7 @@ func TestALocalCopyInProgressIsNotListed(t *testing.T) {
 }
 
 func TestALocalCopyThatFailsLeavesNothing(t *testing.T) {
-	dir := t.TempDir()
+	dir := markedDir(t)
 	p := localFolder{cfg: store.CloudConfig{URL: dir}}
 	if err := p.put(&failingReader{}, "game__main__snap_1.zip"); err == nil {
 		t.Fatal("a copy whose source failed reported success")
@@ -963,7 +966,7 @@ func TestALocalCopyIsAsReadableAsBefore(t *testing.T) {
 	if runtime.GOOS == "windows" {
 		t.Skip("permission bits are not how Windows shares a folder")
 	}
-	dir := t.TempDir()
+	dir := markedDir(t)
 	plain := filepath.Join(dir, "plain")
 	f, err := os.Create(plain)
 	if err != nil {

internal/cloud/provider_local.go

@@ -17,6 +17,85 @@ type localFolder struct {
 	cfg store.CloudConfig
 }
 
+// folderMarker is the file that says a folder is OpenSave's backup folder.
+// It is written when the folder is chosen (MarkLocalFolder), and a folder
+// without it is not written to or read as a backup.
+//
+// A network share that is not connected leaves its mount point behind: an
+// empty folder, or none. Backups were written into it, onto this machine's
+// own disk, and counted as sent, while the share never got them; and its
+// listing read as a backup with nothing in it. The marker is on the share,
+// so it is missing from the mount point.
+const folderMarker = ".opensave-backup-folder"
+
+// MarkLocalFolder makes dir OpenSave's backup folder: created if need be,
+// and marked. It runs when the folder is chosen, which is when it is known to
+// be the one meant.
+func MarkLocalFolder(dir string) error {
+	dir = strings.TrimSpace(dir)
+	if dir == "" {
+		return nil
+	}
+	if err := os.MkdirAll(dir, 0o777); err != nil {
+		return fmt.Errorf("could not use %q as the backup folder: %w", dir, err)
+	}
+	return writeMarker(dir)
+}
+
+func writeMarker(dir string) error {
+	path := filepath.Join(dir, folderMarker)
+	if _, err := os.Stat(path); err == nil {
+		return nil
+	}
+	body := "This folder holds OpenSave's backups. OpenSave looks for this file " +
+		"to know the folder is there: please leave it in place."
+	if err := os.WriteFile(path, []byte(body), 0o666); err != nil {
+		return fmt.Errorf("could not use %q as the backup folder: %w", dir, err)
+	}
+	return nil
+}
+
+// ErrFolderUnavailable is a backup folder that is not there to be used: not
+// connected, moved, or never chosen as one.
+type ErrFolderUnavailable struct{ Dir string }
+
+func (e ErrFolderUnavailable) Error() string {
+	return fmt.Sprintf("the backup folder %q is not available — if it is on a network share or a removable drive, "+
+		"check that it is connected; if it was moved, choose it again in Cloud Backup", e.Dir)
+}
+
+// ready says whether the configured folder is the backup folder now: there,
+// and marked. A folder already holding OpenSave's backups, chosen before the
+// marker existed, is marked now rather than refused.
+func (p localFolder) ready() error {
+	dir := p.cfg.URL
+	if info, err := os.Stat(dir); err != nil || !info.IsDir() {
+		return ErrFolderUnavailable{Dir: dir}
+	}
+	if _, err := os.Stat(filepath.Join(dir, folderMarker)); err == nil {
+		return nil
+	}
+	if holdsBackups(dir) {
+		return writeMarker(dir)
+	}
+	return ErrFolderUnavailable{Dir: dir}
+}
+
+// holdsBackups says whether dir holds a file named as OpenSave names what it
+// stores: <game>__<branch>__<snapshot>.zip, <game>__head-<device>-<ms>.zip.
+func holdsBackups(dir string) bool {
+	entries, err := os.ReadDir(dir)
+	if err != nil {
+		return false
+	}
+	for _, e := range entries {
+		if e.Type().IsRegular() && strings.HasSuffix(e.Name(), ".zip") && strings.Contains(e.Name(), "__") {
+			return true
+		}
+	}
+	return false
+}
+
 func (p localFolder) upload(f *os.File, size int64, fileName string) error {
 	return p.put(f, fileName)
 }
@@ -29,6 +108,9 @@ func (p localFolder) put(r io.Reader, fileName string) error {
 	if p.cfg.URL == "" {
 		return fmt.Errorf("no local folder destination configured")
 	}
+	if err := p.ready(); err != nil {
+		return err
+	}
 	return writeFileWhole(filepath.Join(p.cfg.URL, fileName), func(w io.Writer) error {
 		_, err := io.Copy(w, r)
 		return err
@@ -39,6 +121,9 @@ func (p localFolder) list() ([]CloudFile, error) {
 	if p.cfg.URL == "" {
 		return []CloudFile{}, nil
 	}
+	if err := p.ready(); err != nil {
+		return nil, err
+	}
 	entries, err := os.ReadDir(p.cfg.URL)
 	if err != nil {
 		return nil, fmt.Errorf("read local folder destination %q: %w", p.cfg.URL, err)

internal/cloud/provider_local_marker_test.go

@@ -0,0 +1,89 @@
+package cloud
+
+import (
+	"errors"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"github.com/opensave/opensave/internal/store"
+)
+
+// markedDir is a folder chosen as the backup folder, as Cloud Backup's
+// settings make one (MarkLocalFolder).
+func markedDir(t *testing.T) string {
+	t.Helper()
+	dir := t.TempDir()
+	if err := MarkLocalFolder(dir); err != nil {
+		t.Fatal(err)
+	}
+	return dir
+}
+
+// A network share that is not connected leaves an empty mount point behind.
+// Backups were written into it, onto this machine's own disk, and counted as
+// sent; its listing read as a backup with nothing in it (GitHub #46). Neither
+// happens to a folder that was never chosen as the backup folder.
+func TestBackupFolder_AnUnconnectedShareIsNotWrittenTo(t *testing.T) {
+	mountPoint := t.TempDir() // the share's folder, with the share not there
+	p := localFolder{cfg: store.CloudConfig{URL: mountPoint}}
+
+	err := p.put(strings.NewReader("archive"), "game__main__snap_1.zip")
+	var unavailable ErrFolderUnavailable
+	if !errors.As(err, &unavailable) {
+		t.Fatalf("put into an unconnected share = %v, want the folder reported unavailable", err)
+	}
+	if entries, _ := os.ReadDir(mountPoint); len(entries) != 0 {
+		t.Errorf("something was written into the mount point: %v", entries)
+	}
+	if files, err := p.list(); err == nil {
+		t.Errorf("an unconnected share listed as a backup holding %v", files)
+	}
+}
+
+// A backup folder that is gone is not made again by a backup: that is a
+// folder on this machine standing in for one on a drive that is not there.
+func TestBackupFolder_AMissingFolderIsNotCreated(t *testing.T) {
+	gone := filepath.Join(t.TempDir(), "NAS", "backups")
+	p := localFolder{cfg: store.CloudConfig{URL: gone}}
+	if err := p.put(strings.NewReader("archive"), "game__main__snap_1.zip"); err == nil {
+		t.Fatal("a backup into a missing folder succeeded")
+	}
+	if _, err := os.Stat(gone); err == nil {
+		t.Error("the missing backup folder was created by a backup")
+	}
+}
+
+// Choosing a folder makes it the backup folder, creating it if need be; a
+// share connected again is found by its marker.
+func TestBackupFolder_ChosenFolderTakesBackups(t *testing.T) {
+	dir := filepath.Join(t.TempDir(), "new backups")
+	if err := MarkLocalFolder(dir); err != nil {
+		t.Fatal(err)
+	}
+	p := localFolder{cfg: store.CloudConfig{URL: dir}}
+	if err := p.put(strings.NewReader("archive"), "game__main__snap_1.zip"); err != nil {
+		t.Fatalf("put into the chosen folder: %v", err)
+	}
+	files, err := p.list()
+	if err != nil || len(files) != 1 || files[0].Name != "game__main__snap_1.zip" {
+		t.Fatalf("list = %+v, %v; want the one backup, and not the marker", files, err)
+	}
+}
+
+// A folder chosen before the marker existed, already holding OpenSave's
+// backups, keeps working: it is marked the first time it is used.
+func TestBackupFolder_AFolderWithBackupsFromBeforeIsAdopted(t *testing.T) {
+	dir := t.TempDir()
+	if err := os.WriteFile(filepath.Join(dir, "game__main__snap_1.zip"), []byte("old backup"), 0o666); err != nil {
+		t.Fatal(err)
+	}
+	p := localFolder{cfg: store.CloudConfig{URL: dir}}
+	if err := p.put(strings.NewReader("archive"), "game__main__snap_2.zip"); err != nil {
+		t.Fatalf("put into a folder of earlier backups: %v", err)
+	}
+	if _, err := os.Stat(filepath.Join(dir, folderMarker)); err != nil {
+		t.Errorf("the folder was not marked: %v", err)
+	}
+}

internal/daemon/cloud_retry_test.go

@@ -48,6 +48,9 @@ func TestAFailedCloudUploadIsSentAgainWhenTheCloudIsBack(t *testing.T) {
 	if err := os.MkdirAll(cloudDir, 0o777); err != nil {
 		t.Fatal(err)
 	}
+	if err := cloud.MarkLocalFolder(cloudDir); err != nil { // the share, back with its marker
+		t.Fatal(err)
+	}
 	dev.check()
 
 	if _, err := os.Stat(filepath.Join(cloudDir, name)); err != nil {
@@ -76,11 +79,14 @@ func TestAFailedUploadOfASnapshotSinceRemovedIsForgotten(t *testing.T) {
 	if err := os.MkdirAll(cloudDir, 0o777); err != nil {
 		t.Fatal(err)
 	}
+	if err := cloud.MarkLocalFolder(cloudDir); err != nil { // the share, back with its marker
+		t.Fatal(err)
+	}
 	dev.check()
 	if pending, _ := dev.d.Store.CloudRetries(); len(pending) != 0 {
 		t.Errorf("a snapshot that is gone is still waiting to be sent: %+v", pending)
 	}
-	if entries, _ := os.ReadDir(cloudDir); len(entries) != 0 {
+	if entries, _ := os.ReadDir(cloudDir); len(entries) != 1 { // the backup folder's marker, and nothing sent
 		t.Errorf("something was sent for a snapshot that is gone: %v", entries)
 	}
 }

internal/daemon/cloudsync_test.go

@@ -44,6 +44,13 @@ func newCloudDevice(t *testing.T, name, cloudDir string) *cloudDevice {
 	cfg.Enabled = true
 	cfg.Provider = "local"
 	cfg.URL = cloudDir
+	// Chosen as the backup folder, as Cloud Backup's settings do — unless the
+	// test starts with the cloud out of reach (a file where the folder goes).
+	if info, err := os.Stat(cloudDir); err != nil || info.IsDir() {
+		if err := cloud.MarkLocalFolder(cloudDir); err != nil {
+			t.Fatal(err)
+		}
+	}
 	if err := d.Store.UpdateCloudConfig(cfg); err != nil {
 		t.Fatal(err)
 	}