Fat controllers in rails

Question

I started working through the tutorial at railstutorial.org to get exposure to the framework. My controllers aren't monstrous yet, but I can see that Single Responsibility Principal(SRP) isn't being applied throughout the tutorial, as it extends beyond the scope of the tutorial.

I've got this relatively simple controller here. I can already see different concerns (e.g. authentication and authorization) leaking into this controller, which contains too many actions to begin with. This assigns too many actions to one controller. I stumbled upon rails focused controllers which solves one of these issues and looks quite interesting.

Is this a common solution? Or are better solutions available?

In the .net world, we tend to use aspect oriented programming(AOP) to achieve a cleaner Separation of Concerns (SoC). However, recently a few guys have written a new front controller framework called Fubu Behaviours. It captures the idea of a request pipeline nicely. Something that makes more and more sense to me.

In order to handle a request we tend to go through a few steps before (and sometimes after) the action is executed. In some cases, conditionally ending the request. It seems natural to use something like behaviors, a pipeline or a russian dolls pattern. So that each link in the chain is responsible for either continuation or cessation. Inheritance doesn't seem to be the best solution.

Is there anything similar to that in rails? Would it make sense in rails?

Recommended readings will be just as well welcome!

Answer 1

I agree with you that having those authorization functions like is_admin and correct_user are a bit of a code smell. I would remove them and handle it a bit better with a gem I often use called CanCan.

It allows you to move all authorization rules out of your controllers into a manifest (i.e. the Ability model), only requiring your controllers to initiate the authorization check through authorize_resource call in your controller. Then you can handle simple redirects in your ApplicationController:

class ApplicationController < ActionController::Base
  rescue_from CanCan::AccessDenied do |exception|
    if current_user
      redirect_to signin_url, :alert => exception.message
    else
      redirect_to root_path, :alert => exception.message
    end
  end
end

Other than that, I would move all the @user = User.find(params[:id]) calls into a before_filter, and clean up your indenting and your action ordering (should be index,new, create, show, edit, update, destroy) and I think your controller would be nice and skinny.

Answer 2

To be honest, I can't really tell if authorization/authentication should be a model's or a controller's job and people will give you various answers on this. It's like a grey area.

So I prefer to go with the Rails conventions there, as they have been proven trustworthy for all these years. Your controllers seem fine to me, I wouldn't call them fat. You could ofcourse move these private methods into a helper.

Answer 3

For any sort of authorization logic, I would actually remove that from the controller layer, and move that to a 'policy' layer.

using this gem: https://github.com/NullVoxPopuli/skinny_controllers it gives you two additional layers

• operations
  • where business logic goes
• policies
  • where authorization goes

An example from the Readme:

module EventOperations
  class Read < SkinnyControllers::Operation::Base
    def run
      # the business logic here is to only check if we will allow this model
      # to be returned
      model if allowed?
    end
  end
end



class EventPolicy < SkinnyControllers::Policy::Base
  # allowed? from the operation delegates to this method
  # here, you could do whatever logic you need to check if the operation
  # is allowed
  def read?(o = object)
    o.is_accessible_to?(user)
  end
end