Refactor RecentItems to prepare for epics

We add 2 refactors:

1. Add `attr_reader :user` to RecentItems because this will be used by
`RecentEpics` since the `EpicsFinder` cannot search outside of a single
group like the `IssuesFinder` does so we need to override `#search`
instead
2. Apply the limit=5 in the RecentItems module instead of the
SearchHelper caller. This is necessary for when we implement Epics
search because we'll need to do a permissions check on the limited items
before returning them

app/helpers/search_helper.rb

@@ -186,10 +186,10 @@ module SearchHelper
     end
   end
 
-  def recent_merge_requests_autocomplete(term, limit = 5)
+  def recent_merge_requests_autocomplete(term)
     return [] unless current_user
 
-    ::Gitlab::Search::RecentMergeRequests.new(user: current_user).search(term).limit(limit).map do |mr|
+    ::Gitlab::Search::RecentMergeRequests.new(user: current_user).search(term).map do |mr|
       {
         category: "Recent merge requests",
         id: mr.id,
@@ -200,10 +200,10 @@ module SearchHelper
     end
   end
 
-  def recent_issues_autocomplete(term, limit = 5)
+  def recent_issues_autocomplete(term)
     return [] unless current_user
 
-    ::Gitlab::Search::RecentIssues.new(user: current_user).search(term).limit(limit).map do |i|
+    ::Gitlab::Search::RecentIssues.new(user: current_user).search(term).map do |i|
       {
         category: "Recent issues",
         id: i.id,

lib/gitlab/search/recent_items.rb

@@ -6,9 +6,12 @@ module Gitlab
     # items. The #type and #finder methods are the only ones needed to be
     # implemented by classes inheriting from this.
     class RecentItems
-      ITEMS_LIMIT = 100
+      ITEMS_LIMIT = 100 # How much history to remember from the user
+      SEARCH_LIMIT = 5 # How many matching items to return from search
       EXPIRES_AFTER = 7.days
 
+      attr_reader :user
+
       def initialize(user:, items_limit: ITEMS_LIMIT, expires_after: EXPIRES_AFTER)
         @user = user
         @items_limit = items_limit
@@ -30,21 +33,25 @@ module Gitlab
       end
 
       def search(term)
-        ids = with_redis do |redis|
-          redis.zrevrange(key, 0, @items_limit - 1)
-        end.map(&:to_i)
-
-        finder.new(@user, search: term, in: 'title').execute.reorder(nil).id_in_ordered(ids) # rubocop: disable CodeReuse/ActiveRecord
+        finder.new(user, search: term, in: 'title')
+          .execute
+          .limit(SEARCH_LIMIT).reorder(nil).id_in_ordered(latest_ids) # rubocop: disable CodeReuse/ActiveRecord
       end
 
       private
 
+      def latest_ids
+        with_redis do |redis|
+          redis.zrevrange(key, 0, @items_limit - 1)
+        end.map(&:to_i)
+      end
+
       def with_redis(&blk)
         Gitlab::Redis::SharedState.with(&blk) # rubocop: disable CodeReuse/ActiveRecord
       end
 
       def key
-        "recent_items:#{type.name.downcase}:#{@user.id}"
+        "recent_items:#{type.name.downcase}:#{user.id}"
       end
 
       def type

spec/helpers/search_helper_spec.rb

@@ -73,7 +73,7 @@ RSpec.describe SearchHelper do
         expect(result.keys).to match_array(%i[category id label url avatar_url])
       end
 
-      it 'includes the first 5 of the users recent issues' do
+      it 'includes the users recent issues' do
         recent_issues = instance_double(::Gitlab::Search::RecentIssues)
         expect(::Gitlab::Search::RecentIssues).to receive(:new).with(user: user).and_return(recent_issues)
         project1 = create(:project, :with_avatar, namespace: user.namespace)
@@ -81,13 +81,11 @@ RSpec.describe SearchHelper do
         issue1 = create(:issue, title: 'issue 1', project: project1)
         issue2 = create(:issue, title: 'issue 2', project: project2)
 
-        other_issues = create_list(:issue, 5)
-
-        expect(recent_issues).to receive(:search).with('the search term').and_return(Issue.id_in_ordered([issue1.id, issue2.id, *other_issues.map(&:id)]))
+        expect(recent_issues).to receive(:search).with('the search term').and_return(Issue.id_in_ordered([issue1.id, issue2.id]))
 
         results = search_autocomplete_opts("the search term")
 
-        expect(results.count).to eq(5)
+        expect(results.count).to eq(2)
 
         expect(results[0]).to include({
           category: 'Recent issues',
@@ -106,7 +104,7 @@ RSpec.describe SearchHelper do
         })
       end
 
-      it 'includes the first 5 of the users recent merge requests' do
+      it 'includes the users recent merge requests' do
         recent_merge_requests = instance_double(::Gitlab::Search::RecentMergeRequests)
         expect(::Gitlab::Search::RecentMergeRequests).to receive(:new).with(user: user).and_return(recent_merge_requests)
         project1 = create(:project, :with_avatar, namespace: user.namespace)
@@ -114,13 +112,11 @@ RSpec.describe SearchHelper do
         merge_request1 = create(:merge_request, :unique_branches, title: 'Merge request 1', target_project: project1, source_project: project1)
         merge_request2 = create(:merge_request, :unique_branches, title: 'Merge request 2', target_project: project2, source_project: project2)
 
-        other_merge_requests = create_list(:merge_request, 5)
-
-        expect(recent_merge_requests).to receive(:search).with('the search term').and_return(MergeRequest.id_in_ordered([merge_request1.id, merge_request2.id, *other_merge_requests.map(&:id)]))
+        expect(recent_merge_requests).to receive(:search).with('the search term').and_return(MergeRequest.id_in_ordered([merge_request1.id, merge_request2.id]))
 
         results = search_autocomplete_opts("the search term")
 
-        expect(results.count).to eq(5)
+        expect(results.count).to eq(2)
 
         expect(results[0]).to include({
           category: 'Recent merge requests',

spec/support/shared_examples/lib/gitlab/search/recent_items.rb

 # frozen_string_literal: true
 
 require 'spec_helper'
-
 RSpec.shared_examples 'search recent items' do
   let_it_be(:user) { create(:user) }
-  let_it_be(:recent_items) { described_class.new(user: user, items_limit: 5) }
+  let_it_be(:recent_items) { described_class.new(user: user) }
   let(:item) { create_item(content: 'hello world 1', parent: parent) }
   let(:parent) { create(parent_type, :public) }
 
@@ -18,13 +17,15 @@ RSpec.shared_examples 'search recent items' do
     end
 
     it 'removes an item when it exceeds the size items_limit' do
-      (1..6).each do |i|
+      recent_items = described_class.new(user: user, items_limit: 3)
+
+      4.times do |i|
         recent_items.log_view(create_item(content: "item #{i}", parent: parent))
       end
 
       results = recent_items.search('item')
 
-      expect(results.map(&:title)).to contain_exactly('item 6', 'item 5', 'item 4', 'item 3', 'item 2')
+      expect(results.map(&:title)).to contain_exactly('item 3', 'item 2', 'item 1')
     end
 
     it 'expires the items after expires_after' do
@@ -83,5 +84,15 @@ RSpec.shared_examples 'search recent items' do
 
       expect(recent_items.search('matching')).not_to include(private_item)
     end
+
+    it "limits results to #{Gitlab::Search::RecentItems::SEARCH_LIMIT} items" do
+      (Gitlab::Search::RecentItems::SEARCH_LIMIT + 1).times do |i|
+        recent_items.log_view(create_item(content: "item #{i}", parent: parent))
+      end
+
+      results = recent_items.search('item')
+
+      expect(results.count).to eq(Gitlab::Search::RecentItems::SEARCH_LIMIT)
+    end
   end
 end