Merge branch 'optimize-user-find-by-any-email' into 'master'

Improve performance of User.find_by_any_email



See merge request !1698

3 files changed, 37 insertions, 15 deletions

diff --git a/CHANGELOG b/CHANGELOG
index cf44a5c7c82..6bec4f606e7 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -2,6 +2,7 @@ Please view this file on the master branch, on stable branches it's out of date.
 
 v 8.2.0 (unreleased)
   - Force update refs/merge-requests/X/head upon a push to the source branch of a merge request (Stan Hu)
+  - Improved performance of finding users by one of their Email addresses
   - Improved performance of replacing references in comments
   - Show last project commit to default branch on project home page
   - Highlight comment based on anchor in URL
diff --git a/app/models/user.rb b/app/models/user.rb
index c72beacbf0f..67fef1c1e6a 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -235,21 +235,16 @@ class User < ActiveRecord::Base
 
     # Find a User by their primary email or any associated secondary email
     def find_by_any_email(email)
-      user_table = arel_table
-      email_table = Email.arel_table
-
-      # Use ARel to build a query:
-      query = user_table.
-        # SELECT "users".* FROM "users"
-        project(user_table[Arel.star]).
-        # LEFT OUTER JOIN "emails"
-        join(email_table, Arel::Nodes::OuterJoin).
-        # ON "users"."id" = "emails"."user_id"
-        on(user_table[:id].eq(email_table[:user_id])).
-        # WHERE ("user"."email" = '<email>' OR "emails"."email" = '<email>')
-        where(user_table[:email].eq(email).or(email_table[:email].eq(email)))
-
-      find_by_sql(query.to_sql).first
+      sql = 'SELECT *
+      FROM users
+      WHERE id IN (
+        SELECT id FROM users WHERE email = :email
+        UNION
+        SELECT emails.user_id FROM emails WHERE email = :email
+      )
+      LIMIT 1;'
+
+      User.find_by_sql([sql, { email: email }]).first
     end
 
     def filter(filter_name)
diff --git a/spec/benchmarks/models/user_spec.rb b/spec/benchmarks/models/user_spec.rb
index cc5c3904193..4cdba66939b 100644
--- a/spec/benchmarks/models/user_spec.rb
+++ b/spec/benchmarks/models/user_spec.rb
@@ -39,4 +39,30 @@ describe User, benchmark: true do
       it { is_expected.to iterate_per_second(iterations) }
     end
   end
+
+  describe '.find_by_any_email' do
+    let(:user) { create(:user) }
+
+    describe 'using a user with only a single Email address' do
+      let(:email) { user.email }
+
+      benchmark_subject { User.find_by_any_email(email) }
+
+      it { is_expected.to iterate_per_second(1000) }
+    end
+
+    describe 'using a user with multiple Email addresses' do
+      let(:email) { user.emails.first.email }
+
+      benchmark_subject { User.find_by_any_email(email) }
+
+      before do
+        10.times do
+          user.emails.create(email: FFaker::Internet.email)
+        end
+      end
+
+      it { is_expected.to iterate_per_second(1000) }
+    end
+  end
 end