Merge branch '207324-search-api-scoped-to-blobs-does-not-honor-per_page' into 'master'

Honor per_page in all Search API endpoints

Closes #207324

See merge request gitlab-org/gitlab!29197

app/services/search_service.rb

@@ -6,6 +6,9 @@ class SearchService
   SEARCH_TERM_LIMIT = 64
   SEARCH_CHAR_LIMIT = 4096
 
+  DEFAULT_PER_PAGE = Gitlab::SearchResults::DEFAULT_PER_PAGE
+  MAX_PER_PAGE = 200
+
   def initialize(current_user, params = {})
     @current_user = current_user
     @params = params.dup
@@ -60,11 +63,19 @@ class SearchService
   end
 
   def search_objects
-    @search_objects ||= redact_unauthorized_results(search_results.objects(scope, params[:page]))
+    @search_objects ||= redact_unauthorized_results(search_results.objects(scope, page: params[:page], per_page: per_page))
   end
 
   private
 
+  def per_page
+    per_page_param = params[:per_page].to_i
+
+    return DEFAULT_PER_PAGE unless per_page_param.positive?
+
+    [MAX_PER_PAGE, per_page_param].min
+  end
+
   def visible_result?(object)
     return true unless object.respond_to?(:to_ability_name) && DeclarativePolicy.has_policy?(object)
 
@@ -75,13 +86,13 @@ class SearchService
     results = results_collection.to_a
     permitted_results = results.select { |object| visible_result?(object) }
 
-    filtered_results = (results - permitted_results).each_with_object({}) do |object, memo|
+    redacted_results = (results - permitted_results).each_with_object({}) do |object, memo|
       memo[object.id] = { ability: :"read_#{object.to_ability_name}", id: object.id, class_name: object.class.name }
     end
 
-    log_redacted_search_results(filtered_results.values) if filtered_results.any?
+    log_redacted_search_results(redacted_results.values) if redacted_results.any?
 
-    return results_collection.id_not_in(filtered_results.keys) if results_collection.is_a?(ActiveRecord::Relation)
+    return results_collection.id_not_in(redacted_results.keys) if results_collection.is_a?(ActiveRecord::Relation)
 
     Kaminari.paginate_array(
       permitted_results,

changelogs/unreleased/207324-search-api-scoped-to-blobs-does-not-honor-per_page.yml

+---
+title: Honor per_page in Search API
+merge_request: 29197
+author:
+type: fixed

ee/lib/gitlab/elastic/group_search_results.rb

@@ -11,7 +11,7 @@ module Gitlab
 
       attr_reader :group, :default_project_filter
 
-      def initialize(current_user, limit_project_ids, limit_projects, group, query, public_and_internal_projects, default_project_filter: false, per_page: 20)
+      def initialize(current_user, limit_project_ids, limit_projects, group, query, public_and_internal_projects, default_project_filter: false)
         super(current_user, query, limit_project_ids, limit_projects, public_and_internal_projects)
 
         @default_project_filter = default_project_filter

ee/lib/gitlab/elastic/project_search_results.rb

@@ -25,7 +25,7 @@ module Gitlab
 
       private
 
-      def blobs(page: 1, per_page: 20)
+      def blobs(page: 1, per_page: DEFAULT_PER_PAGE)
         return Kaminari.paginate_array([]) unless Ability.allowed?(@current_user, :download_code, project)
         return Kaminari.paginate_array([]) if project.empty_repo? || query.blank?
         return Kaminari.paginate_array([]) unless root_ref?
@@ -37,7 +37,7 @@ module Gitlab
         )
       end
 
-      def wiki_blobs(page: 1, per_page: 20)
+      def wiki_blobs(page: 1, per_page: DEFAULT_PER_PAGE)
         return Kaminari.paginate_array([]) unless Ability.allowed?(@current_user, :read_wiki, project)
 
         if project.wiki_enabled? && !project.wiki.empty? && query.present?
@@ -61,7 +61,7 @@ module Gitlab
         Note.elastic_search(query, options: opt)
       end
 
-      def commits(page: 1, per_page: 20)
+      def commits(page: 1, per_page: DEFAULT_PER_PAGE)
         return Kaminari.paginate_array([]) unless Ability.allowed?(@current_user, :download_code, project)
 
         if project.empty_repo? || query.blank?

ee/lib/gitlab/elastic/search_results.rb

@@ -5,6 +5,8 @@ module Gitlab
     class SearchResults
       include Gitlab::Utils::StrongMemoize
 
+      DEFAULT_PER_PAGE = Gitlab::SearchResults::DEFAULT_PER_PAGE
+
       attr_reader :current_user, :query, :public_and_internal_projects
 
       # Limit search results by passed project ids
@@ -22,20 +24,20 @@ module Gitlab
         @public_and_internal_projects = public_and_internal_projects
       end
 
