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

Improve AutocompleteController#users.json performance

Closes #36879

See merge request !13754

6 files changed, 108 insertions, 7 deletions

diff --git a/app/models/user.rb b/app/models/user.rb
index fbd08bc4d0a..70787de4b40 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -5,6 +5,7 @@ class User < ActiveRecord::Base
 
   include Gitlab::ConfigHelper
   include Gitlab::CurrentSettings
+  include Gitlab::SQL::Pattern
   include Avatarable
   include Referable
   include Sortable
@@ -303,7 +304,7 @@ class User < ActiveRecord::Base
     # Returns an ActiveRecord::Relation.
     def search(query)
       table   = arel_table
-      pattern = "%#{query}%"
+      pattern = User.to_pattern(query)
 
       order = <<~SQL
         CASE
diff --git a/changelogs/unreleased/improve-autocomplete-user-performance.yml b/changelogs/unreleased/improve-autocomplete-user-performance.yml
new file mode 100644
index 00000000000..5a7153771ff
--- /dev/null
+++ b/changelogs/unreleased/improve-autocomplete-user-performance.yml
@@ -0,0 +1,5 @@
+---
+title: Improve performance for AutocompleteController#users.json
+merge_request: 13754
+author: Hiroyuki Sato
+type: changed
diff --git a/lib/gitlab/sql/pattern.rb b/lib/gitlab/sql/pattern.rb
new file mode 100644
index 00000000000..b42bc67ccfc
--- /dev/null
+++ b/lib/gitlab/sql/pattern.rb
@@ -0,0 +1,23 @@
+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
diff --git a/spec/features/issues/filtered_search/dropdown_author_spec.rb b/spec/features/issues/filtered_search/dropdown_author_spec.rb
index 975dc035f2d..3cec59050ab 100644
--- a/spec/features/issues/filtered_search/dropdown_author_spec.rb
+++ b/spec/features/issues/filtered_search/dropdown_author_spec.rb
@@ -6,7 +6,7 @@ describe 'Dropdown author', js: true do
   let!(:project) { create(:project) }
   let!(:user) { create(:user, name: 'administrator', username: 'root') }
   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(:js_dropdown_author) { '#js-dropdown-author' }
 
@@ -82,31 +82,31 @@ describe 'Dropdown author', js: true do
     end
 
     it 'filters by name' do
-      send_keys_to_filtered_search('ja')
+      send_keys_to_filtered_search('jac')
 
       expect(dropdown_author_size).to eq(1)
     end
 
     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)
     end
 
     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)
     end
 
     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)
     end
 
     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)
     end
diff --git a/spec/lib/gitlab/sql/pattern_spec.rb b/spec/lib/gitlab/sql/pattern_spec.rb
new file mode 100644
index 00000000000..9d7b2136dab
--- /dev/null
+++ b/spec/lib/gitlab/sql/pattern_spec.rb
@@ -0,0 +1,55 @@
+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
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 8e04eea56a7..b70ab5581ac 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -789,6 +789,7 @@ describe User do
   describe '.search' do
     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!(:user3) { create(:user, name: 'us', username: 'se', email: 'foo@gmail.com') }
 
     describe 'name matching' do
       it 'returns users with a matching name with exact match first' do
@@ -802,6 +803,14 @@ describe User do
       it 'returns users with a matching name regardless of the casing' do
         expect(described_class.search(user2.name.upcase)).to eq([user2])
       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
 
     describe 'email matching' do
@@ -830,6 +839,14 @@ describe User do
       it 'returns users with a matching username regardless of the casing' do
         expect(described_class.search(user2.username.upcase)).to eq([user2])
       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