Removed ReverseList from KeyValueStore interfaces (#1306)

Removed ReverseList from KeyValueStore interfaces

pkg/kademlia/routing_helpers.go

@@ -299,18 +299,20 @@ func (rt *RoutingTable) getUnmarshaledNodesFromBucket(bID bucketID) ([]*pb.Node,
 
 // getKBucketRange: helper, returns the left and right endpoints of the range of node ids contained within the bucket
 func (rt *RoutingTable) getKBucketRange(bID bucketID) ([]bucketID, error) {
-	kadIDs, err := rt.kadBucketDB.ReverseList(bID[:], 2)
+	kadBucketIDs, err := rt.kadBucketDB.List(nil, 0)
 	if err != nil {
-		return nil, RoutingErr.New("could not reverse list k bucket ids %s", err)
+		return nil, RoutingErr.New("could not list all k bucket ids: %s", err)
 	}
-	coords := make([]bucketID, 2)
-	if len(kadIDs) < 2 {
-		coords[0] = bucketID{}
-	} else {
-		copy(coords[0][:], kadIDs[1])
+	previousBucket := bucketID{}
+	for _, k := range kadBucketIDs {
+		thisBucket := keyToBucketID(k)
+		if thisBucket == bID {
+			return []bucketID{previousBucket, bID}, nil
+		}
+		previousBucket = thisBucket
 	}
-	copy(coords[1][:], kadIDs[0])
-	return coords, nil
+	// shouldn't happen BUT return error if no matching kbucket...
+	return nil, RoutingErr.New("could not find k bucket")
 }
 
 // determineLeafDepth determines the level of the bucket id in question.

pkg/storj/node.go

@@ -21,8 +21,11 @@ var (
 	ErrNodeID = errs.Class("node ID error")
 )
 
+//NodeIDSize is the byte length of a NodeID
+const NodeIDSize = sha256.Size
+
 // NodeID is a unique node identifier
-type NodeID [sha256.Size]byte
+type NodeID [NodeIDSize]byte
 
 // NodeIDList is a slice of NodeIDs (implements sort)
 type NodeIDList []NodeID

storage/boltdb/client.go

@@ -155,13 +155,6 @@ func (client *Client) List(first storage.Key, limit int) (storage.Keys, error) {
 	return rv, Error.Wrap(err)
 }
 
-// ReverseList returns either a list of keys for which boltdb has values or an error.
-// Starts from first and iterates backwards
-func (client *Client) ReverseList(first storage.Key, limit int) (storage.Keys, error) {
-	rv, err := storage.ReverseListKeys(client, first, limit)
-	return rv, Error.Wrap(err)
-}
-
 // Close closes a BoltDB client
 func (client *Client) Close() error {
 	if atomic.AddInt32(client.referenceCount, -1) == 0 {

storage/common.go

@@ -62,8 +62,6 @@ type KeyValueStore interface {
 	Delete(Key) error
 	// List lists all keys starting from start and upto limit items
 	List(start Key, limit int) (Keys, error)
-	// ReverseList lists all keys in revers order
-	ReverseList(Key, int) (Keys, error)
 	// Iterate iterates over items based on opts
 	Iterate(opts IterateOptions, fn func(Iterator) error) error
 	// Close closes the store

storage/postgreskv/client.go

@@ -115,12 +115,6 @@ func (client *Client) List(first storage.Key, limit int) (storage.Keys, error) {
 	return storage.ListKeys(client, first, limit)
 }
 
-// ReverseList returns either a list of known keys, in reverse order, or an error.
-// Starts from first and iterates backwards
-func (client *Client) ReverseList(first storage.Key, limit int) (storage.Keys, error) {
-	return storage.ReverseListKeys(client, first, limit)
-}
-
 // Close closes the client
 func (client *Client) Close() error {
 	return client.pgConn.Close()

storage/redis/client.go

@@ -104,12 +104,6 @@ func (client *Client) List(first storage.Key, limit int) (storage.Keys, error) {
 	return storage.ListKeys(client, first, limit)
 }
 
-// ReverseList returns either a list of keys for which redis has values or an error.
-// Starts from first and iterates backwards
-func (client *Client) ReverseList(first storage.Key, limit int) (storage.Keys, error) {
-	return storage.ReverseListKeys(client, first, limit)
-}
-
 // Delete deletes a key/value pair from redis, for a given the key
 func (client *Client) Delete(key storage.Key) error {
 	if key.IsZero() {

storage/storelogger/logger.go

@@ -58,13 +58,6 @@ func (store *Logger) List(first storage.Key, limit int) (storage.Keys, error) {
 	return keys, err
 }
 
-// ReverseList lists all keys in reverse order, starting from first
-func (store *Logger) ReverseList(first storage.Key, limit int) (storage.Keys, error) {
-	keys, err := store.store.ReverseList(first, limit)
-	store.log.Debug("ReverseList", zap.String("first", string(first)), zap.Int("limit", limit), zap.Any("keys", keys.Strings()))
-	return keys, err
-}
-
 // Iterate iterates over items based on opts
 func (store *Logger) Iterate(opts storage.IterateOptions, fn func(storage.Iterator) error) error {
 	store.log.Debug("Iterate",

storage/teststore/store.go

@@ -178,18 +178,6 @@ func (store *Client) List(first storage.Key, limit int) (storage.Keys, error) {
 	return storage.ListKeys(store, first, limit)
 }
 
-// ReverseList lists all keys in revers order
-func (store *Client) ReverseList(first storage.Key, limit int) (storage.Keys, error) {
-	store.mu.Lock()
-	store.CallCount.ReverseList++
-	if store.forcedError() {
-		store.mu.Unlock()
-		return nil, errors.New("internal error")
-	}
-	store.mu.Unlock()
-	return storage.ReverseListKeys(store, first, limit)
-}
-
 // Close closes the store
 func (store *Client) Close() error {
 	defer store.locked()()

storage/testsuite/test.go

@@ -70,18 +70,9 @@ func testConstraints(t *testing.T, store storage.KeyValueStore) {
 		if err != nil || len(keys) != storage.LookupLimit {
 			t.Fatalf("List LookupLimit should succeed: %v / got %d", err, len(keys))
 		}
-		keys, err = store.ReverseList(nil, storage.LookupLimit)
-		if err != nil || len(keys) != storage.LookupLimit {
-			t.Fatalf("ReverseList LookupLimit should succeed: %v / got %d", err, len(keys))
-		}
-
 		_, err = store.List(nil, storage.LookupLimit+1)
 		if err != nil || len(keys) != storage.LookupLimit {
 			t.Fatalf("List LookupLimit+1 shouldn't fail: %v / got %d", err, len(keys))
 		}
-		_, err = store.ReverseList(nil, storage.LookupLimit+1)
-		if err != nil || len(keys) != storage.LookupLimit {
-			t.Fatalf("ReverseList LookupLimit+1 shouldn't fail: %v / got %d", err, len(keys))
-		}
 	})
 }

storage/testsuite/test_list.go

@@ -31,7 +31,6 @@ func testList(t *testing.T, store storage.KeyValueStore) {
 
 	type Test struct {
 		Name     string
-		Reverse  bool
 		First    storage.Key
 		Limit    int
 		Expected storage.Keys
@@ -46,37 +45,24 @@ func testList(t *testing.T, store storage.KeyValueStore) {
 	}
 
 	tests := []Test{
-		{"without key", false,
+		{"without key",
 			nil, 3,
 			newKeys("path/0", "path/1", "path/2")},
-		{"without key, limit 0", false,
+		{"without key, limit 0",
 			nil, 0,
 			newKeys("path/0", "path/1", "path/2", "path/3", "path/4", "path/5")},
-		{"with key", false,
+		{"with key",
 			storage.Key("path/2"), 3,
 			newKeys("path/2", "path/3", "path/4")},
-		{"without key 100", false,
+		{"without key 100",
 			nil, 100,
 			newKeys("path/0", "path/1", "path/2", "path/3", "path/4", "path/5")},
-		{"reverse without key", true,
-			nil, 3,
-			newKeys("path/5", "path/4", "path/3")},
-		{"reverse with key", true,
-			storage.Key("path/2"), 3,
-			newKeys("path/2", "path/1", "path/0")},
-		{"reverse without key 100", true,
-			nil, 100,
-			newKeys("path/5", "path/4", "path/3", "path/2", "path/1", "path/0")},
 	}
 
 	for _, test := range tests {
 		var keys storage.Keys
 		var err error
-		if !test.Reverse {
-			keys, err = store.List(test.First, test.Limit)
-		} else {
-			keys, err = store.ReverseList(test.First, test.Limit)
-		}
+		keys, err = store.List(test.First, test.Limit)
 		if err != nil {
 			t.Errorf("%s: %s", test.Name, err)
 			continue