From cfc9e8b2faf7a0c1d2f5916599f0639453c16c49 Mon Sep 17 00:00:00 2001 From: Alexander Neumann Date: Mon, 23 Jan 2017 17:20:08 +0100 Subject: [PATCH] backends: Remove Load() --- .../archiver/archiver_duplication_test.go | 4 +- src/restic/backend.go | 10 +- src/restic/backend/local/backend_test.go | 14 -- src/restic/backend/local/local.go | 36 --- src/restic/backend/mem/backend_test.go | 14 -- src/restic/backend/mem/mem_backend.go | 40 ---- src/restic/backend/rest/backend_test.go | 14 -- src/restic/backend/rest/rest.go | 57 ----- src/restic/backend/s3/backend_test.go | 14 -- src/restic/backend/s3/s3.go | 73 ------ src/restic/backend/sftp/backend_test.go | 14 -- src/restic/backend/sftp/sftp.go | 38 ---- src/restic/backend/test/backend_test.go | 14 -- src/restic/backend/test/tests.go | 207 +----------------- src/restic/mock/backend.go | 10 - 15 files changed, 16 insertions(+), 543 deletions(-) diff --git a/src/restic/archiver/archiver_duplication_test.go b/src/restic/archiver/archiver_duplication_test.go index aeab5585c..43baf011d 100644 --- a/src/restic/archiver/archiver_duplication_test.go +++ b/src/restic/archiver/archiver_duplication_test.go @@ -43,8 +43,8 @@ func forgetfulBackend() restic.Backend { return false, nil } - be.LoadFn = func(h restic.Handle, p []byte, off int64) (int, error) { - return 0, errors.New("not found") + be.GetFn = func(h restic.Handle, length int, offset int64) (io.ReadCloser, error) { + return nil, errors.New("not found") } be.SaveFn = func(h restic.Handle, rd io.Reader) error { diff --git a/src/restic/backend.go b/src/restic/backend.go index af3e5b6dc..ca2eb73d3 100644 --- a/src/restic/backend.go +++ b/src/restic/backend.go @@ -17,18 +17,12 @@ type Backend interface { // Close the backend Close() error - // Load returns the data stored in the backend for h at the given offset - // and saves it in p. Load has the same semantics as io.ReaderAt, except - // that a negative offset is also allowed. In this case it references a - // position relative to the end of the file (similar to Seek()). - Load(h Handle, p []byte, off int64) (int, error) - // Save stores the data in the backend under the given handle. Save(h Handle, rd io.Reader) error // Get returns a reader that yields the contents of the file at h at the - // given offset. If length is nonzero, only a portion of the file is - // returned. rd must be closed after use. If an error is returned, the + // given offset. If length is larger than zero, only a portion of the file + // is returned. rd must be closed after use. If an error is returned, the // ReadCloser must be nil. Get(h Handle, length int, offset int64) (io.ReadCloser, error) diff --git a/src/restic/backend/local/backend_test.go b/src/restic/backend/local/backend_test.go index c018b4947..e7efedb57 100644 --- a/src/restic/backend/local/backend_test.go +++ b/src/restic/backend/local/backend_test.go @@ -44,20 +44,6 @@ func TestLocalBackendConfig(t *testing.T) { test.TestConfig(t) } -func TestLocalBackendLoad(t *testing.T) { - if SkipMessage != "" { - t.Skip(SkipMessage) - } - test.TestLoad(t) -} - -func TestLocalBackendLoadNegativeOffset(t *testing.T) { - if SkipMessage != "" { - t.Skip(SkipMessage) - } - test.TestLoadNegativeOffset(t) -} - func TestLocalBackendGet(t *testing.T) { if SkipMessage != "" { t.Skip(SkipMessage) diff --git a/src/restic/backend/local/local.go b/src/restic/backend/local/local.go index b50587642..4698c8d43 100644 --- a/src/restic/backend/local/local.go +++ b/src/restic/backend/local/local.go @@ -101,42 +101,6 @@ func dirname(base string, t restic.FileType, name string) string { return filepath.Join(base, n) } -// Load returns the data stored in the backend for h at the given offset and -// saves it in p. Load has the same semantics as io.ReaderAt, with one -// exception: when off is lower than zero, it is treated as an offset relative -// to the end of the file. -func (b *Local) Load(h restic.Handle, p []byte, off int64) (n int, err error) { - debug.Log("Load %v, length %v at %v", h, len(p), off) - if err := h.Valid(); err != nil { - return 0, err - } - - f, err := fs.Open(filename(b.p, h.Type, h.Name)) - if err != nil { - return 0, errors.Wrap(err, "Open") - } - - defer func() { - e := f.Close() - if err == nil { - err = errors.Wrap(e, "Close") - } - }() - - switch { - case off > 0: - _, err = f.Seek(off, 0) - case off < 0: - _, err = f.Seek(off, 2) - } - - if err != nil { - return 0, errors.Wrap(err, "Seek") - } - - return io.ReadFull(f, p) -} - // copyToTempfile saves p into a tempfile in tempdir. func copyToTempfile(tempdir string, rd io.Reader) (filename string, err error) { tmpfile, err := ioutil.TempFile(tempdir, "temp-") diff --git a/src/restic/backend/mem/backend_test.go b/src/restic/backend/mem/backend_test.go index 09857b64d..7fda28128 100644 --- a/src/restic/backend/mem/backend_test.go +++ b/src/restic/backend/mem/backend_test.go @@ -44,20 +44,6 @@ func TestMemBackendConfig(t *testing.T) { test.TestConfig(t) } -func TestMemBackendLoad(t *testing.T) { - if SkipMessage != "" { - t.Skip(SkipMessage) - } - test.TestLoad(t) -} - -func TestMemBackendLoadNegativeOffset(t *testing.T) { - if SkipMessage != "" { - t.Skip(SkipMessage) - } - test.TestLoadNegativeOffset(t) -} - func TestMemBackendGet(t *testing.T) { if SkipMessage != "" { t.Skip(SkipMessage) diff --git a/src/restic/backend/mem/mem_backend.go b/src/restic/backend/mem/mem_backend.go index 791753755..ec13e4321 100644 --- a/src/restic/backend/mem/mem_backend.go +++ b/src/restic/backend/mem/mem_backend.go @@ -55,46 +55,6 @@ func (be *MemoryBackend) Test(t restic.FileType, name string) (bool, error) { return false, nil } -// Load reads data from the backend. -func (be *MemoryBackend) Load(h restic.Handle, p []byte, off int64) (int, error) { - if err := h.Valid(); err != nil { - return 0, err - } - - be.m.Lock() - defer be.m.Unlock() - - if h.Type == restic.ConfigFile { - h.Name = "" - } - - debug.Log("get %v offset %v len %v", h, off, len(p)) - - if _, ok := be.data[entry{h.Type, h.Name}]; !ok { - return 0, errors.New("no such data") - } - - buf := be.data[entry{h.Type, h.Name}] - switch { - case off > int64(len(buf)): - return 0, errors.New("offset beyond end of file") - case off < -int64(len(buf)): - off = 0 - case off < 0: - off = int64(len(buf)) + off - } - - buf = buf[off:] - - n := copy(p, buf) - - if len(p) > len(buf) { - return n, io.ErrUnexpectedEOF - } - - return n, nil -} - // Save adds new Data to the backend. func (be *MemoryBackend) Save(h restic.Handle, rd io.Reader) error { if err := h.Valid(); err != nil { diff --git a/src/restic/backend/rest/backend_test.go b/src/restic/backend/rest/backend_test.go index a9beec25f..936076f89 100644 --- a/src/restic/backend/rest/backend_test.go +++ b/src/restic/backend/rest/backend_test.go @@ -44,20 +44,6 @@ func TestRestBackendConfig(t *testing.T) { test.TestConfig(t) } -func TestRestBackendLoad(t *testing.T) { - if SkipMessage != "" { - t.Skip(SkipMessage) - } - test.TestLoad(t) -} - -func TestRestBackendLoadNegativeOffset(t *testing.T) { - if SkipMessage != "" { - t.Skip(SkipMessage) - } - test.TestLoadNegativeOffset(t) -} - func TestRestBackendGet(t *testing.T) { if SkipMessage != "" { t.Skip(SkipMessage) diff --git a/src/restic/backend/rest/rest.go b/src/restic/backend/rest/rest.go index b521b4938..cf28dcad4 100644 --- a/src/restic/backend/rest/rest.go +++ b/src/restic/backend/rest/rest.go @@ -74,63 +74,6 @@ func (b *restBackend) Location() string { return b.url.String() } -// Load returns the data stored in the backend for h at the given offset -// and saves it in p. Load has the same semantics as io.ReaderAt. -func (b *restBackend) Load(h restic.Handle, p []byte, off int64) (n int, err error) { - debug.Log("Load(%v, length %v, offset %v)", h, len(p), off) - if err := h.Valid(); err != nil { - return 0, err - } - - if len(p) == 0 { - return 0, errors.New("buffer length is zero") - } - - // invert offset - if off < 0 { - info, err := b.Stat(h) - if err != nil { - return 0, errors.Wrap(err, "Stat") - } - - if -off > info.Size { - off = 0 - } else { - off = info.Size + off - } - } - - req, err := http.NewRequest("GET", restPath(b.url, h), nil) - if err != nil { - return 0, errors.Wrap(err, "http.NewRequest") - } - debug.Log("Load(%v) send range %d-%d", h, off, off+int64(len(p)-1)) - req.Header.Add("Range", fmt.Sprintf("bytes=%d-%d", off, off+int64(len(p)))) - <-b.connChan - resp, err := b.client.Do(req) - b.connChan <- struct{}{} - - if resp != nil { - defer func() { - io.Copy(ioutil.Discard, resp.Body) - e := resp.Body.Close() - - if err == nil { - err = errors.Wrap(e, "Close") - } - }() - } - - if err != nil { - return 0, errors.Wrap(err, "client.Do") - } - if resp.StatusCode != 200 && resp.StatusCode != 206 { - return 0, errors.Errorf("unexpected HTTP response code %v", resp.StatusCode) - } - - return io.ReadFull(resp.Body, p) -} - // Save stores data in the backend at the handle. func (b *restBackend) Save(h restic.Handle, rd io.Reader) (err error) { if err := h.Valid(); err != nil { diff --git a/src/restic/backend/s3/backend_test.go b/src/restic/backend/s3/backend_test.go index c4709d279..44d8b6377 100644 --- a/src/restic/backend/s3/backend_test.go +++ b/src/restic/backend/s3/backend_test.go @@ -44,20 +44,6 @@ func TestS3BackendConfig(t *testing.T) { test.TestConfig(t) } -func TestS3BackendLoad(t *testing.T) { - if SkipMessage != "" { - t.Skip(SkipMessage) - } - test.TestLoad(t) -} - -func TestS3BackendLoadNegativeOffset(t *testing.T) { - if SkipMessage != "" { - t.Skip(SkipMessage) - } - test.TestLoadNegativeOffset(t) -} - func TestS3BackendGet(t *testing.T) { if SkipMessage != "" { t.Skip(SkipMessage) diff --git a/src/restic/backend/s3/s3.go b/src/restic/backend/s3/s3.go index 0ad1cf91f..b9bd28033 100644 --- a/src/restic/backend/s3/s3.go +++ b/src/restic/backend/s3/s3.go @@ -74,79 +74,6 @@ func (be *s3) Location() string { return be.bucketname } -// Load returns the data stored in the backend for h at the given offset -// and saves it in p. Load has the same semantics as io.ReaderAt. -func (be *s3) Load(h restic.Handle, p []byte, off int64) (n int, err error) { - var obj *minio.Object - - debug.Log("%v, offset %v, len %v", h, off, len(p)) - objName := be.s3path(h.Type, h.Name) - - <-be.connChan - defer func() { - be.connChan <- struct{}{} - }() - - obj, err = be.client.GetObject(be.bucketname, objName) - if err != nil { - debug.Log(" err %v", err) - return 0, errors.Wrap(err, "client.GetObject") - } - - // make sure that the object is closed properly. - defer func() { - e := obj.Close() - if err == nil { - err = errors.Wrap(e, "Close") - } - }() - - info, err := obj.Stat() - if err != nil { - return 0, errors.Wrap(err, "obj.Stat") - } - - // handle negative offsets - if off < 0 { - // if the negative offset is larger than the object itself, read from - // the beginning. - if -off > info.Size { - off = 0 - } else { - // otherwise compute the offset from the end of the file. - off = info.Size + off - } - } - - // return an error if the offset is beyond the end of the file - if off > info.Size { - return 0, errors.Wrap(io.EOF, "") - } - - var nextError error - - // manually create an io.ErrUnexpectedEOF - if off+int64(len(p)) > info.Size { - newlen := info.Size - off - p = p[:newlen] - - nextError = io.ErrUnexpectedEOF - - debug.Log(" capped buffer to %v byte", len(p)) - } - - n, err = obj.ReadAt(p, off) - if int64(n) == info.Size-off && errors.Cause(err) == io.EOF { - err = nil - } - - if err == nil { - err = nextError - } - - return n, err -} - // Save stores data in the backend at the handle. func (be *s3) Save(h restic.Handle, rd io.Reader) (err error) { if err := h.Valid(); err != nil { diff --git a/src/restic/backend/sftp/backend_test.go b/src/restic/backend/sftp/backend_test.go index b066d4966..8ff93b124 100644 --- a/src/restic/backend/sftp/backend_test.go +++ b/src/restic/backend/sftp/backend_test.go @@ -44,20 +44,6 @@ func TestSftpBackendConfig(t *testing.T) { test.TestConfig(t) } -func TestSftpBackendLoad(t *testing.T) { - if SkipMessage != "" { - t.Skip(SkipMessage) - } - test.TestLoad(t) -} - -func TestSftpBackendLoadNegativeOffset(t *testing.T) { - if SkipMessage != "" { - t.Skip(SkipMessage) - } - test.TestLoadNegativeOffset(t) -} - func TestSftpBackendGet(t *testing.T) { if SkipMessage != "" { t.Skip(SkipMessage) diff --git a/src/restic/backend/sftp/sftp.go b/src/restic/backend/sftp/sftp.go index c49126b41..8485ad8e4 100644 --- a/src/restic/backend/sftp/sftp.go +++ b/src/restic/backend/sftp/sftp.go @@ -326,44 +326,6 @@ func (r *SFTP) dirname(t restic.FileType, name string) string { return Join(r.p, n) } -// Load returns the data stored in the backend for h at the given offset -// and saves it in p. Load has the same semantics as io.ReaderAt. -func (r *SFTP) Load(h restic.Handle, p []byte, off int64) (n int, err error) { - debug.Log("load %v, %d bytes, offset %v", h, len(p), off) - if err := r.clientError(); err != nil { - return 0, err - } - - if err := h.Valid(); err != nil { - return 0, err - } - - f, err := r.c.Open(r.filename(h.Type, h.Name)) - if err != nil { - return 0, errors.Wrap(err, "Open") - } - - defer func() { - e := f.Close() - if err == nil { - err = errors.Wrap(e, "Close") - } - }() - - switch { - case off > 0: - _, err = f.Seek(off, 0) - case off < 0: - _, err = f.Seek(off, 2) - } - - if err != nil { - return 0, errors.Wrap(err, "Seek") - } - - return io.ReadFull(f, p) -} - // Save stores data in the backend at the handle. func (r *SFTP) Save(h restic.Handle, rd io.Reader) (err error) { debug.Log("save to %v", h) diff --git a/src/restic/backend/test/backend_test.go b/src/restic/backend/test/backend_test.go index 9239d10a7..6ea08c882 100644 --- a/src/restic/backend/test/backend_test.go +++ b/src/restic/backend/test/backend_test.go @@ -44,20 +44,6 @@ func TestTestBackendConfig(t *testing.T) { test.TestConfig(t) } -func TestTestBackendLoad(t *testing.T) { - if SkipMessage != "" { - t.Skip(SkipMessage) - } - test.TestLoad(t) -} - -func TestTestBackendLoadNegativeOffset(t *testing.T) { - if SkipMessage != "" { - t.Skip(SkipMessage) - } - test.TestLoadNegativeOffset(t) -} - func TestTestBackendGet(t *testing.T) { if SkipMessage != "" { t.Skip(SkipMessage) diff --git a/src/restic/backend/test/tests.go b/src/restic/backend/test/tests.go index 421c382f6..9027c12e4 100644 --- a/src/restic/backend/test/tests.go +++ b/src/restic/backend/test/tests.go @@ -12,7 +12,6 @@ import ( "strings" "testing" - "restic/errors" "restic/test" "restic/backend" @@ -179,197 +178,6 @@ func TestConfig(t testing.TB) { } } -// TestLoad tests the backend's Load function. -func TestLoad(t testing.TB) { - b := open(t) - defer close(t) - - _, err := b.Load(restic.Handle{}, nil, 0) - if err == nil { - t.Fatalf("Load() did not return an error for invalid handle") - } - - _, err = b.Load(restic.Handle{Type: restic.DataFile, Name: "foobar"}, nil, 0) - if err == nil { - t.Fatalf("Load() did not return an error for non-existing blob") - } - - length := rand.Intn(1<<24) + 2000 - - data := test.Random(23, length) - id := restic.Hash(data) - - handle := restic.Handle{Type: restic.DataFile, Name: id.String()} - err = b.Save(handle, bytes.NewReader(data)) - if err != nil { - t.Fatalf("Save() error: %v", err) - } - - for i := 0; i < 50; i++ { - l := rand.Intn(length + 2000) - o := rand.Intn(length + 2000) - - d := data - if o < len(d) { - d = d[o:] - } else { - o = len(d) - d = d[:0] - } - - if l > 0 && l < len(d) { - d = d[:l] - } - - buf := make([]byte, l) - n, err := b.Load(handle, buf, int64(o)) - - // if we requested data beyond the end of the file, require - // ErrUnexpectedEOF error - if l > len(d) { - if errors.Cause(err) != io.ErrUnexpectedEOF { - t.Errorf("Load(%d, %d) did not return io.ErrUnexpectedEOF", len(buf), int64(o)) - } - err = nil - buf = buf[:len(d)] - } - - if err != nil { - t.Errorf("Load(%d, %d): unexpected error: %v", len(buf), int64(o), err) - continue - } - - if n != len(buf) { - t.Errorf("Load(%d, %d): wrong length returned, want %d, got %d", - len(buf), int64(o), len(buf), n) - continue - } - - buf = buf[:n] - if !bytes.Equal(buf, d) { - t.Errorf("Load(%d, %d) returned wrong bytes", len(buf), int64(o)) - continue - } - } - - // test with negative offset - for i := 0; i < 50; i++ { - l := rand.Intn(length + 2000) - o := rand.Intn(length + 2000) - - d := data - if o < len(d) { - d = d[len(d)-o:] - } else { - o = 0 - } - - if l > 0 && l < len(d) { - d = d[:l] - } - - buf := make([]byte, l) - n, err := b.Load(handle, buf, -int64(o)) - - // if we requested data beyond the end of the file, require - // ErrUnexpectedEOF error - if l > len(d) { - if errors.Cause(err) != io.ErrUnexpectedEOF { - t.Errorf("Load(%d, %d) did not return io.ErrUnexpectedEOF", len(buf), int64(o)) - continue - } - err = nil - buf = buf[:len(d)] - } - - if err != nil { - t.Errorf("Load(%d, %d): unexpected error: %v", len(buf), int64(o), err) - continue - } - - if n != len(buf) { - t.Errorf("Load(%d, %d): wrong length returned, want %d, got %d", - len(buf), int64(o), len(buf), n) - continue - } - - buf = buf[:n] - if !bytes.Equal(buf, d) { - t.Errorf("Load(%d, %d) returned wrong bytes", len(buf), int64(o)) - continue - } - } - - // load with a too-large buffer, this should return io.ErrUnexpectedEOF - buf := make([]byte, length+100) - n, err := b.Load(handle, buf, 0) - if n != length { - t.Errorf("wrong length for larger buffer returned, want %d, got %d", length, n) - } - - if errors.Cause(err) != io.ErrUnexpectedEOF { - t.Errorf("wrong error returned for larger buffer: want io.ErrUnexpectedEOF, got %#v", err) - } - - test.OK(t, b.Remove(restic.DataFile, id.String())) -} - -// TestLoadNegativeOffset tests the backend's Load function with negative offsets. -func TestLoadNegativeOffset(t testing.TB) { - b := open(t) - defer close(t) - - length := rand.Intn(1<<24) + 2000 - - data := test.Random(23, length) - id := restic.Hash(data) - - handle := restic.Handle{Type: restic.DataFile, Name: id.String()} - err := b.Save(handle, bytes.NewReader(data)) - if err != nil { - t.Fatalf("Save() error: %v", err) - } - - // test normal reads - for i := 0; i < 50; i++ { - l := rand.Intn(length + 2000) - o := -rand.Intn(length + 2000) - - buf := make([]byte, l) - n, err := b.Load(handle, buf, int64(o)) - - // if we requested data beyond the end of the file, require - // ErrUnexpectedEOF error - if len(buf) > -o { - if errors.Cause(err) != io.ErrUnexpectedEOF { - t.Errorf("Load(%d, %d) did not return io.ErrUnexpectedEOF", len(buf), o) - continue - } - err = nil - buf = buf[:-o] - } - - if err != nil { - t.Errorf("Load(%d, %d) returned error: %v", len(buf), o, err) - continue - } - - if n != len(buf) { - t.Errorf("Load(%d, %d) returned short read, only got %d bytes", len(buf), o, n) - continue - } - - p := len(data) + o - if !bytes.Equal(buf, data[p:p+len(buf)]) { - t.Errorf("Load(%d, %d) returned wrong bytes", len(buf), o) - continue - } - - } - - test.OK(t, b.Remove(restic.DataFile, id.String())) -} - // TestGet tests the backend's Get function. func TestGet(t testing.TB) { b := open(t) @@ -590,7 +398,7 @@ func TestBackend(t testing.TB) { test.Assert(t, err != nil, "blob data could be extracted before creation") // try to read not existing blob - _, err = b.Load(h, nil, 0) + _, err = b.Get(h, 0, 0) test.Assert(t, err != nil, "blob reader could be obtained before creation") // try to get string out, should fail @@ -615,9 +423,18 @@ func TestBackend(t testing.TB) { length := end - start buf2 := make([]byte, length) - n, err := b.Load(h, buf2, int64(start)) + rd, err := b.Get(h, len(buf2), int64(start)) test.OK(t, err) - test.Equals(t, length, n) + n, err := io.ReadFull(rd, buf2) + test.OK(t, err) + test.Equals(t, len(buf2), n) + + remaining, err := io.Copy(ioutil.Discard, rd) + test.OK(t, err) + test.Equals(t, int64(0), remaining) + + test.OK(t, rd.Close()) + test.Equals(t, ts.data[start:end], string(buf2)) } diff --git a/src/restic/mock/backend.go b/src/restic/mock/backend.go index b2a390661..fbbc9e658 100644 --- a/src/restic/mock/backend.go +++ b/src/restic/mock/backend.go @@ -10,7 +10,6 @@ import ( // Backend implements a mock backend. type Backend struct { CloseFn func() error - LoadFn func(h restic.Handle, p []byte, off int64) (int, error) SaveFn func(h restic.Handle, rd io.Reader) error GetFn func(h restic.Handle, length int, offset int64) (io.ReadCloser, error) StatFn func(h restic.Handle) (restic.FileInfo, error) @@ -39,15 +38,6 @@ func (m *Backend) Location() string { return m.LocationFn() } -// Load loads data from the backend. -func (m *Backend) Load(h restic.Handle, p []byte, off int64) (int, error) { - if m.LoadFn == nil { - return 0, errors.New("not implemented") - } - - return m.LoadFn(h, p, off) -} - // Save data in the backend. func (m *Backend) Save(h restic.Handle, rd io.Reader) error { if m.SaveFn == nil {