diff --git a/model/repo.go b/model/repo.go index cab0c02e1..4ef2157a6 100644 --- a/model/repo.go +++ b/model/repo.go @@ -31,6 +31,7 @@ type Repo struct { AllowPush bool `json:"allow_push" meddler:"repo_allow_push"` AllowDeploy bool `json:"allow_deploys" meddler:"repo_allow_deploys"` AllowTag bool `json:"allow_tags" meddler:"repo_allow_tags"` + Counter int `json:"last_build" meddler:"repo_counter"` Config string `json:"config_file" meddler:"repo_config_path"` Hash string `json:"-" meddler:"repo_hash"` } diff --git a/store/datastore/builds.go b/store/datastore/builds.go index e43fb639e..2e97d34b4 100644 --- a/store/datastore/builds.go +++ b/store/datastore/builds.go @@ -1,9 +1,11 @@ package datastore import ( + "fmt" "time" "github.com/drone/drone/model" + "github.com/drone/drone/store/datastore/sql" "github.com/russross/meddler" ) @@ -56,12 +58,14 @@ func (db *datastore) GetBuildQueue() ([]*model.Feed, error) { } func (db *datastore) CreateBuild(build *model.Build, procs ...*model.Proc) error { - var number int - db.QueryRow(rebind(buildNumberLast), build.RepoID).Scan(&number) - build.Number = number + 1 + id, err := db.incrementRepoRetry(build.RepoID) + if err != nil { + return err + } + build.Number = id build.Created = time.Now().UTC().Unix() build.Enqueued = build.Created - err := meddler.Insert(db, buildTable, build) + err = meddler.Insert(db, buildTable, build) if err != nil { return err } @@ -75,6 +79,39 @@ func (db *datastore) CreateBuild(build *model.Build, procs ...*model.Proc) error return nil } +func (db *datastore) incrementRepoRetry(id int64) (int, error) { + repo, err := db.GetRepo(id) + if err != nil { + return 0, fmt.Errorf("database: cannot fetch repository: %s", err) + } + for i := 0; i < 10; i++ { + seq, err := db.incrementRepo(repo.ID, repo.Counter+i, repo.Counter+i+1) + if err != nil { + return 0, err + } + if seq == 0 { + continue + } + return seq, nil + } + return 0, fmt.Errorf("cannot increment next build number") +} + +func (db *datastore) incrementRepo(id int64, old, new int) (int, error) { + results, err := db.Exec(sql.Lookup(db.driver, "repo-update-counter"), new, old, id) + if err != nil { + return 0, fmt.Errorf("database: update repository counter: %s", err) + } + updated, err := results.RowsAffected() + if err != nil { + return 0, fmt.Errorf("database: update repository counter: %s", err) + } + if updated == 0 { + return 0, nil + } + return new, nil +} + func (db *datastore) UpdateBuild(build *model.Build) error { return meddler.Update(db, buildTable, build) } diff --git a/store/datastore/builds_test.go b/store/datastore/builds_test.go index 783578216..19fed18a7 100644 --- a/store/datastore/builds_test.go +++ b/store/datastore/builds_test.go @@ -8,27 +8,40 @@ import ( ) func TestBuilds(t *testing.T) { - db := openTest() - defer db.Close() + repo := &model.Repo{ + UserID: 1, + FullName: "bradrydzewski/drone", + Owner: "bradrydzewski", + Name: "drone", + } + + s := newTest() + defer s.Close() - s := From(db) g := goblin.Goblin(t) g.Describe("Builds", func() { + g.Before(func() { + s.Exec("DELETE FROM repos") + s.CreateRepo(repo) + }) + g.After(func() { + s.Exec("DELETE FROM repos") + }) // before each test be sure to purge the package // table data from the database. g.BeforeEach(func() { - db.Exec("DELETE FROM builds") - db.Exec("DELETE FROM jobs") + s.Exec("DELETE FROM builds") + s.Exec("DELETE FROM jobs") }) g.It("Should Post a Build", func() { build := model.Build{ - RepoID: 1, + RepoID: repo.ID, Status: model.StatusSuccess, Commit: "85f8c029b902ed9400bc600bac301a0aadb144ac", } - err := s.CreateBuild(&build, []*model.Proc{}...) + err := s.CreateBuild(&build) g.Assert(err == nil).IsTrue() g.Assert(build.ID != 0).IsTrue() g.Assert(build.Number).Equal(1) @@ -37,12 +50,12 @@ func TestBuilds(t *testing.T) { g.It("Should Put a Build", func() { build := model.Build{ - RepoID: 1, + RepoID: repo.ID, Number: 5, Status: model.StatusSuccess, Commit: "85f8c029b902ed9400bc600bac301a0aadb144ac", } - s.CreateBuild(&build, []*model.Proc{}...) + s.CreateBuild(&build) build.Status = model.StatusRunning err1 := s.UpdateBuild(&build) getbuild, err2 := s.GetBuild(build.ID) @@ -56,7 +69,7 @@ func TestBuilds(t *testing.T) { g.It("Should Get a Build", func() { build := model.Build{ - RepoID: 1, + RepoID: repo.ID, Status: model.StatusSuccess, } s.CreateBuild(&build, []*model.Proc{}...) @@ -69,11 +82,11 @@ func TestBuilds(t *testing.T) { g.It("Should Get a Build by Number", func() { build1 := &model.Build{ - RepoID: 1, + RepoID: repo.ID, Status: model.StatusPending, } build2 := &model.Build{ - RepoID: 1, + RepoID: repo.ID, Status: model.StatusPending, } err1 := s.CreateBuild(build1, []*model.Proc{}...) @@ -89,12 +102,12 @@ func TestBuilds(t *testing.T) { g.It("Should Get a Build by Ref", func() { build1 := &model.Build{ - RepoID: 1, + RepoID: repo.ID, Status: model.StatusPending, Ref: "refs/pull/5", } build2 := &model.Build{ - RepoID: 1, + RepoID: repo.ID, Status: model.StatusPending, Ref: "refs/pull/6", } @@ -112,12 +125,12 @@ func TestBuilds(t *testing.T) { g.It("Should Get a Build by Ref", func() { build1 := &model.Build{ - RepoID: 1, + RepoID: repo.ID, Status: model.StatusPending, Ref: "refs/pull/5", } build2 := &model.Build{ - RepoID: 1, + RepoID: repo.ID, Status: model.StatusPending, Ref: "refs/pull/6", } @@ -135,13 +148,13 @@ func TestBuilds(t *testing.T) { g.It("Should Get a Build by Commit", func() { build1 := &model.Build{ - RepoID: 1, + RepoID: repo.ID, Status: model.StatusPending, Branch: "master", Commit: "85f8c029b902ed9400bc600bac301a0aadb144ac", } build2 := &model.Build{ - RepoID: 1, + RepoID: repo.ID, Status: model.StatusPending, Branch: "dev", Commit: "85f8c029b902ed9400bc600bac301a0aadb144aa", @@ -161,14 +174,14 @@ func TestBuilds(t *testing.T) { g.It("Should Get the last Build", func() { build1 := &model.Build{ - RepoID: 1, + RepoID: repo.ID, Status: model.StatusFailure, Branch: "master", Commit: "85f8c029b902ed9400bc600bac301a0aadb144ac", Event: model.EventPush, } build2 := &model.Build{ - RepoID: 1, + RepoID: repo.ID, Status: model.StatusSuccess, Branch: "master", Commit: "85f8c029b902ed9400bc600bac301a0aadb144aa", @@ -190,19 +203,19 @@ func TestBuilds(t *testing.T) { g.It("Should Get the last Build Before Build N", func() { build1 := &model.Build{ - RepoID: 1, + RepoID: repo.ID, Status: model.StatusFailure, Branch: "master", Commit: "85f8c029b902ed9400bc600bac301a0aadb144ac", } build2 := &model.Build{ - RepoID: 1, + RepoID: repo.ID, Status: model.StatusSuccess, Branch: "master", Commit: "85f8c029b902ed9400bc600bac301a0aadb144aa", } build3 := &model.Build{ - RepoID: 1, + RepoID: repo.ID, Status: model.StatusRunning, Branch: "master", Commit: "85f8c029b902ed9400bc600bac301a0aadb144aa", @@ -225,11 +238,11 @@ func TestBuilds(t *testing.T) { g.It("Should get recent Builds", func() { build1 := &model.Build{ - RepoID: 1, + RepoID: repo.ID, Status: model.StatusFailure, } build2 := &model.Build{ - RepoID: 1, + RepoID: repo.ID, Status: model.StatusSuccess, } s.CreateBuild(build1, []*model.Proc{}...) @@ -243,3 +256,58 @@ func TestBuilds(t *testing.T) { }) }) } + +func TestBuildIncrement(t *testing.T) { + s := newTest() + defer func() { + s.Exec("delete from repos") + s.Exec("delete from builds") + s.Close() + }() + + repo := &model.Repo{ + UserID: 1, + FullName: "bradrydzewski/drone", + Owner: "bradrydzewski", + Name: "drone", + } + if err := s.CreateRepo(repo); err != nil { + t.Error(err) + } + + num, err := s.incrementRepo(repo.ID, 0, 1) + if err != nil { + t.Error(err) + } + + if got, want := num, 1; got != want { + t.Errorf("Want repository counter incremented to %d, got %d", want, got) + } + + num, err = s.incrementRepo(repo.ID, 1, 2) + if err != nil { + t.Error(err) + } + + if got, want := num, 2; got != want { + t.Errorf("Want repository counter incremented to %d, got %d", want, got) + } + + // this block tests incrementing the repository counter + // should fail when attempting to increment the counter + // from a stale base. + num, _ = s.incrementRepo(repo.ID, 1, 2) + if num != 0 { + t.Errorf("Want error when trying to increment stale number") + } + + // this block tests incrementing the repository counter + // using the given repository id with backoff. + num, err = s.incrementRepoRetry(repo.ID) + if err != nil { + t.Error(err) + } + if got, want := num, 3; got != want { + t.Errorf("Want repository counter incremented to %d, got %d", want, got) + } +} diff --git a/store/datastore/config_test.go b/store/datastore/config_test.go index 9586fed0a..cdcceaab3 100644 --- a/store/datastore/config_test.go +++ b/store/datastore/config_test.go @@ -62,14 +62,23 @@ func TestConfigApproved(t *testing.T) { defer func() { s.Exec("delete from config") s.Exec("delete from builds") + s.Exec("delete from repos") s.Close() }() + repo := &model.Repo{ + UserID: 1, + FullName: "bradrydzewski/drone", + Owner: "bradrydzewski", + Name: "drone", + } + s.CreateRepo(repo) + var ( data = "pipeline: [ { image: golang, commands: [ go build, go test ] } ]" hash = "8d8647c9aa90d893bfb79dddbe901f03e258588121e5202632f8ae5738590b26" conf = &model.Config{ - RepoID: 1, + RepoID: repo.ID, Data: data, Hash: hash, } @@ -79,15 +88,14 @@ func TestConfigApproved(t *testing.T) { t.Errorf("Unexpected error: insert config: %s", err) return } - s.CreateBuild(&model.Build{ - RepoID: 1, + RepoID: repo.ID, ConfigID: conf.ID, Status: model.StatusBlocked, Commit: "85f8c029b902ed9400bc600bac301a0aadb144ac", }) s.CreateBuild(&model.Build{ - RepoID: 1, + RepoID: repo.ID, ConfigID: conf.ID, Status: model.StatusPending, Commit: "85f8c029b902ed9400bc600bac301a0aadb144ac", @@ -99,7 +107,7 @@ func TestConfigApproved(t *testing.T) { } s.CreateBuild(&model.Build{ - RepoID: 1, + RepoID: repo.ID, ConfigID: conf.ID, Status: model.StatusRunning, Commit: "85f8c029b902ed9400bc600bac301a0aadb144ac", diff --git a/store/datastore/ddl/mysql/ddl_gen.go b/store/datastore/ddl/mysql/ddl_gen.go index e70048265..f3f9efe2d 100644 --- a/store/datastore/ddl/mysql/ddl_gen.go +++ b/store/datastore/ddl/mysql/ddl_gen.go @@ -96,6 +96,14 @@ var migrations = []struct { name: "update-table-set-repo-visibility", stmt: updateTableSetRepoVisibility, }, + { + name: "alter-table-add-repo-seq", + stmt: alterTableAddRepoSeq, + }, + { + name: "update-table-set-repo-seq", + stmt: updateTableSetRepoSeq, + }, } // Migrate performs the database migration. If the migration fails @@ -466,3 +474,19 @@ SET repo_visibility = CASE ELSE 'private' END ` + +// +// 014_add_column_repo_seq.sql +// + +var alterTableAddRepoSeq = ` +ALTER TABLE repos ADD COLUMN repo_counter INTEGER; +` + +var updateTableSetRepoSeq = ` +UPDATE repos SET repo_counter = ( + SELECT max(build_number) + FROM builds + WHERE builds.build_repo_id = repos.repo_id +) +` diff --git a/store/datastore/ddl/mysql/files/014_add_column_repo_seq.sql b/store/datastore/ddl/mysql/files/014_add_column_repo_seq.sql new file mode 100644 index 000000000..ed04c9848 --- /dev/null +++ b/store/datastore/ddl/mysql/files/014_add_column_repo_seq.sql @@ -0,0 +1,11 @@ +-- name: alter-table-add-repo-seq + +ALTER TABLE repos ADD COLUMN repo_counter INTEGER; + +-- name: update-table-set-repo-seq + +UPDATE repos SET repo_counter = ( + SELECT max(build_number) + FROM builds + WHERE builds.build_repo_id = repos.repo_id +) diff --git a/store/datastore/ddl/postgres/ddl_gen.go b/store/datastore/ddl/postgres/ddl_gen.go index cd44d250f..a39cebd74 100644 --- a/store/datastore/ddl/postgres/ddl_gen.go +++ b/store/datastore/ddl/postgres/ddl_gen.go @@ -96,6 +96,14 @@ var migrations = []struct { name: "update-table-set-repo-visibility", stmt: updateTableSetRepoVisibility, }, + { + name: "alter-table-add-repo-seq", + stmt: alterTableAddRepoSeq, + }, + { + name: "update-table-set-repo-seq", + stmt: updateTableSetRepoSeq, + }, } // Migrate performs the database migration. If the migration fails @@ -466,3 +474,19 @@ SET repo_visibility = (CASE ELSE 'private' END) ` + +// +// 014_add_column_repo_seq.sql +// + +var alterTableAddRepoSeq = ` +ALTER TABLE repos ADD COLUMN repo_counter INTEGER; +` + +var updateTableSetRepoSeq = ` +UPDATE repos SET repo_counter = ( + SELECT max(build_number) + FROM builds + WHERE builds.build_repo_id = repos.repo_id +) +` diff --git a/store/datastore/ddl/postgres/files/014_add_column_repo_seq.sql b/store/datastore/ddl/postgres/files/014_add_column_repo_seq.sql new file mode 100644 index 000000000..ed04c9848 --- /dev/null +++ b/store/datastore/ddl/postgres/files/014_add_column_repo_seq.sql @@ -0,0 +1,11 @@ +-- name: alter-table-add-repo-seq + +ALTER TABLE repos ADD COLUMN repo_counter INTEGER; + +-- name: update-table-set-repo-seq + +UPDATE repos SET repo_counter = ( + SELECT max(build_number) + FROM builds + WHERE builds.build_repo_id = repos.repo_id +) diff --git a/store/datastore/ddl/sqlite/ddl_gen.go b/store/datastore/ddl/sqlite/ddl_gen.go index 22cfb91d8..5d660ee89 100644 --- a/store/datastore/ddl/sqlite/ddl_gen.go +++ b/store/datastore/ddl/sqlite/ddl_gen.go @@ -100,6 +100,14 @@ var migrations = []struct { name: "update-table-set-repo-visibility", stmt: updateTableSetRepoVisibility, }, + { + name: "alter-table-add-repo-seq", + stmt: alterTableAddRepoSeq, + }, + { + name: "update-table-set-repo-seq", + stmt: updateTableSetRepoSeq, + }, } // Migrate performs the database migration. If the migration fails @@ -467,3 +475,19 @@ SET repo_visibility = CASE ELSE 'private' END ` + +// +// 014_add_column_repo_seq.sql +// + +var alterTableAddRepoSeq = ` +ALTER TABLE repos ADD COLUMN repo_counter INTEGER; +` + +var updateTableSetRepoSeq = ` +UPDATE repos SET repo_counter = ( + SELECT max(build_number) + FROM builds + WHERE builds.build_repo_id = repos.repo_id +) +` diff --git a/store/datastore/ddl/sqlite/files/014_add_column_repo_seq.sql b/store/datastore/ddl/sqlite/files/014_add_column_repo_seq.sql new file mode 100644 index 000000000..ed04c9848 --- /dev/null +++ b/store/datastore/ddl/sqlite/files/014_add_column_repo_seq.sql @@ -0,0 +1,11 @@ +-- name: alter-table-add-repo-seq + +ALTER TABLE repos ADD COLUMN repo_counter INTEGER; + +-- name: update-table-set-repo-seq + +UPDATE repos SET repo_counter = ( + SELECT max(build_number) + FROM builds + WHERE builds.build_repo_id = repos.repo_id +) diff --git a/store/datastore/sql/postgres/files/repos.sql b/store/datastore/sql/postgres/files/repos.sql new file mode 100644 index 000000000..7b07f5156 --- /dev/null +++ b/store/datastore/sql/postgres/files/repos.sql @@ -0,0 +1,5 @@ +-- name: repo-update-counter + +UPDATE repos SET repo_counter = $1 +WHERE repo_counter = $2 + AND repo_id = $3 diff --git a/store/datastore/sql/postgres/sql_gen.go b/store/datastore/sql/postgres/sql_gen.go index 4528f38b4..3eee16d34 100644 --- a/store/datastore/sql/postgres/sql_gen.go +++ b/store/datastore/sql/postgres/sql_gen.go @@ -25,6 +25,7 @@ var index = map[string]string{ "registry-find-repo-addr": registryFindRepoAddr, "registry-delete-repo": registryDeleteRepo, "registry-delete": registryDelete, + "repo-update-counter": repoUpdateCounter, "secret-find-repo": secretFindRepo, "secret-find-repo-name": secretFindRepoName, "secret-delete": secretDelete, @@ -249,6 +250,12 @@ var registryDelete = ` DELETE FROM registry WHERE registry_id = $1 ` +var repoUpdateCounter = ` +UPDATE repos SET repo_counter = $1 +WHERE repo_counter = $2 + AND repo_id = $3 +` + var secretFindRepo = ` SELECT secret_id diff --git a/store/datastore/sql/sqlite/files/repos.sql b/store/datastore/sql/sqlite/files/repos.sql new file mode 100644 index 000000000..0958acbf1 --- /dev/null +++ b/store/datastore/sql/sqlite/files/repos.sql @@ -0,0 +1,5 @@ +-- name: repo-update-counter + +UPDATE repos SET repo_counter = ? +WHERE repo_counter = ? + AND repo_id = ? diff --git a/store/datastore/sql/sqlite/sql_gen.go b/store/datastore/sql/sqlite/sql_gen.go index 6fe62934b..ba056c411 100644 --- a/store/datastore/sql/sqlite/sql_gen.go +++ b/store/datastore/sql/sqlite/sql_gen.go @@ -25,6 +25,7 @@ var index = map[string]string{ "registry-find-repo-addr": registryFindRepoAddr, "registry-delete-repo": registryDeleteRepo, "registry-delete": registryDelete, + "repo-update-counter": repoUpdateCounter, "secret-find-repo": secretFindRepo, "secret-find-repo-name": secretFindRepoName, "secret-delete": secretDelete, @@ -249,6 +250,12 @@ var registryDelete = ` DELETE FROM registry WHERE registry_id = ? ` +var repoUpdateCounter = ` +UPDATE repos SET repo_counter = ? +WHERE repo_counter = ? + AND repo_id = ? +` + var secretFindRepo = ` SELECT secret_id diff --git a/store/datastore/users_test.go b/store/datastore/users_test.go index fae9931c0..37d45a7e5 100644 --- a/store/datastore/users_test.go +++ b/store/datastore/users_test.go @@ -8,9 +8,8 @@ import ( ) func TestUsers(t *testing.T) { - db := openTest() - defer db.Close() - s := From(db) + s := newTest() + defer s.Close() g := goblin.Goblin(t) g.Describe("User", func() { @@ -18,10 +17,10 @@ func TestUsers(t *testing.T) { // before each test be sure to purge the package // table data from the database. g.BeforeEach(func() { - db.Exec("DELETE FROM users") - db.Exec("DELETE FROM repos") - db.Exec("DELETE FROM builds") - db.Exec("DELETE FROM jobs") + s.Exec("DELETE FROM users") + s.Exec("DELETE FROM repos") + s.Exec("DELETE FROM builds") + s.Exec("DELETE FROM jobs") }) g.It("Should Update a User", func() { @@ -138,7 +137,11 @@ func TestUsers(t *testing.T) { s.CreateUser(&user2) count, err := s.GetUserCount() g.Assert(err == nil).IsTrue() - g.Assert(count).Equal(2) + if s.driver != "postgres" { + // we have to skip this check for postgres because it uses + // an estimate which may not be updated. + g.Assert(count).Equal(2) + } }) g.It("Should Get a User Count Zero", func() {