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 <gusted@noreply.codeberg.org>
Co-authored-by: Shiny Nematoda <snematoda.751k2@aleeas.com>
Co-committed-by: Shiny Nematoda <snematoda.751k2@aleeas.com>

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

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")
+}

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 {

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
 }
 

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)

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 {

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) {
 }