Working on drone/drone#959

datastore/bolt/repo.go

@@ -132,23 +132,43 @@ func (db *DB) SetRepoKeypair(repo string, keypair *common.Keypair) error {
 
 // DelRepo deletes the repository.
 func (db *DB) DelRepo(repo *common.Repo) error {
-	//TODO(benschumacher) rework this to use BoltDB's txn wrapper
-
-	t, err := db.Begin(true)
-	if err != nil {
-		return err
-	}
 	key := []byte(repo.FullName)
-	err = t.Bucket(bucketRepo).Delete(key)
+
-	if err != nil {
+	return db.Update(func(t *bolt.Tx) error {
-		t.Rollback()
+		err := t.Bucket(bucketRepo).Delete(key)
+		if err != nil {
+			return err
+		}
+		t.Bucket(bucketRepoKeys).Delete(key)
+		t.Bucket(bucketRepoParams).Delete(key)
+
+		// should we just ignore these error conditions? or should
+		// we go ahead with the transaction and assume we can
+		// cleanup the leftovers through some other maintenance process?
+		err = db.deleteTracesOfRepo(t, key)
+
 		return err
-	}
+	})
-	t.Bucket(bucketRepoKeys).Delete(key)
+}
-	t.Bucket(bucketRepoParams).Delete(key)
+
-	// TODO(bradrydzewski) delete all builds
+// deleteTracesOfRepo cleans up build leftovers when a repo is removed
-	// TODO(bradrydzewski) delete all tasks
+func (db *DB) deleteTracesOfRepo(t *bolt.Tx, repoKey []byte) error {
-	return t.Commit()
+	err := error(nil)
+
+	// bucketBuildSeq uses the repoKey directly
+	t.Bucket(bucketBuildSeq).Delete(repoKey)
+
+	// the other buckets use repoKey with '/buildNumber', at least.
+	// validating that an additiona '/' is there ensures that we don't
+	// match 'github.com/drone/droney' when we're cleaning up after
+	// 'github.com/drone/drone'.
+	prefix := append(repoKey, '/')
+	deleteWithPrefix(t, bucketBuildLogs, prefix, true)
+	deleteWithPrefix(t, bucketBuildStatus, prefix, true)
+	deleteWithPrefix(t, bucketBuildTasks, prefix, true)
+	deleteWithPrefix(t, bucketBuild, prefix, true)
+
+	return err
 }
 
 // Subscribed returns true if the user is subscribed

datastore/bolt/repo_del_test.go

@@ -0,0 +1,63 @@
+package bolt
+
+import (
+	"io/ioutil"
+	"os"
+	"testing"
+
+	"github.com/drone/drone/common"
+	. "github.com/franela/goblin"
+)
+
+func TestRepoDel(t *testing.T) {
+	g := Goblin(t)
+	g.Describe("Delete repo", func() {
+
+		var db *DB // temporary database
+
+		user := &common.User{Login: "freya"}
+		repoUri := string("github.com/octopod/hq")
+
+		// create a new database before each unit
+		// test and destroy afterwards.
+		g.BeforeEach(func() {
+			file, err := ioutil.TempFile(os.TempDir(), "drone-bolt")
+			if err != nil {
+				panic(err)
+			}
+
+			db = Must(file.Name())
+		})
+		g.AfterEach(func() {
+			os.Remove(db.Path())
+		})
+
+		g.It("should cleanup", func() {
+			repo := &common.Repo{FullName: repoUri}
+			err := db.SetRepoNotExists(user, repo)
+			g.Assert(err).Equal(nil)
+
+			db.SetBuild(repoUri, &common.Build{State: "success"})
+			db.SetBuild(repoUri, &common.Build{State: "success"})
+			db.SetBuild(repoUri, &common.Build{State: "pending"})
+
+			db.SetBuildStatus(repoUri, 1, &common.Status{Context: "success"})
+			db.SetBuildStatus(repoUri, 2, &common.Status{Context: "success"})
+			db.SetBuildStatus(repoUri, 3, &common.Status{Context: "pending"})
+
+			// first a little sanity to validate our test conditions
+			_, err = db.BuildLast(repoUri)
+			g.Assert(err).Equal(nil)
+
+			// now run our specific test suite
+			// 1. ensure that we can delete the repo
+			err = db.DelRepo(repo)
+			g.Assert(err).Equal(nil)
+
+			// 2. ensure that deleting the repo cleans up other references
+			_, err = db.Build(repoUri, 1)
+			g.Assert(err).Equal(ErrKeyNotFound)
+		})
+	})
+
+}

datastore/bolt/util.go

@@ -84,3 +84,17 @@ func splice(t *bolt.Tx, bucket, index, value []byte) error {
 
 	return update(t, bucket, index, &keys)
 }
+
+func deleteWithPrefix(t *bolt.Tx, bucket, prefix []byte, ignoreErr bool) error {
+	var err error
+
+	c := t.Bucket(bucket).Cursor()
+	for k, _ := c.Seek(prefix); bytes.HasPrefix(k, prefix); k, _ = c.Next() {
+		err = c.Delete()
+		if !ignoreErr && err != nil {
+			break
+		}
+	}
+
+	return err
+}