Skip to content

Commit 4a83973

Browse files
ali-grahamali-graham
committed
fix resource linkage in JSONAPI base POST/PATCH requests
1 parent 1f63d3d commit 4a83973

File tree

9 files changed

+144
-65
lines changed

9 files changed

+144
-65
lines changed

‎lib/flapjack/gateways/jsonapi/helpers/miscellaneous.rb‎

Lines changed: 19 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -74,51 +74,30 @@ def validate_data(data, options = {})
7474
halt(err(403, "Link(s) must be a Hash with String keys")) unless links.is_a?(Hash) &&
7575
links.keys.all? {|k| k.is_a?(String)}
7676
invalid_links = links.keys - all_links
77-
halt(err(404, "Link(s) not found: #{invalid_links.join(', ')}")) unless invalid_links.empty?
77+
halt(err(404, "Unknown link type(s): #{invalid_links.join(', ')}")) unless invalid_links.empty?
7878
links.each_pair do |k, v|
79+
halt(err(403, "Link data for '#{k}' must be a Hash with String keys")) unless v.is_a?(Hash) && v.keys.all? {|vk| vk.is_a?(String)}
80+
halt(err(403, "Link data for '#{k}' must have linkage references")) unless links[k].has_key?('linkage')
7981

8082
if sl.include?(k)
81-
unless v.nil?
82-
if !v['id'].nil? && !v['type'].nil?
83-
unless is_link_type?(k, v['type'], :klass => klass)
84-
halt(err(403, "Linked '#{k}' has wrong type #{v['type']}"))
85-
end
86-
87-
unless v['id'].is_a?(String)
88-
halt(err(403, "Linked '#{k}' 'id' must be a String" ))
89-
end
90-
else
91-
halt(err(403, "Linked '#{k}' must have 'id' and 'type' fields"))
92-
end
93-
end
94-
elsif !v.is_a?(Hash)
95-
halt(err(403, "Linked '#{k}' must be a Hash"))
96-
# # Flapjack does not support heterogenous to_many types
97-
# elsif !v['data'].nil?
98-
# if v['data'].is_a?(Array) && !v['data'].empty?
99-
# halt(err(403, "Linked '#{k}' 'data' Array objects must all have 'id' and 'type' fields")) unless v['data'].all? {|vv|
100-
# vv.has_key?('id') && vv['id'].is_a?(String) &&
101-
# vv.has_key?('type') && vv['type'].is_a?(String)
102-
# }
103-
104-
# bad_type = v['data'].detect {|vv| !is_link_type?(k, vv['type'], :klass => klass)}
105-
106-
# unless bad_type.nil?
107-
# halt(err(403, "Linked '#{k}' 'data' Array object has wrong type #{bad_type['type']}"))
108-
# end
109-
# else
110-
# halt(err(403, "Linked '#{k}' 'data' must be an Array"))
111-
# end
112-
elsif !v['id'].nil? && !v['type'].nil?
113-
unless is_link_type?(k, v['type'], :klass => klass)
114-
halt(err(403, "Linked '#{k}' has wrong type #{v['type']}"))
115-
end
116-
117-
unless v['id'].is_a?(Array) && v['id'].all? {|vv| vv.is_a?(String)}
118-
halt(err(403, "Linked '#{k}' 'id' must be an Array of Strings"))
83+
halt(err(403, "Linkage reference for '#{k}' must be a Hash with 'id' & 'type' String values")) unless links[k]['linkage'].is_a?(Hash) &&
84+
v['linkage'].has_key?('id') &&
85+
v['linkage'].has_key?('type') &&
86+
v['linkage']['id'].is_a?(String) &&
87+
v['linkage']['type'].is_a?(String)
88+
89+
unless is_link_type?(k, v['linkage']['type'], :klass => klass)
90+
halt(err(403, "Linked '#{k}' has wrong type #{v['linkage']['type']}"))
11991
end
12092
else
121-
halt(err(403, "Linked '#{k}' must have 'id' and 'type' fields, or a 'data' Array"))
93+
halt(err(403, "Linkage reference for '#{k}' must be an Array of Hashes with 'id' & 'type' String values")) unless v['linkage'].is_a?(Array) &&
94+
v['linkage'].all? {|l| l.has_key?('id') } &&
95+
v['linkage'].all? {|l| l.has_key?('type') } &&
96+
v['linkage'].all? {|l| l['id'].is_a?(String) } &&
97+
v['linkage'].all? {|l| l['type'].is_a?(String) }
98+
99+
bad_type = v['linkage'].detect {|l| !is_link_type?(k, l['type'], :klass => klass)}
100+
halt(err(403, "Linked '#{k}' has wrong type #{bad_type['type']}")) unless bad_type.nil?
122101
end
123102
end
124103
end

‎lib/flapjack/gateways/jsonapi/helpers/resources.rb‎

Lines changed: 48 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,13 @@ def initialize(attrs = {})
2121
def resource_post(klass, resources_name, options = {})
2222
resources_data, unwrap = wrapped_params
2323

24+
singular_links, multiple_links = klass.association_klasses
2425
attributes = klass.respond_to?(:jsonapi_attributes) ?
2526
klass.jsonapi_attributes : []
26-
27-
validate_data(resources_data, :attributes => attributes, :klass => klass)
27+
validate_data(resources_data, :attributes => attributes,
28+
:singular_links => singular_links.keys,
29+
:multiple_links => multiple_links.keys,
30+
:klass => klass)
2831

2932
resources = nil
3033

@@ -34,11 +37,21 @@ def resource_post(klass, resources_name, options = {})
3437
data_ids = resources_data.reject {|d| d[idf.to_s].nil? }.
3538
map {|i| i[idf.to_s].to_s }
3639

40+
assoc_klasses = singular_links.values.inject([]) {|memo, slv|
41+
memo << slv[:data]
42+
memo += slv[:related]
43+
memo
44+
} | multiple_links.values.inject([]) {|memo, mlv|
45+
memo << mlv[:data]
46+
memo += mlv[:related]
47+
memo
48+
}
49+
3750
attribute_types = klass.attribute_types
3851

3952
jsonapi_type = klass.jsonapi_type
4053

41-
klass.lock do
54+
klass.lock(*assoc_klasses) do
4255
unless data_ids.empty?
4356
conflicted_ids = klass.intersect(idf => data_ids).ids
4457
halt(err(409, "#{klass.name.split('::').last.pluralize} already exist with the following #{idf}s: " +
@@ -55,8 +68,37 @@ def resource_post(klass, resources_name, options = {})
5568
memo[r] = rd['links']
5669
end
5770

71+
# get linked objects, fail before save if we don't find them
72+
resource_links = links_by_resource.each_with_object({}) do |(r, links), memo|
73+
next if links.nil?
74+
75+
singular_links.each_pair do |assoc, assoc_klass|
76+
next unless links.has_key?(assoc.to_s)
77+
memo[r.object_id] ||= {}
78+
memo[r.object_id][assoc.to_s] = assoc_klass[:data].find_by_id!(links[assoc.to_s]['linkage']['id'])
79+
end
80+
81+
multiple_links.each_pair do |assoc, assoc_klass|
82+
next unless links.has_key?(assoc.to_s)
83+
memo[r.object_id] ||= {}
84+
memo[r.object_id][assoc.to_s] = assoc_klass[:data].find_by_ids!(*(links[assoc.to_s]['linkage'].map {|l| l['id']}))
85+
end
86+
end
87+
88+
links_by_resource.keys.each do |r|
89+
r.save
90+
rl = resource_links[r.object_id]
91+
next if rl.nil?
92+
rl.each_pair do |assoc, value|
93+
case value
94+
when Array
95+
r.send(assoc.to_sym).add(*value)
96+
else
97+
r.send("#{assoc}=".to_sym, value)
98+
end
99+
end
100+
end
58101
resources = links_by_resource.keys
59-
resources.each {|r| r.save }
60102
end
61103

62104
resource_ids = resources.map(&:id)
@@ -155,15 +197,15 @@ def resource_patch(klass, resources_name, id, options = {})
155197
singular_links.each_pair do |assoc, assoc_klass|
156198
next unless links.has_key?(assoc.to_s)
157199
memo[r.id] ||= {}
158-
memo[r.id][assoc.to_s] = assoc_klass[:data].find_by_id!(links[assoc.to_s]['id'])
200+
memo[r.id][assoc.to_s] = assoc_klass[:data].find_by_id!(links[assoc.to_s]['linkage']['id'])
159201
end
160202

161203
multiple_links.each_pair do |assoc, assoc_klass|
162204
next unless links.has_key?(assoc.to_s)
163205
current_assoc_ids = r.send(assoc.to_sym).ids
164206
memo[r.id] ||= {}
165207

166-
link_ids = links[assoc.to_s]['id']
208+
link_ids = links[assoc.to_s]['linkage'].map {|l| l['id']}
167209

168210
to_remove = current_assoc_ids - link_ids
169211
to_add = link_ids - current_assoc_ids

‎spec/lib/flapjack/gateways/jsonapi/methods/checks_spec.rb‎

Lines changed: 49 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,9 @@
1212
let(:check_presenter) { double(Flapjack::Gateways::JSONAPI::Helpers::CheckPresenter) }
1313

1414
it "creates a check" do
15-
expect(Flapjack::Data::Check).to receive(:lock).with(no_args).and_yield
15+
expect(Flapjack::Data::Check).to receive(:lock).
16+
with(Flapjack::Data::Tag, Flapjack::Data::Rule, Flapjack::Data::Route).
17+
and_yield
1618

1719
empty_ids = double('empty_ids')
1820
expect(empty_ids).to receive(:ids).and_return([])
@@ -44,7 +46,9 @@
4446
end
4547

4648
it 'creates two checks' do
47-
expect(Flapjack::Data::Check).to receive(:lock).and_yield
49+
expect(Flapjack::Data::Check).to receive(:lock).
50+
with(Flapjack::Data::Tag, Flapjack::Data::Rule, Flapjack::Data::Route).
51+
and_yield
4852

4953
empty_ids = double('empty_ids')
5054
expect(empty_ids).to receive(:ids).and_return([])
@@ -88,6 +92,48 @@
8892
))
8993
end
9094

95+
it 'creates a link to a tag along with a check' do
96+
expect(Flapjack::Data::Check).to receive(:lock).with(Flapjack::Data::Tag,
97+
Flapjack::Data::Rule, Flapjack::Data::Route).and_yield
98+
99+
empty_ids = double('empty_ids')
100+
expect(empty_ids).to receive(:ids).and_return([])
101+
expect(Flapjack::Data::Check).to receive(:intersect).
102+
with(:id => [check_data[:id]]).and_return(empty_ids)
103+
104+
expect(check).to receive(:invalid?).and_return(false)
105+
expect(check).to receive(:save).and_return(true)
106+
expect(Flapjack::Data::Check).to receive(:new).with(check_data).
107+
and_return(check)
108+
109+
expect(Flapjack::Data::Tag).to receive(:find_by_ids!).
110+
with(tag.id).and_return([tag])
111+
112+
tags = double('tags')
113+
expect(tags).to receive(:add).with(tag)
114+
expect(check).to receive(:tags).and_return(tags)
115+
116+
expect(check).to receive(:as_json).with(:only => an_instance_of(Array)).
117+
and_return(check_data)
118+
119+
expect(Flapjack::Data::Check).to receive(:jsonapi_type).and_return('check')
120+
121+
post "/checks", Flapjack.dump_json(:data => check_data.merge(:type => 'check', :links => {
122+
:tags => {:linkage => [:id => tag.id, :type => 'tag']}
123+
})), jsonapi_env
124+
expect(last_response.status).to eq(201)
125+
expect(last_response.body).to be_json_eql(Flapjack.dump_json(:data =>
126+
check_data.merge(
127+
:type => 'check',
128+
:links => {:self => "http://example.org/checks/#{check.id}",
129+
:scheduled_maintenances => "http://example.org/checks/#{check.id}/scheduled_maintenances",
130+
:tags => "http://example.org/checks/#{check.id}/tags",
131+
:unscheduled_maintenances => "http://example.org/checks/#{check.id}/unscheduled_maintenances",
132+
}
133+
)
134+
))
135+
end
136+
91137
it 'rejects a request to create a check with an invalid bulk MIME type' do
92138
post "/checks", Flapjack.dump_json(:data => check_data.merge(:type => 'check')), jsonapi_bulk_env
93139
expect(last_response.status).to eq(406)
@@ -394,7 +440,7 @@
394440

395441
patch "/checks/#{check.id}",
396442
Flapjack.dump_json(:data => {:id => check.id, :type => 'check', :links =>
397-
{:tags => {:type => 'tag', :id => ['database']}}}),
443+
{:tags => {:linkage => [{:type => 'tag', :id => 'database'}]}}}),
398444
jsonapi_env
399445
expect(last_response.status).to eq(204)
400446
end

‎spec/lib/flapjack/gateways/jsonapi/methods/contacts_spec.rb‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010

1111
it "creates a contact" do
1212
expect(Flapjack::Data::Contact).to receive(:lock).
13-
with(no_args).and_yield
13+
with(Flapjack::Data::Medium, Flapjack::Data::Rule).and_yield
1414

1515
empty_ids = double('empty_ids')
1616
expect(empty_ids).to receive(:ids).and_return([])
@@ -39,7 +39,8 @@
3939
end
4040

4141
it "does not create a contact if the data is improperly formatted" do
42-
expect(Flapjack::Data::Contact).to receive(:lock).with(no_args).and_yield
42+
expect(Flapjack::Data::Contact).to receive(:lock).
43+
with(Flapjack::Data::Medium, Flapjack::Data::Rule).and_yield
4344

4445
empty_ids = double('empty_ids')
4546
expect(empty_ids).to receive(:ids).and_return([])

‎spec/lib/flapjack/gateways/jsonapi/methods/media_spec.rb‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212

1313
it "creates a medium" do
1414
expect(Flapjack::Data::Medium).to receive(:lock).
15-
with(no_args).and_yield
15+
with(Flapjack::Data::Contact, Flapjack::Data::Rule).and_yield
1616

1717
empty_ids = double('empty_ids')
1818
expect(empty_ids).to receive(:ids).and_return([])
@@ -42,7 +42,7 @@
4242

4343
it "does not create a medium if the data is improperly formatted" do
4444
expect(Flapjack::Data::Medium).to receive(:lock).
45-
with(no_args).and_yield
45+
with(Flapjack::Data::Contact, Flapjack::Data::Rule).and_yield
4646

4747
empty_ids = double('empty_ids')
4848
expect(empty_ids).to receive(:ids).and_return([])

‎spec/lib/flapjack/gateways/jsonapi/methods/rules_spec.rb‎

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,9 @@
1313
let(:medium) { double(Flapjack::Data::Medium, :id => email_data[:id]) }
1414

1515
it "creates a rule" do
16-
expect(Flapjack::Data::Rule).to receive(:lock).with(no_args).and_yield
16+
expect(Flapjack::Data::Rule).to receive(:lock).with(Flapjack::Data::Contact,
17+
Flapjack::Data::Medium, Flapjack::Data::Tag, Flapjack::Data::Check,
18+
Flapjack::Data::Route).and_yield
1719

1820
empty_ids = double('empty_ids')
1921
expect(empty_ids).to receive(:ids).and_return([])
@@ -40,7 +42,9 @@
4042
end
4143

4244
it "does not create a rule if the data is improperly formatted" do
43-
expect(Flapjack::Data::Rule).to receive(:lock).with(no_args).and_yield
45+
expect(Flapjack::Data::Rule).to receive(:lock).with(Flapjack::Data::Contact,
46+
Flapjack::Data::Medium, Flapjack::Data::Tag, Flapjack::Data::Check,
47+
Flapjack::Data::Route).and_yield
4448
empty_ids = double('empty_ids')
4549
expect(empty_ids).to receive(:ids).and_return([])
4650
expect(Flapjack::Data::Rule).to receive(:intersect).
@@ -286,7 +290,7 @@
286290