-      def objects(scope, page = 1)
+      def objects(scope, page: 1, per_page: DEFAULT_PER_PAGE)
         page = (page || 1).to_i
 
         case scope
         when 'projects'
-          eager_load(projects, page, eager: [:route, :namespace, :compliance_framework_setting])
+          eager_load(projects, page, per_page, eager: [:route, :namespace, :compliance_framework_setting])
         when 'issues'
-          eager_load(issues, page, eager: { project: [:route, :namespace], labels: [], timelogs: [], assignees: [] })
+          eager_load(issues, page, per_page, eager: { project: [:route, :namespace], labels: [], timelogs: [], assignees: [] })
         when 'merge_requests'
-          eager_load(merge_requests, page, eager: { target_project: [:route, :namespace] })
+          eager_load(merge_requests, page, per_page, eager: { target_project: [:route, :namespace] })
         when 'milestones'
-          eager_load(milestones, page, eager: { project: [:route, :namespace] })
+          eager_load(milestones, page, per_page, eager: { project: [:route, :namespace] })
         when 'notes'
-          eager_load(notes, page, eager: { project: [:route, :namespace] })
+          eager_load(notes, page, per_page, eager: { project: [:route, :namespace] })
         when 'blobs'
           blobs(page: page, per_page: per_page)
         when 'wiki_blobs'
@@ -165,7 +167,7 @@ module Gitlab
 
       # Apply some eager loading to the `records` of an ES result object without
       # losing pagination information
-      def eager_load(es_result, page, eager:)
+      def eager_load(es_result, page, per_page, eager:)
         paginated_base = es_result.page(page).per(per_page)
         relation = paginated_base.records.includes(eager) # rubocop:disable CodeReuse/ActiveRecord
 
@@ -226,7 +228,7 @@ module Gitlab
         end
       end
 
-      def blobs(page: 1, per_page: 20)
+      def blobs(page: 1, per_page: DEFAULT_PER_PAGE)
         return Kaminari.paginate_array([]) if query.blank?
 
         strong_memoize(:blobs) do
@@ -243,7 +245,7 @@ module Gitlab
         end
       end
 
-      def wiki_blobs(page: 1, per_page: 20)
+      def wiki_blobs(page: 1, per_page: DEFAULT_PER_PAGE)
         return Kaminari.paginate_array([]) if query.blank?
 
         strong_memoize(:wiki_blobs) do
@@ -266,7 +268,7 @@ module Gitlab
       # hitting ES twice for any page that's not page 1, and that's something we want to avoid.
       #
       # It is safe to memoize the page we get here because this method is _always_ called before `#commits_count`
-      def commits(page: 1, per_page: 20)
+      def commits(page: 1, per_page: DEFAULT_PER_PAGE)
         return Kaminari.paginate_array([]) if query.blank?
 
         strong_memoize(:commits) do
@@ -370,10 +372,6 @@ module Gitlab
       def default_scope
         'projects'
       end
-
-      def per_page
-        20
-      end
     end
   end
 end

ee/lib/gitlab/elastic/snippet_search_results.rb

@@ -3,10 +3,10 @@
 module Gitlab
   module Elastic
     class SnippetSearchResults < Gitlab::Elastic::SearchResults
-      def objects(scope, page = 1)
+      def objects(scope, page: 1, per_page: DEFAULT_PER_PAGE)
         page = (page || 1).to_i
 
-        eager_load(snippet_titles, page, eager: { project: [:route, :namespace] })
+        eager_load(snippet_titles, page, per_page, eager: { project: [:route, :namespace] })
       end
 
       def formatted_count(scope)

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

@@ -3,6 +3,8 @@
 require 'spec_helper'
 
 describe Gitlab::Elastic::GroupSearchResults do
+  subject(:results) { described_class.new(user, nil, nil, group, query, nil) }
+
   let_it_be(:user) { create(:user) }
   let_it_be(:group) { create(:group) }
   let_it_be(:guest) { create(:user).tap { |u| group.add_user(u, Gitlab::Access::GUEST) } }
@@ -12,7 +14,7 @@ describe Gitlab::Elastic::GroupSearchResults do
   end
 
   context 'user search' do
-    subject(:results) { described_class.new(user, nil, nil, group, guest.username, nil) }
+    let(:query) { guest.username }
 
     before do
       expect(Gitlab::GroupSearchResults).to receive(:new).and_call_original
@@ -20,5 +22,20 @@ describe Gitlab::Elastic::GroupSearchResults do
 
     it { expect(results.objects('users')).to eq([guest]) }
     it { expect(results.limited_users_count).to eq(1) }
+
+    describe 'pagination' do
+      let(:query) {}
+
+      let_it_be(:user2) { create(:user).tap { |u| group.add_user(u, Gitlab::Access::REPORTER) } }
+
+      it 'returns the correct page of results' do
+        expect(results.objects('users', page: 1, per_page: 1)).to eq([user2])
+        expect(results.objects('users', page: 2, per_page: 1)).to eq([guest])
+      end
+
+      it 'returns the correct number of results for one page' do
+        expect(results.objects('users', page: 1, per_page: 2).count).to eq(2)
+      end
+    end
   end
 end

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

@@ -214,7 +214,9 @@ describe Gitlab::Elastic::ProjectSearchResults, :elastic do
   end
 
   context 'user search' do
-    subject(:results) { described_class.new(user, project.owner.username, project) }
+    subject(:results) { described_class.new(user, query, project) }
+
+    let(:query) { project.owner.username }
 
     before do
       expect(Gitlab::ProjectSearchResults).to receive(:new).and_call_original
@@ -222,5 +224,20 @@ describe Gitlab::Elastic::ProjectSearchResults, :elastic do
 
     it { expect(results.objects('users')).to eq([project.owner]) }
     it { expect(results.limited_users_count).to eq(1) }
+
+    describe 'pagination' do
+      let(:query) {}
+
+      let!(:user2) { create(:user).tap { |u| project.add_user(u, Gitlab::Access::REPORTER) } }
+
+      it 'returns the correct page of results' do
+        expect(results.objects('users', page: 1, per_page: 1)).to eq([project.owner])
+        expect(results.objects('users', page: 2, per_page: 1)).to eq([user2])
+      end
+
+      it 'returns the correct number of results for one page' do
+        expect(results.objects('users', page: 1, per_page: 2).count).to eq(2)
+      end
+    end
   end
 end

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

@@ -17,7 +17,7 @@ describe Gitlab::Elastic::SearchResults, :elastic, :sidekiq_might_not_need_inlin
       expect(Repository).to receive(:find_commits_by_message_with_elastic).with('hello world', anything).once.and_call_original
 
       results = described_class.new(user, 'hello world', limit_project_ids)
-      expect(results.objects('commits', 2)).to be_empty
+      expect(results.objects('commits', page: 2)).to be_empty
       expect(results.commits_count).to eq 0
     end
   end
@@ -56,14 +56,19 @@ describe Gitlab::Elastic::SearchResults, :elastic, :sidekiq_might_not_need_inlin
     let(:results) { described_class.new(user, 'hello world', limit_project_ids) }
 
     it 'does not explode when given a page as a string' do
-      expect { results.objects(object_type, "2") }.not_to raise_error
+      expect { results.objects(object_type, page: "2") }.not_to raise_error
     end
 
     it 'paginates' do
-      objects = results.objects(object_type, 2)
+      objects = results.objects(object_type, page: 2)
       expect(objects).to respond_to(:total_count, :limit, :offset)
       expect(objects.offset_value).to eq(20)
     end
+
+    it 'uses the per_page value if passed' do
+      objects = results.objects(object_type, page: 5, per_page: 1)
+      expect(objects.offset_value).to eq(4)
+    end
   end
 
   describe 'parse_search_result' do

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

@@ -13,6 +13,24 @@ describe Gitlab::Elastic::SnippetSearchResults, :elastic, :sidekiq_might_not_nee
     ensure_elasticsearch_index!
   end
 
+  describe 'pagination' do
+    let(:snippet2) { create(:personal_snippet, title: 'foo 2', author: snippet.author) }
+
+    before do
+      perform_enqueued_jobs { snippet2 }
+      ensure_elasticsearch_index!
+    end
+
+    it 'returns the correct page of results' do
+      expect(results.objects('snippet_titles', page: 1, per_page: 1)).to eq([snippet2])
+      expect(results.objects('snippet_titles', page: 2, per_page: 1)).to eq([snippet])
+    end
+
+    it 'returns the correct number of results for one page' do
+      expect(results.objects('snippet_titles', page: 1, per_page: 2)).to eq([snippet, snippet2])
+    end
+  end
+
   describe '#snippet_titles_count' do
     it 'returns the amount of matched snippet titles' do
       expect(results.snippet_titles_count).to eq(1)

ee/spec/requests/api/search_spec.rb

@@ -14,6 +14,30 @@ describe API::Search do
     it { expect(json_response.size).to eq(size) }
   end
 
+  shared_examples 'pagination' do |scope:, search: '*'|
+    it 'returns a different result for each page' do
+      get api(endpoint, user), params: { scope: scope, search: search, page: 1, per_page: 1 }
+      first = json_response.first
+
+      get api(endpoint, user), params: { scope: scope, search: search, page: 2, per_page: 1 }
+      second = Gitlab::Json.parse(response.body).first
+
+      expect(first).not_to eq(second)
+    end
+
+    it 'returns 1 result when per_page is 1' do
+      get api(endpoint, user), params: { scope: scope, search: search, per_page: 1 }
+
+      expect(json_response.count).to eq(1)
+    end
+
+    it 'returns 2 results when per_page is 2' do
+      get api(endpoint, user), params: { scope: scope, search: search, per_page: 2 }
+
+      expect(Gitlab::Json.parse(response.body).count).to eq(2)
+    end
+  end
+
   shared_examples 'elasticsearch disabled' do
     it 'returns 400 error for wiki_blobs scope' do
       get api(endpoint, user), params: { scope: 'wiki_blobs', search: 'awesome' }
