Unwanted statefulness causing mayhem in a Rails application

The Use Cases pattern

At UNCAP, we have structured the code base of our Ruby-on-Rails based web platform around the pattern of "use cases". It's a simple concept: Isolated classes that do one thing and one thing only, and employ other use cases to achieve their goal. In that way, complex business logic is constructed from simpler building blocks, which can be individually tested.

These classes are usually initialized with references to the (simpler) use cases they depend on, the advantage being that we can swap them out for mock implementations in our tests, so that a test can focus on the behaviour of just the use case it belongs to, controlling any other variables.

Here's an example:

module Uncap
  module Investment
    module UseCases
      class GenerateStatementDocx

        def initialize(
          get_document_template: GetDocumentTemplate.new,
          get_statement_values: GetStatementValues.new
        )
          @get_document_template = get_document_template
          @get_statement_values = get_statement_values
        end

        def call(investment:)
          template = get_document_template.call
          # ...
        end
      end
    end
  end
end

This class uses two other, simpler use cases, each doing a part of the overall job. In the test, we pass mock implementations of GetDocumentTemplate and GetStatementValues to the constructor, which return expected values, so the test is really focusing on the behaviour of GenerateStatementDocx and none of the other use cases it employs.

Memoization

Something you see in Rails code all the time is memoization, i. e. storing the result of a costly operation (like a database query) in an instance variable, and returning the cached value the next time it is accessed.

def transactions
  @transactions ||= get_statement_values.call(investment: investment)
end

This introduces state into the use case, so one must be careful to discard the instance whenever it has done its job. Which leads me to..

The mysterious case of the empty account statements

The job of this particular use case is to generate an account statement PDF for every investment, and send it to the contact email address.

def call
  investment_model.paid_out.each do |investment|
    generate_and_send_statement.call(investment: investment)
  end
end

You can see immediately that we are re-using the same instance of the GenerateAndSendStatement use case for every investment. This is not a problem as long as that use case does not keep any internal state, which we thought it didn't.

However, our users started complaining that they were receiving empty account statements, even though they had made repayments on the investment they received from us!

After investigating, we found that one of the use cases further down the chain was memoizing the transactions it received from the database. And that use case should have been initialized with a new instance for every investment, but it wasn't.

So the first investment was processed correctly, but all subsequent investments processed the same transactions as the first one. Luckily, the first investment in our production database was a test investment with no transactions, so we sent everyone an empty account statements, instead of the genuine transactions of another company, which would have been a much bigger problem..

How to avoid these bugs?

  • First of all, run each test of a use case multiple times with different arguments to call, so that you can spot if any unwanted state is being kept between invocations.

  • Simply don't memoize — this is the safest option, but it can be costly if the same data is needed multiple times in the same use case, and your queries are expensive. In our case, this would not have been a problem, since the use case ran just once per month, so the memoization was over-eager and not necessary.

  • You could clear any memoized state at the end of the call method, but this is not a very elegant solution, too verbose, and easy to forget.

  • Enforce the "single-use" character of use cases. One way of achieving this would be by adding a class method self.call which creates a new instance and invokes call on it, and use only that. The problem with this approach is the constructor and the instance call method have to remain public in order to be able to pass in mock implementations in the tests, so the use of only the class method cannot be enforced.

  • Create a super class for all use cases, which wraps the call method and saves a flag in an instance variable to indicate whether it has been called already. If call is invoked a second time, it raises an exception.

As with all dynamic, weakly typed languages, it is a trade-off between programmer productivity and freedom one one side, and type safety and robustness on the other. The best way to avoid these bugs with Ruby/Rails is to have very thorough test coverage, even though writing tests can seem verbose and tedious at times.

Also, don't memoize without being certain the performance hit is really going to be a problem. The old adage that "premature optimization is the root of all evil" applies here, too.

In a functional language like Elixir, memoization with instance variables is not possible, since there are no classes/instances, only pure functions, and data is immutable. So, in order to optimize, you would store the result of a query in a local variable, and pass it to the next function or use case in the chain. This is a much safer approach, since the local variable is automatically discarded at the end of the use case.