Close
Glad You're Ready. Let's Get Started!

Let us know how we can contact you.

Thank you!

We'll respond shortly.

LABS
Rails 4: Testing strong parameters

UPDATE: Thanks to fellow Pivots Alex Kwiatkowski and Rick Reilly, we found that inheriting from ActionController::Parameters didn’t work for update_attribtues. Alex explains some of the changes they made. In the mean time, check out my repo for the example, including a commit for the failing test and the fix.

Since Rails 4 has been released we’ve been getting involved and learning about what has changed. One of the changes most discussed is strong parameters and I wanted to explore some ideas about how to test this new feature. Strong parameters are a way of white listing HTTP query parameters and moves the burden of whitelisting from the ActiveModel/ActiveRecord classes and into the controllers. I think this feels like a better place for this to happen.

Most examples I’ve seen encourage the following:


class UsersController < ApplicationController

  def create
    User.create(user_params)
  end

  private
  def user_params
    params.require(:user).permit(:name)
  end
end

However I think this encourages asking the question of how defensive to test the strong parameters portion of this. Should you do the defensive code for each action or is one enough?


describe "#create" do
    it 'creates a user' do
      User.should_receive(:create).
        with({name: 'Sideshow Bob'}.with_indifferent_access)
      post :create, user:
        { first_name: 'Sideshow', last_name: 'Bob', name: 'Sideshow Bob' }
    end
end

There are a few ways to extract the parameterisation whilst making it testable but the one which feels most natural for Rails is to extend the ActionController::Parameters class where it can be tested independently of the controller actions.

My first attempt lead me one way but I eventually ended up extending the ActionController::Parameters, but as Alex pointed out this doesn't work for update_attributes


class UsersController < ApplicationController

  class UserParams < ActionController::Parameters
    def initialize params
      filtered_params = params.
        require(:user).
        permit(:name)
      super(filtered_params)
    end
  end

  def create
    User.create(UserParams.new(params))
  end

end

I went back to a previous thought and made the params call a class method.


class UsersController < ApplicationController

  class UserParams
    def build params
      params.require(:user).permit(:name)
    end
  end

  def create
    User.create(UserParams.build(params))
  end

end

Now the strong parameter aspects can be tested independently


  describe UsersController::UserParams do
    it 'cleans the params' do
      params = ActionController::Parameters.new(
        user: {foo: 'bar', name: 'baz'})
      user_params = UsersController::UserParams.build(params)
      expect(user_params).to eq({name: 'baz'}.with_indifferent_access)
    end
  end

The controller can then choose it's interaction with the UsersController::UserParams class through strong (which requires knowing the controller and action) or weak stubbing:


describe "#create" do
    let(:http_params) do
      { user:
        { first_name: 'Sideshow', last_name: 'Bob', name: 'Sideshow Bob' }}
    end

    let(:model_params) { double(:model_params) }

    before do
      UsersController::UserParams.stub(:build) { model_params }
    end

    it 'creates a user with strong stubbing' do
      UsersController::UserParams.stub(:build).
        with(
          http_params.merge(controller: 'users', action: 'create').
            with_indifferent_access) { model_params }
      User.should_receive(:create).with(model_params)
      post :create, http_params
    end

    it 'creates a user with weak stubbing' do
      User.should_receive(:create).with(model_params)
      post :create, http_params
    end
  end

This now allows the tests to focus on the business logic of the actions rather than worrying about the defensive coding of strong parameters and also gives the developer confidence that the strong parameters are adequately tested.

The evolution of this can be seen in this repository, with thanks to Bryan Helmkamp and Alex Kwiatkowski.

Comments
  1. Daniel Finnie says:

    For better or worse, most of the projects I’ve seen with Strong Params rely on a feature test that creates a valid object and then visits the show page for the object and ensures that the values came through. This allows your feature test to draw out the fact that you need to allow some params, but doesn’t really test that you whitelist the right params.

    In your example above, it tests that you don’t simply defeat the purpose of strong params by calling permit! in the controller [1], but it doesn’t detect if you were overly permissive — for instance, allowing through :admin or :roles or :account_is_paid or something. If I were writing a model with fields that had role-based access, I would probably specifically test that those fields couldn’t get through.

    All in all, I think strong params blurs the line betweeen code and configuration. In all cases I would add a test to specifically make sure that unpriviledged users cannot overwrite priviledged fields. In projects where the mentality is to not test configuration, then I think the feature test method to ensure that strong params was somehow setup seems sufficient.

    1: https://github.com/robb1e/strong-params-example/blob/master/app/controllers/users_controller.rb#L7

  2. Robbie Clutton says:

    If you have a feature test to drive this level of detail out you’re sure to be on the road to slow tests. By all means use outside-in testing to help the design of your application but keeping these tests around can be detrimental in the long run. Previously with whitelisting in the model it was awkward to test but now with strong parameters it’s become much easier. I think it’d also be appropriate to pass a user instance into this class and ask for the parameters that the user can mutate and once again take the responsibility out of the controller and into the params class.

  3. Gary Weaver says:

    Great post and thanks for sharing, but having to write 22-26 lines of test code for basically 2 lines of real code is a little extravagant. Maybe a more generic solution in a minitest/rspec extension to come closer to a 1:1 ratio between test code and the code it is testing would be a good idea?

  4. Robbie Clutton says:

    @Gary, would like to see a version of how you would test this, the rspec user in me naturally structures tests in this way.

  5. Joe says:

    Why not just use @ variables for the params and use assigns(:var) in the spec?

  6. Robbie Clutton says:

    @Joe, you could do but if you were doing something with a redirect you’d be add the instance variable without any use but for the test. If you’re ok with that, then yes, it’s a good way forward.

  7. Clay Shentrup says:

    My view is, this is way too complex. I advise a simpler solution, which includes abstaining from using require() at all.

    http://clayshentrup.blogspot.com/2013/12/testing-strong-parameters.html

  8. Robbie Clutton says:

    Hi Clay,

    This post is an exploration is trying to achieve single responsibility and removing redundancy of valid and invalid attributes across actions.

    To be honest, it’s not something that I’ve used a great deal. As you said, it’s not the simplest thing to do and can add some extra weight.

    Please bare in mind this was written when Rails 4 was newly released and this was blogging about what I discovered with it.

    Robbie

  9. Dan Draper says:

    I actually like this approach but you have an error in the last example. You call build on UserParams as a class method but it was defined as an instance method.

  10. Javid Jamae says:

    Thanks for writing the article. I like that you’re separating out the params logic for better testability, but nested classes in Ruby are often a code smell to me. Coming from a Java background, using a nested class seems like a very Java-esque way of doing things. If UserParams can be used independently across multiple controller, then pull it out as a separate top-level class. Otherwise, if you’re just looking for more code modularity and testability, why not take advantage of mixins or ActiveSupport::Concern’s? You can still test those independently (by mixing them into your test), but you’re not adding an unnecessary construct that serves no purpose outside of the class itself.

  11. Arthur Santos says:

    It’s an old post, but it’s still one of the few to discuss how to test this aspect of the application.

    Are you currently using this approach in your apps? I’m tempted to do something like this to avoid duplication of logic, since I have an admin controller and an API controller to manage one resource and both have the same permitted attributes.

Post a Comment

Your Information (Name required. Email address will not be displayed with comment.)

* Copy This Password *

* Type Or Paste Password Here *