From 816284d73924ceb453c76d571bcb315be91b2b08 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Sat, 3 Dec 2016 18:21:26 +0100 Subject: [PATCH] Fix #248 - Reload all accounts when fetching from cache --- app/controllers/application_controller.rb | 4 +++- app/models/concerns/cacheable.rb | 1 + app/models/feed.rb | 4 ++-- app/models/notification.rb | 23 +++++++++++-------- app/models/status.rb | 16 +++++++++++++ ...20_add_from_account_id_to_notifications.rb | 14 +++++++++++ db/schema.rb | 7 +++--- 7 files changed, 53 insertions(+), 16 deletions(-) create mode 100644 db/migrate/20161203164520_add_from_account_id_to_notifications.rb diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index fbe4af07c3..7270686de4 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -61,7 +61,7 @@ class ApplicationController < ActionController::Base def cache_collection(raw, klass) return raw unless klass.respond_to?(:with_includes) - raw = raw.select(:id, :updated_at).to_a if raw.is_a?(ActiveRecord::Relation) + raw = raw.cache_ids.to_a if raw.is_a?(ActiveRecord::Relation) uncached_ids = [] cached_keys_with_value = Rails.cache.read_multi(*raw.map(&:cache_key)) @@ -69,6 +69,8 @@ class ApplicationController < ActionController::Base uncached_ids << item.id unless cached_keys_with_value.key?(item.cache_key) end + klass.reload_stale_associations!(cached_keys_with_value.values) if klass.respond_to?(:reload_stale_associations!) + unless uncached_ids.empty? uncached = klass.where(id: uncached_ids).with_includes.map { |item| [item.id, item] }.to_h diff --git a/app/models/concerns/cacheable.rb b/app/models/concerns/cacheable.rb index cd01670482..51451d2607 100644 --- a/app/models/concerns/cacheable.rb +++ b/app/models/concerns/cacheable.rb @@ -11,5 +11,6 @@ module Cacheable included do scope :with_includes, -> { includes(@cache_associated) } + scope :cache_ids, -> { select(:id, :updated_at) } end end diff --git a/app/models/feed.rb b/app/models/feed.rb index 7b181d529c..5e1905e152 100644 --- a/app/models/feed.rb +++ b/app/models/feed.rb @@ -14,9 +14,9 @@ class Feed # If we're after most recent items and none are there, we need to precompute the feed if unhydrated.empty? && max_id == '+inf' && since_id == '-inf' RegenerationWorker.perform_async(@account.id, @type) - @statuses = Status.send("as_#{@type}_timeline", @account).paginate_by_max_id(limit, nil, nil) + @statuses = Status.send("as_#{@type}_timeline", @account).cache_ids.paginate_by_max_id(limit, nil, nil) else - status_map = Status.where(id: unhydrated).map { |s| [s.id, s] }.to_h + status_map = Status.where(id: unhydrated).cache_ids.map { |s| [s.id, s] }.to_h @statuses = unhydrated.map { |id| status_map[id] }.compact end diff --git a/app/models/notification.rb b/app/models/notification.rb index b066cd87a1..2d48abce56 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -5,6 +5,7 @@ class Notification < ApplicationRecord include Cacheable belongs_to :account + belongs_to :from_account, class_name: 'Account' belongs_to :activity, polymorphic: true belongs_to :mention, foreign_type: 'Mention', foreign_key: 'activity_id' @@ -16,7 +17,7 @@ class Notification < ApplicationRecord STATUS_INCLUDES = [:account, :stream_entry, :media_attachments, :tags, mentions: :account, reblog: [:stream_entry, :account, :media_attachments, :tags, mentions: :account]].freeze - cache_associated status: STATUS_INCLUDES, mention: [status: STATUS_INCLUDES], favourite: [:account, status: STATUS_INCLUDES], follow: :account + cache_associated :from_account, status: STATUS_INCLUDES, mention: [status: STATUS_INCLUDES], favourite: [:account, status: STATUS_INCLUDES], follow: :account def activity send(activity_type.downcase) @@ -31,15 +32,6 @@ class Notification < ApplicationRecord end end - def from_account - case type - when :mention - activity.status.account - when :follow, :favourite, :reblog - activity.account - end - end - def target_status case type when :reblog @@ -48,4 +40,15 @@ class Notification < ApplicationRecord activity.status end end + + class << self + def reload_stale_associations!(cached_items) + account_ids = cached_items.map(&:from_account_id).uniq + accounts = Account.where(id: account_ids).map { |a| [a.id, a] }.to_h + + cached_items.each do |item| + item.from_account = accounts[item.from_account_id] + end + end + end end diff --git a/app/models/status.rb b/app/models/status.rb index 1e70101a32..f6d8fc9bbd 100644 --- a/app/models/status.rb +++ b/app/models/status.rb @@ -130,6 +130,22 @@ class Status < ApplicationRecord select('reblog_of_id').where(reblog_of_id: status_ids).where(account_id: account_id).map { |s| [s.reblog_of_id, true] }.to_h end + def reload_stale_associations!(cached_items) + account_ids = [] + + cached_items.each do |item| + account_ids << item.account_id + account_ids << item.reblog.account_id if item.reblog? + end + + accounts = Account.where(id: account_ids.uniq).map { |a| [a.id, a] }.to_h + + cached_items.each do |item| + item.account = accounts[item.account_id] + item.reblog.account = accounts[item.reblog.account_id] if item.reblog? + end + end + private def filter_timeline(query, account) diff --git a/db/migrate/20161203164520_add_from_account_id_to_notifications.rb b/db/migrate/20161203164520_add_from_account_id_to_notifications.rb new file mode 100644 index 0000000000..295f95599f --- /dev/null +++ b/db/migrate/20161203164520_add_from_account_id_to_notifications.rb @@ -0,0 +1,14 @@ +class AddFromAccountIdToNotifications < ActiveRecord::Migration[5.0] + def up + add_column :notifications, :from_account_id, :integer, null: false, default: 1 + + Notification.where(activity_type: 'Status').update_all('from_account_id = (SELECT statuses.account_id FROM notifications AS notifications1 INNER JOIN statuses ON notifications1.activity_id = statuses.id WHERE notifications1.activity_type = \'Status\' AND notifications1.id = notifications.id)') + Notification.where(activity_type: 'Mention').update_all('from_account_id = (SELECT statuses.account_id FROM notifications AS notifications1 INNER JOIN mentions ON notifications1.activity_id = mentions.id INNER JOIN statuses ON mentions.status_id = statuses.id WHERE notifications1.activity_type = \'Mention\' AND notifications1.id = notifications.id)') + Notification.where(activity_type: 'Favourite').update_all('from_account_id = (SELECT favourites.account_id FROM notifications AS notifications1 INNER JOIN favourites ON notifications1.activity_id = favourites.id WHERE notifications1.activity_type = \'Favourite\' AND notifications1.id = notifications.id)') + Notification.where(activity_type: 'Follow').update_all('from_account_id = (SELECT follows.account_id FROM notifications AS notifications1 INNER JOIN follows ON notifications1.activity_id = follows.id WHERE notifications1.activity_type = \'Follow\' AND notifications1.id = notifications.id)') + end + + def down + remove_column :notifications, :from_account_id + end +end diff --git a/db/schema.rb b/db/schema.rb index f9080ae2d9..c9676ca400 100644 --- a/db/schema.rb +++ b/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: 20161202132159) do +ActiveRecord::Schema.define(version: 20161203164520) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -100,8 +100,9 @@ ActiveRecord::Schema.define(version: 20161202132159) do t.integer "account_id" t.integer "activity_id" t.string "activity_type" - t.datetime "created_at", null: false - t.datetime "updated_at", null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.integer "from_account_id", default: 1, null: false t.index ["account_id", "activity_id", "activity_type"], name: "account_activity", unique: true, using: :btree end