From c750bfb4af26235dbcd7ec28cb67d58c9254ab8e Mon Sep 17 00:00:00 2001 From: Jarek Radosz Date: Wed, 14 Jul 2021 13:51:33 +0200 Subject: [PATCH] FIX: A memoization bug in UserLookup and refactor (#13692) `@group_lookup` memoization bug was introduced in #13587. --- lib/user_lookup.rb | 57 ++++++++++++++++------------------------------ 1 file changed, 20 insertions(+), 37 deletions(-) diff --git a/lib/user_lookup.rb b/lib/user_lookup.rb index 8ea880b4dd2..590c6e27545 100644 --- a/lib/user_lookup.rb +++ b/lib/user_lookup.rb @@ -1,6 +1,13 @@ # frozen_string_literal: true class UserLookup + def self.lookup_columns + @user_lookup_columns ||= %i{id username name uploaded_avatar_id primary_group_id flair_group_id admin moderator trust_level} + end + + def self.group_lookup_columns + @group_lookup_columns ||= %i{id name flair_icon flair_upload_id flair_bg_color flair_color} + end def initialize(user_ids = []) @user_ids = user_ids.tap(&:compact!).tap(&:uniq!).tap(&:flatten!) @@ -12,66 +19,42 @@ class UserLookup end def primary_groups - @primary_groups ||= begin - hash = {} - users.values.each do |u| - if u.primary_group_id - hash[u.id] = groups[u.primary_group_id] - end + @primary_groups ||= users.values.each_with_object({}) do |user, hash| + if user.primary_group_id + hash[user.id] = groups[user.primary_group_id] end - hash end end def flair_groups - @flair_groups ||= begin - hash = {} - users.values.each do |u| - if u.flair_group_id - hash[u.id] = groups[u.flair_group_id] - end + @flair_groups ||= users.values.each_with_object({}) do |user, hash| + if user.flair_group_id + hash[user.id] = groups[user.flair_group_id] end - hash end end private - def self.lookup_columns - @user_lookup_columns ||= %i{id username name uploaded_avatar_id primary_group_id flair_group_id admin moderator trust_level} - end - - def self.group_lookup_columns - @group_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 - hash = {} - User.where(id: @user_ids) + @users ||= User + .where(id: @user_ids) .select(self.class.lookup_columns) - .each { |user| hash[user.id] = user } - hash + .index_by(&:id) end def groups - @group_lookup = begin + @group_lookup ||= begin group_ids = users.values.map { |u| [u.primary_group_id, u.flair_group_id] } group_ids.flatten! group_ids.uniq! group_ids.compact! - hash = {} - - Group.includes(:flair_upload) + Group + .includes(:flair_upload) .where(id: group_ids) .select(self.class.group_lookup_columns) - .each { |g| hash[g.id] = g } - - hash + .index_by(&:id) end end end