@@ -34,11 +58,12 @@ describe API::Search do
     end
   end
 
-  shared_examples 'elasticsearch enabled' do
+  shared_examples 'elasticsearch enabled' do |level:|
     context 'for wiki_blobs scope', :sidekiq_might_not_need_inline do
       before do
         wiki = create(:project_wiki, project: project)
         create(:wiki_page, wiki: wiki, title: 'home', content: "Awesome page")
+        create(:wiki_page, wiki: wiki, title: 'other', content: "Another page")
 
         project.wiki.index_wiki_blobs
         ensure_elasticsearch_index!
@@ -47,6 +72,8 @@ describe API::Search do
       end
 
       it_behaves_like 'response is correct', schema: 'public_api/v4/blobs'
+
+      it_behaves_like 'pagination', scope: 'wiki_blobs'
     end
 
     context 'for commits scope', :sidekiq_might_not_need_inline do
@@ -58,6 +85,8 @@ describe API::Search do
       end
 
       it_behaves_like 'response is correct', schema: 'public_api/v4/commits_details', size: 2
+
+      it_behaves_like 'pagination', scope: 'commits'
     end
 
     context 'for blobs scope', :sidekiq_might_not_need_inline do
@@ -70,6 +99,8 @@ describe API::Search do
 
       it_behaves_like 'response is correct', schema: 'public_api/v4/blobs'
 
+      it_behaves_like 'pagination', scope: 'blobs'
+
       context 'filters' do
         it 'by filename' do
           get api("/projects/#{project.id}/search", user), params: { scope: 'blobs', search: 'mon filename:PROCESS.md' }
@@ -117,6 +148,79 @@ describe API::Search do
         # Some N+1 queries still exist
         expect { get api(endpoint, user), params: { scope: 'issues', search: '*' } }.not_to exceed_query_limit(control.count + new_issues.count * 4)
       end
+
+      it_behaves_like 'pagination', scope: 'issues'
+    end
+
+    context 'for merge_requests scope', :sidekiq_inline do
+      before do
+        create(:merge_request, target_branch: 'feature_2', source_project: project)
+        create(:merge_request, target_branch: 'feature_3', source_project: project)
+
+        ensure_elasticsearch_index!
+      end
+
+      it_behaves_like 'pagination', scope: 'merge_requests'
+    end
+
+    unless level == :project
+      context 'for projects scope', :sidekiq_inline do
+        before do
+          project
+          create(:project, :public, name: 'second project', group: group)
+
+          ensure_elasticsearch_index!
+        end
+
+        it_behaves_like 'pagination', scope: 'projects'
+      end
+    end
+
+    context 'for milestones scope', :sidekiq_inline do
+      before do
+        create_list(:milestone, 2, project: project)
+
+        ensure_elasticsearch_index!
+      end
+
+      it_behaves_like 'pagination', scope: 'milestones'
+    end
+
+    context 'for users scope', :sidekiq_inline do
+      before do
+        create_list(:user, 2).each do |user|
+          project.add_developer(user)
+          group.add_developer(user)
+        end
+      end
+
+      it_behaves_like 'pagination', scope: 'users', search: ''
+    end
+
+    if level == :global
+      context 'for snippet_titles scope', :sidekiq_inline do
+        before do
+          create_list(:snippet, 2, :public, title: 'Some code', content: 'Check it out')
+
+          ensure_elasticsearch_index!
+        end
+
+        it_behaves_like 'pagination', scope: 'snippet_titles'
+      end
+    end
+
+    if level == :project
+      context 'for notes scope', :sidekiq_inline do
+        before do
+          create(:note_on_merge_request, project: project, note: 'awesome note')
+          mr = create(:merge_request, source_project: project, target_branch: 'another_branch')
+          create(:note, project: project, noteable: mr, note: 'another note')
+
+          ensure_elasticsearch_index!
+        end
+
+        it_behaves_like 'pagination', scope: 'notes'
+      end
     end
   end
 
@@ -146,7 +250,7 @@ describe API::Search do
             stub_ee_application_setting(elasticsearch_limit_indexing: false)
           end
 
-          it_behaves_like 'elasticsearch enabled'
+          it_behaves_like 'elasticsearch enabled', level: :global
         end
       end
     end
