Skip to content
Snippets Groups Projects
Verified Commit 49c83155 authored by Markus Koller's avatar Markus Koller
Browse files

Load search result counts asynchronously

Querying all counts for the different search results in the same request
led to timeouts, so we now only calculate the count for the *current*
search results, and request the others in separate asynchronous calls.
parent 71ec7932
No related branches found
No related tags found
No related merge requests found
Showing
with 352 additions and 176 deletions
Loading
Loading
@@ -36,6 +36,15 @@ class SearchController < ApplicationController
check_single_commit_result
end
 
def count
params.require([:search, :scope])
scope = search_service.scope
count = search_service.search_results.formatted_count(scope)
render json: { count: count }
end
# rubocop: disable CodeReuse/ActiveRecord
def autocomplete
term = params[:term]
Loading
Loading
Loading
Loading
@@ -145,17 +145,28 @@ module SearchHelper
Sanitize.clean(str)
end
 
def search_filter_path(options = {})
exist_opts = {
search: params[:search],
project_id: params[:project_id],
group_id: params[:group_id],
scope: params[:scope],
repository_ref: params[:repository_ref]
}
def search_filter_link(scope, label, data: {}, search: {})
search_params = params
.merge(search)
.merge({ scope: scope })
.permit(:search, :scope, :project_id, :group_id, :repository_ref, :snippets)
if @scope == scope
li_class = 'active'
count = @search_results.formatted_count(scope)
else
count = 0
badge_class = 'js-search-count'
badge_data = { scope: scope, url: search_count_path(search_params) }
end
 
options = exist_opts.merge(options)
search_path(options)
content_tag :li, class: li_class, data: data do
link_to search_path(search_params) do
concat label
concat ' '
concat content_tag(:span, count, class: 'badge badge-pill', data: { scope: scope })
end
end
end
 
def search_filter_input_options(type)
Loading
Loading
@@ -212,10 +223,6 @@ module SearchHelper
sanitize(html, tags: %w(a p ol ul li pre code))
end
 
def limited_count(count, limit = 1000)
count > limit ? "#{limit}+" : count
end
def search_tabs?(tab)
return false if Feature.disabled?(:users_search, default_enabled: true)
 
Loading
Loading
Loading
Loading
@@ -47,5 +47,6 @@
= hidden_field_tag :snippets, true
= hidden_field_tag :repository_ref, @ref
= hidden_field_tag :nav_source, 'navbar'
-# workaround for non-JS feature specs, for JS you need to use find('#search').send_keys(:enter)
= button_tag 'Go' if ENV['RAILS_ENV'] == 'test'
.search-autocomplete-opts.hide{ :'data-autocomplete-path' => search_autocomplete_path, :'data-autocomplete-project-id' => @project.try(:id), :'data-autocomplete-project-ref' => @ref }
- users = capture_haml do
- if search_tabs?(:members)
%li{ class: active_when(@scope == 'users') }
= link_to search_filter_path(scope: 'users') do
Users
%span.badge.badge-pill
= limited_count(@search_results.limited_users_count)
= search_filter_link 'users', _("Users")
 
