add MoveSubBucket test

Signed-off-by: Mustafa Elbehery <[email protected]>

bucket.go

@@ -297,7 +297,8 @@ func (b *Bucket) DeleteBucket(key []byte) error {
 // Returns an error if
 //  1. the sub-bucket cannot be found in the source bucket;
 //  2. or the key already exists in the destination bucket;
-//  3. the key represents a non-bucket value.
+//  3. or the key represents a non-bucket value;
+//  4. the source and destination buckets are the same.
 func (b *Bucket) MoveBucket(key []byte, dstBucket *Bucket) error {
 	if b.tx.db == nil || dstBucket.tx.db == nil {
 		return errors.ErrTxClosed
@@ -319,7 +320,7 @@ func (b *Bucket) MoveBucket(key []byte, dstBucket *Bucket) error {
 	// Do nothing (return true directly) if the source bucket and the
 	// destination bucket are actually the same bucket.
 	if b == dstBucket || (b.RootPage() == dstBucket.RootPage() && b.RootPage() != 0) {
-		return nil
+		return fmt.Errorf("source bucket %s and target bucket %s are the same: %w", b.String(), dstBucket.String(), errors.ErrSameBuckets)
 	}
 
 	// check whether the key already exists in the destination bucket

errors/errors.go

@@ -69,8 +69,12 @@ var (
 	// ErrValueTooLarge is returned when inserting a value that is larger than MaxValueSize.
 	ErrValueTooLarge = errors.New("value too large")
 
-	// ErrIncompatibleValue is returned when trying create or delete a bucket
+	// ErrIncompatibleValue is returned when trying to create or delete a bucket
 	// on an existing non-bucket key or when trying to create or delete a
 	// non-bucket key on an existing bucket key.
 	ErrIncompatibleValue = errors.New("incompatible value")
+
+	// ErrSameBuckets is returned when trying to move a sub-bucket between
+	// source and target buckets, while source and target buckets are the same.
+	ErrSameBuckets = errors.New("the source and target are the same bucket")
 )

movebucket_test.go

@@ -0,0 +1,290 @@
+package bbolt_test
+
+import (
+	"bytes"
+	crand "crypto/rand"
+	"math/rand"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"go.etcd.io/bbolt"
+	"go.etcd.io/bbolt/errors"
+	"go.etcd.io/bbolt/internal/btesting"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestTx_MoveBucket(t *testing.T) {
+	testCases := []struct {
+		name                 string
+		srcBucketPath        []string
+		dstBucketPath        []string
+		bucketToMove         string
+		incompatibleKeyInSrc bool
+		incompatibleKeyInDst bool
+		expActErr            error
+	}{
+		{
+			name:                 "happy path",
+			srcBucketPath:        []string{"rootBucketX", "parentBucketY", "myBucketZ"},
+			dstBucketPath:        []string{"rootBucketA", "parentBucketB"},
+			bucketToMove:         "myBucketZ",
+			incompatibleKeyInSrc: false,
+			incompatibleKeyInDst: false,
+			expActErr:            nil,
+		},
+		{
+			name:                 "bucketToMove not exist in srcBucket",
+			srcBucketPath:        []string{"rootBucketX", "parentBucketY"},
+			dstBucketPath:        []string{"rootBucketA", "parentBucketB"},
+			bucketToMove:         "myBucketZ",
+			incompatibleKeyInSrc: false,
+			incompatibleKeyInDst: false,
+			expActErr:            errors.ErrBucketNotFound,
+		},
+		{
+			name:                 "bucketToMove exist in dstBucket",
+			srcBucketPath:        []string{"rootBucketX", "parentBucketY", "myBucketZ"},
+			dstBucketPath:        []string{"rootBucketA", "parentBucketB", "myBucketZ"},
+			bucketToMove:         "myBucketZ",
+			incompatibleKeyInSrc: false,
+			incompatibleKeyInDst: false,
+			expActErr:            errors.ErrBucketExists,
+		},
+		{
+			name:                 "bucketToMove key exist in srcBucket, but no subBucket value",
+			srcBucketPath:        []string{"rootBucketX", "parentBucketY"},
+			dstBucketPath:        []string{"rootBucketA", "parentBucketB"},
+			bucketToMove:         "myBucketZ",
+			incompatibleKeyInSrc: true,
+			incompatibleKeyInDst: false,
+			expActErr:            errors.ErrIncompatibleValue,
+		},
+		{
+			name:                 "bucketToMove key exist in dstBucket, but no subBucket value",
+			srcBucketPath:        []string{"rootBucketX", "parentBucketY", "myBucketZ"},
+			dstBucketPath:        []string{"rootBucketA", "parentBucketB"},
+			bucketToMove:         "myBucketZ",
+			incompatibleKeyInSrc: false,
+			incompatibleKeyInDst: true,
+			expActErr:            errors.ErrIncompatibleValue,
+		},
+		{
+			name:                 "srcBucket is rootBucket",
+			srcBucketPath:        []string{"", "myBucketZ"},
+			dstBucketPath:        []string{"rootBucketA", "parentBucketB"},
+			bucketToMove:         "myBucketZ",
+			incompatibleKeyInSrc: false,
+			incompatibleKeyInDst: false,
+			expActErr:            nil,
+		},
+		{
+			name:                 "dstBucket is rootBucket",
+			srcBucketPath:        []string{"rootBucketX", "parentBucketY", "myBucketZ"},
+			dstBucketPath:        []string{""},
+			bucketToMove:         "myBucketZ",
+			incompatibleKeyInSrc: false,
+			incompatibleKeyInDst: false,
+			expActErr:            nil,
+		},
+		{
+			name:                 "srcBucket is rootBucket, and dstBucket is rootBucket",
+			srcBucketPath:        []string{"", "myBucketZ"},
+			dstBucketPath:        []string{""},
+			bucketToMove:         "myBucketZ",
+			incompatibleKeyInSrc: false,
+			incompatibleKeyInDst: false,
+			expActErr:            errors.ErrSameBuckets,
+		},
+	}
+
+	for _, tc := range testCases {
+
+		t.Run(tc.name, func(*testing.T) {
+			db := btesting.MustCreateDBWithOption(t, &bbolt.Options{PageSize: pageSize})
+
+			dumpBucketBeforeMoving := filepath.Join(t.TempDir(), "beforeBucket.db")
+			dumpBucketAfterMoving := filepath.Join(t.TempDir(), "afterBucket.db")
+
+			// arrange
+			if err := db.Update(func(tx *bbolt.Tx) error {
+				srcBucket := createBucketIfNotExist(t, tx, tc.incompatibleKeyInSrc, tc.srcBucketPath...)
+				dstBucket := createBucketIfNotExist(t, tx, tc.incompatibleKeyInDst, tc.dstBucketPath...)
+
+				if tc.incompatibleKeyInSrc {
+					if pErr := srcBucket.Put([]byte(tc.bucketToMove), []byte("0")); pErr != nil {
+						t.Fatalf("error inserting key %v, and value %v in bucket %v: %v", tc.bucketToMove, "0", srcBucket, pErr)
+					}
+				}
+
+				if tc.incompatibleKeyInDst {
+					if pErr := dstBucket.Put([]byte(tc.bucketToMove), []byte("0")); pErr != nil {
+						t.Fatalf("error inserting key %v, and value %v in bucket %v: %v", tc.bucketToMove, "0", dstBucket, pErr)
+					}
+				}
+
+				return nil
+			}); err != nil {
+				t.Fatal(err)
+			}
+			db.MustCheck()
+
+			// act
+			if err := db.Update(func(tx *bbolt.Tx) error {
+				srcBucket := retrieveBucket(t, tx, tc.srcBucketPath...)
+				dstBucket := retrieveBucket(t, tx, tc.dstBucketPath...)
+
+				var bucketToMove *bbolt.Bucket
+				if srcBucket != nil {
+					bucketToMove = srcBucket.Bucket([]byte(tc.bucketToMove))
+				} else {
+					bucketToMove = tx.Bucket([]byte(tc.bucketToMove))
+				}
+
+				if bucketToMove != nil {
+					if wErr := dumpBucket([]byte(tc.bucketToMove), bucketToMove, dumpBucketBeforeMoving); wErr != nil {
+						t.Fatalf("error dumping bucket %v to file %v: %v", bucketToMove.String(), dumpBucketBeforeMoving, wErr)
+					}
+				}
+
+				mErr := tx.MoveBucket([]byte(tc.bucketToMove), srcBucket, dstBucket)
+				require.ErrorIs(t, mErr, tc.expActErr)
+
+				return nil
+			}); err != nil {
+				t.Fatal(err)
+			}
+			db.MustCheck()
+
+			// skip assertion if failure expected
+			if tc.expActErr != nil {
+				return
+			}
+
+			// assert
+			if err := db.View(func(tx *bbolt.Tx) error {
+				var movedBucket *bbolt.Bucket
+				srcBucket := retrieveBucket(t, tx, tc.srcBucketPath...)
+
+				if srcBucket != nil {
+					if movedBucket = srcBucket.Bucket([]byte(tc.bucketToMove)); movedBucket != nil {
+						t.Fatalf("expected childBucket %v to be moved from srcBucket %v", tc.bucketToMove, srcBucket)
+					}
+				} else {
+					if movedBucket = tx.Bucket([]byte(tc.bucketToMove)); movedBucket != nil {
+						t.Fatalf("expected childBucket %v to be moved from root bucket %v", tc.bucketToMove, "root bucket")
+					}
+				}
+
+				dstBucket := retrieveBucket(t, tx, tc.dstBucketPath...)
+				if dstBucket != nil {
+					if movedBucket = dstBucket.Bucket([]byte(tc.bucketToMove)); movedBucket == nil {
+						t.Fatalf("expected childBucket %v to be child of dstBucket %v", tc.bucketToMove, dstBucket)
+					}
+				} else {
+					if movedBucket = tx.Bucket([]byte(tc.bucketToMove)); movedBucket == nil {
+						t.Fatalf("expected childBucket %v to be child of dstBucket %v", tc.bucketToMove, "root bucket")
+					}
+				}
+
+				wErr := dumpBucket([]byte(tc.bucketToMove), movedBucket, dumpBucketAfterMoving)
+				if wErr != nil {
+					t.Fatalf("error dumping bucket %v to file %v", movedBucket.String(), dumpBucketAfterMoving)
+				}
+
+				beforeBucket := readBucketFromFile(t, dumpBucketBeforeMoving)
+				afterBucket := readBucketFromFile(t, dumpBucketAfterMoving)
+
+				if !bytes.Equal(beforeBucket, afterBucket) {
+					t.Fatalf("bucket's content before moving is different than after moving")
+				}
+
+				return nil
+			}); err != nil {
+				t.Fatal(err)
+			}
+			db.MustCheck()
+		})
+	}
+}
+
+func createBucketIfNotExist(t testing.TB, tx *bbolt.Tx, incompatibleKey bool, paths ...string) *bbolt.Bucket {
+	t.Helper()
+
+	var bk *bbolt.Bucket
+	var err error
+
+	idx := len(paths) - 1
+	for i, key := range paths {
+		if len(key) == 0 || (incompatibleKey && i == idx) {
+			continue
+		}
+		if bk == nil {
+			bk, err = tx.CreateBucketIfNotExists([]byte(key))
+		} else {
+			bk, err = bk.CreateBucketIfNotExists([]byte(key))
+		}
+		if err != nil {
+			t.Fatalf("error creating bucket %v: %v", key, err)
+		}
+		insertRandKeysValuesBucket(t, bk, rand.Intn(4096))
+	}
+
+	return bk
+}
+
+func retrieveBucket(t testing.TB, tx *bbolt.Tx, paths ...string) *bbolt.Bucket {
+	t.Helper()
+
+	paths = paths[:len(paths)-1]
+	var bk *bbolt.Bucket = nil
+	for _, path := range paths {
+		// skip root bucket
+		if path == "" {
+			continue
+		}
+		if bk == nil {
+			bk = tx.Bucket([]byte(path))
+		} else {
+			bk = bk.Bucket([]byte(path))
+		}
+		if bk == nil {
+			t.Fatalf("error retrieving bucket %v within paths %v", path, strings.TrimSuffix(strings.Join(paths, "->"), "->"))
+		}
+	}
+
+	return bk
+}
+
+func readBucketFromFile(t testing.TB, tmpFile string) []byte {
+	data, err := os.ReadFile(tmpFile)
+	if err != nil {
+		t.Fatalf("error reading temp file %v", tmpFile)
+	}
+
+	return data
+}
+
+func insertRandKeysValuesBucket(t testing.TB, bk *bbolt.Bucket, n int) {
+	var min, max = 1, 1024
+
+	for i := 0; i < n; i++ {
+		// generate rand key/value length
+		keyLength := rand.Intn(max-min) + min
+		valLength := rand.Intn(max-min) + min
+
+		keyData := make([]byte, keyLength)
+		valData := make([]byte, valLength)
+
+		_, err := crand.Read(keyData)
+		require.NoError(t, err)
+
+		_, err = crand.Read(valData)
+		require.NoError(t, err)
+
+		err = bk.Put(keyData, valData)
+		require.NoError(t, err)
+	}
+}

utils_test.go

@@ -0,0 +1,46 @@
+package bbolt_test
+
+import (
+	bolt "go.etcd.io/bbolt"
+	"go.etcd.io/bbolt/internal/common"
+)
+
+// `dumpBucket` dumps all the data, including both key/value data
+// and child buckets, from the source bucket into the target db file.
+func dumpBucket(srcBucketName []byte, srcBucket *bolt.Bucket, dstFilename string) error {
+	common.Assert(len(srcBucketName) != 0, "source bucket name can't be empty")
+	common.Assert(srcBucket != nil, "the source bucket can't be nil")
+	common.Assert(len(dstFilename) != 0, "the target file path can't be empty")
+
+	dstDB, err := bolt.Open(dstFilename, 0600, nil)
+	if err != nil {
+		return err
+	}
+
+	return dstDB.Update(func(tx *bolt.Tx) error {
+		dstBucket, err := tx.CreateBucket(srcBucketName)
+		if err != nil {
+			return err
+		}
+		return cloneBucket(srcBucket, dstBucket)
+	})
+}
+
+func cloneBucket(src *bolt.Bucket, dst *bolt.Bucket) error {
+	return src.ForEach(func(k, v []byte) error {
+		if v == nil {
+			srcChild := src.Bucket(k)
+			dstChild, err := dst.CreateBucket(k)
+			if err != nil {
+				return err
+			}
+			if err = dstChild.SetSequence(srcChild.Sequence()); err != nil {
+				return err
+			}
+
+			return cloneBucket(srcChild, dstChild)
+		}
+
+		return dst.Put(k, v)
+	})
+}