From e074651fdc56afad734e5367ce3a718ff4318a85 Mon Sep 17 00:00:00 2001
From: Sam <sam.saffron@gmail.com>
Date: Thu, 14 May 2015 14:38:47 +1000
Subject: [PATCH] PERF: refactor user search so works more efficiently

Stop scanning entire user table
---
 app/models/user_search.rb       | 83 +++++++++++++++++++++++++--------
 spec/models/user_search_spec.rb |  4 +-
 2 files changed, 65 insertions(+), 22 deletions(-)

diff --git a/app/models/user_search.rb b/app/models/user_search.rb
index cbb18312f8aa2b..2c0a950d7f2f6c 100644
--- a/app/models/user_search.rb
+++ b/app/models/user_search.rb
@@ -10,10 +10,29 @@ def initialize(term, opts={})
     @limit = opts[:limit] || 20
   end
 
-  def search
-    users = User.order(User.sql_fragment("CASE WHEN username_lower = ? THEN 0 ELSE 1 END ASC", @term.downcase))
+  def scoped_users
+    users = User.where("active")
+
+    unless @searching_user && @searching_user.staff?
+      users = users.not_suspended
+    end
+
+    # Only show users who have access to private topic
+    if @topic_id && @topic_allowed_users == "true"
+      topic = Topic.find_by(id: @topic_id)
+
+      if topic.category && topic.category.read_restricted
+        users = users.includes(:secure_categories)
+                     .where("users.admin = TRUE OR categories.id = ?", topic.category.id)
+                     .references(:categories)
+      end
+    end
 
-    users = users.where(active: true)
+    users.limit(@limit)
+  end
+
+  def filtered_by_term_users
+    users = scoped_users
 
     if @term.present?
       if SiteSetting.enable_names?
@@ -28,29 +47,53 @@ def search
       end
     end
 
-    if @topic_id
-      users = users.joins(User.sql_fragment("LEFT JOIN (SELECT DISTINCT p.user_id FROM POSTS p WHERE topic_id = ?) s ON s.user_id = users.id", @topic_id))
-                   .order("CASE WHEN s.user_id IS NULL THEN 0 ELSE 1 END DESC")
-    end
+    users
+  end
+
+  def search_ids
+    users = Set.new
+
+    # 1. exact username matches
+    if @term.present?
+      scoped_users.where(username_lower: @term.downcase)
+                  .pluck(:id)
+                  .each{|id| users << id}
 
-    unless @searching_user && @searching_user.staff?
-      users = users.not_suspended
     end
 
-    # Only show users who have access to private topic
-    if @topic_id && @topic_allowed_users == "true"
-      allowed_user_ids = []
-      topic = Topic.find_by(id: @topic_id)
+    return users.to_a if users.length == @limit
 
-      if topic.category && topic.category.read_restricted
-        users = users.includes(:secure_categories)
-                     .where("users.admin = TRUE OR categories.id = ?", topic.category.id)
-                     .references(:categories)
-      end
+    # 2. in topic
+    if @topic_id
+      filtered_by_term_users.where('users.id in (SELECT p.user_id FROM posts p WHERE topic_id = ?)', @topic_id)
+                            .order('last_seen_at DESC')
+                            .limit(@limit - users.length)
+                            .pluck(:id)
+                            .each{|id| users << id}
     end
 
-    users.order("CASE WHEN last_seen_at IS NULL THEN 0 ELSE 1 END DESC, last_seen_at DESC, username ASC")
-         .limit(@limit)
+    return users.to_a if users.length == @limit
+
+    # 3. global matches
+    filtered_by_term_users.order('last_seen_at DESC')
+                            .limit(@limit - users.length)
+                            .pluck(:id)
+                            .each{|id| users << id}
+
+    users.to_a
+
+  end
+
+  def search
+
+    ids = search_ids
+    return User.where("0=1") if ids.empty?
+
+    User.joins("JOIN (SELECT unnest uid, row_number() OVER () AS rn
+      FROM unnest('{#{ids.join(",")}}'::int[])
+    ) x on uid = users.id")
+        .order("rn")
+
   end
 
 end
diff --git a/spec/models/user_search_spec.rb b/spec/models/user_search_spec.rb
index 13a932c45a6f45..48ee7beccda2d8 100644
--- a/spec/models/user_search_spec.rb
+++ b/spec/models/user_search_spec.rb
@@ -38,7 +38,7 @@ def search_for(*args)
     # normal search
     results = search_for(user1.name.split(" ").first)
     expect(results.size).to eq(1)
-    expect(results.first).to eq(user1)
+    expect(results.first.username).to eq(user1.username)
 
     # lower case
     results = search_for(user1.name.split(" ").first.downcase)
@@ -106,7 +106,7 @@ def search_for(*args)
 
     # find an exact match first
     results = search_for("mrB")
-    expect(results.first).to eq(user1)
+    expect(results.first.username).to eq(user1.username)
 
     # don't return inactive users
     results = search_for("Ghost")