In Progress
Unit 1, Lesson 21
In Progress

Self Save Part 2

Video transcript & code

Today's episode is part 2 in an ongoing miniseries about the pitfalls of domain model objects which save themselves to the database. If you haven't yet watched part 1, you'll probably want to go watch that for context before you start on this episode.

Yesterday, we started with one of the ways self-saving objects can make testing more difficult. Let's move on to a second testing scenario.

This time, we have a series of tests which verify the state that the object is in after each domain event.

require "rspec"
require "./monkey_purchase2"

RSpec.describe MonkeyPurchase do
  it "awaits signature after submission" do
    mp = MonkeyPurchase.new
    mp.submitted
    expect(mp.state).to eq("awaiting_waiver")
  end

  it "awaits approval after waiver" do
    mp = MonkeyPurchase.new
    mp.submitted
    mp.waived
    expect(mp.state).to eq("awaiting_approval")
  end

  it "ships after receiving approval" do
    mp = MonkeyPurchase.new
    mp.submitted
    mp.waived
    mp.approved
    expect(mp.state).to eq("shipping")
  end

  it "is marked complete after shipping" do
    mp = MonkeyPurchase.new
    mp.submitted
    mp.waived
    mp.approved
    mp.shipped
    expect(mp.state).to eq("complete")
  end
end
rspec validations_spec.rb

These tests might not be quite as fast as they could be because of the multiple, unnecessary saves, but at least they are passing.

But then, one day someone adds a validation to check that the quantity of monkeys ordered is between 1 and 99.

validates :quantity, inclusion: { in: 0..99 }

Suddenly, all the tests are failing!

SELFSAVE=YES rspec validations_spec.rb
true

The issue here is that because the object implicitly saves itself, we are now required to make sure all of its attributes are valid when working with it—even if some of those attributes are completely irrelevant to the task at hand!

At this point, this is where a lot of Rails developers turn to using a test factory approach, such as the FactoryGirl library, to automate building valid example objects. But in my opinion this is really just a cosmetic band-aid for a deeper design issue. In general, if a single unrelated change breaks numerous tests, it's a good idea to ask if perhaps this is a sign of tangled responsibilities.

Such as, for instance, methods that mix domain logic with persistence decisions.

Next scenario! Our monkey delivery service has been doing brisk sales, and we're expanding our business. Lately we've begun setting up booths at traveling circuses, collecting orders on paper, and then entering them into the system in batches once we reach an internet connection.

The batch-entry code looks like this:

def batch_submit_orders(orders)
  MonkeyPurchase.create(orders) do |mp|
    mp.submitted
    mp.waived
  end
end

It makes use of the fact that ActiveRecord makes it possible to batch-create new records by passing a list of hashes to the create class method.

We're also making use of the fact that we can operate on records inside the block given to a .create call. Here, we're letting each record know that it should consider itself submitted, and that the customer has already signed a waiver of liability.

Let's do a little benchmarking here. First, we'll submit a batch of 1000 orders with self-saving manually disabled.

Then we'll do the same thing, but with the objects permitted to save themselves whenever they want.

When we run this benchmark, the results are clear: the version where self-saving was enabled was considerably slower.

Each time we sent an object a message to tell it that a lifecycle event had occurred, it went ahead and saved itself. None of these extra saves were necessary. The newly created objects would have wound up persisted to the database either way.

And remember, just as before we're using an in-memory SQLite database to benchmark this code. Based on our experience in the preceding episode of running tests against a database file, that a file-based database, or one on the other end of a TCP connection, would be many times slower still.

require "./monkey_purchase"

def batch_submit_orders(orders)
  MonkeyPurchase.create(orders) do |mp|
    mp.submitted
    mp.waived
  end
end


require "benchmark"
require "faker"

orders = 1000.times.map {
  { customer: Faker::Name.name, quantity: rand(1..99) }
}

Benchmark.bm(12) do |x|
  x.report("no self-save") do
    batch_submit_orders(orders)
  end
  x.report("self-save") do
    ENV["SELFSAVE"] = "YES"
    batch_submit_orders(orders)
  end
end

# >>                    user     system      total        real
# >> no self-save   0.610000   0.010000   0.620000 (  0.618479)
# >> self-save      1.200000   0.010000   1.210000 (  1.211359)

Are we done with scenarios yet? Not remotely!

Here's another unit test. Right now, it's passing.

