Zombie Population Explosion - a Lesson in Factorygirl Creation

The other day, purely by accident, I discovered that lurking behind our cute little factory girl tests was an apocolypse of angry zombies. “REALLY?,” you ask. Well no, not actual zombies.

But let me back up a second and explain.

Factory Girl is a testing helper library we use to simplify data creation in Rails. In it’s basic form, it makes creating test objects this simple:

1
2
3
4
5
FactoryGirl.define do
  factory :user do
    name 'Thomas Rogan'
  end
end
1
2
3
4
5
let(:user) { create(:user) }

describe "user is a user object" do
  expect { user.name }.to eq 'Thomas Rogan'
end

Neat, right? Over time, your factories start to get some more meat on their bones:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
FactoryGirl.define do
  factory :user do
    sequence(:name) { 'Zombie #{i}' }
    faction { faction name: 'Zombies' }
    trait(:with_location) { location name: "House of the Dead" }
    trait(:with_shotgun) { gun factory: :gun, :shotgun }
    ...
  end

  factory :location do
    user
    name 'Earth'
  end

  factory :gun
    user # (owner)
    kind "pistol"
    trait(:shotgun) { kind "shotgun" }
  end

  factory :faction
    name "zombies"
  end

  ...
end

The problem

So what’s the problem with the above setup (Other than that we are providing shotguns to an army of zombies)?

Well let’s see. Imagine (like I did) that you want to write a test for a worker that iterates over all of our users in a slow method:

1
2
3
4
5
6
7
8
class ArmAllUsersWithBetterWeaponWorker < Worker
  def perform
    User.all.each do |user|
      ArmUser.call(user)
      sleep 1.second # to avoid being rate limited by some poorly implemented external service
    end
  end
end
1
2
3
4
5
6
7
8
9
10
11
describe ArmAllUsersWithBetterWeaponWorker do
  subject { described_class.new.perform }
  let!(:thomas) do
    create(:user, :with_location, :with_shotgun, name: "Thomas Rogan")
  end

  it "arms the user" do
    subject
    expect(thomas.armed_with_better_weapon?).to eq true
  end
end

Imagine my surprise when the simple test took over 3 seconds to run! (And yes, I could stub out the sleep time to be shorter, or stub User.all, but surely something else must be wrong?).

Diving into the code with pry, I realized that my simple creation of Thomas had created 2 lurking zombies.

See, what happens when I run the line create(:user, ..., name: "Thomas Rogan") is factorygirl creates location and gun objects (along with a default zombie user for each one), and after creating them sets their user properties to be the “Thomas” user.

The ‘zombie’ users that were originally created for those objects then just lurk around in the background and cause problems. The User.all call was now arming these zombies as well as the Thomas. And taking over 3 seconds in the process.

What’s the big deal?

Ummm. Speed for starters – Every extra has_one/has_many trait you add to those users spawns another zombie. That takes time. And not only time – it leads to bugs in your test code since you have extra objects you didn’t intend to create lying around.

Our code (we don’t actually deal with zombies, we set up people on 3-3 blind dates) had over half a dozen traits that were creating zombies on the user model alone. Which translates into serious time loss in our test suite, and bugs like the above.

Deriving a Solution

So how to solve this? Well the first thing I thought of was to move all of those traits that sneakily create another zombie to be after_create callbacks so you can pass the user in to them:

1
2
3
  trait(:with_shotgun) do
    after(:create) { |user| create(:gun, :shotgun, user: user) }
  end

Rewriting this simply for our main User class resulted in a 10% speed increase in our test suite.

Unfortunately there’s a caveat. Not all test objects are created using factory girl’s create strategy. Ideally, as much code as possible should be created using build, or even better, build_stubbed, which doesn’t persist to the database and is much faster. However, using this strategy to create a test user breaks because the after_create hook is not called.

1
2
user = build_stubbed(:user, :with_shotgun)
user.gun # nil

Given that ideally we want to be moving towards a situation where we use build_stubbed more than create, this seemed not ideal.

Should we write an after(:build) and after(:build_stubbed) callback too then? Seems kind of wet. Not to mention, it won’t work because calling the create factorygirl method runs both the after_create and after_build callbacks, so our code would be run twice.

The alternative would be to define an extra set of traits for stubbed calls:

1
trait(:with_stubbed_location) { location, name: "House of the Dead" }

and then use these when building a stubbed model:

1
build_stubbed(:user, :with_stubbed_location)

Zombie objects in stubbed calls don’t matter so much because they aren’t persisted and so don’t take much time to create and also aren’t returned in database queries so they won’t interfere with your other tests.

After a bit of time thinking about this, I decided we faced 2 alternatives.

  1. Just deal with the fact that we create extra objects – simple but slower
  2. Use the above method of after_create hooks and separate trait names for stubbed/build calls. – less dry but faster

The solution

Each team prioritizes speed/code quality differently – so you may opt to deal with the extra users and test time to reduce code complexity.

As for us? We ended up going with option 2). A slight decrease in dryness with 10% gains in speed seem worth it to our company, at least at the moment.

Other thoughts

  • The above problem is only a problem for ‘has_one’/‘has_many’ relationships – by nature, those objects need to be created after the primary object has been created to be valid. So in the above code, a belongs_to relationship like faction can be created before the user object has been created, so no sneaky extra objects are created.
  • Thanks for reading – let me know if you have thoughts/comments.

Configuring Rails Environments

Recently at Grouper we stopped configuring our staging environment with a staging.rb config file and instead started using ENV variables.

Why would we do this? The twelve factor app says it all. But in summary, multiple configuration files are problematic:

  • You cannot update your server’s configuration without committing and deploying new code.
  • There are more code paths. Due to Ruby’s lack of type checking, the following code has a full code path that is not tested.
1
2
3
4
5
if Rails.env.production?
  Rails.confugurution.url_options[:host] = 'http://example.com'
else
  Rails.configuration.url_options[:host] = 'http://localhost'
end
  • Bundler won’t install ‘production’ grouped gems on your staging server, so this adds another discrepency between your production and staging servers. Just another way things can go wrong.
  • It’s easy to make mistakes when configuring code by environment. For each new environment you add, you have to worry more about the following types of conditions:
1
2
3
if Rails.env.production? || Rails.env.staging? || Rails.env.qa? ...
  ...
end

The new path forward: Using Config Variables

Our changes mainly took statements like:

1
2
3
4
if Rails.env.development? || Rails.env.staging?
  # Mount admin panel
  ...
end

and turned them into the more readable and prettier:

1
2
3
4
if ENV['MOUNT_ADMIN_PANEL']
  # Mount admin panel
  ...
end

Of course, to show an example of how to configure your environment, we need a model for your coworkers to work off of:

1
2
3
4
# .env.example
#
# MOUNT_ADMIN_PANEL = true # Comment this to not mount the admin panel
#

Not quite a straight path: the pitfalls of this approach

Configuring your application with environment variables does have a few gotchas to watch out for, though:

Forcibly Failing fast with fetch

Excuse the alliteration, but if it sticks with you, it’s worth it. I can’t tell you how much time I’ve spent debugging production code because an environment variable was not set or was misspelled.

Often times, this failure will only happen in an asynchronous job long after we deployed and all the devs are out drinking.

A recent example of such code was in a job that sends out messages to our members. The guilty class was so:

1
2
3
4
5
6
7
8
class SmsMessage

  ...

  def set_default_attributes
    self.number_from ||= ENV['OUTBOUND_SMS_NUMBER']
  end
end

At some point we need to send a message to someone, and only then, and further down the stack, do we get errors about a missing number.

The solution? First of all, Always use fetch to access ENV variables:

1
2
3
def set_default_attributes
  self.number_from ||= ENV.fetch('OUTBOUND_SMS_NUMBER')
end

fetch will immediately raise an error if the config variable hasn’t been set for that environment, making debugging the above code much easier.

Taking this one step further, you can fetch that variable in an initialization file when the app first starts up. This way you realize your mistake as soon as you deploy rather than after the job first runs.

Truthy Falsey variables

Imagine you have a scary env variable like the following:

1
DISPLAY_CONFIDENTIAL_INFORMATION = false

That controls your flow like so:

1
2
3
4
if ENV['DISPLAY_CONFIDENTIAL_INFORMATION']
  # render member's credit card information on page

end

First of all, if you are storing/displaying full credit card information on your servers, you need to stop reading this, shut down your servers, and integrate Stripe to manage your cards before going further.

But another problem exists. The above conditional actually evaluates to true, because in Ruby the string ‘False’ is not false but true. Confusing, I know.

So when it comes to booleans in your ENV config variables, you have a couple main options.

Check for explicit truth values:

1
2
3
if ENV.fetch('DISPLAY_CONFIDENTIAL_INFORMATION').to_s.downcase == 'true'
  ...
end

This is pretty ugly to write everywhere. So instead, define a global WrappedEnv:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
class WrappedEnv < SimpleDelegator
  def fetch(key)
    val = super
    return true if val.to_s.downcase == 'true'
    return false if val.to_s.downcase == 'false'
    return val
  end
end

WRAPPED_ENV = WrappedEnv.new(ENV)

if WRAPPED_ENV.fetch('DISPLAY_CONFIDENTIAL_INFORMATION')
  ...
end

Alternatively, our first solution was to treat any environment variable that is set to anything as true, and just comment it out or unset it if you want it to be false:

1
2
3
if ENV['DISPLAY_CONFIDENTIAL_INFORMATION'].present?
   ...
end

This last solution has the major drawback of not being able to use fetch for booleans anymore (since missing ENV variables will raise on fetch).

Document your env variables in one place

A new dev (or you in 3 months) needs to be able to look at your app and know all the configuration options without opening dozens of files.

Listing all of your ENV variable keys, with potential comments next to them in your example file, is key (sorry, couldn’t help it) to maintaining sanity.

Otherwise, it’s really difficult to understand if your production server is potentially missing ENV variables, or setting them incorrectly.

1
2
3
# .env.example
#
# ACTUALLY_SEND_PUSH_NOTIFICATIONS = false # set to true in production to send real push notifications

Soft link your .env to .env.example

If you go this route, you’ll probably see a lot of ENV variables showing up all over the codebase. Our new project has 32, and it’s only been around for a couple months. As each new variable gets added, it can be annoying to have to manually update them every time you pull in from master.

If you can, set your .env.example as the ideal developer environment, so that you can just link your .env to it locally:

1
ln -s .env.example .env

Alternatively, you can find gems for managing your local env.

As an aside, your development environment should not include any sensitive API keys, only test versions, free to share with even the most dubious of outsourcing agencies.

Conclusion

Configure your app using ENV variables instead of config files. You’ll be much happier you did!

If you love working with a great team and figuring out new ways to do things, come work with us, we are hiring :)

Related Links:

- Ben Kies, engineer at Grouper

Too Many Signals - Resque on Heroku

At Grouper, like a lot of the Rails community, we use Resque to manage our asynchronous background jobs. It lets us do everything from sending emails to processing charges and most of the time everything works perfectly.

The Problem

But sometimes things don’t work quite as well as we’d like. Recently we ran into a common problem with Resque deployments on Heroku – our jobs were being sent TERM signals in the middle of processing.

A quick review of Heroku signal handling can be found here.

TL;DR – Heroku reserves the right to send TERM signals to any dyno whenever it wants. Note that the TERM signal is sent to EVERY process in the dyno, not just the process type. Anything running is given 10 seconds to clean itself up and shut down before Heroku sends it a KILL signal which causes it to die immediately.

At first this didn’t seem like too big of an issue, just catch the TERM signal and reenqueue the job (we use Resque Retry) – no harm done.

Except that not all of our jobs are fully idempotent. Running them multiple times could cause users to be charged more than once or to get emails more than once which is definitely not desirable.

An Example

Here’s an example short-running job similar to one we run all the time:

1
2
3
4
5
class TrackEventJob
  def self.perfom(tracking_id, event_name)
    EventTrackingService.track(tracking_id, event_name)
  end
end

And a snippet from our Procfile:

1
worker: env TERM_CHILD=1 bundle exec rake resque:work QUEUES=*

What happens if this job receives a TERM signal in the middle of processing? It immediately raises Resque::TermException which aborts execution. If we had set up Resque Retry to reenqueue the job then that would happen at that time. However, there’s no easy way to tell if the event tracking service received data from us before the TERM signal ends the job. Hence by retrying we could end up sending the event more than once and thus confuse our analytics.

Similar reasoning applies to charging credit cards or anything that you can’t make atomic.

In particular we found that our very frequent short running jobs were failing the most. On the order of several hundred per day as a result of using Hirefire to dynamically scale our Resque workers, and deploying continuously throughout the day. Frankly, we were pretty surprised by the sheer number of jobs that were failing and it was obvious we needed to do something about it.

Since retrying the jobs was not an option, we settled on giving them a few seconds between when Heroku sent a TERM signal and when they actually raised Resque::TermException. At the very least this would let our short running jobs complete execution in most cases and return normally as opposed to being sent KILL signals and ending with Resque::DirtyExit.

The Solution

A Possible Solution

First some terminology:

  • Parent: The Process that pulls down jobs but processes them in a fork.
  • Child: The fork of the parent process that does the actual processing.

We wanted to give our short running jobs (which generally take < 1 second to process) an opportunity to finish executing rather than having them die immediately. Unfortunately this didn’t seem to be easily possible given how Resque was dealing with signals.

So we extended Resque with an optional PRE_TERM_TIMEOUT length that caused the parent to wait before sending the TERM signal to the child (A quick overview of the changes).

This was a good start, but didn’t actually resolve the problem. Since Heroku sends TERM signals to EVERY process running on a dyno (and a Resque worker is just a dyno), the parent and child processes would receive the signal at the same time despite our modification to delay the parent from sending it to the child.

Unfortunately, a few more changes were needed to get this working.

Our Actual Solution

After evaluating all our options, including swapping out Resque in favor of Sidekiq, we determined that the best solution was to add another small modification to Resque.

We needed to have the child process be able to ignore TERM signals from Heroku but let the parent process command it to die in keeping with normal Resque signal handling.

Our solution was to add another configuration option to Resque. This time called TERM_CHILD_SIGNAL which controls what signal is sent from the parent to the child when the parent is telling the child to die gracefully. To complete the configuration we set our TERM_CHILD_SIGNAL to be QUIT (rather than the default TERM).

Our updated procfile looked like this:

1
worker: env PRE_TERM_TIMEOUT=5 TERM_CHILD_SIGNAL=QUIT TERM_CHILD=1 bundle exec rake resque:work QUEUES=*

The effect of doing this is to have the child ignore TERM signals which are sent by Heroku but not ignore QUIT signals sent by the parent process. Now receiving QUIT causes Resque::TermException (somewhat confusingly named unfortunately) to be raised in the child which plugs directly into any retry logic we may have implemented. That set of changes is visible here

The best part of this approach is that no modifications need to be made to the job itself.

Conclusion

While it certainly isn’t desirable to fork your own version of a gem (we submitted this PR to resque but it hasn’t been merged), in the short term it can serve as a passable solution to a tough problem like this.

While this isn’t the only possible way to solve this problem, it is a way that worked fairly well for us (though not perfectly). We still get sporadic errors from jobs taking longer than the PRE_TERM_TIMEOUT to finish and therefore being killed. However, the volume of errors is significantly reduced from where it was.

Despite a lot of searching around, we couldn’t find much discussion about people facing a similar issue, though it is inevitable that this problem would crop up given how Heroku works.

- Ethan Langevin, @ejl6266, engineer at Grouper

Rails - Internationalization - Translation

Internationalization? Even the name is so complicated that we need a shorthand (i18n) to talk about it.

Yes, the process of internationalizing your site can be pretty painful. Patterns that work well in English (like dynamically inserting prepositions into a string) suddenly don’t work when you have multiple languages to consider. Granted, it’s a pretty good problem to have. At Grouper, after 2 years of amazing growth at home, we are excited to be expanding into several new markets. Having successfully launched in London, and soon Berlin and Paris, we are now starting to feel the pain of maintaining a site in multiple languages.

Fortunately, there are plenty of tools and resources out there to help you out. This series will present some lessons we learned along the way.

Part 1 – Translation

Rails already has a great guide on translation. This post serves to complement it with some other considerations and best practices we used at Grouper.

Use a Translation Tool

Since the people doing translation work for you will probably not know how to navigate a codebase, you’ll need a translation tool like phraseapp to provide an interface for them to translate.

This means you don’t have to give up access to your codebase and helps to manage a distributed team.

Translating snippets within larger translations

A theme we often came across in our translations was simplification. At Grouper, we put a lot of effort to tailor the product towards each individual member.

As a very basic example, when talking about a member’s wings (the 2 pals you bring with you on a grouper), we say your wingmen for guys, and your wingwomen for girls. (As background, Grouper sets up drinks between groups of friends, 3 on 3)

This level of customization becomes tricky when multiple languages become involved. Consider the translation:

1
2
3
4
I18n.translate("invite_others", friend: @friend)

en:
  invite_others: "bring a %{friend} on the Grouper with you"

In English, this translation scheme works. However in French, articles differ depending on whether friend is male or female:

1
2
3
fr:
  invite_others: "Apporter un ami sur le voyage avec vous" # boy friend
  invite_others: "Apporter une amie sur le voyage avec vous" # girl friend

We can’t just pass in ami or amie to the translation string, as un and une are different depending on what gender friend is.

So the French translator can’t create a single translation that works for both guys and girls. This is something that most people will initially miss on their first pass of establishing translation keys.

In this case, you can either rephrase the sentence to not include gender, or accept some repetition and have two full translations for each case:

1
2
3
4
fr:
  invite_others:
    girls: "bring a girl friend on the Grouper with you"
    boys: "bring a guy friend on the Grouper with you"

They both have their drawbacks, but if personalization is important, repitition is unavoidable.

Split up translations into different files

A common mistake is placing all our translations into a single file en.yml. This might not seem like a big deal, but the merge conflict pain it causes is diresome. Do this from the beginning. We split ours up like so:

1
2
3
4
5
6
7
8
9
10
11
config
 - locale
   - active_record.en.yml
   - en.yml
   - javascript.en.yml
   - mailers.en.yml
   - policies.en.yml
   - simple_form.en.yml
   - team.en.yml
   - time.en.yml
   - views.en.yml

Use Generic Keys. But not too Generic

Don’t use overly generic keys. Or on the other end of the spectrum, keys that copy the English translation word for word.

Instead, use keys that symbolize the intent of the string.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
### 1. The key is a description of what the text is for. This is the subject
# line for the email.
confirm_info_reminder_email:
  subject: Let's get you on a Grouper - Finish your app!

### 2. The key describes what the text means.
confirm_info_reminder_email:
  finish_your_application: Let's get you on a Grouper - Finish your app!

### 3. Literal key based off the text.
confirm_info_reminder_email:
  lets_get_you_on_a_grouper: Let's get you on a Grouper - Finish your app!


# Approach 1 is too loose. Somebody can change the entire meaning of the
# subject line without touching the app.

# Approach 2 is the sweet spot. Somebody can edit the wording of the text, if
# the meaning changes then the key should be changed too. This will trigger
# missing translations and the new meaning can be worked on.

# Approach 3 is too tightly couped. We'll end up slightly changing the text
# wording and the key doens't exactly match the text anymore, which is what
# this approach aims for.

Javascript Translations

One part of a web page that is easily missed when translating is javascript. On more complex pages we use Backbone views to do our front-end dynamic rendering. These javascript files are static and so we cannot translate them dynamically on each request (which we need to do because translations are for a locale specific to each request)

One solution we use is to attach all javascript translations a javascript object in our layout.

1
2
3
4
5
6
7
8
9
10
11
12
13
:javascript
  G.translations = #{I18n.t('javascript').to_json};

  G.t = function(key) {
    var current_scope = G.translations;
    $.each(key.split("."), function(i, key_part) {
      current_scope = current_scope[key_part];
        if (current_scope == undefined) {
          return false;
        }
    });
    return current_scope || "missing translation: #{I18n.locale}.javascript." + key;
  };

We can then access these translations directly through a translator function G.t:

1
2
3
4
5
6
7
8
9
10
11
12
class G.Views.CreditCards.CreditCardView extends Backbone.View
  ...

  # Save Credit Card to Grouper
  saveCreditCard: (token) =>
    $.post(@options.creditCardsPath, {
        stripe_single_use_token: token,
        credit_card: @creditCardPlaceholders()
      }).fail((railsResponse) =>
        @showError G.t('credit_card.could_not_be_added')
      )
  ...

As your codebase grows, you can optimize this by precompiling these translations per locale and linking to the relevant static locale file per request.

Namespacing Keys

Lazy translation provides a concise way to write translation keys in views.

We expanded on this idea to provide lazy lookups in our model decorators. (If you are translating strings directly in your models, you should read up on decorators!)

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
class ApplicationDecorator < Draper::Decorator
  delegate_all

  private

  def translate(*args)
    key = args.first
    if key.is_a?(String) && (key[0] == '.')
      underscored_scope = self.class.name.gsub('Decorator', '').underscore
      args[0] = underscored_scope + key
    end

    I18n.translate(*args)
  end
  alias :t :translate

  def localize(*args)
    I18n.localize(*args)
  end
  alias :l :localize
end

Subclass all your decorators from this, and your translations will automatically be namespaced to the model they represent. Note that we remove the word Decorator from the namespace – your translations namespace should not depend on the particular design pattern you use.

For example, a translation for our member could be:

1
2
3
4
5
MemberDecorator < ApplicationDecorator

  def friends
    t(".friends_gender") # key scope: 'member.friends_gender'
  end

Conclusion

These ideas aren’t the only way to do things, but they worked well for us. Hopefully they’ll give some insight to teams who are just starting out their internationalization.

Look out for the next part in this series, on Timezone Havoc. In the meantime, if you love figuring out new ways to do things, check out our jobs page, we are hiring.

You can contribute to the discussion on Hacker News.

Other Useful tools:

  • i15r gem for automated string to key replacement in views.

- Ben Kies, engineer at Grouper

Rails - the Missing Parts - Policies

This is the second in a series of posts on “Rails – The Missing Parts”. The first post inspired some vigorous debate in the Rails community – check out the discussion for more detail.

In this post, we’ll be discussing “Policies” or “Policy Objects” as a way of DRYing up your controllers.

By way of background, Grouper is a site that organises drinks between groups of 3 guys and 3 girls. They’re tons of fun.

As our business requirements have become more complex, we’ve noticed a trend in our codebase – controllers often need to check a fairly complex combination of rules before allowing an action to be executed.

For example, before a user can join a Grouper, we might want to check the following:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
class TicketsController < ApplicationController

  before_filter :check_user_can_join_grouper

  def create
    if ticket.save
      redirect_to home_path
    else
      render :new, error: ticket.errors.full_messages
    end
  end

  private

  def check_user_can_join_grouper
    if current_user.blacklisted? # They've been banned for bad-behaviour
      redirect_to home_path, notice: You can't book a Grouper at this time
    elsif grouper.members.count >= 6
      redirect_to reserve_path, notice: This grouper is full, please pick
      another date
    elsif grouper.datetime < Time.now
      redirect_to home_path, error: This grouper has already occured!
    elsif current_user.has_existing_grouper?(grouper)
      redirect_to home_path, error: Youre already going on a Grouper that day
    elsif ticket.confirmed?
      redirect_to grouper_path(grouper), error: Youve already confirmed this grouper
    end
  end
  
end

This occurs across our codebase in multiple places – in our TicketsController, in our API::TicketsController (for the iPhone app), and in our Admin::TicketsController. For authentication and permissioning reasons, we want to keep these controllers separate.

This is not the worst code in the world, but it’s certainly not the best. It causes problems in particular when it comes to testing and re-use. There are now an additional 4 branches of logic that need to be tested for every action in the controller that uses this before_filter. You might be tempted to reach for shared example groups to DRY up your specs, but this level of testing feels unnecessarily repetitive and will make your suite painfully slow. You’d typically end up stubbing the before_filter on the controller for the majority of your tests.

Re-use is also a problem, as we implement these checks in 3 separate controllers. We could move the logic to a controller mixin, or define the method on the ApplicationController, but it would still not be available in our views, and we often want to check if a user can book a ticket before displaying helpful button or link.

The solution is to move the logic into a method on an object that can be tested in isolation and be called safely from controllers and views. Many Rails developers’ natural tendency seems to be to put this kind of logic onto an ActiveRecord model – User#can_join?(grouper) or Ticket#can_be_confirmed_by?(user) – but we don’t like this approach. These kinds of “permission” methods often ends up reaching across many different models, and require the User or Ticket class to have way to much knowledge of their associated objects. A recurring theme in these posts is that ActiveRecord models should be very simple interfaces to a datastore – the User model should be responsible for validating and persisting attributes of a user, like name and date-of-birth, and not much else. The tendency for Rails applications to revolve around ActiveRecord models with vast public APIs tends to increase model coupling and massively slows down test suites.

Instead, at times like this, the seasoned Ruby developer should reach for a plain-old Ruby object that implements a very narrow public API. For the sake of common language, we call these objects “Policies” – they’re similar in many ways to Interactors, but are concerned with complex read operations, rather than writing new data.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
class CanConfirmTicketPolicy
  include Policy::PolicyObject

  def allowed?
    grouper = ticket.grouper
    user    = ticket.user

    if user.blacklisted? # They've been banned for bad-behaviour
      fail! You can't book a Grouper at this time
    elsif grouper.full?
      fail! This grouper is full, please pick another date
    elsif grouper.past?
      fail! This grouper has already occured!
    elsif user.has_existing_grouper?(grouper)
      fail! Youre already going on a Grouper that day
    elsif ticket.confirmed?
      fail! Youve already confirmed this grouper
    end
  end
end

(We also cleaned up some of the long method chains into shorter methods like Grouper#full? and Grouper#past? to make the Policy easier to test, and avoid Law of Demeter violations)

We’ve extracted some of the common functionality in a gem called Policy, which provides more concise syntax for use in Rails controllers:

1
2
3
4
5
6
7
8
9
10
11
12
13
controller TicketsController < ApplicationController

  policy(:can_confirm_ticket) 

  def create
    if ticket.save
      redirect_to home_path
    else
      render :new, error: ticket.errors.full_messages
    end
  end
  
end

By default, this will redirect_to :back with a flash[:error] set, but you can always customize the response by defining an unauthorized method on the controller.

You can now call CanConfirmTicketPolicy.new(ticket: ticket).allowed? from views, controllers or even models. It’s also quick to test because you can initialize the Policy with test doubles, rather than ActiveRecord models. You don’t even need to load Rails to run your tests.

If you really want to expose a convenience method on your User, you can do this:

1
2
3
4
5
6
7
class User < ActiveRecord::Base

  def can_confirm?(ticket)
    CanConfirmTicketPolicy.new(ticket: ticket).allowed?
  end

end

which lets you do this in your views:

1
link_to ticket if current_user.can_confirm?(ticket)

These Policy objects aren’t always the right solution – if you’ve only got one of two simple permission checks that are run on a single controller, stick them directly in a before_filter. But when you find yourself copy-pasting code changes between controller and struggling with unwieldy multi-line before_filters, Policies provide a useful addition. They simplify your application by following the single-responsibility principle, they’re extraordinarily quick to test and trivially easy to re-use around your code-base.

They also provide a hint to more complex code-bases that burdening your core models classes with extremely vast public APIs is usually a bad idea.

We believe that any “Advanced” Rails deployment needs a concept similar to Policies for DRYing up controller permissioning, in a similar way to ActiveModel Validations. Rails developers are of course free to add this gem to their code-base, but the advantage of a set of centrally standardised APIs is that any experienced Rails developer who comes across this pattern is immediately familiar with it.

For the final part in the series, see our upcoming post on Decorators!


If you’re interested in Rails design-patterns and best-practices, you should check out our jobs page – we’re hiring.

You can also get involved in the discussion on hacker news.

Rails - the Missing Parts - Interactors

Here at Grouper, we’re long-time users of Ruby on Rails – along with other New York startups like RapGenius and Kickstarter. It’s simple to pick up and is optimized for developer productivity.

However, people have started noticing the downsides – once codebases evolve past a few thousand lines of code, test suites often become sluggish and the framework load-time increases.

We’ve come to realise it doesn’t have to be this way – a number of unhelpful Rails features encourage poor design patterns, which in turn lead to tightly-coupled code and slow, unmaintainable test-suites.

While Rails may be the perfect tool for building an MVP, it isn’t optimised for medium and large codebases. We’re solving these problems with 3 concepts we believe should be part of any “Advanced” Rails deployment; Interactors, Policies and Presenters.

Part 1 – Interactors

The overwhelming trend in Rails codebases is for the majority of business logic to reside in very large God classes in the ActiveRecord /models directory. This is normally a clue that each class has too many responsibilities, a vast public API and methods that require the presence of a complex graph of associated objects in order to function at all.

The nail in the coffin is ActiveRecord callbacks; before_save hooks on one class that modify the state of other objects. This is a problem because those other objects are now required to be present before we can play with the object we care about. When testing, we are stuck between tediously slow test-suites (as these objects are often fetched from the database) or the laborious process of stubbing out callbacks and long chains of method calls.

The solution is a concept called “Interactors” (often called “Service Objects”. Interactors demand that the core of your application should live in a set of plain-old-Ruby-objects (POROs) that are responsible for the main use-cases of your application, leaving your ActiveRecord classes as skinny interfaces to your data-store. By looking at the names of your Interactors, you should be able to tell what your application does; SignUp, BookGrouper, AssignBarForGrouper, etc. Classes like Member and Bar just validate and store attributes like name, location and date.

(By way of background – Grouper organises “Groupers” – 3-on-3 drinks events at bars between groups of friends. They’re a lot of fun)

There’s a very lightweight gem called Interactor which exposes a couple of convenient methods like success? and failure?, so your controllers can now look like this:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
class GroupersController < ApplicationController::Base
  
  def create
    interactor = ConfirmGrouper.perform(leader: current_member)

    if interactor.success?
      redirect_to home_path
    else
      flash[:error] = interactor.message
      render :new
    end
  end
  
end

and your interactor might look like this:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
# Responsible for creating a Grouper, email
#
#
class ConfirmGrouper
  include Interactor

  def perform
    grouper = Grouper.new(leader: member)
    fail!(grouper.errors.full_messages) unless grouper.save
    send_emails_for(grouper)
    assign_bar_for(grouper)
  end

  private

  def send_emails_for(grouper)
    LeaderMailer.grouper_confirmed(member: grouper.leader.id).deliver
    WingMailer.grouper_confirmed(wings: grouper.wings.map(&:id)).deliver
    AdminMailer.grouper_confirmed(grouper: grouper.admin.id).deliver
  end

  def assign_bar_for(grouper)
    # Asynchronous job because it’s a little slow
    AssignBarForGrouper.enqueue(grouper.id)
  end
end

The advantages are numerous – both in terms of code-complexity and test-speed. For one, your tests should now be lightning-fast because you rarely need to hit the database, and you can test each ActiveRecord model in isolation without worrying about its associations. The Interactors themselves can be be tested with doubles rather than ActiveRecord models, and Rails often doesn’t even need to be loaded by your test suite.

Your controller spec might look like this:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
describe GroupersController do
  
  describe #create” do
    subject { post :create }

    before { ConfirmGrouper.stub(:perform).and_return(interactor) }

    let(:interactor) { double(Interactor, success?: success, message: foo) }

    context when the interactor is a success do
      let(:success) { true }

      it { should redirect_to home_path }
    end

    context when the interactor fails do
      let(:success) { false }

      it { should render :new }
    end
  end
end

With all ActiveRecord database calls stubbed out, this test file now runs in around 0.15 seconds, compared to around 4 or 5 seconds previously.

Additionally, your application will be easier to reason about because each class has a single responsibility – the Bar class knows where the bar is located and when it’s open, for example. It doesn’t need to know about Groupers. The complex logic for determining which Bar to assign to which Grouper can sit safely on its own in the AssignBarForGrouper Interactor.

Finally, if you keep your Interactors small and single-purpose, you can compose multiple Interactors together for more complex operations. This mix-and-match approach further decouples your codebase and lets you reuse operations as necessary.

Conclusion

You obviously need to choose the patterns that fit the problem you’re trying to solve – it’s rarely one-size-fits-all, and some of these principles may be overkill in a very simple 15-minute blog application. Our objection is that Rails feels like it’s a Beginners’ Version, often suggesting that one size does fit all. Once your codebase surpasses a very basic level of complexity, you’re left on your own grasping for solutions. We’d suggest Interactors are one of these solutions.

For the next in the series, see Policy Objects – slimming down your controllers.

If you’re interested in Rails design-patterns and best-practices, you should check out our jobs page – we’re hiring.

You can also get involved in the discussion on hacker news.

The Right Tool for the Job, Isn’t

I always wince when I hear teams bragging about the number of datastores/frameworks/languages that they use.

No big deal. We keep our relational data in Postgres, our non-relational data in Mongo, any sets that we might have in redis and obviously we use good ol’ memcached when we need it. We use rails for our web workers, but it’s not async so we use node for our background workers. We make sure we use the right tool for the job.

When it’s not mere self-indulgence, adding “the right tool” is usually a premature optimization. If you’ve chosen a decent stack, odds are that whatever benefits an additional tool might have are outweighed by the complexity it adds to your project.

Minimize complexity

Be aware of the maintenance cost of adding a new tool to your stack. For example, if you’re adding a new datastore, do you have a way of setting it up in your development, and staging environments? Do you have a plan for testing it? Can you scale it? Does your backup system work? Can you allot time to teach every current and future teammate how it works? If you don’t have an answer to any of these, you’re adding risk to your startup.

Even if you’re rigorous in properly installing your new tool, adding it to your stack increases your complexity. When complexity increases, so does the risk that something will fall apart.

Use tools that fail together

Counterintuitively, putting all of your eggs in one basket is often good for stability. Imagine that you’re choosing between two stacks for a Python app with Postgres and Redis.

Stack A

  • Google AppEngine
  • Heroku Postgres
  • RedisCloud cluster on Rackspace

Stack B

  • Heroku
  • Heroku Postgres
  • OpenRedis Addon

Let’s say that each of the services have 99.9% uptime. You might think that both stacks would have the same uptime. The thing is, all the services in Stack B are served on EC2, so when they fail, they fail at the same time. All the services in Stack A are in different datacenters. This means that they’re likely to each fail at different times, and in total, the stack will be down three times as often as Stack B.*

When is “the right tool” the right tool?

Pick your risks. When you start a project you’re not going to choose to implement a new product using a new framework in a new language backed by a new database. Instead, choose something to experiment with and work on a branch.

At Grouper we like to use small projects to test out new tools. For example, we tested ember.js on a simple embedded questions game before using it for our mission-critical admin pages.

- Tom Brown – find me on Twitter here.

* Stack A → (1 - 0.999^3)*(730 hours/month) = 2.18 hours. Stack B → (1 - 0.999)*(730 hours/month) = 43.8 minutes. For simplicity, this presupposes that the only reason services go down is due to data center issues. That would be sweet.