.scrolling-tabs-container.inner-page-scroll-tabs.is-smaller
.fade-left= icon('angle-left')
Loading
Loading
@@ -12,80 +8,28 @@
%ul.nav-links.search-filter.scrolling-tabs.nav.nav-tabs
- if @project
- if project_search_tabs?(:blobs)
%li{ class: active_when(@scope == 'blobs'), data: { qa_selector: 'code_tab' } }
= link_to search_filter_path(scope: 'blobs') do
= _("Code")
%span.badge.badge-pill
= @search_results.blobs_count
= search_filter_link 'blobs', _("Code"), data: { qa_selector: 'code_tab' }
- if project_search_tabs?(:issues)
%li{ class: active_when(@scope == 'issues') }
= link_to search_filter_path(scope: 'issues') do
= _("Issues")
%span.badge.badge-pill
= limited_count(@search_results.limited_issues_count)
= search_filter_link 'issues', _("Issues")
- if project_search_tabs?(:merge_requests)
%li{ class: active_when(@scope == 'merge_requests') }
= link_to search_filter_path(scope: 'merge_requests') do
= _("Merge requests")
%span.badge.badge-pill
= limited_count(@search_results.limited_merge_requests_count)
= search_filter_link 'merge_requests', _("Merge requests")
- if project_search_tabs?(:milestones)
%li{ class: active_when(@scope == 'milestones') }
= link_to search_filter_path(scope: 'milestones') do
= _("Milestones")
%span.badge.badge-pill
= limited_count(@search_results.limited_milestones_count)
= search_filter_link 'milestones', _("Milestones")
- if project_search_tabs?(:notes)
%li{ class: active_when(@scope == 'notes') }
= link_to search_filter_path(scope: 'notes') do
= _("Comments")
%span.badge.badge-pill
= limited_count(@search_results.limited_notes_count)
= search_filter_link 'notes', _("Comments")
- if project_search_tabs?(:wiki)
%li{ class: active_when(@scope == 'wiki_blobs') }
= link_to search_filter_path(scope: 'wiki_blobs') do
= _("Wiki")
%span.badge.badge-pill
= @search_results.wiki_blobs_count
= search_filter_link 'wiki_blobs', _("Wiki")
- if project_search_tabs?(:commits)
%li{ class: active_when(@scope == 'commits') }
= link_to search_filter_path(scope: 'commits') do
= _("Commits")
%span.badge.badge-pill
= @search_results.commits_count
= search_filter_link 'commits', _("Commits")
= users
 
- elsif @show_snippets
%li{ class: active_when(@scope == 'snippet_blobs') }
= link_to search_filter_path(scope: 'snippet_blobs', snippets: true, group_id: nil, project_id: nil) do
= _("Snippet Contents")
%span.badge.badge-pill
= @search_results.snippet_blobs_count
%li{ class: active_when(@scope == 'snippet_titles') }
= link_to search_filter_path(scope: 'snippet_titles', snippets: true, group_id: nil, project_id: nil) do
= _("Titles and Filenames")
%span.badge.badge-pill
= @search_results.snippet_titles_count
= search_filter_link 'snippet_blobs', _("Snippet Contents"), search: { snippets: true, group_id: nil, project_id: nil }
= search_filter_link 'snippet_titles', _("Titles and Filenames"), search: { snippets: true, group_id: nil, project_id: nil }
- else
%li{ class: active_when(@scope == 'projects') }
= link_to search_filter_path(scope: 'projects') do
= _("Projects")
%span.badge.badge-pill
= limited_count(@search_results.limited_projects_count)
%li{ class: active_when(@scope == 'issues') }
= link_to search_filter_path(scope: 'issues') do
= _("Issues")
%span.badge.badge-pill
= limited_count(@search_results.limited_issues_count)
%li{ class: active_when(@scope == 'merge_requests') }
= link_to search_filter_path(scope: 'merge_requests') do
= _("Merge requests")
%span.badge.badge-pill
= limited_count(@search_results.limited_merge_requests_count)
%li{ class: active_when(@scope == 'milestones') }
= link_to search_filter_path(scope: 'milestones') do
= _("Milestones")
%span.badge.badge-pill
= limited_count(@search_results.limited_milestones_count)
= search_filter_link 'projects', _("Projects")
= search_filter_link 'issues', _("Issues")
= search_filter_link 'merge_requests', _("Merge requests")
= search_filter_link 'milestones', _("Milestones")
= render_if_exists 'search/category_elasticsearch'
= users
---
title: Load search result counts asynchronously
merge_request: 31663
author:
type: changed
Loading
Loading
@@ -58,6 +58,7 @@ Rails.application.routes.draw do
# Search
get 'search' => 'search#show'
get 'search/autocomplete' => 'search#autocomplete', as: :search_autocomplete
get 'search/count' => 'search#count', as: :search_count
 