require "rspec/autorun"
require "./monkey_purchase3"

ENV["SELFSAVE"] = "YES"

RSpec.describe MonkeyPurchase do
  it "awaits approval after waiver" do
    mp = MonkeyPurchase.new
    mp.submitted
    mp.waived
    expect(mp.state).to eq("awaiting_approval")
  end
end

But then someone goes and adds an after-save hook.

The hook checks to see if the object is awaiting approval. If so, it kicks off a request to the ZooPS shipping service to see if exotic animal delivery is approved to the customer's postal code.

require "active_record"
require "net/http"

class ZooPS
  def self.can_ship_to?(postal_code)
    Net::HTTP.get_response("localhost", "/shipping_approval?code=#{postal_code}")
    true
  end
end

class MonkeyPurchase < ActiveRecord::Base
  establish_connection :adapter  => "sqlite3",
                       :database => ENV.fetch("DB") { ":memory:" }

  connection.create_table( :monkey_purchases ) do |t|
    t.integer    :quantity
    t.string     :customer
    t.string     :postal_code
    t.string     :state
    t.timestamps null: false
  end

  after_save do
    if state == "awaiting_approval"
      if ZooPS.can_ship_to?(postal_code)
        self.state = "approved"
      else
        self.state = "denied"
      end
    end
  end

  def save!
    if ENV["SELFSAVE"] == "YES"
      super
    else
      # NOOP
    end
  end

  def submitted
    self.state = "awaiting_waiver"
    save!
  end

  def waived
    self.state = "awaiting_approval"
    save!
  end
end

Now when we run this test about state transitions we get, of all things, an HTTP exception.

require "rspec/autorun"
require "./monkey_purchase3"

ENV["SELFSAVE"] = "YES"

RSpec.describe MonkeyPurchase do
  it "awaits approval after waiver" do
    mp = MonkeyPurchase.new
    mp.submitted
    mp.waived
    expect(mp.state).to eq("awaiting_approval")
  end
end

# >> F
# >>
# >> Failures:
# >>
# >>   1) MonkeyPurchase awaits approval after waiver
# >>      Failure/Error: $SiB.record_result(10, (mp.waived))
# >>
# >>      Errno::ECONNREFUSED:
# >>        Failed to open TCP connection to localhost:80 (Connection refused - connect(2) for "localhost" port 80)
# >>      # ./monkey_purchase3.rb:6:in `can_ship_to?'
# >>      # ./monkey_purchase3.rb:25:in `block in <class:MonkeyPurchase>'
# >>      # /home/avdi/.gem/ruby/2.3.0/gems/activesupport-4.2.6/lib/active_support/callbacks.rb:446:in `instance_exec'
# >>      # /home/avdi/.gem/ruby/2.3.0/gems/activesupport-4.2.6/lib/active_support/callbacks.rb:446:in `block in make_lambda'
# >>      # /home/avdi/.gem/ruby/2.3.0/gems/activesupport-4.2.6/lib/active_support/callbacks.rb:228:in `block in halting_and_conditional'
# >>      # /home/avdi/.gem/ruby/2.3.0/gems/activesupport-4.2.6/lib/active_support/callbacks.rb:506:in `block in call'
# >>      # /home/avdi/.gem/ruby/2.3.0/gems/activesupport-4.2.6/lib/active_support/callbacks.rb:506:in `each'
# >>      # /home/avdi/.gem/ruby/2.3.0/gems/activesupport-4.2.6/lib/active_support/callbacks.rb:506:in `call'
# >>      # /home/avdi/.gem/ruby/2.3.0/gems/activesupport-4.2.6/lib/active_support/callbacks.rb:92:in `__run_callbacks__'
# >>      # /home/avdi/.gem/ruby/2.3.0/gems/activesupport-4.2.6/lib/active_support/callbacks.rb:778:in `_run_save_callbacks'
# >>      # /home/avdi/.gem/ruby/2.3.0/gems/activerecord-4.2.6/lib/active_record/callbacks.rb:302:in `create_or_update'
# >>      # /home/avdi/.gem/ruby/2.3.0/gems/activerecord-4.2.6/lib/active_record/persistence.rb:142:in `save!'
# >>      # /home/avdi/.gem/ruby/2.3.0/gems/activerecord-4.2.6/lib/active_record/validations.rb:43:in `save!'
# >>      # /home/avdi/.gem/ruby/2.3.0/gems/activerecord-4.2.6/lib/active_record/attribute_methods/dirty.rb:29:in `save!'
# >>      # /home/avdi/.gem/ruby/2.3.0/gems/activerecord-4.2.6/lib/active_record/transactions.rb:291:in `block in save!'
# >>      # /home/avdi/.gem/ruby/2.3.0/gems/activerecord-4.2.6/lib/active_record/transactions.rb:351:in `block in with_transaction_returning_status'
# >>      # /home/avdi/.gem/ruby/2.3.0/gems/activerecord-4.2.6/lib/active_record/connection_adapters/abstract/database_statements.rb:213:in `block in transaction'
# >>      # /home/avdi/.gem/ruby/2.3.0/gems/activerecord-4.2.6/lib/active_record/connection_adapters/abstract/transaction.rb:184:in `within_new_transaction'
# >>      # /home/avdi/.gem/ruby/2.3.0/gems/activerecord-4.2.6/lib/active_record/connection_adapters/abstract/database_statements.rb:213:in `transaction'
# >>      # /home/avdi/.gem/ruby/2.3.0/gems/activerecord-4.2.6/lib/active_record/transactions.rb:220:in `transaction'
# >>      # /home/avdi/.gem/ruby/2.3.0/gems/activerecord-4.2.6/lib/active_record/transactions.rb:348:in `with_transaction_returning_status'
# >>      # /home/avdi/.gem/ruby/2.3.0/gems/activerecord-4.2.6/lib/active_record/transactions.rb:291:in `save!'
# >>      # ./monkey_purchase3.rb:35:in `save!'
# >>      # ./monkey_purchase3.rb:48:in `waived'
# >>      # xmptmp-in3707QgC.rb:10:in `block (2 levels) in <main>'
# >>      # ------------------
# >>      # --- Caused by: ---
# >>      # Errno::ECONNREFUSED:
# >>      #   Connection refused - connect(2) for "localhost" port 80
# >>      #   ./monkey_purchase3.rb:6:in `can_ship_to?'
# >>
# >> Finished in 0.01224 seconds (files took 0.46124 seconds to load)
# >> 1 example, 1 failure
# >>
# >> Failed examples:
# >>
# >> rspec xmptmp-in3707QgC.rb:7 # MonkeyPurchase awaits approval after waiver
# >>

