From c97a68864ab20b1011637e8ee276015d6fa3de57 Mon Sep 17 00:00:00 2001 From: Thiago Araujo Date: Thu, 30 Apr 2026 21:33:51 -0600 Subject: [PATCH 1/5] fix: lazy loading This fix initializes lazy loading or eager loading after the first time a generator is called, which can be a bit surprising. But this guarantees that the `Faker::Config.lazy_loading` setting is respected. --- lib/faker.rb | 82 +++++++++++++++++++++++++++++++--------------------- 1 file changed, 49 insertions(+), 33 deletions(-) diff --git a/lib/faker.rb b/lib/faker.rb index a5d7d472b9..f88fd2685b 100644 --- a/lib/faker.rb +++ b/lib/faker.rb @@ -288,46 +288,62 @@ 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('/') + 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 + + # TODO: refactor this + + def self.resolve_const(context_name, class_name) + load_path = case class_name + when :DnD + load_path('faker/games/dnd') + else + load_path(context_name, class_name) + end + + begin + require(load_path) + rescue LoadError + require(load_path.gsub('faker/', 'faker/default/')) end + end - 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 + private_class_method :resolve_const - const_get(class_name) + EAGER_LOAD_MUTEX = Mutex.new + private_constant :EAGER_LOAD_MUTEX + + # initial usage determines lazy loading or eager loading + # TODO: this can be a bit surprising and error-prone + def self.const_missing(class_name) + EAGER_LOAD_MUTEX.synchronize do + if Config.lazy_loading? + resolve_const(name, class_name) + elsif !@eager_load + @eager_load = true + Dir.glob(["#{__dir__}/faker/*.rb", "#{__dir__}/faker/**/*.rb"]).each { |f| require f } 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') + def self.lazy_load(klass) + mutex = Mutex.new + + klass.define_singleton_method(:const_missing) do |class_name| + mutex.synchronize { Faker.resolve_const(name, class_name) } - Dir.glob(rb_files).each { |file| require file } + const_get(class_name) + end + end end From d754091476e5e52be46c85211ba2bb379a766f5f Mon Sep 17 00:00:00 2001 From: Thiago Araujo Date: Thu, 30 Apr 2026 21:41:17 -0600 Subject: [PATCH 2/5] fix: private method call Fix this failure when lazy loading: https://github.com/faker-ruby/faker/actions/runs/25201119276/job/73892235147?pr=3256 --- lib/faker.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/faker.rb b/lib/faker.rb index f88fd2685b..a0a28f4251 100644 --- a/lib/faker.rb +++ b/lib/faker.rb @@ -317,8 +317,6 @@ def self.resolve_const(context_name, class_name) end end - private_class_method :resolve_const - EAGER_LOAD_MUTEX = Mutex.new private_constant :EAGER_LOAD_MUTEX From 7def9ec7cd5a4979afa00f9da5202e3aed3aefbb Mon Sep 17 00:00:00 2001 From: Thiago Araujo Date: Sat, 9 May 2026 14:55:00 -0600 Subject: [PATCH 3/5] refactor: extract loading strategies to new class --- lib/faker.rb | 71 ++++++++++----------------------------------- lib/faker/loader.rb | 70 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 85 insertions(+), 56 deletions(-) create mode 100644 lib/faker/loader.rb diff --git a/lib/faker.rb b/lib/faker.rb index a0a28f4251..7b644df29c 100644 --- a/lib/faker.rb +++ b/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,60 +287,20 @@ def disable_enforce_available_locales end end - 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 - - # TODO: refactor this - - def self.resolve_const(context_name, class_name) - load_path = case class_name - when :DnD - load_path('faker/games/dnd') - else - load_path(context_name, class_name) - end + @loader = Loader.new(__dir__, Config) - begin - require(load_path) - rescue LoadError - require(load_path.gsub('faker/', 'faker/default/')) - end - end - - EAGER_LOAD_MUTEX = Mutex.new - private_constant :EAGER_LOAD_MUTEX - - # initial usage determines lazy loading or eager loading - # TODO: this can be a bit surprising and error-prone + # 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) - EAGER_LOAD_MUTEX.synchronize do - if Config.lazy_loading? - resolve_const(name, class_name) - elsif !@eager_load - @eager_load = true - Dir.glob(["#{__dir__}/faker/*.rb", "#{__dir__}/faker/**/*.rb"]).each { |f| require f } - end - end + @loader.load_const(name, class_name) const_get(class_name) end def self.lazy_load(klass) - mutex = Mutex.new - - klass.define_singleton_method(:const_missing) do |class_name| - mutex.synchronize { Faker.resolve_const(name, class_name) } - - const_get(class_name) - end + @loader.install_on(klass) end end diff --git a/lib/faker/loader.rb b/lib/faker/loader.rb new file mode 100644 index 0000000000..df0ded1749 --- /dev/null +++ b/lib/faker/loader.rb @@ -0,0 +1,70 @@ +# 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 lazy_loading? + 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 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 + + Dir.glob(["#{@base_dir}/faker/*.rb", "#{@base_dir}/faker/**/*.rb"]) + .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 + + def lazy_loading? + @lazy_loading ||= @config.lazy_loading? + end + end +end From f1db97830b1c17cc4e4a3837457d94a6ea274702 Mon Sep 17 00:00:00 2001 From: Thiago Araujo Date: Sun, 31 May 2026 17:52:17 -0600 Subject: [PATCH 4/5] test: `Faker::Loader` tests --- lib/faker/loader.rb | 30 ++++-- test/faker/test_loader.rb | 146 ++++++++++++++++++++++++++ test/fixtures/faker/default/widget.rb | 10 ++ test/fixtures/faker/gadget.rb | 8 ++ test/fixtures/faker/games/dnd.rb | 10 ++ 5 files changed, 193 insertions(+), 11 deletions(-) create mode 100644 test/faker/test_loader.rb create mode 100644 test/fixtures/faker/default/widget.rb create mode 100644 test/fixtures/faker/gadget.rb create mode 100644 test/fixtures/faker/games/dnd.rb diff --git a/lib/faker/loader.rb b/lib/faker/loader.rb index df0ded1749..3397973968 100644 --- a/lib/faker/loader.rb +++ b/lib/faker/loader.rb @@ -13,7 +13,7 @@ def initialize(base_dir, config) def load_const(context_name, class_name) @mutex.synchronize do - if lazy_loading? + if loading_strategy == :lazy resolve_const(context_name, class_name) else eager_load! @@ -31,24 +31,36 @@ def install_on(klass) end end + def loading_strategy + @loading_strategy ||= if @config.lazy_loading? + :lazy + else + :eager + end + end + + private + def resolve_const(context_name, class_name) load_path = build_path(context_name, class_name) - require(load_path) + require load_path rescue LoadError # try to load default generators - require(load_path.gsub('faker/', 'faker/default/')) + require load_path.gsub('faker/', 'faker/default/') end - private - def eager_load! return if @eager_loaded @eager_loaded = true - Dir.glob(["#{@base_dir}/faker/*.rb", "#{@base_dir}/faker/**/*.rb"]) - .each { |f| require f } + paths = [ + "#{@base_dir}/faker/*.rb", + "#{@base_dir}/faker/**/*.rb" + ] + + Dir.glob(paths).uniq.each { |f| require f } end def build_path(*constants) @@ -62,9 +74,5 @@ def build_path(*constants) .downcase end.join('/') end - - def lazy_loading? - @lazy_loading ||= @config.lazy_loading? - end end end diff --git a/test/faker/test_loader.rb b/test/faker/test_loader.rb new file mode 100644 index 0000000000..6e3eb6ab4a --- /dev/null +++ b/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] + 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] + 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 diff --git a/test/fixtures/faker/default/widget.rb b/test/fixtures/faker/default/widget.rb new file mode 100644 index 0000000000..b94d0deed0 --- /dev/null +++ b/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 diff --git a/test/fixtures/faker/gadget.rb b/test/fixtures/faker/gadget.rb new file mode 100644 index 0000000000..fade2a51f0 --- /dev/null +++ b/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 diff --git a/test/fixtures/faker/games/dnd.rb b/test/fixtures/faker/games/dnd.rb new file mode 100644 index 0000000000..92cd42db50 --- /dev/null +++ b/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 From e00e385450481b0057f9764f40c87056215b5dbc Mon Sep 17 00:00:00 2001 From: Thiago Araujo Date: Sun, 31 May 2026 19:42:17 -0600 Subject: [PATCH 5/5] fix: private method error on `resolve_const` --- lib/faker/loader.rb | 4 ++-- test/test_faker.rb | 8 ++++++++ 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/lib/faker/loader.rb b/lib/faker/loader.rb index 3397973968..ab914efead 100644 --- a/lib/faker/loader.rb +++ b/lib/faker/loader.rb @@ -39,8 +39,6 @@ def loading_strategy end end - private - def resolve_const(context_name, class_name) load_path = build_path(context_name, class_name) @@ -50,6 +48,8 @@ def resolve_const(context_name, class_name) require load_path.gsub('faker/', 'faker/default/') end + private + def eager_load! return if @eager_loaded diff --git a/test/test_faker.rb b/test/test_faker.rb index dc564432ac..85d5d38442 100644 --- a/test/test_faker.rb +++ b/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