Object associations behave inconsistently inside factories

[odinhb]

In our rails app, we had some code like this:

class Account < ApplicationRecord
  belongs_to :unit

  validates_presence_of :number
  validates_uniqueness_of :number, scope: :unit_id, message: "Number has to be unique for this unit"
end

class Unit < ApplicationRecord
  has_many :accounts, dependent: :destroy
end

# factories/accounts.rb
FactoryBot.define do
  factory :account do
    association :unit
    sequence(:name) { |n| "Fake account #{n}" }
    number {
      generate_account_number(
        unit.accounts.pluck(:number).to_set
      )
    }
  end
end

# factory_bot_helpers.rb
def generate_account_number exclude_list
  l = -> { "%04d" % rand(10**4) } # generate a 4-digit random number
  number = 1
  attempt = 0
  loop do
    attempt += 1
    number = l.call
    break unless exclude_list.include?(number.to_s)
    raise "We've run out of account numbers for this unit" if attempt > 1000
  end
  number.to_s
end

# some_spec.rb
some_unit = FactoryBot.create :unit

# ... pretend we create ~10 accounts here

# this one fails randomly because the association unit.accounts returns an empty collection
FactoryBot.create :account, unit_id: some_unit.id

# this one always works because the association returns the accounts already created
FactoryBot.create :account, unit: some_unit

Our problem lies inside the number-block (where we call generate_account_number) When passing unit_id to the factory unit.accounts returns [] When passing a unit object to the factory unit.accounts returns actual accounts.

Our current solution looks like this: Account.where(unit_id: unit_id).pluck(:number).to_set There shouldn't be a difference between these two.

The problem is that subtleties like this cause failing tests and weird unintuitive behavior.

There are a couple of things I would like to be able to do about it: If I could configure this particular factory to simply raise if passed any {attribute}_id (or an arbitrary attribute), we could prevent ourselves from accidentally building test data incorrectly. We would also want to configure this factory to be unbuildable (raise on calling :build), as it would have to run a query against the database to ensure that the data generated is valid.

(yes I know we should use a sequence)

System configuration

factory_bot version: 4.11.1 rails version: 5.2.2 (activerecord 5.2.2) ruby version: 2.4.2

[composerinteralia]

This is an interesting one! It seems to be a bug related to association aliasing (factory_bot aliases #{association} and #{association}_id, and if one of those is passed in as an override it will not set the other one).

You should be able to work around this by either ensuring you always pass a unit instead of a unit_id, or by changing

unit.accounts.pluck(:number).to_set

to

Unit.find(unit_id).accounts.pluck(:number).to_set`

(although in this case you will need to be sure to define the unit association before the attribute that relies on it).

---

I created a reproduction script to demonstrate this problem without the code specific to your application. In this example we have a denormalized author_name stored on a Post.

require "bundler/inline"

gemfile(true) do
  source "https://rubygems.org"
  git_source(:github) { |repo| "https://github.com/#{repo}.git" }
  gem "factory_bot", path: "~/Desktop/thoughtbot/factory_bot"
  gem "activerecord"
  gem "sqlite3"
  gem "byebug"
end

require "active_record"
require "factory_bot"
require "minitest/autorun"
require "logger"
require "byebug"

ActiveRecord::Base.establish_connection(adapter: "sqlite3", database: ":memory:")
ActiveRecord::Base.logger = Logger.new(STDOUT)

ActiveRecord::Schema.define do
  create_table :authors, force: true do |t|
    t.string :name
  end

  create_table :posts, force: true do |t|
    t.references :author
    t.string :author_name
  end
end

class ApplicationRecord < ActiveRecord::Base
  self.abstract_class = true
end

class Post < ApplicationRecord
  belongs_to :author

  validates_presence_of :author_name
end

class Author < ApplicationRecord
  has_many :posts
end

# factories/accounts.rb
FactoryBot.define do
  factory :author do
    sequence(:name) { |n| "Name #{n}" }
  end

  factory :post do
    association :author
    author_name { author.name }
  end

  factory :post_with_swapped_order, class: Post do
    author_name { author.name }
    association :author
  end

  factory :post_with_explicit_lookup, class: Post do
    association :author
    author_name { Author.find(author_id).name }
  end

  factory :post_with_explicit_lookup_and_swapped_order, class: Post do
    author_name { Author.find(author_id).name }
    association :author
  end
end

class FactoryBotTest < Minitest::Test
  def test_passing_author
    author = FactoryBot.create(:author)
    post = FactoryBot.create(:post, author: author)

    # This passes because the `author` override is put into the Evaluator
    # @cached_attributes on initialization, and so it is available when used by
    # the `author_name` attribute.
    assert_equal author.name, post.author_name
  end

  def test_passing_author_id
    author = FactoryBot.create(:author)
    post = FactoryBot.create(:post, author_id: author.id)

    # This test fails. When we call `author` inside of `author_name`, the
    # Evaluator calls its defined `author` method, which creates a brand new
    # `Author` instance, unrelated to the `author_id` passed in
    assert_equal author.name, post.author_name
  end

  def test_passing_author_with_swapped_order
    author = FactoryBot.create(:author)
    post = FactoryBot.create(:post_with_swapped_order, author: author)

    # This passes. The order of the attributes doesn't matter here since the
    # Evaluator caches the `author` override on initialization.
    assert_equal author.name, post.author_name
  end

  def test_passing_author_id_with_swapped_order
    author = FactoryBot.create(:author)
    post = FactoryBot.create(:post_with_swapped_order, author_id: author.id)

    # This test fails. The order of the attributes isn't significant for
    # this case
    assert_equal author.name, post.author_name
  end

  def test_passing_author_with_explicit_lookup
    author = FactoryBot.create(:author)
    post = FactoryBot.create(:post_with_explicit_lookup, author: author)

    # This passes because we never end up defining the `author_id` method on
    # Evaluator, so it falls back to calling `author_id` on the Post instance
    # itself. `author` is set by the time we call `author_id`.
    assert_equal author.name, post.author_name
  end

  def test_passing_author_id_with_explicit_lookup
    author = FactoryBot.create(:author)
    post = FactoryBot.create(:post_with_explicit_lookup, author_id: author.id)

    # This passes since the `author_id` is put into the Evaluator's
    # @cached_attributes on initialization, making it available to the
    # `author_name` attribute
    assert_equal author.name, post.author_name
  end

  def test_passing_author_with_explicit_lookup_and_swapped_order
    author = FactoryBot.create(:author)
    post = FactoryBot.create(:post_with_explicit_lookup_and_swapped_order, author: author)

    # This test errors because we fall back to calling `author_id` on the Post
    # instance itself, but we do that before we have set the `author` on the
    # post (attributes get applied in the order they are defined)
    assert_equal author.name, post.author_name
  end

  def test_passing_author_id_with_explicit_lookup_and_swapped_order
    author = FactoryBot.create(:author)
    post = FactoryBot.create(:post_with_explicit_lookup_and_swapped_order, author_id: author.id)

    # This passes since the `author_id` is put into the Evaluator's
    # @cached_attributes on initialization, making it available to the
    # `author_name` attribute. The order is not significant for this case.
    assert_equal author.name, post.author_name
  end
end

I think there is probably some work we can do to improve this. I assume it would be mostly in the AttributeAssigner and Evaluator classes.

For whoever ends up looking into this: the Evaluator class can be a bit tricky to debug since it undefines most of the standard class methods. I have been using byebug and setting breakpoints with ::Kernel.debugger (debugger by itself was failing with mysterious errors).