From b8e166894b1d41ef417c89836a336590d93532f9 Mon Sep 17 00:00:00 2001
From: Eugen Rochko <eugen@zeonfederated.com>
Date: Sat, 6 May 2017 13:05:03 +0200
Subject: [PATCH] Fix Scheduler::SubscriptionsScheduler (#2834)

* Fix Scheduler::SubscriptionsScheduler, add worker test for it

* Change production log level of Sidekiq to "warn" instead of "info"
---
 app/workers/application_worker.rb             |  7 -------
 app/workers/distribution_worker.rb            |  4 ++--
 .../pubsubhubbub/confirmation_worker.rb       |  2 +-
 app/workers/pubsubhubbub/subscribe_worker.rb  |  2 +-
 .../scheduler/subscriptions_scheduler.rb      |  4 ++--
 config/environments/production.rb             |  1 +
 .../scheduler/subscriptions_scheduler_spec.rb | 19 +++++++++++++++++++
 7 files changed, 26 insertions(+), 13 deletions(-)
 delete mode 100644 app/workers/application_worker.rb
 create mode 100644 spec/workers/scheduler/subscriptions_scheduler_spec.rb

diff --git a/app/workers/application_worker.rb b/app/workers/application_worker.rb
deleted file mode 100644
index 436f24763..000000000
--- a/app/workers/application_worker.rb
+++ /dev/null
@@ -1,7 +0,0 @@
-# frozen_string_literal: true
-
-class ApplicationWorker
-  def info(message)
-    Rails.logger.info("#{self.class.name} - #{message}")
-  end
-end
diff --git a/app/workers/distribution_worker.rb b/app/workers/distribution_worker.rb
index f7953689b..f423d43ae 100644
--- a/app/workers/distribution_worker.rb
+++ b/app/workers/distribution_worker.rb
@@ -1,11 +1,11 @@
 # frozen_string_literal: true
 
-class DistributionWorker < ApplicationWorker
+class DistributionWorker
   include Sidekiq::Worker
 
   def perform(status_id)
     FanOutOnWriteService.new.call(Status.find(status_id))
   rescue ActiveRecord::RecordNotFound
-    info("Couldn't find the status")
+    true
   end
 end
diff --git a/app/workers/pubsubhubbub/confirmation_worker.rb b/app/workers/pubsubhubbub/confirmation_worker.rb
index 868fd9f97..b02dd3f55 100644
--- a/app/workers/pubsubhubbub/confirmation_worker.rb
+++ b/app/workers/pubsubhubbub/confirmation_worker.rb
@@ -25,7 +25,7 @@ class Pubsubhubbub::ConfirmationWorker
 
     body = response.body.to_s
 
-    Rails.logger.debug "Confirming PuSH subscription for #{subscription.callback_url} with challenge #{challenge}: #{body}"
+    logger.debug "Confirming PuSH subscription for #{subscription.callback_url} with challenge #{challenge}: #{body}"
 
     if mode == 'subscribe' && body == challenge
       subscription.save!
diff --git a/app/workers/pubsubhubbub/subscribe_worker.rb b/app/workers/pubsubhubbub/subscribe_worker.rb
index 0c4111a8c..5b0956b6b 100644
--- a/app/workers/pubsubhubbub/subscribe_worker.rb
+++ b/app/workers/pubsubhubbub/subscribe_worker.rb
@@ -7,7 +7,7 @@ class Pubsubhubbub::SubscribeWorker
 
   def perform(account_id)
     account = Account.find(account_id)
-    Rails.logger.debug "PuSH re-subscribing to #{account.acct}"
+    logger.debug "PuSH re-subscribing to #{account.acct}"
     ::SubscribeService.new.call(account)
   end
 end
diff --git a/app/workers/scheduler/subscriptions_scheduler.rb b/app/workers/scheduler/subscriptions_scheduler.rb
index 03622e95b..3ea3ad2b8 100644
--- a/app/workers/scheduler/subscriptions_scheduler.rb
+++ b/app/workers/scheduler/subscriptions_scheduler.rb
@@ -5,9 +5,9 @@ class Scheduler::SubscriptionsScheduler
   include Sidekiq::Worker
 
   def perform
-    Rails.logger.debug 'Queueing PuSH re-subscriptions'
+    logger.info 'Queueing PuSH re-subscriptions'
 
-    expiring_accounts.pluck(:id) do |id|
+    expiring_accounts.pluck(:id).each do |id|
       Pubsubhubbub::SubscribeWorker.perform_async(id)
     end
   end
diff --git a/config/environments/production.rb b/config/environments/production.rb
index 3cbf5f899..2c13707c2 100644
--- a/config/environments/production.rb
+++ b/config/environments/production.rb
@@ -87,6 +87,7 @@ Rails.application.configure do
 
   config.to_prepare do
     StatsD.backend = StatsD::Instrument::Backends::NullBackend.new if ENV['STATSD_ADDR'].blank?
+    Sidekiq::Logging.logger.level = Logger::WARN
   end
 
   config.action_dispatch.default_headers = {
diff --git a/spec/workers/scheduler/subscriptions_scheduler_spec.rb b/spec/workers/scheduler/subscriptions_scheduler_spec.rb
new file mode 100644
index 000000000..a7d1046de
--- /dev/null
+++ b/spec/workers/scheduler/subscriptions_scheduler_spec.rb
@@ -0,0 +1,19 @@
+require 'rails_helper'
+
+describe Scheduler::SubscriptionsScheduler do
+  subject { Scheduler::SubscriptionsScheduler.new }
+
+  let!(:expiring_account1) { Fabricate(:account, subscription_expires_at: 20.minutes.from_now, domain: 'example.com', followers_count: 1, hub_url: 'http://hub.example.com') }
+  let!(:expiring_account2) { Fabricate(:account, subscription_expires_at: 4.hours.from_now, domain: 'example.org', followers_count: 1, hub_url: 'http://hub.example.org') }
+
+  before do
+    stub_request(:post, 'http://hub.example.com/').to_return(status: 202)
+    stub_request(:post, 'http://hub.example.org/').to_return(status: 202)
+  end
+
+  it 're-subscribes for all expiring accounts' do
+    subject.perform
+    expect(a_request(:post, 'http://hub.example.com/')).to have_been_made.once
+    expect(a_request(:post, 'http://hub.example.org/')).to have_been_made.once
+  end
+end