fix: Faker::Config.lazy_loading configuration must be respected (WIP)

lib/faker.rb

@@ -1,5 +1,7 @@
 # frozen_string_literal: true
 
+require_relative 'faker/loader'
+
 mydir = __dir__
 
 require 'psych'
@@ -14,9 +16,10 @@
 module Faker
   module Config
     @default_locale = nil
+    @lazy_loading = false
 
     class << self
-      attr_writer :default_locale
+      attr_writer :default_locale, :lazy_loading
 
       def locale=(new_locale)
         Thread.current[:faker_config_locale] = new_locale
@@ -40,16 +43,12 @@ def random
       end
 
       def lazy_loading?
-        if ENV.key?('FAKER_LAZY_LOAD') && !ENV['FAKER_LAZY_LOAD'].nil?
-          %w[true TRUE 1].include?(ENV.fetch('FAKER_LAZY_LOAD', nil))
+        if ENV.key?('FAKER_LAZY_LOAD')
+          %w[true TRUE 1].include?(ENV['FAKER_LAZY_LOAD'])
         else
-          Thread.current[:faker_lazy_loading] == true
+          @lazy_loading
         end
       end
-
-      def lazy_loading=(value)
-        Thread.current[:faker_lazy_loading] = value
-      end
     end
   end
 
@@ -288,46 +287,20 @@ def disable_enforce_available_locales
     end
   end
 
-  if Faker::Config.lazy_loading?
-    def self.load_path(*constants)
-      constants.map do |class_name|
-        class_name
-          .to_s
-          .gsub('::', '/')
-          .gsub(/([A-Z]+)([A-Z][a-z])/, '\1_\2')
-          .gsub(/([a-z\d])([A-Z])/, '\1_\2')
-          .tr('-', '_')
-          .downcase
-      end.join('/')
-    end
+  @loader = Loader.new(__dir__, Config)
 
-    def self.lazy_load(klass)
-      def klass.const_missing(class_name)
-        load_path = case class_name
-                    when :DnD
-                      Faker.load_path('faker/games/dnd')
-                    else
-                      Faker.load_path(name, class_name)
-                    end
-
-        begin
-          require(load_path)
-        rescue LoadError
-          require(load_path.gsub('faker/', 'faker/default/'))
-        end
+  # Resolves missing constants by either lazy or eager loading generator files,
+  # depending on +Config.lazy_loading?+ at the time of first use.
+  #
+  # The loading strategy is determined on the first access. Setting
+  # +Config.lazy_loading+ after any generator has been referenced has no effect.
+  def self.const_missing(class_name)
+    @loader.load_const(name, class_name)
 
-        const_get(class_name)
-      end
-    end
-
-    lazy_load(self)
+    const_get(class_name)
   end
-end
 
-unless Faker::Config.lazy_loading?
-  rb_files = []
-  rb_files << File.join(mydir, 'faker', '*.rb')
-  rb_files << File.join(mydir, 'faker', '/**/*.rb')
-
-  Dir.glob(rb_files).each { |file| require file }
+  def self.lazy_load(klass)
+    @loader.install_on(klass)
+  end
 end

lib/faker/loader.rb

