Skip to content

Commit

Permalink
Reject sequence definitions for Active Record primary keys
Browse files Browse the repository at this point in the history
Alternative to [thoughtbot/factory_bot#1586][]
Depends on [thoughtbot/factory_bot#1587][]

First, introduce the `FactoryBotRails::FactoryValidator` class to serve
as a generic `factory_bot.compile_factory` event observer.

Throughout the lifecycle of the `FactoryBotRails::Railtie`, afford
various initializers with opportunities to add purpose-built validators
to the instance's internal set.

When `factory_bot.compile_factory` events are published, iterate through
the list of validators and forward along the value of the
[event.payload][] to the validator's `#validate!` method.

Next, introduce the Active Record-specific
`FactoryBotRails::FactoryValidator::ActiveRecordValidator` class. Only
require the module whenever [Active Record's engine is loaded][on_load].

The `ActiveRecordValidator#validate!` method rejects attributes that
define primary key generation logic for `ActiveRecord::Base`
descendants.

In order to test this behavior, add a development dependency on
`sqlite3` and `activerecord`, along with some model and database table
generating helper methods.

[thoughtbot/factory_bot#1586]: thoughtbot/factory_bot#1586
[thoughtbot/factory_bot#1587]: thoughtbot/factory_bot#1587
[event.payload]: module-ActiveSupport::Notifications-label-Subscribers
[on_load]: https://guides.rubyonrails.org/engines.html#avoid-loading-rails-frameworks
  • Loading branch information
seanpdoyle committed Aug 12, 2023
1 parent a0adccc commit a75db2d
Show file tree
Hide file tree
Showing 9 changed files with 162 additions and 2 deletions.
1 change: 1 addition & 0 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,4 @@ gem "cucumber"
gem "rake"
gem "rspec-rails"
gem "standard"
gem "factory_bot", github: "thoughtbot/factory_bot", branch: "compile_factory_instrumentation"
20 changes: 18 additions & 2 deletions Gemfile.lock
Original file line number Diff line number Diff line change
@@ -1,3 +1,11 @@
GIT
remote: https://github.com/thoughtbot/factory_bot.git
revision: 45862362521acfda32f5c3e200e529f120d99265
branch: compile_factory_instrumentation
specs:
factory_bot (6.2.1)
activesupport (>= 5.0.0)

PATH
remote: .
specs:
Expand All @@ -21,6 +29,11 @@ GEM
erubi (~> 1.4)
rails-dom-testing (~> 2.0)
rails-html-sanitizer (~> 1.1, >= 1.2.0)
activemodel (7.0.4.3)
activesupport (= 7.0.4.3)
activerecord (7.0.4.3)
activemodel (= 7.0.4.3)
activesupport (= 7.0.4.3)
activesupport (7.0.4.3)
concurrent-ruby (~> 1.0, >= 1.0.2)
i18n (>= 1.6, < 2)
Expand Down Expand Up @@ -69,8 +82,6 @@ GEM
cucumber-tag-expressions (4.1.0)
diff-lcs (1.5.0)
erubi (1.12.0)
factory_bot (6.2.1)
activesupport (>= 5.0.0)
ffi (1.15.5)
ffi (1.15.5-java)
i18n (1.12.0)
Expand Down Expand Up @@ -150,6 +161,8 @@ GEM
rubocop (>= 1.7.0, < 2.0)
rubocop-ast (>= 0.4.0)
ruby-progressbar (1.13.0)
sqlite3 (1.6.2)
mini_portile2 (~> 2.8.0)
standard (1.27.0)
language_server-protocol (~> 3.17.0.2)
rubocop (~> 1.50.2)
Expand All @@ -167,12 +180,15 @@ PLATFORMS
ruby

DEPENDENCIES
activerecord (>= 5.0.0)
appraisal
aruba
cucumber
factory_bot!
factory_bot_rails!
rake
rspec-rails
sqlite3
standard

BUNDLED WITH
Expand Down
3 changes: 3 additions & 0 deletions factory_bot_rails.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,7 @@ Gem::Specification.new do |s|

s.add_runtime_dependency("factory_bot", "~> 6.2.0")
s.add_runtime_dependency("railties", ">= 5.0.0")

s.add_development_dependency("sqlite3")
s.add_development_dependency("activerecord", ">= 5.0.0")
end
17 changes: 17 additions & 0 deletions lib/factory_bot_rails/factory_validator.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
module FactoryBotRails
class FactoryValidator
def initialize(validators = [])
@validators = Array(validators)
end

def add_validator(validator)
@validators << validator
end

def run
ActiveSupport::Notifications.subscribe("factory_bot.compile_factory") do |event|
@validators.each { |validator| validator.validate!(event.payload) }
end
end
end
end
16 changes: 16 additions & 0 deletions lib/factory_bot_rails/factory_validator/active_record_validator.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
module FactoryBotRails
class FactoryValidator
class ActiveRecordValidator
def validate!(payload)
attributes, for_class = payload.values_at(:attributes, :class)
attributes.each do |attribute|
if for_class < ActiveRecord::Base && for_class.primary_key == attribute.name.to_s
raise FactoryBot::AttributeDefinitionError, <<~ERROR
Attribute generates #{for_class.primary_key.inspect} primary key for #{for_class.name}"
ERROR
end
end
end
end
end
end
14 changes: 14 additions & 0 deletions lib/factory_bot_rails/railtie.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,25 @@
require "factory_bot"
require "factory_bot_rails/generator"
require "factory_bot_rails/reloader"
require "factory_bot_rails/factory_validator"
require "rails"

module FactoryBotRails
class Railtie < Rails::Railtie
config.factory_bot = ActiveSupport::OrderedOptions.new
config.factory_bot.definition_file_paths = FactoryBot.definition_file_paths
config.factory_bot.validator = FactoryBotRails::FactoryValidator.new
config.factory_bot.reject_primary_key_attributes = true

ActiveSupport.on_load :active_record do
config = Rails.configuration.factory_bot

if config.reject_primary_key_attributes
require "factory_bot_rails/factory_validator/active_record_validator"

Rails.configuration.factory_bot.validator.add_validator FactoryValidator::ActiveRecordValidator.new
end
end

initializer "factory_bot.set_fixture_replacement" do
Generator.new(config).run
Expand All @@ -21,6 +34,7 @@ class Railtie < Rails::Railtie
config.after_initialize do |app|
FactoryBot.find_definitions
Reloader.new(app).run
app.config.factory_bot.validator.run
end

private
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
# frozen_string_literal: true

describe FactoryBotRails::FactoryValidator do
after { FactoryBot.reload }

describe "ActiveRecordValidator" do
context "when defined with a class that descends from ActiveRecord::Base" do
it "raises an error for a sequence generating its primary key" do
define_model "Article", an_id: :integer do
self.primary_key = :an_id
end

FactoryBot.define do
factory :article do
sequence(:an_id)
end
end

expect { FactoryBot.create(:article) }
.to raise_error(FactoryBot::AttributeDefinitionError)
end
end
end
end
2 changes: 2 additions & 0 deletions spec/spec_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
require "factory_bot_rails"
require "fake_app"

Dir["spec/support/**/*.rb"].each { |f| require File.expand_path(f) }

RSpec.configure do |config|
config.run_all_when_everything_filtered = true
config.filter_run :focus
Expand Down
67 changes: 67 additions & 0 deletions spec/support/macros/define_constant.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
require "active_record"

module DefineConstantMacros
def define_class(path, base = Object, &block)
const = stub_const(path, Class.new(base))
const.class_eval(&block) if block
const
end

def define_model(name, columns = {}, &block)
model = define_class(name, ActiveRecord::Base, &block)
create_table(model.table_name) do |table|
columns.each do |column_name, type|
table.column column_name, type
end
end
model
end

def create_table(table_name, &block)
connection = ActiveRecord::Base.connection

begin
connection.execute("DROP TABLE IF EXISTS #{table_name}")
connection.create_table(table_name, &block)
created_tables << table_name
connection
rescue Exception => e # rubocop:disable Lint/RescueException
connection.execute("DROP TABLE IF EXISTS #{table_name}")
raise e
end
end

def clear_generated_tables
created_tables.each do |table_name|
clear_generated_table(table_name)
end
created_tables.clear
end

def clear_generated_table(table_name)
ActiveRecord::Base
.connection
.execute("DROP TABLE IF EXISTS #{table_name}")
end

private

def created_tables
@created_tables ||= []
end
end

RSpec.configure do |config|
config.include DefineConstantMacros

config.before(:all) do
ActiveRecord::Base.establish_connection(
adapter: "sqlite3",
database: ":memory:"
)
end

config.after do
clear_generated_tables
end
end

0 comments on commit a75db2d

Please sign in to comment.