Extract generic Gitlab::Search::RecentItems

This is the first step to support recent merge requests as we can reuse
all of this logic.

lib/gitlab/search/recent_issues.rb

@@ -2,51 +2,16 @@
 
 module Gitlab
   module Search
-    class RecentIssues
-      ITEMS_LIMIT = 100
-      EXPIRES_AFTER = 7.days
-
-      def initialize(user:, items_limit: ITEMS_LIMIT, expires_after: EXPIRES_AFTER)
-        @user = user
-        @items_limit = items_limit
-        @expires_after = expires_after
-      end
-
-      def log_view(issue)
-        with_redis do |redis|
-          redis.zadd(key, Time.now.to_f, issue.id)
-          redis.expire(key, @expires_after)
-
-          # There is a race condition here where we could end up removing an
-          # item from 2 places concurrently but this is fine since worst case
-          # scenario we remove an extra item from the end of the list.
-          if redis.zcard(key) > @items_limit
-            redis.zremrangebyrank(key, 0, 0) # Remove least recent
-          end
-        end
-      end
-
-      def search(term)
-        ids = with_redis do |redis|
-          redis.zrevrange(key, 0, @items_limit - 1)
-        end.map(&:to_i)
-
-        IssuesFinder.new(@user, search: term, in: 'title').execute.reorder(nil).id_in_ordered(ids) # rubocop: disable CodeReuse/ActiveRecord
-      end
-
+    class RecentIssues < RecentItems
       private
 
-      def with_redis(&blk)
-        Gitlab::Redis::SharedState.with(&blk) # rubocop: disable CodeReuse/ActiveRecord
-      end
-
-      def key
-        "recent_items:#{type.name.downcase}:#{@user.id}"
-      end
-
       def type
         Issue
       end
+
+      def finder
+        IssuesFinder
+      end
     end
   end
 end

lib/gitlab/search/recent_items.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Search
+    # This is an abstract class used for storing/searching recently viewed
+    # items. The #type and #finder methods are the only ones needed to be
+    # implemented by classes inheriting from this.
+    class RecentItems
+      ITEMS_LIMIT = 100
+      EXPIRES_AFTER = 7.days
+
+      def initialize(user:, items_limit: ITEMS_LIMIT, expires_after: EXPIRES_AFTER)
+        @user = user
+        @items_limit = items_limit
+        @expires_after = expires_after
+      end
+
+      def log_view(item)
+        with_redis do |redis|
+          redis.zadd(key, Time.now.to_f, item.id)
+          redis.expire(key, @expires_after)
+
+          # There is a race condition here where we could end up removing an
+          # item from 2 places concurrently but this is fine since worst case
+          # scenario we remove an extra item from the end of the list.
+          if redis.zcard(key) > @items_limit
+            redis.zremrangebyrank(key, 0, 0) # Remove least recent
+          end
+        end
+      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
+      end
+
+      private
+
+      def with_redis(&blk)
+        Gitlab::Redis::SharedState.with(&blk) # rubocop: disable CodeReuse/ActiveRecord
+      end
+
+      def key
+        "recent_items:#{type.name.downcase}:#{@user.id}"
+      end
+
+      def type
+        raise NotImplementedError
+      end
+
+      def finder
+        raise NotImplementedError
+      end
+    end
+  end
+end

spec/lib/gitlab/search/recent_issues_spec.rb

@@ -2,86 +2,10 @@
 
 require 'spec_helper'
 
-RSpec.describe ::Gitlab::Search::RecentIssues, :clean_gitlab_redis_shared_state do
-  let(:user) { create(:user) }
-  let(:issue) { create(:issue, title: 'hello world 1', project: project) }
-  let(:recent_issues) { described_class.new(user: user, items_limit: 5) }
-  let(:project) { create(:project, :public) }
-
-  describe '#log_viewing' do
-    it 'adds the item to the recent items' do
-      recent_issues.log_view(issue)
-
-      results = recent_issues.search('hello')
-
-      expect(results).to eq([issue])
-    end
-
-    it 'removes an item when it exceeds the size items_limit' do
-      (1..6).each do |i|
-        recent_issues.log_view(create(:issue, title: "issue #{i}", project: project))
-      end
-
-      results = recent_issues.search('issue')
-
-      expect(results.map(&:title)).to contain_exactly('issue 6', 'issue 5', 'issue 4', 'issue 3', 'issue 2')
-    end
-
-    it 'expires the items after expires_after' do
-      recent_issues = described_class.new(user: user, expires_after: 0)
-
-      recent_issues.log_view(issue)
-
-      results = recent_issues.search('hello')
-
-      expect(results).to be_empty
-    end
-
-    it 'does not include results logged for another user' do
-      another_user = create(:user)
-      another_issue = create(:issue, title: 'hello world 2', project: project)
-      described_class.new(user: another_user).log_view(another_issue)
-      recent_issues.log_view(issue)
-
-      results = recent_issues.search('hello')
-
-      expect(results).to eq([issue])
-    end
+RSpec.describe ::Gitlab::Search::RecentIssues do
+  def create_item(content:, project:)
+    create(:issue, title: content, project: project)
   end
 
