Rspec notes from the trenches-2

Courtenay : June 18th, 2007

In followup to the previous article, another way I'm using rspec. This is a variation on the "fat model" idea, but I'm pushing it a little further.

In controllers, we frequently see code like this:

def index
  @people = Person.find(:all, :order => 'id desc', :conditions => ['activated=?', true])
end

def show
  @person = Person.find(:first, :conditions => ['id = ? and activated = ?', params[:id], true])
end

Now, to my eye, that looks like we're leaking commands from the database (which should be hidden away under the model). I even think that in many cases, the 'find' call should be a protected method to the model.

Exposing find and its parameters allows the coder to make database calls from the controller, which makes the spec incredibly brittle and rigid.

def index
  @people = Person.find_all_activated
end

def show
  @person = Person.find_activated( params[:id] )
end

The model will look something like:

def find_activated( person_id )
  find(:first, :conditions => ['id = ? and activated = ?', person_id, true]
end

We can now stub out the method happily. Also, this interface -- User.find_activated -- will never need to change.

it "should render show" do
  User.should_receive(:find_activated).and_return mock_model(User)
  get :show
  response.should be_success
end

You can also make that return a User.new object or whatever mock you want.

Posted by Courtenay Filed in rails, sql, testing

6 Responses to “Rspec notes from the trenches-2”

  1. Dr Nic Says:
    June 19th, 2007 at 12:41 AM
    I even think that in many cases, the ‘find’ call should be a protected method to the model.

    I think this is an interesting premise. Difficult to justify for all use-cases of #find, but just reading it above makes me rethink all the lengthy #find requests I have sitting around in controllers.

  2. Jacob Atzen Says:
    June 19th, 2007 at 01:47 AM

    To me it makes good sense not having complex find calls in the controller and using well named, intention revealing methodnames instead. OTOH I find it a bit messy to burden the model with methods which might only be used by a single action. In an ideal world the model would encapsulate the logic to ensure it is in a consistent state and provide methods for access usable by many parties and nothing more.

    An alternative to stuffing your finders in the model is to use private controller methods. Then you get nice simple calls in the controller action and keep the model clean of finder logic. How well this will play along with mocking or stubbing I don’t know.

    Perhaps one could also group finders into modules and include these in the relevant controllers. This adds extra complexity but keeps the model and controller code clean of finder logic. Any thoughts on this?

  3. court3nay Says:
    June 19th, 2007 at 02:33 AM

    actually you’re right - I do this too. Something like

    class ProductsController < ApplicationController
  before_filter :load_product, :only => [ :show, :edit, :update, &#8230; ]
private
  def load_product
    unless @product = account.products.find_by_permalink(params[:id])  
    flash[:notice] = &#8220;Could not find that product&#8221;
    redirect_to :action => :index
    return false
  end
end

    Then the spec can do something like

    controller.stub!(:load_product)
controller.instance_variable_set(:@product, mock_product)

    .. or thereabouts.

  4. Brian Says:
    June 26th, 2007 at 10:48 AM

    Hey Courtenay, thanks for the good posts. I have been using rspec ever since your sample app, so thanks a ton. BTW, it looks like you have upgraded to rspec 1.x now. Will you be updating the sample app to be working with the now current API? Thanks again.

  5. Alex G Says:
    June 27th, 2007 at 02:24 PM

    I apologize for off topic post, but looked everywhere and don’t see your contact info. Please delete this post after you have read it.

    I’m trying to get in touch with somebody from Caboose in regards to integrating updated docs into my API site (http://www.noobkit.com). I would love to get a patch/diff/zip of the source code. Please email me ASAP.

    Thank you.

  6. Adi Azar Says:
    July 8th, 2007 at 11:23 PM

    Cool post. Thanks for nice and useful information.

Sorry, comments are closed for this article.