Limit snippet search scope in GitLab.com

The scope we use when searching for snippets it's
quite wide. It searchs in personal snippets and also
project snippets (in authorized projects and also visible
when not authorized).

When the dataset is quite big, like in GitLab.com, the requests
times out because they take around 30s.

While we address and rearrange the snippet search to reduce
this scope for every type of instance, we're going
to narrow it just for GitLab.com.

Now, we're going to search only in the user personal and in
authorized project snippets.

app/finders/snippets_finder.rb

@@ -59,18 +59,20 @@ class SnippetsFinder < UnionFinder
   end
 
   def execute
-    base =
-      if project
-        snippets_for_a_single_project
-      else
-        snippets_for_multiple_projects
-      end
-
+    base = init_collection
     base.with_optional_visibility(visibility_from_scope).fresh
   end
 
   private
 
+  def init_collection
+    if project
+      snippets_for_a_single_project
+    else
+      snippets_for_multiple_projects
+    end
+  end
+
   # Produces a query that retrieves snippets from multiple projects.
   #
   # The resulting query will, depending on the user's permissions, include the
@@ -115,7 +117,7 @@ class SnippetsFinder < UnionFinder
   # This method requires that `current_user` returns a `User` instead of `nil`,
   # and is optimised for this specific scenario.
   def snippets_of_authorized_projects
-    base = author ? snippets_for_author : Snippet.all
+    base = author ? author.snippets : Snippet.all
 
     base
       .only_include_projects_with_snippets_enabled(include_private: true)
@@ -157,3 +159,5 @@ class SnippetsFinder < UnionFinder
     end
   end
 end
+
+SnippetsFinder.prepend_if_ee('EE::SnippetsFinder')

changelogs/unreleased/fj-26123-narrow-snippet-search-scope-in-com.yml

+---
+title: Narrow snippet search scope in GitLab.com
+merge_request: 17625
+author:
+type: performance

ee/app/finders/ee/snippets_finder.rb

+# frozen_string_literal: true
+
+module EE
+  module SnippetsFinder
+    extend ::Gitlab::Utils::Override
+
+    attr_reader :authorized_and_user_personal
+
+    def initialize(current_user = nil, params = {})
+      super
+
+      @authorized_and_user_personal = params[:authorized_and_user_personal]
+    end
+
+    private
+
+    override :init_collection
+    def init_collection
+      return snippets_of_authorized_projects_or_personal if authorized_and_user_personal.present?
+
+      super
+    end
+
+    # This method returns snippets from a more restrictive scope.
+    # When current_user is not nil we return the personal snippets
+    # authored by the user and also snippets from the authorized projects.
+    #
+    # When current_user is nil it returns only public personal snippets
+    def snippets_of_authorized_projects_or_personal
+      queries = [restricted_global_snippets]
+
+      if current_user && Ability.allowed?(current_user, :read_cross_project)
+        queries << snippets_of_authorized_projects
+      end
+
+      find_union(queries, ::Snippet)
+    end
+
+    def restricted_global_snippets
+      if author
+        snippets_for_author
+      elsif current_user
+        current_user.snippets
+      else
+        ::Snippet.public_to_user
+      end.only_global_snippets
+    end
+  end
+end

ee/app/helpers/ee/search_helper.rb

@@ -3,6 +3,7 @@ module EE
   module SearchHelper
     extend ::Gitlab::Utils::Override
 
+    override :search_filter_input_options
     def search_filter_input_options(type)
       options = super
       options[:data][:'multiple-assignees'] = 'true' if search_multiple_assignees?(type)
@@ -42,6 +43,23 @@ module EE
       end
     end
 
+    # This is a special case for snippet searches in .com.
+    # The scope used to gather the snippets is too wide and
+    # we have to process a lot of them, what leads to time outs.
+    # We're reducing the scope only in .com because the current
+    # one is still valid in smaller installations.
+    # https://gitlab.com/gitlab-org/gitlab/issues/26123
+    override :search_entries_info_template
+    def search_entries_info_template(collection)
+      return super unless gitlab_com_snippet_db_search?
+
+      if collection.total_pages > 1
+        s_("SearchResults|Showing %{from} - %{to} of %{count} %{scope} for \"%{term}\" in your personal and project snippets")
+      else
+        s_("SearchResults|Showing %{count} %{scope} for \"%{term}\" in your personal and project snippets")
+      end
+    end
+
     private
 
     def search_multiple_assignees?(type)