# JSON Web Token
get 'jwt/auth' => 'jwt#auth'
Loading
Loading
Loading
Loading
@@ -29,6 +29,21 @@ module Gitlab
end
end
 
def formatted_count(scope)
case scope
when 'blobs'
blobs_count.to_s
when 'notes'
formatted_limited_count(limited_notes_count)
when 'wiki_blobs'
wiki_blobs_count.to_s
when 'commits'
commits_count.to_s
else
super
end
end
def users
super.where(id: @project.team.members) # rubocop:disable CodeReuse/ActiveRecord
end
Loading
Loading
Loading
Loading
@@ -43,6 +43,29 @@ module Gitlab
without_count ? collection.without_count : collection
end
 
def formatted_count(scope)
case scope
when 'projects'
formatted_limited_count(limited_projects_count)
when 'issues'
formatted_limited_count(limited_issues_count)
when 'merge_requests'
formatted_limited_count(limited_merge_requests_count)
when 'milestones'
formatted_limited_count(limited_milestones_count)
when 'users'
formatted_limited_count(limited_users_count)
end
end
def formatted_limited_count(count)
if count >= COUNT_LIMIT
"#{COUNT_LIMIT - 1}+"
else
count.to_s
end
end
def limited_projects_count
@limited_projects_count ||= limited_count(projects)
end
Loading
Loading
Loading
Loading
@@ -22,6 +22,17 @@ module Gitlab
end
end
 
def formatted_count(scope)
case scope
when 'snippet_titles'
snippet_titles_count.to_s
when 'snippet_blobs'
snippet_blobs_count.to_s
else
super
end
end
def snippet_titles_count
@snippet_titles_count ||= snippet_titles.count
end
Loading
Loading
Loading
Loading
@@ -11,151 +11,173 @@ describe SearchController do
sign_in(user)
end
 
context 'uses the right partials depending on scope' do
using RSpec::Parameterized::TableSyntax
render_views
set(:project) { create(:project, :public, :repository, :wiki_repo) }
shared_examples_for 'when the user cannot read cross project' do |action, params|
before do
expect(::Gitlab::GitalyClient).to receive(:allow_ref_name_caching).and_call_original
allow(Ability).to receive(:allowed?).and_call_original
allow(Ability).to receive(:allowed?)
.with(user, :read_cross_project, :global) { false }
end
 
subject { get(:show, params: { project_id: project.id, scope: scope, search: 'merge' }) }
it 'blocks access without a project_id' do
get action, params: params
 
where(:partial, :scope) do
'_blob' | :blobs
'_wiki_blob' | :wiki_blobs
'_commit' | :commits
expect(response).to have_gitlab_http_status(403)
end
 
with_them do
it do
project_wiki = create(:project_wiki, project: project, user: user)
create(:wiki_page, wiki: project_wiki, attrs: { title: 'merge', content: 'merge' })
it 'allows access with a project_id' do
get action, params: params.merge(project_id: create(:project, :public).id)
 
expect(subject).to render_template("search/results/#{partial}")
end
expect(response).to have_gitlab_http_status(200)
end
end
 
context 'global search' do
render_views
it 'omits pipeline status from load' do
project = create(:project, :public)
expect(Gitlab::Cache::Ci::ProjectPipelineStatus).not_to receive(:load_in_batch_for_projects)
get :show, params: { scope: 'projects', search: project.name }
shared_examples_for 'with external authorization service enabled' do |action, params|
let(:project) { create(:project, namespace: user.namespace) }
let(:note) { create(:note_on_issue, project: project) }
 
expect(assigns[:search_objects].first).to eq project
before do
enable_external_authorization_service_check
end
end
it 'finds issue comments' do
project = create(:project, :public)
note = create(:note_on_issue, project: project)
 
get :show, params: { project_id: project.id, scope: 'notes', search: note.note }
it 'renders a 403 when no project is given' do
get action, params: params
 
