Clean up for api/base controller (#3629)

* Move ApiController to Api/BaseController * API controllers inherit from Api::BaseController * Add coverage for various error cases in api/base controller
8 years ago · 73540ffe6b
parent 92bb166246
commit 73540ffe6b
35 changed files with 87 additions and 54 deletions
--- a/app/controllers/api/activitypub/activities_controller.rb
+++ b/app/controllers/api/activitypub/activities_controller.rb
@ -1,6 +1,6 @@
 # frozen_string_literal: true

-class Api::Activitypub::ActivitiesController < ApiController
+class Api::Activitypub::ActivitiesController < Api::BaseController
  include Authorization

  # before_action :set_follow, only: [:show_follow]
--- a/app/controllers/api/activitypub/notes_controller.rb
+++ b/app/controllers/api/activitypub/notes_controller.rb
@ -1,6 +1,6 @@
 # frozen_string_literal: true

-class Api::Activitypub::NotesController < ApiController
+class Api::Activitypub::NotesController < Api::BaseController
  include Authorization

  before_action :set_status
--- a/app/controllers/api/activitypub/outbox_controller.rb
+++ b/app/controllers/api/activitypub/outbox_controller.rb
@ -1,6 +1,6 @@
 # frozen_string_literal: true

-class Api::Activitypub::OutboxController < ApiController
+class Api::Activitypub::OutboxController < Api::BaseController
  before_action :set_account

  respond_to :activitystreams2
--- a/app/controllers/api/base_controller.rb
+++ b/app/controllers/api/base_controller.rb
@ -1,6 +1,6 @@
 # frozen_string_literal: true

-class ApiController < ApplicationController
+class Api::BaseController < ApplicationController
  DEFAULT_STATUSES_LIMIT = 20
  DEFAULT_ACCOUNTS_LIMIT = 40

--- a/app/controllers/api/oembed_controller.rb
+++ b/app/controllers/api/oembed_controller.rb
@ -1,6 +1,6 @@
 # frozen_string_literal: true

-class Api::OEmbedController < ApiController
+class Api::OEmbedController < Api::BaseController
  respond_to :json

  def show
--- a/app/controllers/api/push_controller.rb
+++ b/app/controllers/api/push_controller.rb
@ -1,6 +1,6 @@
 # frozen_string_literal: true

-class Api::PushController < ApiController
+class Api::PushController < Api::BaseController
  def update
    response, status = process_push_request
    render plain: response, status: status
--- a/app/controllers/api/salmon_controller.rb
+++ b/app/controllers/api/salmon_controller.rb
@ -1,6 +1,6 @@
 # frozen_string_literal: true

-class Api::SalmonController < ApiController
+class Api::SalmonController < Api::BaseController
  before_action :set_account
  respond_to :txt

--- a/app/controllers/api/subscriptions_controller.rb
+++ b/app/controllers/api/subscriptions_controller.rb
@ -1,6 +1,6 @@
 # frozen_string_literal: true

-class Api::SubscriptionsController < ApiController
+class Api::SubscriptionsController < Api::BaseController
  before_action :set_account
  respond_to :txt

--- a/app/controllers/api/v1/accounts/credentials_controller.rb
+++ b/app/controllers/api/v1/accounts/credentials_controller.rb
@ -1,6 +1,6 @@
 # frozen_string_literal: true

-class Api::V1::Accounts::CredentialsController < ApiController
+class Api::V1::Accounts::CredentialsController < Api::BaseController
  before_action -> { doorkeeper_authorize! :write }, only: [:update]
  before_action :require_user!

--- a/app/controllers/api/v1/accounts/follower_accounts_controller.rb
+++ b/app/controllers/api/v1/accounts/follower_accounts_controller.rb
@ -1,6 +1,6 @@
 # frozen_string_literal: true

-class Api::V1::Accounts::FollowerAccountsController < ApiController
+class Api::V1::Accounts::FollowerAccountsController < Api::BaseController
  before_action -> { doorkeeper_authorize! :read }
  before_action :set_account
  after_action :insert_pagination_headers
--- a/app/controllers/api/v1/accounts/following_accounts_controller.rb
+++ b/app/controllers/api/v1/accounts/following_accounts_controller.rb
@ -1,6 +1,6 @@
 # frozen_string_literal: true

-class Api::V1::Accounts::FollowingAccountsController < ApiController
+class Api::V1::Accounts::FollowingAccountsController < Api::BaseController
  before_action -> { doorkeeper_authorize! :read }
  before_action :set_account
  after_action :insert_pagination_headers
--- a/app/controllers/api/v1/accounts/relationships_controller.rb
+++ b/app/controllers/api/v1/accounts/relationships_controller.rb
@ -1,6 +1,6 @@
 # frozen_string_literal: true

-class Api::V1::Accounts::RelationshipsController < ApiController
+class Api::V1::Accounts::RelationshipsController < Api::BaseController
  before_action -> { doorkeeper_authorize! :read }
  before_action :require_user!

--- a/app/controllers/api/v1/accounts/search_controller.rb
+++ b/app/controllers/api/v1/accounts/search_controller.rb
@ -1,6 +1,6 @@
 # frozen_string_literal: true

-class Api::V1::Accounts::SearchController < ApiController
+class Api::V1::Accounts::SearchController < Api::BaseController
  before_action -> { doorkeeper_authorize! :read }
  before_action :require_user!

--- a/app/controllers/api/v1/accounts/statuses_controller.rb
+++ b/app/controllers/api/v1/accounts/statuses_controller.rb
@ -1,6 +1,6 @@
 # frozen_string_literal: true

-class Api::V1::Accounts::StatusesController < ApiController
+class Api::V1::Accounts::StatusesController < Api::BaseController
  before_action -> { doorkeeper_authorize! :read }
  before_action :set_account
  after_action :insert_pagination_headers
--- a/app/controllers/api/v1/accounts_controller.rb
+++ b/app/controllers/api/v1/accounts_controller.rb
@ -1,6 +1,6 @@
 # frozen_string_literal: true

-class Api::V1::AccountsController < ApiController
+class Api::V1::AccountsController < Api::BaseController
  before_action -> { doorkeeper_authorize! :read }, except: [:follow, :unfollow, :block, :unblock, :mute, :unmute]
  before_action -> { doorkeeper_authorize! :follow }, only: [:follow, :unfollow, :block, :unblock, :mute, :unmute]
  before_action :require_user!, except: [:show]
--- a/app/controllers/api/v1/apps_controller.rb
+++ b/app/controllers/api/v1/apps_controller.rb
@ -1,6 +1,6 @@
 # frozen_string_literal: true

-class Api::V1::AppsController < ApiController
+class Api::V1::AppsController < Api::BaseController
  respond_to :json

  def create
--- a/app/controllers/api/v1/blocks_controller.rb
+++ b/app/controllers/api/v1/blocks_controller.rb
@ -1,6 +1,6 @@
 # frozen_string_literal: true

-class Api::V1::BlocksController < ApiController
+class Api::V1::BlocksController < Api::BaseController
  before_action -> { doorkeeper_authorize! :follow }
  before_action :require_user!
  after_action :insert_pagination_headers
--- a/app/controllers/api/v1/domain_blocks_controller.rb
+++ b/app/controllers/api/v1/domain_blocks_controller.rb
@ -1,6 +1,6 @@
 # frozen_string_literal: true

-class Api::V1::DomainBlocksController < ApiController
+class Api::V1::DomainBlocksController < Api::BaseController
  BLOCK_LIMIT = 100

  before_action -> { doorkeeper_authorize! :follow }
--- a/app/controllers/api/v1/favourites_controller.rb
+++ b/app/controllers/api/v1/favourites_controller.rb
@ -1,6 +1,6 @@
 # frozen_string_literal: true

-class Api::V1::FavouritesController < ApiController
+class Api::V1::FavouritesController < Api::BaseController
  before_action -> { doorkeeper_authorize! :read }
  before_action :require_user!
  after_action :insert_pagination_headers
--- a/app/controllers/api/v1/follow_requests_controller.rb
+++ b/app/controllers/api/v1/follow_requests_controller.rb
@ -1,6 +1,6 @@
 # frozen_string_literal: true

-class Api::V1::FollowRequestsController < ApiController
+class Api::V1::FollowRequestsController < Api::BaseController
  before_action -> { doorkeeper_authorize! :follow }
  before_action :require_user!
  after_action :insert_pagination_headers, only: :index
--- a/app/controllers/api/v1/follows_controller.rb
+++ b/app/controllers/api/v1/follows_controller.rb
@ -1,6 +1,6 @@
 # frozen_string_literal: true

-class Api::V1::FollowsController < ApiController
+class Api::V1::FollowsController < Api::BaseController
  before_action -> { doorkeeper_authorize! :follow }
  before_action :require_user!

--- a/app/controllers/api/v1/instances_controller.rb
+++ b/app/controllers/api/v1/instances_controller.rb
@ -1,6 +1,6 @@
 # frozen_string_literal: true

-class Api::V1::InstancesController < ApiController
+class Api::V1::InstancesController < Api::BaseController
  respond_to :json

  def show; end
--- a/app/controllers/api/v1/media_controller.rb
+++ b/app/controllers/api/v1/media_controller.rb
@ -1,6 +1,6 @@
 # frozen_string_literal: true

-class Api::V1::MediaController < ApiController
+class Api::V1::MediaController < Api::BaseController
  before_action -> { doorkeeper_authorize! :write }
  before_action :require_user!

--- a/app/controllers/api/v1/mutes_controller.rb
+++ b/app/controllers/api/v1/mutes_controller.rb
@ -1,6 +1,6 @@
 # frozen_string_literal: true

-class Api::V1::MutesController < ApiController
+class Api::V1::MutesController < Api::BaseController
  before_action -> { doorkeeper_authorize! :follow }
  before_action :require_user!
  after_action :insert_pagination_headers
--- a/app/controllers/api/v1/notifications_controller.rb
+++ b/app/controllers/api/v1/notifications_controller.rb
@ -1,6 +1,6 @@
 # frozen_string_literal: true

-class Api::V1::NotificationsController < ApiController
+class Api::V1::NotificationsController < Api::BaseController
  before_action -> { doorkeeper_authorize! :read }
  before_action :require_user!
  after_action :insert_pagination_headers, only: :index
--- a/app/controllers/api/v1/reports_controller.rb
+++ b/app/controllers/api/v1/reports_controller.rb
@ -1,6 +1,6 @@
 # frozen_string_literal: true

-class Api::V1::ReportsController < ApiController
+class Api::V1::ReportsController < Api::BaseController
  before_action -> { doorkeeper_authorize! :read }, except: [:create]
  before_action -> { doorkeeper_authorize! :write }, only:  [:create]
  before_action :require_user!
--- a/app/controllers/api/v1/search_controller.rb
+++ b/app/controllers/api/v1/search_controller.rb
@ -1,6 +1,6 @@
 # frozen_string_literal: true

-class Api::V1::SearchController < ApiController
+class Api::V1::SearchController < Api::BaseController
  RESULTS_LIMIT = 5

  respond_to :json
--- a/app/controllers/api/v1/statuses_controller.rb
+++ b/app/controllers/api/v1/statuses_controller.rb
@ -1,6 +1,6 @@
 # frozen_string_literal: true

-class Api::V1::StatusesController < ApiController
+class Api::V1::StatusesController < Api::BaseController
  include Authorization

  before_action :authorize_if_got_token, except:            [:create, :destroy, :reblog, :unreblog, :favourite, :unfavourite, :mute, :unmute]
--- a/app/controllers/api/v1/streaming_controller.rb
+++ b/app/controllers/api/v1/streaming_controller.rb
@ -1,6 +1,6 @@
 # frozen_string_literal: true

-class Api::V1::StreamingController < ApiController
+class Api::V1::StreamingController < Api::BaseController
  respond_to :json

  def index
--- a/app/controllers/api/v1/timelines/home_controller.rb
+++ b/app/controllers/api/v1/timelines/home_controller.rb
@ -1,6 +1,6 @@
 # frozen_string_literal: true

-class Api::V1::Timelines::HomeController < ApiController
+class Api::V1::Timelines::HomeController < Api::BaseController
  before_action -> { doorkeeper_authorize! :read }, only: [:show]
  before_action :require_user!, only: [:show]
  after_action :insert_pagination_headers, unless: -> { @statuses.empty? }
--- a/app/controllers/api/v1/timelines/public_controller.rb
+++ b/app/controllers/api/v1/timelines/public_controller.rb
@ -1,6 +1,6 @@
 # frozen_string_literal: true

-class Api::V1::Timelines::PublicController < ApiController
+class Api::V1::Timelines::PublicController < Api::BaseController
  after_action :insert_pagination_headers, unless: -> { @statuses.empty? }

  respond_to :json
--- a/app/controllers/api/v1/timelines/tag_controller.rb
+++ b/app/controllers/api/v1/timelines/tag_controller.rb
@ -1,6 +1,6 @@
 # frozen_string_literal: true

-class Api::V1::Timelines::TagController < ApiController
+class Api::V1::Timelines::TagController < Api::BaseController
  before_action :load_tag
  after_action :insert_pagination_headers, unless: -> { @statuses.empty? }

--- a/app/controllers/api/web/settings_controller.rb
+++ b/app/controllers/api/web/settings_controller.rb
@ -1,6 +1,6 @@
 # frozen_string_literal: true

-class Api::Web::SettingsController < ApiController
+class Api::Web::SettingsController < Api::BaseController
  respond_to :json

  before_action :require_user!
--- a/spec/controllers/api/base_controller_spec.rb
+++ b/spec/controllers/api/base_controller_spec.rb
@ -0,0 +1,54 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+class FakeService; end
+
+describe Api::BaseController do
+  controller do
+    def success
+      head 200
+    end
+
+    def error
+      FakeService.new
+    end
+  end
+
+  describe 'Forgery protection' do
+    before do
+      routes.draw { post 'success' => 'api/base#success' }
+    end
+
+    it 'does not protect from forgery' do
+      ActionController::Base.allow_forgery_protection = true
+      post 'success'
+      expect(response).to have_http_status(:success)
+    end
+  end
+
+  describe 'Error handling' do
+    ERRORS_WITH_CODES = {
+      ActiveRecord::RecordInvalid => 422,
+      Mastodon::ValidationError => 422,
+      ActiveRecord::RecordNotFound => 404,
+      Goldfinger::Error => 422,
+      HTTP::Error => 503,
+      OpenSSL::SSL::SSLError => 503,
+      Mastodon::NotPermittedError => 403,
+    }
+
+    before do
+      routes.draw { get 'error' => 'api/base#error' }
+    end
+
+    ERRORS_WITH_CODES.each do |error, code|
+      it "Handles error class of #{error}" do
+        expect(FakeService).to receive(:new).and_raise(error)
+
+        get 'error'
+        expect(response).to have_http_status(code)
+      end
+    end
+  end
+end
--- a/spec/controllers/api_controller_spec.rb
+++ b/spec/controllers/api_controller_spec.rb
@ -1,21 +0,0 @@
-# frozen_string_literal: true
-
-require 'rails_helper'
-
-describe ApiController, type: :controller do
-  controller do
-    def success
-      head 200
-    end
-  end
-
-  before do
-    routes.draw { post 'success' => 'api#success' }
-  end
-
-  it 'does not protect from forgery' do
-    ActionController::Base.allow_forgery_protection = true
-    post 'success'
-    expect(response).to have_http_status(:success)
-  end
-end