From 9a02f17cc23be8813b17ed6bbd555f2fa185e309 Mon Sep 17 00:00:00 2001 From: Alexander Neumann Date: Sun, 20 May 2018 16:11:36 +0200 Subject: [PATCH 1/3] archiver: Add tests for Save() for fs.Reader --- internal/archiver/archiver_test.go | 149 +++++++++++++++++++++++++++++ 1 file changed, 149 insertions(+) diff --git a/internal/archiver/archiver_test.go b/internal/archiver/archiver_test.go index 136cf1291..df37a7b09 100644 --- a/internal/archiver/archiver_test.go +++ b/internal/archiver/archiver_test.go @@ -43,6 +43,11 @@ func saveFile(t testing.TB, repo restic.Repository, filename string, filesystem arch := New(repo, filesystem, Options{}) arch.runWorkers(ctx, &tmb) + arch.Error = func(item string, fi os.FileInfo, err error) error { + t.Errorf("archiver error for %v: %v", item, err) + return err + } + var ( completeCallbackNode *restic.Node completeCallbackStats ItemStats @@ -196,6 +201,150 @@ func TestArchiverSaveFileReaderFS(t *testing.T) { } } +func TestArchiverSave(t *testing.T) { + var tests = []TestFile{ + TestFile{Content: ""}, + TestFile{Content: "foo"}, + TestFile{Content: string(restictest.Random(23, 12*1024*1024+1287898))}, + } + + for _, testfile := range tests { + t.Run("", func(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + tempdir, repo, cleanup := prepareTempdirRepoSrc(t, TestDir{"file": testfile}) + defer cleanup() + + var tmb tomb.Tomb + + arch := New(repo, fs.Track{fs.Local{}}, Options{}) + arch.Error = func(item string, fi os.FileInfo, err error) error { + t.Errorf("archiver error for %v: %v", item, err) + return err + } + arch.runWorkers(tmb.Context(ctx), &tmb) + + node, excluded, err := arch.Save(ctx, "/", filepath.Join(tempdir, "file"), nil) + if err != nil { + t.Fatal(err) + } + + if excluded { + t.Errorf("Save() excluded the node, that's unexpected") + } + + node.wait(ctx) + if node.err != nil { + t.Fatal(node.err) + } + + if node.node == nil { + t.Fatalf("returned node is nil") + } + + stats := node.stats + + err = repo.Flush(ctx) + if err != nil { + t.Fatal(err) + } + + TestEnsureFileContent(ctx, t, repo, "file", node.node, testfile) + if stats.DataSize != uint64(len(testfile.Content)) { + t.Errorf("wrong stats returned in DataSize, want %d, got %d", len(testfile.Content), stats.DataSize) + } + if stats.DataBlobs <= 0 && len(testfile.Content) > 0 { + t.Errorf("wrong stats returned in DataBlobs, want > 0, got %d", stats.DataBlobs) + } + if stats.TreeSize != 0 { + t.Errorf("wrong stats returned in TreeSize, want 0, got %d", stats.TreeSize) + } + if stats.TreeBlobs != 0 { + t.Errorf("wrong stats returned in DataBlobs, want 0, got %d", stats.DataBlobs) + } + }) + } +} + +func TestArchiverSaveReaderFS(t *testing.T) { + var tests = []struct { + Data string + }{ + {Data: ""}, + {Data: "foo"}, + {Data: string(restictest.Random(23, 12*1024*1024+1287898))}, + } + + for _, test := range tests { + t.Run("", func(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + repo, cleanup := repository.TestRepository(t) + defer cleanup() + + ts := time.Now() + filename := "xx" + readerFs := &fs.Reader{ + ModTime: ts, + Mode: 0123, + Name: filename, + ReadCloser: ioutil.NopCloser(strings.NewReader(test.Data)), + } + + var tmb tomb.Tomb + + arch := New(repo, readerFs, Options{}) + arch.Error = func(item string, fi os.FileInfo, err error) error { + t.Errorf("archiver error for %v: %v", item, err) + return err + } + arch.runWorkers(tmb.Context(ctx), &tmb) + + node, excluded, err := arch.Save(ctx, "/", filename, nil) + t.Logf("Save returned %v %v", node, err) + if err != nil { + t.Fatal(err) + } + + if excluded { + t.Errorf("Save() excluded the node, that's unexpected") + } + + node.wait(ctx) + if node.err != nil { + t.Fatal(node.err) + } + + if node.node == nil { + t.Fatalf("returned node is nil") + } + + stats := node.stats + + err = repo.Flush(ctx) + if err != nil { + t.Fatal(err) + } + + TestEnsureFileContent(ctx, t, repo, "file", node.node, TestFile{Content: test.Data}) + if stats.DataSize != uint64(len(test.Data)) { + t.Errorf("wrong stats returned in DataSize, want %d, got %d", len(test.Data), stats.DataSize) + } + if stats.DataBlobs <= 0 && len(test.Data) > 0 { + t.Errorf("wrong stats returned in DataBlobs, want > 0, got %d", stats.DataBlobs) + } + if stats.TreeSize != 0 { + t.Errorf("wrong stats returned in TreeSize, want 0, got %d", stats.TreeSize) + } + if stats.TreeBlobs != 0 { + t.Errorf("wrong stats returned in DataBlobs, want 0, got %d", stats.DataBlobs) + } + }) + } +} + func BenchmarkArchiverSaveFileSmall(b *testing.B) { const fileSize = 4 * 1024 d := TestDir{"file": TestFile{ From 1e9744c9a42972e93d026ea1638f3ab56b4e34d7 Mon Sep 17 00:00:00 2001 From: Alexander Neumann Date: Sun, 20 May 2018 15:58:55 +0200 Subject: [PATCH 2/3] archiver: Refuse to save an empty snapshot --- internal/archiver/archiver.go | 4 ++++ internal/archiver/archiver_test.go | 15 ++++++++++++++- 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/internal/archiver/archiver.go b/internal/archiver/archiver.go index 3295cb6a1..be80e0a74 100644 --- a/internal/archiver/archiver.go +++ b/internal/archiver/archiver.go @@ -762,6 +762,10 @@ func (arch *Archiver) Snapshot(ctx context.Context, targets []string, opts Snaps return restic.ID{}, ItemStats{}, err } + if len(tree.Nodes) == 0 { + return restic.ID{}, ItemStats{}, errors.New("snapshot is empty") + } + return arch.saveTree(wctx, tree) }() debug.Log("saved tree, error: %v", err) diff --git a/internal/archiver/archiver_test.go b/internal/archiver/archiver_test.go index df37a7b09..cff28c519 100644 --- a/internal/archiver/archiver_test.go +++ b/internal/archiver/archiver_test.go @@ -1343,6 +1343,7 @@ func TestArchiverSnapshotSelect(t *testing.T) { src TestDir want TestDir selFn SelectFunc + err string }{ { name: "include-all", @@ -1377,7 +1378,7 @@ func TestArchiverSnapshotSelect(t *testing.T) { selFn: func(item string, fi os.FileInfo) bool { return false }, - want: TestDir{}, + err: "snapshot is empty", }, { name: "exclude-txt-files", @@ -1462,6 +1463,18 @@ func TestArchiverSnapshotSelect(t *testing.T) { targets := []string{"."} _, snapshotID, err := arch.Snapshot(ctx, targets, SnapshotOptions{Time: time.Now()}) + if test.err != "" { + if err == nil { + t.Fatalf("expected error not found, got %v, wanted %q", err, test.err) + } + + if err.Error() != test.err { + t.Fatalf("unexpected error, want %q, got %q", test.err, err) + } + + return + } + if err != nil { t.Fatal(err) } From adb682bc437c99237c07d9173ec85c415e33798f Mon Sep 17 00:00:00 2001 From: Alexander Neumann Date: Sun, 20 May 2018 16:05:53 +0200 Subject: [PATCH 3/3] archiver: Don't open files with O_NONBLOCK This is not necessary any more, we're doing an lstat() before opening an item, so we already known it's a file and not a pipe. --- internal/archiver/archiver.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/archiver/archiver.go b/internal/archiver/archiver.go index be80e0a74..0bd839b65 100644 --- a/internal/archiver/archiver.go +++ b/internal/archiver/archiver.go @@ -338,7 +338,7 @@ func (arch *Archiver) Save(ctx context.Context, snPath, target string, previous // reopen file and do an fstat() on the open file to check it is still // a file (and has not been exchanged for e.g. a symlink) - file, err := arch.FS.OpenFile(target, fs.O_RDONLY|fs.O_NOFOLLOW|fs.O_NONBLOCK, 0) + file, err := arch.FS.OpenFile(target, fs.O_RDONLY|fs.O_NOFOLLOW, 0) if err != nil { debug.Log("Openfile() for %v returned error: %v", target, err) err = arch.error(abstarget, fi, err)