From 6793bec4c67e695100cb4d8551f0bda0b7e87b12 Mon Sep 17 00:00:00 2001
From: Eugen Rochko <eugen@zeonfederated.com>
Date: Fri, 4 May 2018 21:14:34 +0200
Subject: Store URIs of follows, follow requests and blocks for ActivityPub
 (#7160)

Same URI passed between follow request and follow, since they are
the same thing in ActivityPub. Local URIs are generated during
creation using UUIDs and are passed to serializers.
---
 spec/models/follow_request_spec.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'spec/models')

diff --git a/spec/models/follow_request_spec.rb b/spec/models/follow_request_spec.rb
index 59893a14f..2cf28b263 100644
--- a/spec/models/follow_request_spec.rb
+++ b/spec/models/follow_request_spec.rb
@@ -7,7 +7,7 @@ RSpec.describe FollowRequest, type: :model do
     let(:target_account) { Fabricate(:account) }
 
     it 'calls Account#follow!, MergeWorker.perform_async, and #destroy!' do
-      expect(account).to        receive(:follow!).with(target_account, reblogs: true)
+      expect(account).to        receive(:follow!).with(target_account, reblogs: true, uri: follow_request.uri)
       expect(MergeWorker).to    receive(:perform_async).with(target_account.id, account.id)
       expect(follow_request).to receive(:destroy!)
       follow_request.authorize!
-- 
cgit 


