summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDongqing Hu <sorra@outlook.com>2017-03-31 13:03:55 +0000
committerRémy Coutable <remy@rymai.me>2017-03-31 13:03:55 +0000
commitb0ab0e4eff22ad35fcbad12c3feac6e8ac8b3822 (patch)
tree631e242fa529383e20921f8057f89be66f982f18
parent53b21c1e8e27a284a8691a73276204a85694eeb8 (diff)
downloadgitlab-ce-b0ab0e4eff22ad35fcbad12c3feac6e8ac8b3822.tar.gz
Refactor SearchController#show
-rw-r--r--app/controllers/search_controller.rb40
-rw-r--r--app/services/search/global_service.rb4
-rw-r--r--app/services/search/project_service.rb4
-rw-r--r--app/services/search/snippet_service.rb4
-rw-r--r--app/services/search_service.rb63
-rw-r--r--spec/services/search/global_service_spec.rb66
-rw-r--r--spec/services/search_service_spec.rb299
7 files changed, 408 insertions, 72 deletions
diff --git a/app/controllers/search_controller.rb b/app/controllers/search_controller.rb
index 612d69cf557..4a579601785 100644
--- a/app/controllers/search_controller.rb
+++ b/app/controllers/search_controller.rb
@@ -6,45 +6,19 @@ class SearchController < ApplicationController
layout 'search'
def show
- if params[:project_id].present?
- @project = Project.find_by(id: params[:project_id])
- @project = nil unless can?(current_user, :download_code, @project)
- end
+ search_service = SearchService.new(current_user, params)
- if params[:group_id].present?
- @group = Group.find_by(id: params[:group_id])
- @group = nil unless can?(current_user, :read_group, @group)
- end
+ @project = search_service.project
+ @group = search_service.group
return if params[:search].blank?
@search_term = params[:search]
- @scope = params[:scope]
- @show_snippets = params[:snippets].eql? 'true'
-
- @search_results =
- if @project
- unless %w(blobs notes issues merge_requests milestones wiki_blobs
- commits).include?(@scope)
- @scope = 'blobs'
- end
-
- Search::ProjectService.new(@project, current_user, params).execute
- elsif @show_snippets
- unless %w(snippet_blobs snippet_titles).include?(@scope)
- @scope = 'snippet_blobs'
- end
-
- Search::SnippetService.new(current_user, params).execute
- else
- unless %w(projects issues merge_requests milestones).include?(@scope)
- @scope = 'projects'
- end
- Search::GlobalService.new(current_user, params).execute
- end
-
- @search_objects = @search_results.objects(@scope, params[:page])
+ @scope = search_service.scope
+ @show_snippets = search_service.show_snippets?
+ @search_results = search_service.search_results
+ @search_objects = search_service.search_objects
check_single_commit_result
end
diff --git a/app/services/search/global_service.rb b/app/services/search/global_service.rb
index 781cd13b44b..a3c655493a5 100644
--- a/app/services/search/global_service.rb
+++ b/app/services/search/global_service.rb
@@ -16,5 +16,9 @@ module Search
Gitlab::SearchResults.new(current_user, projects, params[:search])
end
+
+ def scope
+ @scope ||= %w[issues merge_requests milestones].delete(params[:scope]) { 'projects' }
+ end
end
end
diff --git a/app/services/search/project_service.rb b/app/services/search/project_service.rb
index 4b500914cfb..9a22abae635 100644
--- a/app/services/search/project_service.rb
+++ b/app/services/search/project_service.rb
@@ -12,5 +12,9 @@ module Search
params[:search],
params[:repository_ref])
end
+
+ def scope
+ @scope ||= %w[notes issues merge_requests milestones wiki_blobs commits].delete(params[:scope]) { 'blobs' }
+ end
end
end
diff --git a/app/services/search/snippet_service.rb b/app/services/search/snippet_service.rb
index 0b3e713e220..4f161beea4d 100644
--- a/app/services/search/snippet_service.rb
+++ b/app/services/search/snippet_service.rb
@@ -11,5 +11,9 @@ module Search
Gitlab::SnippetSearchResults.new(snippets, params[:search])
end
+
+ def scope
+ @scope ||= %w[snippet_titles].delete(params[:scope]) { 'snippet_blobs' }
+ end
end
end
diff --git a/app/services/search_service.rb b/app/services/search_service.rb
new file mode 100644
index 00000000000..8d46a8dab3e
--- /dev/null
+++ b/app/services/search_service.rb
@@ -0,0 +1,63 @@
+class SearchService
+ include Gitlab::Allowable
+
+ def initialize(current_user, params = {})
+ @current_user = current_user
+ @params = params.dup
+ end
+
+ def project
+ return @project if defined?(@project)
+
+ @project =
+ if params[:project_id].present?
+ the_project = Project.find_by(id: params[:project_id])
+ can?(current_user, :download_code, the_project) ? the_project : nil
+ else
+ nil
+ end
+ end
+
+ def group
+ return @group if defined?(@group)
+
+ @group =
+ if params[:group_id].present?
+ the_group = Group.find_by(id: params[:group_id])
+ can?(current_user, :read_group, the_group) ? the_group : nil
+ else
+ nil
+ end
+ end
+
+ def show_snippets?
+ return @show_snippets if defined?(@show_snippets)
+
+ @show_snippets = params[:snippets] == 'true'
+ end
+
+ delegate :scope, to: :search_service
+
+ def search_results
+ @search_results ||= search_service.execute
+ end
+
+ def search_objects
+ @search_objects ||= search_results.objects(scope, params[:page])
+ end
+
+ private
+
+ def search_service
+ @search_service ||=
+ if project
+ Search::ProjectService.new(project, current_user, params)
+ elsif show_snippets?
+ Search::SnippetService.new(current_user, params)
+ else
+ Search::GlobalService.new(current_user, params)
+ end
+ end
+
+ attr_reader :current_user, :params
+end
diff --git a/spec/services/search/global_service_spec.rb b/spec/services/search/global_service_spec.rb
new file mode 100644
index 00000000000..2531607acad
--- /dev/null
+++ b/spec/services/search/global_service_spec.rb
@@ -0,0 +1,66 @@
+require 'spec_helper'
+
+describe Search::GlobalService, services: true do
+ let(:user) { create(:user) }
+ let(:internal_user) { create(:user) }
+
+ let!(:found_project) { create(:empty_project, :private, name: 'searchable_project') }
+ let!(:unfound_project) { create(:empty_project, :private, name: 'unfound_project') }
+ let!(:internal_project) { create(:empty_project, :internal, name: 'searchable_internal_project') }
+ let!(:public_project) { create(:empty_project, :public, name: 'searchable_public_project') }
+
+ before do
+ found_project.add_master(user)
+ end
+
+ describe '#execute' do
+ context 'unauthenticated' do
+ it 'returns public projects only' do
+ results = Search::GlobalService.new(nil, search: "searchable").execute
+
+ expect(results.objects('projects')).to match_array [public_project]
+ end
+ end
+
+ context 'authenticated' do
+ it 'returns public, internal and private projects' do
+ results = Search::GlobalService.new(user, search: "searchable").execute
+
+ expect(results.objects('projects')).to match_array [public_project, found_project, internal_project]
+ end
+
+ it 'returns only public & internal projects' do
+ results = Search::GlobalService.new(internal_user, search: "searchable").execute
+
+ expect(results.objects('projects')).to match_array [internal_project, public_project]
+ end
+
+ it 'namespace name is searchable' do
+ results = Search::GlobalService.new(user, search: found_project.namespace.path).execute
+
+ expect(results.objects('projects')).to match_array [found_project]
+ end
+
+ context 'nested group' do
+ let!(:nested_group) { create(:group, :nested) }
+ let!(:project) { create(:empty_project, namespace: nested_group) }
+
+ before do
+ project.add_master(user)
+ end
+
+ it 'returns result from nested group' do
+ results = Search::GlobalService.new(user, search: project.path).execute
+
+ expect(results.objects('projects')).to match_array [project]
+ end
+
+ it 'returns result from descendants when search inside group' do
+ results = Search::GlobalService.new(user, search: project.path, group_id: nested_group.parent).execute
+
+ expect(results.objects('projects')).to match_array [project]
+ end
+ end
+ end
+ end
+end
diff --git a/spec/services/search_service_spec.rb b/spec/services/search_service_spec.rb
index 6ef5fa008aa..2112f1cf9ea 100644
--- a/spec/services/search_service_spec.rb
+++ b/spec/services/search_service_spec.rb
@@ -1,65 +1,286 @@
require 'spec_helper'
-describe 'Search::GlobalService', services: true do
+describe SearchService, services: true do
let(:user) { create(:user) }
- let(:public_user) { create(:user) }
- let(:internal_user) { create(:user) }
- let!(:found_project) { create(:empty_project, :private, name: 'searchable_project') }
- let!(:unfound_project) { create(:empty_project, :private, name: 'unfound_project') }
- let!(:internal_project) { create(:empty_project, :internal, name: 'searchable_internal_project') }
- let!(:public_project) { create(:empty_project, :public, name: 'searchable_public_project') }
+ let(:accessible_group) { create(:group, :private) }
+ let(:inaccessible_group) { create(:group, :private) }
+ let!(:group_member) { create(:group_member, group: accessible_group, user: user) }
+
+ let!(:accessible_project) { create(:empty_project, :private, name: 'accessible_project') }
+ let!(:inaccessible_project) { create(:empty_project, :private, name: 'inaccessible_project') }
+ let(:note) { create(:note_on_issue, project: accessible_project) }
+
+ let(:snippet) { create(:snippet, author: user) }
+ let(:group_project) { create(:empty_project, group: accessible_group, name: 'group_project') }
+ let(:public_project) { create(:empty_project, :public, name: 'public_project') }
before do
- found_project.team << [user, :master]
+ accessible_project.add_master(user)
+ end
+
+ describe '#project' do
+ context 'when the project is accessible' do
+ it 'returns the project' do
+ project = SearchService.new(user, project_id: accessible_project.id).project
+
+ expect(project).to eq accessible_project
+ end
+ end
+
+ context 'when the project is not accessible' do
+ it 'returns nil' do
+ project = SearchService.new(user, project_id: inaccessible_project.id).project
+
+ expect(project).to be_nil
+ end
+ end
+
+ context 'when there is no project_id' do
+ it 'returns nil' do
+ project = SearchService.new(user).project
+
+ expect(project).to be_nil
+ end
+ end
end
- describe '#execute' do
- context 'unauthenticated' do
- it 'returns public projects only' do
- context = Search::GlobalService.new(nil, search: "searchable")
- results = context.execute
- expect(results.objects('projects')).to match_array [public_project]
+ describe '#group' do
+ context 'when the group is accessible' do
+ it 'returns the group' do
+ group = SearchService.new(user, group_id: accessible_group.id).group
+
+ expect(group).to eq accessible_group
end
end
- context 'authenticated' do
- it 'returns public, internal and private projects' do
- context = Search::GlobalService.new(user, search: "searchable")
- results = context.execute
- expect(results.objects('projects')).to match_array [public_project, found_project, internal_project]
+ context 'when the group is not accessible' do
+ it 'returns nil' do
+ group = SearchService.new(user, group_id: inaccessible_group.id).group
+
+ expect(group).to be_nil
end
+ end
+
+ context 'when there is no group_id' do
+ it 'returns nil' do
+ group = SearchService.new(user).group
- it 'returns only public & internal projects' do
- context = Search::GlobalService.new(internal_user, search: "searchable")
- results = context.execute
- expect(results.objects('projects')).to match_array [internal_project, public_project]
+ expect(group).to be_nil
end
+ end
+ end
+
+ describe '#show_snippets?' do
+ context 'when :snippets is \'true\'' do
+ it 'returns true' do
+ show_snippets = SearchService.new(user, snippets: 'true').show_snippets?
- it 'namespace name is searchable' do
- context = Search::GlobalService.new(user, search: found_project.namespace.path)
- results = context.execute
- expect(results.objects('projects')).to match_array [found_project]
+ expect(show_snippets).to be_truthy
end
+ end
- context 'nested group' do
- let!(:nested_group) { create(:group, :nested) }
- let!(:project) { create(:empty_project, namespace: nested_group) }
+ context 'when :snippets is not \'true\'' do
+ it 'returns false' do
+ show_snippets = SearchService.new(user, snippets: 'tru').show_snippets?
+
+ expect(show_snippets).to be_falsey
+ end
+ end
- before { project.add_master(user) }
+ context 'when :snippets is missing' do
+ it 'returns false' do
+ show_snippets = SearchService.new(user).show_snippets?
- it 'returns result from nested group' do
- context = Search::GlobalService.new(user, search: project.path)
- results = context.execute
- expect(results.objects('projects')).to match_array [project]
+ expect(show_snippets).to be_falsey
+ end
+ end
+ end
+
+ describe '#scope' do
+ context 'with accessible project_id' do
+ context 'and allowed scope' do
+ it 'returns the specified scope' do
+ scope = SearchService.new(user, project_id: accessible_project.id, scope: 'notes').scope
+
+ expect(scope).to eq 'notes'
end
+ end
+
+ context 'and disallowed scope' do
+ it 'returns the default scope' do
+ scope = SearchService.new(user, project_id: accessible_project.id, scope: 'projects').scope
- it 'returns result from descendants when search inside group' do
- context = Search::GlobalService.new(user, search: project.path, group_id: nested_group.parent)
- results = context.execute
- expect(results.objects('projects')).to match_array [project]
+ expect(scope).to eq 'blobs'
end
end
+
+ context 'and no scope' do
+ it 'returns the default scope' do
+ scope = SearchService.new(user, project_id: accessible_project.id).scope
+
+ expect(scope).to eq 'blobs'
+ end
+ end
+ end
+
+ context 'with \'true\' snippets' do
+ context 'and allowed scope' do
+ it 'returns the specified scope' do
+ scope = SearchService.new(user, snippets: 'true', scope: 'snippet_titles').scope
+
+ expect(scope).to eq 'snippet_titles'
+ end
+ end
+
+ context 'and disallowed scope' do
+ it 'returns the default scope' do
+ scope = SearchService.new(user, snippets: 'true', scope: 'projects').scope
+
+ expect(scope).to eq 'snippet_blobs'
+ end
+ end
+
+ context 'and no scope' do
+ it 'returns the default scope' do
+ scope = SearchService.new(user, snippets: 'true').scope
+
+ expect(scope).to eq 'snippet_blobs'
+ end
+ end
+ end
+
+ context 'with no project_id, no snippets' do
+ context 'and allowed scope' do
+ it 'returns the specified scope' do
+ scope = SearchService.new(user, scope: 'issues').scope
+
+ expect(scope).to eq 'issues'
+ end
+ end
+
+ context 'and disallowed scope' do
+ it 'returns the default scope' do
+ scope = SearchService.new(user, scope: 'blobs').scope
+
+ expect(scope).to eq 'projects'
+ end
+ end
+
+ context 'and no scope' do
+ it 'returns the default scope' do
+ scope = SearchService.new(user).scope
+
+ expect(scope).to eq 'projects'
+ end
+ end
+ end
+ end
+
+ describe '#search_results' do
+ context 'with accessible project_id' do
+ it 'returns an instance of Gitlab::ProjectSearchResults' do
+ search_results = SearchService.new(
+ user,
+ project_id: accessible_project.id,
+ scope: 'notes',
+ search: note.note).search_results
+
+ expect(search_results).to be_a Gitlab::ProjectSearchResults
+ end
+ end
+
+ context 'with accessible project_id and \'true\' snippets' do
+ it 'returns an instance of Gitlab::ProjectSearchResults' do
+ search_results = SearchService.new(
+ user,
+ project_id: accessible_project.id,
+ snippets: 'true',
+ scope: 'notes',
+ search: note.note).search_results
+
+ expect(search_results).to be_a Gitlab::ProjectSearchResults
+ end
+ end
+
+ context 'with \'true\' snippets' do
+ it 'returns an instance of Gitlab::SnippetSearchResults' do
+ search_results = SearchService.new(
+ user,
+ snippets: 'true',
+ search: snippet.content).search_results
+
+ expect(search_results).to be_a Gitlab::SnippetSearchResults
+ end
+ end
+
+ context 'with no project_id and no snippets' do
+ it 'returns an instance of Gitlab::SearchResults' do
+ search_results = SearchService.new(
+ user,
+ search: public_project.name).search_results
+
+ expect(search_results).to be_a Gitlab::SearchResults
+ end
+ end
+ end
+
+ describe '#search_objects' do
+ context 'with accessible project_id' do
+ it 'returns objects in the project' do
+ search_objects = SearchService.new(
+ user,
+ project_id: accessible_project.id,
+ scope: 'notes',
+ search: note.note).search_objects
+
+ expect(search_objects.first).to eq note
+ end
+ end
+
+ context 'with accessible project_id and \'true\' snippets' do
+ it 'returns objects in the project' do
+ search_objects = SearchService.new(
+ user,
+ project_id: accessible_project.id,
+ snippets: 'true',
+ scope: 'notes',
+ search: note.note).search_objects
+
+ expect(search_objects.first).to eq note
+ end
+ end
+
+ context 'with \'true\' snippets' do
+ it 'returns objects in snippets' do
+ search_objects = SearchService.new(
+ user,
+ snippets: 'true',
+ search: snippet.content).search_objects
+
+ expect(search_objects.first).to eq snippet
+ end
+ end
+
+ context 'with accessible group_id' do
+ it 'returns objects in the group' do
+ search_objects = SearchService.new(
+ user,
+ group_id: accessible_group.id,
+ search: group_project.name).search_objects
+
+ expect(search_objects.first).to eq group_project
+ end
+ end
+
+ context 'with no project_id, group_id or snippets' do
+ it 'returns objects in global' do
+ search_objects = SearchService.new(
+ user,
+ search: public_project.name).search_objects
+
+ expect(search_objects.first).to eq public_project
+ end
end
end
end