Improve code style for error handling [SCI-2773]

app/controllers/api/v1/base_controller.rb

@@ -5,6 +5,14 @@ module Api
     class BaseController < ApiController
       class TypeError < StandardError; end
       class IDMismatchError < StandardError; end
+      class PermissionError < StandardError
+        attr_reader :klass
+        attr_reader :mode
+        def initialize(klass, mode)
+          @klass = klass
+          @mode = mode
+        end
+      end
 
       rescue_from StandardError do |e|
         logger.error e.message
@@ -14,6 +22,15 @@ module Api
                      :bad_request)
       end
 
+      rescue_from PermissionError do |e|
+        model = e.klass.name.underscore
+        render_error(
+          I18n.t("api.core.errors.#{e.mode}_permission.title"),
+          I18n.t("api.core.errors.#{e.mode}_permission.detail", model: model),
+          :forbidden
+        )
+      end
+
       rescue_from TypeError do
         render_error(I18n.t('api.core.errors.type.title'),
                      I18n.t('api.core.errors.type.detail'),
@@ -64,43 +81,40 @@ module Api
         }, status: status
       end
 
-      def permission_error(klass, mode)
-        model = klass.name.underscore
-        render_error(
-          I18n.t("api.core.errors.#{mode}_permission.title"),
-          I18n.t("api.core.errors.#{mode}_permission.detail", model: model),
-          :forbidden
-        )
+      def load_team(key = :team_id)
+        @team = Team.find(params.require(key))
+        raise PermissionError.new(Team, :read) unless can_read_team?(@team)
       end
 
-      def load_team
-        @team = Team.find(params.require(:team_id))
-        permission_error(Team, :read) unless can_read_team?(@team)
+      def load_inventory(key = :inventory_id)
+        @inventory = @team.repositories.find(params.require(key))
       end
 
-      def load_inventory
-        @inventory = @team.repositories.find(params.require(:inventory_id))
-      end
-
-      def load_inventory_column
+      def load_inventory_column(key = :column_id)
         @inventory_column = @inventory.repository_columns
-                                      .find(inventory_cell_params[:column_id])
+                                      .find(params.require(key))
       end
 
-      def load_project
-        @project = @team.projects.find(params.require(:project_id))
-        permission_error(Project, :read) unless can_read_project?(@project)
+      def load_inventory_item(key = :item_id)
+        @inventory_item = @inventory.repository_rows.find(params[key].to_i)
       end
 
-      def load_experiment
-        @experiment = @project.experiments.find(params.require(:experiment_id))
-        permission_error(Experiment, :read) unless can_read_experiment?(
-          @experiment
-        )
+      def load_project(key = :project_id)
+        @project = @team.projects.find(params.require(key))
+        unless can_read_project?(@project)
+          raise PermissionError.new(Project, :read)
+        end
       end
 
-      def load_task
-        @task = @experiment.my_modules.find(params.require(:task_id))
+      def load_experiment(key = :experiment_id)
+        @experiment = @project.experiments.find(params.require(key))
+        unless can_read_experiment?(@experiment)
+          raise PermissionError.new(Experiment, :read)
+        end
+      end
+
+      def load_task(key = :task_id)
+        @task = @experiment.my_modules.find(params.require(key))
       end
     end
   end

app/controllers/api/v1/experiments_controller.rb

@@ -5,7 +5,9 @@ module Api
     class ExperimentsController < BaseController
       before_action :load_team
       before_action :load_project
-      before_action :load_experiment, only: :show
+      before_action only: :show do
+        load_experiment(:id)
+      end
 
       def index
         experiments = @project.experiments
@@ -17,15 +19,6 @@ module Api
       def show
         render jsonapi: @experiment, serializer: ExperimentSerializer
       end
-
-      private
-
-      def load_experiment
-        @experiment = @project.experiments.find(params.require(:id))
-        permission_error(Experiment, :read) unless can_read_experiment?(
-          @experiment
-        )
-      end
     end
   end
 end

app/controllers/api/v1/inventories_controller.rb