@@ -0,0 +1,78 @@
+# frozen_string_literal: true
+
+module Faker
+  class Loader
+    INFLECTIONS = { 'DnD' => 'dnd' }.freeze
+
+    def initialize(base_dir, config)
+      @base_dir = base_dir
+      @config = config
+      @eager_loaded = false
+      @mutex = Mutex.new
+    end
+
+    def load_const(context_name, class_name)
+      @mutex.synchronize do
+        if loading_strategy == :lazy
+          resolve_const(context_name, class_name)
+        else
+          eager_load!
+        end
+      end
+    end
+
+    def install_on(klass)
+      loader = self
+
+      klass.define_singleton_method(:const_missing) do |class_name|
+        loader.resolve_const(name, class_name)
+
+        const_get(class_name)
+      end
+    end
+
+    def loading_strategy
+      @loading_strategy ||= if @config.lazy_loading?
+                              :lazy
+                            else
+                              :eager
+                            end
+    end
+
+    def resolve_const(context_name, class_name)
+      load_path = build_path(context_name, class_name)
+
+      require load_path
+    rescue LoadError
+      # try to load default generators
+      require load_path.gsub('faker/', 'faker/default/')
+    end
+
+    private
+
+    def eager_load!
+      return if @eager_loaded
+
+      @eager_loaded = true
+
+      paths = [
+        "#{@base_dir}/faker/*.rb",
+        "#{@base_dir}/faker/**/*.rb"
+      ]
+
+      Dir.glob(paths).uniq.each { |f| require f }
+    end
+
+    def build_path(*constants)
+      constants.map do |c|
+        INFLECTIONS
+          .reduce(c.to_s) { |s, (word, replacement)| s.gsub(word, replacement) }
+          .gsub('::', '/')
+          .gsub(/([A-Z]+)([A-Z][a-z])/, '\1_\2')
+          .gsub(/([a-z\d])([A-Z])/, '\1_\2')
+          .tr('-', '_')
+          .downcase
+      end.join('/')
+    end
+  end
+end

test/faker/test_loader.rb

@@ -0,0 +1,146 @@
+# frozen_string_literal: true
+
+require_relative '../test_helper'
+
+class TestLoader < Test::Unit::TestCase
+  FIXTURES_DIR = File.expand_path('../fixtures', __dir__)
+
+  FakeConfig = Struct.new(:lazy_loading?)
+
+  def eager_loader = Faker::Loader.new(FIXTURES_DIR, FakeConfig.new(false))
+  def lazy_loader = Faker::Loader.new(FIXTURES_DIR, FakeConfig.new(true))
+
+  def with_require_spy(fail_if: nil)
+    original = Kernel.instance_method(:require)
+    loaded_files = []
+    mutex = Mutex.new
+
+    Kernel.define_method(:require) do |f|
+      if fail_if&.call(f)
+        raise LoadError
+      end
+
+      mutex.synchronize { loaded_files << f }
+    end
+
+    yield loaded_files
+  ensure
+    Kernel.define_method(:require, original)
+  end
+
+  def test_strategy_does_not_change_after_first_use
+    config = FakeConfig.new(false)
+    loader = Faker::Loader.new(FIXTURES_DIR, config)
+
+    with_require_spy do
+      loader.load_const('Faker', :Gadget)
+
+      config[:lazy_loading?] = true
+
+      assert_equal :eager, loader.loading_strategy
+    end
+  end
+
+  def test_falls_back_to_default_path_on_load_error
+    loader = lazy_loader
+
+    fail_when_requiring_non_default_path = ->(f) { f.end_with?('faker/gadget') }
+
+    with_require_spy(fail_if: fail_when_requiring_non_default_path) do |loaded_files|
+      loader.load_const('Faker', :Gadget)
+
+      assert loaded_files.any? { |f| f.include?('faker/default/gadget') }
+    end
+  end
+
+  def test_inflection_resolves_correctly
+    loader = lazy_loader
+
+    with_require_spy do |loaded_files|
+      loader.load_const('Faker::Games', :DnD)
+
+      assert_includes loaded_files.first, 'faker/games/dnd'
+      refute_includes loaded_files.first, 'dn_d'
+    end
+  end
+
+  def test_install_on_installs_const_missing
+    loader = lazy_loader
+    klass = Class.new
+
+    loader.install_on(klass)
+
+    assert_equal klass.singleton_class, klass.method(:const_missing).owner
+  end
+
+  def test_eager_loads_all_files_on_first_const_access
+    loader = eager_loader
+
+    with_require_spy do |loaded_files|
+      loader.load_const('Faker', :Gadget)
+
+      actual_files = loaded_files.map do |loaded|
+        loaded.match(/fixtures\/(?<path>.*)/)[:path]
+      end.uniq
+
+      expected_files = %w[
+        faker/gadget.rb
+        faker/default/widget.rb
+        faker/games/dnd.rb
+      ]
+
+      expected_files.each do |file|
+        assert_includes actual_files, file, "expected #{file} to be loaded"
+      end
+    end
+  end
+
+  def test_eager_loads_only_once
+    loader = eager_loader
+
+    with_require_spy do |loaded_files|
+      loader.load_const('Faker', :Gadget)
+
+      count = loaded_files.size
+
+      loader.load_const('Faker', :Gadget)
+
+      assert_equal count, loaded_files.size
+    end
+  end
+
+  def test_lazy_loads_single_file_on_const_access
+    loader = lazy_loader
+
+    with_require_spy do |loaded_files|
+      loader.load_const('Faker', :Gadget)
+
+      assert_equal 1, loaded_files.size
+      assert_includes loaded_files.first, 'faker/gadget'
+    end
+  end
+
+  def test_eager_loads_only_once_across_threads
+    loader = eager_loader
+
+    with_require_spy do |loaded_files|
+      threads = 10.times.map do
+        Thread.new { loader.load_const('Faker', :Gadget) }
+      end
+
+      threads.each(&:join)
+
+      actual_files = loaded_files.map do |loaded|
+        loaded.match(/fixtures\/(?<path>.*)/)[:path]
+      end.compact
+
+      assert_equal actual_files.uniq, actual_files
+    end
+  end
+
+  def test_raises_on_unknown_const
+    loader = lazy_loader
+
+    assert_raises(LoadError) { loader.load_const('Faker', :NonExistent) }
+  end
+end

