Implement recent issue autocomplete in search

This adds support for autocompleting issues that were recently viewed by
the current user when they use the search bar. This data is persisted in
Redis sorted sets as this provides the ability to automatically expire
the data for inactive users, keep the data sorted based on most recently
viewed first and easily keep a limited number of items per user expiring
the least recent ones first.

The recent item tracking is partly implemented as a generic solution as
this will quickly be followed up with merge requests and perhaps other
features people wish to quickly navigate to. The full generic
refactoring will happen when we implement the 2nd usage but for now the
most important thing is to include the data type in the Redis key so
that we don't need to migrate data when we make this generic later.

This feature is behind a feature flag per user for now so we can track
the performance implications in production.

app/controllers/projects/issues_controller.rb

@@ -26,6 +26,7 @@ class Projects::IssuesController < Projects::ApplicationController
   before_action :whitelist_query_limiting, only: [:create, :create_merge_request, :move, :bulk_update]
   before_action :check_issues_available!
   before_action :issue, unless: ->(c) { c.issue_except_actions.include?(c.action_name.to_sym) }
+  after_action :log_issue_show, unless: ->(c) { c.issue_except_actions.include?(c.action_name.to_sym) }
 
   before_action :set_issuables_index, if: ->(c) { c.set_issuables_index_only_actions.include?(c.action_name.to_sym) }
 
@@ -249,6 +250,13 @@ class Projects::IssuesController < Projects::ApplicationController
     @issue
   end
   # rubocop: enable CodeReuse/ActiveRecord
+
+  def log_issue_show
+    return unless current_user && @issue
+
+    ::Gitlab::Search::RecentIssues.new(user: current_user).log_view(@issue)
+  end
+
   alias_method :subscribable_resource, :issue
   alias_method :issuable, :issue
   alias_method :awardable, :issue

app/helpers/search_helper.rb

@@ -7,6 +7,7 @@ module SearchHelper
     return unless current_user
 
     resources_results = [
+      recent_issues_autocomplete(term),
       groups_autocomplete(term),
       projects_autocomplete(term)
     ].flatten
@@ -178,6 +179,20 @@ module SearchHelper
       }
     end
   end
+
+  def recent_issues_autocomplete(term, limit = 5)
+    return [] unless current_user
+
+    ::Gitlab::Search::RecentIssues.new(user: current_user).search(term).limit(limit).map do |i|
+      {
+        category: "Recent issues",
+        id: i.id,
+        label: search_result_sanitize(i.title),
+        url: issue_path(i),
+        avatar_url: i.project.avatar_url || ''
+      }
+    end
+  end
   # rubocop: enable CodeReuse/ActiveRecord
 
   def search_result_sanitize(str)

app/models/concerns/id_in_ordered.rb

+# frozen_string_literal: true
+
+module IdInOrdered
+  extend ActiveSupport::Concern
+
+  included do
+    scope :id_in_ordered, -> (ids) do
+      raise ArgumentError, "ids must be an array of integers" unless ids.is_a?(Enumerable) && ids.all? { |id| id.is_a?(Integer) }
+
+      # No need to sort if no more than 1 and the sorting code doesn't work
+      # with an empty array
+      return id_in(ids) unless ids.count > 1
+
+      id_attribute = arel_table[:id]
+      id_in(ids)
+        .order(
+          Arel.sql("array_position(ARRAY[#{ids.join(',')}], #{id_attribute.relation.name}.#{id_attribute.name})"))
+    end
+  end
+end

app/models/issue.rb

@@ -18,6 +18,7 @@ class Issue < ApplicationRecord
   include MilestoneEventable
   include WhereComposite
   include StateEventable
+  include IdInOrdered
 
   DueDateStruct                   = Struct.new(:title, :name).freeze
   NoDueDate                       = DueDateStruct.new('No Due Date', '0').freeze

config/feature_flags/development/recent_items_search.yml

+---
+name: recent_items_search
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/40669
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/244277
+group: group::global search
+type: development
+default_enabled: false
\ No newline at end of file

lib/gitlab/search/recent_issues.rb

