From 05b72368ed56f23e247d28c2798789f91bb096c5 Mon Sep 17 00:00:00 2001 From: alpaca-tc Date: Sun, 7 May 2017 04:55:40 +0900 Subject: [PATCH] Fixes slow query in notifications api (#2851) * Add inclusion validation for activity_type * When excluding all types with `WHERE NOT IN`, the query search all rows. --- app/models/notification.rb | 31 +++++++++++---------- spec/fabricators/notification_fabricator.rb | 2 +- 2 files changed, 17 insertions(+), 16 deletions(-) diff --git a/app/models/notification.rb b/app/models/notification.rb index 5b8f46cadb..fe1fff11fa 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -16,6 +16,16 @@ class Notification < ApplicationRecord include Paginable include Cacheable + TYPE_CLASS_MAP = { + mention: 'Mention', + reblog: 'Status', + follow: 'Follow', + follow_request: 'FollowRequest', + favourite: 'Favourite', + }.freeze + + STATUS_INCLUDES = [:account, :stream_entry, :media_attachments, :tags, mentions: :account, reblog: [:stream_entry, :account, :media_attachments, :tags, mentions: :account]].freeze + belongs_to :account belongs_to :from_account, class_name: 'Account' belongs_to :activity, polymorphic: true @@ -27,19 +37,15 @@ class Notification < ApplicationRecord belongs_to :favourite, foreign_type: 'Favourite', foreign_key: 'activity_id' validates :account_id, uniqueness: { scope: [:activity_type, :activity_id] } - - TYPE_CLASS_MAP = { - mention: 'Mention', - reblog: 'Status', - follow: 'Follow', - follow_request: 'FollowRequest', - favourite: 'Favourite', - }.freeze - - STATUS_INCLUDES = [:account, :stream_entry, :media_attachments, :tags, mentions: :account, reblog: [:stream_entry, :account, :media_attachments, :tags, mentions: :account]].freeze + validates :activity_type, inclusion: { in: TYPE_CLASS_MAP.values } scope :cache_ids, -> { select(:id, :updated_at, :activity_type, :activity_id) } + scope :browserable, ->(exclude_types = []) { + types = TYPE_CLASS_MAP.values - activity_types_from_types(exclude_types + [:follow_request]) + where(activity_type: types) + } + cache_associated :from_account, status: STATUS_INCLUDES, mention: [status: STATUS_INCLUDES], favourite: [:account, status: STATUS_INCLUDES], follow: :account def activity(eager_loaded = true) @@ -64,11 +70,6 @@ class Notification < ApplicationRecord end class << self - def browserable(types = []) - types.concat([:follow_request]) - where.not(activity_type: activity_types_from_types(types)) - end - 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 diff --git a/spec/fabricators/notification_fabricator.rb b/spec/fabricators/notification_fabricator.rb index 08e9849048..b92af06836 100644 --- a/spec/fabricators/notification_fabricator.rb +++ b/spec/fabricators/notification_fabricator.rb @@ -1,4 +1,4 @@ Fabricator(:notification) do activity_id 1 - activity_type "MyString" + activity_type 'Favourite' end