From c73ce7b695aef1bbfe36bf674173d5a9fb988df5 Mon Sep 17 00:00:00 2001
From: Eugen Rochko <eugen@zeonfederated.com>
Date: Sat, 5 May 2018 00:54:24 +0200
Subject: Store home feeds for 7 days instead of 14 (#7354)

* Store home feeds for 7 days instead of 14

Reduces workload for status fan-out to active followers

* Fix test for user model
---
 app/models/user.rb                       | 2 +-
 app/services/fan_out_on_write_service.rb | 4 ++--
 spec/models/user_spec.rb                 | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

(limited to 'spec/models')

diff --git a/app/models/user.rb b/app/models/user.rb
index a9f3e1da2..f5f542f07 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -41,7 +41,7 @@ class User < ApplicationRecord
   include Settings::Extend
   include Omniauthable
 
-  ACTIVE_DURATION = 14.days
+  ACTIVE_DURATION = 7.days
 
   devise :two_factor_authenticatable,
          otp_secret_encryption_key: Rails.configuration.x.otp_secret
diff --git a/app/services/fan_out_on_write_service.rb b/app/services/fan_out_on_write_service.rb
index 510b80c82..cb82a79ed 100644
--- a/app/services/fan_out_on_write_service.rb
+++ b/app/services/fan_out_on_write_service.rb
@@ -37,7 +37,7 @@ class FanOutOnWriteService < BaseService
   def deliver_to_followers(status)
     Rails.logger.debug "Delivering status #{status.id} to followers"
 
-    status.account.followers.where(domain: nil).joins(:user).where('users.current_sign_in_at > ?', 14.days.ago).select(:id).reorder(nil).find_in_batches do |followers|
+    status.account.followers.where(domain: nil).joins(:user).where('users.current_sign_in_at > ?', User::ACTIVE_DURATION.ago).select(:id).reorder(nil).find_in_batches do |followers|
       FeedInsertWorker.push_bulk(followers) do |follower|
         [status.id, follower.id, :home]
       end
@@ -47,7 +47,7 @@ class FanOutOnWriteService < BaseService
   def deliver_to_lists(status)
     Rails.logger.debug "Delivering status #{status.id} to lists"
 
-    status.account.lists.joins(account: :user).where('users.current_sign_in_at > ?', 14.days.ago).select(:id).reorder(nil).find_in_batches do |lists|
+    status.account.lists.joins(account: :user).where('users.current_sign_in_at > ?', User::ACTIVE_DURATION.ago).select(:id).reorder(nil).find_in_batches do |lists|
       FeedInsertWorker.push_bulk(lists) do |list|
         [status.id, list.id, :list]
       end
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 760214ded..cc8d88cc8 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -75,7 +75,7 @@ RSpec.describe User, type: :model do
     describe 'inactive' do
       it 'returns a relation of inactive users' do
         specified = Fabricate(:user, current_sign_in_at: 15.days.ago)
-        Fabricate(:user, current_sign_in_at: 13.days.ago)
+        Fabricate(:user, current_sign_in_at: 6.days.ago)
 
         expect(User.inactive).to match_array([specified])
       end
-- 
cgit 


From b4fb766b23f4b50b51a366f55b451770ece3153a Mon Sep 17 00:00:00 2001
From: Eugen Rochko <eugen@zeonfederated.com>
Date: Fri, 11 May 2018 11:49:12 +0200
Subject: Add REST API for Web Push Notifications subscriptions (#7445)

- POST /api/v1/push/subscription
- PUT /api/v1/push/subscription
- DELETE /api/v1/push/subscription
- New OAuth scope: "push" (required for the above methods)
---
 .../api/v1/push/subscriptions_controller.rb        | 50 +++++++++++++
 .../api/web/push_subscriptions_controller.rb       | 11 +--
 app/controllers/shares_controller.rb               |  1 +
 app/models/user.rb                                 |  2 +-
 app/models/web/push_subscription.rb                | 47 +++++++-----
 app/serializers/initial_state_serializer.rb        |  4 +-
 .../rest/web_push_subscription_serializer.rb       | 13 ++++
 app/serializers/web/notification_serializer.rb     |  2 +-
 app/services/notify_service.rb                     | 21 +++---
 app/workers/web/push_notification_worker.rb        | 18 +++++
 app/workers/web_push_notification_worker.rb        | 25 -------
 config/initializers/doorkeeper.rb                  |  2 +-
 config/locales/doorkeeper.en.yml                   |  1 +
 config/routes.rb                                   |  4 ++
 ...dd_access_token_id_to_web_push_subscriptions.rb |  6 ++
 ...0180510230049_migrate_web_push_subscriptions.rb | 13 ++++
 db/schema.rb                                       |  8 ++-
 .../api/v1/push/subscriptions_controller_spec.rb   | 83 ++++++++++++++++++++++
 .../api/web/push_subscriptions_controller_spec.rb  | 16 ++---
 spec/models/web/push_subscription_spec.rb          | 12 ----
 20 files changed, 258 insertions(+), 81 deletions(-)
 create mode 100644 app/controllers/api/v1/push/subscriptions_controller.rb
 create mode 100644 app/serializers/rest/web_push_subscription_serializer.rb
 create mode 100644 app/workers/web/push_notification_worker.rb
 delete mode 100644 app/workers/web_push_notification_worker.rb
 create mode 100644 db/migrate/20180510214435_add_access_token_id_to_web_push_subscriptions.rb
 create mode 100644 db/migrate/20180510230049_migrate_web_push_subscriptions.rb
 create mode 100644 spec/controllers/api/v1/push/subscriptions_controller_spec.rb

(limited to 'spec/models')

diff --git a/app/controllers/api/v1/push/subscriptions_controller.rb b/app/controllers/api/v1/push/subscriptions_controller.rb
new file mode 100644
index 000000000..5038cc03c
--- /dev/null
+++ b/app/controllers/api/v1/push/subscriptions_controller.rb
@@ -0,0 +1,50 @@
+# frozen_string_literal: true
+
+class Api::V1::Push::SubscriptionsController < Api::BaseController
+  before_action -> { doorkeeper_authorize! :push }
+  before_action :require_user!
+  before_action :set_web_push_subscription
+
+  def create
+    @web_subscription&.destroy!
+
+    @web_subscription = ::Web::PushSubscription.create!(
+      endpoint: subscription_params[:endpoint],
+      key_p256dh: subscription_params[:keys][:p256dh],
+      key_auth: subscription_params[:keys][:auth],
+      data: data_params,
+      user_id: current_user.id,
+      access_token_id: doorkeeper_token.id
+    )
+
+    render json: @web_subscription, serializer: REST::WebPushSubscriptionSerializer
+  end
+
+  def update
+    raise ActiveRecord::RecordNotFound if @web_subscription.nil?
+
+    @web_subscription.update!(data: data_params)
+
+    render json: @web_subscription, serializer: REST::WebPushSubscriptionSerializer
+  end
+
+  def destroy
+    @web_subscription&.destroy!
+    render_empty
+  end
+
+  private
+
+  def set_web_push_subscription
+    @web_subscription = ::Web::PushSubscription.find_by(access_token_id: doorkeeper_token.id)
+  end
+
+  def subscription_params
+    params.require(:subscription).permit(:endpoint, keys: [:auth, :p256dh])
+  end
+
+  def data_params
+    return {} if params[:data].blank?
+    params.require(:data).permit(alerts: [:follow, :favourite, :reblog, :mention])
+  end
+end
diff --git a/app/controllers/api/web/push_subscriptions_controller.rb b/app/controllers/api/web/push_subscriptions_controller.rb
index 249e7c186..fe8e42580 100644
--- a/app/controllers/api/web/push_subscriptions_controller.rb
+++ b/app/controllers/api/web/push_subscriptions_controller.rb
@@ -31,22 +31,23 @@ class Api::Web::PushSubscriptionsController < Api::Web::BaseController
       endpoint: subscription_params[:endpoint],
       key_p256dh: subscription_params[:keys][:p256dh],
       key_auth: subscription_params[:keys][:auth],
-      data: data
+      data: data,
+      user_id: active_session.user_id,
+      access_token_id: active_session.access_token_id
     )
 
     active_session.update!(web_push_subscription: web_subscription)
 
-    render json: web_subscription.as_payload
+    render json: web_subscription, serializer: REST::WebPushSubscriptionSerializer
   end
 
   def update
     params.require([:id])
 
     web_subscription = ::Web::PushSubscription.find(params[:id])
-
     web_subscription.update!(data: data_params)
 
-    render json: web_subscription.as_payload
+    render json: web_subscription, serializer: REST::WebPushSubscriptionSerializer
   end
 
   private
@@ -56,6 +57,6 @@ class Api::Web::PushSubscriptionsController < Api::Web::BaseController
   end
 
   def data_params
-    @data_params ||= params.require(:data).permit(:alerts)
+    @data_params ||= params.require(:data).permit(alerts: [:follow, :favourite, :reblog, :mention])
   end
 end
diff --git a/app/controllers/shares_controller.rb b/app/controllers/shares_controller.rb
index 3ec831a72..9ef1e0749 100644
--- a/app/controllers/shares_controller.rb
+++ b/app/controllers/shares_controller.rb
@@ -15,6 +15,7 @@ class SharesController < ApplicationController
 
   def initial_state_params
     text = [params[:title], params[:text], params[:url]].compact.join(' ')
+
     {
       settings: Web::Setting.find_by(user: current_user)&.data || {},
       push_subscription: current_account.user.web_push_subscription(current_session),
diff --git a/app/models/user.rb b/app/models/user.rb
index f5f542f07..21c217e77 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -245,7 +245,7 @@ class User < ApplicationRecord
   end
 
   def web_push_subscription(session)
-    session.web_push_subscription.nil? ? nil : session.web_push_subscription.as_payload
+    session.web_push_subscription.nil? ? nil : session.web_push_subscription
   end
 
   def invite_code=(code)
diff --git a/app/models/web/push_subscription.rb b/app/models/web/push_subscription.rb
index 1736106f7..df549c6d3 100644
--- a/app/models/web/push_subscription.rb
+++ b/app/models/web/push_subscription.rb
@@ -3,38 +3,51 @@
 #
 # Table name: web_push_subscriptions
 #
-#  id         :bigint(8)        not null, primary key
-#  endpoint   :string           not null
-#  key_p256dh :string           not null
-#  key_auth   :string           not null
-#  data       :json
-#  created_at :datetime         not null
-#  updated_at :datetime         not null
+#  id              :bigint(8)        not null, primary key
+#  endpoint        :string           not null
+#  key_p256dh      :string           not null
+#  key_auth        :string           not null
+#  data            :json
+#  created_at      :datetime         not null
+#  updated_at      :datetime         not null
+#  access_token_id :bigint(8)
+#  user_id         :bigint(8)
 #
 
-require 'webpush'
-
 class Web::PushSubscription < ApplicationRecord
+  belongs_to :user, optional: true
+  belongs_to :access_token, class_name: 'Doorkeeper::AccessToken', optional: true
+
   has_one :session_activation
 
   def push(notification)
-    I18n.with_locale(session_activation.user.locale || I18n.default_locale) do
+    I18n.with_locale(associated_user.locale || I18n.default_locale) do
       push_payload(message_from(notification), 48.hours.seconds)
     end
   end
 
   def pushable?(notification)
-    data&.key?('alerts') && data['alerts'][notification.type.to_s]
+    data&.key?('alerts') && ActiveModel::Type::Boolean.new.cast(data['alerts'][notification.type.to_s])
   end
 
-  def as_payload
-    payload = { id: id, endpoint: endpoint }
-    payload[:alerts] = data['alerts'] if data&.key?('alerts')
-    payload
+  def associated_user
+    return @associated_user if defined?(@associated_user)
+
+    @associated_user = if user_id.nil?
+                         session_activation.user
+                       else
+                         user
+                       end
   end
 
-  def access_token
-    find_or_create_access_token.token
+  def associated_access_token
+    return @associated_access_token if defined?(@associated_access_token)
+
+    @associated_access_token = if access_token_id.nil?
+                                 find_or_create_access_token.token
+                               else
+                                 access_token
+                               end
   end
 
   private
diff --git a/app/serializers/initial_state_serializer.rb b/app/serializers/initial_state_serializer.rb
index 3b908e224..6c9fba2f5 100644
--- a/app/serializers/initial_state_serializer.rb
+++ b/app/serializers/initial_state_serializer.rb
@@ -2,7 +2,9 @@
 
 class InitialStateSerializer < ActiveModel::Serializer
   attributes :meta, :compose, :accounts,
-             :media_attachments, :settings, :push_subscription
+             :media_attachments, :settings
+
+  has_one :push_subscription, serializer: REST::WebPushSubscriptionSerializer
 
   def meta
     store = {
diff --git a/app/serializers/rest/web_push_subscription_serializer.rb b/app/serializers/rest/web_push_subscription_serializer.rb
new file mode 100644
index 000000000..7fd952a56
--- /dev/null
+++ b/app/serializers/rest/web_push_subscription_serializer.rb
@@ -0,0 +1,13 @@
+# frozen_string_literal: true
+
+class REST::WebPushSubscriptionSerializer < ActiveModel::Serializer
+  attributes :id, :endpoint, :alerts, :server_key
+
+  def alerts
+    object.data&.dig('alerts') || {}
+  end
+
+  def server_key
+    Rails.configuration.x.vapid_public_key
+  end
+end
diff --git a/app/serializers/web/notification_serializer.rb b/app/serializers/web/notification_serializer.rb
index e5524fe7a..31c703832 100644
--- a/app/serializers/web/notification_serializer.rb
+++ b/app/serializers/web/notification_serializer.rb
@@ -54,7 +54,7 @@ class Web::NotificationSerializer < ActiveModel::Serializer
 
     def access_token
       return if actions.empty?
-      current_push_subscription.access_token
+      current_push_subscription.associated_access_token
     end
 
     def message
diff --git a/app/services/notify_service.rb b/app/services/notify_service.rb
index ba086449c..6490d2735 100644
--- a/app/services/notify_service.rb
+++ b/app/services/notify_service.rb
@@ -9,6 +9,7 @@ class NotifyService < BaseService
     return if recipient.user.nil? || blocked?
 
     create_notification
+    push_notification if @notification.browserable?
     send_email if email_enabled?
   rescue ActiveRecord::RecordInvalid
     return
@@ -101,25 +102,27 @@ class NotifyService < BaseService
 
   def create_notification
     @notification.save!
-    return unless @notification.browserable?
+  end
+
+  def push_notification
+    return if @notification.activity.nil?
+
     Redis.current.publish("timeline:#{@recipient.id}", Oj.dump(event: :notification, payload: InlineRenderer.render(@notification, @recipient, :notification)))
     send_push_notifications
   end
 
   def send_push_notifications
-    # HACK: Can be caused by quickly unfavouriting a status, since creating
-    # a favourite and creating a notification are not wrapped in a transaction.
-    return if @notification.activity.nil?
-
-    sessions_with_subscriptions = @recipient.user.session_activations.where.not(web_push_subscription: nil)
-    sessions_with_subscriptions_ids = sessions_with_subscriptions.select { |session| session.web_push_subscription.pushable? @notification }.map(&:id)
+    subscriptions_ids = ::Web::PushSubscription.where(user_id: @recipient.user.id)
+                                               .select { |subscription| subscription.pushable?(@notification) }
+                                               .map(&:id)
 
-    WebPushNotificationWorker.push_bulk(sessions_with_subscriptions_ids) do |session_activation_id|
-      [session_activation_id, @notification.id]
+    ::Web::PushNotificationWorker.push_bulk(subscriptions_ids) do |subscription_id|
+      [subscription_id, @notification.id]
     end
   end
 
   def send_email
+    return if @notification.activity.nil?
     NotificationMailer.public_send(@notification.type, @recipient, @notification).deliver_later
   end
 
diff --git a/app/workers/web/push_notification_worker.rb b/app/workers/web/push_notification_worker.rb
new file mode 100644
index 000000000..4a40e5c8b
--- /dev/null
+++ b/app/workers/web/push_notification_worker.rb
@@ -0,0 +1,18 @@
+# frozen_string_literal: true
+
+class Web::PushNotificationWorker
+  include Sidekiq::Worker
+
+  sidekiq_options backtrace: true
+
+  def perform(subscription_id, notification_id)
+    subscription = ::Web::PushSubscription.find(subscription_id)
+    notification = Notification.find(notification_id)
+
+    subscription.push(notification) unless notification.activity.nil?
+  rescue Webpush::InvalidSubscription, Webpush::ExpiredSubscription
+    subscription.destroy!
+  rescue ActiveRecord::RecordNotFound
+    true
+  end
+end
diff --git a/app/workers/web_push_notification_worker.rb b/app/workers/web_push_notification_worker.rb
deleted file mode 100644
index eacea04c3..000000000
--- a/app/workers/web_push_notification_worker.rb
+++ /dev/null
@@ -1,25 +0,0 @@
-# frozen_string_literal: true
-
-class WebPushNotificationWorker
-  include Sidekiq::Worker
-
-  sidekiq_options backtrace: true
-
-  def perform(session_activation_id, notification_id)
-    session_activation = SessionActivation.find(session_activation_id)
-    notification       = Notification.find(notification_id)
-
-    return if session_activation.web_push_subscription.nil? || notification.activity.nil?
-
-    session_activation.web_push_subscription.push(notification)
-  rescue Webpush::InvalidSubscription, Webpush::ExpiredSubscription
-    # Subscription expiration is not currently implemented in any browser
-
-    session_activation.web_push_subscription.destroy!
-    session_activation.update!(web_push_subscription: nil)
-
-    true
-  rescue ActiveRecord::RecordNotFound
-    true
-  end
-end
diff --git a/config/initializers/doorkeeper.rb b/config/initializers/doorkeeper.rb
index 074f8c410..469553803 100644
--- a/config/initializers/doorkeeper.rb
+++ b/config/initializers/doorkeeper.rb
@@ -55,7 +55,7 @@ Doorkeeper.configure do
   # For more information go to
   # https://github.com/doorkeeper-gem/doorkeeper/wiki/Using-Scopes
   default_scopes  :read
-  optional_scopes :write, :follow
+  optional_scopes :write, :follow, :push
 
   # Change the way client credentials are retrieved from the request object.
   # By default it retrieves first from the `HTTP_AUTHORIZATION` header, then
diff --git a/config/locales/doorkeeper.en.yml b/config/locales/doorkeeper.en.yml
index 33d544bed..eca1fc675 100644
--- a/config/locales/doorkeeper.en.yml
+++ b/config/locales/doorkeeper.en.yml
@@ -115,5 +115,6 @@ en:
         title: OAuth authorization required
     scopes:
       follow: follow, block, unblock and unfollow accounts
+      push: receive push notifications for your account
       read: read your account's data
       write: post on your behalf
diff --git a/config/routes.rb b/config/routes.rb
index 4c920cf74..b7bd1a7ed 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -306,6 +306,10 @@ Rails.application.routes.draw do
       resources :lists, only: [:index, :create, :show, :update, :destroy] do
         resource :accounts, only: [:show, :create, :destroy], controller: 'lists/accounts'
       end
+
+      namespace :push do
+        resource :subscription, only: [:create, :update, :destroy]
+      end
     end
 
     namespace :web do
diff --git a/db/migrate/20180510214435_add_access_token_id_to_web_push_subscriptions.rb b/db/migrate/20180510214435_add_access_token_id_to_web_push_subscriptions.rb
new file mode 100644
index 000000000..94ef8e0f5
--- /dev/null
+++ b/db/migrate/20180510214435_add_access_token_id_to_web_push_subscriptions.rb
@@ -0,0 +1,6 @@
+class AddAccessTokenIdToWebPushSubscriptions < ActiveRecord::Migration[5.2]
+  def change
+    add_reference :web_push_subscriptions, :access_token, null: true, default: nil, foreign_key: { on_delete: :cascade, to_table: :oauth_access_tokens }, index: false
+    add_reference :web_push_subscriptions, :user, null: true, default: nil, foreign_key: { on_delete: :cascade }, index: false
+  end
+end
diff --git a/db/migrate/20180510230049_migrate_web_push_subscriptions.rb b/db/migrate/20180510230049_migrate_web_push_subscriptions.rb
new file mode 100644
index 000000000..6de1bed79
--- /dev/null
+++ b/db/migrate/20180510230049_migrate_web_push_subscriptions.rb
@@ -0,0 +1,13 @@
+class MigrateWebPushSubscriptions < ActiveRecord::Migration[5.2]
+  disable_ddl_transaction!
+
+  def up
+    add_index :web_push_subscriptions, :user_id, algorithm: :concurrently
+    add_index :web_push_subscriptions, :access_token_id, algorithm: :concurrently
+  end
+
+  def down
+    remove_index :web_push_subscriptions, :user_id
+    remove_index :web_push_subscriptions, :access_token_id
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index f7fa24b83..221c08d98 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: 2018_05_06_221944) do
+ActiveRecord::Schema.define(version: 2018_05_10_230049) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -539,6 +539,10 @@ ActiveRecord::Schema.define(version: 2018_05_06_221944) do
     t.json "data"
     t.datetime "created_at", null: false
     t.datetime "updated_at", null: false
+    t.bigint "access_token_id"
+    t.bigint "user_id"
+    t.index ["access_token_id"], name: "index_web_push_subscriptions_on_access_token_id"
+    t.index ["user_id"], name: "index_web_push_subscriptions_on_user_id"
   end
 
   create_table "web_settings", force: :cascade do |t|
@@ -605,5 +609,7 @@ ActiveRecord::Schema.define(version: 2018_05_06_221944) do
   add_foreign_key "subscriptions", "accounts", name: "fk_9847d1cbb5", on_delete: :cascade
   add_foreign_key "users", "accounts", name: "fk_50500f500d", on_delete: :cascade
   add_foreign_key "users", "invites", on_delete: :nullify
+  add_foreign_key "web_push_subscriptions", "oauth_access_tokens", column: "access_token_id", on_delete: :cascade
+  add_foreign_key "web_push_subscriptions", "users", on_delete: :cascade
   add_foreign_key "web_settings", "users", name: "fk_11910667b2", on_delete: :cascade
 end
diff --git a/spec/controllers/api/v1/push/subscriptions_controller_spec.rb b/spec/controllers/api/v1/push/subscriptions_controller_spec.rb
new file mode 100644
index 000000000..01146294f
--- /dev/null
+++ b/spec/controllers/api/v1/push/subscriptions_controller_spec.rb
@@ -0,0 +1,83 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+describe Api::V1::Push::SubscriptionsController do
+  render_views
+
+  let(:user)  { Fabricate(:user) }
+  let(:token) { Fabricate(:accessible_access_token, resource_owner_id: user.id, scopes: 'push') }
+
+  before do
+    allow(controller).to receive(:doorkeeper_token) { token }
+  end
+
+  let(:create_payload) do
+    {
+      subscription: {
+        endpoint: 'https://fcm.googleapis.com/fcm/send/fiuH06a27qE:APA91bHnSiGcLwdaxdyqVXNDR9w1NlztsHb6lyt5WDKOC_Z_Q8BlFxQoR8tWFSXUIDdkyw0EdvxTu63iqamSaqVSevW5LfoFwojws8XYDXv_NRRLH6vo2CdgiN4jgHv5VLt2A8ah6lUX',
+        keys: {
+          p256dh: 'BEm_a0bdPDhf0SOsrnB2-ategf1hHoCnpXgQsFj5JCkcoMrMt2WHoPfEYOYPzOIs9mZE8ZUaD7VA5vouy0kEkr8=',
+          auth: 'eH_C8rq2raXqlcBVDa1gLg==',
+        },
+      }
+    }.with_indifferent_access
+  end
+
+  let(:alerts_payload) do
+    {
+      data: {
+        alerts: {
+          follow: true,
+          favourite: false,
+          reblog: true,
+          mention: false,
+        }
+      }
+    }.with_indifferent_access
+  end
+
+  describe 'POST #create' do
+    it 'saves push subscriptions' do
+      post :create, params: create_payload
+
+      push_subscription = Web::PushSubscription.find_by(endpoint: create_payload[:subscription][:endpoint])
+
+      expect(push_subscription.endpoint).to eq(create_payload[:subscription][:endpoint])
+      expect(push_subscription.key_p256dh).to eq(create_payload[:subscription][:keys][:p256dh])
+      expect(push_subscription.key_auth).to eq(create_payload[:subscription][:keys][:auth])
+      expect(push_subscription.user_id).to eq user.id
+      expect(push_subscription.access_token_id).to eq token.id
+    end
+
+    it 'replaces old subscription on repeat calls' do
+      post :create, params: create_payload
+      post :create, params: create_payload
+
+      expect(Web::PushSubscription.where(endpoint: create_payload[:subscription][:endpoint]).count).to eq 1
+    end
+  end
+
+  describe 'PUT #update' do
+    it 'changes alert settings' do
+      post :create, params: create_payload
+      put :update, params: alerts_payload
+
+      push_subscription = Web::PushSubscription.find_by(endpoint: create_payload[:subscription][:endpoint])
+
+      expect(push_subscription.data.dig('alerts', 'follow')).to eq(alerts_payload[:data][:alerts][:follow].to_s)
+      expect(push_subscription.data.dig('alerts', 'favourite')).to eq(alerts_payload[:data][:alerts][:favourite].to_s)
+      expect(push_subscription.data.dig('alerts', 'reblog')).to eq(alerts_payload[:data][:alerts][:reblog].to_s)
+      expect(push_subscription.data.dig('alerts', 'mention')).to eq(alerts_payload[:data][:alerts][:mention].to_s)
+    end
+  end
+
+  describe 'DELETE #destroy' do
+    it 'removes the subscription' do
+      post :create, params: create_payload
+      delete :destroy
+
+      expect(Web::PushSubscription.find_by(endpoint: create_payload[:subscription][:endpoint])).to be_nil
+    end
+  end
+end
diff --git a/spec/controllers/api/web/push_subscriptions_controller_spec.rb b/spec/controllers/api/web/push_subscriptions_controller_spec.rb
index bbf94c5c6..381cdeab9 100644
--- a/spec/controllers/api/web/push_subscriptions_controller_spec.rb
+++ b/spec/controllers/api/web/push_subscriptions_controller_spec.rb
@@ -59,10 +59,10 @@ describe Api::Web::PushSubscriptionsController do
 
         push_subscription = Web::PushSubscription.find_by(endpoint: create_payload[:subscription][:endpoint])
 
-        expect(push_subscription.data['follow']).to eq(alerts_payload[:data][:follow])
-        expect(push_subscription.data['favourite']).to eq(alerts_payload[:data][:favourite])
-        expect(push_subscription.data['reblog']).to eq(alerts_payload[:data][:reblog])
-        expect(push_subscription.data['mention']).to eq(alerts_payload[:data][:mention])
+        expect(push_subscription.data['alerts']['follow']).to eq(alerts_payload[:data][:alerts][:follow].to_s)
+        expect(push_subscription.data['alerts']['favourite']).to eq(alerts_payload[:data][:alerts][:favourite].to_s)
+        expect(push_subscription.data['alerts']['reblog']).to eq(alerts_payload[:data][:alerts][:reblog].to_s)
+        expect(push_subscription.data['alerts']['mention']).to eq(alerts_payload[:data][:alerts][:mention].to_s)
       end
     end
   end
@@ -81,10 +81,10 @@ describe Api::Web::PushSubscriptionsController do
 
       push_subscription = Web::PushSubscription.find_by(endpoint: create_payload[:subscription][:endpoint])
 
-      expect(push_subscription.data['follow']).to eq(alerts_payload[:data][:follow])
-      expect(push_subscription.data['favourite']).to eq(alerts_payload[:data][:favourite])
-      expect(push_subscription.data['reblog']).to eq(alerts_payload[:data][:reblog])
-      expect(push_subscription.data['mention']).to eq(alerts_payload[:data][:mention])
+      expect(push_subscription.data['alerts']['follow']).to eq(alerts_payload[:data][:alerts][:follow].to_s)
+      expect(push_subscription.data['alerts']['favourite']).to eq(alerts_payload[:data][:alerts][:favourite].to_s)
+      expect(push_subscription.data['alerts']['reblog']).to eq(alerts_payload[:data][:alerts][:reblog].to_s)
+      expect(push_subscription.data['alerts']['mention']).to eq(alerts_payload[:data][:alerts][:mention].to_s)
     end
   end
 end
diff --git a/spec/models/web/push_subscription_spec.rb b/spec/models/web/push_subscription_spec.rb
index 574da55ac..c6665611c 100644
--- a/spec/models/web/push_subscription_spec.rb
+++ b/spec/models/web/push_subscription_spec.rb
@@ -2,20 +2,8 @@ require 'rails_helper'
 
 RSpec.describe Web::PushSubscription, type: :model do
   let(:alerts) { { mention: true, reblog: false, follow: true, follow_request: false, favourite: true } }
-  let(:payload_no_alerts) { Web::PushSubscription.new(id: 1, endpoint: 'a', key_p256dh: 'c', key_auth: 'd').as_payload }
-  let(:payload_alerts) { Web::PushSubscription.new(id: 1, endpoint: 'a', key_p256dh: 'c', key_auth: 'd', data: { alerts: alerts }).as_payload }
   let(:push_subscription) { Web::PushSubscription.new(data: { alerts: alerts }) }
 
-  describe '#as_payload' do
-    it 'only returns id and endpoint' do
-      expect(payload_no_alerts.keys).to eq [:id, :endpoint]
-    end
-
-    it 'returns alerts if set' do
-      expect(payload_alerts.keys).to eq [:id, :endpoint, :alerts]
-    end
-  end
-
   describe '#pushable?' do
     it 'obeys alert settings' do
       expect(push_subscription.send(:pushable?, Notification.new(activity_type: 'Mention'))).to eq true
-- 
cgit 


From b87a1229c74aebec02cd08091f25613f6dff4428 Mon Sep 17 00:00:00 2001
From: tateisu <tateisu@gmail.com>
Date: Mon, 28 May 2018 18:04:06 +0900
Subject: optimize direct timeline (#7614)

* optimize direct timeline

* fix typo in class name

* change filter condition for direct timeline

* fix codestyle issue

* revoke index_accounts_not_silenced because direct timeline does not use it.

* revoke index_accounts_not_silenced because direct timeline does not use it.

* fix rspec test condition.

* fix rspec test condition.

* fix rspec test condition.

* revoke adding column and partial index

* (direct timeline) move merging logic to model

* fix pagination parameter

* add method arguments that switches return array of status or cache_ids

* fix order by

* returns ActiveRecord.Relation in default behavor

* fix codestyle issue
---
 .../api/v1/timelines/direct_controller.rb          | 15 +++++---
 app/models/status.rb                               | 43 +++++++++++++++++++---
 spec/models/status_spec.rb                         | 23 ++++++------
 3 files changed, 59 insertions(+), 22 deletions(-)

(limited to 'spec/models')

diff --git a/app/controllers/api/v1/timelines/direct_controller.rb b/app/controllers/api/v1/timelines/direct_controller.rb
index d455227eb..ef64078be 100644
--- a/app/controllers/api/v1/timelines/direct_controller.rb
+++ b/app/controllers/api/v1/timelines/direct_controller.rb
@@ -23,15 +23,18 @@ class Api::V1::Timelines::DirectController < Api::BaseController
   end
 
   def direct_statuses
-    direct_timeline_statuses.paginate_by_max_id(
-      limit_param(DEFAULT_STATUSES_LIMIT),
-      params[:max_id],
-      params[:since_id]
-    )
+    direct_timeline_statuses
   end
 
   def direct_timeline_statuses
-    Status.as_direct_timeline(current_account)
+    # this query requires built in pagination.
+    Status.as_direct_timeline(
+      current_account,
+      limit_param(DEFAULT_STATUSES_LIMIT),
+      params[:max_id],
+      params[:since_id],
+      true # returns array of cache_ids object
+    )
   end
 
   def insert_pagination_headers
diff --git a/app/models/status.rb b/app/models/status.rb
index 853e75b43..54f3f68f5 100644
--- a/app/models/status.rb
+++ b/app/models/status.rb
@@ -188,12 +188,45 @@ class Status < ApplicationRecord
       where(account: [account] + account.following).where(visibility: [:public, :unlisted, :private])
     end
 
-    def as_direct_timeline(account)
-      query = joins("LEFT OUTER JOIN mentions ON statuses.id = mentions.status_id AND mentions.account_id = #{account.id}")
-              .where("mentions.account_id = #{account.id} OR statuses.account_id = #{account.id}")
-              .where(visibility: [:direct])
+    def as_direct_timeline(account, limit = 20, max_id = nil, since_id = nil, cache_ids = false)
+      # direct timeline is mix of direct message from_me and to_me.
+      # 2 querys are executed with pagination.
+      # constant expression using arel_table is required for partial index
+
+      # _from_me part does not require any timeline filters
+      query_from_me = where(account_id: account.id)
+                      .where(Status.arel_table[:visibility].eq(3))
+                      .limit(limit)
+                      .order('statuses.id DESC')
+
+      # _to_me part requires mute and block filter.
+      # FIXME: may we check mutes.hide_notifications?
+      query_to_me = Status
+                    .joins(:mentions)
+                    .merge(Mention.where(account_id: account.id))
+                    .where(Status.arel_table[:visibility].eq(3))
+                    .limit(limit)
+                    .order('mentions.status_id DESC')
+                    .not_excluded_by_account(account)
+
+      if max_id.present?
+        query_from_me = query_from_me.where('statuses.id < ?', max_id)
+        query_to_me = query_to_me.where('mentions.status_id < ?', max_id)
+      end
+
+      if since_id.present?
+        query_from_me = query_from_me.where('statuses.id > ?', since_id)
+        query_to_me = query_to_me.where('mentions.status_id > ?', since_id)
+      end
 
-      apply_timeline_filters(query, account, false)
+      if cache_ids
+        # returns array of cache_ids object that have id and updated_at
+        (query_from_me.cache_ids.to_a + query_to_me.cache_ids.to_a).uniq(&:id).sort_by(&:id).reverse.take(limit)
+      else
+        # returns ActiveRecord.Relation
+        items = (query_from_me.select(:id).to_a + query_to_me.select(:id).to_a).uniq(&:id).sort_by(&:id).reverse.take(limit)
+        Status.where(id: items.map(&:id))
+      end
     end
 
     def as_public_timeline(account = nil, local_only = false)
diff --git a/spec/models/status_spec.rb b/spec/models/status_spec.rb
index c6701018e..aee4f49b4 100644
--- a/spec/models/status_spec.rb
+++ b/spec/models/status_spec.rb
@@ -154,7 +154,7 @@ RSpec.describe Status, type: :model do
 
   describe '#target' do
     it 'returns nil if the status is self-contained' do
-      expect(subject.target).to be_nil
+     expect(subject.target).to be_nil
     end
 
     it 'returns nil if the status is a reply' do
@@ -333,24 +333,25 @@ RSpec.describe Status, type: :model do
       expect(@results).to_not include(@followed_public_status)
     end
 
-    it 'includes direct statuses mentioning recipient from followed' do
-      Fabricate(:mention, account: account, status: @followed_direct_status)
-      expect(@results).to include(@followed_direct_status)
-    end
-
     it 'does not include direct statuses not mentioning recipient from followed' do
       expect(@results).to_not include(@followed_direct_status)
     end
 
-    it 'includes direct statuses mentioning recipient from non-followed' do
-      Fabricate(:mention, account: account, status: @not_followed_direct_status)
-      expect(@results).to include(@not_followed_direct_status)
-    end
-
     it 'does not include direct statuses not mentioning recipient from non-followed' do
       expect(@results).to_not include(@not_followed_direct_status)
     end
 
+    it 'includes direct statuses mentioning recipient from followed' do
+      Fabricate(:mention, account: account, status: @followed_direct_status)
+      results2 = Status.as_direct_timeline(account)
+      expect(results2).to include(@followed_direct_status)
+    end
+
+    it 'includes direct statuses mentioning recipient from non-followed' do
+      Fabricate(:mention, account: account, status: @not_followed_direct_status)
+      results2 = Status.as_direct_timeline(account)
+      expect(results2).to include(@not_followed_direct_status)
+    end
   end
 
   describe '.as_public_timeline' do
-- 
cgit