@@ -175,7 +279,7 @@ describe API::Search do
               create :elasticsearch_indexed_namespace, namespace: group
             end
 
-            it_behaves_like 'elasticsearch enabled'
+            it_behaves_like 'elasticsearch enabled', level: :group
           end
 
           context 'when the namespace is not indexed' do
@@ -188,7 +292,7 @@ describe API::Search do
             stub_ee_application_setting(elasticsearch_limit_indexing: false)
           end
 
-          it_behaves_like 'elasticsearch enabled'
+          it_behaves_like 'elasticsearch enabled', level: :group
         end
       end
     end
@@ -278,7 +382,7 @@ describe API::Search do
               create :elasticsearch_indexed_project, project: project
             end
 
-            it_behaves_like 'elasticsearch enabled'
+            it_behaves_like 'elasticsearch enabled', level: :project
           end
 
           context 'when the project is not indexed' do
@@ -291,7 +395,7 @@ describe API::Search do
             stub_ee_application_setting(elasticsearch_limit_indexing: false)
           end
 
-          it_behaves_like 'elasticsearch enabled'
+          it_behaves_like 'elasticsearch enabled', level: :project
         end
       end
     end

lib/gitlab/group_search_results.rb

@@ -4,8 +4,8 @@ module Gitlab
   class GroupSearchResults < SearchResults
     attr_reader :group
 
-    def initialize(current_user, limit_projects, group, query, default_project_filter: false, per_page: 20)
-      super(current_user, limit_projects, query, default_project_filter: default_project_filter, per_page: per_page)
+    def initialize(current_user, limit_projects, group, query, default_project_filter: false)
+      super(current_user, limit_projects, query, default_project_filter: default_project_filter)
 
       @group = group
     end

lib/gitlab/project_search_results.rb

@@ -2,30 +2,29 @@
 
 module Gitlab
   class ProjectSearchResults < SearchResults
-    attr_reader :project, :repository_ref, :per_page
+    attr_reader :project, :repository_ref
 
-    def initialize(current_user, project, query, repository_ref = nil, per_page: 20)
+    def initialize(current_user, project, query, repository_ref = nil)
       @current_user = current_user
       @project = project
       @repository_ref = repository_ref.presence
       @query = query
-      @per_page = per_page
     end
 
-    def objects(scope, page = nil)
+    def objects(scope, page: nil, per_page: DEFAULT_PER_PAGE)
       case scope
       when 'notes'
         notes.page(page).per(per_page)
       when 'blobs'
-        paginated_blobs(blobs(page), page)
+        paginated_blobs(blobs(limit: limit_up_to_page(page, per_page)), page, per_page)
       when 'wiki_blobs'
-        paginated_blobs(wiki_blobs, page)
+        paginated_blobs(wiki_blobs(limit: limit_up_to_page(page, per_page)), page, per_page)
       when 'commits'
         Kaminari.paginate_array(commits).page(page).per(per_page)
       when 'users'
         users.page(page).per(per_page)
       else
-        super(scope, page, false)
+        super(scope, page: page, per_page: per_page, without_count: false)
       end
     end
 
@@ -49,7 +48,7 @@ module Gitlab
     end
 
     def limited_blobs_count
-      @limited_blobs_count ||= blobs.count
+      @limited_blobs_count ||= blobs(limit: count_limit).count
     end
 
     # rubocop: disable CodeReuse/ActiveRecord
@@ -69,7 +68,7 @@ module Gitlab
     # rubocop: enable CodeReuse/ActiveRecord
 
     def wiki_blobs_count
-      @wiki_blobs_count ||= wiki_blobs.count
+      @wiki_blobs_count ||= wiki_blobs(limit: count_limit).count
     end
 
     def commits_count
@@ -87,7 +86,7 @@ module Gitlab
 
     private
 
-    def paginated_blobs(blobs, page)
+    def paginated_blobs(blobs, page, per_page)
       results = Kaminari.paginate_array(blobs).page(page).per(per_page)
 
       Gitlab::Search::FoundBlob.preload_blobs(results)
@@ -95,19 +94,19 @@ module Gitlab
       results
     end
 
-    def limit_up_to_page(page)
+    def limit_up_to_page(page, per_page)
       current_page = page&.to_i || 1
       offset = per_page * (current_page - 1)
       count_limit + offset
     end
 
-    def blobs(page = 1)
+    def blobs(limit: count_limit)
       return [] unless Ability.allowed?(@current_user, :download_code, @project)
 
-      @blobs ||= Gitlab::FileFinder.new(project, repository_project_ref).find(query, content_match_cutoff: limit_up_to_page(page))
+      @blobs ||= Gitlab::FileFinder.new(project, repository_project_ref).find(query, content_match_cutoff: limit)
     end
 
-    def wiki_blobs
+    def wiki_blobs(limit: count_limit)
       return [] unless Ability.allowed?(@current_user, :read_wiki, @project)
 
       @wiki_blobs ||= begin
