From 059ebbf48dc56971b88e26a15303a75643de8b98 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Tue, 20 Sep 2016 00:39:03 +0200 Subject: [PATCH] Separate PuSH subscriptions from following, add mastodon:push:refresh task, respect hub.lease_seconds (fix #46) --- .../api/subscriptions_controller.rb | 1 + app/models/account.rb | 6 ++++ app/services/follow_remote_account_service.rb | 29 ++++--------------- app/services/follow_service.rb | 5 ++++ app/services/process_feed_service.rb | 8 +++-- app/services/process_interaction_service.rb | 8 +++-- app/services/subscribe_service.rb | 20 +++++++++++++ ...add_subscription_expires_at_to_accounts.rb | 5 ++++ db/schema.rb | 27 ++++++++--------- lib/tasks/mastodon.rake | 11 +++++-- 10 files changed, 77 insertions(+), 43 deletions(-) create mode 100644 app/services/subscribe_service.rb create mode 100644 db/migrate/20160919221059_add_subscription_expires_at_to_accounts.rb diff --git a/app/controllers/api/subscriptions_controller.rb b/app/controllers/api/subscriptions_controller.rb index e923b9b66..84b88765a 100644 --- a/app/controllers/api/subscriptions_controller.rb +++ b/app/controllers/api/subscriptions_controller.rb @@ -4,6 +4,7 @@ class Api::SubscriptionsController < ApiController def show if @account.subscription(api_subscription_url(@account.id)).valid?(params['hub.topic'], params['hub.verify_token']) + @account.update(subscription_expires_at: Time.now + (params['hub.lease_seconds'].to_i).seconds) render plain: HTMLEntities.new.encode(params['hub.challenge']), status: 200 else head 404 diff --git a/app/models/account.rb b/app/models/account.rb index 6e96defe4..449075aa8 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -38,6 +38,12 @@ class Account < ApplicationRecord has_many :media_attachments, dependent: :destroy + scope :remote, -> { where.not(domain: nil) } + scope :local, -> { where(domain: nil) } + scope :without_followers, -> { where('(select count(f.id) from follows as f where f.target_account_id = accounts.id) = 0') } + scope :with_followers, -> { where('(select count(f.id) from follows as f where f.target_account_id = accounts.id) > 0') } + scope :expiring, -> (time) { where(subscription_expires_at: nil).or(where('subscription_expires_at < ?', time)).remote.with_followers } + def follow!(other_account) self.active_relationships.where(target_account: other_account).first_or_create!(target_account: other_account) end diff --git a/app/services/follow_remote_account_service.rb b/app/services/follow_remote_account_service.rb index 00285f47a..f3a384ecc 100644 --- a/app/services/follow_remote_account_service.rb +++ b/app/services/follow_remote_account_service.rb @@ -3,22 +3,18 @@ class FollowRemoteAccountService < BaseService # When creating, look up the user's webfinger and fetch all # important information from their feed # @param [String] uri User URI in the form of username@domain - # @param [Boolean] subscribe Whether to initiate a PubSubHubbub subscription # @return [Account] - def call(uri, subscribe = true) + def call(uri) username, domain = uri.split('@') return Account.find_local(username) if domain == Rails.configuration.x.local_domain || domain.nil? account = Account.find_remote(username, domain) - if account.nil? - Rails.logger.debug "Creating new remote account for #{uri}" - account = Account.new(username: username, domain: domain) - elsif account.subscribed? - Rails.logger.debug "Already subscribed to remote account #{uri}" - return account - end + return account unless account.nil? + + Rails.logger.debug "Creating new remote account for #{uri}" + account = Account.new(username: username, domain: domain) data = Goldfinger.finger("acct:#{uri}") @@ -45,16 +41,6 @@ class FollowRemoteAccountService < BaseService get_profile(feed, account) account.save! - if subscribe - account.secret = SecureRandom.hex - account.verify_token = SecureRandom.hex - - subscription = account.subscription(api_subscription_url(account.id)) - subscription.subscribe - - account.save! - end - return account end @@ -90,8 +76,3 @@ class FollowRemoteAccountService < BaseService end end -class NoAuthorFeedError < StandardError -end - -class NoHubError < StandardError -end diff --git a/app/services/follow_service.rb b/app/services/follow_service.rb index 45f145df3..fb782e871 100644 --- a/app/services/follow_service.rb +++ b/app/services/follow_service.rb @@ -12,6 +12,7 @@ class FollowService < BaseService if target_account.local? NotificationMailer.follow(target_account, source_account).deliver_later else + subscribe_service.(target_account) NotificationWorker.perform_async(follow.stream_entry.id, target_account.id) end @@ -40,4 +41,8 @@ class FollowService < BaseService def follow_remote_account_service @follow_remote_account_service ||= FollowRemoteAccountService.new end + + def subscribe_service + @subscribe_service ||= SubscribeService.new + end end diff --git a/app/services/process_feed_service.rb b/app/services/process_feed_service.rb index 5fde19547..a69205494 100644 --- a/app/services/process_feed_service.rb +++ b/app/services/process_feed_service.rb @@ -106,7 +106,7 @@ class ProcessFeedService < BaseService end def delete_post!(status) - RemoveStatusService.new.(status) + remove_status_service.(status) end def find_original_status(_xml, id) @@ -126,7 +126,7 @@ class ProcessFeedService < BaseService account = Account.find_by(username: username, domain: domain) if account.nil? - account = follow_remote_account_service.("#{username}@#{domain}", false) + account = follow_remote_account_service.("#{username}@#{domain}") end status = Status.new(account: account, uri: target_id(xml), text: target_content(xml), url: target_url(xml), created_at: published(xml), updated_at: updated(xml)) @@ -196,4 +196,8 @@ class ProcessFeedService < BaseService def update_remote_profile_service @update_remote_profile_service ||= UpdateRemoteProfileService.new end + + def remove_status_service + @remove_status_service ||= RemoveStatusService.new + end end diff --git a/app/services/process_interaction_service.rb b/app/services/process_interaction_service.rb index 3266f73e3..1e3ed6b12 100644 --- a/app/services/process_interaction_service.rb +++ b/app/services/process_interaction_service.rb @@ -14,7 +14,7 @@ class ProcessInteractionService < BaseService account = Account.find_by(username: username, domain: domain) if account.nil? - account = follow_remote_account_service.("#{username}@#{domain}", false) + account = follow_remote_account_service.("#{username}@#{domain}") end if salmon.verify(envelope, account.keypair) @@ -71,7 +71,7 @@ class ProcessInteractionService < BaseService return if status.nil? if account.id == status.account_id - RemoveStatusService.new.(status) + remove_status_service.(status) end end @@ -108,4 +108,8 @@ class ProcessInteractionService < BaseService def update_remote_profile_service @update_remote_profile_service ||= UpdateRemoteProfileService.new end + + def remove_status_service + @remove_status_service ||= RemoveStatusService.new + end end diff --git a/app/services/subscribe_service.rb b/app/services/subscribe_service.rb new file mode 100644 index 000000000..7ead559d5 --- /dev/null +++ b/app/services/subscribe_service.rb @@ -0,0 +1,20 @@ +class SubscribeService < BaseService + def call(account) + account.secret = SecureRandom.hex + account.verify_token = SecureRandom.hex + + subscription = account.subscription(api_subscription_url(account.id)) + response = subscription.subscribe + + unless response.successful? + account.secret = '' + account.verify_token = '' + + Rails.logger.debug "PuSH subscription request for #{account.acct} failed: #{response.message}" + end + + account.save! + rescue HTTP::Error, OpenSSL::SSL::SSLError + Rails.logger.debug "PuSH subscription request for #{account.acct} could not be made due to HTTP or SSL error" + end +end diff --git a/db/migrate/20160919221059_add_subscription_expires_at_to_accounts.rb b/db/migrate/20160919221059_add_subscription_expires_at_to_accounts.rb new file mode 100644 index 000000000..5fd7f39e6 --- /dev/null +++ b/db/migrate/20160919221059_add_subscription_expires_at_to_accounts.rb @@ -0,0 +1,5 @@ +class AddSubscriptionExpiresAtToAccounts < ActiveRecord::Migration[5.0] + def change + add_column :accounts, :subscription_expires_at, :datetime, null: true, default: nil + end +end diff --git a/db/schema.rb b/db/schema.rb index 4a3078e64..3179942c0 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,26 +10,26 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20160905150353) do +ActiveRecord::Schema.define(version: 20160919221059) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" create_table "accounts", force: :cascade do |t| - t.string "username", default: "", null: false + t.string "username", default: "", null: false t.string "domain" - t.string "verify_token", default: "", null: false - t.string "secret", default: "", null: false + t.string "verify_token", default: "", null: false + t.string "secret", default: "", null: false t.text "private_key" - t.text "public_key", default: "", null: false - t.string "remote_url", default: "", null: false - t.string "salmon_url", default: "", null: false - t.string "hub_url", default: "", null: false - t.datetime "created_at", null: false - t.datetime "updated_at", null: false - t.text "note", default: "", null: false - t.string "display_name", default: "", null: false - t.string "uri", default: "", null: false + t.text "public_key", default: "", null: false + t.string "remote_url", default: "", null: false + t.string "salmon_url", default: "", null: false + t.string "hub_url", default: "", null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.text "note", default: "", null: false + t.string "display_name", default: "", null: false + t.string "uri", default: "", null: false t.string "url" t.string "avatar_file_name" t.string "avatar_content_type" @@ -40,6 +40,7 @@ ActiveRecord::Schema.define(version: 20160905150353) do t.integer "header_file_size" t.datetime "header_updated_at" t.string "avatar_remote_url" + t.datetime "subscription_expires_at" t.index ["username", "domain"], name: "index_accounts_on_username_and_domain", unique: true, using: :btree end diff --git a/lib/tasks/mastodon.rake b/lib/tasks/mastodon.rake index f59cdfcae..5bc056a56 100644 --- a/lib/tasks/mastodon.rake +++ b/lib/tasks/mastodon.rake @@ -11,9 +11,16 @@ namespace :mastodon do namespace :push do desc 'Unsubscribes from PuSH updates of feeds nobody follows locally' task clear: :environment do - Account.where('(select count(f.id) from follows as f where f.target_account_id = accounts.id) = 0').where.not(domain: nil).find_each do |a| + Account.remote.without_followers.find_each do |a| a.subscription('').unsubscribe - a.update!(verify_token: '', secret: '') + a.update!(verify_token: '', secret: '', subscription_expires_at: nil) + end + end + + desc 'Re-subscribes to soon expiring PuSH subscriptions' + task refresh: :environment do + Account.expiring(1.day.from_now).find_each do |a| + SubscribeService.new.(a) end end end