Fix reggression, save not activated repos to database on sync again (#…

…510) * Fix #508 * Add testcase Unrelated nits: * Rm one index (unique will already create an index) * Do not drop on UpdateUser
woodpecker-ci · Nov 14, 2021 · 2524c69 · 2524c69
1 parent c28f7cb
commit 2524c69
Show file tree

Hide file tree

Showing 4 changed files with 42 additions and 28 deletions.
diff --git a/server/api/user.go b/server/api/user.go
@@ -93,7 +93,10 @@ func GetRepos(c *gin.Context) {
 	if flush || time.Unix(user.Synced, 0).Add(time.Hour*72).Before(time.Now()) {
 		log.Debug().Msgf("sync begin: %s", user.Login)
 		user.Synced = time.Now().Unix()
-		store_.UpdateUser(user)
+		if err := store_.UpdateUser(user); err != nil {
+			log.Err(err).Msgf("update user '%s'", user.Login)
+			return
+		}
 
 		config := ToConfig(c)
 

diff --git a/server/model/config.go b/server/model/config.go
@@ -28,7 +28,7 @@ type ConfigStore interface {
 type Config struct {
 	ID     int64  `json:"-"    xorm:"pk autoincr 'config_id'"`
 	RepoID int64  `json:"-"    xorm:"UNIQUE(s) 'config_repo_id'"`
-	Hash   string `json:"hash" xorm:"UNIQUE(s) INDEX 'config_hash'"`
+	Hash   string `json:"hash" xorm:"UNIQUE(s) 'config_hash'"`
 	Name   string `json:"name" xorm:"config_name"`
 	Data   []byte `json:"data" xorm:"config_data"`
 }

diff --git a/server/store/datastore/repo.go b/server/store/datastore/repo.go
@@ -82,8 +82,8 @@ func (s storage) RepoBatch(repos []*model.Repo) error {
 	}
 
 	for i := range repos {
-		if repos[i].UserID == 0 || len(repos[i].Owner) == 0 || len(repos[i].Name) == 0 || len(repos[i].FullName) == 0 {
-			log.Debug().Msgf("skip insert/update repo: %v", repos[i])
+		if len(repos[i].Owner) == 0 || len(repos[i].Name) == 0 || len(repos[i].FullName) == 0 {
+			log.Debug().Msgf("skip insert/update repo: %#v", repos[i])
 			continue
 		}
 		exist, err := sess.

diff --git a/server/store/datastore/repo_test.go b/server/store/datastore/repo_test.go
@@ -279,34 +279,45 @@ func TestRepoBatch(t *testing.T) {
 		return
 	}
 
-	if !assert.NoError(t, store.RepoBatch(
-		[]*model.Repo{
-			{
-				UserID:   1,
-				FullName: "foo/bar",
-				Owner:    "foo",
-				Name:     "bar",
-				IsActive: true,
-			},
-			{
-				UserID:   1,
-				FullName: "bar/baz",
-				Owner:    "bar",
-				Name:     "baz",
-				IsActive: true,
-			},
-			{
-				UserID:   1,
-				FullName: "baz/qux",
-				Owner:    "baz",
-				Name:     "qux",
-				IsActive: true,
-			},
+	repos := []*model.Repo{
+		{
+			UserID:   1,
+			FullName: "foo/bar",
+			Owner:    "foo",
+			Name:     "bar",
+			IsActive: true,
+		},
+		{
+			UserID:   1,
+			FullName: "bar/baz",
+			Owner:    "bar",
+			Name:     "baz",
+			IsActive: true,
+		},
+		{
+			UserID:   1,
+			FullName: "baz/qux",
+			Owner:    "baz",
+			Name:     "qux",
+			IsActive: true,
 		},
-	)) {
+		{
+			UserID:   0, // not activated repos do hot have a user id assigned
+			FullName: "baz/notes",
+			Owner:    "baz",
+			Name:     "notes",
+			IsActive: false,
+		},
+	}
+	if !assert.NoError(t, store.RepoBatch(repos)) {
 		return
 	}
 
+	allRepos := make([]*model.Repo, 0, 4)
+	err := store.engine.Find(&allRepos)
+	assert.NoError(t, err)
+	assert.Len(t, allRepos, 4)
+
 	count, err := store.GetRepoCount()
 	assert.NoError(t, err)
 	assert.EqualValues(t, 3, count)