PERF: Improve topic_user.liked update performance when moving posts

Previously we would re-calculate topic_user.liked for all users who have ever viewed the source or destination topic. This can be very expensive on large sites. Instead, we can use the array of moved post ids to find which users are actually affected by the move, and restrict the update query to only check/update their records.

On an example site this reduced the `update_post_action_cache` time from ~27s to 300ms

app/models/post_mover.rb

@@ -418,8 +418,7 @@ class PostMover
   def update_statistics
     destination_topic.update_statistics
     original_topic.update_statistics
-    TopicUser.update_post_action_cache(topic_id: original_topic.id)
-    TopicUser.update_post_action_cache(topic_id: destination_topic.id)
+    TopicUser.update_post_action_cache(topic_id: [original_topic.id, destination_topic.id], post_id: @post_ids)
   end
 
   def update_user_actions

app/models/topic_user.rb

@@ -372,20 +372,26 @@ class TopicUser < ActiveRecord::Base
 
   end
 
-  def self.update_post_action_cache(opts = {})
-    user_id = opts[:user_id]
-    post_id = opts[:post_id]
-    topic_id = opts[:topic_id]
-    action_type = opts[:post_action_type]
-
-    action_type_name = "liked" if action_type == :like
-
-    raise ArgumentError, "action_type" if action_type && !action_type_name
-
-    unless action_type_name
-      update_post_action_cache(opts.merge(post_action_type: :like))
-      return
-    end
+  # Update the cached topic_user.liked column based on data
+  # from the post_actions table. This is useful when posts
+  # have moved around, or to ensure integrity of the data.
+  #
+  # By default this will update data for all topics and all users.
+  # The parameters can be used to shrink the scope, and make it faster.
+  # user_id, post_id and topic_id can optionally be arrays of ids.
+  #
+  # Providing post_id will automatically scope to the relavent user_id and topic_id.
+  # A provided `topic_id` value will always take presedence, which is
+  # useful when a post has been moved between topics.
+  def self.update_post_action_cache(
+    user_id: nil,
+    post_id: nil,
+    topic_id: nil,
+    post_action_type: :like
+  )
+    raise ArgumentError, "post_action_type must equal :like" if post_action_type != :like
+    raise ArgumentError, "post_id and user_id cannot be supplied together" if user_id && post_id
+    action_type_name = "liked"
 
     builder = DB.build <<~SQL
       UPDATE topic_users tu
@@ -411,21 +417,25 @@ class TopicUser < ActiveRecord::Base
     SQL
 
     if user_id
-      builder.where("tu2.user_id = :user_id", user_id: user_id)
+      builder.where("tu2.user_id IN (:user_id)", user_id: user_id)
     end
 
     if topic_id
-      builder.where("tu2.topic_id = :topic_id", topic_id: topic_id)
+      builder.where("tu2.topic_id IN (:topic_id)", topic_id: topic_id)
     end
 
     if post_id
-      builder.where("tu2.topic_id IN (SELECT topic_id FROM posts WHERE id = :post_id)", post_id: post_id)
-      builder.where("tu2.user_id IN (SELECT user_id FROM post_actions
-                                     WHERE post_id = :post_id AND
-                                           post_action_type_id = :action_type_id)")
+      builder.where("tu2.topic_id IN (SELECT topic_id FROM posts WHERE id IN (:post_id))", post_id: post_id) if !topic_id
+      builder.where(<<~SQL, post_id: post_id)
+        tu2.user_id IN (
+          SELECT user_id FROM post_actions
+          WHERE post_id IN (:post_id)
+          AND post_action_type_id = :action_type_id
+        )
+      SQL
     end
 
-    builder.exec(action_type_id: PostActionType.types[action_type])
+    builder.exec(action_type_id: PostActionType.types[post_action_type])
   end
 
   # cap number of unread topics at count, bumping up last_read if needed

spec/models/post_mover_spec.rb

@@ -769,6 +769,19 @@ describe PostMover do
             end
           end
 
+          it "updates topic_user.liked values for both source and destination topics" do
+            expect(TopicUser.find_by(topic: topic, user: user).liked).to eq(false)
+
+            like = Fabricate(:post_action, post: p3, user: user, post_action_type_id: PostActionType.types[:like])
+            expect(TopicUser.find_by(topic: topic, user: user).liked).to eq(true)
+
+            expect(TopicUser.find_by(topic: destination_topic, user: user)).to eq(nil)
+            topic.move_posts(user, [p3.id], destination_topic_id: destination_topic.id)
+
+            expect(TopicUser.find_by(topic: topic, user: user).liked).to eq(false)
+            expect(TopicUser.find_by(topic: destination_topic, user: user).liked).to eq(true)
+          end
+
           context "read state and other stats per user" do
             def create_topic_user(user, topic, opts = {})
               notification_level = opts.delete(:notification_level) || :regular