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
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
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
You keep showing up and commenting on articles I find interesting :)
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
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