@@ -54,5 +72,13 @@ module EE
     def blob_project_id(blob_result)
       blob_result.dig('_source', 'join_field', 'parent')&.split('_')&.last.to_i
     end
+
+    def gitlab_com_snippet_db_search?
+      @current_user &&
+        @show_snippets &&
+        ::Gitlab.com? &&
+        ::Feature.enabled?(:restricted_snippet_scope_search, default_enabled: true) &&
+        ::Gitlab::CurrentSettings.search_using_elasticsearch?(scope: nil)
+    end
   end
 end

ee/lib/ee/gitlab/snippet_search_results.rb

+# frozen_string_literal: true
+
+module EE
+  module Gitlab
+    module SnippetSearchResults
+      extend ::Gitlab::Utils::Override
+
+      # Special scope for .com
+      # https://gitlab.com/gitlab-org/gitlab/issues/26123
+      override :finder_params
+      def finder_params
+        return super unless ::Gitlab.com?
+        return super unless ::Feature.enabled?(:restricted_snippet_scope_search, default_enabled: true)
+
+        { authorized_and_user_personal: true }
+      end
+    end
+  end
+end

ee/spec/finders/snippets_finder_spec.rb

@@ -3,20 +3,154 @@
 require 'spec_helper'
 
 describe SnippetsFinder do
+  let_it_be(:group) { create(:group, :public) }
+  let_it_be(:project) { create(:project, :public, group: group) }
+  let_it_be(:private_project_snippet) { create(:project_snippet, :private, project: project) }
+  let_it_be(:internal_project_snippet) { create(:project_snippet, :internal, project: project) }
+  let_it_be(:public_project_snippet) { create(:project_snippet, :public, project: project) }
+
+  let(:finder_params) { {} }
+  let(:finder_user) {}
+
+  subject { described_class.new(finder_user, finder_params).execute }
+
   context 'filter by project' do
-    set(:user) { create(:user) }
-    set(:group) { create(:group, :public) }
-    set(:project) { create(:project, :public, group: group) }
-    set(:private_project_snippet) { create(:project_snippet, :private, project: project) }
-    set(:internal_project_snippet) { create(:project_snippet, :internal, project: project) }
-    set(:public_project_snippet) { create(:project_snippet, :public, project: project) }
+    let_it_be(:user) { create(:user, :auditor) }
+
+    let(:finder_params) { { project: project } }
+    let(:finder_user) { user }
 
     it 'returns all snippets for auditor users' do
