Commit 8274e0fe authored by Yorick Peterse's avatar Yorick Peterse

Merge branch 'improve-autocomplete-user-performance' into 'master'

Improve AutocompleteController#users.json performance

Closes #36879

See merge request !13754
parents 978b4b9c 12633b46
...@@ -5,6 +5,7 @@ class User < ActiveRecord::Base ...@@ -5,6 +5,7 @@ class User < ActiveRecord::Base
include Gitlab::ConfigHelper include Gitlab::ConfigHelper
include Gitlab::CurrentSettings include Gitlab::CurrentSettings
include Gitlab::SQL::Pattern
include Avatarable include Avatarable
include Referable include Referable
include Sortable include Sortable
...@@ -303,7 +304,7 @@ class User < ActiveRecord::Base ...@@ -303,7 +304,7 @@ class User < ActiveRecord::Base
# Returns an ActiveRecord::Relation. # Returns an ActiveRecord::Relation.
def search(query) def search(query)
table = arel_table table = arel_table
pattern = "%#{query}%" pattern = User.to_pattern(query)
order = <<~SQL order = <<~SQL
CASE CASE
......
---
title: Improve performance for AutocompleteController#users.json
merge_request: 13754
author: Hiroyuki Sato
type: changed
module Gitlab
module SQL
module Pattern
extend ActiveSupport::Concern
MIN_CHARS_FOR_PARTIAL_MATCHING = 3
class_methods do
def to_pattern(query)
if partial_matching?(query)
"%#{sanitize_sql_like(query)}%"
else
sanitize_sql_like(query)
end
end
def partial_matching?(query)
query.length >= MIN_CHARS_FOR_PARTIAL_MATCHING
end
end
end
end
end
...@@ -6,7 +6,7 @@ describe 'Dropdown author', js: true do ...@@ -6,7 +6,7 @@ describe 'Dropdown author', js: true do
let!(:project) { create(:project) } let!(:project) { create(:project) }
let!(:user) { create(:user, name: 'administrator', username: 'root') } let!(:user) { create(:user, name: 'administrator', username: 'root') }
let!(:user_john) { create(:user, name: 'John', username: 'th0mas') } let!(:user_john) { create(:user, name: 'John', username: 'th0mas') }
let!(:user_jacob) { create(:user, name: 'Jacob', username: 'otter32') } let!(:user_jacob) { create(:user, name: 'Jacob', username: 'ooter32') }
let(:filtered_search) { find('.filtered-search') } let(:filtered_search) { find('.filtered-search') }
let(:js_dropdown_author) { '#js-dropdown-author' } let(:js_dropdown_author) { '#js-dropdown-author' }
...@@ -82,31 +82,31 @@ describe 'Dropdown author', js: true do ...@@ -82,31 +82,31 @@ describe 'Dropdown author', js: true do
end end
it 'filters by name' do it 'filters by name' do
send_keys_to_filtered_search('ja') send_keys_to_filtered_search('jac')
expect(dropdown_author_size).to eq(1) expect(dropdown_author_size).to eq(1)
end end
it 'filters by case insensitive name' do it 'filters by case insensitive name' do
send_keys_to_filtered_search('Ja') send_keys_to_filtered_search('Jac')
expect(dropdown_author_size).to eq(1) expect(dropdown_author_size).to eq(1)
end end
it 'filters by username with symbol' do it 'filters by username with symbol' do
send_keys_to_filtered_search('@ot') send_keys_to_filtered_search('@oot')
expect(dropdown_author_size).to eq(2) expect(dropdown_author_size).to eq(2)
end end
it 'filters by username without symbol' do it 'filters by username without symbol' do
send_keys_to_filtered_search('ot') send_keys_to_filtered_search('oot')
expect(dropdown_author_size).to eq(2) expect(dropdown_author_size).to eq(2)
end end
it 'filters by case insensitive username without symbol' do it 'filters by case insensitive username without symbol' do
send_keys_to_filtered_search('OT') send_keys_to_filtered_search('OOT')
expect(dropdown_author_size).to eq(2) expect(dropdown_author_size).to eq(2)
end end
......
require 'spec_helper'
describe Gitlab::SQL::Pattern do
describe '.to_pattern' do
subject(:to_pattern) { User.to_pattern(query) }
context 'when a query is shorter than 3 chars' do
let(:query) { '12' }
it 'returns exact matching pattern' do
expect(to_pattern).to eq('12')
end
end
context 'when a query with a escape character is shorter than 3 chars' do
let(:query) { '_2' }
it 'returns sanitized exact matching pattern' do
expect(to_pattern).to eq('\_2')
end
end
context 'when a query is equal to 3 chars' do
let(:query) { '123' }
it 'returns partial matching pattern' do
expect(to_pattern).to eq('%123%')
end
end
context 'when a query with a escape character is equal to 3 chars' do
let(:query) { '_23' }
it 'returns partial matching pattern' do
expect(to_pattern).to eq('%\_23%')
end
end
context 'when a query is longer than 3 chars' do
let(:query) { '1234' }
it 'returns partial matching pattern' do
expect(to_pattern).to eq('%1234%')
end
end
context 'when a query with a escape character is longer than 3 chars' do
let(:query) { '_234' }
it 'returns sanitized partial matching pattern' do
expect(to_pattern).to eq('%\_234%')
end
end
end
end
...@@ -789,6 +789,7 @@ describe User do ...@@ -789,6 +789,7 @@ describe User do
describe '.search' do describe '.search' do
let!(:user) { create(:user, name: 'user', username: 'usern', email: 'email@gmail.com') } let!(:user) { create(:user, name: 'user', username: 'usern', email: 'email@gmail.com') }
let!(:user2) { create(:user, name: 'user name', username: 'username', email: 'someemail@gmail.com') } let!(:user2) { create(:user, name: 'user name', username: 'username', email: 'someemail@gmail.com') }
let!(:user3) { create(:user, name: 'us', username: 'se', email: 'foo@gmail.com') }
describe 'name matching' do describe 'name matching' do
it 'returns users with a matching name with exact match first' do it 'returns users with a matching name with exact match first' do
...@@ -802,6 +803,14 @@ describe User do ...@@ -802,6 +803,14 @@ describe User do
it 'returns users with a matching name regardless of the casing' do it 'returns users with a matching name regardless of the casing' do
expect(described_class.search(user2.name.upcase)).to eq([user2]) expect(described_class.search(user2.name.upcase)).to eq([user2])
end end
it 'returns users with a exact matching name shorter than 3 chars' do
expect(described_class.search(user3.name)).to eq([user3])
end
it 'returns users with a exact matching name shorter than 3 chars regardless of the casing' do
expect(described_class.search(user3.name.upcase)).to eq([user3])
end
end end
describe 'email matching' do describe 'email matching' do
...@@ -830,6 +839,14 @@ describe User do ...@@ -830,6 +839,14 @@ describe User do
it 'returns users with a matching username regardless of the casing' do it 'returns users with a matching username regardless of the casing' do
expect(described_class.search(user2.username.upcase)).to eq([user2]) expect(described_class.search(user2.username.upcase)).to eq([user2])
end end
it 'returns users with a exact matching username shorter than 3 chars' do
expect(described_class.search(user3.username)).to eq([user3])
end
it 'returns users with a exact matching username shorter than 3 chars regardless of the casing' do
expect(described_class.search(user3.username.upcase)).to eq([user3])
end
end 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