From 83435c49ea4f31d80d81658d8faa69ed5350e26f Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Tue, 30 May 2017 20:15:09 -0400 Subject: [PATCH] Clean up api/subscriptions controller (#3448) --- .../api/subscriptions_controller.rb | 31 +++++++++++++++---- .../api/subscriptions_controller_spec.rb | 27 +++++++++++----- 2 files changed, 45 insertions(+), 13 deletions(-) diff --git a/app/controllers/api/subscriptions_controller.rb b/app/controllers/api/subscriptions_controller.rb index 135a5632e6..dd2f42aab7 100644 --- a/app/controllers/api/subscriptions_controller.rb +++ b/app/controllers/api/subscriptions_controller.rb @@ -5,18 +5,15 @@ class Api::SubscriptionsController < ApiController respond_to :txt def show - if @account.subscription(api_subscription_url(@account.id)).valid?(params['hub.topic']) - @account.update(subscription_expires_at: Time.now.utc + (params['hub.lease_seconds'] || 86_400).to_i.seconds) - render plain: HTMLEntities.new.encode(params['hub.challenge']), status: 200 + if subscription.valid?(params['hub.topic']) + @account.update(subscription_expires_at: future_expires) + render plain: encoded_challenge, status: 200 else head 404 end end def update - body = request.body.read - subscription = @account.subscription(api_subscription_url(@account.id)) - if subscription.verify(body, request.headers['HTTP_X_HUB_SIGNATURE']) ProcessingWorker.perform_async(@account.id, body.force_encoding('UTF-8')) end @@ -26,6 +23,28 @@ class Api::SubscriptionsController < ApiController private + def subscription + @_subscription ||= @account.subscription( + api_subscription_url(@account.id) + ) + end + + def body + @_body ||= request.body.read + end + + def encoded_challenge + HTMLEntities.new.encode(params['hub.challenge']) + end + + def future_expires + Time.now.utc + lease_seconds_or_default + end + + def lease_seconds_or_default + (params['hub.lease_seconds'] || 86_400).to_i.seconds + end + def set_account @account = Account.find(params[:id]) end diff --git a/spec/controllers/api/subscriptions_controller_spec.rb b/spec/controllers/api/subscriptions_controller_spec.rb index 97e36420d6..76f9740ca5 100644 --- a/spec/controllers/api/subscriptions_controller_spec.rb +++ b/spec/controllers/api/subscriptions_controller_spec.rb @@ -6,16 +6,29 @@ RSpec.describe Api::SubscriptionsController, type: :controller do let(:account) { Fabricate(:account, username: 'gargron', domain: 'quitter.no', remote_url: 'topic_url', secret: 'abc') } describe 'GET #show' do - before do - get :show, params: { :id => account.id, 'hub.topic' => 'topic_url', 'hub.challenge' => '456', 'hub.lease_seconds' => "#{86400 * 30}" } + context 'with valid subscription' do + before do + get :show, params: { :id => account.id, 'hub.topic' => 'topic_url', 'hub.challenge' => '456', 'hub.lease_seconds' => "#{86400 * 30}" } + end + + it 'returns http success' do + expect(response).to have_http_status(:success) + end + + it 'echoes back the challenge' do + expect(response.body).to match '456' + end end - it 'returns http success' do - expect(response).to have_http_status(:success) - end + context 'with invalid subscription' do + before do + expect_any_instance_of(Account).to receive_message_chain(:subscription, :valid?).and_return(false) + get :show, params: { :id => account.id } + end - it 'echoes back the challenge' do - expect(response.body).to match '456' + it 'returns http success' do + expect(response).to have_http_status(:missing) + end end end