@@ -115,7 +114,7 @@ module Gitlab
           if project.wiki.empty?
             []
           else
-            Gitlab::WikiFileFinder.new(project, repository_wiki_ref).find(query)
+            Gitlab::WikiFileFinder.new(project, repository_wiki_ref).find(query, content_match_cutoff: limit)
           end
         else
           []

lib/gitlab/search_results.rb

@@ -4,8 +4,10 @@ module Gitlab
   class SearchResults
     COUNT_LIMIT = 100
     COUNT_LIMIT_MESSAGE = "#{COUNT_LIMIT - 1}+"
+    DEFAULT_PAGE = 1
+    DEFAULT_PER_PAGE = 20
 
-    attr_reader :current_user, :query, :per_page
+    attr_reader :current_user, :query
 
     # Limit search results by passed projects
     # It allows us to search only for projects user has access to
@@ -17,15 +19,14 @@ module Gitlab
     # query
     attr_reader :default_project_filter
 
-    def initialize(current_user, limit_projects, query, default_project_filter: false, per_page: 20)
+    def initialize(current_user, limit_projects, query, default_project_filter: false)
       @current_user = current_user
       @limit_projects = limit_projects || Project.all
       @query = query
       @default_project_filter = default_project_filter
-      @per_page = per_page
     end
 
-    def objects(scope, page = nil, without_count = true)
+    def objects(scope, page: nil, per_page: DEFAULT_PER_PAGE, without_count: true)
       collection = case scope
                    when 'projects'
                      projects
@@ -39,7 +40,9 @@ module Gitlab
                      users
                    else
                      Kaminari.paginate_array([])
-                   end.page(page).per(per_page)
+                   end
+
+      collection = collection.page(page).per(per_page)
 
       without_count ? collection.without_count : collection
     end

lib/gitlab/snippet_search_results.rb

@@ -11,8 +11,8 @@ module Gitlab
       @query = query
     end
 
-    def objects(scope, page = nil)
-      paginated_objects(snippet_titles, page)
+    def objects(scope, page: nil, per_page: DEFAULT_PER_PAGE)
+      paginated_objects(snippet_titles, page, per_page)
     end
 
     def formatted_count(scope)
@@ -38,7 +38,7 @@ module Gitlab
       snippets.search(query)
     end
 
-    def paginated_objects(relation, page)
+    def paginated_objects(relation, page, per_page)
       relation.page(page).per(per_page)
     end
 

spec/features/global_search_spec.rb

@@ -21,7 +21,7 @@ describe 'Global search' do
 
   describe 'I search through the issues and I see pagination' do
     before do
-      allow_next_instance_of(Gitlab::SearchResults) do |instance|
+      allow_next_instance_of(SearchService) do |instance|
         allow(instance).to receive(:per_page).and_return(1)
       end
       create_list(:issue, 2, project: project, title: 'initial')

spec/lib/gitlab/project_search_results_spec.rb

@@ -45,22 +45,36 @@ describe Gitlab::ProjectSearchResults do
         expect(results.formatted_count(scope)).to eq(expected)
       end
     end
+
+    context 'blobs' do
+      it "limits the search to #{described_class::COUNT_LIMIT} items" do
+        expect(results).to receive(:blobs).with(limit: described_class::COUNT_LIMIT).and_call_original
+        expect(results.formatted_count('blobs')).to eq('0')
+      end
+    end
+
+    context 'wiki_blobs' do
+      it "limits the search to #{described_class::COUNT_LIMIT} items" do
+        expect(results).to receive(:wiki_blobs).with(limit: described_class::COUNT_LIMIT).and_call_original
+        expect(results.formatted_count('wiki_blobs')).to eq('0')
+      end
+    end
   end
 
-  shared_examples 'general blob search' do |entity_type, blob_kind|
+  shared_examples 'general blob search' do |entity_type, blob_type|
     let(:query) { 'files' }
     subject(:results) { described_class.new(user, project, query).objects(blob_type) }
 
     context "when #{entity_type} is disabled" do
       let(:project) { disabled_project }
 
-      it "hides #{blob_kind} from members" do
+      it "hides #{blob_type} from members" do
         project.add_reporter(user)
 
         is_expected.to be_empty
       end
 
-      it "hides #{blob_kind} from non-members" do
+      it "hides #{blob_type} from non-members" do
         is_expected.to be_empty
       end
     end
@@ -68,13 +82,13 @@ describe Gitlab::ProjectSearchResults do
     context "when #{entity_type} is internal" do
       let(:project) { private_project }
 
-      it "finds #{blob_kind} for members" do
+      it "finds #{blob_type} for members" do
         project.add_reporter(user)
 
         is_expected.not_to be_empty
       end
 
