Move status counters to separate table, count replies (#8104)

* Move status counters to separate table, count replies

* Migration to remove old counter columns from statuses table

* Fix schema file

app/models/favourite.rb

@@ -32,20 +32,11 @@ class Favourite < ApplicationRecord
   private
 
   def increment_cache_counters
-    if association(:status).loaded?
-      status.update_attribute(:favourites_count, status.favourites_count + 1)
-    else
-      Status.where(id: status_id).update_all('favourites_count = COALESCE(favourites_count, 0) + 1')
-    end
+    status.increment_count!(:favourites_count)
   end
 
   def decrement_cache_counters
     return if association(:status).loaded? && (status.marked_for_destruction? || status.marked_for_mass_destruction?)
-
-    if association(:status).loaded?
-      status.update_attribute(:favourites_count, [status.favourites_count - 1, 0].max)
-    else
-      Status.where(id: status_id).update_all('favourites_count = GREATEST(COALESCE(favourites_count, 0) - 1, 0)')
-    end
+    status.decrement_count!(:favourites_count)
   end
 end

app/models/status.rb

@@ -15,8 +15,6 @@
 #  visibility             :integer          default("public"), not null
 #  spoiler_text           :text             default(""), not null
 #  reply                  :boolean          default(FALSE), not null
-#  favourites_count       :integer          default(0), not null
-#  reblogs_count          :integer          default(0), not null
 #  language               :string
 #  conversation_id        :bigint(8)
 #  local                  :boolean
@@ -26,6 +24,8 @@
 #
 
 class Status < ApplicationRecord
+  self.cache_versioning = false
+
   include Paginable
   include Streamable
   include Cacheable
@@ -59,6 +59,7 @@ class Status < ApplicationRecord
 
   has_one :notification, as: :activity, dependent: :destroy
   has_one :stream_entry, as: :activity, inverse_of: :status
+  has_one :status_stat, inverse_of: :status
 
   validates :uri, uniqueness: true, presence: true, unless: :local?
   validates :text, presence: true, unless: -> { with_media? || reblog? }
@@ -81,7 +82,25 @@ class Status < ApplicationRecord
   scope :not_excluded_by_account, ->(account) { where.not(account_id: account.excluded_from_timeline_account_ids) }
   scope :not_domain_blocked_by_account, ->(account) { account.excluded_from_timeline_domains.blank? ? left_outer_joins(:account) : left_outer_joins(:account).where('accounts.domain IS NULL OR accounts.domain NOT IN (?)', account.excluded_from_timeline_domains) }
 
-  cache_associated :account, :application, :media_attachments, :conversation, :tags, :stream_entry, mentions: :account, reblog: [:account, :application, :stream_entry, :tags, :media_attachments, :conversation, mentions: :account], thread: :account
+  cache_associated :account,
+                   :application,
+                   :media_attachments,
+                   :conversation,
+                   :status_stat,
+                   :tags,
+                   :stream_entry,
+                   mentions: :account,
+                   reblog: [
+                     :account,
+                     :application,
+                     :stream_entry,
+                     :tags,
+                     :media_attachments,
+                     :conversation,
+                     :status_stat,
+                     mentions: :account,
+                   ],
+                   thread: :account
 
   delegate :domain, to: :account, prefix: true
 
@@ -175,6 +194,26 @@ class Status < ApplicationRecord
     @marked_for_mass_destruction
   end
 
+  def replies_count
+    status_stat&.replies_count || 0
+  end
+
+  def reblogs_count
+    status_stat&.reblogs_count || 0
+  end
+
+  def favourites_count
+    status_stat&.favourites_count || 0
+  end
+
+  def increment_count!(key)
+    update_status_stat!(key => public_send(key) + 1)
+  end
+
+  def decrement_count!(key)
+    update_status_stat!(key => [public_send(key) - 1, 0].max)
+  end
+
   after_create  :increment_counter_caches
   after_destroy :decrement_counter_caches
 
@@ -190,6 +229,10 @@ class Status < ApplicationRecord
   before_validation :set_local
 
   class << self
+    def cache_ids
+      left_outer_joins(:status_stat).select('statuses.id, greatest(statuses.updated_at, status_stats.updated_at) AS updated_at')
+    end
+
     def in_chosen_languages(account)
       where(language: nil).or where(language: account.chosen_languages)
     end
@@ -352,6 +395,11 @@ class Status < ApplicationRecord
 
   private
 
+  def update_status_stat!(attrs)
+    record = status_stat || build_status_stat
+    record.update(attrs)
+  end
+
   def store_uri
     update_attribute(:uri, ActivityPub::TagManager.instance.uri_for(self)) if uri.nil?
   end
@@ -408,13 +456,8 @@ class Status < ApplicationRecord
       Account.where(id: account_id).update_all('statuses_count = COALESCE(statuses_count, 0) + 1')
     end
 
-    return unless reblog?
-
-    if association(:reblog).loaded?
-      reblog.update_attribute(:reblogs_count, reblog.reblogs_count + 1)
-    else
-      Status.where(id: reblog_of_id).update_all('reblogs_count = COALESCE(reblogs_count, 0) + 1')
-    end
+    thread.increment_count!(:replies_count) if in_reply_to_id.present?
+    reblog.increment_count!(:reblogs_count) if reblog?
   end
 
   def decrement_counter_caches
@@ -426,12 +469,7 @@ class Status < ApplicationRecord
       Account.where(id: account_id).update_all('statuses_count = GREATEST(COALESCE(statuses_count, 0) - 1, 0)')
     end
 
-    return unless reblog?
-
-    if association(:reblog).loaded?
-      reblog.update_attribute(:reblogs_count, [reblog.reblogs_count - 1, 0].max)
-    else
-      Status.where(id: reblog_of_id).update_all('reblogs_count = GREATEST(COALESCE(reblogs_count, 0) - 1, 0)')
-    end
+    thread.decrement_count!(:replies_count) if in_reply_to_id.present?
+    reblog.decrement_count!(:reblogs_count) if reblog?
   end
 end

app/models/status_stat.rb

+# frozen_string_literal: true
+# == Schema Information
+#
+# Table name: status_stats
+#
+#  id               :bigint(8)        not null, primary key
+#  status_id        :bigint(8)        not null
+#  replies_count    :bigint(8)        default(0), not null
+#  reblogs_count    :bigint(8)        default(0), not null
+#  favourites_count :bigint(8)        default(0), not null
+#  created_at       :datetime         not null
+#  updated_at       :datetime         not null
+#
+
+class StatusStat < ApplicationRecord
+  belongs_to :status, inverse_of: :status_stat
+end

app/serializers/rest/status_serializer.rb

@@ -3,7 +3,8 @@
 class REST::StatusSerializer < ActiveModel::Serializer
   attributes :id, :created_at, :in_reply_to_id, :in_reply_to_account_id,
              :sensitive, :spoiler_text, :visibility, :language,
-             :uri, :content, :url, :reblogs_count, :favourites_count
+             :uri, :content, :url, :replies_count, :reblogs_count,
+             :favourites_count
 
   attribute :favourited, if: :current_user?
   attribute :reblogged, if: :current_user?

db/migrate/20180812162710_create_status_stats.rb

+class CreateStatusStats < ActiveRecord::Migration[5.2]
+  def change
+    create_table :status_stats do |t|
+      t.belongs_to :status, null: false, foreign_key: { on_delete: :cascade }, index: { unique: true }
+      t.bigint :replies_count, null: false, default: 0
+      t.bigint :reblogs_count, null: false, default: 0
+      t.bigint :favourites_count, null: false, default: 0
+
+      t.timestamps
+    end
+  end
+end

db/migrate/20180812173710_copy_status_stats.rb

+class CopyStatusStats < ActiveRecord::Migration[5.2]
+  disable_ddl_transaction!
+
+  def up
+    safety_assured do
+      execute <<-SQL.squish
+        INSERT INTO status_stats (status_id, reblogs_count, favourites_count)
+        SELECT id, reblogs_count, favourites_count
+        FROM statuses
+        ON CONFLICT (status_id) DO UPDATE
+        SET reblogs_count = EXCLUDED.reblogs_count, favourites_count = EXCLUDED.favourites_count
+      SQL
+    end
+  end
+
+  def down
+    # Nothing
+  end
+end

db/post_migrate/20180813113448_copy_status_stats_cleanup.rb

+# frozen_string_literal: true
+
+class CopyStatusStatsCleanup < ActiveRecord::Migration[5.2]
+  disable_ddl_transaction!
+
+  def change
+    safety_assured do
+      remove_column :statuses, :reblogs_count, :integer, default: 0, null: false
+      remove_column :statuses, :favourites_count, :integer, default: 0, null: false
+    end
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 2018_08_12_123222) do
+ActiveRecord::Schema.define(version: 2018_08_13_113448) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -456,6 +456,16 @@ ActiveRecord::Schema.define(version: 2018_08_12_123222) do
     t.index ["account_id", "status_id"], name: "index_status_pins_on_account_id_and_status_id", unique: true
   end
 
