Merge branch 'users-autocomplete' into 'master'
Improve performance of searching for and auto completing of users See merge request gitlab-org/gitlab-ce!17158
This commit is contained in:
commit
bb0fe96f75
11 changed files with 78 additions and 19 deletions
|
@ -14,7 +14,6 @@ export default class DropdownUser extends FilteredSearchDropdown {
|
|||
endpoint: `${gon.relative_url_root || ''}/autocomplete/users.json`,
|
||||
searchKey: 'search',
|
||||
params: {
|
||||
per_page: 20,
|
||||
active: true,
|
||||
group_id: this.getGroupId(),
|
||||
project_id: this.getProjectId(),
|
||||
|
|
|
@ -39,7 +39,6 @@ function UsersSelect(currentUser, els, options = {}) {
|
|||
options.showCurrentUser = $dropdown.data('currentUser');
|
||||
options.todoFilter = $dropdown.data('todoFilter');
|
||||
options.todoStateFilter = $dropdown.data('todoStateFilter');
|
||||
options.perPage = $dropdown.data('perPage');
|
||||
showNullUser = $dropdown.data('nullUser');
|
||||
defaultNullUser = $dropdown.data('nullUserDefault');
|
||||
showMenuAbove = $dropdown.data('showMenuAbove');
|
||||
|
@ -669,7 +668,6 @@ UsersSelect.prototype.users = function(query, options, callback) {
|
|||
const url = this.buildUrl(this.usersPath);
|
||||
const params = {
|
||||
search: query,
|
||||
per_page: options.perPage || 20,
|
||||
active: true,
|
||||
project_id: options.projectId || null,
|
||||
group_id: options.groupId || null,
|
||||
|
|
|
@ -1,6 +1,12 @@
|
|||
class AutocompleteUsersFinder
|
||||
# The number of users to display in the results is hardcoded to 20, and
|
||||
# pagination is not supported. This ensures that performance remains
|
||||
# consistent and removes the need for implementing keyset pagination to ensure
|
||||
# good performance.
|
||||
LIMIT = 20
|
||||
|
||||
attr_reader :current_user, :project, :group, :search, :skip_users,
|
||||
:page, :per_page, :author_id, :params
|
||||
:author_id, :params
|
||||
|
||||
def initialize(params:, current_user:, project:, group:)
|
||||
@current_user = current_user
|
||||
|
@ -8,8 +14,6 @@ class AutocompleteUsersFinder
|
|||
@group = group
|
||||
@search = params[:search]
|
||||
@skip_users = params[:skip_users]
|
||||
@page = params[:page]
|
||||
@per_page = params[:per_page]
|
||||
@author_id = params[:author_id]
|
||||
@params = params
|
||||
end
|
||||
|
@ -20,7 +24,7 @@ class AutocompleteUsersFinder
|
|||
items = items.reorder(:name)
|
||||
items = items.search(search) if search.present?
|
||||
items = items.where.not(id: skip_users) if skip_users.present?
|
||||
items = items.page(page).per(per_page)
|
||||
items = items.limit(LIMIT)
|
||||
|
||||
if params[:todo_filter].present? && current_user
|
||||
items = items.todo_authors(current_user.id, params[:todo_state_filter])
|
||||
|
@ -52,9 +56,13 @@ class AutocompleteUsersFinder
|
|||
end
|
||||
|
||||
def users_from_project
|
||||
user_ids = project.team.users.pluck(:id)
|
||||
user_ids << author_id if author_id.present?
|
||||
if author_id.present?
|
||||
union = Gitlab::SQL::Union
|
||||
.new([project.authorized_users, User.where(id: author_id)])
|
||||
|
||||
User.where(id: user_ids)
|
||||
User.from("(#{union.to_sql}) #{User.table_name}")
|
||||
else
|
||||
project.authorized_users
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -327,8 +327,8 @@ class User < ActiveRecord::Base
|
|||
SQL
|
||||
|
||||
where(
|
||||
fuzzy_arel_match(:name, query)
|
||||
.or(fuzzy_arel_match(:username, query))
|
||||
fuzzy_arel_match(:name, query, lower_exact_match: true)
|
||||
.or(fuzzy_arel_match(:username, query, lower_exact_match: true))
|
||||
.or(arel_table[:email].eq(query))
|
||||
).reorder(order % { query: ActiveRecord::Base.connection.quote(query) }, :name)
|
||||
end
|
||||
|
|
5
changelogs/unreleased/users-autocomplete.yml
Normal file
5
changelogs/unreleased/users-autocomplete.yml
Normal file
|
@ -0,0 +1,5 @@
|
|||
---
|
||||
title: Improve performance of searching for and autocompleting of users
|
||||
merge_request:
|
||||
author:
|
||||
type: performance
|
29
db/migrate/20180215181245_users_name_lower_index.rb
Normal file
29
db/migrate/20180215181245_users_name_lower_index.rb
Normal file
|
@ -0,0 +1,29 @@
|
|||
# See http://doc.gitlab.com/ce/development/migration_style_guide.html
|
||||
# for more information on how to write migrations for GitLab.
|
||||
|
||||
class UsersNameLowerIndex < ActiveRecord::Migration
|
||||
include Gitlab::Database::MigrationHelpers
|
||||
|
||||
# Set this constant to true if this migration requires downtime.
|
||||
DOWNTIME = false
|
||||
INDEX_NAME = 'index_on_users_name_lower'
|
||||
|
||||
disable_ddl_transaction!
|
||||
|
||||
def up
|
||||
return unless Gitlab::Database.postgresql?
|
||||
|
||||
# On GitLab.com this produces an index with a size of roughly 60 MB.
|
||||
execute "CREATE INDEX CONCURRENTLY #{INDEX_NAME} ON users (LOWER(name))"
|
||||
end
|
||||
|
||||
def down
|
||||
return unless Gitlab::Database.postgresql?
|
||||
|
||||
if supports_drop_index_concurrently?
|
||||
execute "DROP INDEX CONCURRENTLY IF EXISTS #{INDEX_NAME}"
|
||||
else
|
||||
execute "DROP INDEX IF EXISTS #{INDEX_NAME}"
|
||||
end
|
||||
end
|
||||
end
|
|
@ -11,7 +11,7 @@
|
|||
#
|
||||
# It's strongly recommended that you check this file into your version control system.
|
||||
|
||||
ActiveRecord::Schema.define(version: 20180213131630) do
|
||||
ActiveRecord::Schema.define(version: 20180215181245) do
|
||||
|
||||
# These are extensions that must be enabled in order to support this database
|
||||
enable_extension "plpgsql"
|
||||
|
|
|
@ -25,7 +25,11 @@ module Gitlab
|
|||
query.length >= MIN_CHARS_FOR_PARTIAL_MATCHING
|
||||
end
|
||||
|
||||
def fuzzy_arel_match(column, query)
|
||||
# column - The column name to search in.
|
||||
# query - The text to search for.
|
||||
# lower_exact_match - When set to `true` we'll fall back to using
|
||||
# `LOWER(column) = query` instead of using `ILIKE`.
|
||||
def fuzzy_arel_match(column, query, lower_exact_match: false)
|
||||
query = query.squish
|
||||
return nil unless query.present?
|
||||
|
||||
|
@ -36,7 +40,13 @@ module Gitlab
|
|||
else
|
||||
# No words of at least 3 chars, but we can search for an exact
|
||||
# case insensitive match with the query as a whole
|
||||
arel_table[column].matches(sanitize_sql_like(query))
|
||||
if lower_exact_match
|
||||
Arel::Nodes::NamedFunction
|
||||
.new('LOWER', [arel_table[column]])
|
||||
.eq(query)
|
||||
else
|
||||
arel_table[column].matches(sanitize_sql_like(query))
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
|
|
|
@ -8,6 +8,7 @@ task setup_postgresql: :environment do
|
|||
require Rails.root.join('db/migrate/20170503185032_index_redirect_routes_path_for_like')
|
||||
require Rails.root.join('db/migrate/20171220191323_add_index_on_namespaces_lower_name.rb')
|
||||
require Rails.root.join('db/migrate/20180113220114_rework_redirect_routes_indexes.rb')
|
||||
require Rails.root.join('db/migrate/20180215181245_users_name_lower_index.rb')
|
||||
|
||||
NamespacesProjectsPathLowerIndexes.new.up
|
||||
AddUsersLowerUsernameEmailIndexes.new.up
|
||||
|
@ -17,4 +18,5 @@ task setup_postgresql: :environment do
|
|||
IndexRedirectRoutesPathForLike.new.up
|
||||
AddIndexOnNamespacesLowerName.new.up
|
||||
ReworkRedirectRoutesIndexes.new.up
|
||||
UsersNameLowerIndex.new.up
|
||||
end
|
||||
|
|
|
@ -109,15 +109,17 @@ describe AutocompleteController do
|
|||
end
|
||||
|
||||
context 'limited users per page' do
|
||||
let(:per_page) { 2 }
|
||||
|
||||
before do
|
||||
25.times do
|
||||
create(:user)
|
||||
end
|
||||
|
||||
sign_in(user)
|
||||
get(:users, per_page: per_page)
|
||||
get(:users)
|
||||
end
|
||||
|
||||
it { expect(json_response).to be_kind_of(Array) }
|
||||
it { expect(json_response.size).to eq(per_page) }
|
||||
it { expect(json_response.size).to eq(20) }
|
||||
end
|
||||
|
||||
context 'unauthenticated user' do
|
||||
|
|
|
@ -154,6 +154,12 @@ describe Gitlab::SQL::Pattern do
|
|||
it 'returns a single equality condition' do
|
||||
expect(fuzzy_arel_match.to_sql).to match(/title.*I?LIKE 'fo'/)
|
||||
end
|
||||
|
||||
it 'uses LOWER instead of ILIKE when LOWER is enabled' do
|
||||
rel = Issue.fuzzy_arel_match(:title, query, lower_exact_match: true)
|
||||
|
||||
expect(rel.to_sql).to match(/LOWER\(.*title.*\).*=.*'fo'/)
|
||||
end
|
||||
end
|
||||
|
||||
context 'with two words both equal to 3 chars' do
|
||||
|
|
Loading…
Reference in a new issue