From 9e67037a3f1008c53faa4285d13811b764845ee2 Mon Sep 17 00:00:00 2001 From: Shiny Nematoda Date: Mon, 9 Mar 2026 18:51:18 +0100 Subject: [PATCH] fix(issue-search): delete issue from indexer on DeleteIssue (#11585) Previously, issues were deleted from the indexer only when the repository was deleted. Individually deleting issues would not remove them from the indexer. Instead, they were merely hidden due to their IDs being absent from the DB. Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/11585 Reviewed-by: Gusted Co-authored-by: Shiny Nematoda Co-committed-by: Shiny Nematoda --- modules/indexer/issues/indexer.go | 13 ++++++++ modules/indexer/issues/indexer_test.go | 43 +++++++++++++++++++++++++- services/indexer/notify.go | 4 +++ services/issue/issue.go | 2 ++ services/notify/notifier.go | 1 + services/notify/notify.go | 7 +++++ services/notify/null.go | 4 +++ 7 files changed, 73 insertions(+), 1 deletion(-) diff --git a/modules/indexer/issues/indexer.go b/modules/indexer/issues/indexer.go index eebcc9a509..dd7102713c 100644 --- a/modules/indexer/issues/indexer.go +++ b/modules/indexer/issues/indexer.go @@ -262,6 +262,19 @@ func DeleteRepoIssueIndexer(ctx context.Context, repoID int64) { } } +// DeleteIssueIndexer deletes a single issue by it's ID +// +// NOTE: This does not perform any DB validation. +// Hence, the issueID does not need to be present in the DB. +func DeleteIssueIndexer(ctx context.Context, issueID int64) { + if err := pushIssueIndexerQueue(ctx, &IndexerMetadata{ + IDs: []int64{issueID}, + IsDelete: true, + }); err != nil { + log.Error("Unable to push deleted issue %d to issue indexer: %v", issueID, err) + } +} + // IsAvailable checks if issue indexer is available func IsAvailable(ctx context.Context) bool { return (*globalIndexer.Load()).Ping(ctx) == nil diff --git a/modules/indexer/issues/indexer_test.go b/modules/indexer/issues/indexer_test.go index 3a913d8e79..2d7aa71236 100644 --- a/modules/indexer/issues/indexer_test.go +++ b/modules/indexer/issues/indexer_test.go @@ -4,7 +4,11 @@ package issues import ( + "path/filepath" + "slices" + "strings" "testing" + "time" "forgejo.org/models/db" "forgejo.org/models/issues" @@ -12,6 +16,7 @@ import ( "forgejo.org/modules/indexer/issues/internal" "forgejo.org/modules/optional" "forgejo.org/modules/setting" + "forgejo.org/modules/test" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -24,7 +29,7 @@ func TestMain(m *testing.M) { func TestDBSearchIssues(t *testing.T) { require.NoError(t, unittest.PrepareTestDatabase()) - setting.Indexer.IssueType = "db" + defer test.MockVariableValue(&setting.Indexer.IssueType, "db")() InitIssueIndexer(true) t.Run("search issues with keyword", searchIssueWithKeyword) @@ -414,3 +419,39 @@ func searchIssueWithPaginator(t *testing.T) { assert.Equal(t, test.expectedTotal, total) } } + +func TestBleveDeleteIssue(t *testing.T) { + require.NoError(t, unittest.PrepareTestDatabase()) + + tmp := t.TempDir() + defer test.MockVariableValue(&setting.Indexer.IssuePath, filepath.Join(tmp, "indexers/issues.bleve"))() + defer test.MockVariableValue(&setting.Indexer.IssueType, "bleve")() + InitIssueIndexer(false) + + ctx := t.Context() + issue := unittest.AssertExistsAndLoadBean(t, &issues.Issue{ID: 1}) + UpdateIssueIndexer(ctx, issue.ID) + + opts := &internal.SearchOptions{ + RepoIDs: []int64{issue.RepoID}, + } + opts.WithKeyword(ctx, "first") + + assert.Eventually(t, func() bool { + ids, _, err := SearchIssues(ctx, opts) + if err != nil && strings.Contains(err.Error(), "not ready") { + return false + } + assert.NoError(t, err) + + assert.NoError(t, err) + return slices.Contains(ids, issue.ID) + }, time.Second*5, time.Millisecond*100, "failed to update issue") + + DeleteIssueIndexer(ctx, issue.ID) + assert.Eventually(t, func() bool { + ids, _, err := SearchIssues(ctx, opts) + assert.NoError(t, err) + return !slices.Contains(ids, issue.ID) + }, time.Second*5, time.Millisecond*100, "failed to delete issue") +} diff --git a/services/indexer/notify.go b/services/indexer/notify.go index ddd89f733c..8362348a62 100644 --- a/services/indexer/notify.go +++ b/services/indexer/notify.go @@ -74,6 +74,10 @@ func (r *indexerNotifier) DeleteRepository(ctx context.Context, doer *user_model } } +func (r indexerNotifier) DeleteIssue(ctx context.Context, doer *user_model.User, issue *issues_model.Issue) { + issue_indexer.DeleteIssueIndexer(ctx, issue.ID) +} + func (r *indexerNotifier) MigrateRepository(ctx context.Context, doer, u *user_model.User, repo *repo_model.Repository) { issue_indexer.UpdateRepoIndexer(ctx, repo.ID) if setting.Indexer.RepoIndexerEnabled && !repo.IsEmpty { diff --git a/services/issue/issue.go b/services/issue/issue.go index 7a65d87cd4..ab42916017 100644 --- a/services/issue/issue.go +++ b/services/issue/issue.go @@ -198,6 +198,8 @@ func DeleteIssue(ctx context.Context, doer *user_model.User, gitRepo *git.Reposi } } + notify_service.DeleteIssue(ctx, doer, issue) + return nil } diff --git a/services/notify/notifier.go b/services/notify/notifier.go index e6598a9377..4d88a7ab95 100644 --- a/services/notify/notifier.go +++ b/services/notify/notifier.go @@ -30,6 +30,7 @@ type Notifier interface { NewIssue(ctx context.Context, issue *issues_model.Issue, mentions []*user_model.User) IssueChangeStatus(ctx context.Context, doer *user_model.User, commitID string, issue *issues_model.Issue, actionComment *issues_model.Comment, closeOrReopen bool) + DeleteIssue(ctx context.Context, doer *user_model.User, issue *issues_model.Issue) IssueChangeMilestone(ctx context.Context, doer *user_model.User, issue *issues_model.Issue, oldMilestoneID int64) IssueChangeAssignee(ctx context.Context, doer *user_model.User, issue *issues_model.Issue, assignee *user_model.User, removed bool, comment *issues_model.Comment) PullRequestReviewRequest(ctx context.Context, doer *user_model.User, issue *issues_model.Issue, reviewer *user_model.User, isRequest bool, comment *issues_model.Comment) diff --git a/services/notify/notify.go b/services/notify/notify.go index 9fdde430c9..3ca704f717 100644 --- a/services/notify/notify.go +++ b/services/notify/notify.go @@ -76,6 +76,13 @@ func IssueChangeStatus(ctx context.Context, doer *user_model.User, commitID stri } } +// DeleteIssue notify when some issue deleted +func DeleteIssue(ctx context.Context, doer *user_model.User, issue *issues_model.Issue) { + for _, notifier := range notifiers { + notifier.DeleteIssue(ctx, doer, issue) + } +} + // MergePullRequest notifies merge pull request to notifiers func MergePullRequest(ctx context.Context, doer *user_model.User, pr *issues_model.PullRequest) { for _, notifier := range notifiers { diff --git a/services/notify/null.go b/services/notify/null.go index c0265e2a3e..9c76e5cbd3 100644 --- a/services/notify/null.go +++ b/services/notify/null.go @@ -37,6 +37,10 @@ func (*NullNotifier) NewIssue(ctx context.Context, issue *issues_model.Issue, me func (*NullNotifier) IssueChangeStatus(ctx context.Context, doer *user_model.User, commitID string, issue *issues_model.Issue, actionComment *issues_model.Comment, isClosed bool) { } +// DeleteIssue notify when some issue deleted +func (*NullNotifier) DeleteIssue(ctx context.Context, doer *user_model.User, issue *issues_model.Issue) { +} + // NewPullRequest places a place holder function func (*NullNotifier) NewPullRequest(ctx context.Context, pr *issues_model.PullRequest, mentions []*user_model.User) { }