blobserver/localdisk: handle renaming error on windows

http://camlistore.org/issue/241

Change-Id: Ib9c042d6074ff0dedc16f1519263c424898337a2

pkg/blobserver/localdisk/localdisk_test.go

@@ -18,7 +18,10 @@ package localdisk
 
 import (
 	"fmt"
+	"io/ioutil"
 	"os"
+	"path/filepath"
+	"runtime"
 	"strconv"
 	"sync"
 	"testing"
@@ -144,3 +147,44 @@ func TestMissingGetReturnsNoEnt(t *testing.T) {
 		t.Errorf("expected nil blob; got a value")
 	}
 }
+
+func rename(old, new string) error {
+	if err := os.Rename(old, new); err != nil {
+		if renameErr := mapRenameError(err, old, new); renameErr != nil {
+			return err
+		}
+	}
+	return nil
+}
+
+type file struct {
+	name     string
+	contents string
+}
+
+func TestRename(t *testing.T) {
+	if runtime.GOOS != "windows" {
+		t.Skip("Skipping test if not on windows")
+	}
+	files := []file{
+		file{name: filepath.Join(os.TempDir(), "foo"), contents: "foo"},
+		file{name: filepath.Join(os.TempDir(), "bar"), contents: "barr"},
+		file{name: filepath.Join(os.TempDir(), "baz"), contents: "foo"},
+	}
+	for _, v := range files {
+		if err := ioutil.WriteFile(v.name, []byte(v.contents), 0755); err != nil {
+			t.Fatal(err)
+		}
+	}
+
+	// overwriting "bar" with "foo" should not be allowed
+	if err := rename(files[0].name, files[1].name); err == nil {
+		t.Fatalf("Renaming %v into %v should not succeed", files[0].name, files[1].name)
+	}
+
+	// but overwriting "baz" with "foo" is ok because they have the same
+	// contents
+	if err := rename(files[0].name, files[2].name); err != nil {
+		t.Fatal(err)
+	}
+}

pkg/blobserver/localdisk/receive.go

@@ -75,7 +75,9 @@ func (ds *DiskStorage) ReceiveBlob(blobRef blob.Ref, source io.Reader) (ref blob
 
 	fileName := ds.blobPath("", blobRef)
 	if err = os.Rename(tempFile.Name(), fileName); err != nil {
-		return
+		if err = mapRenameError(err, tempFile.Name(), fileName); err != nil {
+			return
+		}
 	}
 
 	stat, err = os.Lstat(fileName)

pkg/blobserver/localdisk/receive_posix.go

@@ -33,3 +33,7 @@ func linkOrCopy(src, dst string) error {
 	}
 	return err
 }
+
+func handleRenameError(err error, oldfile, newfile string) error {
+	return err
+}

pkg/blobserver/localdisk/receive_windows.go

@@ -16,6 +16,43 @@ limitations under the License.
 
 package localdisk
 
+import (
+	"fmt"
+	"os"
+	"syscall"
+)
+
 func linkOrCopy(src, dst string) error {
 	return copyFile(src, dst)
 }
+
+// mapRenameError returns nil if and only if
+// 1) the input err is the error returned on windows when trying to rename
+// a file over one that already exists
+// 2) oldfile and newfile are the same files (i.e have the same size)
+func mapRenameError(err error, oldfile, newfile string) error {
+	linkErr, ok := err.(*os.LinkError)
+	if !ok {
+		return err
+	}
+	if linkErr.Err != error(syscall.ERROR_ALREADY_EXISTS) {
+		return err
+	}
+	// TODO(mpl): actually on linux at least, os.Rename apparently
+	// erases the destination with no error even if it is different
+	// from the source.
+	// So why don't we allow the same for windows? and if needed,
+	// do the check size before renaming?
+	statNew, err := os.Stat(newfile)
+	if err != nil {
+		return err
+	}
+	statOld, err := os.Stat(oldfile)
+	if err != nil {
+		return err
+	}
+	if statNew.Size() != statOld.Size() {
+		return fmt.Errorf("Will not overwrite destination file %v with source file %v, as they are different.", newfile, oldfile)
+	}
+	return nil
+}