-      user = create(:user, :auditor)
+      expect(subject).to match_array([private_project_snippet, internal_project_snippet, public_project_snippet])
+    end
+  end
+
+  context 'filter by authorized snippet projects and authored personal' do
+    let_it_be(:user) { create(:user) }
+    let_it_be(:other_user) { create(:user) }
+    let_it_be(:other_project) { create(:project) }
+    let_it_be(:private_personal_snippet) { create(:personal_snippet, :private, author: user) }
+    let_it_be(:internal_personal_snippet) { create(:personal_snippet, :internal, author: user) }
+    let_it_be(:public_personal_snippet) { create(:personal_snippet, :public, author: user) }
+    let_it_be(:other_private_personal_snippet) { create(:personal_snippet, :private, author: other_user) }
+    let_it_be(:other_internal_personal_snippet) { create(:personal_snippet, :internal, author: other_user) }
+    let_it_be(:other_public_personal_snippet) { create(:personal_snippet, :public, author: other_user) }
+    let_it_be(:other_private_project_snippet) { create(:project_snippet, :private, project: other_project, author: other_user) }
+    let_it_be(:other_internal_project_snippet) { create(:project_snippet, :internal, project: other_project, author: other_user) }
+    let_it_be(:other_public_project_snippet) { create(:project_snippet, :public, project: other_project, author: other_user) }
+
+    let(:finder_params) { { authorized_and_user_personal: true } }
+    let(:finder_user) { user }
+
+    context 'when no user' do
+      let(:finder_user) {}
+
+      it 'returns only public personal snippets' do
+        expect(subject).to contain_exactly(public_personal_snippet, other_public_personal_snippet)
+      end
+    end
+
+    context 'when user is not a member of any project' do
+      it 'returns only user personal snippets' do
+        expect(subject).to match_array([public_personal_snippet, internal_personal_snippet, private_personal_snippet])
+      end
+    end
+
+    context 'when the user is a member of a project' do
+      [:guest, :reporter, :developer, :maintainer].each do |role|
+        it 'returns all the authorized project snippets and authored personal ones' do
+          project.add_role(user, role)
+
+          expect(subject)
+            .to contain_exactly(
+              public_personal_snippet,
+              internal_personal_snippet,
+              private_personal_snippet,
+              public_project_snippet,
+              internal_project_snippet,
+              private_project_snippet
+            )
+        end
+      end
+
+      [:guest, :reporter, :developer, :maintainer].each do |role|
+        it 'returns all the authorized project snippets and authored personal ones' do
+          project.add_role(user, role)
+          other_project.add_role(user, role)
+
+          expect(subject)
+            .to contain_exactly(
+              public_personal_snippet,
+              internal_personal_snippet,
+              private_personal_snippet,
+              public_project_snippet,
+              internal_project_snippet,
+              private_project_snippet,
+              other_private_project_snippet,
+              other_internal_project_snippet,
+              other_public_project_snippet
+            )
+        end
+      end
+
+      context 'when user cannot read_cross_project' do
+        before do
+          project.add_maintainer(user)
+          allow(Ability).to receive(:allowed?)
+                          .with(user, :read_cross_project)
+                          .and_return(false)
+        end
+
+        it 'returns only user personal snippets' do
+          expect(subject).to contain_exactly(public_personal_snippet, internal_personal_snippet, private_personal_snippet)
+        end
+      end
+    end
+
+    context 'when the user is a member of a group' do
+      [:guest, :reporter, :developer, :maintainer].each do |role|
+        it 'returns all the authorized project snippets and authored personal ones' do
+          group.add_user(user, role)
+
+          expect(subject)
+            .to contain_exactly(
+              public_personal_snippet,
+              internal_personal_snippet,
+              private_personal_snippet,
+              public_project_snippet,
+              internal_project_snippet,
+              private_project_snippet
+            )
+        end
+      end
+    end
+
+    context 'when param author is passed' do
+      let(:finder_params) { { author: other_user, authorized_and_user_personal: true } }
+
+      context 'when user is not a member of any project' do
+        it 'returns only the author visible personal snippets to the user' do
+          expect(subject).to contain_exactly(other_public_personal_snippet, other_internal_personal_snippet)
+        end
+      end
 
-      snippets = described_class.new(user, project: project).execute
+      context 'when user is a member of a project' do
+        [:guest, :reporter, :developer, :maintainer].each do |role|
+          it 'returns all the authorized project and personal snippets authored by the author' do
+            project.add_role(user, role)
+            other_project.add_role(user, role)
 
-      expect(snippets).to include(private_project_snippet, internal_project_snippet, public_project_snippet)
+            expect(subject)
+              .to contain_exactly(
+                other_public_personal_snippet,
+                other_internal_personal_snippet,
+                other_internal_project_snippet,
+                other_public_project_snippet,
+                other_private_project_snippet
+              )
+          end
+        end
+      end
     end
   end
 end

ee/spec/helpers/search_helper_spec.rb

@@ -111,4 +111,68 @@ describe SearchHelper do
       expect(result_projects).to match_array(projects)
     end
   end
+
+  describe '#search_entries_info_template' do
+    let(:com_value) { true }
+    let(:flag_enabled) { true }
+    let(:elasticsearch_enabled) { true }
+    let(:show_snippets) { true }
+    let(:collection) { Kaminari.paginate_array([:foo]).page(1).per(10) }
+    let(:user) { create(:user) }
+    let(:message) { "Showing %{count} %{scope} for \"%{term}\"" }
+    let(:new_message) { message + " in your personal and project snippets" }
+
+    subject { search_entries_info_template(collection) }
+
+    before do
+      @show_snippets = show_snippets
+      @current_user = user
+
+      allow(Gitlab).to receive(:com?).and_return(com_value)
+      stub_feature_flags(restricted_snippet_scope_search: flag_enabled)
+      stub_ee_application_setting(search_using_elasticsearch: elasticsearch_enabled)
+    end
+
+    shared_examples 'returns old message' do
+      it do
+        expect(subject).to eq message
+      end
+    end
+
+    context 'when all requirements are met' do
+      it 'returns a custom message' do
+        expect(subject).to eq new_message
+      end
+    end
+
+    context 'when not in Gitlab.com' do
+      let(:com_value) { false }
+
+      it_behaves_like 'returns old message'
+    end
+
+    context 'when flag restricted_snippet_scope_search is not enabled' do
+      let(:flag_enabled) { false }
+
+      it_behaves_like 'returns old message'
+    end
+
+    context 'when elastic search is not enabled' do
+      let(:elasticsearch_enabled) { false }
+
+      it_behaves_like 'returns old message'
+    end
+
+    context 'when no user is present' do
+      let(:user) { nil }
+
+      it_behaves_like 'returns old message'
+    end
+
+    context 'when not searching for snippets' do
+      let(:show_snippets) { nil }
+
+      it_behaves_like 'returns old message'
+    end
+  end
 end