expect(assigns[:search_objects].first).to eq note
end
context 'when the user cannot read cross project' do
before do
allow(Ability).to receive(:allowed?).and_call_original
allow(Ability).to receive(:allowed?)
.with(user, :read_cross_project, :global) { false }
expect(response).to have_gitlab_http_status(403)
end
 
it 'still allows accessing the search page' do
get :show
it 'renders a 200 when a project was set' do
get action, params: params.merge(project_id: project.id)
 
expect(response).to have_gitlab_http_status(200)
end
end
 
it 'still blocks searches without a project_id' do
get :show, params: { search: 'hello' }
describe 'GET #show' do
it_behaves_like 'when the user cannot read cross project', :show, { search: 'hello' } do
it 'still allows accessing the search page' do
get :show
 
expect(response).to have_gitlab_http_status(403)
expect(response).to have_gitlab_http_status(200)
end
end
 
it 'allows searches with a project_id' do
get :show, params: { search: 'hello', project_id: create(:project, :public).id }
it_behaves_like 'with external authorization service enabled', :show, { search: 'hello' }
 
expect(response).to have_gitlab_http_status(200)
end
end
context 'uses the right partials depending on scope' do
using RSpec::Parameterized::TableSyntax
render_views
set(:project) { create(:project, :public, :repository, :wiki_repo) }
 
context 'on restricted projects' do
context 'when signed out' do
before do
sign_out(user)
expect(::Gitlab::GitalyClient).to receive(:allow_ref_name_caching).and_call_original
end
 
it "doesn't expose comments on issues" do
project = create(:project, :public, :issues_private)
note = create(:note_on_issue, project: project)
subject { get(:show, params: { project_id: project.id, scope: scope, search: 'merge' }) }
 
get :show, params: { project_id: project.id, scope: 'notes', search: note.note }
where(:partial, :scope) do
'_blob' | :blobs
'_wiki_blob' | :wiki_blobs
'_commit' | :commits
end
 
expect(assigns[:search_objects].count).to eq(0)
with_them do
it do
project_wiki = create(:project_wiki, project: project, user: user)
create(:wiki_page, wiki: project_wiki, attrs: { title: 'merge', content: 'merge' })
expect(subject).to render_template("search/results/#{partial}")
end
end
end
 
it "doesn't expose comments on merge_requests" do
project = create(:project, :public, :merge_requests_private)
note = create(:note_on_merge_request, project: project)
context 'global search' do
render_views
 
get :show, params: { project_id: project.id, scope: 'notes', search: note.note }
it 'omits pipeline status from load' do
project = create(:project, :public)
expect(Gitlab::Cache::Ci::ProjectPipelineStatus).not_to receive(:load_in_batch_for_projects)
get :show, params: { scope: 'projects', search: project.name }
 
expect(assigns[:search_objects].count).to eq(0)
expect(assigns[:search_objects].first).to eq project
end
end
 
it "doesn't expose comments on snippets" do
project = create(:project, :public, :snippets_private)
note = create(:note_on_project_snippet, project: project)
it 'finds issue comments' do
project = create(:project, :public)
note = create(:note_on_issue, project: project)
 
get :show, params: { project_id: project.id, scope: 'notes', search: note.note }
 
expect(assigns[:search_objects].count).to eq(0)
expect(assigns[:search_objects].first).to eq note
end
end
 
context 'with external authorization service enabled' do
let(:project) { create(:project, namespace: user.namespace) }
let(:note) { create(:note_on_issue, project: project) }
context 'on restricted projects' do
context 'when signed out' do
before do
sign_out(user)
end
 
before do
enable_external_authorization_service_check
end
it "doesn't expose comments on issues" do
project = create(:project, :public, :issues_private)
note = create(:note_on_issue, project: project)
 
describe 'GET #show' do
it 'renders a 403 when no project is given' do
get :show, params: { scope: 'notes', search: note.note }
get :show, params: { project_id: project.id, scope: 'notes', search: note.note }
 
expect(response).to have_gitlab_http_status(403)
expect(assigns[:search_objects].count).to eq(0)
end
end
 
