Commit 01354296 authored by Yorick Peterse's avatar Yorick Peterse Committed by Robert Speicher
Browse files

Refactor Gitlab::SearchResults

Instead of plucking IDs this class now uses ActiveRecord::Relation
objects. Plucking IDs is problematic as searching for projects can lead
to a huge amount of IDs being loaded into memory only to be used as an
argument for another query (instead of just using a sub-query).
parent ed412174df11
......@@ -10,9 +10,8 @@ def execute
group = Group.find_by(id: params[:group_id]) if params[:group_id].present?
projects = ProjectsFinder.new.execute(current_user)
projects = projects.in_namespace(group.id) if group
project_ids = projects.pluck(:id)
Gitlab::SearchResults.new(project_ids, params[:search])
Gitlab::SearchResults.new(projects, params[:search])
end
end
end
......@@ -2,12 +2,12 @@ module Gitlab
class SearchResults
attr_reader :query
# Limit search results by passed project ids
# Limit search results by passed projects
# It allows us to search only for projects user has access to
attr_reader :limit_project_ids
attr_reader :limit_projects
def initialize(limit_project_ids, query)
@limit_project_ids = limit_project_ids || Project.all
def initialize(limit_projects, query)
@limit_projects = limit_projects || Project.all
@query = Shellwords.shellescape(query) if query.present?
end
......@@ -27,7 +27,8 @@ def objects(scope, page = nil)
end
def total_count
@total_count ||= projects_count + issues_count + merge_requests_count + milestones_count
@total_count ||= projects_count + issues_count + merge_requests_count +
milestones_count
end
def projects_count
......@@ -53,27 +54,29 @@ def empty?
private
def projects
Project.where(id: limit_project_ids).search(query)
limit_projects.search(query)
end
def issues
issues = Issue.where(project_id: limit_project_ids)
issues = Issue.where(project_id: project_ids_relation)
if query =~ /#(\d+)\z/
issues = issues.where(iid: $1)
else
issues = issues.full_search(query)
end
issues.order('updated_at DESC')
end
def milestones
milestones = Milestone.where(project_id: limit_project_ids)
milestones = Milestone.where(project_id: project_ids_relation)
milestones = milestones.search(query)
milestones.order('updated_at DESC')
end
def merge_requests
merge_requests = MergeRequest.in_projects(limit_project_ids)
merge_requests = MergeRequest.in_projects(project_ids_relation)
if query =~ /[#!](\d+)\z/
merge_requests = merge_requests.where(iid: $1)
else
......@@ -89,5 +92,9 @@ def default_scope
def per_page
20
end
def project_ids_relation
limit_projects.select(:id)
end
end
end
require 'spec_helper'
describe Gitlab::SearchResults do
let!(:project) { create(:project, name: 'foo') }
let!(:issue) { create(:issue, project: project, title: 'foo') }
let!(:merge_request) do
create(:merge_request, source_project: project, title: 'foo')
end
let!(:milestone) { create(:milestone, project: project, title: 'foo') }
let(:results) { described_class.new(Project.all, 'foo') }
describe '#total_count' do
it 'returns the total amount of search hits' do
expect(results.total_count).to eq(4)
end
end
describe '#projects_count' do
it 'returns the total amount of projects' do
expect(results.projects_count).to eq(1)
end
end
describe '#issues_count' do
it 'returns the total amount of issues' do
expect(results.issues_count).to eq(1)
end
end
describe '#merge_requests_count' do
it 'returns the total amount of merge requests' do
expect(results.merge_requests_count).to eq(1)
end
end
describe '#milestones_count' do
it 'returns the total amount of milestones' do
expect(results.milestones_count).to eq(1)
end
end
describe '#empty?' do
it 'returns true when there are no search results' do
allow(results).to receive(:total_count).and_return(0)
expect(results.empty?).to eq(true)
end
it 'returns false when there are search results' do
expect(results.empty?).to eq(false)
end
end
end
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