From de0f63e0ac90da2d6252fccad87cf831fedca605 Mon Sep 17 00:00:00 2001
From: alpaca-tc <alpaca-tc@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