+# frozen_string_literal: true
+
+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)
+        return unless recent_items_enabled?
+
+        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)
+        return Issue.none unless recent_items_enabled?
+
+        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
+
+      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 recent_items_enabled?
+        Feature.enabled?(:recent_items_search, @user)
+      end
+    end
+  end
+end

spec/controllers/projects/issues_controller_spec.rb

@@ -1011,6 +1011,24 @@ RSpec.describe Projects::IssuesController do
           end
         end
       end
+
+      it 'logs the view with Gitlab::Search::RecentIssues' do
+        sign_in(user)
+        recent_issues_double = instance_double(::Gitlab::Search::RecentIssues, log_view: nil)
+        expect(::Gitlab::Search::RecentIssues).to receive(:new).with(user: user).and_return(recent_issues_double)
+
+        go(id: issue.to_param)
+
+        expect(recent_issues_double).to have_received(:log_view)
+      end
+
+      context 'when not logged in' do
+        it 'does not log the view with Gitlab::Search::RecentIssues' do
+          expect(::Gitlab::Search::RecentIssues).not_to receive(:new)
+
+          go(id: issue.to_param)
+        end
+      end
     end
 
     describe 'GET #realtime_changes' do

spec/helpers/search_helper_spec.rb

@@ -73,6 +73,39 @@ 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
+        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)
+        project2 = create(:project, namespace: user.namespace)
+        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)]))
+
+        results = search_autocomplete_opts("the search term")
+
+        expect(results.count).to eq(5)
+
+        expect(results[0]).to include({
+          category: 'Recent issues',
+          id: issue1.id,
+          label: 'issue 1',
+          url: Gitlab::Routing.url_helpers.project_issue_path(issue1.project, issue1),
+          avatar_url: project1.avatar_url
+        })
+
+        expect(results[1]).to include({
+          category: 'Recent issues',
+          id: issue2.id,
+          label: 'issue 2',
+          url: Gitlab::Routing.url_helpers.project_issue_path(issue2.project, issue2),
+          avatar_url: '' # This project didn't have an avatar so set this to ''
+        })
+      end
+
       it "does not include the public group" do
         group = create(:group)
         expect(search_autocomplete_opts(group.name).size).to eq(0)

spec/lib/gitlab/search/recent_issues_spec.rb

+# frozen_string_literal: true
+
+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) }
+
+  before do
+    stub_feature_flags(recent_items_search: true)
+  end
+
+  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
+
+    context 'when recent_items_search feature flag is disabled' do
+      before do
+        stub_feature_flags(recent_items_search: false)
+      end
+
+      it 'does not store anything' do
+        recent_issues.log_view(issue)
+
+        # Re-enable before searching to prove that the `log_view` call did
+        # not persist it
+        stub_feature_flags(recent_items_search: true)
+
+        results = recent_issues.search('hello')
+
+        expect(results).to be_empty
+      end
+    end
+  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
+
+    context 'when recent_items_search feature flag is disabled' do
+      it 'does not return anything' do
+        recent_issues.log_view(issue)
+
+        # Disable after persisting to prove that the `search` is not searching
+        # anything
+        stub_feature_flags(recent_items_search: false)
+
+        results = recent_issues.search('hello')
+
+        expect(results).to be_empty
+      end
+    end
+  end
+end

spec/models/concerns/id_in_ordered_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe IdInOrdered do
+  describe 'Issue' do
+    describe '.id_in_ordered' do
+      it 'returns issues matching the ids in the same order as the ids' do
+        issue1 = create(:issue)
+        issue2 = create(:issue)
+        issue3 = create(:issue)
+        issue4 = create(:issue)
+        issue5 = create(:issue)
+
+        expect(Issue.id_in_ordered([issue3.id, issue1.id, issue4.id, issue5.id, issue2.id])).to eq([
+          issue3, issue1, issue4, issue5, issue2
+])
+      end
+
+      context 'when the ids are not an array of integers' do
+        it 'raises ArgumentError' do
+          expect { Issue.id_in_ordered([1, 'no SQL injection']) }.to raise_error(ArgumentError, "ids must be an array of integers")
+        end
+      end
+
+      context 'when an empty array is given' do
+        it 'does not raise error' do
+          expect(Issue.id_in_ordered([]).to_a).to be_empty
+        end
+      end
+    end
+  end
+end