Merge pull request #987 from benschumacher/cleanup-drone/drone#959

Fix error handling, remove unused bucketBuildTasks
This commit is contained in:
Brad Rydzewski 2015-04-22 11:40:31 -07:00
commit c0f5aec77b
5 changed files with 55 additions and 80 deletions

View file

@ -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

View file

@ -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)
deleteWithPrefix(t, bucketBuildStatus, prefix, true)
deleteWithPrefix(t, bucketBuildTasks, prefix, true)
deleteWithPrefix(t, bucketBuild, prefix, true)
buckets := [][]byte{bucketBuildStatus, bucketBuildLogs, bucketBuild}
for _, b := range buckets {
err = deleteWithPrefix(t, b, prefix)
if err != nil {
break
}
}
return err
}

View file

@ -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)
})
})
}

View file

@ -1,10 +1,12 @@
package bolt
import (
"github.com/drone/drone/common"
. "github.com/franela/goblin"
"io/ioutil"
"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
// test and destroy afterwards.
// create a new database before each unit 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})

View file

@ -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
}