Merge branch '12578-fix-search-for-deleted-projects' into 'master'

Skip ES commit results for deleted projects

See merge request gitlab-org/gitlab-ee!15236

ee/app/models/concerns/elastic/repositories_search.rb

@@ -55,6 +55,8 @@ module Elastic
           options: options
         )[:commits][:results]
 
+        response_count = response.total_count
+
         # Avoid one SELECT per result by loading all projects into a hash
         project_ids = response.map {|result| result["_source"]["commit"]["rid"] }.uniq
         projects = Project.includes(:route).where(id: project_ids).index_by(&:id)
@@ -62,6 +64,12 @@ module Elastic
         commits = response.map do |result|
           project_id = result["_source"]["commit"]["rid"].to_i
           project = projects[project_id]
+
+          if project.nil? || project.pending_delete?
+            response_count -= 1
+            next
+          end
+
           raw_commit = Gitlab::Git::Commit.new(
             project.repository.raw,
             prepare_commit(result['_source']['commit']),
@@ -70,9 +78,12 @@ module Elastic
           Commit.new(raw_commit, project)
         end
 
+        # Remove results for deleted projects
+        commits.compact!
+
         # Before "map" we had a paginated array so we need to recover it
         offset = per_page * ((page || 1) - 1)
-        Kaminari.paginate_array(commits, total_count: response.total_count, limit: per_page, offset: offset)
+        Kaminari.paginate_array(commits, total_count: response_count, limit: per_page, offset: offset)
       end
 
       def prepare_commit(raw_commit_hash)

ee/changelogs/unreleased/12578-fix-search-for-deleted-projects.yml

+---
+title: Skip ES commit results for deleted projects
+merge_request: 15236
+author:
+type: fixed

ee/spec/models/concerns/elastic/repository_spec.rb

@@ -76,19 +76,47 @@ describe Repository, :elastic do
     search_and_check!(project.repository, '-foo', type: :commit)
   end
 
-  describe "class method find_commits_by_message_with_elastic" do
-    it "returns commits" do
-      project = create :project, :repository
-      project1 = create :project, :repository
+  describe 'class method find_commits_by_message_with_elastic' do
+    let(:project) { create :project, :repository }
+    let(:project1) { create :project, :repository }
+    let(:results) { Repository.find_commits_by_message_with_elastic('initial') }
 
+    before do
       project.repository.index_commits
       project1.repository.index_commits
 
       Gitlab::Elastic::Helper.refresh_index
+    end
+
+    it 'returns commits' do
+      expect(results).to contain_exactly(instance_of(Commit), instance_of(Commit))
+      expect(results.count).to eq(2)
+      expect(results.total_count).to eq(2)
+    end
+
+    context 'with a deleted project' do
+      before do
+        # Call DELETE directly to avoid triggering our callback to clear the ES index
+        project.delete
+      end
+
+      it 'skips its commits' do
+        expect(results).to contain_exactly(instance_of(Commit))
+        expect(results.count).to eq(1)
+        expect(results.total_count).to eq(1)
+      end
+    end
+
+    context 'with a project pending deletion' do
+      before do
+        project.update!(pending_delete: true)
+      end
 
-      expect(Repository.find_commits_by_message_with_elastic('initial').first).to be_a(Commit)
-      expect(Repository.find_commits_by_message_with_elastic('initial').count).to eq(2)
-      expect(Repository.find_commits_by_message_with_elastic('initial').total_count).to eq(2)
+      it 'skips its commits' do
+        expect(results).to contain_exactly(instance_of(Commit))
+        expect(results.count).to eq(1)
+        expect(results.total_count).to eq(1)
+      end
     end
   end