Repository: C2FO/vfs
Pull request #128 (2022-08-18 21:16:19)

Fixed in-memory implementation edge cases
There were a couple of edge cases that this fixed:

1. Re-writing data on an in-memory file would previously append data to the existing file rather than overwrite. I fixed this to be consistent with other implementations:

```
file.Write([]byte("foo bar")
file.Close()
// contents: "foo bar"

file.Write([]byte("foo bar")
file.Close()
// contents: "foo barfoo bar"
```
2. Fixed issue where creating a file in a file system, and then opening it from a relative path in an initialized location yielded no file contents

```
fileSystem.NewFile("", "/path/to/some/file.txt")
file.Write([]byte("foo bar")
// contents: "foo bar"

location, _ := fileSystem.NewLocation("", "/path/to/")
f, _ := location.NewFile("some/file.txt")
ioutil.ReadAll(f) // contents: ""
```

codecov[bot]: # [Codecov](https://codecov.io/gh/C2FO/vfs/pull/128?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=C2FO) Report
> Merging [#128](https://codecov.io/gh/C2FO/vfs/pull/128?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=C2FO) (661ede7) into [master](https://codecov.io/gh/C2FO/vfs/commit/ed44db0806cb8b2ad1814e19eaed36d508e63bf0?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=C2FO) (ed44db0) will **increase** coverage by `0.03%`.
> The diff coverage is `100.00%`.

```diff
@@            Coverage Diff             @@
##           master     #128      +/-   ##
==========================================
+ Coverage   68.21%   68.24%   +0.03%     
==========================================
  Files          34       34              
  Lines        3835     3839       +4     
==========================================
+ Hits         2616     2620       +4     
  Misses        967      967              
  Partials      252      252              
```


| [Impacted Files](https://codecov.io/gh/C2FO/vfs/pull/128?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=C2FO) | Coverage Δ | |
|---|---|---|
| [backend/mem/file.go](https://codecov.io/gh/C2FO/vfs/pull/128/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=C2FO#diff-YmFja2VuZC9tZW0vZmlsZS5nbw==) | `61.49% <100.00%> (+0.30%)` | :arrow_up: |
| [backend/mem/location.go](https://codecov.io/gh/C2FO/vfs/pull/128/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=C2FO#diff-YmFja2VuZC9tZW0vbG9jYXRpb24uZ28=) | `84.39% <100.00%> (+0.11%)` | :arrow_up: |

Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=C2FO). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=C2FO)


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index cf4148f..4ba9d5e 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -5,6 +5,8 @@ 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]
 
+- fixed edge cases where in-memory files either weren't found when they should be, or didn't reset state on new writes
+
 ### Fixed
 - Replaced pre-generated ssh keypair in sftp options tests with auto-generated pair to remediate false-positive security alerts for hard-coded credentials.
 
diff --git a/backend/mem/file.go b/backend/mem/file.go
index 8e40c1f..243ccf4 100644
--- a/backend/mem/file.go
+++ b/backend/mem/file.go
@@ -187,6 +187,10 @@ func (f *File) Write(p []byte) (int, error) {
 			return 0, err
 		}
 	}
+	// If it's a new write session, assume data will be overwritten
+	if f.memFile.writeBuffer.Len() == 0 && len(f.memFile.contents) > 0 {
+		f.memFile.contents = []byte{}
+	}
 	f.memFile.Lock()
 	num, err := f.memFile.writeBuffer.Write(p)
 	f.memFile.lastModified = time.Now()
diff --git a/backend/mem/location.go b/backend/mem/location.go
index 077dd82..2abf841 100644
--- a/backend/mem/location.go
+++ b/backend/mem/location.go
@@ -170,7 +170,8 @@ func (l *Location) NewFile(relFilePath string) (vfs.File, error) {
 	// file already exists. if it does, return a reference to it
 	mapRef := l.fileSystem.fsMap
 	if _, ok := mapRef[l.volume]; ok {
-		fileList := mapRef[l.volume].filesHere(l.Path())
+		relativeLocationPath := utils.EnsureTrailingSlash(path.Dir(path.Join(l.Path(), relFilePath)))
+		fileList := mapRef[l.volume].filesHere(relativeLocationPath)
 		for _, file := range fileList {
 			if file.name == path.Base(relFilePath) {
 				fileCopy := deepCopy(file)


Test patch:
diff --git a/backend/mem/file_test.go b/backend/mem/file_test.go
index f4255b8..d9741b9 100644
--- a/backend/mem/file_test.go
+++ b/backend/mem/file_test.go
@@ -753,6 +753,27 @@ func (s *memFileTest) TestStringer() {
 
 }
 
+func (s *memFileTest) TestFileNewWrite() {
+	file, err := s.fileSystem.NewFile("", "/test_files/lots/of/directories/here/we/go/test.txt")
+	s.Require().NoError(err)
+	_, err = file.Write([]byte("hello world"))
+	s.Require().NoError(err)
+	s.Require().NoError(file.Close())
+
+	data, err := io.ReadAll(file)
+	s.Require().NoError(err)
+	s.Equal("hello world", string(data))
+
+	// Re-write the same data should not append
+	_, err = file.Write([]byte("hello world"))
+	s.Require().NoError(err)
+	s.Require().NoError(file.Close())
+
+	data, err = io.ReadAll(file)
+	s.Require().NoError(err)
+	s.Equal("hello world", string(data))
+}
+
 func TestMemFile(t *testing.T) {
 	suite.Run(t, new(memFileTest))
 	_ = os.Remove("test_files/new.txt")
diff --git a/backend/mem/location_test.go b/backend/mem/location_test.go
index ac1c073..b6d3db2 100644
--- a/backend/mem/location_test.go
+++ b/backend/mem/location_test.go
@@ -1,6 +1,7 @@
 package mem
 
 import (
+	"io"
 	"path"
 	"regexp"
 	"testing"
@@ -313,6 +314,32 @@ func (s *memLocationTest) TestDeleteFile() {
 
 }
 
+// TestWriteExistingFile tests that initializing a pre-existing file from a location, using a relative path will not result
+// in a blank file
+func (s *memLocationTest) TestWriteExistingFile() {
+
+	newFile, err := s.fileSystem.NewFile("", "/path/to/file/bar.txt")
+	s.NoError(err, "unexpected error creating a new file")
+
+	_, err = newFile.Write([]byte("hello world"))
+	s.Require().NoError(err)
+	s.Require().NoError(newFile.Close())
+
+	location, err := s.fileSystem.NewLocation("", "/path/")
+	s.Require().NoError(err)
+
+	file, err := location.NewFile("to/file/bar.txt")
+	s.Require().NoError(err)
+
+	exists, err := file.Exists()
+	s.Require().NoError(err)
+	s.True(exists)
+
+	data, err := io.ReadAll(file)
+	s.Require().NoError(err)
+	s.Equal("hello world", string(data))
+}
+
 func TestMemLocation(t *testing.T) {
 	suite.Run(t, new(memLocationTest))
 }