Skip to content

Commit fbd5525

Browse files
committed
Make all relationships spec fields mandatory
At the moment, we're deciding that if a given relationship doesn't have a `primary_key`, we fallback to `:id`. This can lead to unwanted behaviour and suprises from developers. Let's make it mandatory and ensure everything's present.
1 parent 145ab31 commit fbd5525

File tree

2 files changed

+34
-4
lines changed

2 files changed

+34
-4
lines changed

lib/resource_registry/relationship_type.rb

+21-1
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ module RelationshipType
55
extend T::Sig
66
extend T::Helpers
77

8+
InvalidRelationshipSpec = Class.new(StandardError)
9+
810
abstract!
911

1012
# This enables `is_a?` check type to sorbet
@@ -25,6 +27,7 @@ module RelationshipType
2527
sig { params(spec: T::Hash[String, T.untyped]).void }
2628
def initialize(spec)
2729
@spec = spec
30+
validate_spec!
2831
end
2932

3033
sig { abstract.returns(String) }
@@ -110,7 +113,7 @@ def field
110113

111114
sig { returns(Symbol) }
112115
def primary_key
113-
@spec["primary_key"]&.to_sym || :id
116+
@spec["primary_key"]&.to_sym
114117
end
115118

116119
sig do
@@ -127,5 +130,22 @@ def valid_relationship_field?(read_dto)
127130
sig { overridable.returns(T.nilable(Symbol)) }
128131
def relationship_field_name
129132
end
133+
134+
private
135+
136+
sig { void }
137+
def validate_spec!
138+
errors = []
139+
errors << "name is required" unless @spec["name"]
140+
errors << "resource_id is required" unless @spec["resource_id"]
141+
errors << "field is required" unless @spec["field"]
142+
errors << "primary_key is required" unless @spec["primary_key"]
143+
errors << "name must be a string" unless @spec["name"].is_a?(String)
144+
145+
return if errors.empty?
146+
147+
Kernel.raise InvalidRelationshipSpec,
148+
"The given relationship spec is not valid: #{errors.join(", ")}. The spec is: #{@spec}"
149+
end
130150
end
131151
end

spec/resource_registry/relationship_spec.rb

+13-3
Original file line numberDiff line numberDiff line change
@@ -6,21 +6,31 @@
66

77
RSpec.describe ResourceRegistry::Relationship do
88
describe "#load" do
9-
let(:type) { ResourceRegistry::RelationshipTypes::HasOne.new({}) }
109
let(:params) do
1110
{
1211
"name" => "test",
13-
"type" => type.serialize,
12+
"type" => "has_one",
1413
"field" => :test,
1514
"resource_id" => :test,
16-
"optional" => true
15+
"optional" => true,
16+
"primary_key" => :test
1717
}
1818
end
1919

2020
subject { described_class.load(params) }
2121

2222
it { expect(subject).to be_a(described_class) }
2323

24+
context "when the spec is not valid" do
25+
let(:params) { { "type" => "has_one" } }
26+
27+
it "raises an error" do
28+
expect { subject }.to raise_error(
29+
ResourceRegistry::RelationshipType::InvalidRelationshipSpec
30+
)
31+
end
32+
end
33+
2434
# TODO: Check how to test this
2535
xcontext "with a custom type" do
2636
let(:type) { "policy_resolution" }

0 commit comments

Comments
 (0)