This turns out to originate from the ZooPS service.

All things considered, this is probably one of the more benign things that could have gone wrong. It might have been even worse if the HTTP request had silently gone through - we might not have found out about it until we got a nasty call from ZooPS, asking why we are slamming their servers with thousands of spurious requests.

And before you say "yeah, but who would kick off an HTTP request from an after_save handler?": remember, all of these examples are inspired by actual events in real codebases. Often, situations like these occur entirely by accident: a series of relatively innocent, isolated decisions end up causing a completely unpredictable chain of events.

In this particular case, surely one major contributing factor to the accident must be the not-at-all-obvious connection between telling a MonkeyPurchase that it has received its waiver, and the record being saved.

Like in most of the preceding examples, we've demonstrated this particular antipattern in the context of a test. But this problem isn't confined to tests. For instance, consider the case of the batch order submission example we looked at a moment ago.

That's another situation where we would wind up generating an unacceptable storm of HTTP requests, both slowing down our own operations and inundating an external service with requests. And by putting the decision to save the record inside the business logic, we've removed our ability to choose whether this happens or not.

At this point we might consider adding in an HTTP stubbing framework like WebMock, to capture and nullify web requests.

require "rspec/autorun"
require "webmock/rspec"
require "./monkey_purchase3"

ENV["SELFSAVE"] = "YES"

RSpec.describe MonkeyPurchase do
  it "awaits approval after waiver" do
    stub_request(:any, "http://localhost/shipping_approval?code=123567")
    mp = MonkeyPurchase.new(postal_code: 123567)
    mp.submitted
    mp.waived
    expect(mp.state).to eq("approved")
  end
end

# >> .
# >>
# >> Finished in 0.01421 seconds (files took 0.43442 seconds to load)
# >> 1 example, 0 failures
# >>

But this is just adding more complexity and dependencies to our tests. Like the earlier example of adding in test object factories to deal with validation side-effects, it's a band-aid for a deeper problem.

Well, we've once again gone overtime so I think that's enough for today. In the next episode, we'll look at even more troubles arising from self-saving objects. Until then: happy hacking!

Responses