Commit 8ce318be authored by Alishan Ladhani's avatar Alishan Ladhani Committed by Peter Leitzen

Honor per_page param in the Search API

We use a default per_page and don't pass it to the search_service,
this commit fixes it.
parent 294f2e11
...@@ -6,6 +6,9 @@ class SearchService ...@@ -6,6 +6,9 @@ class SearchService
SEARCH_TERM_LIMIT = 64 SEARCH_TERM_LIMIT = 64
SEARCH_CHAR_LIMIT = 4096 SEARCH_CHAR_LIMIT = 4096
DEFAULT_PER_PAGE = Gitlab::SearchResults::DEFAULT_PER_PAGE
MAX_PER_PAGE = 200
def initialize(current_user, params = {}) def initialize(current_user, params = {})
@current_user = current_user @current_user = current_user
@params = params.dup @params = params.dup
...@@ -60,11 +63,19 @@ class SearchService ...@@ -60,11 +63,19 @@ class SearchService
end end
def search_objects 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 end
private 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) def visible_result?(object)
return true unless object.respond_to?(:to_ability_name) && DeclarativePolicy.has_policy?(object) return true unless object.respond_to?(:to_ability_name) && DeclarativePolicy.has_policy?(object)
...@@ -75,13 +86,13 @@ class SearchService ...@@ -75,13 +86,13 @@ class SearchService
results = results_collection.to_a results = results_collection.to_a
permitted_results = results.select { |object| visible_result?(object) } 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 } memo[object.id] = { ability: :"read_#{object.to_ability_name}", id: object.id, class_name: object.class.name }
end 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( Kaminari.paginate_array(
permitted_results, permitted_results,
......
---
title: Honor per_page in Search API
merge_request: 29197
author:
type: fixed
...@@ -11,7 +11,7 @@ module Gitlab ...@@ -11,7 +11,7 @@ module Gitlab
attr_reader :group, :default_project_filter 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) super(current_user, query, limit_project_ids, limit_projects, public_and_internal_projects)
@default_project_filter = default_project_filter @default_project_filter = default_project_filter
......
...@@ -25,7 +25,7 @@ module Gitlab ...@@ -25,7 +25,7 @@ module Gitlab
private 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([]) unless Ability.allowed?(@current_user, :download_code, project)
return Kaminari.paginate_array([]) if project.empty_repo? || query.blank? return Kaminari.paginate_array([]) if project.empty_repo? || query.blank?
return Kaminari.paginate_array([]) unless root_ref? return Kaminari.paginate_array([]) unless root_ref?
...@@ -37,7 +37,7 @@ module Gitlab ...@@ -37,7 +37,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([]) unless Ability.allowed?(@current_user, :read_wiki, project) return Kaminari.paginate_array([]) unless Ability.allowed?(@current_user, :read_wiki, project)
if project.wiki_enabled? && !project.wiki.empty? && query.present? if project.wiki_enabled? && !project.wiki.empty? && query.present?
...@@ -61,7 +61,7 @@ module Gitlab ...@@ -61,7 +61,7 @@ module Gitlab
Note.elastic_search(query, options: opt) Note.elastic_search(query, options: opt)
end 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) return Kaminari.paginate_array([]) unless Ability.allowed?(@current_user, :download_code, project)
if project.empty_repo? || query.blank? if project.empty_repo? || query.blank?
......
...@@ -5,6 +5,8 @@ module Gitlab ...@@ -5,6 +5,8 @@ module Gitlab
class SearchResults class SearchResults
include Gitlab::Utils::StrongMemoize include Gitlab::Utils::StrongMemoize
DEFAULT_PER_PAGE = Gitlab::SearchResults::DEFAULT_PER_PAGE
attr_reader :current_user, :query, :public_and_internal_projects attr_reader :current_user, :query, :public_and_internal_projects
# Limit search results by passed project ids # Limit search results by passed project ids
...@@ -22,20 +24,20 @@ module Gitlab ...@@ -22,20 +24,20 @@ module Gitlab
@public_and_internal_projects = public_and_internal_projects @public_and_internal_projects = public_and_internal_projects
end end
def objects(scope, page = 1) def objects(scope, page: 1, per_page: DEFAULT_PER_PAGE)
page = (page || 1).to_i page = (page || 1).to_i
case scope case scope
when 'projects' 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' 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' 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' when 'milestones'
eager_load(milestones, page, eager: { project: [:route, :namespace] }) eager_load(milestones, page, per_page, eager: { project: [:route, :namespace] })
when 'notes' when 'notes'
eager_load(notes, page, eager: { project: [:route, :namespace] }) eager_load(notes, page, per_page, eager: { project: [:route, :namespace] })
when 'blobs' when 'blobs'
blobs(page: page, per_page: per_page) blobs(page: page, per_page: per_page)
when 'wiki_blobs' when 'wiki_blobs'
...@@ -165,7 +167,7 @@ module Gitlab ...@@ -165,7 +167,7 @@ module Gitlab
# Apply some eager loading to the `records` of an ES result object without # Apply some eager loading to the `records` of an ES result object without
# losing pagination information # 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) paginated_base = es_result.page(page).per(per_page)
relation = paginated_base.records.includes(eager) # rubocop:disable CodeReuse/ActiveRecord relation = paginated_base.records.includes(eager) # rubocop:disable CodeReuse/ActiveRecord
...@@ -226,7 +228,7 @@ module Gitlab ...@@ -226,7 +228,7 @@ module Gitlab
end end
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? return Kaminari.paginate_array([]) if query.blank?
strong_memoize(:blobs) do strong_memoize(:blobs) do
...@@ -243,7 +245,7 @@ module Gitlab ...@@ -243,7 +245,7 @@ module Gitlab
end end
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? return Kaminari.paginate_array([]) if query.blank?
strong_memoize(:wiki_blobs) do strong_memoize(:wiki_blobs) do
...@@ -266,7 +268,7 @@ module Gitlab ...@@ -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. # 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` # 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? return Kaminari.paginate_array([]) if query.blank?
strong_memoize(:commits) do strong_memoize(:commits) do
...@@ -370,10 +372,6 @@ module Gitlab ...@@ -370,10 +372,6 @@ module Gitlab
def default_scope def default_scope
'projects' 'projects'
end end
def per_page
20
end
end end
end end
end end
...@@ -3,10 +3,10 @@ ...@@ -3,10 +3,10 @@
module Gitlab module Gitlab
module Elastic module Elastic
class SnippetSearchResults < Gitlab::Elastic::SearchResults 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 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 end
def formatted_count(scope) def formatted_count(scope)
......
...@@ -3,6 +3,8 @@ ...@@ -3,6 +3,8 @@
require 'spec_helper' require 'spec_helper'
describe Gitlab::Elastic::GroupSearchResults do 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(:user) { create(:user) }
let_it_be(:group) { create(:group) } let_it_be(:group) { create(:group) }
let_it_be(:guest) { create(:user).tap { |u| group.add_user(u, Gitlab::Access::GUEST) } } let_it_be(:guest) { create(:user).tap { |u| group.add_user(u, Gitlab::Access::GUEST) } }
...@@ -12,7 +14,7 @@ describe Gitlab::Elastic::GroupSearchResults do ...@@ -12,7 +14,7 @@ describe Gitlab::Elastic::GroupSearchResults do
end end
context 'user search' do context 'user search' do
subject(:results) { described_class.new(user, nil, nil, group, guest.username, nil) } let(:query) { guest.username }
before do before do
expect(Gitlab::GroupSearchResults).to receive(:new).and_call_original expect(Gitlab::GroupSearchResults).to receive(:new).and_call_original
...@@ -20,5 +22,20 @@ describe Gitlab::Elastic::GroupSearchResults do ...@@ -20,5 +22,20 @@ describe Gitlab::Elastic::GroupSearchResults do
it { expect(results.objects('users')).to eq([guest]) } it { expect(results.objects('users')).to eq([guest]) }
it { expect(results.limited_users_count).to eq(1) } 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
end end
...@@ -214,7 +214,9 @@ describe Gitlab::Elastic::ProjectSearchResults, :elastic do ...@@ -214,7 +214,9 @@ describe Gitlab::Elastic::ProjectSearchResults, :elastic do
end end
context 'user search' do 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 before do
expect(Gitlab::ProjectSearchResults).to receive(:new).and_call_original expect(Gitlab::ProjectSearchResults).to receive(:new).and_call_original
...@@ -222,5 +224,20 @@ describe Gitlab::Elastic::ProjectSearchResults, :elastic do ...@@ -222,5 +224,20 @@ describe Gitlab::Elastic::ProjectSearchResults, :elastic do
it { expect(results.objects('users')).to eq([project.owner]) } it { expect(results.objects('users')).to eq([project.owner]) }
it { expect(results.limited_users_count).to eq(1) } 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
end end
...@@ -17,7 +17,7 @@ describe Gitlab::Elastic::SearchResults, :elastic, :sidekiq_might_not_need_inlin ...@@ -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 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) 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 expect(results.commits_count).to eq 0
end end
end end
...@@ -56,14 +56,19 @@ describe Gitlab::Elastic::SearchResults, :elastic, :sidekiq_might_not_need_inlin ...@@ -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) } let(:results) { described_class.new(user, 'hello world', limit_project_ids) }
it 'does not explode when given a page as a string' do 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 end
it 'paginates' do 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).to respond_to(:total_count, :limit, :offset)
expect(objects.offset_value).to eq(20) expect(objects.offset_value).to eq(20)
end 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 end
describe 'parse_search_result' do describe 'parse_search_result' do
......
...@@ -13,6 +13,24 @@ describe Gitlab::Elastic::SnippetSearchResults, :elastic, :sidekiq_might_not_nee ...@@ -13,6 +13,24 @@ describe Gitlab::Elastic::SnippetSearchResults, :elastic, :sidekiq_might_not_nee
ensure_elasticsearch_index! ensure_elasticsearch_index!
end 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 describe '#snippet_titles_count' do
it 'returns the amount of matched snippet titles' do it 'returns the amount of matched snippet titles' do
expect(results.snippet_titles_count).to eq(1) expect(results.snippet_titles_count).to eq(1)
......
...@@ -14,6 +14,30 @@ describe API::Search do ...@@ -14,6 +14,30 @@ describe API::Search do
it { expect(json_response.size).to eq(size) } it { expect(json_response.size).to eq(size) }
end 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 shared_examples 'elasticsearch disabled' do
it 'returns 400 error for wiki_blobs scope' do it 'returns 400 error for wiki_blobs scope' do
get api(endpoint, user), params: { scope: 'wiki_blobs', search: 'awesome' } get api(endpoint, user), params: { scope: 'wiki_blobs', search: 'awesome' }
...@@ -34,11 +58,12 @@ describe API::Search do ...@@ -34,11 +58,12 @@ describe API::Search do
end end
end end
shared_examples 'elasticsearch enabled' do shared_examples 'elasticsearch enabled' do |level:|
context 'for wiki_blobs scope', :sidekiq_might_not_need_inline do context 'for wiki_blobs scope', :sidekiq_might_not_need_inline do
before do before do
wiki = create(:project_wiki, project: project) wiki = create(:project_wiki, project: project)
create(:wiki_page, wiki: wiki, title: 'home', content: "Awesome page") 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 project.wiki.index_wiki_blobs
ensure_elasticsearch_index! ensure_elasticsearch_index!
...@@ -47,6 +72,8 @@ describe API::Search do ...@@ -47,6 +72,8 @@ describe API::Search do
end end
it_behaves_like 'response is correct', schema: 'public_api/v4/blobs' it_behaves_like 'response is correct', schema: 'public_api/v4/blobs'
it_behaves_like 'pagination', scope: 'wiki_blobs'
end end
context 'for commits scope', :sidekiq_might_not_need_inline do context 'for commits scope', :sidekiq_might_not_need_inline do
...@@ -58,6 +85,8 @@ describe API::Search do ...@@ -58,6 +85,8 @@ describe API::Search do
end end
it_behaves_like 'response is correct', schema: 'public_api/v4/commits_details', size: 2 it_behaves_like 'response is correct', schema: 'public_api/v4/commits_details', size: 2
it_behaves_like 'pagination', scope: 'commits'
end end
context 'for blobs scope', :sidekiq_might_not_need_inline do context 'for blobs scope', :sidekiq_might_not_need_inline do
...@@ -70,6 +99,8 @@ describe API::Search do ...@@ -70,6 +99,8 @@ describe API::Search do
it_behaves_like 'response is correct', schema: 'public_api/v4/blobs' it_behaves_like 'response is correct', schema: 'public_api/v4/blobs'
it_behaves_like 'pagination', scope: 'blobs'
context 'filters' do context 'filters' do
it 'by filename' do it 'by filename' do
get api("/projects/#{project.id}/search", user), params: { scope: 'blobs', search: 'mon filename:PROCESS.md' } get api("/projects/#{project.id}/search", user), params: { scope: 'blobs', search: 'mon filename:PROCESS.md' }
...@@ -117,6 +148,79 @@ describe API::Search do ...@@ -117,6 +148,79 @@ describe API::Search do
# Some N+1 queries still exist # 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) expect { get api(endpoint, user), params: { scope: 'issues', search: '*' } }.not_to exceed_query_limit(control.count + new_issues.count * 4)
end 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
end end
...@@ -146,7 +250,7 @@ describe API::Search do ...@@ -146,7 +250,7 @@ describe API::Search do
stub_ee_application_setting(elasticsearch_limit_indexing: false) stub_ee_application_setting(elasticsearch_limit_indexing: false)
end end
it_behaves_like 'elasticsearch enabled' it_behaves_like 'elasticsearch enabled', level: :global
end end
end end
end end
...@@ -175,7 +279,7 @@ describe API::Search do ...@@ -175,7 +279,7 @@ describe API::Search do
create :elasticsearch_indexed_namespace, namespace: group create :elasticsearch_indexed_namespace, namespace: group
end end
it_behaves_like 'elasticsearch enabled' it_behaves_like 'elasticsearch enabled', level: :group
end end
context 'when the namespace is not indexed' do context 'when the namespace is not indexed' do
...@@ -188,7 +292,7 @@ describe API::Search do ...@@ -188,7 +292,7 @@ describe API::Search do
stub_ee_application_setting(elasticsearch_limit_indexing: false) stub_ee_application_setting(elasticsearch_limit_indexing: false)
end end
it_behaves_like 'elasticsearch enabled' it_behaves_like 'elasticsearch enabled', level: :group
end end
end end
end end
...@@ -278,7 +382,7 @@ describe API::Search do ...@@ -278,7 +382,7 @@ describe API::Search do
create :elasticsearch_indexed_project, project: project create :elasticsearch_indexed_project, project: project
end end
it_behaves_like 'elasticsearch enabled' it_behaves_like 'elasticsearch enabled', level: :project
end end
context 'when the project is not indexed' do context 'when the project is not indexed' do
...@@ -291,7 +395,7 @@ describe API::Search do ...@@ -291,7 +395,7 @@ describe API::Search do
stub_ee_application_setting(elasticsearch_limit_indexing: false) stub_ee_application_setting(elasticsearch_limit_indexing: false)
end end
it_behaves_like 'elasticsearch enabled' it_behaves_like 'elasticsearch enabled', level: :project
end end
end end
end end
......
...@@ -4,8 +4,8 @@ module Gitlab ...@@ -4,8 +4,8 @@ module Gitlab
class GroupSearchResults < SearchResults class GroupSearchResults < SearchResults
attr_reader :group attr_reader :group
def initialize(current_user, limit_projects, group, query, default_project_filter: false, per_page: 20) def initialize(current_user, limit_projects, group, query, default_project_filter: false)
super(current_user, limit_projects, query, default_project_filter: default_project_filter, per_page: per_page) super(current_user, limit_projects, query, default_project_filter: default_project_filter)
@group = group @group = group
end end
......
...@@ -2,30 +2,29 @@ ...@@ -2,30 +2,29 @@
module Gitlab module Gitlab
class ProjectSearchResults < SearchResults 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 @current_user = current_user
@project = project @project = project
@repository_ref = repository_ref.presence @repository_ref = repository_ref.presence
@query = query @query = query
@per_page = per_page
end end
def objects(scope, page = nil) def objects(scope, page: nil, per_page: DEFAULT_PER_PAGE)
case scope case scope
when 'notes' when 'notes'
notes.page(page).per(per_page) notes.page(page).per(per_page)
when 'blobs' when 'blobs'
paginated_blobs(blobs(page), page) paginated_blobs(blobs(limit: limit_up_to_page(page, per_page)), page, per_page)
when 'wiki_blobs' 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' when 'commits'
Kaminari.paginate_array(commits).page(page).per(per_page) Kaminari.paginate_array(commits).page(page).per(per_page)
when 'users' when 'users'
users.page(page).per(per_page) users.page(page).per(per_page)
else else
super(scope, page, false) super(scope, page: page, per_page: per_page, without_count: false)
end end
end end
...@@ -49,7 +48,7 @@ module Gitlab ...@@ -49,7 +48,7 @@ module Gitlab
end end
def limited_blobs_count def limited_blobs_count
@limited_blobs_count ||= blobs.count @limited_blobs_count ||= blobs(limit: count_limit).count
end end
# rubocop: disable CodeReuse/ActiveRecord # rubocop: disable CodeReuse/ActiveRecord
...@@ -69,7 +68,7 @@ module Gitlab ...@@ -69,7 +68,7 @@ module Gitlab
# rubocop: enable CodeReuse/ActiveRecord # rubocop: enable CodeReuse/ActiveRecord
def wiki_blobs_count def wiki_blobs_count
@wiki_blobs_count ||= wiki_blobs.count @wiki_blobs_count ||= wiki_blobs(limit: count_limit).count
end end
def commits_count def commits_count
...@@ -87,7 +86,7 @@ module Gitlab ...@@ -87,7 +86,7 @@ module Gitlab
private private
def paginated_blobs(blobs, page) def paginated_blobs(blobs, page, per_page)
results = Kaminari.paginate_array(blobs).page(page).per(per_page) results = Kaminari.paginate_array(blobs).page(page).per(per_page)
Gitlab::Search::FoundBlob.preload_blobs(results) Gitlab::Search::FoundBlob.preload_blobs(results)
...@@ -95,19 +94,19 @@ module Gitlab ...@@ -95,19 +94,19 @@ module Gitlab
results results
end end
def limit_up_to_page(page) def limit_up_to_page(page, per_page)
current_page = page&.to_i || 1 current_page = page&.to_i || 1
offset = per_page * (current_page - 1) offset = per_page * (current_page - 1)
count_limit + offset count_limit + offset
end end
def blobs(page = 1) def blobs(limit: count_limit)
return [] unless Ability.allowed?(@current_user, :download_code, @project) 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 end
def wiki_blobs def wiki_blobs(limit: count_limit)
return [] unless Ability.allowed?(@current_user, :read_wiki, @project) return [] unless Ability.allowed?(@current_user, :read_wiki, @project)
@wiki_blobs ||= begin @wiki_blobs ||= begin
...@@ -115,7 +114,7 @@ module Gitlab ...@@ -115,7 +114,7 @@ module Gitlab
if project.wiki.empty? if project.wiki.empty?
[] []
else else
Gitlab::WikiFileFinder.new(project, repository_wiki_ref).find(query) Gitlab::WikiFileFinder.new(project, repository_wiki_ref).find(query, content_match_cutoff: limit)
end end
else else
[] []
......
...@@ -4,8 +4,10 @@ module Gitlab ...@@ -4,8 +4,10 @@ module Gitlab
class SearchResults class SearchResults
COUNT_LIMIT = 100 COUNT_LIMIT = 100
COUNT_LIMIT_MESSAGE = "#{COUNT_LIMIT - 1}+" 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 # Limit search results by passed projects
# It allows us to search only for projects user has access to # It allows us to search only for projects user has access to
...@@ -17,15 +19,14 @@ module Gitlab ...@@ -17,15 +19,14 @@ module Gitlab
# query # query
attr_reader :default_project_filter 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 @current_user = current_user
@limit_projects = limit_projects || Project.all @limit_projects = limit_projects || Project.all
@query = query @query = query
@default_project_filter = default_project_filter @default_project_filter = default_project_filter
@per_page = per_page
end 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 collection = case scope
when 'projects' when 'projects'
projects projects
...@@ -39,7 +40,9 @@ module Gitlab ...@@ -39,7 +40,9 @@ module Gitlab
users users
else else
Kaminari.paginate_array([]) Kaminari.paginate_array([])
end.page(page).per(per_page) end
collection = collection.page(page).per(per_page)
without_count ? collection.without_count : collection without_count ? collection.without_count : collection
end end
......
...@@ -11,8 +11,8 @@ module Gitlab ...@@ -11,8 +11,8 @@ module Gitlab
@query = query @query = query
end end
def objects(scope, page = nil) def objects(scope, page: nil, per_page: DEFAULT_PER_PAGE)
paginated_objects(snippet_titles, page) paginated_objects(snippet_titles, page, per_page)
end end
def formatted_count(scope) def formatted_count(scope)
...@@ -38,7 +38,7 @@ module Gitlab ...@@ -38,7 +38,7 @@ module Gitlab
snippets.search(query) snippets.search(query)
end end
def paginated_objects(relation, page) def paginated_objects(relation, page, per_page)
relation.page(page).per(per_page) relation.page(page).per(per_page)
end end
......
...@@ -21,7 +21,7 @@ describe 'Global search' do ...@@ -21,7 +21,7 @@ describe 'Global search' do
describe 'I search through the issues and I see pagination' do describe 'I search through the issues and I see pagination' do
before 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) allow(instance).to receive(:per_page).and_return(1)
end end
create_list(:issue, 2, project: project, title: 'initial') create_list(:issue, 2, project: project, title: 'initial')
......
...@@ -45,22 +45,36 @@ describe Gitlab::ProjectSearchResults do ...@@ -45,22 +45,36 @@ describe Gitlab::ProjectSearchResults do
expect(results.formatted_count(scope)).to eq(expected) expect(results.formatted_count(scope)).to eq(expected)
end end
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 end
shared_examples 'general blob search' do |entity_type, blob_kind| shared_examples 'general blob search' do |entity_type, blob_type|
let(:query) { 'files' } let(:query) { 'files' }
subject(:results) { described_class.new(user, project, query).objects(blob_type) } subject(:results) { described_class.new(user, project, query).objects(blob_type) }
context "when #{entity_type} is disabled" do context "when #{entity_type} is disabled" do
let(:project) { disabled_project } let(:project) { disabled_project }
it "hides #{blob_kind} from members" do it "hides #{blob_type} from members" do
project.add_reporter(user) project.add_reporter(user)
is_expected.to be_empty is_expected.to be_empty
end end
it "hides #{blob_kind} from non-members" do it "hides #{blob_type} from non-members" do
is_expected.to be_empty is_expected.to be_empty
end end
end end
...@@ -68,13 +82,13 @@ describe Gitlab::ProjectSearchResults do ...@@ -68,13 +82,13 @@ describe Gitlab::ProjectSearchResults do
context "when #{entity_type} is internal" do context "when #{entity_type} is internal" do
let(:project) { private_project } let(:project) { private_project }
it "finds #{blob_kind} for members" do it "finds #{blob_type} for members" do
project.add_reporter(user) project.add_reporter(user)
is_expected.not_to be_empty is_expected.not_to be_empty
end end
it "hides #{blob_kind} from non-members" do it "hides #{blob_type} from non-members" do
is_expected.to be_empty is_expected.to be_empty
end end
end end
...@@ -96,7 +110,7 @@ describe Gitlab::ProjectSearchResults do ...@@ -96,7 +110,7 @@ describe Gitlab::ProjectSearchResults do
end end
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(:query) { 'files' }
let(:file_finder) { double } let(:file_finder) { double }
let(:project_branch) { 'project_branch' } let(:project_branch) { 'project_branch' }
...@@ -139,9 +153,41 @@ describe Gitlab::ProjectSearchResults do ...@@ -139,9 +153,41 @@ describe Gitlab::ProjectSearchResults do
end end
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 describe 'blob search' do
let(:project) { create(:project, :public, :repository) } let(:project) { create(:project, :public, :repository) }
let(:blob_type) { 'blobs' }
it_behaves_like 'general blob search', 'repository', 'blobs' do it_behaves_like 'general blob search', 'repository', 'blobs' do
let(:disabled_project) { create(:project, :public, :repository, :repository_disabled) } let(:disabled_project) { create(:project, :public, :repository, :repository_disabled) }
...@@ -150,37 +196,11 @@ describe Gitlab::ProjectSearchResults do ...@@ -150,37 +196,11 @@ describe Gitlab::ProjectSearchResults do
let(:expected_file_by_content) { 'CHANGELOG' } let(:expected_file_by_content) { 'CHANGELOG' }
end end
it_behaves_like 'blob search repository ref', 'project' do it_behaves_like 'blob search repository ref', 'project', 'blobs' do
let(:entity) { project } let(:entity) { project }
end end
context 'pagination' do it_behaves_like 'blob search pagination', 'blobs'
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
end end
describe 'wiki search' do describe 'wiki search' do
...@@ -192,7 +212,7 @@ describe Gitlab::ProjectSearchResults do ...@@ -192,7 +212,7 @@ describe Gitlab::ProjectSearchResults do
wiki.create_page('CHANGELOG', 'Files example') wiki.create_page('CHANGELOG', 'Files example')
end 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(:blob_type) { 'wiki_blobs' }
let(:disabled_project) { create(:project, :public, :wiki_repo, :wiki_disabled) } let(:disabled_project) { create(:project, :public, :wiki_repo, :wiki_disabled) }
let(:private_project) { create(:project, :public, :wiki_repo, :wiki_private) } let(:private_project) { create(:project, :public, :wiki_repo, :wiki_private) }
...@@ -200,10 +220,11 @@ describe Gitlab::ProjectSearchResults do ...@@ -200,10 +220,11 @@ describe Gitlab::ProjectSearchResults do
let(:expected_file_by_content) { 'CHANGELOG.md' } let(:expected_file_by_content) { 'CHANGELOG.md' }
end end
it_behaves_like 'blob search repository ref', 'wiki' do it_behaves_like 'blob search repository ref', 'wiki', 'wiki_blobs' do
let(:blob_type) { 'wiki_blobs' }
let(:entity) { project.wiki } let(:entity) { project.wiki }
end end
it_behaves_like 'blob search pagination', 'wiki_blobs'
end end
it 'does not list issues on private projects' do it 'does not list issues on private projects' do
......
...@@ -28,7 +28,15 @@ describe Gitlab::SearchResults do ...@@ -28,7 +28,15 @@ describe Gitlab::SearchResults do
end end
it 'returns with counts collection when requested' do 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
end end
......
...@@ -20,4 +20,14 @@ describe Gitlab::SnippetSearchResults do ...@@ -20,4 +20,14 @@ describe Gitlab::SnippetSearchResults do
expect(results.formatted_count('snippet_titles')).to eq(max_limited_count) expect(results.formatted_count('snippet_titles')).to eq(max_limited_count)
end end
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 end
...@@ -15,10 +15,36 @@ describe API::Search do ...@@ -15,10 +15,36 @@ describe API::Search do
it { expect(json_response.size).to eq(size) } it { expect(json_response.size).to eq(size) }
end end
describe 'GET /search' do 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
describe 'GET /search' do
let(:endpoint) { '/search' }
context 'when user is not authenticated' do context 'when user is not authenticated' do
it 'returns 401 error' do it 'returns 401 error' do
get api('/search'), params: { scope: 'projects', search: 'awesome' } get api(endpoint), params: { scope: 'projects', search: 'awesome' }
expect(response).to have_gitlab_http_status(:unauthorized) expect(response).to have_gitlab_http_status(:unauthorized)
end end
...@@ -26,7 +52,7 @@ describe API::Search do ...@@ -26,7 +52,7 @@ describe API::Search do
context 'when scope is not supported' do context 'when scope is not supported' do
it 'returns 400 error' do it 'returns 400 error' do
get api('/search', user), params: { scope: 'unsupported', search: 'awesome' } get api(endpoint, user), params: { scope: 'unsupported', search: 'awesome' }
expect(response).to have_gitlab_http_status(:bad_request) expect(response).to have_gitlab_http_status(:bad_request)
end end
...@@ -34,7 +60,7 @@ describe API::Search do ...@@ -34,7 +60,7 @@ describe API::Search do
context 'when scope is missing' do context 'when scope is missing' do
it 'returns 400 error' do it 'returns 400 error' do
get api('/search', user), params: { search: 'awesome' } get api(endpoint, user), params: { search: 'awesome' }
expect(response).to have_gitlab_http_status(:bad_request) expect(response).to have_gitlab_http_status(:bad_request)
end end
...@@ -43,30 +69,48 @@ describe API::Search do ...@@ -43,30 +69,48 @@ describe API::Search do
context 'with correct params' do context 'with correct params' do
context 'for projects scope' do context 'for projects scope' do
before do before do
get api('/search', user), params: { scope: 'projects', search: 'awesome' } get api(endpoint, user), params: { scope: 'projects', search: 'awesome' }
end end
it_behaves_like 'response is correct', schema: 'public_api/v4/projects' it_behaves_like 'response is correct', schema: 'public_api/v4/projects'
it_behaves_like 'pagination', scope: :projects
end end
context 'for issues scope' do context 'for issues scope' do
before do before do
create(:issue, project: project, title: 'awesome issue') create(:issue, project: project, title: 'awesome issue')
get api('/search', user), params: { scope: 'issues', search: 'awesome' } get api(endpoint, user), params: { scope: 'issues', search: 'awesome' }
end end
it_behaves_like 'response is correct', schema: 'public_api/v4/issues' it_behaves_like 'response is correct', schema: 'public_api/v4/issues'
describe 'pagination' do
before do
create(:issue, project: project, title: 'another issue')
end
include_examples 'pagination', scope: :issues
end
end end
context 'for merge_requests scope' do context 'for merge_requests scope' do
before do before do
create(:merge_request, source_project: repo_project, title: 'awesome mr') create(:merge_request, source_project: repo_project, title: 'awesome mr')
get api('/search', user), params: { scope: 'merge_requests', search: 'awesome' } get api(endpoint, user), params: { scope: 'merge_requests', search: 'awesome' }
end end
it_behaves_like 'response is correct', schema: 'public_api/v4/merge_requests' it_behaves_like 'response is correct', schema: 'public_api/v4/merge_requests'
describe 'pagination' do
before do
create(:merge_request, source_project: repo_project, title: 'another mr', target_branch: 'another_branch')
end
include_examples 'pagination', scope: :merge_requests
end
end end
context 'for milestones scope' do context 'for milestones scope' do
...@@ -76,10 +120,18 @@ describe API::Search do ...@@ -76,10 +120,18 @@ describe API::Search do
context 'when user can read project milestones' do context 'when user can read project milestones' do
before do before do
get api('/search', user), params: { scope: 'milestones', search: 'awesome' } get api(endpoint, user), params: { scope: 'milestones', search: 'awesome' }
end end
it_behaves_like 'response is correct', schema: 'public_api/v4/milestones' it_behaves_like 'response is correct', schema: 'public_api/v4/milestones'
describe 'pagination' do
before do
create(:milestone, project: project, title: 'another milestone')
end
include_examples 'pagination', scope: :milestones
end
end end
context 'when user cannot read project milestones' do context 'when user cannot read project milestones' do
...@@ -89,7 +141,7 @@ describe API::Search do ...@@ -89,7 +141,7 @@ describe API::Search do
end end
it 'returns empty array' do it 'returns empty array' do
get api('/search', user), params: { scope: 'milestones', search: 'awesome' } get api(endpoint, user), params: { scope: 'milestones', search: 'awesome' }
milestones = json_response milestones = json_response
...@@ -102,16 +154,18 @@ describe API::Search do ...@@ -102,16 +154,18 @@ describe API::Search do
before do before do
create(:user, name: 'billy') create(:user, name: 'billy')
get api('/search', user), params: { scope: 'users', search: 'billy' } get api(endpoint, user), params: { scope: 'users', search: 'billy' }
end end
it_behaves_like 'response is correct', schema: 'public_api/v4/user/basics' it_behaves_like 'response is correct', schema: 'public_api/v4/user/basics'
it_behaves_like 'pagination', scope: :users
context 'when users search feature is disabled' do context 'when users search feature is disabled' do
before do before do
allow(Feature).to receive(:disabled?).with(:users_search, default_enabled: true).and_return(true) allow(Feature).to receive(:disabled?).with(:users_search, default_enabled: true).and_return(true)
get api('/search', user), params: { scope: 'users', search: 'billy' } get api(endpoint, user), params: { scope: 'users', search: 'billy' }
end end
it 'returns 400 error' do it 'returns 400 error' do
...@@ -124,18 +178,28 @@ describe API::Search do ...@@ -124,18 +178,28 @@ describe API::Search do
before do before do
create(:snippet, :public, title: 'awesome snippet', content: 'snippet content') create(:snippet, :public, title: 'awesome snippet', content: 'snippet content')
get api('/search', user), params: { scope: 'snippet_titles', search: 'awesome' } get api(endpoint, user), params: { scope: 'snippet_titles', search: 'awesome' }
end end
it_behaves_like 'response is correct', schema: 'public_api/v4/snippets' it_behaves_like 'response is correct', schema: 'public_api/v4/snippets'
describe 'pagination' do
before do
create(:snippet, :public, title: 'another snippet', content: 'snippet content')
end
include_examples 'pagination', scope: :snippet_titles
end
end end
end end
end end
describe "GET /groups/:id/search" do describe "GET /groups/:id/search" do
let(:endpoint) { "/groups/#{group.id}/-/search" }
context 'when user is not authenticated' do context 'when user is not authenticated' do
it 'returns 401 error' do it 'returns 401 error' do
get api("/groups/#{group.id}/search"), params: { scope: 'projects', search: 'awesome' } get api(endpoint), params: { scope: 'projects', search: 'awesome' }
expect(response).to have_gitlab_http_status(:unauthorized) expect(response).to have_gitlab_http_status(:unauthorized)
end end
...@@ -143,7 +207,7 @@ describe API::Search do ...@@ -143,7 +207,7 @@ describe API::Search do
context 'when scope is not supported' do context 'when scope is not supported' do
it 'returns 400 error' do it 'returns 400 error' do
get api("/groups/#{group.id}/search", user), params: { scope: 'unsupported', search: 'awesome' } get api(endpoint, user), params: { scope: 'unsupported', search: 'awesome' }
expect(response).to have_gitlab_http_status(:bad_request) expect(response).to have_gitlab_http_status(:bad_request)
end end
...@@ -151,7 +215,7 @@ describe API::Search do ...@@ -151,7 +215,7 @@ describe API::Search do
context 'when scope is missing' do context 'when scope is missing' do
it 'returns 400 error' do it 'returns 400 error' do
get api("/groups/#{group.id}/search", user), params: { search: 'awesome' } get api(endpoint, user), params: { search: 'awesome' }
expect(response).to have_gitlab_http_status(:bad_request) expect(response).to have_gitlab_http_status(:bad_request)
end end
...@@ -178,40 +242,66 @@ describe API::Search do ...@@ -178,40 +242,66 @@ describe API::Search do
context 'with correct params' do context 'with correct params' do
context 'for projects scope' do context 'for projects scope' do
before do before do
get api("/groups/#{group.id}/search", user), params: { scope: 'projects', search: 'awesome' } get api(endpoint, user), params: { scope: 'projects', search: 'awesome' }
end end
it_behaves_like 'response is correct', schema: 'public_api/v4/projects' it_behaves_like 'response is correct', schema: 'public_api/v4/projects'
it_behaves_like 'pagination', scope: :projects
end end
context 'for issues scope' do context 'for issues scope' do
before do before do
create(:issue, project: project, title: 'awesome issue') create(:issue, project: project, title: 'awesome issue')
get api("/groups/#{group.id}/search", user), params: { scope: 'issues', search: 'awesome' } get api(endpoint, user), params: { scope: 'issues', search: 'awesome' }
end end
it_behaves_like 'response is correct', schema: 'public_api/v4/issues' it_behaves_like 'response is correct', schema: 'public_api/v4/issues'
describe 'pagination' do
before do
create(:issue, project: project, title: 'another issue')
end
include_examples 'pagination', scope: :issues
end
end end
context 'for merge_requests scope' do context 'for merge_requests scope' do
before do before do
create(:merge_request, source_project: repo_project, title: 'awesome mr') create(:merge_request, source_project: repo_project, title: 'awesome mr')
get api("/groups/#{group.id}/search", user), params: { scope: 'merge_requests', search: 'awesome' } get api(endpoint, user), params: { scope: 'merge_requests', search: 'awesome' }
end end
it_behaves_like 'response is correct', schema: 'public_api/v4/merge_requests' it_behaves_like 'response is correct', schema: 'public_api/v4/merge_requests'
describe 'pagination' do
before do
create(:merge_request, source_project: repo_project, title: 'another mr', target_branch: 'another_branch')
end
include_examples 'pagination', scope: :merge_requests
end
end end
context 'for milestones scope' do context 'for milestones scope' do
before do before do
create(:milestone, project: project, title: 'awesome milestone') create(:milestone, project: project, title: 'awesome milestone')
get api("/groups/#{group.id}/search", user), params: { scope: 'milestones', search: 'awesome' } get api(endpoint, user), params: { scope: 'milestones', search: 'awesome' }
end end
it_behaves_like 'response is correct', schema: 'public_api/v4/milestones' it_behaves_like 'response is correct', schema: 'public_api/v4/milestones'
describe 'pagination' do
before do
create(:milestone, project: project, title: 'another milestone')
end
include_examples 'pagination', scope: :milestones
end
end end
context 'for milestones scope with group path as id' do context 'for milestones scope with group path as id' do
...@@ -231,16 +321,24 @@ describe API::Search do ...@@ -231,16 +321,24 @@ describe API::Search do
user = create(:user, name: 'billy') user = create(:user, name: 'billy')
create(:group_member, :developer, user: user, group: group) create(:group_member, :developer, user: user, group: group)
get api("/groups/#{group.id}/search", user), params: { scope: 'users', search: 'billy' } get api(endpoint, user), params: { scope: 'users', search: 'billy' }
end end
it_behaves_like 'response is correct', schema: 'public_api/v4/user/basics' it_behaves_like 'response is correct', schema: 'public_api/v4/user/basics'
describe 'pagination' do
before do
create(:group_member, :developer, group: group)
end
include_examples 'pagination', scope: :users
end
context 'when users search feature is disabled' do context 'when users search feature is disabled' do
before do before do
allow(Feature).to receive(:disabled?).with(:users_search, default_enabled: true).and_return(true) allow(Feature).to receive(:disabled?).with(:users_search, default_enabled: true).and_return(true)
get api("/groups/#{group.id}/search", user), params: { scope: 'users', search: 'billy' } get api(endpoint, user), params: { scope: 'users', search: 'billy' }
end end
it 'returns 400 error' do it 'returns 400 error' do
...@@ -263,9 +361,11 @@ describe API::Search do ...@@ -263,9 +361,11 @@ describe API::Search do
end end
describe "GET /projects/:id/search" do describe "GET /projects/:id/search" do
let(:endpoint) { "/projects/#{project.id}/search" }
context 'when user is not authenticated' do context 'when user is not authenticated' do
it 'returns 401 error' do it 'returns 401 error' do
get api("/projects/#{project.id}/search"), params: { scope: 'issues', search: 'awesome' } get api(endpoint), params: { scope: 'issues', search: 'awesome' }
expect(response).to have_gitlab_http_status(:unauthorized) expect(response).to have_gitlab_http_status(:unauthorized)
end end
...@@ -273,7 +373,7 @@ describe API::Search do ...@@ -273,7 +373,7 @@ describe API::Search do
context 'when scope is not supported' do context 'when scope is not supported' do
it 'returns 400 error' do it 'returns 400 error' do
get api("/projects/#{project.id}/search", user), params: { scope: 'unsupported', search: 'awesome' } get api(endpoint, user), params: { scope: 'unsupported', search: 'awesome' }
expect(response).to have_gitlab_http_status(:bad_request) expect(response).to have_gitlab_http_status(:bad_request)
end end
...@@ -281,7 +381,7 @@ describe API::Search do ...@@ -281,7 +381,7 @@ describe API::Search do
context 'when scope is missing' do context 'when scope is missing' do
it 'returns 400 error' do it 'returns 400 error' do
get api("/projects/#{project.id}/search", user), params: { search: 'awesome' } get api(endpoint, user), params: { search: 'awesome' }
expect(response).to have_gitlab_http_status(:bad_request) expect(response).to have_gitlab_http_status(:bad_request)
end end
...@@ -299,7 +399,7 @@ describe API::Search do ...@@ -299,7 +399,7 @@ describe API::Search do
it 'returns 404 error' do it 'returns 404 error' do
project.update!(visibility_level: Gitlab::VisibilityLevel::PRIVATE) project.update!(visibility_level: Gitlab::VisibilityLevel::PRIVATE)
get api("/projects/#{project.id}/search", user), params: { scope: 'issues', search: 'awesome' } get api(endpoint, user), params: { scope: 'issues', search: 'awesome' }
expect(response).to have_gitlab_http_status(:not_found) expect(response).to have_gitlab_http_status(:not_found)
end end
...@@ -310,20 +410,38 @@ describe API::Search do ...@@ -310,20 +410,38 @@ describe API::Search do
before do before do
create(:issue, project: project, title: 'awesome issue') create(:issue, project: project, title: 'awesome issue')
get api("/projects/#{project.id}/search", user), params: { scope: 'issues', search: 'awesome' } get api(endpoint, user), params: { scope: 'issues', search: 'awesome' }
end end
it_behaves_like 'response is correct', schema: 'public_api/v4/issues' it_behaves_like 'response is correct', schema: 'public_api/v4/issues'
describe 'pagination' do
before do
create(:issue, project: project, title: 'another issue')
end
include_examples 'pagination', scope: :issues
end
end end
context 'for merge_requests scope' do context 'for merge_requests scope' do
let(:endpoint) { "/projects/#{repo_project.id}/search" }
before do before do
create(:merge_request, source_project: repo_project, title: 'awesome mr') create(:merge_request, source_project: repo_project, title: 'awesome mr')
get api("/projects/#{repo_project.id}/search", user), params: { scope: 'merge_requests', search: 'awesome' } get api(endpoint, user), params: { scope: 'merge_requests', search: 'awesome' }
end end
it_behaves_like 'response is correct', schema: 'public_api/v4/merge_requests' it_behaves_like 'response is correct', schema: 'public_api/v4/merge_requests'
describe 'pagination' do
before do
create(:merge_request, source_project: repo_project, title: 'another mr', target_branch: 'another_branch')
end
include_examples 'pagination', scope: :merge_requests
end
end end
context 'for milestones scope' do context 'for milestones scope' do
...@@ -333,10 +451,18 @@ describe API::Search do ...@@ -333,10 +451,18 @@ describe API::Search do
context 'when user can read milestones' do context 'when user can read milestones' do
before do before do
get api("/projects/#{project.id}/search", user), params: { scope: 'milestones', search: 'awesome' } get api(endpoint, user), params: { scope: 'milestones', search: 'awesome' }
end end
it_behaves_like 'response is correct', schema: 'public_api/v4/milestones' it_behaves_like 'response is correct', schema: 'public_api/v4/milestones'
describe 'pagination' do
before do
create(:milestone, project: project, title: 'another milestone')
end
include_examples 'pagination', scope: :milestones
end
end end
context 'when user cannot read project milestones' do context 'when user cannot read project milestones' do
...@@ -346,7 +472,7 @@ describe API::Search do ...@@ -346,7 +472,7 @@ describe API::Search do
end end
it 'returns empty array' do it 'returns empty array' do
get api("/projects/#{project.id}/search", user), params: { scope: 'milestones', search: 'awesome' } get api(endpoint, user), params: { scope: 'milestones', search: 'awesome' }
milestones = json_response milestones = json_response
...@@ -360,16 +486,24 @@ describe API::Search do ...@@ -360,16 +486,24 @@ describe API::Search do
user1 = create(:user, name: 'billy') user1 = create(:user, name: 'billy')
create(:project_member, :developer, user: user1, project: project) create(:project_member, :developer, user: user1, project: project)
get api("/projects/#{project.id}/search", user), params: { scope: 'users', search: 'billy' } get api(endpoint, user), params: { scope: 'users', search: 'billy' }
end end
it_behaves_like 'response is correct', schema: 'public_api/v4/user/basics' it_behaves_like 'response is correct', schema: 'public_api/v4/user/basics'
describe 'pagination' do
before do
create(:project_member, :developer, project: project)
end
include_examples 'pagination', scope: :users
end
context 'when users search feature is disabled' do context 'when users search feature is disabled' do
before do before do
allow(Feature).to receive(:disabled?).with(:users_search, default_enabled: true).and_return(true) allow(Feature).to receive(:disabled?).with(:users_search, default_enabled: true).and_return(true)
get api("/projects/#{project.id}/search", user), params: { scope: 'users', search: 'billy' } get api(endpoint, user), params: { scope: 'users', search: 'billy' }
end end
it 'returns 400 error' do it 'returns 400 error' do
...@@ -382,29 +516,51 @@ describe API::Search do ...@@ -382,29 +516,51 @@ describe API::Search do
before do before do
create(:note_on_merge_request, project: project, note: 'awesome note') create(:note_on_merge_request, project: project, note: 'awesome note')
get api("/projects/#{project.id}/search", user), params: { scope: 'notes', search: 'awesome' } get api(endpoint, user), params: { scope: 'notes', search: 'awesome' }
end end
it_behaves_like 'response is correct', schema: 'public_api/v4/notes' it_behaves_like 'response is correct', schema: 'public_api/v4/notes'
describe 'pagination' do
before do
mr = create(:merge_request, source_project: project, target_branch: 'another_branch')
create(:note, project: project, noteable: mr, note: 'another note')
end
include_examples 'pagination', scope: :notes
end
end end
context 'for wiki_blobs scope' do context 'for wiki_blobs scope' do
let(:wiki) { create(:project_wiki, project: project) }
before 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: 'home', content: "Awesome page")
get api("/projects/#{project.id}/search", user), params: { scope: 'wiki_blobs', search: 'awesome' } get api(endpoint, user), params: { scope: 'wiki_blobs', search: 'awesome' }
end end
it_behaves_like 'response is correct', schema: 'public_api/v4/blobs' it_behaves_like 'response is correct', schema: 'public_api/v4/blobs'
describe 'pagination' do
before do
create(:wiki_page, wiki: wiki, title: 'home 2', content: 'Another page')
end
include_examples 'pagination', scope: :wiki_blobs, search: 'page'
end
end end
context 'for commits scope' do context 'for commits scope' do
let(:endpoint) { "/projects/#{repo_project.id}/search" }
before do before do
get api("/projects/#{repo_project.id}/search", user), params: { scope: 'commits', search: '498214de67004b1da3d820901307bed2a68a8ef6' } get api(endpoint, user), params: { scope: 'commits', search: '498214de67004b1da3d820901307bed2a68a8ef6' }
end end
it_behaves_like 'response is correct', schema: 'public_api/v4/commits_details' it_behaves_like 'response is correct', schema: 'public_api/v4/commits_details'
it_behaves_like 'pagination', scope: :commits, search: 'merge'
end end
context 'for commits scope with project path as id' do context 'for commits scope with project path as id' do
...@@ -416,15 +572,19 @@ describe API::Search do ...@@ -416,15 +572,19 @@ describe API::Search do
end end
context 'for blobs scope' do context 'for blobs scope' do
let(:endpoint) { "/projects/#{repo_project.id}/search" }
before do before do
get api("/projects/#{repo_project.id}/search", user), params: { scope: 'blobs', search: 'monitors' } get api(endpoint, user), params: { scope: 'blobs', search: 'monitors' }
end end
it_behaves_like 'response is correct', schema: 'public_api/v4/blobs', size: 2 it_behaves_like 'response is correct', schema: 'public_api/v4/blobs', size: 2
it_behaves_like 'pagination', scope: :blobs, search: 'monitors'
context 'filters' do context 'filters' do
it 'by filename' do it 'by filename' do
get api("/projects/#{repo_project.id}/search", user), params: { scope: 'blobs', search: 'mon filename:PROCESS.md' } get api(endpoint, user), params: { scope: 'blobs', search: 'mon filename:PROCESS.md' }
expect(response).to have_gitlab_http_status(:ok) expect(response).to have_gitlab_http_status(:ok)
expect(json_response.size).to eq(2) expect(json_response.size).to eq(2)
...@@ -433,21 +593,21 @@ describe API::Search do ...@@ -433,21 +593,21 @@ describe API::Search do
end end
it 'by path' do it 'by path' do
get api("/projects/#{repo_project.id}/search", user), params: { scope: 'blobs', search: 'mon path:markdown' } get api(endpoint, user), params: { scope: 'blobs', search: 'mon path:markdown' }
expect(response).to have_gitlab_http_status(:ok) expect(response).to have_gitlab_http_status(:ok)
expect(json_response.size).to eq(8) expect(json_response.size).to eq(8)
end end
it 'by extension' do it 'by extension' do
get api("/projects/#{repo_project.id}/search", user), params: { scope: 'blobs', search: 'mon extension:md' } get api(endpoint, user), params: { scope: 'blobs', search: 'mon extension:md' }
expect(response).to have_gitlab_http_status(:ok) expect(response).to have_gitlab_http_status(:ok)
expect(json_response.size).to eq(11) expect(json_response.size).to eq(11)
end end
it 'by ref' do it 'by ref' do
get api("/projects/#{repo_project.id}/search", user), params: { scope: 'blobs', search: 'This file is used in tests for ci_environments_status', ref: 'pages-deploy' } get api(endpoint, user), params: { scope: 'blobs', search: 'This file is used in tests for ci_environments_status', ref: 'pages-deploy' }
expect(response).to have_gitlab_http_status(:ok) expect(response).to have_gitlab_http_status(:ok)
expect(json_response.size).to eq(1) expect(json_response.size).to eq(1)
......
...@@ -18,7 +18,9 @@ describe SearchService do ...@@ -18,7 +18,9 @@ describe SearchService do
let(:group_project) { create(:project, group: accessible_group, name: 'group_project') } let(:group_project) { create(:project, group: accessible_group, name: 'group_project') }
let(:public_project) { create(:project, :public, name: 'public_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 before do
accessible_project.add_maintainer(user) accessible_project.add_maintainer(user)
...@@ -240,6 +242,76 @@ describe SearchService do ...@@ -240,6 +242,76 @@ describe SearchService do
end end
describe '#search_objects' do 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 context 'with accessible project_id' do
it 'returns objects in the project' do it 'returns objects in the project' do
search_objects = described_class.new( search_objects = described_class.new(
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment