Skip to content

Commit 9bbe32e

Browse files
alassektimriley
andauthored
ManifestRegistrar#finalize! avoids duplicates (#295)
Fix a small bug introduced by #286. When a container is finalizing, providers are finalized first. If one of those providers loads a manifest-registered dependency, that manifest will already have been loaded by the time the manifest registrar finalizes. Keep track of loaded manifest files to avoid attempting to load them a second time, which would result in errors around duplicate registrations. --------- Co-authored-by: Tim Riley <tim@riley.id.au>
1 parent 1c5210d commit 9bbe32e

3 files changed

Lines changed: 47 additions & 1 deletion

File tree

CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,10 @@ and this project adheres to [Break Versioning](https://www.taoensso.com/break-ve
77

88
## [Unreleased]
99

10+
### Fixed
11+
12+
- Avoid errors from attempted duplicate registrations when finalizing a container with manifest-registered dependencies that have already been loaded as part of finalizing providers. (@alassek, @timriley in #295)
13+
1014
[Unreleased]: https://github.com/dry-rb/dry-system/compare/v1.2.5...main
1115

1216
## [1.2.5] - 2025-12-01

lib/dry/system/manifest_registrar.rb

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,17 +22,24 @@ class ManifestRegistrar
2222
def initialize(container)
2323
@container = container
2424
@config = container.config
25+
@loaded = Set.new
2526
end
2627

2728
# @api private
2829
def finalize!
2930
::Dir[registrations_dir.join(RB_GLOB)].each do |file|
30-
call(Identifier.new(File.basename(file, RB_EXT)))
31+
ident = Identifier.new(File.basename(file, RB_EXT))
32+
33+
# Skip files already loaded during earlier stages of finalization, such as a provider
34+
# resolving a component satisfied via a manifest. Since `#call` uses `load`, re-running
35+
# here would attempt a duplicate registration and raise an error.
36+
call(ident) unless @loaded.include?(ident.root_key)
3137
end
3238
end
3339

3440
# @api private
3541
def call(component)
42+
@loaded << component.root_key
3643
load(root.join(config.registrations_dir, "#{component.root_key}#{RB_EXT}"))
3744
end
3845

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 / Lazy loading a manifest-registered dependency as part of finalize" 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)