Skip to content

Commit bc94b30

Browse files
samuelgilesclaude
andauthored
Deduplicate loader classes into a single parameterized Loader (#22)
DeprecatedReferencesLoader and PackageTodoLoader were ~95% identical, differing only in the YAML filename they loaded. Replace both with a single Loader class that accepts the filename as a constructor argument. https://claude.ai/code/session_015Ta3iW5HnXWCEcu9SHtjQ6 Co-authored-by: Claude <noreply@anthropic.com>
1 parent 69bf293 commit bc94b30

File tree

10 files changed

+132
-220
lines changed

10 files changed

+132
-220
lines changed

lib/graphwerk.rb

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,7 @@
88
require 'graphwerk/version'
99
require 'graphwerk/constants'
1010
require 'graphwerk/layout'
11-
require 'graphwerk/deprecated_references_loader'
12-
require 'graphwerk/package_todo_loader'
11+
require 'graphwerk/loader'
1312
require 'graphwerk/presenters/package'
1413
require 'graphwerk/builders/graph'
1514
require 'graphwerk/railtie' if defined?(Rails)

lib/graphwerk/deprecated_references_loader.rb

Lines changed: 0 additions & 31 deletions
This file was deleted.

lib/graphwerk/loader.rb

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
# typed: strict
2+
# frozen_string_literal: true
3+
4+
module Graphwerk
5+
class Loader
6+
extend T::Sig
7+
8+
sig { params(package: Packwerk::Package, root_path: Pathname, filename: String).void }
9+
def initialize(package, root_path, filename)
10+
@package = package
11+
@root_path = root_path
12+
@filename = filename
13+
end
14+
15+
sig { returns(T::Array[String]) }
16+
def load
17+
return [] if !file.exist?
18+
19+
(YAML.safe_load_file(file) || {}).keys
20+
end
21+
22+
private
23+
24+
sig { returns(Pathname) }
25+
def file
26+
@file = T.let(@file, T.nilable(Pathname))
27+
@file ||= @root_path.join(@package.name, @filename)
28+
end
29+
end
30+
end

lib/graphwerk/package_todo_loader.rb

Lines changed: 0 additions & 31 deletions
This file was deleted.

lib/graphwerk/presenters/package.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,14 +24,14 @@ def dependencies
2424

2525
sig { returns(T::Array[String]) }
2626
def deprecated_references
27-
DeprecatedReferencesLoader.new(@package, @root_path).load.map do |reference|
27+
Loader.new(@package, @root_path, 'deprecated_references.yml').load.map do |reference|
2828
Name.new(reference).node_name
2929
end
3030
end
3131

3232
sig { returns(T::Array[String]) }
3333
def package_todos
34-
PackageTodoLoader.new(@package, @root_path).load.map do |todo|
34+
Loader.new(@package, @root_path, 'package_todo.yml').load.map do |todo|
3535
Name.new(todo).node_name
3636
end
3737
end

spec/lib/graphwerk/builders/graph_spec.rb

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -50,23 +50,22 @@ module Builders
5050
subject(:diagram) { builder.build.to_s }
5151

5252
let(:deprecated_references_loader_for_images) do
53-
instance_double(DeprecatedReferencesLoader, load: ['.'])
53+
instance_double(Loader, load: ['.'])
5454
end
5555

5656
let(:package_todo_loader_for_frontend) do
57-
instance_double(PackageTodoLoader, load: ['components/storage_providers/s3'])
57+
instance_double(Loader, load: ['components/storage_providers/s3'])
5858
end
5959

6060
before do
61-
allow(DeprecatedReferencesLoader).to receive(:new).and_call_original
62-
allow(DeprecatedReferencesLoader)
61+
allow(Loader).to receive(:new).and_call_original
62+
allow(Loader)
6363
.to receive(:new)
64-
.with(images_package, an_instance_of(Pathname))
64+
.with(images_package, an_instance_of(Pathname), 'deprecated_references.yml')
6565
.and_return(deprecated_references_loader_for_images)
66-
allow(PackageTodoLoader).to receive(:new).and_call_original
67-
allow(PackageTodoLoader)
66+
allow(Loader)
6867
.to receive(:new)
69-
.with(frontend_package, an_instance_of(Pathname))
68+
.with(frontend_package, an_instance_of(Pathname), 'package_todo.yml')
7069
.and_return(package_todo_loader_for_frontend)
7170
end
7271

@@ -130,7 +129,7 @@ module Builders
130129

131130
context 'when a deprecated reference points to a package not in the graph' do
132131
let(:deprecated_references_loader_for_images) do
133-
instance_double(DeprecatedReferencesLoader, load: ['.', 'components/unknown'])
132+
instance_double(Loader, load: ['.', 'components/unknown'])
134133
end
135134

136135
specify do
@@ -148,7 +147,7 @@ module Builders
148147

149148
context 'when a package todo points to a package not in the graph' do
150149
let(:package_todo_loader_for_frontend) do
151-
instance_double(PackageTodoLoader, load: ['components/storage_providers/s3', 'components/nonexistent'])
150+
instance_double(Loader, load: ['components/storage_providers/s3', 'components/nonexistent'])
152151
end
153152

154153
specify do

spec/lib/graphwerk/deprecated_references_loader_spec.rb

Lines changed: 0 additions & 67 deletions
This file was deleted.

spec/lib/graphwerk/loader_spec.rb

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
# typed: false
2+
# frozen_string_literal: true
3+
4+
module Graphwerk
5+
describe Loader do
6+
let(:service) { described_class.new(package, root_path, filename) }
7+
8+
let(:package) do
9+
Packwerk::Package.new(
10+
name: 'components/admin',
11+
config: { 'dependencies' => ['components/security', 'components/orders'] }
12+
)
13+
end
14+
let(:root_path) { Pathname.new('.') }
15+
16+
shared_examples 'a YAML loader' do
17+
describe '#load' do
18+
subject { service.load }
19+
20+
let(:file) { instance_double(Pathname) }
21+
22+
before do
23+
expect(root_path)
24+
.to receive(:join)
25+
.with('components/admin', filename)
26+
.and_return(file)
27+
expect(file)
28+
.to receive(:exist?)
29+
.and_return(file_is_present)
30+
end
31+
32+
context 'when no file is present' do
33+
let(:file_is_present) { false }
34+
35+
it { is_expected.to be_empty }
36+
end
37+
38+
context 'when a file is present' do
39+
let(:file_is_present) { true }
40+
41+
before do
42+
expect(YAML)
43+
.to receive(:safe_load_file)
44+
.with(file)
45+
.and_return(
46+
'.' => {
47+
"::Order" => {
48+
"violations" => ["dependency"],
49+
"files" => ["components/admin/interfaces/gateway.rb"]
50+
}
51+
}
52+
)
53+
end
54+
55+
it { is_expected.to contain_exactly('.') }
56+
end
57+
end
58+
59+
describe '#load with a real YAML fixture file' do
60+
let(:root_path) { Pathname.new(File.expand_path('../../support/fixtures', __dir__)) }
61+
62+
it 'parses the file with safe_load_file and returns the keys' do
63+
expect(service.load).to contain_exactly('.', 'components/shipping')
64+
end
65+
end
66+
end
67+
68+
context 'with deprecated_references.yml' do
69+
let(:filename) { 'deprecated_references.yml' }
70+
71+
it_behaves_like 'a YAML loader'
72+
end
73+
74+
context 'with package_todo.yml' do
75+
let(:filename) { 'package_todo.yml' }
76+
77+
it_behaves_like 'a YAML loader'
78+
end
79+
end
80+
end

spec/lib/graphwerk/package_todo_loader_spec.rb

Lines changed: 0 additions & 67 deletions
This file was deleted.

0 commit comments

Comments
 (0)