@@ -4,7 +4,9 @@ module Api
   module V1
     class InventoriesController < BaseController
       before_action :load_team
-      before_action :load_inventory, only: %i(show update destroy)
+      before_action only: %i(show update destroy) do
+        load_inventory(:id)
+      end
       before_action :check_manage_permissions, only: %i(update destroy)
 
       def index
@@ -16,7 +18,7 @@ module Api
 
       def create
         unless can_create_repositories?(@team)
-          return permission_error(Repository, :create)
+          raise PermissionError.new(Repository, :create)
         end
         inventory = @team.repositories.create!(
           inventory_params.merge(created_by: current_user)
@@ -48,13 +50,9 @@ module Api
 
       private
 
-      def load_inventory
-        @inventory = @team.repositories.find(params.require(:id))
-      end
-
       def check_manage_permissions
         unless can_manage_repository?(@inventory)
-          permission_error(Repository, :manage)
+          raise PermissionError.new(Repository, :manage)
         end
       end
 

app/controllers/api/v1/inventory_cells_controller.rb

@@ -49,8 +49,9 @@ module Api
 
       private
 
-      def load_inventory_item
-        @inventory_item = @inventory.repository_rows.find(params[:item_id].to_i)
+      def load_inventory_column
+        @inventory_column = @inventory.repository_columns
+                                      .find(inventory_cell_params[:column_id])
       end
 
       def load_inventory_cell
@@ -60,7 +61,7 @@ module Api
 
       def check_manage_permissions
         unless can_manage_repository_rows?(@team)
-          permission_error(RepositoryRow, :manage)
+          raise PermissionError.new(RepositoryRow, :manage)
         end
       end
 

app/controllers/api/v1/inventory_columns_controller.rb

@@ -5,7 +5,9 @@ module Api
     class InventoryColumnsController < BaseController
       before_action :load_team
       before_action :load_inventory
-      before_action :load_inventory_column, only: %i(show update destroy)
+      before_action only: %i(show update destroy) do
+        load_inventory_column(:id)
+      end
       before_action :check_manage_permissions, only: %i(update destroy)
       before_action :check_create_permissions, only: %i(create)
 
@@ -52,20 +54,15 @@ module Api
 
       private
 
-      def load_inventory_column
-        @inventory_column = @inventory.repository_columns
-                                      .find(params.require(:id))
-      end
-
       def check_manage_permissions
         unless can_manage_repository_column?(@inventory_column)
-          permission_error(RepositoryColumn, :manage)
+          raise PermissionError.new(RepositoryColumn, :manage)
         end
       end
 
       def check_create_permissions
         unless can_manage_repository?(@inventory)
-          permission_error(RepositoryColumn, :create)
+          raise PermissionError.new(RepositoryColumn, :create)
         end
       end
 

app/controllers/api/v1/inventory_items_controller.rb

@@ -5,7 +5,9 @@ module Api
     class InventoryItemsController < BaseController
       before_action :load_team
       before_action :load_inventory
-      before_action :load_inventory_item, only: %i(show update destroy)
+      before_action only: %i(show update destroy) do
+        load_inventory_item(:id)
+      end
       before_action :check_manage_permissions, only: %i(create update destroy)
 
       def index
@@ -94,13 +96,9 @@ module Api
 
       private
 
-      def load_inventory_item
-        @inventory_item = @inventory.repository_rows.find(params[:id].to_i)
-      end
-
       def check_manage_permissions
         unless can_manage_repository_rows?(@team)
-          permission_error(RepositoryItem, :manage)
+          raise PermissionError.new(RepositoryItem, :manage)
         end
       end
 

app/controllers/api/v1/inventory_list_items_controller.rb

@@ -6,6 +6,7 @@ module Api
       before_action :load_team
       before_action :load_inventory
       before_action :load_inventory_column
+      before_action :check_column_type
       before_action :load_inventory_list_item, only: %i(show update destroy)
       before_action :check_manage_permissions, only: %i(create update destroy)
 
@@ -46,9 +47,7 @@ module Api
 
       private
 
-      def load_inventory_column
-        @inventory_column =
-          @inventory.repository_columns.find(params.require(:column_id))
+      def check_column_type
         unless @inventory_column.data_type == 'RepositoryListValue'
           raise TypeError
         end
@@ -61,7 +60,7 @@ module Api
 
       def check_manage_permissions
         unless can_manage_repository_column?(@inventory_column)
-          permission_error(RepositoryListItem, :manage)
+          raise PermissionError.new(RepositoryListItem, :manage)
         end
       end
 

app/controllers/api/v1/projects_controller.rb

@@ -4,7 +4,9 @@ module Api
   module V1
     class ProjectsController < BaseController
       before_action :load_team
-      before_action :load_project, only: :show
+      before_action only: :show do
+        load_project(:id)
+      end
       before_action :load_project_relative, only: :activities
 
       def index
@@ -29,14 +31,11 @@ module Api
 
       private
 
-      def load_project
-        @project = @team.projects.find(params.require(:id))
-        permission_error(Project, :read) unless can_read_project?(@project)
-      end
-
       def load_project_relative
         @project = @team.projects.find(params.require(:project_id))
-        permission_error(Project, :read) unless can_read_project?(@project)
+        unless can_read_project?(@project)
+          raise PermissionError.new(Project, :read)
+        end
       end
     end
   end

app/controllers/api/v1/results_controller.rb

@@ -41,7 +41,9 @@ module Api
       end
 
       def check_manage_permissions
-        permission_error(MyModule, :manage) unless can_manage_module?(@task)
+        unless can_manage_module?(@task)
+          raise PermissionError.new(MyModule, :manage)
+        end
       end
 
       def create_text_result

app/controllers/api/v1/tasks_controller.rb

@@ -6,7 +6,9 @@ module Api
       before_action :load_team
       before_action :load_project
       before_action :load_experiment
-      before_action :load_task, only: :show
+      before_action only: :show do
+        load_task(:id)
+      end
       before_action :load_task_relative, only: %i(inputs outputs activities)
 
       def index
@@ -46,10 +48,6 @@ module Api
 
       private
 
-      def load_task
-        @task = @experiment.my_modules.find(params.require(:id))
-      end
-
       # Made the method below because its more elegant than changing parameters
       # in routes file, and here. It exists because when we call input or output
       # for a task, the "id" that used to be task id is now an id for the output

app/controllers/api/v1/teams_controller.rb

@@ -3,7 +3,9 @@
 module Api
   module V1
     class TeamsController < BaseController
-      before_action :load_team, only: :show
+      before_action only: :show do
+        load_team(:id)
+      end
 
       def index
         teams = current_user.teams
@@ -15,13 +17,6 @@ module Api
       def show
         render jsonapi: @team, serializer: TeamSerializer, include: :created_by
       end
-
-      private
-
-      def load_team
-        @team = Team.find(params.require(:id))
-        permission_error(Team, :read) unless can_read_team?(@team)
-      end
     end
   end
 end

app/controllers/api/v1/user_identities_controller.rb

@@ -42,7 +42,7 @@ module Api
 
       def load_user
         @user = current_user if current_user.id == params[:user_id].to_i
-        permission_error(User, :read) unless @user
+        raise PermissionError.new(User, :read) unless @user
       end
 
       def load_user_identity

app/controllers/api/v1/users_controller.rb

@@ -15,7 +15,7 @@ module Api
         @user = User.joins(:user_teams)
                     .where('user_teams.team': current_user.teams)
                     .find_by_id(params[:id])
-        permission_error(User, :read) unless @user
+        raise PermissionError.new(User, :read) unless @user
       end
     end
   end

config/initializers/rack_attack.rb

@@ -1,9 +1,11 @@
 # frozen_string_literal: true
 
+return unless Rails.env.production?
+
 Rack::Attack.throttle('api requests by ip',
                       limit: Api.configuration.core_api_rate_limit,
                       period: 60) do |request|
-  request.ip if request.path =~ %r{^\/api\/}
+  request.ip if request.path.match?(%r{^\/api\/})
 end
 
 Rack::Attack.throttled_response = lambda do |env|