Merge branch '239386-fix-scroll-window-code-search' into 'master'

Fix the scroll position in code search results

See merge request gitlab-org/gitlab!43083

ee/changelogs/unreleased/239386-fix-scroll-window-code-search.yml

+---
+title: Fix the scroll position in code search results
+merge_request: 43083
+author:
+type: fixed

ee/lib/elastic/latest/git_class_proxy.rb

@@ -4,6 +4,8 @@ module Elastic
   module Latest
     module GitClassProxy
       SHA_REGEX = /\A[0-9a-f]{5,40}\z/i.freeze
+      HIGHLIGHT_START_TAG = 'gitlabelasticsearch→'
+      HIGHLIGHT_END_TAG = '←gitlabelasticsearch'
 
       def elastic_search(query, type: 'all', page: 1, per: 20, options: {})
         results = { blobs: [], commits: [] }
@@ -95,8 +97,8 @@ module Elastic
           end
 
           query_hash[:highlight] = {
-            pre_tags: ["gitlabelasticsearch→"],
-            post_tags: ["←gitlabelasticsearch"],
+            pre_tags: [HIGHLIGHT_START_TAG],
+            post_tags: [HIGHLIGHT_END_TAG],
             fields: es_fields
           }
         end
@@ -157,9 +159,9 @@ module Elastic
 
         if options[:highlight]
           query_hash[:highlight] = {
-            pre_tags: ["gitlabelasticsearch→"],
-            post_tags: ["←gitlabelasticsearch"],
-            order: "score",
+            pre_tags: [HIGHLIGHT_START_TAG],
+            post_tags: [HIGHLIGHT_END_TAG],
+            number_of_fragments: 0, # highlighted text fragments do not work well for code as we want to show a few whole lines of code. We need to get the whole content to determine the exact line number that was highlighted.
             fields: {
               "blob.content" => {},
               "blob.file_name" => {}

ee/lib/gitlab/elastic/search_results.rb

@@ -123,16 +123,12 @@ module Gitlab
         project_id = result['_source']['project_id'].to_i
         total_lines = content.lines.size
 
-        term =
-          if result['highlight']
-            highlighted = result['highlight']['blob.content']
-            highlighted && highlighted[0].match(/gitlabelasticsearch→(.*?)←gitlabelasticsearch/)[1]
-          end
+        highlight_content = result.dig('highlight', 'blob.content')&.first || ''
 
         found_line_number = 0
 
-        content.each_line.each_with_index do |line, index|
-          if term && line.include?(term)
+        highlight_content.each_line.each_with_index do |line, index|
+          if line.include?(::Elastic::Latest::GitClassProxy::HIGHLIGHT_START_TAG)
             found_line_number = index
             break
           end

ee/spec/lib/gitlab/elastic/search_results_spec.rb

@@ -58,11 +58,12 @@ RSpec.describe Gitlab::Elastic::SearchResults, :elastic, :sidekiq_might_not_need
 
   describe 'parse_search_result' do
     let(:project) { double(:project) }
+    let(:content) { "foo\nbar\nbaz\n" }
     let(:blob) do
       {
         'blob' => {
           'commit_sha' => 'sha',
-          'content' => "foo\nbar\nbaz\n",
+          'content' => content,
           'path' => 'path/file.ext'
         }
       }
@@ -83,7 +84,7 @@ RSpec.describe Gitlab::Elastic::SearchResults, :elastic, :sidekiq_might_not_need
       result = {
         '_source' => blob,
         'highlight' => {
-          'blob.content' => ["foo\ngitlabelasticsearch→bar←gitlabelasticsearch\nbaz\n"]
+          'blob.content' => ["foo\n#{::Elastic::Latest::GitClassProxy::HIGHLIGHT_START_TAG}bar#{::Elastic::Latest::GitClassProxy::HIGHLIGHT_END_TAG}\nbaz\n"]
         }
       }
 
@@ -100,6 +101,60 @@ RSpec.describe Gitlab::Elastic::SearchResults, :elastic, :sidekiq_might_not_need
         data: "bar\n"
       )
     end
+
+    context 'when the highlighting finds the same terms multiple times' do
+      let(:content) do
+        <<~END
+        bar
+        bar
+        foo
+        bar # this is the highlighted bar
+        baz
+        boo
+        bar
+        END
+      end
+
+      it 'does not mistake a line that happens to include the same term that was highlighted on a later line' do
+        highlighted_content = <<~END
+        bar
+        bar
+        foo
+        #{::Elastic::Latest::GitClassProxy::HIGHLIGHT_START_TAG}bar#{::Elastic::Latest::GitClassProxy::HIGHLIGHT_END_TAG} # this is the highlighted bar
+        baz
+        boo
+        bar
+        END
+
+        result = {
+          '_source' => blob,
+          'highlight' => {
+            'blob.content' => [highlighted_content]
+          }
+        }
+
+        parsed = described_class.parse_search_result(result, project)
+
+        expected_data = <<~END
+        bar
+        foo
+        bar # this is the highlighted bar
+        baz
+        boo
+        END
+
+        expect(parsed).to be_kind_of(::Gitlab::Search::FoundBlob)
+        expect(parsed).to have_attributes(
+          id: nil,
+          path: 'path/file.ext',
+          basename: 'path/file',
+          ref: 'sha',
+          startline: 2,
+          project: project,
+          data: expected_data
+        )
+      end
+    end
   end
 
   describe 'issues' do
@@ -571,7 +626,7 @@ RSpec.describe Gitlab::Elastic::SearchResults, :elastic, :sidekiq_might_not_need
       results = described_class.new(user, 'defau*', limit_project_ids)
       blobs = results.objects('blobs')
 
-      expect(blobs.first.data).to include('default')
+      expect(blobs.first.data).to match(/default/i)
       expect(results.blobs_count).to eq 3
     end