Explaining Focused Controller

Controllers in Rails are a problem. Actions sometimes become unmanageably long, and it’s often difficult to know how to test them when you need it most. At Railsberry in May, I presented a new way of writing controllers.

But the idea was young and I didn’t explain it as well as I could have. I talked too much about testing and “real OOP” (whatever that means). I didn’t talk enough about what problems this solves, with real worked examples.

In reality, making testing better is an important part of Focused Controller, but not its raison d’être. Focused Controller is about breaking action code down into more logical, reusable units.

Over the months I’ve built up plenty of real-world experience using it. This has driven me to refine the concepts and the API. I’ve now pushed version 1.0, so you can confidently adopt it in your own application. Here’s how Focused Controller works and why you should use it:

Actions do different things

The actions in your controller might all relate to the same resource, but they each have different behaviour and different needs. The index action needs to find a collection of objects to display, but the show action needs to find just one.

But actions aren’t completely separate from each other, either. Often they need to do some of the same things. show, edit, create and destroy all need to find a single record. It kinda sucks to repeat that code over and over, so it’s common to abstract it into a method with a before_filter:

before_filter :find_post, only: %w(show edit create destroy)

# some more code...

private

def find_post
  @post = Post.find params[:id]
end

What other things can differ between actions?

  • Authentication requirements
  • Cache requirements (caches_page, caches_action)
  • Layouts
  • respond_to types
  • SSL requirements
  • The list goes on…

To handle actions with different behaviour, there are a plethora of methods in Action Controller which take :only and :except options.

A different way

In Rails, actions are methods in a controller class. Methods cannot have properties of their own, which is why we need the :only and :except options.

If actions were objects, we could push this knowledge onto the object itself. We could call action.layout to find out what layout to render, action.cache_page? to find out whether to use page caching, or action.ssl_required? to find out if SSL is required.

Focused Controller makes your actions into objects by using a separate class for each action. This then makes it easy to use inheritance and mixins to share behaviour between related actions.

Better factored code

Often controllers are quite simple. “Fat models, skinny controllers” has been a catchphrase in the Rails community for some time, and whilst there are problems with fat models, it’s often a good rule of thumb.

But in most Rails apps there’s some complex controller code. Code that does not belong in the model, but that also feels wrong in a huge long action method.

One approach is to move this logic into a completely separate object. This can be valid, but it has its problems as well. For example, you’ll have to write extra code to manage the interaction between your controller and this new object.

Focused Controller is a more lightweight solution. Since you are no longer restricted to putting all of your logic into one action method, you can easily split it out into several methods within the action class. You can then unit-test the logic in any of those individual methods in a very targeted way.

An example

Have a look at the PostsController#create action from an open-source Rails forum engine:

class PostsController < Forem::ApplicationController
  before_filter :authenticate_forem_user
  before_filter :find_topic
  before_filter :block_spammers, :only => [:new, :create]

  def new
    # ...
  end

  def create
    authorize! :reply, @topic
    if @topic.locked?
      flash.alert = t("forem.post.not_created_topic_locked")
      redirect_to [@topic.forum, @topic] and return
    end
    @post = @topic.posts.build(params[:post])
    @post.user = forem_user
    if @post.save
      flash[:notice] = t("forem.post.created")
      redirect_to forum_topic_url(@topic.forum, @topic, :page => last_page)
    else
      params[:reply_to_id] = params[:post][:reply_to_id]
      flash.now.alert = t("forem.post.not_created")
      render :action => "new"
    end
  end

  private

  def find_topic
    @topic = Forem::Topic.find(params[:topic_id])
  end

  def block_spammers
    if forem_user.forem_state == "spam"
      flash[:alert] = t('forem.general.flagged_for_spam') + ' ' +
                        t('forem.general.cannot_create_post')
      redirect_to :back
    end
  end

  def last_page
    (@topic.posts.count.to_f / Forem.per_page.to_f).ceil
  end
end

Some of this complexity could be pushed into the model (the last_page method seems an obvious candidate), but a lot of it cannot.

Here’s how we could rewrite it with Focused Controller:

module PostsController
  class Action < ApplicationController
    include FocusedController::Mixin

    before_filter :authenticate_forem_user
    expose(:topic) { Forem::Topic.find params[:topic_id] }
  end

The Action class is a superclass of all the actions in PostsController. Every action needs to authenticate the user and have access to the topic.

