diff --git a/CHANGELOG.md b/CHANGELOG.md index 2d04ca85..bd7b4e51 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +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.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). -## [Unreleased] +## [5.5.2] - 2020-04-23 +### Fixed +- Ensure that writing truncates existing file. Fixes #40 ## [5.5.1] - 2020-02-20 ### Fixed diff --git a/backend/os/file.go b/backend/os/file.go index c0b5b767..09dc140a 100644 --- a/backend/os/file.go +++ b/backend/os/file.go @@ -1,8 +1,8 @@ package os import ( - "errors" "fmt" + "io/ioutil" "os" "path" "path/filepath" @@ -12,11 +12,17 @@ import ( "github.com/c2fo/vfs/v5/utils" ) +type opener func(filePath string) (*os.File, error) + //File implements vfs.File interface for os fs. type File struct { - file *os.File - name string - location vfs.Location + file *os.File + name string + location vfs.Location + cursorPos int64 + tempFile *os.File + useTempFile bool + fileOpener opener } // Delete unlinks the file returning any error or nil. @@ -61,6 +67,28 @@ func (f *File) Size() (uint64, error) { // Close implements the io.Closer interface, closing the underlying *os.File. its an error, if any. func (f *File) Close() error { + // check if temp file + // close temp file + // os.Rename() (replace) temp file to file + f.useTempFile = false + f.cursorPos = 0 + if f.tempFile != nil { + err := f.tempFile.Close() + if err != nil { + return err + } + + // get original file, open it if it has not been opened + finalFile, err := f.getInternalFile() + if err != nil { + return err + } + err = os.Rename(f.tempFile.Name(), finalFile.Name()) + if err != nil && !os.IsNotExist(err) { + return err + } + f.tempFile = nil + } if f.file == nil { // Do nothing on files that were never referenced return nil @@ -75,37 +103,46 @@ func (f *File) Close() error { // Read implements the io.Reader interface. It returns the bytes read and an error, if any. func (f *File) Read(p []byte) (int, error) { - if exists, err := f.Exists(); err != nil { + + // if we have not written to this file, ensure the original file exists + if !f.useTempFile { + if exists, err := f.Exists(); err != nil { + return 0, err + } else if !exists { + return 0, fmt.Errorf("failed to read. File does not exist at %s", f) + } + } + // get the file we need, either tempFile or original file + useFile, err := f.getInternalFile() + if err != nil { return 0, err - } else if !exists { - return 0, fmt.Errorf("failed to read. File does not exist at %s", f) } - file, err := f.openFile() + read, err := useFile.Read(p) if err != nil { - return 0, err + return read, err } - return file.Read(p) + f.cursorPos += int64(read) + + return read, nil } -//Seek implements the io.Seeker interface. It accepts an offset and "whench" where 0 means relative to the origin of +//Seek implements the io.Seeker interface. It accepts an offset and "whence" where 0 means relative to the origin of // the file, 1 means relative to the current offset, and 2 means relative to the end. It returns the new offset and // an error, if any. func (f *File) Seek(offset int64, whence int) (int64, error) { - - if exists, err := f.Exists(); !exists { - if err != nil { - return 0, err - } - return 0, errors.New("file does not exist") + useFile, err := f.getInternalFile() + if err != nil { + return 0, err } - file, err := f.openFile() + + f.cursorPos, err = useFile.Seek(offset, whence) if err != nil { return 0, err } - return file.Seek(offset, whence) + return f.cursorPos, err } // Exists true if the file exists on the file system, otherwise false, and an error, if any. @@ -125,11 +162,20 @@ func (f *File) Exists() (bool, error) { //Write implements the io.Writer interface. It accepts a slice of bytes and returns the number of bytes written and an error, if any. func (f *File) Write(p []byte) (n int, err error) { - file, err := f.openFile() + f.useTempFile = true + + useFile, err := f.getInternalFile() if err != nil { return 0, err } - return file.Write(p) + write, err := useFile.Write(p) + if err != nil { + return 0, err + } + offset := int64(write) + f.cursorPos += offset + + return write, err } // Location returns the underlying os.Location. @@ -254,14 +300,30 @@ func (f *File) openFile() (*os.File, error) { return f.file, nil } + // replace default file opener, is set in struct + openFunc := openOSFile + if f.fileOpener != nil { + openFunc = f.fileOpener + } + + file, err := openFunc(f.Path()) + if err != nil { + return nil, err + } + f.file = file + + return file, nil +} + +func openOSFile(filePath string) (*os.File, error) { + // Ensure the path exists before opening the file, NoOp if dir already exists. var fileMode os.FileMode = 0666 - if err := os.MkdirAll(f.Location().Path(), os.ModeDir|0777); err != nil { + if err := os.MkdirAll(path.Dir(filePath), os.ModeDir|0777); err != nil { return nil, err } - file, err := os.OpenFile(f.Path(), os.O_RDWR|os.O_CREATE, fileMode) - f.file = file + file, err := os.OpenFile(filePath, os.O_RDWR|os.O_CREATE, fileMode) return file, err } @@ -275,3 +337,65 @@ func ensureDir(location vfs.Location) error { } return nil } + +// If cursor is not (0,0) will copy original file to a temp file, +//opening its file descriptor to the current cursor position. +//If cursor is (0,0), just begin writing to new temp file. +//No need to copy original first. +func (f *File) getInternalFile() (*os.File, error) { + // this is the use case of vfs.file + if f.useTempFile == false { + if f.file == nil { + + // replace default file opener, is set in struct + openFunc := openOSFile + if f.fileOpener != nil { + openFunc = f.fileOpener + } + + finalFile, err := openFunc(f.Path()) + if err != nil { + return nil, err + } + f.file = finalFile + } + return f.file, nil + } + // this is the use case of vfs.tempFile + if f.tempFile == nil { + localTempFile, err := f.copyToLocalTempReader() + if err != nil { + return nil, err + } + f.tempFile = localTempFile + } + + return f.tempFile, nil +} + +func (f *File) copyToLocalTempReader() (*os.File, error) { + tmpFile, err := ioutil.TempFile("", fmt.Sprintf("%s.%d", f.Name(), time.Now().UnixNano())) + if err != nil { + return nil, err + } + + openFunc := openOSFile + if f.fileOpener != nil { + openFunc = f.fileOpener + } + + if _, err = openFunc(f.Path()); err != nil { + return nil, err + } + // todo: editing in place logic/appending logic (see issue #42) + //if _, err := io.Copy(tmpFile, f.file); err != nil { + // return nil, err + //} + // + //// Return cursor to the beginning of the new temp file + //if _, err := tmpFile.Seek(f.cursorPos, 0); err != nil { + // return nil, err + //} + + return tmpFile, nil +} diff --git a/backend/os/fileSystem.go b/backend/os/fileSystem.go index 905fac85..f1a21425 100644 --- a/backend/os/fileSystem.go +++ b/backend/os/fileSystem.go @@ -15,7 +15,7 @@ const name = "os" // FileSystem implements vfs.Filesystem for the OS file system. type FileSystem struct{} -// Retry will return a retrier provided via options, or a no-op if none is provided. +// Retry will return a retriever provided via options, or a no-op if none is provided. func (fs *FileSystem) Retry() vfs.Retry { return vfs.DefaultRetryer() } diff --git a/backend/os/file_test.go b/backend/os/file_test.go index 558af789..a6d17544 100644 --- a/backend/os/file_test.go +++ b/backend/os/file_test.go @@ -1,6 +1,7 @@ package os import ( + "errors" "fmt" "io/ioutil" "os" @@ -106,6 +107,7 @@ func (s *osFileTest) TestTouch() { nextModTime, err := testfile.LastModified() s.NoError(err) s.True(firstModTime.Before(*nextModTime), "Last Modified was updated") + s.NoError(testfile.Close()) } func (s *osFileTest) TestOpenFile() { @@ -130,15 +132,35 @@ func (s *osFileTest) TestRead() { s.NoError(err) b, err := f.Write([]byte("blah")) s.NoError(err) - s.Equal(int(4), b) + s.Equal(4, b) s.NoError(f.Close()) //read from file data = make([]byte, 4) b, err = f.Read(data) s.NoError(err) - s.Equal(int(4), b) + s.Equal(4, b) s.Equal("blah", string(data)) + s.NoError(f.Close()) + + //setup file for err out of opening + f, err = s.tmploc.NewFile("test_files/readFileFail.txt") + s.NoError(err) + f.(*File).useTempFile = true + f.(*File).fileOpener = func(filePath string) (*os.File, error) { return nil, errors.New("Bad Opener") } + data = make([]byte, 4) + b, err = f.Read(data) + s.Error(err) + + f.(*File).fileOpener = nil + b, err = f.Write([]byte("blah")) + s.NoError(err) + s.Equal(4, b) + s.NoError(f.Close()) + f.(*File).fileOpener = func(filePath string) (*os.File, error) { return nil, errors.New("Bad Opener") } + data = make([]byte, 4) + b, err = f.Read(data) + s.Error(err) } func (s *osFileTest) TestSeek() { @@ -149,6 +171,15 @@ func (s *osFileTest) TestSeek() { _, rerr := s.testFile.Read(data) s.NoError(rerr, "read error not expected") s.Equal(expectedText, string(data)) + s.NoError(s.testFile.Close()) + + //setup file for err out of opening + f, err := s.tmploc.NewFile("test_files/seekFileFail.txt") + s.NoError(err) + f.(*File).useTempFile = true + f.(*File).fileOpener = func(filePath string) (*os.File, error) { return nil, errors.New("Bad Opener") } + _, err = f.Seek(0, 4) + s.Error(err) } func (s *osFileTest) TestCopyToLocation() { @@ -350,7 +381,6 @@ func (s *osFileTest) TestMoveToFile() { s.NoError(rerr, "read error not expected") cErr := file2.Close() s.NoError(cErr, "close error not expected") - s.Equal(text, string(data)) // test non-scheme MoveToFile @@ -404,6 +434,133 @@ func (s *osFileTest) TestWrite() { found2, eErr2 := file.Exists() s.NoError(eErr2, "exists error not expected") s.False(found2) + + //setup file for err out of opening + f, err := s.tmploc.NewFile("test_files/writeFileFail.txt") + s.NoError(err) + f.(*File).useTempFile = true + f.(*File).fileOpener = func(filePath string) (*os.File, error) { return nil, errors.New("Bad Opener") } + data = make([]byte, 4) + _, err = f.Write(data) + s.Error(err) +} + +func (s *osFileTest) TestCursor() { + file, err := s.tmploc.NewFile("test_files/originalFile.txt") + s.NoError(err) + + expectedText := "mary had \na little lamb\n" + write, werr := file.Write([]byte(expectedText)) + s.NoError(werr, "write error not expected") + s.Equal(24, write) + s.NoError(file.Close()) + + _, serr := file.Seek(5, 0) // cursor 5 - opens fd to orig file + s.Equal(int64(5), file.(*File).cursorPos) + s.NoError(serr) + + data := make([]byte, 3) + sz, rerr := file.Read(data) // cursor 8 - orig file - data: "had" + s.NoError(rerr) + s.Equal(int64(8), file.(*File).cursorPos) + s.Equal("had", string(data)) // orig file contents = "had" + + negsz := int64(-sz) + _, serr2 := file.Seek(negsz, 1) // cursor 5 - orig file + s.Equal(int64(5), file.(*File).cursorPos) + s.NoError(serr2) + + sz, werr = file.Write([]byte("has")) // cursor 8 - tempfile copy of orig - write on tempfile has occurred + s.NoError(werr) + s.Equal(int64(8), file.(*File).cursorPos) + + _, serr = file.Seek(0, 0) // cursor 0 - in temp file + s.Equal(int64(0), file.(*File).cursorPos) + s.NoError(serr) + + data = make([]byte, 3) + sz, rerr = file.Read(data) + s.NoError(rerr) + s.Equal(int64(3), file.(*File).cursorPos) + s.Equal("has", string(data)) // tempFile contents = "has" + + s.NoError(file.Close()) // moves tempfile containing "has" over original file + + final := make([]byte, 3) + rd, err := file.Read(final) + s.NoError(err) + s.Equal(3, rd) + s.Equal("has", string(final)) + s.NoError(file.Close()) + + // if a file exists and we overwrite with a smaller # of text, then it isn't completely overwritten + // //somefile.txt contains "the quick brown" + file, err = s.tmploc.NewFile("test_files/someFile.txt") + s.NoError(err) + + expectedText = "the quick brown" + write, werr = file.Write([]byte(expectedText)) + s.NoError(werr, "write error not expected") + s.Equal(15, write) + + s.NoError(file.Close()) + + overwrite, err := file.Write([]byte("hello")) // cursor 5 of tempfile + s.NoError(err) + s.Equal(int64(5), file.(*File).cursorPos) + s.Equal(5, overwrite) + + data = make([]byte, 5) + _, serr = file.Seek(0, 0) // cursor 0 of tempfile + s.NoError(serr) + + _, rerr = file.Read(data) // cursor 5 of tempfile - data: "hello" + s.NoError(rerr) + s.Equal("hello", string(data)) + + data = make([]byte, 3) + sought, serr := file.Seek(-3, 2) // cursor 3 from end of tempfile + s.NoError(serr) + s.Equal(int64(2), sought) // seek returns position relative to beginning of file + + rd, rerr = file.Read(data) // cursor 0 from end of tempfile - data: "llo" + s.NoError(rerr) + s.Equal(3, rd) + s.Equal("llo", string(data)) + s.Equal(int64(5), file.(*File).cursorPos) + + _, serr = file.Seek(0, 0) + s.NoError(serr) + final = make([]byte, 5) + rd, err = file.Read(final) + s.NoError(err) + s.Equal(5, rd) + s.Equal("hello", string(final)) + s.NoError(file.Close()) // moves tempfile containing "hello" over somefile.txt +} + +func (s *osFileTest) TestCursorErrs() { + noFile, err := s.tmploc.NewFile("test_files/nonet.txt") + s.NoError(err) + data := make([]byte, 10) + _, err = noFile.Read(data) + s.Error(err) + s.NoError(noFile.Close()) + + _, err = noFile.Seek(10, 10) + s.Error(err) + + _, err = noFile.Seek(-10, 2) + s.Error(err) + s.NoError(noFile.Close()) + + noFile.(*File).fileOpener = nil + noFile.(*File).useTempFile = false + b, err := noFile.Write([]byte("blah")) + s.NoError(err) + s.Equal(4, b) + noFile.(*File).fileOpener = func(filePath string) (*os.File, error) { return nil, errors.New("Bad Opener") } + s.Error(noFile.Close()) } func (s *osFileTest) TestLastModified() { diff --git a/backend/testsuite/backend_integration_test.go b/backend/testsuite/backend_integration_test.go index 11070484..b58cc249 100644 --- a/backend/testsuite/backend_integration_test.go +++ b/backend/testsuite/backend_integration_test.go @@ -848,7 +848,7 @@ func (s *vfsTestSuite) File(baseLoc vfs.Location) { _, err = srcFile.LastModified() s.Error(err, "expected error because file does not exist") - seeked, err := srcFile.Seek(1, 2) + seeked, err := srcFile.Seek(-1, 2) s.Error(err, "expected error because file does not exist") s.Equal(int64(0x0), seeked)