+  create_table "status_stats", force: :cascade do |t|
+    t.bigint "status_id", null: false
+    t.bigint "replies_count", default: 0, null: false
+    t.bigint "reblogs_count", default: 0, null: false
+    t.bigint "favourites_count", default: 0, null: false
+    t.datetime "created_at", null: false
+    t.datetime "updated_at", null: false
+    t.index ["status_id"], name: "index_status_stats_on_status_id", unique: true
+  end
+
   create_table "statuses", id: :bigint, default: -> { "timestamp_id('statuses'::text)" }, force: :cascade do |t|
     t.string "uri"
     t.text "text", default: "", null: false
@@ -468,8 +478,6 @@ ActiveRecord::Schema.define(version: 2018_08_12_123222) do
     t.integer "visibility", default: 0, null: false
     t.text "spoiler_text", default: "", null: false
     t.boolean "reply", default: false, null: false
-    t.integer "favourites_count", default: 0, null: false
-    t.integer "reblogs_count", default: 0, null: false
     t.string "language"
     t.bigint "conversation_id"
     t.boolean "local"
@@ -630,6 +638,7 @@ ActiveRecord::Schema.define(version: 2018_08_12_123222) do
   add_foreign_key "session_activations", "users", name: "fk_e5fda67334", on_delete: :cascade
   add_foreign_key "status_pins", "accounts", name: "fk_d4cb435b62", on_delete: :cascade
   add_foreign_key "status_pins", "statuses", on_delete: :cascade
+  add_foreign_key "status_stats", "statuses", on_delete: :cascade
   add_foreign_key "statuses", "accounts", column: "in_reply_to_account_id", name: "fk_c7fa917661", on_delete: :nullify
   add_foreign_key "statuses", "accounts", name: "fk_9bda1543f7", on_delete: :cascade
   add_foreign_key "statuses", "statuses", column: "in_reply_to_id", on_delete: :nullify

spec/fabricators/status_stat_fabricator.rb

+Fabricator(:status_stat) do
+  status_id        nil
+  replies_count    ""
+  reblogs_count    ""
+  favourites_count ""
+end

spec/models/status_stat_spec.rb

+require 'rails_helper'
+
+RSpec.describe StatusStat, type: :model do
+  pending "add some examples to (or delete) #{__FILE__}"
+end