it 'renders a 200 when a project was set' do
it "doesn't expose comments on merge_requests" do
project = create(:project, :public, :merge_requests_private)
note = create(:note_on_merge_request, project: project)
get :show, params: { project_id: project.id, scope: 'notes', search: note.note }
 
expect(response).to have_gitlab_http_status(200)
expect(assigns[:search_objects].count).to eq(0)
end
end
 
describe 'GET #autocomplete' do
it 'renders a 403 when no project is given' do
get :autocomplete, params: { term: 'hello' }
it "doesn't expose comments on snippets" do
project = create(:project, :public, :snippets_private)
note = create(:note_on_project_snippet, project: project)
 
expect(response).to have_gitlab_http_status(403)
get :show, params: { project_id: project.id, scope: 'notes', search: note.note }
expect(assigns[:search_objects].count).to eq(0)
end
end
end
 
it 'renders a 200 when a project was set' do
get :autocomplete, params: { project_id: project.id, term: 'hello' }
describe 'GET #count' do
it_behaves_like 'when the user cannot read cross project', :count, { search: 'hello', scope: 'projects' }
it_behaves_like 'with external authorization service enabled', :count, { search: 'hello', scope: 'projects' }
 
expect(response).to have_gitlab_http_status(200)
end
it 'returns the result count for the given term and scope' do
create(:project, :public, name: 'hello world')
create(:project, :public, name: 'foo bar')
get :count, params: { search: 'hello', scope: 'projects' }
expect(response).to have_gitlab_http_status(200)
expect(json_response).to eq({ 'count' => '1' })
end
it 'raises an error if search term is missing' do
expect do
get :count, params: { scope: 'projects' }
end.to raise_error(ActionController::ParameterMissing)
end
it 'raises an error if search scope is missing' do
expect do
get :count, params: { search: 'hello' }
end.to raise_error(ActionController::ParameterMissing)
end
end
describe 'GET #autocomplete' do
it_behaves_like 'when the user cannot read cross project', :autocomplete, { term: 'hello' }
it_behaves_like 'with external authorization service enabled', :autocomplete, { term: 'hello' }
end
end
Loading
Loading
@@ -4,7 +4,7 @@ require 'spec_helper'
 
describe 'User searches for users' do
context 'when on the dashboard' do
it 'finds the user' do
it 'finds the user', :js do
create(:user, username: 'gob_bluth', name: 'Gob Bluth')
 
sign_in(create(:user))
Loading
Loading
@@ -12,7 +12,7 @@ describe 'User searches for users' do
visit dashboard_projects_path
 
fill_in 'search', with: 'gob'
click_button 'Go'
find('#search').send_keys(:enter)
 
expect(page).to have_content('Users 1')
 
Loading
Loading
Loading
Loading
@@ -96,6 +96,23 @@ describe 'User uses header search field', :js do
let(:url) { root_path }
let(:scope_name) { 'All GitLab' }
end
context 'when searching through the search field' do
before do
create(:issue, project: project, title: 'project issue')
fill_in('search', with: 'project')
find('#search').send_keys(:enter)
end
it 'displays result counts for all categories' do
expect(page).to have_content('Projects 1')
expect(page).to have_content('Issues 1')
expect(page).to have_content('Merge requests 0')
expect(page).to have_content('Milestones 0')
expect(page).to have_content('Users 0')
end
end
end
 