test/fixtures/faker/default/widget.rb

@@ -0,0 +1,10 @@
+# frozen_string_literal: true
+
+module Faker
+  class Default
+    # rubocop:disable Lint/EmptyClass
+    class Widget
+    end
+    # rubocop:enable Lint/EmptyClass
+  end
+end

test/fixtures/faker/gadget.rb

@@ -0,0 +1,8 @@
+# frozen_string_literal: true
+
+module Faker
+  # rubocop:disable Lint/EmptyClass
+  class Gadget
+  end
+  # rubocop:enable Lint/EmptyClass
+end

test/fixtures/faker/games/dnd.rb

@@ -0,0 +1,10 @@
+# frozen_string_literal: true
+
+module Faker
+  class Games
+    # rubocop:disable Lint/EmptyClass
+    class DnD
+    end
+    # rubocop:enable Lint/EmptyClass
+  end
+end

test/test_faker.rb

@@ -92,6 +92,8 @@ def test_lazy_loading_with_config
       Faker::Config.lazy_loading = false
 
       refute_predicate Faker::Config, :lazy_loading?
+
+      assert Faker::Books::Dune.character.is_a? String
     end
   end
 
@@ -102,6 +104,8 @@ def test_lazy_loading_with_env
       Faker::Config.lazy_loading = false
 
       assert_predicate Faker::Config, :lazy_loading?
+
+      assert Faker::Books::Dune.character.is_a? String
     end
 
     mock_env('FAKER_LAZY_LOAD' => 'true') do
@@ -110,6 +114,8 @@ def test_lazy_loading_with_env
       Faker::Config.lazy_loading = false
 
       assert_predicate Faker::Config, :lazy_loading?
+
+      assert Faker::Books::Dune.character.is_a? String
     end
 
     mock_env('FAKER_LAZY_LOAD' => 'TRUE') do
@@ -118,6 +124,8 @@ def test_lazy_loading_with_env
       Faker::Config.lazy_loading = false
 
       assert_predicate Faker::Config, :lazy_loading?
+
+      assert Faker::Books::Dune.character.is_a? String
     end
 
     # ignore config if env is set