From 611f2c5478f508128ef762a648af4db1c8b4b45e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lauris=20Buk=C5=A1is-Haberkorns?= Date: Wed, 29 Jan 2020 09:22:27 +0200 Subject: [PATCH 1/3] Update topics repo count when deleting repository --- models/repo.go | 6 ++++++ models/topic.go | 24 +++++++++++++++++++++--- 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/models/repo.go b/models/repo.go index 5528a8e9d2405..2d0f6fccbdf64 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 1a76c4915643a..4a5bffa08a5ce 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 } From c03e90fb1ce716aaacddd9a7b03cec114e46cacf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lauris=20Buk=C5=A1is-Haberkorns?= Date: Wed, 29 Jan 2020 09:45:12 +0200 Subject: [PATCH 2/3] Add migration to fix incorrect data --- models/migrations/migrations.go | 2 ++ models/migrations/v126.go | 49 +++++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+) create mode 100644 models/migrations/v126.go diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index 286e809abb232..99104906be67b 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 0000000000000..6155ed7b7f8ed --- /dev/null +++ b/models/migrations/v126.go @@ -0,0 +1,49 @@ +// 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 { + type Topic struct { + ID int64 + RepoCount int + } + + _, err := x.Exec(builder.Delete(builder.NotIn("`repo_id`", builder.Select("`id`").From("`repository`"))).From("`repo_topic`")) + if err != nil { + return err + } + + var last int + const batchSize = 50 + for { + var results = make([]Topic, 0, batchSize) + err := x.OrderBy("id"). + Limit(batchSize, last). + Find(&results) + if err != nil { + return err + } + if len(results) == 0 { + break + } + last += len(results) + + for _, res := range results { + _, err := x.ID(res.ID).Cols("repo_count"). + SetExpr("repo_count", builder.Select("count(*)").From("repo_topic").Where( + builder.Eq{"topic_id": res.ID}, + )).Update(res) + if err != nil { + return err + } + } + } + return nil +} From 50b6963d1b80329b09022daaa59f3d45a2a054c6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lauris=20Buk=C5=A1is-Haberkorns?= Date: Thu, 30 Jan 2020 18:53:53 +0200 Subject: [PATCH 3/3] Optimize to use single update to recalculate values --- models/migrations/v126.go | 38 +++++++------------------------------- 1 file changed, 7 insertions(+), 31 deletions(-) diff --git a/models/migrations/v126.go b/models/migrations/v126.go index 6155ed7b7f8ed..bce13a61bd963 100644 --- a/models/migrations/v126.go +++ b/models/migrations/v126.go @@ -10,40 +10,16 @@ import ( ) func fixTopicRepositoryCount(x *xorm.Engine) error { - type Topic struct { - ID int64 - RepoCount int - } - _, err := x.Exec(builder.Delete(builder.NotIn("`repo_id`", builder.Select("`id`").From("`repository`"))).From("`repo_topic`")) if err != nil { return err } - var last int - const batchSize = 50 - for { - var results = make([]Topic, 0, batchSize) - err := x.OrderBy("id"). - Limit(batchSize, last). - Find(&results) - if err != nil { - return err - } - if len(results) == 0 { - break - } - last += len(results) - - for _, res := range results { - _, err := x.ID(res.ID).Cols("repo_count"). - SetExpr("repo_count", builder.Select("count(*)").From("repo_topic").Where( - builder.Eq{"topic_id": res.ID}, - )).Update(res) - if err != nil { - return err - } - } - } - return nil + _, 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 }