From a51386a280a22bbfb4262b3344efc7460fb0fc3e Mon Sep 17 00:00:00 2001
From: Sam <sam.saffron@gmail.com>
Date: Wed, 5 Aug 2015 16:01:52 +1000
Subject: [PATCH] FEATURE: allow efficient preloading of custom fields in topic
 list

---
 app/models/concerns/has_custom_fields.rb | 57 ++++++++++++++++++++++++
 app/models/topic_list.rb                 |  7 +++
 spec/components/topic_query_spec.rb      | 40 ++++++++++++++---
 3 files changed, 99 insertions(+), 5 deletions(-)

diff --git a/app/models/concerns/has_custom_fields.rb b/app/models/concerns/has_custom_fields.rb
index b97e6cf3fb5..c9910bf0b48 100644
--- a/app/models/concerns/has_custom_fields.rb
+++ b/app/models/concerns/has_custom_fields.rb
@@ -45,6 +45,8 @@ module HasCustomFields
     has_many :_custom_fields, dependent: :destroy, :class_name => "#{name}CustomField"
     after_save :save_custom_fields
 
+    attr_accessor :preloaded_custom_fields
+
     # To avoid n+1 queries, use this function to retrieve lots of custom fields in one go
     # and create a "sideloaded" version for easy querying by id.
     def self.custom_fields_for_ids(ids, whitelisted_fields)
@@ -73,6 +75,37 @@ module HasCustomFields
       @custom_field_types[name] = type
     end
 
+    def self.preload_custom_fields(objects, fields)
+      if objects.present?
+        map = {}
+
+        empty = {}
+        fields.each do |field|
+          empty[field] = nil
+        end
+
+        objects.each do |obj|
+          map[obj.id] = obj
+          obj.preloaded_custom_fields = empty.dup
+        end
+
+        fk = (name.underscore << "_id")
+
+        "#{name}CustomField".constantize.where("#{fk} in (?)", map.keys)
+          .pluck(fk, :name, :value).each do |id, name, value|
+
+            preloaded = map[id].preloaded_custom_fields
+
+            if preloaded[name].nil?
+              preloaded.delete(name)
+            end
+
+            HasCustomFields::Helpers.append_field(preloaded, name, value, @custom_field_types)
+        end
+
+      end
+    end
+
   end
 
   def reload(options = nil)
@@ -80,12 +113,36 @@ module HasCustomFields
     super
   end
 
+  def custom_field_preloaded?(name)
+    @preloaded_custom_fields && @preloaded_custom_fields.key?(name)
+  end
+
   def clear_custom_fields
     @custom_fields = nil
     @custom_fields_orig = nil
   end
 
+  class PreloadedProxy
+    def initialize(preloaded)
+      @preloaded = preloaded
+    end
+
+    def [](key)
+      if @preloaded.key?(key)
+        @preloaded[key]
+      else
+        # for now you can not mix preload an non preload, it better just to fail
+        raise StandardError, "Attempting to access a non preloaded custom field, this is disallowed to prevent N+1 queries."
+      end
+    end
+  end
+
   def custom_fields
+
+    if @preloaded_custom_fields
+      return @preloaded_proxy ||= PreloadedProxy.new(@preloaded_custom_fields)
+    end
+
     @custom_fields ||= refresh_custom_fields_from_db.dup
   end
 
diff --git a/app/models/topic_list.rb b/app/models/topic_list.rb
index d58aa67be31..cce332a6d05 100644
--- a/app/models/topic_list.rb
+++ b/app/models/topic_list.rb
@@ -3,6 +3,9 @@ require_dependency 'avatar_lookup'
 class TopicList
   include ActiveModel::Serialization
 
+  cattr_accessor :preloaded_custom_fields
+  self.preloaded_custom_fields = []
+
   attr_accessor :more_topics_url,
                 :prev_topics_url,
                 :draft,
@@ -78,6 +81,10 @@ class TopicList
       ft.topic_list = self
     end
 
+    if TopicList.preloaded_custom_fields.present?
+      Topic.preload_custom_fields(@topics, TopicList.preloaded_custom_fields)
+    end
+
     @topics
   end
 
diff --git a/spec/components/topic_query_spec.rb b/spec/components/topic_query_spec.rb
index a961665bafc..c06c6623b56 100644
--- a/spec/components/topic_query_spec.rb
+++ b/spec/components/topic_query_spec.rb
@@ -320,21 +320,51 @@ describe TopicQuery do
       end
     end
 
+    context 'preload api' do
+      let(:topics) { }
+
+      it "preloads data correctly" do
+        TopicList.preloaded_custom_fields << "tag"
+        TopicList.preloaded_custom_fields << "age"
+        TopicList.preloaded_custom_fields << "foo"
+
+        topic = Fabricate.build(:topic, user: creator, bumped_at: 10.minutes.ago)
+        topic.custom_fields["tag"] = ["a","b","c"]
+        topic.custom_fields["age"] = 22
+        topic.save
+
+        new_topic = topic_query.list_new.topics.first
+
+        expect(new_topic.custom_fields["tag"].sort).to eq(["a","b","c"])
+        expect(new_topic.custom_fields["age"]).to eq("22")
+
+        expect(new_topic.custom_field_preloaded?("tag")).to eq(true)
+        expect(new_topic.custom_field_preloaded?("age")).to eq(true)
+        expect(new_topic.custom_field_preloaded?("foo")).to eq(true)
+        expect(new_topic.custom_field_preloaded?("bar")).to eq(false)
+
+        TopicList.preloaded_custom_fields.clear
+
+        # if we attempt to access non preloaded fields explode
+        expect{new_topic.custom_fields["boom"]}.to raise_error
+
+      end
+    end
+
     context 'with a new topic' do
       let!(:new_topic) { Fabricate(:topic, user: creator, bumped_at: 10.minutes.ago) }
       let(:topics) { topic_query.list_new.topics }
 
 
-      it "contains the new topic" do
-        expect(topics).to eq([new_topic])
-      end
-
       it "contains no new topics for a user that has missed the window" do
+
+        expect(topic_query.list_new.topics).to eq([new_topic])
+
         user.new_topic_duration_minutes = 5
         user.save
         new_topic.created_at = 10.minutes.ago
         new_topic.save
-        expect(topics).to eq([])
+        expect(topic_query.list_new.topics).to eq([])
       end
 
       context "muted topics" do