Merge pull request '[GITEA] Use join for the deleting issue actions query' (#1165) from Gusted/forgejo:backport-1154 into v1.20/forgejo

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/1165

models/activities/action.go

@@ -687,11 +687,21 @@ func NotifyWatchersActions(acts []*Action) error {
 // DeleteIssueActions delete all actions related with issueID
 func DeleteIssueActions(ctx context.Context, repoID, issueID int64) error {
 	// delete actions assigned to this issue
-	subQuery := builder.Select("`id`").
+
-		From("`comment`").
+	// MySQL doesn't use the indexes on comment_id when using a subquery.
-		Where(builder.Eq{"`issue_id`": issueID})
+	// It does uses the indexes when using an JOIN, however SQLite doesn't
-	if _, err := db.GetEngine(ctx).In("comment_id", subQuery).Delete(&Action{}); err != nil {
+	// allow JOINs in DELETE statements and XORM doesn't allow them as well.
-		return err
+	// So, an specific raw SQL query for MySQL so the query makes use of indexes.
+	if setting.Database.Type.IsMySQL() {
+		if _, err := db.GetEngine(ctx).Exec("DELETE action FROM action JOIN comment ON action.comment_id = comment.id WHERE comment.issue_id = ?", issueID); err != nil {
+			return err
+		}
+	} else {
+		subQuery := builder.Select("`id`").From("`comment`").
+			Where(builder.Eq{"`issue_id`": issueID})
+		if _, err := db.GetEngine(ctx).In("comment_id", subQuery).Delete(&Action{}); err != nil {
+			return err
+		}
 	}
 
 	_, err := db.GetEngine(ctx).Table("action").Where("repo_id = ?", repoID).

models/activities/action_test.go

@@ -242,6 +242,15 @@ func TestGetFeedsCorrupted(t *testing.T) {
 	assert.Equal(t, int64(0), count)
 }
 
+func TestDeleteIssueActions(t *testing.T) {
+	assert.NoError(t, unittest.PrepareTestDatabase())
+
+	comment := unittest.AssertExistsAndLoadBean(t, &issue_model.Comment{ID: 8})
+	unittest.AssertExistsAndLoadBean(t, &activities_model.Action{ID: 10, CommentID: comment.ID})
+	assert.NoError(t, activities_model.DeleteIssueActions(db.DefaultContext, 32, 17))
+	unittest.AssertNotExistsBean(t, &activities_model.Action{ID: 10, CommentID: comment.ID})
+}
+
 func TestConsistencyUpdateAction(t *testing.T) {
 	if !setting.Database.Type.IsSQLite3() {
 		t.Skip("Test is only for SQLite database.")

models/fixtures/action.yml

@@ -73,3 +73,13 @@
   is_private: false
   created_unix: 1680454039
   content: '4|' # issueId 5
+
+- id: 10
+  user_id: 15
+  op_type: 10 # issue comment
+  act_user_id: 15
+  repo_id: 32 # public
+  comment_id: 8
+  is_private: false
+  created_unix: 1680454039
+  content: '2|meh...' # issueId 5

models/fixtures/comment.yml

@@ -66,3 +66,12 @@
   tree_path: "README.md"
   created_unix: 946684812
   invalidated: true
+
+-
+  id: 8
+  type: 0 # comment
+  poster_id: 15
+  issue_id: 17 # in repo_id 32
+  content: "meh..."
+  created_unix: 946684812
+  updated_unix: 946684812

models/fixtures/issue.yml

@@ -283,7 +283,7 @@
   priority: 0
   is_closed: false
   is_pull: false
-  num_comments: 0
+  num_comments: 1
   created_unix: 1602935696
   updated_unix: 1602935696
   is_locked: false

tests/integration/api_nodeinfo_test.go

@@ -35,6 +35,6 @@ func TestNodeinfo(t *testing.T) {
 		assert.Equal(t, "forgejo", nodeinfo.Software.Name)
 		assert.Equal(t, 25, nodeinfo.Usage.Users.Total)
 		assert.Equal(t, 18, nodeinfo.Usage.LocalPosts)
-		assert.Equal(t, 2, nodeinfo.Usage.LocalComments)
+		assert.Equal(t, 3, nodeinfo.Usage.LocalComments)
 	})
 }