context 'when user is in a project scope' do
Loading
Loading
Loading
Loading
@@ -177,4 +177,48 @@ describe SearchHelper do
end
end
end
describe 'search_filter_link' do
it 'renders a search filter link for the current scope' do
@scope = 'projects'
@search_results = double
expect(@search_results).to receive(:formatted_count).with('projects').and_return('23')
link = search_filter_link('projects', 'Projects')
expect(link).to have_css('li.active')
expect(link).to have_link('Projects', href: search_path(scope: 'projects'))
expect(link).to have_css('span.badge.badge-pill:not(.js-search-count):not(.hidden):not([data-url])', text: '23')
end
it 'renders a search filter link for another scope' do
link = search_filter_link('projects', 'Projects')
count_path = search_count_path(scope: 'projects')
expect(link).to have_css('li:not([class="active"])')
expect(link).to have_link('Projects', href: search_path(scope: 'projects'))
expect(link).to have_css("span.badge.badge-pill.js-search-count.hidden[data-url='#{count_path}']", text: '')
end
it 'merges in the current search params and given params' do
expect(self).to receive(:params).and_return(
ActionController::Parameters.new(
search: 'hello',
scope: 'ignored',
other_param: 'ignored'
)
)
link = search_filter_link('projects', 'Projects', search: { project_id: 23 })
expect(link).to have_link('Projects', href: search_path(scope: 'projects', search: 'hello', project_id: 23))
end
it 'assigns given data attributes on the list container' do
link = search_filter_link('projects', 'Projects', data: { foo: 'bar' })
expect(link).to have_css('li[data-foo="bar"]')
end
end
end
Loading
Loading
@@ -22,6 +22,28 @@ describe Gitlab::ProjectSearchResults do
it { expect(results.query).to eq('hello world') }
end
 
describe '#formatted_count' do
using RSpec::Parameterized::TableSyntax
let(:results) { described_class.new(user, project, query) }
where(:scope, :count_method, :expected) do
'blobs' | :blobs_count | '1234'
'notes' | :limited_notes_count | '1000+'
'wiki_blobs' | :wiki_blobs_count | '1234'
'commits' | :commits_count | '1234'
'projects' | :limited_projects_count | '1000+'
'unknown' | nil | nil
end
with_them do
it 'returns the expected formatted count' do
expect(results).to receive(count_method).and_return(1234) if count_method
expect(results.formatted_count(scope)).to eq(expected)
end
end
end
shared_examples 'general blob search' do |entity_type, blob_kind|
let(:query) { 'files' }
subject(:results) { described_class.new(user, project, query).objects(blob_type) }
Loading
Loading
Loading
Loading
@@ -29,6 +29,43 @@ describe Gitlab::SearchResults do
end
end
 
describe '#formatted_count' do
using RSpec::Parameterized::TableSyntax
where(:scope, :count_method, :expected) do
'projects' | :limited_projects_count | '1000+'
'issues' | :limited_issues_count | '1000+'
'merge_requests' | :limited_merge_requests_count | '1000+'
'milestones' | :limited_milestones_count | '1000+'
'users' | :limited_users_count | '1000+'
'unknown' | nil | nil
end
with_them do
it 'returns the expected formatted count' do
expect(results).to receive(count_method).and_return(1234) if count_method
expect(results.formatted_count(scope)).to eq(expected)
end
end
end
describe '#formatted_limited_count' do
using RSpec::Parameterized::TableSyntax
where(:count, :expected) do
23 | '23'
1000 | '1000'
1001 | '1000+'
1234 | '1000+'
end
with_them do
it 'returns the expected formatted limited count' do
expect(results.formatted_limited_count(count)).to eq(expected)
end
end
end
context "when count_limit is lower than total amount" do
before do
allow(results).to receive(:count_limit).and_return(1)
Loading
Loading
Loading
Loading
@@ -16,4 +16,22 @@ describe Gitlab::SnippetSearchResults do
expect(results.snippet_blobs_count).to eq(1)
end
end
describe '#formatted_count' do
using RSpec::Parameterized::TableSyntax
where(:scope, :count_method, :expected) do
'snippet_titles' | :snippet_titles_count | '1234'
'snippet_blobs' | :snippet_blobs_count | '1234'
'projects' | :limited_projects_count | '1000+'
'unknown' | nil | nil
end
with_them do
it 'returns the expected formatted count' do
expect(results).to receive(count_method).and_return(1234) if count_method
expect(results.formatted_count(scope)).to eq(expected)
end
end
end
end
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment