Encapsulation and Global State in Rails

Rails makes many of our day to day tasks much easier, but sometimes this ease comes at a cost. Consistently across applications we see a lack of encapsulation and the shared global state of controllers and views to be one of the biggest sources of complexity and bugs. In this episode we dive into the causes, and solutions to this complexity in hopes of building easier to maintain Rails apps.

Rails and Encapsulation

In each of controllers, views, and helpers we have access to each of session, params, flash, cookies, as well as any instance variables set in the controllers.

To expand on the instance variable point, while we are only running a single action in a single controller at a given time, it turns out that at a minimum we also pick up any instance variables defined in the ApplicationController as well as we're inheriting from it (and possibly others). When we couple this with implicit methods invoked via before_filters, the complexity can grow very quickly.

The Gravity of the Problem

When we start out on new applications, these sorts of coupling may seem like nothing to worry much over. It's very easy to justify one or two special cases where params are accessed in views or helpers, or an instance variable is accessed in a partial. The danger is that these sorts of state leakages tend to compound, and tend to do so quickly, causing a few special cases to suddenly be the norm and pervasive.

Further, it's worth noting that this sort of coupling is very resistant to refactoring and often when an application has become entangled in these ways, the most common reality is that it will stay that way and the team will just come to accept the complexity and difficulty in making changes as facts of life. This is certainly a case where an ounce of prevention is worth a pound of cure!

Examples of Coupling

Assigning Instance Variables in before_filter

A common example of introducing implicit global state is the use of a before_filter in ApplicationController to assign an instance variable.

class ApplicationController < ActionController::Base
  include Authentication
  protect_from_forgery with: :exception

  before_filter :find_organization

  def find_organization
    if current_user
      @organization = current_user.organization
    end
  end
end

In this application, a user is part of an organization, so logically we might want to always have the @organization instance variable set to allow us to display the org name in the header, and having this automated via a before_filter lookup seems might nice and efficient.

Unfortunately this can quickly lead to problems if we have a concrete controller, for instance OrganizationsController which ends up overriding the @organization instance variable. It might be easy to forget about the @organization instance variable as it is automatically assigned to in the ApplicationController, out of sight, out of mind. This can quickly lead to displaying the wrong data to the user, or much more seriously, allowing users to act on organizations they are not part of.

Extract to a Helper

One step we can take to help clean this up is to make the user's organization lookup exposed via a helper method, rather than as an instance variable. This makes it a bit more explicit, and protects from overriding via the child controllers.

class ApplicationController < ActionController::Base
  # ...

  def current_user_organization
    if current_user
      @_current_user_organization ||= current_user.organization
    end
  end
  helper_method :current_user_organization
end

Pass Locals into Partials

In the initial implementation, the _header partial used the @organization instance variable directly. This is almost always something we want to avoid as it couples our partial to the implicit global state of the controller the current action was rendered through, and it makes it harder to understand what value the @organization might have when rendering the partial.

Instead, we almost always want to explicitly render a partial by passing in any objects we need as locals:

# app/views/layouts/application.html.erb
-  <%= render "header" %>
+  <%= render "header", user: current_user, organization: current_user_organization %>

# app/views/application/_header.html.erb
-  Welcome, <%= current_user.name %> from <%= @organization.name %>
+  Welcome, <%= user.name %> from <%= organization.name %>

Moving Towards Functional Views

Here at thoughtbot we've recently been exploring a number of alternative frameworks for building web applications including [Elixir+Phoenix][], [React][], [Elm][], end even [more recent versions of Ember][], and a common theme is a move towards a more functional approach to defining UIs.

When a view is pulling from any number of objects provided by implicit global state (session, flash, instance variables, etc.), it becomes increasingly difficult to look at that view in isolation and understand how it will behave. That said, if we can transition to only rendering our view with data explicitly passed in (as locals in the case of Rails partials), then that view becomes very easy to understand and test. Big wins for long term maintainability!

Taking this to the logical extreme in Rails, we currently have a pull request open in the [thoughtbot guides repo][] that suggests passing in all data via locals even when rendering top level views: [Advocate using locals to render views][].

[Advocate using locals to render views]: https://github.com/thoughtbot/guides/pull/398 [Elixir+Phoenix]: https://upcase.com/videos/intro-to-elixir [React]: https://upcase.com/videos/intro-to-react [more recent versions of Ember]: http://emberjs.com/blog/2015/08/13/ember-2-0-released.html#toc_new-features-in-the-pipeline [Elm]: https://upcase.com/videos/elm-a-front-end-language-with-style [thoughtbot guides repo]: https://github.com/thoughtbot/guides

High Level Rules

