thoughtbot / thoughtbot/factory_bot

insert_all or active_record-import integration

Open
#1,788 3 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

feature
Dominant language
Ruby
Stars
8.2k
Forks
2.6k
PR merge metrics
No merged PRs in 30d

Description

Problem this feature will solve

When inserting large amounts of data, or running a large test suite where a lot of lists are created, this is generally slow because each record is created individually, and then callbacks fired, which doesn't scale very well.

In other parts of our app, we handle bulk inserts either with ActiveRecord::Import which lets us do recursive inserts, building out the hierarchy first and then optimizing the insert statements. We occasionally also use the now standard insert_all

Desired solution

Would love some way of integrating active_record-import or ActiveRecord#insert_all with FactoryBot, so that create_list would:

  1. Build out required associations, then insert those in bulk, and fire their callbacks
    2.Bulk insert the list of records and fire callbacks

Basically, a seemless, plug and play, solution that optimizes inserts for lists. Providing something like to_create { ... } but to_create_list { ... } would work too, but I think the issue is how the list strategies are generated and dont really allow for much customization

Alternatives considered

I first attempted to write my own strategy for doing this, but it turns out that all the _list strategies are really just i.times { send(strategy, ...) }, which means that with the current implementation, I couldn't see a good way of doing this.

I ended up writing a pretty hacky way to do something similar, but I feel like this hooks into too many internals, and doesn't actually handle association factories' callbacks (it just inserts the records):

FactoryBot.define_singleton_method(:import_list) do |name, amount, *traits_and_overrides, &block|
  unless amount.respond_to?(:times)
    raise ArgumentError, "count missing for import_list"
  end

  records = Array.new(amount) do |i|
    block_with_index = FactoryBot::StrategySyntaxMethodRegistrar.with_index(block, i)
    send(:build, name, *traits_and_overrides, &block_with_index)
  end

  # Collect belongs_to associations that haven't been persisted and need to be persisted
  # before we insert the list
  associated_records = {}

  # Proc to find unsaved belongs_to associations on a record and add to cache
  find_belongs_to_associations = lambda do |record|
    record.class.reflect_on_all_associations(:belongs_to).each do |association|
      associated_record = record.send(association.name)
      next unless associated_record.present?
      next if associated_record.persisted?
      associated_records[associated_record.class] ||= []
      next if associated_records[associated_record.class].include?(associated_record)
      associated_records[associated_record.class] << associated_record
      find_belongs_to_associations.call(associated_record)
    end
  end
 
  records.each do |record|
    find_belongs_to_associations.call(record)
  end

  # For each model, import the records in bulk
  associated_records.each_value do |assoc_records|
    next if assoc_records.empty?
    assoc_records.first.class.import!(assoc_records, recursive: true, ignore: true)
  end

  # Now import the main list of records
  records.first.class.import!(records.reject(&:id), recursive: true, ignore: true)

  overrides = traits_and_overrides.select { |t| t.is_a?(Hash) }.reduce({}, :merge)
  traits = traits_and_overrides.select { |t| t.is_a?(Symbol) }

  # Apply callbacks (here comes the hackiest bit .... This is all hooking into internals)
  factory = FactoryBot.factories[name]
  factory = factory.with_traits(traits) if traits.any?

  # We want to fire after(:create) callbacks after importing records
  strategy = FactoryBot.strategy_by_name(:create).new
  
  evaluator = factory.send(:evaluator_class).new(
    strategy,
    overrides.symbolize_keys
  )
  observer = FactoryBot::CallbacksObserver.new(factory.send(:callbacks), evaluator)
  attribute_assigner = FactoryBot::AttributeAssigner.new(
    evaluator,
    factory.send(:build_class)
  )
  evaluation = FactoryBot::Evaluation.new(
    evaluator,
    attribute_assigner,
    factory.send(:compiled_to_create),
    observer
  )

  # For each imported record, fire after_create. Unfortunately we dont have the factories
  # for the belongs_to records we imported, and cant fire their callbacks
  records.each do |record|
    evaluation.notify(:after_create, record)
  end

  records
end

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by reading FactoryBot's create_list/list strategy generation and the StrategySyntaxMethodRegistrar entry point referenced in the issue, then compare the proposed import_list flow with the existing create strategy and callback handling. Done means a supported bulk-list integration with ActiveRecord::Import or insert_all that handles required associations and callbacks, with tests covering the list behavior.

Written by the indexing model from the issue text.

Assessment

Tech stack
rails, ruby
Domain
testing
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.