287291
patch "/rules/#{rule.id}",
288292
Flapjack.dump_json(:data => {:id => rule.id, :type => 'rule', :links =>
289-
{:contact => {:type => 'contact', :id => contact.id}}}),
293+
{:contact => {:linkage => {:type => 'contact', :id => contact.id}}}}),
290294
jsonapi_env
291295
expect(last_response.status).to eq(204)
292296
end
@@ -310,10 +314,11 @@
310314
patch "/rules",
311315
Flapjack.dump_json(:data => [
312316
{:id => rule.id, :type => 'rule', :links =>
313-
{:contact => {:type => 'contact', :id => contact.id}}},
317+
{:contact => {:linkage => {:type => 'contact', :id => contact.id}}}
318+
},
314319
{:id => rule_2.id, :type => 'rule', :links =>
315-
{:contact => {:type => 'contact', :id => contact.id}}}
316-
]),
320+
{:contact => {:linkage => {:type => 'contact', :id => contact.id}}}
321+
}]),
317322
jsonapi_bulk_env
318323
expect(last_response.status).to eq(204)
319324
end
@@ -326,7 +331,8 @@
326331
patch "/rules/#{rule.id}",
327332
Flapjack.dump_json(:data =>
328333
{:id => rule.id, :type => 'rule', :links =>
329-
{:contact => {:type => 'contact', :id => contact.id}}}),
334+
{:contact => {:linkage => {:type => 'contact', :id => contact.id}}}
335+
}),
330336
jsonapi_env
331337
expect(last_response.status).to eq(404)
332338
end

‎spec/lib/flapjack/gateways/jsonapi/methods/scheduled_maintenances_spec.rb‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,8 @@
1212
let(:check) { double(Flapjack::Data::Check, :id => check_data[:id]) }
1313

1414
it "creates a scheduled maintenance period" do
15-
expect(Flapjack::Data::ScheduledMaintenance).to receive(:lock).with(no_args).and_yield
15+
expect(Flapjack::Data::ScheduledMaintenance).to receive(:lock).
16+
with(Flapjack::Data::Check).and_yield
1617

1718
empty_ids = double('empty_ids')
1819
expect(empty_ids).to receive(:ids).and_return([])
@@ -41,7 +42,8 @@
4142
end
4243

4344
it "doesn't create a scheduled maintenance period if the start time isn't passed" do
44-
expect(Flapjack::Data::ScheduledMaintenance).to receive(:lock).with(no_args).and_yield
45+
expect(Flapjack::Data::ScheduledMaintenance).to receive(:lock).
46+
with(Flapjack::Data::Check).and_yield
4547

4648
empty_ids = double('empty_ids')
4749
expect(empty_ids).to receive(:ids).and_return([])

‎spec/lib/flapjack/gateways/jsonapi/methods/tags_spec.rb‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,9 @@
1515
let(:rule) { double(Flapjack::Data::Rule, :id => rule_data[:id]) }
1616

1717
it "creates a tag" do
18-
expect(Flapjack::Data::Tag).to receive(:lock).with(no_args).and_yield
18+
expect(Flapjack::Data::Tag).to receive(:lock).
19+
with(Flapjack::Data::Check, Flapjack::Data::Rule, Flapjack::Data::Route).
20+
and_yield
1921

2022
empty_ids = double('empty_ids')
2123
expect(empty_ids).to receive(:ids).and_return([])
@@ -186,7 +188,7 @@
186188

187189
patch "/tags/#{tag.id}",
188190
Flapjack.dump_json(:data => {:id => tag.id, :type => 'tag', :links =>
189-
{:checks => {:type => 'check', :id => [check.id]}}}),
191+
{:checks => {:linkage => [{:type => 'check', :id => check.id}]}}}),
190192
jsonapi_env
191193
expect(last_response.status).to eq(204)
192194
end

‎spec/lib/flapjack/gateways/jsonapi/methods/unscheduled_maintenances_spec.rb‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,8 @@
1313
let(:check) { double(Flapjack::Data::Check, :id => check_data[:id]) }
1414

1515
it "creates an unscheduled maintenance period" do
16-
expect(Flapjack::Data::UnscheduledMaintenance).to receive(:lock).with(no_args).and_yield
16+
expect(Flapjack::Data::UnscheduledMaintenance).to receive(:lock).
17+
with(Flapjack::Data::Check).and_yield
1718

1819
empty_ids = double('empty_ids')
1920
expect(empty_ids).to receive(:ids).and_return([])

0 commit comments

Comments
 (0)