In hopes of keeping this practical, here are some of the high level rules we suggest as a way to mitigate coupling within the controller and view layers of your application:

Never Rely on Instance Variables in Partials

This one is pretty hard and fast. We should always explicitly pass data into a partial as locals, and not rely on an instance variable from the parent context. This decouples our partial allowing for easier reuse throughout the application, and makes the partial easier to understand and test in isolation.

Avoid Assigning Instance Variables in a before_filter

Whenever possible, and this is likely the vast majority of the time, avoid assigning to instance variables via before_filter methods. This hides the assignment making it less clear what views rely on that data, and makes it possible to accidentally override or mutate that data in child contexts.

Instead, expose the same lookup logic via a helper method and either use that method in your views, or use that method to assign to an instance variable in the specific action where that data is needed.

Rails Scaffold Notes

Note: currently the rails scaffold generator will generate a controller with the default model lookup for show, edit, update, and destroy extracted to a before_action:

$ bin/rails generate scaffold Project

will produce the following controller:

class ProjectsController < ApplicationController
  before_action :set_project, only: [:show, :edit, :update, :destroy]

  # GET /projects/1
  def show
  end

  # ... Other actions omitted

  private
    # Use callbacks to share common setup or constraints between actions.
    def set_project
      @project = Project.find(params[:id])
    end
end

We recommend avoiding this and instead consider having a private method for model lookup, but keeping the instance variable assignment local to the action currently being executed:

class ProjectsController < ApplicationController
  def show
    @project = find_project
  end

  private

  def find_project
    Project.find(params[:id])
  end
end

the lookup / assignment line above will be duplicated in the edit, update, and destroy actions, but we feel this minor duplication is absolutely worth it for the added clarity and explicitness it provides.

Using the session

The [session][] in Rails is way to associate additional state with a user, typically by storing it in a cookie, to make that state available when rendering a view. At a minimum we'll likely end up identifying a signed in user via the session, but often we'll end up reaching for the session to store other ad-hoc data. This is another sneaky source of global state.

For instance, in our example we want to show the current user a list of other users who have been added to the organization since last the current user viewed the list. We can use the session as a quick and easy way to store the state of when the current user last viewed:

# app/views/users/_list.html.erb

<ul>
  <% @organization.users.where("created_at > ?", Time.parse(session[:last_viewed])) do |user| %>
    <li>
      <%= user.email %>
    </li>
  <% end %>
</ul>

<% session[:last_viewed] = Time.current %>

Here we're first breaking the rule of never referencing instance variables in partials, but we're also using (and mutating) the session. This global state again makes the view harder to understand and test as we now have to deal with all of the values that could be in the session.

Likewise, since session is stored on a given device, we're left with a situation where users can see out of sync data across various machines. Instead, if the data is important to your application, it likely deserves persistence in the database.

[session]: http://guides.rubyonrails.org/security.html#what-are-sessions-questionmark

Dependency Inversion

An extension of the explicitness described above is to embrace the [dependency inversion principle]. Rather than hard coding references to our dependencies in our classes, we instead invert control and write our classes such that all dependencies are passed in as arguments. For example:

class SubscriberCounter
  def count
    User.where.not(subscription: nil)
  end
end

In the above class, we have hard coded our dependency on the User class by directly referencing the User constant. Instead, we could write our class as:

class SubscriberCounter
  def initialize(users:)
    @users = users
  end

  def count
    users.where.not(subscription: nil)
  end
end

In this case, we've inverted control by requiring that the users collection be passed in as an argument to the constructor (user [Ruby 2.0 required keyword argument][]). Now if we want to operate on just users in the current organization rather than all users, the SubscriberCounter will not need to change. We've isolated it from changes in its dependencies by passing them in, rather than hard coding them.

Joe Ferris took this to the extreme in the [upcase-exercises app][] by creating the [payload][] gem to take advantage of dependency inversion within the app.

[dependency inversion principle]: https://robots.thoughtbot.com/back-to-basics-solid#dependency-inversion-principle [SOLID]: https://robots.thoughtbot.com/back-to-basics-solid [Ruby 2.0 required keyword argument]: https://robots.thoughtbot.com/ruby-2-keyword-arguments [upcase-exercises app]: https://github.com/thoughtbot/upcase-exercises [payload]: https://github.com/thoughtbot/payload

Final Recommendations

To recap, here are the collected recommendations for encouraging encapsulation and avoiding global state in your Rails applications:

• Never access instance variables in partials
• Avoid setting instance variables in before_action/before_filters, especially in parent controllers like ApplicationController
• Avoid accessing session, params, cookies, and flashs in views
• General guideline: aim for your views to be as close to pure functions as possible.