ee/spec/lib/ee/gitlab/snippet_search_results_spec.rb

+# frozen_string_literal: true
+require 'spec_helper'
+
+describe Gitlab::SnippetSearchResults do
+  let!(:snippet) { create(:snippet, content: 'foo', file_name: 'foo') }
+  let(:user) { snippet.author }
+  let(:com_value) { true }
+  let(:flag_enabled) { true }
+
+  subject { described_class.new(user, 'foo').objects('snippet_titles') }
+
+  before do
+    allow(Gitlab).to receive(:com?).and_return(com_value)
+    stub_feature_flags(restricted_snippet_scope_search: flag_enabled)
+  end
+
+  context 'when all requirements are met' do
+    it 'calls the finder with the restrictive scope' do
+      expect(SnippetsFinder).to receive(:new).with(user, authorized_and_user_personal: true).and_call_original
+
+      subject
+    end
+  end
+
+  context 'when not in Gitlab.com' do
+    let(:com_value) { false }
+
+    it 'calls the finder with the restrictive scope' do
+      expect(SnippetsFinder).to receive(:new).with(user, {}).and_call_original
+
+      subject
+    end
+  end
+
+  context 'when flag restricted_snippet_scope_search is not enabled' do
+    let(:flag_enabled) { false }
+
+    it 'calls the finder with the restrictive scope' do
+      expect(SnippetsFinder).to receive(:new).with(user, {}).and_call_original
+
+      subject
+    end
+  end
+end

lib/gitlab/snippet_search_results.rb

@@ -45,7 +45,7 @@ module Gitlab
 
     # rubocop: disable CodeReuse/ActiveRecord
     def snippets
-      SnippetsFinder.new(current_user)
+      SnippetsFinder.new(current_user, finder_params)
         .execute
         .includes(:author)
         .reorder(updated_at: :desc)
@@ -67,5 +67,11 @@ module Gitlab
     def paginated_objects(relation, page)
       relation.page(page).per(per_page)
     end
+
+    def finder_params
+      {}
+    end
   end
 end
+
+Gitlab::SnippetSearchResults.prepend_if_ee('::EE::Gitlab::SnippetSearchResults')

locale/gitlab.pot

@@ -13945,9 +13945,15 @@ msgstr ""
 msgid "SearchResults|Showing %{count} %{scope} for \"%{term}\""
 msgstr ""
 
+msgid "SearchResults|Showing %{count} %{scope} for \"%{term}\" in your personal and project snippets"
+msgstr ""
+
 msgid "SearchResults|Showing %{from} - %{to} of %{count} %{scope} for \"%{term}\""
 msgstr ""
 
+msgid "SearchResults|Showing %{from} - %{to} of %{count} %{scope} for \"%{term}\" in your personal and project snippets"
+msgstr ""
+
 msgid "SearchResults|We couldn't find any %{scope} matching %{term}"
 msgstr ""
 

spec/finders/snippets_finder_spec.rb

@@ -150,6 +150,26 @@ describe SnippetsFinder do
 
         expect(snippets).to contain_exactly(private_project_snippet, internal_project_snippet, public_project_snippet)
       end
+
+      context 'filter by author' do
+        let!(:other_user) { create(:user) }
+        let!(:other_private_project_snippet) { create(:project_snippet, :private, project: project, author: other_user) }
+        let!(:other_internal_project_snippet) { create(:project_snippet, :internal, project: project, author: other_user) }
+        let!(:other_public_project_snippet) { create(:project_snippet, :public, project: project, author: other_user) }
+
+        it 'returns all snippets for project members' do
+          project.add_developer(user)
+
+          snippets = described_class.new(user, author: other_user).execute
+
+          expect(snippets)
+            .to contain_exactly(
+              other_private_project_snippet,
+              other_internal_project_snippet,
+              other_public_project_snippet
+            )
+        end
+      end
     end
 
     context 'when the user cannot read cross project' do