Psst. Hey, kid. Yeah, you. You know about around_filter? Yeah? You ever use with_scope with it? Yeah? Okay, cool. Let me give you a spoiler:
using with_scope in conjunction with a controller around_filter is evil.
Sorry. You’re about to get some vinegar in your scope. Read on.
The Scoop
There’s a pretty cool feature in Rails which is real useful but (don’t say it) often abused: with_scope. It goes a something like this:
Movie.with_scope :find => { :conditions => [ 'state = ?', Movie::NOW_PLAYING ] } do Movie.find(:all) end
Cha. Any find calls within the with_scope block will have their scope limited to movies with a state equal to the value of the Movie::NOW_PLAYING constant. That Movie.find(:all) call is basically equivalent to Movie.find(:all, :conditions => [ ‘state = ?’, Movie::NOW_PLAYING ]).
Now, here’s an idea. Let’s say you have a MovieController which is in charge of, erm, movies. How’d it look?
class MovieController < ApplicationController def director @director = Movie.find(params[:movie_id]).director end def related_movies @related_movies = Movie.find(params[:movie_id]).related_movies end end
Nice and clean. Just standard find and associations. Let’s spice it up and say we only want visible movies that are NOW_PLAYING:
class MovieController < ApplicationController def director movie = Movie.find_by_id(params[:movie_id], :conditions => [ 'state = ? AND visible = ?', Movie::NOW_PLAYING, true ]) @director = movie.director end def related_movies movie = Movie.find_by_id(params[:movie_id], :conditions => [ 'state = ? AND visible = ?', Movie::NOW_PLAYING, true ]) @related_movies = movie.related_movies end end
Here we’re conforming to the new spec by checking for some :conditions. Twice. Kinda gross, and not very DRY. Maybe we could use with_scope? No, not yet—it’d definitely be simpler to setup the movie in a before_filter.
class MovieController < ApplicationController before_filter :set_movie def director @director = @movie.director end def related_movies @related_movies = @movie.related_movies end private def set_movie @movie = Movie.find_by_id(params[:movie_id], :conditions => [ 'state = ? AND visible = ?', Movie::NOW_PLAYING, true ]) end end
Wow, much better. And now we always have the movie available as an instance variable. However, sometimes you just need more than one movie, ya know?
The List
Say I want, nay, demand a list of movies sorted by their release dates. First we’d have to change the before_filter to this:
before_filter :set_movie, :except => :all_movies
Then we’d add some sort of all_movies method, like yah:
def all_movies @movies = Movie.find(:all, :conditions => [ 'state = ? AND visible = ?', Movie::NOW_PLAYING, true ], :order => 'release_date DESC') end
Those damn :conditions again. Exactly the same as before, only this time without the elegance of a before_filter. Le sigh. Okay, bust out with_scope.
The around_filter
Rails 1.2 has a new feature called around_filter. It allows you to run code both before and after (“around”) the actions in a controller. From docs:
around_filter do |controller, action| logger.debug "before #{controller.action_name}" action.call logger.debug "after #{controller.action_name}" end
Hey… what if we just ran our controller actions in a with_scope block, scoped to NOW_PLAYING and visible movies? Then we could delete all those :conditions parameters and let the around_filter worry about it!
Let’s throw something together:
class MovieController < ApplicationController before_filter :set_movie, :except => :all_movies around_filter do |controller, action| Movie.with_scope :find => { :conditions => [ 'state = ? AND visible = ?', Movie::NOW_PLAYING, true ] } do action.call end end def director @director = @movie.director end def related_movies @related_movies = @movie.related_movies end def all_movies @movies = Movie.find(:all, :order => 'release_date DESC') end private def set_movie @movie = Movie.find_by_id(params[:movie_id]) end end
Cool. I was able to cut down all that :conditions repetition by adding in the around_filter call… but at what cost?
This. Is. Wrong.
No. Not cool.
Don’t put with_scope in an around_filter. In fact, don’t ever call Model.with_scope outside the context of Model. When I do this, I just make my code harder to comprehend. You may not think so, you may grok fine, but there is definitely a simpler way which both you and your cohorts will grok without having to always keep in mind there’s an omnipotent around_filter floating in the sky, affecting your every find.
(Also, this sort of thinking is a nasty habit in that with_scope is becoming protected in 2.0, sayeth DHH. You’ll only be able to use it in the context of a model.)
So, what’s the simpler way? Is with_scope even useful at all? Yeah.
The Rails Way
Jamis Buck (the J-bomb, as I call him) recently wrote about slimming code in Skinny Controller, Fat Model—a superb article. Following his lead, here’s a real clean way to scope your controller without around_filter:
class MovieController < ApplicationController before_filter :set_movie, :except => :all_movies def director @director = @movie.director end def related_movies @related_movies = @movie.related_movies end def all_movies @movies = Movie.find_playing(:all, :order => 'release_date DESC') end private def set_movie @movie = Movie.find_playing(:first, :conditions => [ 'id = ?', params[:movie_id] ]) end end
See it? I added a find_playing method to the Movie class. And what’s that gem of a method look like?
class Movie < ActiveRecord::Base def self.find_playing(*args) with_scope :find => { :conditions => [ 'state = ? AND visible = ?', NOW_PLAYING, true ] } do find(*args) end end
Now find_playing is delivering to me NOW_PLAYING, visible movies without fail through with_scope’s magic.
Refreshing. Less, simpler code without any side effects. You see find_playing and don’t know what it does, you look it up; you’re not working under the false assumption that find is totally normal while some tricksy with_scope is mucking around in your around_filters. And now I can write a unit test for find_playing directly, to make sure I really know what’s going on (and that I’m not programming by accident). Not bad.
Scope!
It’s okay to realize “wow, my code sucks” then frantically refactor it to be cleaner and simpler. I do it daily. In fact, if you’ve got any other tips about how to skinny up controllers while fattening up models, leave ‘em in the comments.
Thirsty for more? Here’s some bedtime reading: another of Jamis’ articles, on custom finders.
And, finally, a challenge: how would you implement find_playing_by_id? I want to Movie.find_playing_by_id(id), pass in options, etc etc. Just like a real find_by_id, but scoped like find_playing. Lemme know! Ta.
find_playing_by_id
Update! Dan Milliron offers one suggestion: with_playing.
class Movie < ActiveRecord::Base def self.with_playing with_scope :find => { :conditions => [ ‘state = ? AND visible = ?’, NOW_PLAYING, true ] } do yield end end end
Then you can, in your controller:
class MovieController < ApplicationController def director Movie.with_playing do @director = Movie.find_by_id(params[:movie_id]).director end end end
Not bad, but I do like having find_playing handle the yielding for me to keep my controllers slim. What about this:
class Movie < ActiveRecord::Base def self.find_playing(*args) with_playing do find(*args) end end def self.find_playing_by_id with_playing do find_by_id(*args) end end def self.with_playing with_scope :find => { :conditions => [ ‘state = ? AND visible = ?’, NOW_PLAYING, true ] } do yield end end end
That works, but if I do Dan’s with_playing in the controller I get access to all the find_by_* methods—not just ones I hardcode. That’s cool, and I want that in my model. So let’s add some method_missing into my find_playing lifestyle:
def self.method_missing(method, *args, &block) if method.to_s =~ /^find_(all_)?playing_by/ with_playing do super(method.to_s.sub('playing_', ''), *args, &block) end else super(method, *args, &block) end end
This will give us find_playing_by_blah and find_all_playing_by_jlah. Tricky.
Ooh ooh! Alias method_missing! Hey! Pick me! Hey, hey!!
Great article! You’ve just saved my soul! :)
What if all the find calls has a condition user_id = session[:user].id ? That is every model except user has belongs_to :user.
use a @current_user instance var, which gets defined in your ApplicationController. Then you can use e.g:
@current_user.movies @current_user.friends
I prefer to put all uses of “with_scope” into models and never into controllers. For the use-case in the article, I would define:
class Movie < ActiveRecord::Base def self.with_playing with_scope :find => { :conditions => [ ‘state = ? AND visible = ?’, NOW_PLAYING, true ] } do yield end end end
Then I would use it in a controller like this:
class MovieController < ApplicationController def director Movie.with_playing do @director = Movie.find(params[:movie_id]).director end end end
Hey, put the call to with_playing in your around_filter!!
Nooooooooooooooooooooooo!
Thanks for the clear writeup. with_scope is making sense to me now.
excellent post.
I’m curious about Manu’s question, too. If all my models “belongs_to :user”, how can I use “with_scope” and “current_user” to insure that I only get get models where “[user_id = ?, current_user.id]”?
I just don’t see a way to do that without passing “current_user” to all the find methods. Perhaps that’s more appropriate anyway…
MrChurcho: You could do something like what this post by koz describes. I’m not sure how I feel about it though. Give me time to think.
Oh…this is sweeeett. Thanks!
In the interest of keep my C and V slim and putting the fat in my M, I’m trying to come up with a solution for using custom-build finds within an association. Using your example, say you wanted to look at a list of directors and then each of the movies they’ve done – pretty simple, you can Director.find(:all, :include => :movies). So how about if you want to put date restrictions on this list. Say you already have a method like:
def Movie.find_between_dates(start, end) find :all, :conditions => [‘release_date BETWEEN ? AND ?’, start, end] end
Now if I have one director, I can go: some_director.find_between_dates(5.months.ago, 3.months.ago) What can I do with the list of all directors? The easy thing would be a controller that does: @directors = Director.find :all and a view that iterates through @directors and uses director.find_between_dates() (which would be a whole bunch of DB queries – even if you use :include => :movies in the original find). Or alternatively, you could do: @directors = Director.find :all, :include => :movies, :conditions => [‘movies.release_date BETWEEN ? AND ?’, start, end] # start and end would probably be params or something but then I’ve just wasted the custom find I built into my Movie model. This example it doesn’t make much difference, but I have a more complicated real-life situation and I’d really like to keep this finder in my model. The best I can come up with would be to duplicate the Movie.find_between_dates functionality inside Director, as an object method (e.g., movies_between_dates) that either performs the find (defeating eager loading) or uses a non-AR ruby select.
My first reaction to reading this post was quite positive: “Here are some techniques to help move more logic to the model†which is high on my list of objectives. But I was dismayed to see the trendy fervor building up to hobble with_scope (mostly on other sites). Permit me to outline some excellent uses for with_scope in a controller….
First and most topically is the issue of nested RESTful resources. Imagine a MessagesController that deals with Messages. Messages can be viewed as a resource in their own right and RESTfully represented as http://somesite/messages. But they can also be considered child resources of two other resources, dogs and cats, and consequently messages can also be represented as http://somesite/dog/1/messages or http://somesite/cat/26/messages. In each action of my MessagesController, I need to check for the parameter ‘dog_id’ or ‘cat_id’ (or none) and ensure that proper restrictions are in place for every model query. I could perform this test in every action and call a custom model method that implements the appropriate scope. Or I could send a programatically constructed method name. But duplicating the test/construction in every action is not very DRY (my controller gets complicated with cats and dogs before I even start thinking about Messages) and my Message model now has some complex method_missing code, or unDRY method definitions. And please don’t suggest I pass a parameter to a custom finder -the M-C coupling will hurt badly when my second scenario is considered. I would argue that the decoupled nature of with_scope in an around filter suits the explicit scoping implied by nested resources very nicely. It would be slick as snot if a REST resource definition could also support automatically defining a model scope for that resource. Something like
would automatically Message.with_scope(cat=cat_id) within the resource controller. OK, so this might be going a bit too far…
Secondly, with_scope has the nifty ability to merge independently defined scopes. This allows for a very clean decoupling of orthogonal concepts that would otherwise result in a multitude of custom finders with garishly coupled names. If I need need restrictions for authorization (get authorized data), and I need restrictions for currency (get current data) and I need restrictions for RESTful nesting (get data belonging to parent resource), and I need restrictions for active data (get data that has not been logically deleted) there might be hundreds of different custom model finders needed. Obviously method_missing could DRY that up quickly, but the method constructor would be horrendously complicated that could build Message.find_active_owner-authorized_current_dog and Message.find_deleted_view-authorized_old_cat. A better solution would be to decouple the overarching restrictions of authorization and explicit parent resources into around_filters leaving a custom finder to deal with currency, logical deletion and, of course, run-time user-defined filter conditions. Now my method_missing only has to build Message.find_active_current and Message.find_deleted_old.
To summarize, with_scope in a controller has two excellent applications: 1. The controller’s entry points are explicitly or consistently scoped, such as with nested RESTful resource controllers, and in many cases, authorization. 2. Numerous orthogonal restrictions that need to be combined, such as (logical deletion) AND (authorizations) AND (belongs_to_parent). In these cases, I believe controller-defined with_scope is The Right Stuff.
Chris, I think your analysis is right on. I was running into exactly those problems with cross-cutting concerns. And I really like your suggestion for enhancing the resource routing to automatically add the scoping!
I hope someone is listening!
For a plugin to easily use this in your models check out scope out – I blogged about the advantages
I’m trying to use with_scope to implement private records (e.g., del.icio.us bookmarks where some bookmarks can only be seen by the logged-in-user who created them). It seems like the benefit of using with_scope with around_filter or some other controller magic is so that you don’t have to remember to use with_playing all the time; you can use the normal find() methods and you will be guaranteed that you will only find the public records that this user is allowed to see.
The case I’m worried about is when the other developers on my project start adding new views, and they fail to obey the “always use with_playing” rule to find records. This solution seems to require a fair amount of code hygiene to ensure that people never see records they aren’t permitted to see.
“using with_scope in conjunction with a controller around_filter is evil.” => I do not totally agree with you. You are right that the code is harder to comprehend. But it’s the only way that I have seen to force scope on relation ships.
For instance, if you want to strictly forbid access some objects to some profile, there is (sadly) no other way, ATM. For instance, the around_filter with_scope trick allows to reduce what is returned by a : @person.dogs . Only dogs that the connected user can see are returned, not all dogs attached to @person.
Confidentiality can be necessary on some case, and that’s the only way I ‘ve found to ensure a total encapsulation between different profile and different user.
This is an excellent post!
I made a post on my blog describing a way to override the default find method for a model, in case you always want your find method using a scope.
Chime in.