Rather than setting up the topic in a before_filter, we use expose, which is a shortcut for:

  def topic
    if defined?(@topic)
      @topic
    else
      @topic = Forem::Topic.find params[:topic_id]
    end
  end
  helper_method :topic

The helper_method declaration means that we can call topic instead of controller.topic in the view template. Dependencies declared via expose can be easily stubbed out in a test if necessary, which I will show in a moment.

Both New and Create need to perform authorisation, block spammers, and have access to a new post, attached to the topic:

  class New < Action
    before_filter { authorize! :reply, topic }
    before_filter :block_spammers

    expose(:post) { topic.posts.build }

    def call ... end

    def block_spammers
      if forem_user.forem_state == "spam"
        flash[:alert] = t('forem.general.flagged_for_spam') + ' ' +
                          t('forem.general.cannot_create_post')
        redirect_to :back
      end
    end
  end

Create extends the behaviour of New to actually save the post back to the database. So we can just subclass New:

  class Create < New
    before_filter :ensure_topic_not_locked

    def call
      post.attributes = params[:post]
      post.user       = forem_user

      if post.save
        flash[:notice] = t("forem.post.created")
        redirect_to forum_topic_url(topic.forum, topic, :page => last_page)
      else
        params[:reply_to_id] = params[:post][:reply_to_id]
        flash.now.alert = t("forem.post.not_created")
        render :action => "new"
      end
    end

    def ensure_topic_not_locked
      if topic.locked?
        flash.alert = t("forem.post.not_created_topic_locked")
        redirect_to [topic.forum, topic]
      end
    end

    def last_page
      (topic.posts.count.to_f / Forem.per_page.to_f).ceil
    end
  end
end

The precondition to ensure a topic is not locked gets extracted. This allows the call method to be more directly focused on the logic it is trying to perform. We can test ensure_topic_not_locked directly if we wish.

We’ve made the code longer, for sure. But we’ve also split it up into more logical chunks and reduced duplication.

A quick test

Often it’s sufficient to just cover controllers with acceptance tests, but when there’s fiddly logic happening, you really need some unit tests too. Focused Controller makes that much easier.

Suppose we wanted to test Create#call when the save succeeds or fails. We’ve separated out our post dependency with expose, so it’s easy to stub out, letting us focus on the logic under test:

describe PostsController do
  include FocusedController::RSpecHelper

  describe PostsController::Create do
    before { subject.stub(post: double) }

    it "renders new if save fails" do
      subject.post.stub(save: false)
      subject.call
      response.should render_template('new')
    end
  end
end

Try it!

This is a much more enjoyable way to write controllers. It’s easy to share code and easy to jump in and write tests where necessary.

I’d love it if more people gave this a go in their own applications. If you like the idea, please do try it and let me know how you get on.

https://github.com/jonleighton/focused_controller

I am very grateful to Steve, Tekin, Murray, Paul, Jeff and Avdi for providing invaluable feedback on this article. You guys are awesome!

Comments

I'd love to hear from you here instead of on corporate social media platforms! You can also contact me privately.

Jon Leighton's avatar

Jon Leighton

If anyone is reading this and thinking "yeah but it's too verbose", I personally don't find this to be a problem most of the time. However, Avdi Grimm has proposed a DSL syntax which will probably work it's way into FC at some point. It should allow for some interesting patterns of code reuse too. See https://github.com/jonleigh... for details!

donaldball's avatar

donaldball

This is an interesting compromise solution. Myself, I've been moving towards writing discrete service objects that own the business logic, leaving my controllers focused on receiving arguments from http requests and handling the response appropriately. It works very nicely for complex business rules, but is awfully verbose for simpler CRUD. I'll definitely be giving FC a whirl.

josh_at_dailydrip's avatar

josh_at_dailydrip

You keep showing up and commenting on articles I find interesting :)

Rahul Chaudhari's avatar

Rahul Chaudhari

This is what I always think, why rails controller is not following single responsibility principle.
Well Jon, what ever your explained earlier is also superb.
Really thanks for it...

Marnen Laibow-Koser's avatar

Marnen Laibow-Koser

My initial reaction is that this is syntactically interesting but ultimately palliative: if your controller actions are long, then your controllers are doing too much and you should refactor it into the model layer. There's simply no reason to have long controller actions, so I'm uneasy with a gem whose stated purpose is to make that easier.

That said, though, now that I know that this exists, I'll be thinking about where I could use it. Thanks!

Add your comment