If you’ve done much Rails coding, you’ve probably heard the guideline: “skinny controller, fat model”. But achieving this can be easier said than done. Especially when your controllers are bloated with HTTP-centric responsibilities, such as websocket notifications, that don’t really seem to belong in your domain model objects either.
In this screencast and article, you’ll learn a refactoring technique that puts your controllers on a diet, without shoehorning a bunch of web server-centric code into your database-backed model classes.
Start with a single controller action
Let’s take a look at a typical Rails controller action. This action is from a project management application.
class TasksController < ApplicationController
# ...
def update
old_project_id = @task.project && @task.project.id
previous_status = @task.status
if @task.update_attributes(params[:task])
if previous_status != @task.status
notifiee = task.readers(false) - [current_user]
if notifiee
mail_completion_notice(notifiee) if new_status == Status::COMPLETED
mail_uncomplete_notice(notifiee) if previous_status == Status::COMPLETED
end
end
if old_project_id != (@task.project && @task.project.id)
push_project_update(old_project_id)
push_project_update(@task.project.id) if @task.project
end
if @task.project
push_task('update_task')
else
push_task('create_task', assignee) if assignee
push_task('delete_task', previous_assignee) if previous_assignee
update_users = @task.readers
update_users = update_users - [assignee] if assignee
push_task('update_task', update_users)
end
mail_assignment if assignee
mail_assignment_removal(previous_assignee) if previous_assignee
#respond_with defaults to a blank response, we need the object sent back so that the id can be read
respond_to do |format|
format.json {render 'show', status: :accepted}
end
else
respond_with @task do |format|
format.json {render @task.errors.messages, status: :unprocessable_entity}
end
end
end
# ...
end
In this action we’re updating a task in a project. First, we make note of the current values of a few of the task’s attributes, for later use. We then try to update the task with the given parameters. When this is successful, we take several actions:
- If the task’s status has changed, e.g. from “in progress” to “complete”, we mail out notifications to interested users.
- If the task has been moved to another project, we push live updates out to users, using websockets or some other form of asynchronous browser notification.
- We then push out one or more task-related notifications, letting interested users know that the task has been created, updated, or deleted.
- If the person the task is assigned to has changed, we send out email notifications to let the new and previous assignees know about the change.
This clearly qualifies as a fat controller action. But when we try to put it on a diet, by moving logic into the model, we run into a problem: it turns out that all of activities this action performs are dependent on session-specific knowledge. Methods like #push_task
and #mail_assignment
need to know things like who the current user is, or what the browser’s asynchronous socket ID is. We don’t want to push that kind of knowledge down into domain model code.
Identify implicit domain lifecycle events
Looking over this action again, we realize something: hidden in all these conditionals is a series of domain-model lifecycle events, each with different concrete actions which trigger on those events.
- There are some actions to perform anytime the task is successfully updated.
- There are actions to take when the task has moved from one project to another.
- There are actions to perform when the task is newly created.
- There are actions that happen when the task’s status has changed.
- There are actions for when the task has been reassigned.
Make the events observable
It’s the controller’s job to know things like the ID of the current user, and how to push out a notification to their browser. But it’s really the model’s job to know when events occur in its lifecycle.
With that in mind, we set out to give the Task
model the ability to notify interested objects when these events occur. We give it a method called #add_listeners
, which adds an interested object to an internal list.
class Task < ActiveRecord::Base
# ...
def add_listener(listener)
(@listeners ||= []) << listener
end
# ...
end
We add another method, #notify_listeners
, which loops through the listener list…
def notify_listeners(event_name, *args)
@listeners && @listeners.each do |listener|
# ...
end
end
…and sends a message to each one, named for a specific event, such as :on_create
or :on_status_changed
.
if listener.respond_to?(event_name)
listener.public_send(event_name, self, *args)
end
Then we add an ActiveModel “around” callback which will call a method called #notify_on_save
whenever a Task
is saved.
around_save :notify_on_save
Finally, we implement the #notify_on_save
callback method.
def notify_on_save
# ...
end
The first part of the method makes various checks to determine what kind of save this is. That is, whether the task is being created or updated, whether it is moving from one project to another, and so on. We make heavy use of ActiveModel
‘s “dirty attributes” features here, using methods like project_id_changed?
to see if the project_id
has been changed from its database value.
is_create_save = !persisted?
project_changed = project_id_changed?
status_changed = status_changed?
assignee_changed = assignee_id_changed?
old_project_id = project_id_was
old_status = status_was
old_assignee = User.find_by_id(assignee_id_was)
# ...
Then it yields
to its caller. Because this is used as an “around” callback, this yield is the point at which the actual save occurs.
yield
After the task is saved, this method proceeds to send various notifications to any listeners that have signed up. If the task is newly created, the listeners will receive the #on_create
message:
if is_create_save
notify_listeners(:on_create)
If the status has changed they will receive #on_status_change
, and so forth.
else
notify_listeners(:on_project_change, old_project_id, project_id) if project_changed
notify_listeners(:on_status_change, old_status, status) if status_changed
notify_listeners(:on_assignment_change, old_assignee, assignee) if assignee_changed
new_assignee = assignee if assignee_changed
notify_listeners(:on_update, new_assignee)
end
end
Some of these messages also have some extra arguments to go with them; for instance, in the case where the task is moved to a new project, the notification provides both the original project and the new project.
Make the controller observe the model
Now that we’ve made our model observable, (to use Gang of Four terminology), we turn our attention back to the #update
controller action. We decide, in the interest of taking small steps, to simply make the controller itself a Task
listener for now. So before anything else, we add self
to the @task
‘s list of listeners.
class TasksController < ApplicationController
def update
@task.add_listener self
# ...
end
The original fat if-else-end becomes a slim if-else-end.
if @task.update_attributes(params[:task])
respond_to do |format|
format.json {render 'show', status: :accepted}
end
else
respond_with @task do |format|
format.json {render @task.errors.messages, status: :unprocessable_entity}
end
end
We then proceed down through the method, pulling code out into methods named for Task
lifecycle events.
Such as when the task is newly created…
def on_create(task)
push_task('create_task')
push_project_update(task.project.id) if task.project
mail_assignment if @assignee && @assignee != current_user
end
…where the task is moved to a new project…
def on_project_change(task, previous_project_id, new_project_id)
push_project_update(previous_project_id)
push_project_update(new_project_id) if new_project_id
end
…when the task’s status has changed…
def on_status_change(task, previous_status, new_status)
notifiee = task.readers(false) - [current_user]
if notifiee
mail_completion_notice(notifiee) if new_status == Status::COMPLETED
mail_uncomplete_notice(notifiee) if previous_status == Status::COMPLETED
end
end
…when the task has been reassigned…
def on_assignment_change(task, previous_assignee, new_assignee)
push_task('create_task', new_assignee) if new_assignee
mail_assignment if new_assignee
push_task('delete_task', previous_assignee) if previous_assignee
mail_assignment_removal(previous_assignee) if previous_assignee
end
…and when the task is updated.
def on_update(task, new_assignee)
update_users = task.readers - [new_assignee] if task.project
push_task('update_task', update_users)
end
Extract dedicated listener classes
Once we satisfy ourselves that the controller still works the same way it did before this refactoring, we begin to tease these methods apart further, breaking them up into new “listener” classes that each correspond to a specific aspect of the controller’s former responsibilities. For instance, here’s a Task
listener which only handles the browser push notifications, and not email notifications.
class PusherTaskListener
def initialize(socket_id, queue=QC, worker=PusherWorker)
@socket_id = socket_id
@worker = worker
@queue = queue
end
def on_create(task)
push_task(task, 'create_task')
push_project_update(task.project.id) if task.project
end
def on_project_change(task, previous_project_id, new_project_id)
push_project_update(previous_project_id) if previous_project_id
push_project_update(new_project_id) if new_project_id
end
def on_assignment_change(task, previous_assignee, new_assignee)
push_task(task, 'create_task', new_assignee) if new_assignee
push_task(task, 'delete_task', previous_assignee) if previous_assignee
end
def on_update(task, new_assignee)
update_users = task.readers - [new_assignee] if task.project
push_task(task, 'update_task', update_users)
end
# ...details of how notifications are pushed omitted...
end
Wire up the listener classes to the model(s)
Back in the controller, instead of passing self
to #add_listener
, we can now add a series of listener objects, one concerned with pushing browser notifications, another with sending emails. Each of these objects encapsulates the details of how its particular mode of communication is implemented.
class TasksController < ApplicationController
def update
@task.add_listener TaskPusherListener.new(@socket_id)
@task.add_listener TaskEmailListener.new(current_user)
# ...
end
# ...
end
Aside: What about Rails Observers?
If you’ve been working with Rails for a while, you might be wondering how this approach differs from Rails 3-era Observers.
When you create a Rails ActiveRecord Observer, it is automatically wired into every invocation of every model action to which it applies. This results in “spooky action at a distance“, where invoking a model method causes code in other classes to be unexpectedly executed, often with surprising and unwanted results. Over the years Rails programmers have found that these kind of implicit observers cause so many problems that many project style guides have banned their use, and the library was eventually removed from Rails.
By contrast, the kind of observability we’ve added here is completely explicit and opt-in. We add our listeners at the beginning of the controller action we want them to observe. They stick around for the length of that one method invocation, and then they are gone. Anyone reading the controller code can see which listeners are added to which model objects, and can know to expect callbacks into those listeners.
An added bonus is that, since we create the observers in the controller action, we’re able to explicitly pass the session-specific information and collaborator objects that the observers need as constructor arguments. Instead of having to sneak that information into the observers by less obvious means.
Conclusion: Skinny controller, model, and observers
We’ve now divided a fat controller into three distinct areas of knowledge:
- Information about the current session and request, including user ID and parameters. The controller takes responsibility for this.
- The events that may occur in a
Task
‘s lifetime. TheTask
model is now responsible for this knowledge. - Who should be notified about various
Task
lifecycle events, and how they should be notified. Various medium-specific listener classes encapsulate this knowledge.
We’ve looked at this in the context of a Rails controller action, but it’s really a technique that’s applicable to any kind application, web-based or otherwise. Splitting logic into events and observers is a fundamental technique for untangling domain-model and user-interface responsibilities.
Thank you to OpusWorkspace for allowing me to use this real-world code example.
Happy hacking!
Get the source code
Want to see the full source code for this article, including original, midpoint, and final states? [thrive_2step id=’11752′]Click here to download![/thrive_2step]
What do you feel are the pros and cons of rolling your own event observers like this versus the original built-in Rails Observers (now a gem at https://github.com/rails/rails-observers )?
The final outcome seems very similar, although maybe a *bit* less magic since you explicitly write out calls to named events—with Rails Observers you respond to the top level before_save, before_create, etc. callbacks and then flesh out your specific implementation from that.
Did you read the sidebar I wrote about that? 🙂
Thanks so much for this ( and all of your other work)!
I posted an alternate approach and would love your take.
https://ecommerceonrails.com/2017/04/13/an-alternate-approach-to-avdis-method-of-slimming-down-rails-controllers/
I’m glad to see that the idea to extract observable events is gaining traction.
Several years ago I published a gem Ventable, — https://github.com/kigster/ventable — and a corresponding blog post on the subject. Check this out — while the approach is somewhat similar, the execution is not Rails specific and has some additional advantages:
http://kig.re/2013/08/05/detangling-business-logic-in-rails-apps-with-poro-events-and-observers.html
Thanks for the post! It was great to see a real world implementation of this while refactoring code. Have you ever used the wisper gem (https://github.com/krisleech/wisper)? If so, what are you thoughts on it?
Thanks, Avdi, for showing that interesting approach. One question on it. How would you suggest to add listeners for new records (like in #create action)? Should new record be initialized first to have an ability to add listeners?
Yes, exactly. It’s important not to think of this in terms of Rails observer conventions. By definition, any model listeners are interested in events that the model can tell them about, so it doesn’t make any sense to have a listener without a model to listen to.
This may be a dumb question, but I’ll ask it anyway. Your TasksController#update action passes @socket_id to the TasksPusherListener. Where does @socket_id come from?
Mark, to be honest it’s been long enough since I encountered the original code that I don’t recall the details. However, I believe the @socket_id was related to a WebSocket library.
Note that this code sample is nowhere near complete.
Why do you loop through the listeners like this “@listeners && @listeners.each”?
How come you can’t just do “@listeners.each”?
That may have been an artifact of the source material, at this remove I’m not sure.
—