commit: 5bea42412e41b1121af04dbe6ef5c9bed22219b8
parent: 04166c4a35c97f5540252617c7858bf20d315a7e
Author: Matt Jankowski <mjankowski@thoughtbot.com>
Date: Mon, 8 May 2017 18:45:02 -0400
Services specs for subscribe and unsubscribe (#2928)
* Add specs for unsubscribe service
* Fix non existent methods in unsubscribe service
* Clean up status handling in subscribe service
Diffstat:
4 files changed, 52 insertions(+), 10 deletions(-)
diff --git a/app/services/subscribe_service.rb b/app/services/subscribe_service.rb
@@ -8,28 +8,28 @@ class SubscribeService < BaseService
response = subscription.subscribe
if response_failed_permanently?(response)
- # An error in the 4xx range (except for 429, which is rate limiting)
- # means we're not allowed to subscribe. Fail and move on
+ # We're not allowed to subscribe. Fail and move on.
account.secret = ''
account.save!
elsif response_successful?(response)
- # Anything in the 2xx range means the subscription will be confirmed
- # asynchronously, we've done what we needed to do
+ # The subscription will be confirmed asynchronously.
account.save!
else
- # What's left is the 5xx range and 429, which means we need to retry
- # at a later time. Fail loudly!
+ # The response was either a 429 rate limit, or a 5xx error.
+ # We need to retry at a later time. Fail loudly!
raise "Subscription attempt failed for #{account.acct} (#{account.hub_url}): HTTP #{response.code}"
end
end
private
+ # Any response in the 3xx or 4xx range, except for 429 (rate limit)
def response_failed_permanently?(response)
- response.code > 299 && response.code < 500 && response.code != 429
+ (response.status.redirect? || response.status.client_error?) && !response.status.too_many_requests?
end
+ # Any response in the 2xx range
def response_successful?(response)
- response.code > 199 && response.code < 300
+ response.status.success?
end
end
diff --git a/app/services/unsubscribe_service.rb b/app/services/unsubscribe_service.rb
@@ -5,8 +5,8 @@ class UnsubscribeService < BaseService
subscription = account.subscription(api_subscription_url(account.id))
response = subscription.unsubscribe
- unless response.successful?
- Rails.logger.debug "PuSH unsubscribe for #{account.acct} failed: #{response.message}"
+ unless response.status.success?
+ Rails.logger.debug "PuSH unsubscribe for #{account.acct} failed: #{response.status}"
end
account.secret = ''
diff --git a/spec/services/subscribe_service_spec.rb b/spec/services/subscribe_service_spec.rb
@@ -35,4 +35,9 @@ RSpec.describe SubscribeService do
stub_request(:post, 'http://hub.example.com/').to_return(status: 503)
expect { subject.call(account) }.to raise_error(/Subscription attempt failed/)
end
+
+ it 'fails loudly if rate limited' do
+ stub_request(:post, 'http://hub.example.com/').to_return(status: 429)
+ expect { subject.call(account) }.to raise_error(/Subscription attempt failed/)
+ end
end
diff --git a/spec/services/unsubscribe_service_spec.rb b/spec/services/unsubscribe_service_spec.rb
@@ -0,0 +1,37 @@
+require 'rails_helper'
+
+RSpec.describe UnsubscribeService do
+ let(:account) { Fabricate(:account, username: 'bob', domain: 'example.com', hub_url: 'http://hub.example.com') }
+ subject { UnsubscribeService.new }
+
+ it 'removes the secret and resets expiration on account' do
+ stub_request(:post, 'http://hub.example.com/').to_return(status: 204)
+ subject.call(account)
+ account.reload
+
+ expect(account.secret).to be_blank
+ expect(account.subscription_expires_at).to be_blank
+ end
+
+ it 'logs error on subscription failure' do
+ logger = stub_logger
+ stub_request(:post, 'http://hub.example.com/').to_return(status: 404)
+ subject.call(account)
+
+ expect(logger).to have_received(:debug).with(/unsubscribe for bob@example.com failed/)
+ end
+
+ it 'logs error on connection failure' do
+ logger = stub_logger
+ stub_request(:post, 'http://hub.example.com/').to_raise(HTTP::Error)
+ subject.call(account)
+
+ expect(logger).to have_received(:debug).with(/PuSH subscription request for bob@example.com could not be made due to HTTP or SSL error/)
+ end
+
+ def stub_logger
+ double(debug: nil).tap do |logger|
+ allow(Rails).to receive(:logger).and_return(logger)
+ end
+ end
+end