-      it "hides #{blob_kind} from non-members" do
+      it "hides #{blob_type} from non-members" do
         is_expected.to be_empty
       end
     end
@@ -96,7 +110,7 @@ describe Gitlab::ProjectSearchResults do
     end
   end
 
-  shared_examples 'blob search repository ref' do |entity_type|
+  shared_examples 'blob search repository ref' do |entity_type, blob_type|
     let(:query) { 'files' }
     let(:file_finder) { double }
     let(:project_branch) { 'project_branch' }
@@ -139,9 +153,41 @@ describe Gitlab::ProjectSearchResults do
     end
   end
 
+  shared_examples 'blob search pagination' do |blob_type|
+    let(:per_page) { 20 }
+    let(:count_limit) { described_class::COUNT_LIMIT }
+    let(:file_finder) { instance_double('Gitlab::FileFinder') }
+    let(:results) { described_class.new(user, project, query) }
+    let(:repository_ref) { 'master' }
+
+    before do
+      allow(file_finder).to receive(:find).and_return([])
+      expect(Gitlab::FileFinder).to receive(:new).with(project, repository_ref).and_return(file_finder)
+    end
+
+    it 'limits search results based on the first page' do
+      expect(file_finder).to receive(:find).with(query, content_match_cutoff: count_limit)
+      results.objects(blob_type, page: 1, per_page: per_page)
+    end
+
+    it 'limits search results based on the second page' do
+      expect(file_finder).to receive(:find).with(query, content_match_cutoff: count_limit + per_page)
+      results.objects(blob_type, page: 2, per_page: per_page)
+    end
+
+    it 'limits search results based on the third page' do
+      expect(file_finder).to receive(:find).with(query, content_match_cutoff: count_limit + per_page * 2)
+      results.objects(blob_type, page: 3, per_page: per_page)
+    end
+
+    it 'uses the per_page value when passed' do
+      expect(file_finder).to receive(:find).with(query, content_match_cutoff: count_limit + 10 * 2)
+      results.objects(blob_type, page: 3, per_page: 10)
+    end
+  end
+
   describe 'blob search' do
     let(:project) { create(:project, :public, :repository) }
-    let(:blob_type) { 'blobs' }
 
     it_behaves_like 'general blob search', 'repository', 'blobs' do
       let(:disabled_project) { create(:project, :public, :repository, :repository_disabled) }
@@ -150,37 +196,11 @@ describe Gitlab::ProjectSearchResults do
       let(:expected_file_by_content) { 'CHANGELOG' }
     end
 
-    it_behaves_like 'blob search repository ref', 'project' do
+    it_behaves_like 'blob search repository ref', 'project', 'blobs' do
       let(:entity) { project }
     end
 
-    context 'pagination' do
-      let(:per_page) { 20 }
-      let(:count_limit) { described_class::COUNT_LIMIT }
-      let(:file_finder) { instance_double('Gitlab::FileFinder') }
-      let(:results) { described_class.new(user, project, query, per_page: per_page) }
-      let(:repository_ref) { 'master' }
-
-      before do
-        allow(file_finder).to receive(:find).and_return([])
-        expect(Gitlab::FileFinder).to receive(:new).with(project, repository_ref).and_return(file_finder)
-      end
-
-      it 'limits search results based on the first page' do
-        expect(file_finder).to receive(:find).with(query, content_match_cutoff: count_limit)
-        results.objects(blob_type, 1)
-      end
-
-      it 'limits search results based on the second page' do
-        expect(file_finder).to receive(:find).with(query, content_match_cutoff: count_limit + per_page)
-        results.objects(blob_type, 2)
-      end
-
-      it 'limits search results based on the third page' do
-        expect(file_finder).to receive(:find).with(query, content_match_cutoff: count_limit + per_page * 2)
-        results.objects(blob_type, 3)
-      end
-    end
+    it_behaves_like 'blob search pagination', 'blobs'
   end
 
   describe 'wiki search' do
@@ -192,7 +212,7 @@ describe Gitlab::ProjectSearchResults do
       wiki.create_page('CHANGELOG', 'Files example')
     end
 
-    it_behaves_like 'general blob search', 'wiki', 'wiki blobs' do
+    it_behaves_like 'general blob search', 'wiki', 'wiki_blobs' do
       let(:blob_type) { 'wiki_blobs' }
       let(:disabled_project) { create(:project, :public, :wiki_repo, :wiki_disabled) }
       let(:private_project) { create(:project, :public, :wiki_repo, :wiki_private) }
@@ -200,10 +220,11 @@ describe Gitlab::ProjectSearchResults do
       let(:expected_file_by_content) { 'CHANGELOG.md' }
     end
 
-    it_behaves_like 'blob search repository ref', 'wiki' do
-      let(:blob_type) { 'wiki_blobs' }
+    it_behaves_like 'blob search repository ref', 'wiki', 'wiki_blobs' do
       let(:entity) { project.wiki }
     end
