Repository: C2FO/vfs
Pull request #178 (2024-03-08 23:35:38)

Issue 177: SFTP MoveToFile fails if destination file already exists


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index c42edd6..ab7ff36 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -4,6 +4,9 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 ## [Unreleased]
+### Fixed
+- Fixed #177 - SFTP MoveToFile fails if destination file already exists
+
 ### Added
 - Updated all backends to use error wrapping for Read, Write, Close, Seek. #174
 - Update GS backend to comply with new io integration test suite. #172
diff --git a/backend/sftp/file.go b/backend/sftp/file.go
index 009117c..62b1db0 100644
--- a/backend/sftp/file.go
+++ b/backend/sftp/file.go
@@ -168,6 +168,19 @@ func (f *File) MoveToFile(t vfs.File) error {
 				return err
 			}
 		}
+
+		// check if file already exists in the destination and delete if exists
+		exists, err = t.Exists()
+		if err != nil {
+			return err
+		}
+		if exists {
+			err := t.Delete()
+			if err != nil {
+				return err
+			}
+		}
+
 		return f.sftpRename(t.(*File))
 	}
 


Test patch:
diff --git a/backend/sftp/file_test.go b/backend/sftp/file_test.go
index 0f42250..d4244d8 100644
--- a/backend/sftp/file_test.go
+++ b/backend/sftp/file_test.go
@@ -588,7 +588,7 @@ func (ts *fileTestSuite) TestMoveToFile_sameAuthority() {
 	targetFileInfo.On("IsDir").Return(true).Once()
 
 	targetClient := &mocks.Client{}
-	targetClient.On("Stat", mock.Anything).Return(nil, os.ErrNotExist).Once()
+	targetClient.On("Stat", mock.Anything).Return(nil, os.ErrNotExist).Twice()
 
 	auth2, err := utils.NewAuthority("user@host1.com:22")
 	ts.NoError(err)
@@ -610,6 +610,58 @@ func (ts *fileTestSuite) TestMoveToFile_sameAuthority() {
 	targetClient.AssertExpectations(ts.T())
 }
 
+func (ts *fileTestSuite) TestMoveToFile_fileExists() {
+	// set up source
+	sourceClient := &mocks.Client{}
+
+	sourceClient.On("Rename", mock.Anything, mock.Anything).Return(nil).Once()
+	sourceClient.On("MkdirAll", mock.Anything).Return(nil).Once()
+
+	auth, err := utils.NewAuthority("user@host1.com:22")
+	ts.NoError(err)
+
+	sourceFile := &File{
+		fileSystem: &FileSystem{
+			sftpclient: sourceClient,
+			options:    Options{},
+		},
+		Authority: auth,
+		path:      "/some/path.txt",
+	}
+
+	rws := &mocks.SFTPFile{}
+	rws.On("Seek", int64(0), 1).Return(int64(0), nil)
+	sourceFile.opener = func(c Client, p string, f int) (ReadWriteSeekCloser, error) { return rws, nil }
+
+	// set up target
+	targetFileInfo := &mocks.FileInfo{}
+	targetFileInfo.On("IsDir").Return(true).Once()
+
+	targetClient := &mocks.Client{}
+
+	auth2, err := utils.NewAuthority("user@host1.com:22")
+	ts.NoError(err)
+
+	targetFile := &File{
+		fileSystem: &FileSystem{
+			sftpclient: targetClient,
+			options:    Options{},
+		},
+		Authority: auth2,
+		path:      "/some/other/path.txt",
+	}
+	targetClient.On("Stat", targetFile.Location().Path()).Return(nil, os.ErrNotExist).Once()
+	targetClient.On("Stat", targetFile.path).Return(targetFileInfo, nil).Once()
+	targetClient.On("Remove", targetFile.path).Return(nil).Once()
+
+	// run tests
+	err = sourceFile.MoveToFile(targetFile)
+	ts.Nil(err, "Error shouldn't be returned from successful call to CopyToFile")
+
+	sourceClient.AssertExpectations(ts.T())
+	targetClient.AssertExpectations(ts.T())
+}
+
 func (ts *fileTestSuite) TestMoveToLocation() {
 	content := "loc test"