From 02a0f584876ff5c7a6d6fa4965da58ed70464a73 Mon Sep 17 00:00:00 2001
From: Daniel Colson <danieljamescolson@gmail.com>
Date: Mon, 16 Jul 2018 18:31:25 -0400
Subject: [PATCH] Allow reloading of factory definitions

Closes #236

This commit uses ActiveSupport's FileUpdateChecker to allow reloading
FactoryBot of definitions whenever a file in
`FactoryBot.definition_file_paths` gets updated.
This is similar to reloading for
[I18n](https://github.com/rails/rails/blob/ced104d57997c7bceef3d1e6c8a713431363c3bb/activesupport/lib/active_support/i18n_railtie.rb#L60-L70)
and [react rails](https://github.com/reactjs/react-rails/blob/83b6175460b6fd19d667854ebea4777d8c73705a/lib/react/rails/railtie.rb#L35-L41)

This allows us to get rid of any Spring-specific logic in the railtie,
since [Spring hooks into the application
reloader](https://github.com/rails/spring/blob/0c711ff10b4ad7dcc34282b5e08573c2ce1e668a/lib/spring/application.rb#L161).

This partly solves #211, since we no longer call `FactoryBot.reload` at
all in `after_initialize`. Instead, we will only call it when one of the
files in `definition_file_paths` gets updated. I say it partly
solves #211 because once a definition file gets updated, reloading
would still give warnings about redefining any constants in the
definition files. I wonder how common it is to define constants in the
same file as factory definitions. It's probably better to keep constants
in the autoload path, allowing Rails to handle unloading and reloading
them in development. I would want to see some specific examples before
worrying too much about it.

I would also like to offer a better way to configure the definition file
paths (see #165 and #166) and possibly an option to opt out of loading
definitions at all (could help with issues like #192).

A couple of quirks here:
* the to_prepare block could potentially get
[called multiple times](https://github.com/rails/rails/issues/28108).
It shouldn't matter, since `execute_if_updated` is a no-op if no
updates have been made.

* I noticed that the first time I call `reload!` in the console the
factory definitions get reloaded even when I made no updates to the
definition files. I think it is related to
[this](https://github.com/rails/rails/blob/f7c5a8ce262cf16ad8d7c0b0d631a4b88afec414/activesupport/lib/active_support/evented_file_update_checker.rb#L18). After the first call
`reload!` works as expected, only reloading the factory definitions
if the definition files were updated.

* Rails uses execute rather than execute_if_updated for the
[route
reloader](https://github.com/rails/rails/blob/master/railties/lib/rails/application/finisher.rb#L133)
and for the [watchable file
reloader](https://github.com/rails/rails/blob/master/railties/lib/rails/application/finisher.rb#L173).
This means that changes to factory definitions will cause reloading of
the whole application. I think this is fine.
---
 .rspec                                        |  3 +
 Rakefile                                      |  5 +-
 .../definition_file_paths.rb                  | 17 ++++++
 lib/factory_bot_rails/railtie.rb              | 27 +++++----
 lib/factory_bot_rails/reloader.rb             | 40 +++++++++++++
 .../factory_bot/model/model_generator.rb      |  2 +-
 .../definition_file_paths_spec.rb             | 34 +++++++++++
 spec/factory_bot_rails/railtie_spec.rb        | 57 +++++++++++++++++++
 spec/fake_app.rb                              | 12 ++++
 spec/fixtures/factories.rb                    |  1 +
 spec/fixtures/factories/definitions.rb        |  1 +
 spec/spec_helper.rb                           | 12 ++++
 12 files changed, 197 insertions(+), 14 deletions(-)
 create mode 100644 .rspec
 create mode 100644 lib/factory_bot_rails/definition_file_paths.rb
 create mode 100644 lib/factory_bot_rails/reloader.rb
 create mode 100644 spec/factory_bot_rails/definition_file_paths_spec.rb
 create mode 100644 spec/factory_bot_rails/railtie_spec.rb
 create mode 100644 spec/fake_app.rb
 create mode 100644 spec/fixtures/factories.rb
 create mode 100644 spec/fixtures/factories/definitions.rb
 create mode 100644 spec/spec_helper.rb

diff --git a/.rspec b/.rspec
new file mode 100644
index 0000000..2559e39
--- /dev/null
+++ b/.rspec
@@ -0,0 +1,3 @@
+--color
+--format progress
+--require spec_helper
diff --git a/Rakefile b/Rakefile
index dfe61c2..6b5fb85 100644
--- a/Rakefile
+++ b/Rakefile
@@ -1,5 +1,6 @@
 require 'bundler/setup'
 require 'cucumber/rake/task'
+require 'rspec/core/rake_task'
 
 Bundler::GemHelper.install_tasks name: 'factory_bot_rails'
 
@@ -8,12 +9,14 @@ Cucumber::Rake::Task.new(:cucumber) do |t|
   t.cucumber_opts = ['--format', (ENV['CUCUMBER_FORMAT'] || 'progress')]
 end
 
+RSpec::Core::RakeTask.new(:spec)
+
 require 'appraisal'
 
 desc 'Run the test suite'
 task :default do |t|
   if ENV['BUNDLE_GEMFILE'] =~ /gemfiles/
-    exec 'rake cucumber'
+    exec 'rake spec && rake cucumber'
   else
     Rake::Task['appraise'].execute
   end
diff --git a/lib/factory_bot_rails/definition_file_paths.rb b/lib/factory_bot_rails/definition_file_paths.rb
new file mode 100644
index 0000000..dd0178b
--- /dev/null
+++ b/lib/factory_bot_rails/definition_file_paths.rb
@@ -0,0 +1,17 @@
+# frozen_string_literal: true
+
+module FactoryBotRails
+  class DefinitionFilePaths
+    attr_reader :files, :directories
+
+    def initialize(definition_file_paths)
+      @files = []
+      @directories = {}
+
+      definition_file_paths.each do |path|
+        @files << "#{path}.rb"
+        @directories[path.to_s] = [:rb]
+      end
+    end
+  end
+end
diff --git a/lib/factory_bot_rails/railtie.rb b/lib/factory_bot_rails/railtie.rb
index f7af2b0..bd39882 100644
--- a/lib/factory_bot_rails/railtie.rb
+++ b/lib/factory_bot_rails/railtie.rb
@@ -1,28 +1,31 @@
-require 'factory_bot'
-require 'factory_bot_rails/generator'
-require 'rails'
+# frozen_string_literal: true
 
-module FactoryBot
+require "factory_bot"
+require "factory_bot_rails/generator"
+require "factory_bot_rails/reloader"
+require "rails"
+
+module FactoryBotRails
   class Railtie < Rails::Railtie
 
     initializer "factory_bot.set_fixture_replacement" do
-      FactoryBotRails::Generator.new(config).run
+      Generator.new(config).run
     end
 
     initializer "factory_bot.set_factory_paths" do
       FactoryBot.definition_file_paths = [
-        Rails.root.join('factories'),
-        Rails.root.join('test', 'factories'),
-        Rails.root.join('spec', 'factories')
+        Rails.root.join("factories"),
+        Rails.root.join("test", "factories"),
+        Rails.root.join("spec", "factories"),
       ]
     end
 
+    initializer "factory_bot.register_reloader" do |app|
+      Reloader.new(app, config).run
+    end
+
     config.after_initialize do
       FactoryBot.find_definitions
-
-      if defined?(Spring)
-        Spring.after_fork { FactoryBot.reload }
-      end
     end
   end
 end
diff --git a/lib/factory_bot_rails/reloader.rb b/lib/factory_bot_rails/reloader.rb
new file mode 100644
index 0000000..329ac26
--- /dev/null
+++ b/lib/factory_bot_rails/reloader.rb
@@ -0,0 +1,40 @@
+# frozen_string_literal: true
+
+require "factory_bot_rails/definition_file_paths"
+
+module FactoryBotRails
+  class Reloader
+    def initialize(app, config)
+      @app = app
+      @config = config
+    end
+
+    def run
+      register_reloader(build_reloader)
+    end
+
+    private
+
+    attr_reader :app, :config
+
+    def build_reloader
+      paths = DefinitionFilePaths.new(FactoryBot.definition_file_paths)
+
+      reloader_class.new(paths.files, paths.directories) do
+        FactoryBot.reload
+      end
+    end
+
+    def reloader_class
+      app.config.file_watcher
+    end
+
+    def register_reloader(reloader)
+      config.to_prepare do
+        reloader.execute_if_updated
+      end
+
+      app.reloaders << reloader
+    end
+  end
+end
diff --git a/lib/generators/factory_bot/model/model_generator.rb b/lib/generators/factory_bot/model/model_generator.rb
index d5a49eb..4db0e21 100644
--- a/lib/generators/factory_bot/model/model_generator.rb
+++ b/lib/generators/factory_bot/model/model_generator.rb
@@ -91,7 +91,7 @@ RUBY
       end
 
       def generators
-        config = FactoryBot::Railtie.config
+        config = FactoryBotRails::Railtie.config
         config.respond_to?(:app_generators) ? config.app_generators : config.generators
       end
     end
diff --git a/spec/factory_bot_rails/definition_file_paths_spec.rb b/spec/factory_bot_rails/definition_file_paths_spec.rb
new file mode 100644
index 0000000..68ccad6
--- /dev/null
+++ b/spec/factory_bot_rails/definition_file_paths_spec.rb
@@ -0,0 +1,34 @@
+# frozen_string_literal: true
+
+describe FactoryBotRails::DefinitionFilePaths do
+  describe "#files" do
+    it "returns a list of definition files" do
+      definition_file_paths = ["definition_path", "another_definition_path"]
+
+      files = described_class.new(definition_file_paths).files
+
+      expect(files).to eq ["definition_path.rb", "another_definition_path.rb"]
+    end
+  end
+
+  describe "#directories" do
+    it "returns a hash of definition directories" do
+      definition_file_paths = ["definition_path", "another_definition_path"]
+
+      directories = described_class.new(definition_file_paths).directories
+
+      expect(directories).to eq(
+        "definition_path" => [:rb],
+        "another_definition_path" => [:rb],
+      )
+    end
+
+    it "converts Pathname objects to strings" do
+      definition_file_paths = [Pathname.new("definition_path")]
+
+      directories = described_class.new(definition_file_paths).directories
+
+      expect(directories).to eq("definition_path" => [:rb])
+    end
+  end
+end
diff --git a/spec/factory_bot_rails/railtie_spec.rb b/spec/factory_bot_rails/railtie_spec.rb
new file mode 100644
index 0000000..b03e680
--- /dev/null
+++ b/spec/factory_bot_rails/railtie_spec.rb
@@ -0,0 +1,57 @@
+# frozen_string_literal: true
+
+describe FactoryBotRails::Railtie do
+  describe "application reloading" do
+    context "when a definition file has been updated" do
+      it "reloads the factory definitions" do
+        allow(FactoryBot).to receive(:reload)
+
+        touch("factories.rb")
+        reload_rails!
+
+        expect(FactoryBot).to have_received(:reload)
+      end
+    end
+
+    context "when a file in a definition directory has been updated" do
+      it "reloads the factory definitions" do
+        allow(FactoryBot).to receive(:reload)
+
+        touch("factories/definitions.rb")
+        reload_rails!
+
+        expect(FactoryBot).to have_received(:reload)
+      end
+    end
+
+    context "when the factory definitions have NOT been updated" do
+      it "does NOT reload the factory definitions" do
+        allow(FactoryBot).to receive(:reload)
+
+        reload_rails!
+
+        expect(FactoryBot).not_to have_received(:reload)
+      end
+    end
+
+    def touch(file)
+      FileUtils.touch(Rails.root.join(file))
+    end
+
+    def reload_rails!
+      if defined? ActiveSupport::Reloader
+        Rails.application.reloader.reload!
+      else
+        # For Rails 4
+        ActionDispatch::Reloader.cleanup!
+        ActionDispatch::Reloader.prepare!
+      end
+
+      wait_for_rails_to_reload
+    end
+
+    def wait_for_rails_to_reload
+      sleep 0.01
+    end
+  end
+end
diff --git a/spec/fake_app.rb b/spec/fake_app.rb
new file mode 100644
index 0000000..37e7b2a
--- /dev/null
+++ b/spec/fake_app.rb
@@ -0,0 +1,12 @@
+# frozen_string_literal: true
+
+module Dummy
+  class Application < Rails::Application
+    config.eager_load = false
+    config.root = "spec/fixtures"
+  end
+end
+
+Rails.logger = Logger.new("/dev/null")
+
+Rails.application.initialize!
diff --git a/spec/fixtures/factories.rb b/spec/fixtures/factories.rb
new file mode 100644
index 0000000..82e878d
--- /dev/null
+++ b/spec/fixtures/factories.rb
@@ -0,0 +1 @@
+# Rails.root.join('factories.rb')
diff --git a/spec/fixtures/factories/definitions.rb b/spec/fixtures/factories/definitions.rb
new file mode 100644
index 0000000..37149d4
--- /dev/null
+++ b/spec/fixtures/factories/definitions.rb
@@ -0,0 +1 @@
+# Rails.root.join('factories', 'definitions.rb')
diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
new file mode 100644
index 0000000..5cbcde8
--- /dev/null
+++ b/spec/spec_helper.rb
@@ -0,0 +1,12 @@
+# frozen_string_literal: true
+
+ENV["RAILS_ENV"] = "test"
+
+require "factory_bot_rails"
+require "fake_app"
+
+RSpec.configure do |config|
+  config.run_all_when_everything_filtered = true
+  config.filter_run :focus
+  config.order = "random"
+end