+
+    it_behaves_like 'blob search pagination', 'wiki_blobs'
   end
 
   it 'does not list issues on private projects' do

spec/lib/gitlab/search_results_spec.rb

@@ -28,7 +28,15 @@ describe Gitlab::SearchResults do
       end
 
       it 'returns with counts collection when requested' do
-        expect(results.objects('projects', 1, false)).not_to be_kind_of(Kaminari::PaginatableWithoutCount)
+        expect(results.objects('projects', page: 1, per_page: 1, without_count: false)).not_to be_kind_of(Kaminari::PaginatableWithoutCount)
+      end
+
+      it 'uses page and per_page to paginate results' do
+        project2 = create(:project, name: 'foo')
+
+        expect(results.objects('projects', page: 1, per_page: 1).to_a).to eq([project])
+        expect(results.objects('projects', page: 2, per_page: 1).to_a).to eq([project2])
+        expect(results.objects('projects', page: 1, per_page: 2).count).to eq(2)
       end
     end
 

spec/lib/gitlab/snippet_search_results_spec.rb

@@ -20,4 +20,14 @@ describe Gitlab::SnippetSearchResults do
       expect(results.formatted_count('snippet_titles')).to eq(max_limited_count)
     end
   end
+
+  describe '#objects' do
+    it 'uses page and per_page to paginate results' do
+      snippet2 = create(:snippet, :public, content: 'foo', file_name: 'foo')
+
+      expect(results.objects('snippet_titles', page: 1, per_page: 1).to_a).to eq([snippet2])
+      expect(results.objects('snippet_titles', page: 2, per_page: 1).to_a).to eq([snippet])
+      expect(results.objects('snippet_titles', page: 1, per_page: 2).count).to eq(2)
+    end
+  end
 end

spec/services/search_service_spec.rb

@@ -18,7 +18,9 @@ describe SearchService do
   let(:group_project) { create(:project, group: accessible_group, name: 'group_project') }
   let(:public_project) { create(:project, :public, name: 'public_project') }
 
-  subject(:search_service) { described_class.new(user, search: search, scope: scope, page: 1) }
+  let(:per_page) { described_class::DEFAULT_PER_PAGE }
+
+  subject(:search_service) { described_class.new(user, search: search, scope: scope, page: 1, per_page: per_page) }
 
   before do
     accessible_project.add_maintainer(user)
@@ -240,6 +242,76 @@ describe SearchService do
   end
 
   describe '#search_objects' do
+    context 'handling per_page param' do
+      let(:search) { '' }
+      let(:scope) { nil }
+
+      context 'when nil' do
+        let(:per_page) { nil }
+
+        it "defaults to #{described_class::DEFAULT_PER_PAGE}" do
+          expect_any_instance_of(Gitlab::SearchResults)
+            .to receive(:objects)
+            .with(anything, hash_including(per_page: described_class::DEFAULT_PER_PAGE))
+            .and_call_original
+
+          subject.search_objects
+        end
+      end
+
+      context 'when empty string' do
+        let(:per_page) { '' }
+
+        it "defaults to #{described_class::DEFAULT_PER_PAGE}" do
+          expect_any_instance_of(Gitlab::SearchResults)
+            .to receive(:objects)
+            .with(anything, hash_including(per_page: described_class::DEFAULT_PER_PAGE))
+            .and_call_original
+
+          subject.search_objects
+        end
+      end
+
+      context 'when negative' do
+        let(:per_page) { '-1' }
+
+        it "defaults to #{described_class::DEFAULT_PER_PAGE}" do
+          expect_any_instance_of(Gitlab::SearchResults)
+            .to receive(:objects)
+            .with(anything, hash_including(per_page: described_class::DEFAULT_PER_PAGE))
+            .and_call_original
+
+          subject.search_objects
+        end
+      end
+
+      context 'when present' do
+        let(:per_page) { '50' }
+
+        it "converts to integer and passes to search results" do
+          expect_any_instance_of(Gitlab::SearchResults)
+            .to receive(:objects)
+            .with(anything, hash_including(per_page: 50))
+            .and_call_original
+
+          subject.search_objects
+        end
+      end
+
+      context "when greater than #{described_class::MAX_PER_PAGE}" do
+        let(:per_page) { described_class::MAX_PER_PAGE + 1 }
+
+        it "passes #{described_class::MAX_PER_PAGE}" do
+          expect_any_instance_of(Gitlab::SearchResults)
+            .to receive(:objects)
+            .with(anything, hash_including(per_page: described_class::MAX_PER_PAGE))
+            .and_call_original
+
+          subject.search_objects
+        end
+      end
+    end
+
     context 'with accessible project_id' do
       it 'returns objects in the project' do
         search_objects = described_class.new(