Fix error handling, remove unused bucketBuildTasks

datastore/bolt/bolt.go

@@ -24,7 +24,6 @@ var (
 	bucketRepoUsers   = []byte("repo_users")
 	bucketBuild       = []byte("build")
 	bucketBuildStatus = []byte("build_status")
-	bucketBuildTasks  = []byte("build_tasks")
 	bucketBuildLogs   = []byte("build_logs")
 	bucketBuildSeq    = []byte("build_seq")
 )
@@ -51,7 +50,6 @@ func New(path string) (*DB, error) {
 		tx.CreateBucketIfNotExists(bucketRepoUsers)
 		tx.CreateBucketIfNotExists(bucketBuild)
 		tx.CreateBucketIfNotExists(bucketBuildStatus)
-		tx.CreateBucketIfNotExists(bucketBuildTasks)
 		tx.CreateBucketIfNotExists(bucketBuildLogs)
 		tx.CreateBucketIfNotExists(bucketBuildSeq)
 		return nil

datastore/bolt/repo.go

@@ -153,20 +153,25 @@ func (db *DB) DelRepo(repo *common.Repo) error {
 
 // deleteTracesOfRepo cleans up build leftovers when a repo is removed
 func (db *DB) deleteTracesOfRepo(t *bolt.Tx, repoKey []byte) error {
-	err := error(nil)
-
 	// bucketBuildSeq uses the repoKey directly
-	t.Bucket(bucketBuildSeq).Delete(repoKey)
+	err := t.Bucket(bucketBuildSeq).Delete(repoKey)
+	if err != nil {
+		// only error here is if our Tx is read-only
+		return err
+	}
 
 	// 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)
+	buckets := [][]byte{bucketBuildStatus, bucketBuildLogs, bucketBuild}
-	deleteWithPrefix(t, bucketBuildStatus, prefix, true)
+	for _, b := range buckets {
-	deleteWithPrefix(t, bucketBuildTasks, prefix, true)
+		err = deleteWithPrefix(t, b, prefix)
-	deleteWithPrefix(t, bucketBuild, prefix, true)
+		if err != nil {
+			break
+		}
+	}
 
 	return err
 }

datastore/bolt/repo_del_test.go

@@ -1,63 +0,0 @@
-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/repo_test.go

@@ -1,10 +1,12 @@
 package bolt
 
 import (
-	"github.com/drone/drone/common"
+	"io/ioutil"
-	. "github.com/franela/goblin"
 	"os"
 	"testing"
+
+	"github.com/drone/drone/common"
+	. "github.com/franela/goblin"
 )
 
 func TestRepo(t *testing.T) {
@@ -13,12 +15,17 @@ func TestRepo(t *testing.T) {
 		testUser := "octocat"
 		testRepo := "github.com/octopod/hq"
 		testRepo2 := "github.com/octopod/avengers"
+		commUser := &common.User{Login: "freya"}
 		var db *DB // Temp database
 
-		// create a new database before each unit
+		// create a new database before each unit test and destroy afterwards.
-		// test and destroy afterwards.
 		g.BeforeEach(func() {
-			db = Must("/tmp/drone.test.db")
+			file, err := ioutil.TempFile(os.TempDir(), "drone-bolt")
+			if err != nil {
+				panic(err)
+			}
+
+			db = Must(file.Name())
 		})
 		g.AfterEach(func() {
 			os.Remove(db.Path())
@@ -41,7 +48,7 @@ func TestRepo(t *testing.T) {
 			g.Assert(repo.FullName).Equal(testRepo)
 		})
 
-		g.It("Should del Repo", func() {
+		g.It("Should be deletable", func() {
 			db.SetRepo(&common.Repo{FullName: testRepo})
 
 			db.Repo(testRepo)
@@ -49,6 +56,33 @@ func TestRepo(t *testing.T) {
 			g.Assert(err_).Equal(nil)
 		})
 
+		g.It("Should cleanup builds when deleted", func() {
+			repo := &common.Repo{FullName: testRepo}
+			err := db.SetRepoNotExists(commUser, repo)
+			g.Assert(err).Equal(nil)
+
+			db.SetBuild(testRepo, &common.Build{State: "success"})
+			db.SetBuild(testRepo, &common.Build{State: "success"})
+			db.SetBuild(testRepo, &common.Build{State: "pending"})
+
+			db.SetBuildStatus(testRepo, 1, &common.Status{Context: "success"})
+			db.SetBuildStatus(testRepo, 2, &common.Status{Context: "success"})
+			db.SetBuildStatus(testRepo, 3, &common.Status{Context: "pending"})
+
+			// first a little sanity to validate our test conditions
+			_, err = db.BuildLast(testRepo)
+			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(testRepo, 1)
+			g.Assert(err).Equal(ErrKeyNotFound)
+		})
+
 		g.It("Should get RepoList", func() {
 			db.SetRepoNotExists(&common.User{Login: testUser}, &common.Repo{FullName: testRepo})
 			db.SetRepoNotExists(&common.User{Login: testUser}, &common.Repo{FullName: testRepo2})

datastore/bolt/util.go

@@ -85,16 +85,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 {
+func deleteWithPrefix(t *bolt.Tx, bucket, prefix []byte) 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 {
+		if err != nil {
 			break
 		}
 	}
 
+	// only error here is if our Tx is read-only
 	return err
 }