-  describe '#search' do
-    let(:issue1) { create(:issue, title: "matching issue 1", project: project) }
-    let(:issue2) { create(:issue, title: "matching issue 2", project: project) }
-    let(:issue3) { create(:issue, title: "matching issue 3", project: project) }
-    let(:non_matching_issue) { create(:issue, title: "different issue", project: project) }
-    let!(:non_viewed_issued) { create(:issue, title: "matching but not viewed issue", project: project) }
-
-    before do
-      recent_issues.log_view(issue1)
-      recent_issues.log_view(issue2)
-      recent_issues.log_view(issue3)
-      recent_issues.log_view(non_matching_issue)
-    end
-
-    it 'matches partial text in the issue title' do
-      expect(recent_issues.search('matching')).to contain_exactly(issue1, issue2, issue3)
-    end
-
-    it 'returns results sorted by recently viewed' do
-      recent_issues.log_view(issue2)
-
-      expect(recent_issues.search('matching')).to eq([issue2, issue3, issue1])
-    end
-
-    it 'does not leak issues you no longer have access to' do
-      private_project = create(:project, :public, namespace: create(:group))
-      private_issue = create(:issue, project: private_project, title: 'matching issue title')
-
-      recent_issues.log_view(private_issue)
-
-      private_project.update!(visibility_level: Project::PRIVATE)
-
-      expect(recent_issues.search('matching')).not_to include(private_issue)
-    end
-  end
+  it_behaves_like 'search recent items'
 end

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(:item) { create_item(content: 'hello world 1', project: project) }
+  let(:project) { create(:project, :public) }
+
+  describe '#log_view', :clean_gitlab_redis_shared_state do
+    it 'adds the item to the recent items' do
+      recent_items.log_view(item)
+
+      results = recent_items.search('hello')
+
+      expect(results).to eq([item])
+    end
+
+    it 'removes an item when it exceeds the size items_limit' do
+      (1..6).each do |i|
+        recent_items.log_view(create_item(content: "item #{i}", project: project))
+      end
+
+      results = recent_items.search('item')
+
+      expect(results.map(&:title)).to contain_exactly('item 6', 'item 5', 'item 4', 'item 3', 'item 2')
+    end
+
+    it 'expires the items after expires_after' do
+      recent_items = described_class.new(user: user, expires_after: 0)
+
+      recent_items.log_view(item)
+
+      results = recent_items.search('hello')
+
+      expect(results).to be_empty
+    end
+
+    it 'does not include results logged for another user' do
+      another_user = create(:user)
+      another_item = create_item(content: 'hello world 2', project: project)
+      described_class.new(user: another_user).log_view(another_item)
+      recent_items.log_view(item)
+
+      results = recent_items.search('hello')
+
+      expect(results).to eq([item])
+    end
+  end
+
+  describe '#search', :clean_gitlab_redis_shared_state do
+    let(:item1) { create_item(content: "matching item 1", project: project) }
+    let(:item2) { create_item(content: "matching item 2", project: project) }
+    let(:item3) { create_item(content: "matching item 3", project: project) }
+    let(:non_matching_item) { create_item(content: "different item", project: project) }
+    let!(:non_viewed_item) { create_item(content: "matching but not viewed item", project: project) }
+
+    before do
+      recent_items.log_view(item1)
+      recent_items.log_view(item2)
+      recent_items.log_view(item3)
+      recent_items.log_view(non_matching_item)
+    end
+
+    it 'matches partial text in the item title' do
+      expect(recent_items.search('matching')).to contain_exactly(item1, item2, item3)
+    end
+
+    it 'returns results sorted by recently viewed' do
+      recent_items.log_view(item2)
+
+      expect(recent_items.search('matching')).to eq([item2, item3, item1])
+    end
+
+    it 'does not leak items you no longer have access to' do
+      private_project = create(:project, :public, namespace: create(:group))
+      private_item = create_item(content: 'matching item title', project: private_project)
+
+      recent_items.log_view(private_item)
+
+      private_project.update!(visibility_level: Project::PRIVATE)
+
+      expect(recent_items.search('matching')).not_to include(private_item)
+    end
+  end
+end