Skip to content

Commit 9fa2364

Browse files
committed
ManifestRegistrar#finalize! avoids duplicates
This resolves a subtlety introduced by the fix in #286 `Container#finalize!` processes providers first, before moving on to manifest registrations. If a class injects something defined by a manifest file, and this class is instantiated within a provider, this eager-loads the manifest key before `finalize!` reaches the ManifestRegistrar. Moving `ManifestRegistrar#call` to use `load` instead of `require` introduced the possibility of double-registering a key.
1 parent 1c5210d commit 9fa2364

2 files changed

Lines changed: 41 additions & 1 deletion

File tree

lib/dry/system/manifest_registrar.rb

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,13 @@ def initialize(container)
2626

2727
# @api private
2828
def finalize!
29+
eager_loaded = container.keys.each_with_object(Set.new) do |key, set|
30+
set << Identifier.new(key).root_key
31+
end
32+
2933
::Dir[registrations_dir.join(RB_GLOB)].each do |file|
30-
call(Identifier.new(File.basename(file, RB_EXT)))
34+
ident = Identifier.new(File.basename(file, RB_EXT))
35+
call(ident) unless eager_loaded.include?(ident.root_key)
3136
end
3237
end
3338

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
# frozen_string_literal: true
2+
3+
RSpec.describe "Providers / Eager-loading a manifest-registered dependency" do
4+
before :context do
5+
@dir = make_tmp_directory
6+
7+
with_directory(@dir) do
8+
write "system/registrations/deps.rb", <<~RUBY
9+
Test::Container.namespace(:deps) do |container|
10+
container.register(:thing) { Object.new }
11+
end
12+
RUBY
13+
end
14+
end
15+
16+
before do
17+
root = @dir
18+
Test::Container = Class.new(Dry::System::Container) do
19+
configure do |config|
20+
config.root = root
21+
end
22+
23+
register_provider :my_provider do
24+
start do
25+
target["deps.thing"]
26+
end
27+
end
28+
end
29+
end
30+
31+
it "does not register the manifest dependency more than once" do
32+
expect { Test::Container.finalize! }.not_to raise_error
33+
expect(Test::Container["deps.thing"]).to be
34+
end
35+
end

0 commit comments

Comments
 (0)