Calculate build number on creation (#615)

* Calculate build number on creation
* Delete repo.Counter (form code and database)

server/api/repo.go

@@ -147,9 +147,6 @@ func PatchRepo(c *gin.Context) {
 			return
 		}
 	}
-	if in.BuildCounter != nil {
-		repo.Counter = *in.BuildCounter
-	}
 
 	err := _store.UpdateRepo(repo)
 	if err != nil {

server/model/repo.go

@@ -42,11 +42,9 @@ type Repo struct {
 	IsGated      bool        `json:"gated"                    xorm:"repo_gated"`
 	IsActive     bool        `json:"active"                   xorm:"repo_active"`
 	AllowPull    bool        `json:"allow_pr"                 xorm:"repo_allow_pr"`
-	// Counter is used as index to determine new build numbers
-	Counter int64  `json:"last_build"                  xorm:"NOT NULL DEFAULT 0 'repo_counter'"`
-	Config  string `json:"config_file"                 xorm:"varchar(500) 'repo_config_path'"`
-	Hash    string `json:"-"                           xorm:"varchar(500) 'repo_hash'"`
-	Perm    *Perm  `json:"-"                           xorm:"-"`
+	Config       string      `json:"config_file"                 xorm:"varchar(500) 'repo_config_path'"`
+	Hash         string      `json:"-"                           xorm:"varchar(500) 'repo_hash'"`
+	Perm         *Perm       `json:"-"                           xorm:"-"`
 }
 
 // TableName return database table name for xorm
@@ -92,11 +90,10 @@ func (r *Repo) Update(from *Repo) {
 
 // RepoPatch represents a repository patch object.
 type RepoPatch struct {
-	Config       *string `json:"config_file,omitempty"`
-	IsTrusted    *bool   `json:"trusted,omitempty"`
-	IsGated      *bool   `json:"gated,omitempty"`
-	Timeout      *int64  `json:"timeout,omitempty"`
-	Visibility   *string `json:"visibility,omitempty"`
-	AllowPull    *bool   `json:"allow_pr,omitempty"`
-	BuildCounter *int64  `json:"build_counter,omitempty"`
+	Config     *string `json:"config_file,omitempty"`
+	IsTrusted  *bool   `json:"trusted,omitempty"`
+	IsGated    *bool   `json:"gated,omitempty"`
+	Timeout    *int64  `json:"timeout,omitempty"`
+	Visibility *string `json:"visibility,omitempty"`
+	AllowPull  *bool   `json:"allow_pr,omitempty"`
 }

server/store/datastore/build.go

@@ -91,17 +91,13 @@ func (s storage) CreateBuild(build *model.Build, procList ...*model.Proc) error
 		return err
 	}
 
-	// increment counter
-	if _, err := sess.ID(build.RepoID).Incr("repo_counter").Update(new(model.Repo)); err != nil {
+	// calc build number
+	var number int64
+	if _, err := sess.SQL("SELECT MAX(build_number) FROM `builds` WHERE build_repo_id = ?", build.RepoID).Get(&number); err != nil {
 		return err
 	}
+	build.Number = number + 1
 
-	repo := new(model.Repo)
-	if err := wrapGet(sess.ID(build.RepoID).Get(repo)); err != nil {
-		return err
-	}
-
-	build.Number = repo.Counter
 	build.Created = time.Now().UTC().Unix()
 	build.Enqueued = build.Created
 	// only Insert set auto created ID back to object

server/store/datastore/build_test.go

@@ -19,6 +19,7 @@ import (
 	"testing"
 
 	"github.com/franela/goblin"
+	"github.com/stretchr/testify/assert"
 
 	"github.com/woodpecker-ci/woodpecker/server/model"
 )
@@ -287,34 +288,20 @@ func TestBuilds(t *testing.T) {
 }
 
 func TestBuildIncrement(t *testing.T) {
-	store, closer := newTestStore(t, new(model.Build), new(model.Repo))
+	store, closer := newTestStore(t, new(model.Build))
 	defer closer()
 
-	repo := &model.Repo{
-		UserID:   1,
-		FullName: "bradrydzewski/test",
-		Owner:    "bradrydzewski",
-		Name:     "test",
-	}
-	if err := store.CreateRepo(repo); err != nil {
-		t.Error(err)
+	buildA := &model.Build{RepoID: 1}
+	if !assert.NoError(t, store.CreateBuild(buildA)) {
+		return
 	}
+	assert.EqualValues(t, 1, buildA.Number)
 
-	if err := store.CreateBuild(&model.Build{RepoID: repo.ID}); err != nil {
-		t.Error(err)
-	}
-	repo, _ = store.GetRepo(repo.ID)
+	buildB := &model.Build{RepoID: 1}
+	assert.NoError(t, store.CreateBuild(buildB))
+	assert.EqualValues(t, 2, buildB.Number)
 
-	if got, want := repo.Counter, int64(1); got != want {
-		t.Errorf("Want repository counter incremented to %d, got %d", want, got)
-	}
-
-	if err := store.CreateBuild(&model.Build{RepoID: repo.ID}); err != nil {
-		t.Error(err)
-	}
-	repo, _ = store.GetRepo(repo.ID)
-
-	if got, want := repo.Counter, int64(2); got != want {
-		t.Errorf("Want repository counter incremented to %d, got %d", want, got)
-	}
+	buildC := &model.Build{RepoID: 2}
+	assert.NoError(t, store.CreateBuild(buildC))
+	assert.EqualValues(t, 1, buildC.Number)
 }

server/store/datastore/migration/004_repos_drop_repo_counter.go

@@ -0,0 +1,26 @@
+// Copyright 2021 Woodpecker Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package migration
+
+import (
+	"xorm.io/xorm"
+)
+
+var alterTableReposDropCounter = task{
+	name: "alter-table-drop-counter",
+	fn: func(sess *xorm.Session) error {
+		return dropTableColumns(sess, "repos", "repo_counter")
+	},
+}

server/store/datastore/migration/migration.go

@@ -31,6 +31,7 @@ var migrationTasks = []task{
 	alterTableReposDropFallback,
 	alterTableReposDropAllowDeploysAllowTags,
 	fixPRSecretEventName,
+	alterTableReposDropCounter,
 }
 
 type migrations struct {