Improve swift backend

run_integration_tests.go

@@ -164,6 +164,13 @@ func (env *TravisEnvironment) RunTests() error {
 		msg("S3 repository not available\n")
 	}
 
+	// if the test swift service is available, make sure that the test is not skipped
+	if os.Getenv("RESTIC_TEST_SWIFT") != "" {
+		ensureTests = append(ensureTests, "restic/backend/swift.TestBackendSwift")
+	} else {
+		msg("Swift service not available\n")
+	}
+
 	env.env["RESTIC_TEST_DISALLOW_SKIP"] = strings.Join(ensureTests, ",")
 
 	if *runCrossCompile {

src/cmds/restic/global.go

@@ -360,39 +360,8 @@ func parseConfig(loc location.Location, opts options.Options) (interface{}, erro
 	case "swift":
 		cfg := loc.Config.(swift.Config)
 
-		for _, val := range []struct {
+		if err := swift.ApplyEnvironment("", &cfg); err != nil {
-			s   *string
+			return nil, err
-			env string
-		}{
-			// v2/v3 specific
-			{&cfg.UserName, "OS_USERNAME"},
-			{&cfg.APIKey, "OS_PASSWORD"},
-			{&cfg.Region, "OS_REGION_NAME"},
-			{&cfg.AuthURL, "OS_AUTH_URL"},
-
-			// v3 specific
-			{&cfg.Domain, "OS_USER_DOMAIN_NAME"},
-			{&cfg.Tenant, "OS_PROJECT_NAME"},
-			{&cfg.TenantDomain, "OS_PROJECT_DOMAIN_NAME"},
-
-			// v2 specific
-			{&cfg.TenantID, "OS_TENANT_ID"},
-			{&cfg.Tenant, "OS_TENANT_NAME"},
-
-			// v1 specific
-			{&cfg.AuthURL, "ST_AUTH"},
-			{&cfg.UserName, "ST_USER"},
-			{&cfg.APIKey, "ST_KEY"},
-
-			// Manual authentication
-			{&cfg.StorageURL, "OS_STORAGE_URL"},
-			{&cfg.AuthToken, "OS_AUTH_TOKEN"},
-
-			{&cfg.DefaultContainerPolicy, "SWIFT_DEFAULT_CONTAINER_POLICY"},
-		} {
-			if *val.s == "" {
-				*val.s = os.Getenv(val.env)
-			}
 		}
 
 		if err := opts.Apply(loc.Scheme, &cfg); err != nil {

src/restic/backend/swift/backend_test.go

@@ -1,87 +0,0 @@
-// DO NOT EDIT, AUTOMATICALLY GENERATED
-package swift_test
-
-import (
-	"testing"
-
-	"restic/backend/test"
-)
-
-var SkipMessage string
-
-func TestSwiftBackendCreate(t *testing.T) {
-	if SkipMessage != "" {
-		t.Skip(SkipMessage)
-	}
-	test.TestCreate(t)
-}
-
-func TestSwiftBackendOpen(t *testing.T) {
-	if SkipMessage != "" {
-		t.Skip(SkipMessage)
-	}
-	test.TestOpen(t)
-}
-
-func TestSwiftBackendCreateWithConfig(t *testing.T) {
-	if SkipMessage != "" {
-		t.Skip(SkipMessage)
-	}
-	test.TestCreateWithConfig(t)
-}
-
-func TestSwiftBackendLocation(t *testing.T) {
-	if SkipMessage != "" {
-		t.Skip(SkipMessage)
-	}
-	test.TestLocation(t)
-}
-
-func TestSwiftBackendConfig(t *testing.T) {
-	if SkipMessage != "" {
-		t.Skip(SkipMessage)
-	}
-	test.TestConfig(t)
-}
-
-func TestSwiftBackendLoad(t *testing.T) {
-	if SkipMessage != "" {
-		t.Skip(SkipMessage)
-	}
-	test.TestLoad(t)
-}
-
-func TestSwiftBackendSave(t *testing.T) {
-	if SkipMessage != "" {
-		t.Skip(SkipMessage)
-	}
-	test.TestSave(t)
-}
-
-func TestSwiftBackendSaveFilenames(t *testing.T) {
-	if SkipMessage != "" {
-		t.Skip(SkipMessage)
-	}
-	test.TestSaveFilenames(t)
-}
-
-func TestSwiftBackendBackend(t *testing.T) {
-	if SkipMessage != "" {
-		t.Skip(SkipMessage)
-	}
-	test.TestBackend(t)
-}
-
-func TestSwiftBackendDelete(t *testing.T) {
-	if SkipMessage != "" {
-		t.Skip(SkipMessage)
-	}
-	test.TestDelete(t)
-}
-
-func TestSwiftBackendCleanup(t *testing.T) {
-	if SkipMessage != "" {
-		t.Skip(SkipMessage)
-	}
-	test.TestCleanup(t)
-}

src/restic/backend/swift/config.go

@@ -1,13 +1,9 @@
 package swift
 
 import (
-	"net/url"
+	"os"
-	"regexp"
 	"restic/errors"
-)
+	"strings"
-
-var (
-	urlParser = regexp.MustCompile("^([^:]+):/(.*)$")
 )
 
 // Config contains basic configuration needed to specify swift location for a swift server
@@ -32,21 +28,69 @@ type Config struct {
 
 // ParseConfig parses the string s and extract swift's container name and prefix.
 func ParseConfig(s string) (interface{}, error) {
-
+	data := strings.SplitN(s, ":", 3)
-	url, err := url.Parse(s)
+	if len(data) != 3 {
-	if err != nil {
+		return nil, errors.New("invalid URL, expected: swift:container-name:/[prefix]")
-		return nil, errors.Wrap(err, "url.Parse")
 	}
 
-	m := urlParser.FindStringSubmatch(url.Opaque)
+	scheme, container, prefix := data[0], data[1], data[2]
-	if len(m) == 0 {
+	if scheme != "swift" {
-		return nil, errors.New("swift: invalid URL, valid syntax is: 'swift:container-name:/[optional-prefix]'")
+		return nil, errors.Errorf("unexpected prefix: %s", data[0])
 	}
 
+	if len(prefix) == 0 {
+		return nil, errors.Errorf("prefix is empty")
+	}
+
+	if prefix[0] != '/' {
+		return nil, errors.Errorf("prefix does not start with slash (/)")
+	}
+	prefix = prefix[1:]
+
 	cfg := Config{
-		Container: m[1],
+		Container: container,
-		Prefix:    m[2],
+		Prefix:    prefix,
 	}
 
 	return cfg, nil
 }
+
+// ApplyEnvironment saves values from the environment to the config.
+func ApplyEnvironment(prefix string, cfg interface{}) error {
+	c := cfg.(*Config)
+	for _, val := range []struct {
+		s   *string
+		env string
+	}{
+		// v2/v3 specific
+		{&c.UserName, prefix + "OS_USERNAME"},
+		{&c.APIKey, prefix + "OS_PASSWORD"},
+		{&c.Region, prefix + "OS_REGION_NAME"},
+		{&c.AuthURL, prefix + "OS_AUTH_URL"},
+
+		// v3 specific
+		{&c.Domain, prefix + "OS_USER_DOMAIN_NAME"},
+		{&c.Tenant, prefix + "OS_PROJECT_NAME"},
+		{&c.TenantDomain, prefix + "OS_PROJECT_DOMAIN_NAME"},
+
+		// v2 specific
+		{&c.TenantID, prefix + "OS_TENANT_ID"},
+		{&c.Tenant, prefix + "OS_TENANT_NAME"},
+
+		// v1 specific
+		{&c.AuthURL, prefix + "ST_AUTH"},
+		{&c.UserName, prefix + "ST_USER"},
+		{&c.APIKey, prefix + "ST_KEY"},
+
+		// Manual authentication
+		{&c.StorageURL, prefix + "OS_STORAGE_URL"},
+		{&c.AuthToken, prefix + "OS_AUTH_TOKEN"},
+
+		{&c.DefaultContainerPolicy, prefix + "SWIFT_DEFAULT_CONTAINER_POLICY"},
+	} {
+		if *val.s == "" {
+			*val.s = os.Getenv(val.env)
+		}
+	}
+	return nil
+}

src/restic/backend/swift/config_test.go

@@ -9,23 +9,26 @@ var configTests = []struct {
 	{"swift:cnt1:/", Config{Container: "cnt1", Prefix: ""}},
 	{"swift:cnt2:/prefix", Config{Container: "cnt2", Prefix: "prefix"}},
 	{"swift:cnt3:/prefix/longer", Config{Container: "cnt3", Prefix: "prefix/longer"}},
-	{"swift:cnt4:/prefix?params", Config{Container: "cnt4", Prefix: "prefix"}},
-	{"swift:cnt5:/prefix#params", Config{Container: "cnt5", Prefix: "prefix"}},
 }
 
-func TestParseConfigInternal(t *testing.T) {
+func TestParseConfig(t *testing.T) {
-	for i, test := range configTests {
+	for _, test := range configTests {
-		cfg, err := ParseConfig(test.s)
+		t.Run("", func(t *testing.T) {
+			v, err := ParseConfig(test.s)
 			if err != nil {
-			t.Errorf("test %d:%s failed: %v", i, test.s, err)
+				t.Fatalf("parsing %q failed: %v", test.s, err)
-			continue
+			}
+
+			cfg, ok := v.(Config)
+			if !ok {
+				t.Fatalf("wrong type returned, want Config, got %T", cfg)
 			}
 
 			if cfg != test.cfg {
-			t.Errorf("test %d:\ninput:\n  %s\n wrong config, want:\n  %v\ngot:\n  %v",
+				t.Fatalf("wrong output for %q, want:\n  %#v\ngot:\n  %#v",
-				i, test.s, test.cfg, cfg)
+					test.s, test.cfg, cfg)
-			continue
 			}
+		})
 	}
 }
 

src/restic/backend/swift/swift.go

@@ -1,8 +1,11 @@
 package swift
 
 import (
+	"fmt"
 	"io"
+	"net/http"
 	"path"
+	"path/filepath"
 	"restic"
 	"restic/backend"
 	"restic/debug"
@@ -21,11 +24,13 @@ type beSwift struct {
 	connChan  chan struct{}
 	container string // Container name
 	prefix    string // Prefix of object names in the container
+	backend.Layout
 }
 
 // Open opens the swift backend at a container in region. The container is
 // created if it does not exist yet.
 func Open(cfg Config) (restic.Backend, error) {
+	debug.Log("config %#v", cfg)
 
 	be := &beSwift{
 		conn: &swift.Connection{
@@ -42,9 +47,15 @@ func Open(cfg Config) (restic.Backend, error) {
 			AuthToken:      cfg.AuthToken,
 			ConnectTimeout: time.Minute,
 			Timeout:        time.Minute,
+
+			Transport: backend.Transport(),
 		},
 		container: cfg.Container,
 		prefix:    cfg.Prefix,
+		Layout: &backend.DefaultLayout{
+			Path: cfg.Prefix,
+			Join: path.Join,
+		},
 	}
 	be.createConnections()
 
@@ -70,34 +81,19 @@ func Open(cfg Config) (restic.Backend, error) {
 		return nil, errors.Wrap(err, "conn.Container")
 	}
 
+	// check that the server supports byte ranges
+	_, hdr, err := be.conn.Account()
+	if err != nil {
+		return nil, errors.Wrap(err, "Account()")
+	}
+
+	if hdr["Accept-Ranges"] != "bytes" {
+		return nil, errors.New("backend does not support byte range")
+	}
+
 	return be, nil
 }
 
-func (be *beSwift) swiftpath(h restic.Handle) string {
-
-	var dir string
-
-	switch h.Type {
-	case restic.ConfigFile:
-		dir = ""
-		h.Name = backend.Paths.Config
-	case restic.DataFile:
-		dir = backend.Paths.Data
-	case restic.SnapshotFile:
-		dir = backend.Paths.Snapshots
-	case restic.IndexFile:
-		dir = backend.Paths.Index
-	case restic.LockFile:
-		dir = backend.Paths.Locks
-	case restic.KeyFile:
-		dir = backend.Paths.Keys
-	default:
-		dir = string(h.Type)
-	}
-
-	return path.Join(be.prefix, dir, h.Name)
-}
-
 func (be *beSwift) createConnections() {
 	be.connChan = make(chan struct{}, connLimit)
 	for i := 0; i < connLimit; i++ {
@@ -138,60 +134,44 @@ func (be *beSwift) Load(h restic.Handle, length int, offset int64) (io.ReadClose
 		return nil, errors.Errorf("invalid length %d", length)
 	}
 
-	objName := be.swiftpath(h)
+	objName := be.Filename(h)
 
 	<-be.connChan
 	defer func() {
 		be.connChan <- struct{}{}
 	}()
 
-	obj, _, err := be.conn.ObjectOpen(be.container, objName, false, nil)
+	headers := swift.Headers{}
+	if offset > 0 {
+		headers["Range"] = fmt.Sprintf("bytes=%d-", offset)
+	}
+
+	if length > 0 {
+		headers["Range"] = fmt.Sprintf("bytes=%d-%d", offset, offset+int64(length)-1)
+	}
+
+	if _, ok := headers["Range"]; ok {
+		debug.Log("Load(%v) send range %v", h, headers["Range"])
+	}
+
+	obj, _, err := be.conn.ObjectOpen(be.container, objName, false, headers)
 	if err != nil {
 		debug.Log("  err %v", err)
 		return nil, errors.Wrap(err, "conn.ObjectOpen")
 	}
 
-	// if we're going to read the whole object, just pass it on.
-	if length == 0 {
-		debug.Log("Load %v: pass on object", h)
-		_, err = obj.Seek(offset, 0)
-		if err != nil {
-			_ = obj.Close()
-			return nil, errors.Wrap(err, "obj.Seek")
-		}
-
 	return obj, nil
 }
 
-	// otherwise pass a LimitReader
-	size, err := obj.Length()
-	if err != nil {
-		return nil, errors.Wrap(err, "obj.Length")
-	}
-
-	if offset > size {
-		_ = obj.Close()
-		return nil, errors.Errorf("offset larger than file size")
-	}
-
-	_, err = obj.Seek(offset, 0)
-	if err != nil {
-		_ = obj.Close()
-		return nil, errors.Wrap(err, "obj.Seek")
-	}
-
-	return backend.LimitReadCloser(obj, int64(length)), nil
-}
-
 // Save stores data in the backend at the handle.
 func (be *beSwift) Save(h restic.Handle, rd io.Reader) (err error) {
 	if err = h.Valid(); err != nil {
 		return err
 	}
 
-	debug.Log("Save %v", h)
+	objName := be.Filename(h)
 
-	objName := be.swiftpath(h)
+	debug.Log("Save %v at %v", h, objName)
 
 	// Check key does not already exist
 	switch _, _, err = be.conn.Object(be.container, objName); err {
@@ -213,9 +193,7 @@ func (be *beSwift) Save(h restic.Handle, rd io.Reader) (err error) {
 
 	encoding := "binary/octet-stream"
 
-	debug.Log("PutObject(%v, %v, %v)",
+	debug.Log("PutObject(%v, %v, %v)", be.container, objName, encoding)
-		be.container, objName, encoding)
-	//err = be.conn.ObjectPutBytes(be.container, objName, p, encoding)
 	_, err = be.conn.ObjectPut(be.container, objName, rd, true, "", encoding, nil)
 	debug.Log("%v, err %#v", objName, err)
 
@@ -226,7 +204,7 @@ func (be *beSwift) Save(h restic.Handle, rd io.Reader) (err error) {
 func (be *beSwift) Stat(h restic.Handle) (bi restic.FileInfo, err error) {
 	debug.Log("%v", h)
 
-	objName := be.swiftpath(h)
+	objName := be.Filename(h)
 
 	obj, _, err := be.conn.Object(be.container, objName)
 	if err != nil {
@@ -239,7 +217,7 @@ func (be *beSwift) Stat(h restic.Handle) (bi restic.FileInfo, err error) {
 
 // Test returns true if a blob of the given type and name exists in the backend.
 func (be *beSwift) Test(h restic.Handle) (bool, error) {
-	objName := be.swiftpath(h)
+	objName := be.Filename(h)
 	switch _, _, err := be.conn.Object(be.container, objName); err {
 	case nil:
 		return true, nil
@@ -254,7 +232,7 @@ func (be *beSwift) Test(h restic.Handle) (bool, error) {
 
 // Remove removes the blob with the given name and type.
 func (be *beSwift) Remove(h restic.Handle) error {
-	objName := be.swiftpath(h)
+	objName := be.Filename(h)
 	err := be.conn.ObjectDelete(be.container, objName)
 	debug.Log("Remove(%v) -> err %v", h, err)
 	return errors.Wrap(err, "conn.ObjectDelete")
@@ -267,19 +245,19 @@ func (be *beSwift) List(t restic.FileType, done <-chan struct{}) <-chan string {
 	debug.Log("listing %v", t)
 	ch := make(chan string)
 
-	prefix := be.swiftpath(restic.Handle{Type: t}) + "/"
+	prefix := be.Filename(restic.Handle{Type: t}) + "/"
 
 	go func() {
 		defer close(ch)
 
-		be.conn.ObjectsWalk(be.container, &swift.ObjectsOpts{Prefix: prefix},
+		err := be.conn.ObjectsWalk(be.container, &swift.ObjectsOpts{Prefix: prefix},
 			func(opts *swift.ObjectsOpts) (interface{}, error) {
 				newObjects, err := be.conn.ObjectNames(be.container, opts)
 				if err != nil {
 					return nil, errors.Wrap(err, "conn.ObjectNames")
 				}
 				for _, obj := range newObjects {
-					m := strings.TrimPrefix(obj, prefix)
+					m := filepath.Base(strings.TrimPrefix(obj, prefix))
 					if m == "" {
 						continue
 					}
@@ -292,6 +270,10 @@ func (be *beSwift) List(t restic.FileType, done <-chan struct{}) <-chan string {
 				}
 				return newObjects, nil
 			})
+
+		if err != nil {
+			debug.Log("ObjectsWalk returned error: %v", err)
+		}
 	}()
 
 	return ch
@@ -301,8 +283,8 @@ func (be *beSwift) List(t restic.FileType, done <-chan struct{}) <-chan string {
 func (be *beSwift) removeKeys(t restic.FileType) error {
 	done := make(chan struct{})
 	defer close(done)
-	for key := range be.List(restic.DataFile, done) {
+	for key := range be.List(t, done) {
-		err := be.Remove(restic.Handle{Type: restic.DataFile, Name: key})
+		err := be.Remove(restic.Handle{Type: t, Name: key})
 		if err != nil {
 			return err
 		}
@@ -311,6 +293,15 @@ func (be *beSwift) removeKeys(t restic.FileType) error {
 	return nil
 }
 
+// IsNotExist returns true if the error is caused by a not existing file.
+func (be *beSwift) IsNotExist(err error) bool {
+	if e, ok := errors.Cause(err).(*swift.Error); ok {
+		return e.StatusCode == http.StatusNotFound
+	}
+
+	return false
+}
+
 // Delete removes all restic objects in the container.
 // It will not remove the container itself.
 func (be *beSwift) Delete() error {
@@ -328,7 +319,12 @@ func (be *beSwift) Delete() error {
 		}
 	}
 
-	return be.Remove(restic.Handle{Type: restic.ConfigFile})
+	err := be.Remove(restic.Handle{Type: restic.ConfigFile})
+	if err != nil && !be.IsNotExist(err) {
+		return err
+	}
+
+	return nil
 }
 
 // Close does nothing

src/restic/backend/swift/swift_test.go

@@ -2,42 +2,43 @@ package swift_test
 
 import (
 	"fmt"
-	"math/rand"
+	"os"
 	"restic"
+	"testing"
 	"time"
 
 	"restic/errors"
+	. "restic/test"
 
 	"restic/backend/swift"
 	"restic/backend/test"
-	. "restic/test"
-
-	swiftclient "github.com/ncw/swift"
 )
 
-//go:generate go run ../test/generate_backend_tests.go
+func newSwiftTestSuite(t testing.TB) *test.Suite {
+	return &test.Suite{
+		// do not use excessive data
+		MinimalData: true,
 
-func init() {
+		// NewConfig returns a config for a new temporary backend that will be used in tests.
-	if TestSwiftServer == "" {
+		NewConfig: func() (interface{}, error) {
-		SkipMessage = "swift test server not available"
+			swiftcfg, err := swift.ParseConfig(os.Getenv("RESTIC_TEST_SWIFT"))
-		return
+			if err != nil {
+				return nil, err
 			}
 
-	// Generate random container name to allow simultaneous test
+			cfg := swiftcfg.(swift.Config)
-	// on the same swift backend
+			if err = swift.ApplyEnvironment("RESTIC_TEST_", &cfg); err != nil {
-	containerName := fmt.Sprintf(
+				return nil, err
-		"restictestcontainer_%d_%d",
-		time.Now().Unix(),
-		rand.Uint32(),
-	)
-
-	cfg := swift.Config{
-		Container:  containerName,
-		StorageURL: TestSwiftServer,
-		AuthToken:  TestSwiftToken,
 			}
+			cfg.Prefix += fmt.Sprintf("/test-%d", time.Now().UnixNano())
+			t.Logf("using prefix %v", cfg.Prefix)
+			return cfg, nil
+		},
+
+		// CreateFn is a function that creates a temporary repository for the tests.
+		Create: func(config interface{}) (restic.Backend, error) {
+			cfg := config.(swift.Config)
 
-	test.CreateFn = func() (restic.Backend, error) {
 			be, err := swift.Open(cfg)
 			if err != nil {
 				return nil, err
@@ -53,24 +54,54 @@ func init() {
 			}
 
 			return be, nil
-	}
+		},
 
-	test.OpenFn = func() (restic.Backend, error) {
+		// OpenFn is a function that opens a previously created temporary repository.
+		Open: func(config interface{}) (restic.Backend, error) {
+			cfg := config.(swift.Config)
 			return swift.Open(cfg)
-	}
+		},
 
-	test.CleanupFn = func() error {
+		// CleanupFn removes data created during the tests.
-		client := swiftclient.Connection{
+		Cleanup: func(config interface{}) error {
-			StorageUrl: TestSwiftServer,
+			cfg := config.(swift.Config)
-			AuthToken:  TestSwiftToken,
+
-		}
+			be, err := swift.Open(cfg)
-		objects, err := client.ObjectsAll(containerName, nil)
 			if err != nil {
 				return err
 			}
-		for _, o := range objects {
+
-			client.ObjectDelete(containerName, o.Name)
+			if err := be.(restic.Deleter).Delete(); err != nil {
+				return err
 			}
-		return client.ContainerDelete(containerName)
+
+			return nil
+		},
 	}
 }
+
+func TestBackendSwift(t *testing.T) {
+	defer func() {
+		if t.Skipped() {
+			SkipDisallowed(t, "restic/backend/swift.TestBackendSwift")
+		}
+	}()
+
+	if os.Getenv("RESTIC_TEST_SWIFT") == "" {
+		t.Skip("RESTIC_TEST_SWIFT unset, skipping test")
+		return
+	}
+
+	t.Logf("run tests")
+	newSwiftTestSuite(t).RunTests(t)
+}
+
+func BenchmarkBackendSwift(t *testing.B) {
+	if os.Getenv("RESTIC_TEST_SWIFT") == "" {
+		t.Skip("RESTIC_TEST_SWIFT unset, skipping test")
+		return
+	}
+
+	t.Logf("run tests")
+	newSwiftTestSuite(t).RunBenchmarks(t)
+}

src/restic/backend/test/tests.go

@@ -438,8 +438,12 @@ func delayedRemove(b restic.Backend, h restic.Handle) error {
 	// Some backend (swift, I'm looking at you) may implement delayed
 	// removal of data. Let's wait a bit if this happens.
 	err := b.Remove(h)
+	if err != nil {
+		return err
+	}
+
 	found, err := b.Test(h)
-	for i := 0; found && i < 10; i++ {
+	for i := 0; found && i < 20; i++ {
 		found, err = b.Test(h)
 		if found {
 			time.Sleep(100 * time.Millisecond)

src/restic/test/vars.go

@@ -18,8 +18,6 @@ var (
 	BenchArchiveDirectory       = getStringVar("RESTIC_BENCH_DIR", ".")
 	TestS3Server                = getStringVar("RESTIC_TEST_S3_SERVER", "")
 	TestRESTServer              = getStringVar("RESTIC_TEST_REST_SERVER", "")
-	TestSwiftServer             = getStringVar("RESTIC_TEST_SWIFT_SERVER", "")
-	TestSwiftToken              = getStringVar("RESTIC_TEST_SWIFT_TOKEN", "")
 	TestIntegrationDisallowSkip = getStringVar("RESTIC_TEST_DISALLOW_SKIP", "")
 )