fix: add tests for file rename across volumes (influxdata#23787)

Also move shared code from file_unix.go

pkg/file/file.go

@@ -0,0 +1,44 @@
+package file
+
+import (
+	"fmt"
+	"io"
+	"os"
+
+	"github.com/influxdata/influxdb/pkg/errors"
+)
+
+func copyFile(src, dst string) (err error) {
+	in, err := os.Open(src)
+	if err != nil {
+		return err
+	}
+
+	out, err := os.Create(dst)
+	if err != nil {
+		return err
+	}
+
+	defer errors.Capture(&err, out.Close)()
+
+	defer errors.Capture(&err, in.Close)()
+
+	if _, err = io.Copy(out, in); err != nil {
+		return err
+	}
+
+	return out.Sync()
+}
+
+// MoveFileWithReplacement copies the file contents at `src` to `dst`.
+// and deletes `src` on success.
+//
+// If the file at `dst` already exists, it will be truncated and its contents
+// overwritten.
+func MoveFileWithReplacement(src, dst string) error {
+	if err := copyFile(src, dst); err != nil {
+		return fmt.Errorf("copy: %w", err)
+	}
+
+	return os.Remove(src)
+}

pkg/file/file_test.go

@@ -0,0 +1,142 @@
+package file_test
+
+import (
+	"io"
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/influxdata/influxdb/pkg/file"
+)
+
+func TestRenameFileWithReplacement(t *testing.T) {
+	testFileMoveOrRename(t, "rename", file.RenameFileWithReplacement)
+}
+
+func TestMoveFileWithReplacement(t *testing.T) {
+	testFileMoveOrRename(t, "move", file.MoveFileWithReplacement)
+}
+
+func testFileMoveOrRename(t *testing.T, name string, testFunc func(src string, dst string) error) {
+	// sample data for loading into files
+	sampleData1 := "this is some data"
+	sampleData2 := "we got some more data"
+
+	t.Run("exists", func(t *testing.T) {
+		oldPath := MustCreateTempFile(t, sampleData1)
+		newPath := MustCreateTempFile(t, sampleData2)
+		defer MustRemoveAll(oldPath)
+		defer MustRemoveAll(newPath)
+
+		oldContents := MustReadAllFile(oldPath)
+		newContents := MustReadAllFile(newPath)
+
+		if got, exp := oldContents, sampleData1; got != exp {
+			t.Fatalf("got contents %q, expected %q", got, exp)
+		} else if got, exp := newContents, sampleData2; got != exp {
+			t.Fatalf("got contents %q, expected %q", got, exp)
+		}
+
+		if err := testFunc(oldPath, newPath); err != nil {
+			t.Fatalf("%s returned an error: %s", name, err)
+		}
+
+		if err := file.SyncDir(filepath.Dir(oldPath)); err != nil {
+			panic(err)
+		}
+
+		// Contents of newpath will now be equivalent to oldpath' contents.
+		newContents = MustReadAllFile(newPath)
+		if newContents != oldContents {
+			t.Fatalf("contents for files differ: %q versus %q", newContents, oldContents)
+		}
+
+		// oldpath will be removed.
+		if MustFileExists(oldPath) {
+			t.Fatalf("file %q still exists, but it shouldn't", oldPath)
+		}
+	})
+
+	t.Run("not exists", func(t *testing.T) {
+		oldpath := MustCreateTempFile(t, sampleData1)
+		defer MustRemoveAll(oldpath)
+
+		oldContents := MustReadAllFile(oldpath)
+		if got, exp := oldContents, sampleData1; got != exp {
+			t.Fatalf("got contents %q, expected %q", got, exp)
+		}
+
+		root := filepath.Dir(oldpath)
+		newpath := filepath.Join(root, "foo")
+
+		if err := testFunc(oldpath, newpath); err != nil {
+			t.Fatalf("%s returned an error: %s", name, err)
+		}
+
+		if err := file.SyncDir(filepath.Dir(oldpath)); err != nil {
+			panic(err)
+		}
+
+		// Contents of newpath will now be equivalent to oldpath's contents.
+		newContents := MustReadAllFile(newpath)
+		if newContents != oldContents {
+			t.Fatalf("contents for files differ: %q versus %q", newContents, oldContents)
+		}
+
+		// oldpath will be removed.
+		if MustFileExists(oldpath) {
+			t.Fatalf("file %q still exists, but it shouldn't", oldpath)
+		}
+	})
+}
+
+// CreateTempFileOrFail creates a temporary file returning the path to the file.
+func MustCreateTempFile(t testing.TB, data string) string {
+	t.Helper()
+
+	f, err := os.CreateTemp("", "fs-test")
+	if err != nil {
+		t.Fatalf("failed to create temp file: %v", err)
+	} else if _, err := f.WriteString(data); err != nil {
+		t.Fatal(err)
+	} else if err := f.Close(); err != nil {
+		t.Fatal(err)
+	}
+	return f.Name()
+}
+
+func MustRemoveAll(path string) {
+	if err := os.RemoveAll(path); err != nil {
+		panic(err)
+	}
+}
+
+// MustFileExists determines if a file exists, panicking if any error
+// (other than one associated with the file not existing) is returned.
+func MustFileExists(path string) bool {
+	_, err := os.Stat(path)
+	if err == nil {
+		return true
+	} else if os.IsNotExist(err) {
+		return false
+	}
+	panic(err)
+}
+
+// MustReadAllFile reads the contents of path, panicking if there is an error.
+func MustReadAllFile(path string) string {
+	fd, err := os.Open(path)
+	if err != nil {
+		panic(err)
+	}
+	defer func() {
+		if err = fd.Close(); err != nil {
+			panic(err)
+		}
+	}()
+	data, err := io.ReadAll(fd)
+	if err != nil {
+		panic(err)
+	}
+	return string(data)
+}

pkg/file/file_unix.go

@@ -4,12 +4,8 @@ package file
 
 import (
 	"errors"
-	"fmt"
-	"io"
 	"os"
 	"syscall"
-
-	errors2 "github.com/influxdata/influxdb/pkg/errors"
 )
 
 func SyncDir(dirName string) error {
@@ -58,38 +54,3 @@ func RenameFileWithReplacement(oldpath, newpath string) error {
 func RenameFile(oldpath, newpath string) error {
 	return RenameFileWithReplacement(oldpath, newpath)
 }
-
-func copyFile(src, dst string) (err error) {
-	in, err := os.Open(src)
-	if err != nil {
-		return err
-	}
-
-	out, err := os.Create(dst)
-	if err != nil {
-		return err
-	}
-
-	defer errors2.Capture(&err, out.Close)()
-
-	defer errors2.Capture(&err, in.Close)()
-
-	if _, err = io.Copy(out, in); err != nil {
-		return err
-	}
-
-	return out.Sync()
-}
-
-// MoveFileWithReplacement copies the file contents at `src` to `dst`.
-// and deletes `src` on success.
-//
-// If the file at `dst` already exists, it will be truncated and its contents
-// overwritten.
-func MoveFileWithReplacement(src, dst string) error {
-	if err := copyFile(src, dst); err != nil {
-		return fmt.Errorf("copy: %w", err)
-	}
-
-	return os.Remove(src)
-}

pkg/file/file_windows.go

@@ -16,3 +16,19 @@ func RenameFile(oldpath, newpath string) error {
 
 	return os.Rename(oldpath, newpath)
 }
+
+// RenameFileWithReplacement will replace any existing file at newpath with the contents
+// of oldpath.
+//
+// If no file already exists at newpath, newpath will be created using the contents
+// of oldpath. If this function returns successfully, the contents of newpath will
+// be identical to oldpath, and oldpath will be removed.
+func RenameFileWithReplacement(oldpath, newpath string) error {
+	if _, err := os.Stat(newpath); err == nil {
+		if err = os.Remove(newpath); nil != err {
+			return err
+		}
+	}
+
+	return os.Rename(oldpath, newpath)
+}