PERF: Combine avatar_lookup and primary_group_lookup into user_lookup (#10253)

PERF: Combine avatar_lookup and primary_group_lookup into user_lookup (#10253)

These two classes were running very similar queries, which could be expensive on large topics

diff --git a/app/models/topic_list.rb b/app/models/topic_list.rb
index 52b0ee0..d282cef 100644
--- a/app/models/topic_list.rb
+++ b/app/models/topic_list.rb
@@ -110,8 +110,7 @@ class TopicList
       user_ids << ft.user_id << ft.last_post_user_id << ft.featured_user_ids << ft.allowed_user_ids
     end
 
-    avatar_lookup = AvatarLookup.new(user_ids)
-    primary_group_lookup = PrimaryGroupLookup.new(user_ids)
+    user_lookup = UserLookup.new(user_ids)
 
     @topics.each do |ft|
       ft.user_data = @topic_lookup[ft.id] if @topic_lookup.present?
@@ -122,11 +121,13 @@ class TopicList
       end
 
       ft.posters = ft.posters_summary(
-        avatar_lookup: avatar_lookup,
-        primary_group_lookup: primary_group_lookup
+        user_lookup: user_lookup
       )
 
-      ft.participants = ft.participants_summary(avatar_lookup: avatar_lookup, user: @current_user)
+      ft.participants = ft.participants_summary(
+        user_lookup: user_lookup,
+        user: @current_user
+      )
       ft.topic_list = self
     end
 
diff --git a/app/models/topic_participants_summary.rb b/app/models/topic_participants_summary.rb
index df3f385..ba36507 100644
--- a/app/models/topic_participants_summary.rb
+++ b/app/models/topic_participants_summary.rb
@@ -27,7 +27,7 @@ class TopicParticipantsSummary
   end
 
   def top_participants
-    user_ids.map { |id| avatar_lookup[id] }.compact.uniq.take(PARTICIPANT_COUNT)
+    user_ids.map { |id| user_lookup[id] }.compact.uniq.take(PARTICIPANT_COUNT)
   end
 
   def user_ids
@@ -35,7 +35,7 @@ class TopicParticipantsSummary
     [topic.user_id] + topic.allowed_user_ids - [@user.id]
   end
 
-  def avatar_lookup
-    @avatar_lookup ||= options[:avatar_lookup] || AvatarLookup.new(user_ids)
+  def user_lookup
+    @user_lookup ||= options[:user_lookup] || UserLookup.new(user_ids)
   end
 end
diff --git a/app/models/topic_posters_summary.rb b/app/models/topic_posters_summary.rb
index a429956..9eb40bf 100644
--- a/app/models/topic_posters_summary.rb
+++ b/app/models/topic_posters_summary.rb
@@ -31,7 +31,7 @@ class TopicPostersSummary
     topic_poster = TopicPoster.new
     topic_poster.user = user
     topic_poster.description = descriptions_for(user)
-    topic_poster.primary_group = primary_group_lookup[user.id]
+    topic_poster.primary_group = user_lookup.primary_groups[user.id]
     if topic.last_post_user_id == user.id
       topic_poster.extras = +'latest'
       topic_poster.extras << ' single' if user_ids.uniq.size == 1
@@ -70,7 +70,7 @@ class TopicPostersSummary
   def shuffle_last_poster_to_back_in(summary)
     unless last_poster_is_topic_creator?
       summary.reject! { |u| u.id == topic.last_post_user_id }
-      summary << avatar_lookup[topic.last_post_user_id]
+      summary << user_lookup[topic.last_post_user_id]
     end
     summary
   end
@@ -84,18 +84,14 @@ class TopicPostersSummary
   end
 
   def top_posters
-    user_ids.map { |id| avatar_lookup[id] }.compact.uniq.take(5)
+    user_ids.map { |id| user_lookup[id] }.compact.uniq.take(5)
   end
 
   def user_ids
     [ topic.user_id, topic.last_post_user_id, *topic.featured_user_ids ]
   end
 
-  def avatar_lookup
-    @avatar_lookup ||= options[:avatar_lookup] || AvatarLookup.new(user_ids)
-  end
-
-  def primary_group_lookup
-    @primary_group_lookup ||= options[:primary_group_lookup] || PrimaryGroupLookup.new(user_ids)
+  def user_lookup
+    @user_lookup ||= options[:user_lookup] || UserLookup.new(user_ids)
   end
 end
diff --git a/app/models/user_action.rb b/app/models/user_action.rb
index 5346c06..e417167 100644
--- a/app/models/user_action.rb
+++ b/app/models/user_action.rb
@@ -167,7 +167,7 @@ class UserAction < ActiveRecord::Base
       'u.name AS acting_name'
     ]
 
-    AvatarLookup.lookup_columns.each do |c|
+    UserLookup.lookup_columns.each do |c|
       next if c == :id || c['.']
       acting_cols << "u.#{c} AS acting_#{c}"
     end
diff --git a/app/models/user_summary.rb b/app/models/user_summary.rb
index a62e7d1..4b0fc54 100644
--- a/app/models/user_summary.rb
+++ b/app/models/user_summary.rb
@@ -194,7 +194,7 @@ protected
   def user_counts(user_hash)
     user_ids = user_hash.keys
 
-    lookup = AvatarLookup.new(user_ids)
+    lookup = UserLookup.new(user_ids)
     user_ids.map do |user_id|
       lookup_hash = lookup[user_id]
 
diff --git a/lib/avatar_lookup.rb b/lib/avatar_lookup.rb
deleted file mode 100644
index 535c64e..0000000
--- a/lib/avatar_lookup.rb
+++ /dev/null
@@ -1,32 +0,0 @@
-# frozen_string_literal: true
-
-class AvatarLookup
-
-  def initialize(user_ids = [])
-    @user_ids = user_ids.tap(&:compact!).tap(&:uniq!).tap(&:flatten!)
-  end
-
-  # Lookup a user by id
-  def [](user_id)
-    users[user_id]
-  end
-
-  private
-
-  def self.lookup_columns
-    @lookup_columns ||= %i{id user_emails.email username name uploaded_avatar_id}
-  end
-
-  def users
-    @users ||= user_lookup_hash
-  end
-
-  def user_lookup_hash
-    hash = {}
-    User.joins(:user_emails)
-      .where(id: @user_ids)
-      .select(AvatarLookup.lookup_columns)
-      .each { |user| hash[user.id] = user }
-    hash
-  end
-end
diff --git a/lib/plugin/instance.rb b/lib/plugin/instance.rb
index 2a9b4c8..6b5ece1 100644
--- a/lib/plugin/instance.rb
+++ b/lib/plugin/instance.rb
@@ -192,8 +192,8 @@ class Plugin::Instance
 
   def custom_avatar_column(column)
     reloadable_patch do |plugin|
-      AvatarLookup.lookup_columns << column
-      AvatarLookup.lookup_columns.uniq!
+      UserLookup.lookup_columns << column
+      UserLookup.lookup_columns.uniq!
     end
   end
 
diff --git a/lib/primary_group_lookup.rb b/lib/primary_group_lookup.rb
deleted file mode 100644
index 8110a37..0000000
--- a/lib/primary_group_lookup.rb
+++ /dev/null
@@ -1,41 +0,0 @@
-# frozen_string_literal: true
-
-class PrimaryGroupLookup
-  def initialize(user_ids = [])
-    @user_ids = user_ids.tap(&:compact!).tap(&:uniq!).tap(&:flatten!)
-  end
-
-  # Lookup primary group for a given user id
-  def [](user_id)
-    users[user_id]
-  end
-
-  private
-
-  def self.lookup_columns
-    @lookup_columns ||= %i{id name flair_icon flair_upload_id flair_bg_color flair_color}
-  end
-
-  def users
-    @users ||= user_lookup_hash
-  end
-
-  def user_lookup_hash
-    users_with_primary_group = User.where(id: @user_ids)
-      .where.not(primary_group_id: nil)
-      .select(:id, :primary_group_id)
-
-    group_lookup = {}
-    group_ids = users_with_primary_group.map(&:primary_group_id)
-    group_ids.uniq!
-
-    Group.includes(:flair_upload).where(id: group_ids).select(self.class.lookup_columns)
-      .each { |g| group_lookup[g.id] = g }
-
-    hash = {}
-    users_with_primary_group.each do |u|
-      hash[u.id] = group_lookup[u.primary_group_id]
-    end
-    hash
-  end
-end
diff --git a/lib/topic_query.rb b/lib/topic_query.rb
index d8cdfe8..bf614e7 100644
--- a/lib/topic_query.rb
+++ b/lib/topic_query.rb
@@ -433,16 +433,14 @@ class TopicQuery
         user_ids << ft.user_id << ft.last_post_user_id << ft.featured_user_ids << ft.allowed_user_ids
       end
 
-      avatar_lookup = AvatarLookup.new(user_ids)
-      primary_group_lookup = PrimaryGroupLookup.new(user_ids)
+      user_lookup = UserLookup.new(user_ids)
 
       # memoize for loop so we don't keep looking these up
       translations = TopicPostersSummary.translations
 
       topics.each do |t|
         t.posters = t.posters_summary(
-          avatar_lookup: avatar_lookup,
-          primary_group_lookup: primary_group_lookup,
+          user_lookup: user_lookup,
           translations: translations
         )
       end
diff --git a/lib/user_lookup.rb b/lib/user_lookup.rb
new file mode 100644
index 0000000..faced05
--- /dev/null
+++ b/lib/user_lookup.rb
@@ -0,0 +1,59 @@
+# frozen_string_literal: true
+
+class UserLookup
+
+  def initialize(user_ids = [])
+    @user_ids = user_ids.tap(&:compact!).tap(&:uniq!).tap(&:flatten!)
+  end
+
+  # Lookup a user by id
+  def [](user_id)
+    users[user_id]
+  end
+

[... diff too long, it was truncated ...]

GitHub sha: fab8b864

This commit appears in #10253 which was merged by davidtaylorhq.