diff --git a/pkg/metainfo/kvmetainfo/objects.go b/pkg/metainfo/kvmetainfo/objects.go index 06dc17b4e..a3a7f8f6d 100644 --- a/pkg/metainfo/kvmetainfo/objects.go +++ b/pkg/metainfo/kvmetainfo/objects.go @@ -20,6 +20,7 @@ import ( "storj.io/storj/pkg/storage/segments" "storj.io/storj/pkg/storage/streams" "storj.io/storj/pkg/storj" + "storj.io/storj/storage" ) const ( @@ -65,12 +66,17 @@ func (db *DB) ModifyObject(ctx context.Context, bucket string, path storj.Path) // DeleteObject deletes an object from database func (db *DB) DeleteObject(ctx context.Context, bucket string, path storj.Path) error { - objects, err := db.buckets.GetObjectStore(ctx, bucket) + store, err := db.buckets.GetObjectStore(ctx, bucket) if err != nil { return err } - return objects.Delete(ctx, path) + err = store.Delete(ctx, path) + if objects.NoPathError.Has(err) { + return storage.ErrEmptyKey.Wrap(err) + } + + return err } // ModifyPendingObject creates an interface for updating a partially uploaded object @@ -146,6 +152,10 @@ func (db *DB) getInfo(ctx context.Context, prefix string, bucket string, path st return object{}, storj.Object{}, err } + if path == "" { + return object{}, storj.Object{}, storage.ErrEmptyKey.New("") + } + fullpath := bucket + "/" + path encryptedPath, err := streams.EncryptAfterBucket(fullpath, bucketInfo.PathCipher, db.rootKey) diff --git a/pkg/metainfo/kvmetainfo/objects_test.go b/pkg/metainfo/kvmetainfo/objects_test.go new file mode 100644 index 000000000..c97587926 --- /dev/null +++ b/pkg/metainfo/kvmetainfo/objects_test.go @@ -0,0 +1,580 @@ +// Copyright (C) 2018 Storj Labs, Inc. +// See LICENSE for copying information. + +package kvmetainfo + +import ( + "bytes" + "context" + "fmt" + "testing" + "time" + + "github.com/stretchr/testify/assert" + + "storj.io/storj/pkg/storage/objects" + "storj.io/storj/pkg/storj" + "storj.io/storj/storage" +) + +func TestGetObject(t *testing.T) { + runTest(t, func(ctx context.Context, db *DB) { + bucket, err := db.CreateBucket(ctx, TestBucket, nil) + if !assert.NoError(t, err) { + return + } + + store, err := db.buckets.GetObjectStore(ctx, bucket.Name) + if !assert.NoError(t, err) { + return + } + + var exp time.Time + _, err = store.Put(ctx, "test-file", bytes.NewReader(nil), objects.SerializableMeta{}, exp) + if !assert.NoError(t, err) { + return + } + + _, err = db.GetObject(ctx, "", "") + assert.True(t, storj.ErrNoBucket.Has(err)) + + _, err = db.GetObject(ctx, bucket.Name, "") + assert.True(t, storage.ErrEmptyKey.Has(err)) + + _, err = db.GetObject(ctx, "non-existing-bucket", "test-file") + // TODO: Should return storj.ErrBucketNotFound + assert.True(t, storage.ErrKeyNotFound.Has(err)) + + _, err = db.GetObject(ctx, bucket.Name, "non-existing-file") + assert.True(t, storage.ErrKeyNotFound.Has(err)) + + object, err := db.GetObject(ctx, bucket.Name, "test-file") + if assert.NoError(t, err) { + assert.Equal(t, "test-file", object.Path) + } + }) +} + +func TestGetObjectStream(t *testing.T) { + runTest(t, func(ctx context.Context, db *DB) { + bucket, err := db.CreateBucket(ctx, TestBucket, nil) + if !assert.NoError(t, err) { + return + } + + store, err := db.buckets.GetObjectStore(ctx, bucket.Name) + if !assert.NoError(t, err) { + return + } + + var exp time.Time + _, err = store.Put(ctx, "empty-file", bytes.NewReader(nil), objects.SerializableMeta{}, exp) + if !assert.NoError(t, err) { + return + } + + _, err = store.Put(ctx, "test-file", bytes.NewReader([]byte("test")), objects.SerializableMeta{}, exp) + if !assert.NoError(t, err) { + return + } + + _, err = db.GetObjectStream(ctx, "", "") + assert.True(t, storj.ErrNoBucket.Has(err)) + + _, err = db.GetObjectStream(ctx, bucket.Name, "") + assert.True(t, storage.ErrEmptyKey.Has(err)) + + _, err = db.GetObjectStream(ctx, "non-existing-bucket", "test-file") + // TODO: Should return storj.ErrBucketNotFound + assert.True(t, storage.ErrKeyNotFound.Has(err)) + + _, err = db.GetObject(ctx, bucket.Name, "non-existing-file") + assert.True(t, storage.ErrKeyNotFound.Has(err)) + + stream, err := db.GetObjectStream(ctx, bucket.Name, "empty-file") + if assert.NoError(t, err) { + assertStream(ctx, t, stream, "empty-file", []byte(nil)) + } + + stream, err = db.GetObjectStream(ctx, bucket.Name, "test-file") + if assert.NoError(t, err) { + assertStream(ctx, t, stream, "test-file", []byte("test")) + } + }) +} + +func assertStream(ctx context.Context, t *testing.T, stream storj.ReadOnlyStream, path storj.Path, content []byte) bool { + assert.Equal(t, path, stream.Info().Path) + + segments, more, err := stream.Segments(ctx, 0, 0) + if !assert.NoError(t, err) { + return false + } + + assert.False(t, more) + if !assert.Equal(t, 1, len(segments)) { + return false + + } + assert.EqualValues(t, 0, segments[0].Index) + assert.EqualValues(t, len(content), segments[0].Size) + + // TODO: Currently Inline is always empty + // assert.Equal(t, content, segments[0].Inline) + + return true +} + +func TestDeleteObject(t *testing.T) { + runTest(t, func(ctx context.Context, db *DB) { + bucket, err := db.CreateBucket(ctx, TestBucket, nil) + if !assert.NoError(t, err) { + return + } + + store, err := db.buckets.GetObjectStore(ctx, bucket.Name) + if !assert.NoError(t, err) { + return + } + + var exp time.Time + _, err = store.Put(ctx, "test-file", bytes.NewReader(nil), objects.SerializableMeta{}, exp) + if !assert.NoError(t, err) { + return + } + + err = db.DeleteObject(ctx, "", "") + assert.True(t, storj.ErrNoBucket.Has(err)) + + err = db.DeleteObject(ctx, bucket.Name, "") + assert.True(t, storage.ErrEmptyKey.Has(err)) + + _ = db.DeleteObject(ctx, "non-existing-bucket", "test-file") + // TODO: Currently returns minio.BucketNotFound, should return storj.ErrBucketNotFound + // assert.True(t, storj.ErrBucketNotFound.Has(err)) + + err = db.DeleteObject(ctx, bucket.Name, "non-existing-file") + assert.True(t, storage.ErrKeyNotFound.Has(err)) + + err = db.DeleteObject(ctx, bucket.Name, "test-file") + assert.NoError(t, err) + }) +} + +func TestListObjectsEmpty(t *testing.T) { + runTest(t, func(ctx context.Context, db *DB) { + bucket, err := db.CreateBucket(ctx, TestBucket, nil) + if !assert.NoError(t, err) { + return + } + + _, err = db.ListObjects(ctx, "", storj.ListOptions{}) + assert.True(t, storj.ErrNoBucket.Has(err)) + + _, err = db.ListObjects(ctx, bucket.Name, storj.ListOptions{}) + assert.EqualError(t, err, "kvmetainfo: invalid direction 0") + + for _, direction := range []storj.ListDirection{ + storj.Before, + storj.Backward, + storj.Forward, + storj.After, + } { + list, err := db.ListObjects(ctx, bucket.Name, storj.ListOptions{Direction: direction}) + if assert.NoError(t, err) { + assert.False(t, list.More) + assert.Equal(t, 0, len(list.Items)) + } + } + }) +} + +func TestListObjects(t *testing.T) { + runTest(t, func(ctx context.Context, db *DB) { + var exp time.Time + bucket, err := db.CreateBucket(ctx, TestBucket, &storj.Bucket{PathCipher: storj.Unencrypted}) + if !assert.NoError(t, err) { + return + } + + store, err := db.buckets.GetObjectStore(ctx, bucket.Name) + if !assert.NoError(t, err) { + return + } + + filePaths := []string{ + "a", "aa", "b", "bb", "c", + "a/xa", "a/xaa", "a/xb", "a/xbb", "a/xc", + "b/ya", "b/yaa", "b/yb", "b/ybb", "b/yc", + } + for _, path := range filePaths { + _, err = store.Put(ctx, path, bytes.NewReader(nil), objects.SerializableMeta{}, exp) + if !assert.NoError(t, err) { + return + } + } + + otherBucket, err := db.CreateBucket(ctx, "otherbucket", nil) + if !assert.NoError(t, err) { + return + } + + otherStore, err := db.buckets.GetObjectStore(ctx, otherBucket.Name) + if !assert.NoError(t, err) { + return + } + + _, err = otherStore.Put(ctx, "file-in-other-bucket", bytes.NewReader(nil), objects.SerializableMeta{}, exp) + if !assert.NoError(t, err) { + return + } + + for i, tt := range []struct { + options storj.ListOptions + more bool + result []string + }{ + { + options: options("", "", storj.After, 0), + result: []string{"a", "a/", "aa", "b", "b/", "bb", "c"}, + }, { + options: options("", "`", storj.After, 0), + result: []string{"a", "a/", "aa", "b", "b/", "bb", "c"}, + }, { + options: options("", "b", storj.After, 0), + result: []string{"b/", "bb", "c"}, + }, { + options: options("", "c", storj.After, 0), + result: []string{}, + }, { + options: options("", "ca", storj.After, 0), + result: []string{}, + }, { + options: options("", "", storj.After, 1), + more: true, + result: []string{"a"}, + }, { + options: options("", "`", storj.After, 1), + more: true, + result: []string{"a"}, + }, { + options: options("", "aa", storj.After, 1), + more: true, + result: []string{"b"}, + }, { + options: options("", "c", storj.After, 1), + result: []string{}, + }, { + options: options("", "ca", storj.After, 1), + result: []string{}, + }, { + options: options("", "", storj.After, 2), + more: true, + result: []string{"a", "a/"}, + }, { + options: options("", "1", storj.After, 2), + more: true, + result: []string{"a", "a/"}, + }, { + options: options("", "aa", storj.After, 2), + more: true, + result: []string{"b", "b/"}, + }, { + options: options("", "bb", storj.After, 2), + result: []string{"c"}, + }, { + options: options("", "c", storj.After, 2), + result: []string{}, + }, { + options: options("", "ca", storj.After, 2), + result: []string{}, + }, { + options: optionsRecursive("", "", storj.After, 0), + result: []string{"a", "a/xa", "a/xaa", "a/xb", "a/xbb", "a/xc", "aa", "b", "b/ya", "b/yaa", "b/yb", "b/ybb", "b/yc", "bb", "c"}, + }, { + options: options("a", "", storj.After, 0), + result: []string{"xa", "xaa", "xb", "xbb", "xc"}, + }, { + options: options("a/", "", storj.After, 0), + result: []string{"xa", "xaa", "xb", "xbb", "xc"}, + }, { + options: options("a/", "xb", storj.After, 0), + result: []string{"xbb", "xc"}, + }, { + options: optionsRecursive("", "a/xbb", storj.After, 5), + more: true, + result: []string{"a/xc", "aa", "b", "b/ya", "b/yaa"}, + }, { + options: options("a/", "xaa", storj.After, 2), + more: true, + result: []string{"xb", "xbb"}, + }, { + options: options("", "", storj.Forward, 0), + result: []string{"a", "a/", "aa", "b", "b/", "bb", "c"}, + }, { + options: options("", "`", storj.Forward, 0), + result: []string{"a", "a/", "aa", "b", "b/", "bb", "c"}, + }, { + options: options("", "b", storj.Forward, 0), + result: []string{"b", "b/", "bb", "c"}, + }, { + options: options("", "c", storj.Forward, 0), + result: []string{"c"}, + }, { + options: options("", "ca", storj.Forward, 0), + result: []string{}, + }, { + options: options("", "", storj.Forward, 1), + more: true, + result: []string{"a"}, + }, { + options: options("", "`", storj.Forward, 1), + more: true, + result: []string{"a"}, + }, { + options: options("", "aa", storj.Forward, 1), + more: true, + result: []string{"aa"}, + }, { + options: options("", "c", storj.Forward, 1), + result: []string{"c"}, + }, { + options: options("", "ca", storj.Forward, 1), + result: []string{}, + }, { + options: options("", "", storj.Forward, 2), + more: true, + result: []string{"a", "a/"}, + }, { + options: options("", "`", storj.Forward, 2), + more: true, + result: []string{"a", "a/"}, + }, { + options: options("", "aa", storj.Forward, 2), + more: true, + result: []string{"aa", "b"}, + }, { + options: options("", "bb", storj.Forward, 2), + result: []string{"bb", "c"}, + }, { + options: options("", "c", storj.Forward, 2), + result: []string{"c"}, + }, { + options: options("", "ca", storj.Forward, 2), + result: []string{}, + }, { + options: optionsRecursive("", "", storj.Forward, 0), + result: []string{"a", "a/xa", "a/xaa", "a/xb", "a/xbb", "a/xc", "aa", "b", "b/ya", "b/yaa", "b/yb", "b/ybb", "b/yc", "bb", "c"}, + }, { + options: options("a", "", storj.Forward, 0), + result: []string{"xa", "xaa", "xb", "xbb", "xc"}, + }, { + options: options("a/", "", storj.Forward, 0), + result: []string{"xa", "xaa", "xb", "xbb", "xc"}, + }, { + options: options("a/", "xb", storj.Forward, 0), + result: []string{"xb", "xbb", "xc"}, + }, { + options: optionsRecursive("", "a/xbb", storj.Forward, 5), + more: true, + result: []string{"a/xbb", "a/xc", "aa", "b", "b/ya"}, + }, { + options: options("a/", "xaa", storj.Forward, 2), + more: true, + result: []string{"xaa", "xb"}, + }, { + options: options("", "", storj.Backward, 0), + result: []string{"a", "a/", "aa", "b", "b/", "bb", "c"}, + }, { + options: options("", "1", storj.Backward, 0), + result: []string{}, + }, { + options: options("", "b", storj.Backward, 0), + result: []string{"a", "a/", "aa", "b"}, + }, { + options: options("", "c", storj.Backward, 0), + result: []string{"a", "a/", "aa", "b", "b/", "bb", "c"}, + }, { + options: options("", "ca", storj.Backward, 0), + result: []string{"a", "a/", "aa", "b", "b/", "bb", "c"}, + }, { + options: options("", "", storj.Backward, 1), + more: true, + result: []string{"c"}, + }, { + options: options("", "1", storj.Backward, 1), + result: []string{}, + }, { + options: options("", "aa", storj.Backward, 1), + more: true, + result: []string{"aa"}, + }, { + options: options("", "c", storj.Backward, 1), + more: true, + result: []string{"c"}, + }, { + options: options("", "ca", storj.Backward, 1), + more: true, + result: []string{"c"}, + }, { + options: options("", "", storj.Backward, 2), + more: true, + result: []string{"bb", "c"}, + }, { + options: options("", "`", storj.Backward, 2), + result: []string{}, + }, { + options: options("", "a/", storj.Backward, 2), + result: []string{"a"}, + }, { + options: options("", "bb", storj.Backward, 2), + more: true, + result: []string{"b/", "bb"}, + }, { + options: options("", "c", storj.Backward, 2), + more: true, + result: []string{"bb", "c"}, + }, { + options: options("", "ca", storj.Backward, 2), + more: true, + result: []string{"bb", "c"}, + }, { + options: optionsRecursive("", "", storj.Backward, 0), + result: []string{"a", "a/xa", "a/xaa", "a/xb", "a/xbb", "a/xc", "aa", "b", "b/ya", "b/yaa", "b/yb", "b/ybb", "b/yc", "bb", "c"}, + }, { + options: options("a", "", storj.Backward, 0), + result: []string{"xa", "xaa", "xb", "xbb", "xc"}, + }, { + options: options("a/", "", storj.Backward, 0), + result: []string{"xa", "xaa", "xb", "xbb", "xc"}, + }, { + options: options("a/", "xb", storj.Backward, 0), + result: []string{"xa", "xaa", "xb"}, + }, { + options: optionsRecursive("", "b/yaa", storj.Backward, 5), + more: true, + result: []string{"a/xc", "aa", "b", "b/ya", "b/yaa"}, + }, { + options: options("a/", "xbb", storj.Backward, 2), + more: true, + result: []string{"xb", "xbb"}, + }, { + options: options("", "", storj.Before, 0), + result: []string{"a", "a/", "aa", "b", "b/", "bb", "c"}, + }, { + options: options("", "`", storj.Before, 0), + result: []string{}, + }, { + options: options("", "a", storj.Before, 0), + result: []string{}, + }, { + options: options("", "b", storj.Before, 0), + result: []string{"a", "a/", "aa"}, + }, { + options: options("", "c", storj.Before, 0), + result: []string{"a", "a/", "aa", "b", "b/", "bb"}, + }, { + options: options("", "ca", storj.Before, 0), + result: []string{"a", "a/", "aa", "b", "b/", "bb", "c"}, + }, { + options: options("", "", storj.Before, 1), + more: true, + result: []string{"c"}, + }, { + options: options("", "`", storj.Before, 1), + result: []string{}, + }, { + options: options("", "a/", storj.Before, 1), + result: []string{"a"}, + }, { + options: options("", "c", storj.Before, 1), + more: true, + result: []string{"bb"}, + }, { + options: options("", "ca", storj.Before, 1), + more: true, + result: []string{"c"}, + }, { + options: options("", "", storj.Before, 2), + more: true, + result: []string{"bb", "c"}, + }, { + options: options("", "`", storj.Before, 2), + result: []string{}, + }, { + options: options("", "a/", storj.Before, 2), + result: []string{"a"}, + }, { + options: options("", "bb", storj.Before, 2), + more: true, + result: []string{"b", "b/"}, + }, { + options: options("", "c", storj.Before, 2), + more: true, + result: []string{"b/", "bb"}, + }, { + options: options("", "ca", storj.Before, 2), + more: true, + result: []string{"bb", "c"}, + }, { + options: optionsRecursive("", "", storj.Before, 0), + result: []string{"a", "a/xa", "a/xaa", "a/xb", "a/xbb", "a/xc", "aa", "b", "b/ya", "b/yaa", "b/yb", "b/ybb", "b/yc", "bb", "c"}, + }, { + options: options("a", "", storj.Before, 0), + result: []string{"xa", "xaa", "xb", "xbb", "xc"}, + }, { + options: options("a/", "", storj.Before, 0), + result: []string{"xa", "xaa", "xb", "xbb", "xc"}, + }, { + options: options("a/", "xb", storj.Before, 0), + result: []string{"xa", "xaa"}, + }, { + options: optionsRecursive("", "b/yaa", storj.Before, 5), + more: true, + result: []string{"a/xbb", "a/xc", "aa", "b", "b/ya"}, + }, { + options: options("a/", "xbb", storj.Before, 2), + more: true, + result: []string{"xaa", "xb"}, + }, + } { + errTag := fmt.Sprintf("%d. %+v", i, tt) + + list, err := db.ListObjects(ctx, bucket.Name, tt.options) + + if assert.NoError(t, err, errTag) { + assert.Equal(t, tt.more, list.More, errTag) + assert.Equal(t, tt.result, getObjectPaths(list), errTag) + } + } + }) +} + +func options(prefix, cursor string, direction storj.ListDirection, limit int) storj.ListOptions { + return storj.ListOptions{ + Prefix: prefix, + Cursor: cursor, + Direction: direction, + Limit: limit, + } +} + +func optionsRecursive(prefix, cursor string, direction storj.ListDirection, limit int) storj.ListOptions { + return storj.ListOptions{ + Prefix: prefix, + Cursor: cursor, + Direction: direction, + Limit: limit, + Recursive: true, + } +} + +func getObjectPaths(list storj.ObjectList) []string { + names := make([]string, len(list.Items)) + + for i, item := range list.Items { + names[i] = item.Path + } + + return names +} diff --git a/pkg/metainfo/kvmetainfo/stream.go b/pkg/metainfo/kvmetainfo/stream.go index e0283a706..d6fd31c4f 100644 --- a/pkg/metainfo/kvmetainfo/stream.go +++ b/pkg/metainfo/kvmetainfo/stream.go @@ -93,6 +93,6 @@ func (stream *readonlyStream) Segments(ctx context.Context, index int64, limit i infos = append(infos, segment) } - more = index+limit >= stream.info.SegmentCount + more = index < stream.info.SegmentCount return infos, more, nil } diff --git a/storage/boltdb/client.go b/storage/boltdb/client.go index bd08ef2dd..926c2a6c4 100644 --- a/storage/boltdb/client.go +++ b/storage/boltdb/client.go @@ -113,9 +113,10 @@ func (client *Client) view(fn func(*bolt.Bucket) error) error { // Put adds a value to the provided key in boltdb, returning an error on failure. func (client *Client) Put(key storage.Key, value storage.Value) error { - if len(key) == 0 { - return Error.New("invalid key") + if key.IsZero() { + return storage.ErrEmptyKey.New("") } + return client.update(func(bucket *bolt.Bucket) error { return bucket.Put(key, value) }) @@ -123,6 +124,10 @@ func (client *Client) Put(key storage.Key, value storage.Value) error { // Get looks up the provided key from boltdb returning either an error or the result. func (client *Client) Get(key storage.Key) (storage.Value, error) { + if key.IsZero() { + return nil, storage.ErrEmptyKey.New("") + } + var value storage.Value err := client.view(func(bucket *bolt.Bucket) error { data := bucket.Get([]byte(key)) @@ -137,6 +142,10 @@ func (client *Client) Get(key storage.Key) (storage.Value, error) { // Delete deletes a key/value pair from boltdb, for a given the key func (client *Client) Delete(key storage.Key) error { + if key.IsZero() { + return storage.ErrEmptyKey.New("") + } + return client.update(func(bucket *bolt.Bucket) error { return bucket.Delete(key) }) diff --git a/storage/common.go b/storage/common.go index e6edccb18..b106215bd 100644 --- a/storage/common.go +++ b/storage/common.go @@ -17,7 +17,7 @@ const Delimiter = '/' var ErrKeyNotFound = errs.Class("key not found") // ErrEmptyKey is returned when an empty key is used in Put -var ErrEmptyKey = errors.New("empty key") +var ErrEmptyKey = errs.Class("empty key") // ErrEmptyQueue is returned when attempting to Dequeue from an empty queue var ErrEmptyQueue = errors.New("empty queue") diff --git a/storage/postgreskv/client.go b/storage/postgreskv/client.go index 20238ff81..4ccbcd5fb 100644 --- a/storage/postgreskv/client.go +++ b/storage/postgreskv/client.go @@ -50,7 +50,7 @@ func (client *Client) Put(key storage.Key, value storage.Value) error { // PutPath sets the value for the provided key (in the given bucket). func (client *Client) PutPath(bucket, key storage.Key, value storage.Value) error { if key.IsZero() { - return Error.New("invalid key") + return storage.ErrEmptyKey.New("") } q := ` INSERT INTO pathdata (bucket, fullpath, metadata) @@ -68,6 +68,10 @@ func (client *Client) Get(key storage.Key) (storage.Value, error) { // GetPath looks up the provided key (in the given bucket) and returns its value (or an error). func (client *Client) GetPath(bucket, key storage.Key) (storage.Value, error) { + if key.IsZero() { + return nil, storage.ErrEmptyKey.New("") + } + q := "SELECT metadata FROM pathdata WHERE bucket = $1::BYTEA AND fullpath = $2::BYTEA" row := client.pgConn.QueryRow(q, []byte(bucket), []byte(key)) var val []byte @@ -88,6 +92,10 @@ func (client *Client) Delete(key storage.Key) error { // DeletePath deletes the given key (in the given bucket) and its associated value. func (client *Client) DeletePath(bucket, key storage.Key) error { + if key.IsZero() { + return storage.ErrEmptyKey.New("") + } + q := "DELETE FROM pathdata WHERE bucket = $1::BYTEA AND fullpath = $2::BYTEA" result, err := client.pgConn.Exec(q, []byte(bucket), []byte(key)) if err != nil { diff --git a/storage/redis/client.go b/storage/redis/client.go index 804f9b959..155279d4f 100644 --- a/storage/redis/client.go +++ b/storage/redis/client.go @@ -10,6 +10,7 @@ import ( "github.com/go-redis/redis" "github.com/zeebo/errs" + "storj.io/storj/pkg/utils" "storj.io/storj/storage" ) @@ -69,6 +70,10 @@ func NewClientFrom(address string) (*Client, error) { // Get looks up the provided key from redis returning either an error or the result. func (client *Client) Get(key storage.Key) (storage.Value, error) { + if key.IsZero() { + return nil, storage.ErrEmptyKey.New("") + } + value, err := client.db.Get(string(key)).Bytes() if err == redis.Nil { return nil, storage.ErrKeyNotFound.New(key.String()) @@ -81,9 +86,10 @@ func (client *Client) Get(key storage.Key) (storage.Value, error) { // Put adds a value to the provided key in redis, returning an error on failure. func (client *Client) Put(key storage.Key, value storage.Value) error { - if len(key) == 0 { - return Error.New("invalid key") + if key.IsZero() { + return storage.ErrEmptyKey.New("") } + err := client.db.Set(key.String(), []byte(value), client.TTL).Err() if err != nil { return Error.New("put error: %v", err) @@ -104,6 +110,10 @@ func (client *Client) ReverseList(first storage.Key, limit int) (storage.Keys, e // Delete deletes a key/value pair from redis, for a given the key func (client *Client) Delete(key storage.Key) error { + if key.IsZero() { + return storage.ErrEmptyKey.New("") + } + err := client.db.Del(key.String()).Err() if err != nil { return Error.New("delete error: %v", err) diff --git a/storage/teststore/store.go b/storage/teststore/store.go index 0cbbe592a..43cb7ed10 100644 --- a/storage/teststore/store.go +++ b/storage/teststore/store.go @@ -64,7 +64,7 @@ func (store *Client) Put(key storage.Key, value storage.Value) error { } if key.IsZero() { - return storage.ErrEmptyKey + return storage.ErrEmptyKey.New("") } keyIndex, found := store.indexOf(key) @@ -92,6 +92,10 @@ func (store *Client) Get(key storage.Key) (storage.Value, error) { return nil, errors.New("internal error") } + if key.IsZero() { + return nil, storage.ErrEmptyKey.New("") + } + keyIndex, found := store.indexOf(key) if !found { return nil, storage.ErrKeyNotFound.New(key.String()) @@ -132,6 +136,10 @@ func (store *Client) Delete(key storage.Key) error { return errInternal } + if key.IsZero() { + return storage.ErrEmptyKey.New("") + } + keyIndex, found := store.indexOf(key) if !found { return storage.ErrKeyNotFound.New(key.String())