Update topics repo count when deleting repository (#10051)

* Update topics repo count when deleting repository

* Add migration to fix incorrect data

* Optimize to use single update to recalculate values

4 files changed, 54 insertions, 3 deletions

diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go
index 286e809abb..99104906be 100644
--- a/models/migrations/migrations.go
+++ b/models/migrations/migrations.go
@@ -306,6 +306,8 @@ var migrations = []Migration{
 	NewMigration("Add columns to user and repository", addUserRepoMissingColumns),
 	// v125 -> v126
 	NewMigration("Add some columns on review for migration", addReviewMigrateInfo),
+	// v126 -> v127
+	NewMigration("Fix topic repository count", fixTopicRepositoryCount),
 }
 
 // Migrate database to current version
diff --git a/models/migrations/v126.go b/models/migrations/v126.go
new file mode 100644
index 0000000000..bce13a61bd
--- /dev/null
+++ b/models/migrations/v126.go
@@ -0,0 +1,25 @@
+// Copyright 2020 The Gitea Authors. All rights reserved.
+// Use of this source code is governed by a MIT-style
+// license that can be found in the LICENSE file.
+
+package migrations
+
+import (
+	"xorm.io/builder"
+	"xorm.io/xorm"
+)
+
+func fixTopicRepositoryCount(x *xorm.Engine) error {
+	_, err := x.Exec(builder.Delete(builder.NotIn("`repo_id`", builder.Select("`id`").From("`repository`"))).From("`repo_topic`"))
+	if err != nil {
+		return err
+	}
+
+	_, err = x.Exec(builder.Update(
+		builder.Eq{
+			"`repo_count`": builder.Select("count(*)").From("`repo_topic`").Where(builder.Eq{
+				"`repo_topic`.`topic_id`": builder.Expr("`topic`.`id`"),
+			}),
+		}).From("`topic`").Where(builder.Eq{"'1'": "1"}))
+	return err
+}
diff --git a/models/repo.go b/models/repo.go
index 5528a8e9d2..2d0f6fccbd 100644
--- a/models/repo.go
+++ b/models/repo.go
@@ -1571,6 +1571,12 @@ func DeleteRepository(doer *User, uid, repoID int64) error {
 		return err
 	}
 
+	if len(repo.Topics) > 0 {
+		if err = removeTopicsFromRepo(sess, repo.ID); err != nil {
+			return err
+		}
+	}
+
 	// FIXME: Remove repository files should be executed after transaction succeed.
 	repoPath := repo.RepoPath()
 	removeAllWithNotice(sess, "Delete repository files", repoPath)
diff --git a/models/topic.go b/models/topic.go
index 1a76c49156..4a5bffa08a 100644
--- a/models/topic.go
+++ b/models/topic.go
@@ -129,7 +129,7 @@ func addTopicByNameToRepo(e Engine, repoID int64, topicName string) (*Topic, err
 }
 
 // removeTopicFromRepo remove a topic from a repo and decrements the topic repo count
-func removeTopicFromRepo(repoID int64, topic *Topic, e Engine) error {
+func removeTopicFromRepo(e Engine, repoID int64, topic *Topic) error {
 	topic.RepoCount--
 	if _, err := e.ID(topic.ID).Cols("repo_count").Update(topic); err != nil {
 		return err
@@ -145,6 +145,24 @@ func removeTopicFromRepo(repoID int64, topic *Topic, e Engine) error {
 	return nil
 }
 
+// removeTopicsFromRepo remove all topics from the repo and decrements respective topics repo count
+func removeTopicsFromRepo(e Engine, repoID int64) error {
+	_, err := e.Where(
+		builder.In("id",
+			builder.Select("topic_id").From("repo_topic").Where(builder.Eq{"repo_id": repoID}),
+		),
+	).Cols("repo_count").SetExpr("repo_count", "repo_count-1").Update(&Topic{})
+	if err != nil {
+		return err
+	}
+
+	if _, err = e.Delete(&RepoTopic{RepoID: repoID}); err != nil {
+		return err
+	}
+
+	return nil
+}
+
 // FindTopicOptions represents the options when fdin topics
 type FindTopicOptions struct {
 	ListOptions
@@ -216,7 +234,7 @@ func DeleteTopic(repoID int64, topicName string) (*Topic, error) {
 		return nil, nil
 	}
 
-	err = removeTopicFromRepo(repoID, topic, x)
+	err = removeTopicFromRepo(x, repoID, topic)
 
 	return topic, err
 }
@@ -277,7 +295,7 @@ func SaveTopics(repoID int64, topicNames ...string) error {
 	}
 
 	for _, topic := range removeTopics {
-		err := removeTopicFromRepo(repoID, topic, sess)
+		err := removeTopicFromRepo(sess, repoID, topic)
 		if err != nil {
 			return err
 		}