From 31c47b2e218eadb594972e6fcbb01afe7c6896ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Sk=C3=B6ld?= Date: Thu, 2 Apr 2015 11:14:28 +0200 Subject: [PATCH 1/7] Added a few request tests for has_many associations --- test/integration/requests/request_test.rb | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/test/integration/requests/request_test.rb b/test/integration/requests/request_test.rb index 92ee0155d..19aa08859 100644 --- a/test/integration/requests/request_test.rb +++ b/test/integration/requests/request_test.rb @@ -158,7 +158,7 @@ def test_post_single def test_update_association_without_content_type ruby = Section.find_by(name: 'ruby') - patch '/posts/3/links/section', { 'sections' => {type: 'sections', id: ruby.id.to_s }}.to_json + patch '/posts/3/links/section', { 'data' => {type: 'sections', id: ruby.id.to_s }}.to_json assert_equal 415, status end @@ -177,6 +177,27 @@ def test_put_update_association_has_one assert_equal 204, status end + def test_patch_update_association_has_many + like = Comment.find_by(body: 'i liked it') + patch '/posts/3/links/comments', { 'data' => [{type: 'comments', id: like.id.to_s }]}.to_json, "CONTENT_TYPE" => JSONAPI::MEDIA_TYPE + + assert_equal 204, status + end + + def test_post_update_association_has_many + like = Comment.find_by(body: 'i liked it') + post '/posts/3/links/comments', { 'data' => [{type: 'comments', id: like.id.to_s }]}.to_json, "CONTENT_TYPE" => JSONAPI::MEDIA_TYPE + + assert_equal 204, status + end + + def test_put_update_association_has_many + like = Comment.find_by(body: 'i liked it') + put '/posts/3/links/comments', { 'data' => [{type: 'comments', id: like.id.to_s }]}.to_json, "CONTENT_TYPE" => JSONAPI::MEDIA_TYPE + + assert_equal 405, status + end + def test_index_content_type get '/posts' assert_match JSONAPI::MEDIA_TYPE, headers['Content-Type'] From b3aa58d18caa8091d21807502f48ba992ded3038 Mon Sep 17 00:00:00 2001 From: Larry Gebhardt Date: Thu, 2 Apr 2015 13:16:24 -0400 Subject: [PATCH 2/7] Adds HasManySetReplacementForbidden exception --- lib/jsonapi/error_codes.rb | 1 + lib/jsonapi/exceptions.rb | 9 +++++++++ 2 files changed, 10 insertions(+) diff --git a/lib/jsonapi/error_codes.rb b/lib/jsonapi/error_codes.rb index c24607e9c..1a5eb793b 100644 --- a/lib/jsonapi/error_codes.rb +++ b/lib/jsonapi/error_codes.rb @@ -19,6 +19,7 @@ module JSONAPI INVALID_PAGE_VALUE = 118 INVALID_SORT_FORMAT = 119 INVALID_FIELD_FORMAT = 120 + FORBIDDEN = 403 RECORD_NOT_FOUND = 404 UNSUPPORTED_MEDIA_TYPE = 415 LOCKED = 423 diff --git a/lib/jsonapi/exceptions.rb b/lib/jsonapi/exceptions.rb index 9523e07a5..93de6fedc 100644 --- a/lib/jsonapi/exceptions.rb +++ b/lib/jsonapi/exceptions.rb @@ -58,6 +58,15 @@ def errors end end + class HasManySetReplacementForbidden < Error + def errors + [JSONAPI::Error.new(code: JSONAPI::FORBIDDEN, + status: :forbidden, + title: 'Complete replacement forbidden', + detail: 'Complete replacement forbidden for this association')] + end + end + class FilterNotAllowed < Error attr_accessor :filter def initialize(filter) From 74e58322fd362da305c8702ce399d538a1db5501 Mon Sep 17 00:00:00 2001 From: Larry Gebhardt Date: Thu, 2 Apr 2015 13:17:16 -0400 Subject: [PATCH 3/7] Change routing rules to allow routes to be created for associations, including those that do not act_as_set --- lib/jsonapi/routing_ext.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/jsonapi/routing_ext.rb b/lib/jsonapi/routing_ext.rb index 92020f216..09e7ce391 100644 --- a/lib/jsonapi/routing_ext.rb +++ b/lib/jsonapi/routing_ext.rb @@ -130,7 +130,7 @@ def jsonapi_links(*links) action: 'create_association', association: link_type.to_s, via: [:post] end - if methods.include?(:update) && res._association(link_type).acts_as_set + if methods.include?(:update) match "links/#{formatted_association_name}", controller: res._type.to_s, action: 'update_association', association: link_type.to_s, via: [:put, :patch] end From 75c0a9beff655c3d38d7634f4ddded5de1bb8b2e Mon Sep 17 00:00:00 2001 From: Larry Gebhardt Date: Thu, 2 Apr 2015 13:18:53 -0400 Subject: [PATCH 4/7] =?UTF-8?q?Raise=20a=20HasManySetReplacementForbidden?= =?UTF-8?q?=20exception=20(403)=20when=20a=20PATCH/PUT=20is=20made=20for?= =?UTF-8?q?=20an=20association=20that=20can=E2=80=99t=20be=20updated=20as?= =?UTF-8?q?=20a=20set.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- lib/jsonapi/request.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/jsonapi/request.rb b/lib/jsonapi/request.rb index bd80daae1..cf311ed0d 100644 --- a/lib/jsonapi/request.rb +++ b/lib/jsonapi/request.rb @@ -387,6 +387,10 @@ def parse_update_association_operation(data, association_type, parent_key) association_type, verified_param_set[:has_one].values[0]) else + unless association.acts_as_set + raise JSONAPI::Exceptions::HasManySetReplacementForbidden.new + end + object_params = {links: {association.name => {linkage: data}}} verified_param_set = parse_params(object_params, @resource_klass.updateable_fields(@context)) From 32fb4b5fa5e3356d6b7a01847acb28108a4d3fbb Mon Sep 17 00:00:00 2001 From: Larry Gebhardt Date: Thu, 2 Apr 2015 13:20:34 -0400 Subject: [PATCH 5/7] Refactoring acts_as_set. --- lib/jsonapi/resource.rb | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/jsonapi/resource.rb b/lib/jsonapi/resource.rb index 7a9a1026d..9daa77d0e 100644 --- a/lib/jsonapi/resource.rb +++ b/lib/jsonapi/resource.rb @@ -428,9 +428,7 @@ def _updateable_associations associations = [] @_associations.each do |key, association| - if association.is_a?(JSONAPI::Association::HasOne) || association.acts_as_set - associations.push(key) - end + associations.push(key) end associations end From 14533b2889700ba27a045cf8e08f11ef25f9a659 Mon Sep 17 00:00:00 2001 From: Larry Gebhardt Date: Thu, 2 Apr 2015 13:21:12 -0400 Subject: [PATCH 6/7] Update tests for acts_as_set support. --- test/integration/requests/request_test.rb | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/test/integration/requests/request_test.rb b/test/integration/requests/request_test.rb index 19aa08859..d944dd660 100644 --- a/test/integration/requests/request_test.rb +++ b/test/integration/requests/request_test.rb @@ -177,11 +177,13 @@ def test_put_update_association_has_one assert_equal 204, status end - def test_patch_update_association_has_many + def test_patch_update_association_has_many_acts_as_set + # Comments are acts_as_set=false so PUT/PATCH should respond with 403 + like = Comment.find_by(body: 'i liked it') patch '/posts/3/links/comments', { 'data' => [{type: 'comments', id: like.id.to_s }]}.to_json, "CONTENT_TYPE" => JSONAPI::MEDIA_TYPE - assert_equal 204, status + assert_equal 403, status end def test_post_update_association_has_many @@ -191,11 +193,13 @@ def test_post_update_association_has_many assert_equal 204, status end - def test_put_update_association_has_many + def test_put_update_association_has_many_acts_as_set + # Comments are acts_as_set=false so PUT/PATCH should respond with 403. Note: JR currently treats PUT and PATCH as equivalent + like = Comment.find_by(body: 'i liked it') put '/posts/3/links/comments', { 'data' => [{type: 'comments', id: like.id.to_s }]}.to_json, "CONTENT_TYPE" => JSONAPI::MEDIA_TYPE - assert_equal 405, status + assert_equal 403, status end def test_index_content_type From c7a348249d13815ef0c82ca0a4201d94aafff3ec Mon Sep 17 00:00:00 2001 From: Larry Gebhardt Date: Thu, 2 Apr 2015 13:32:06 -0400 Subject: [PATCH 7/7] Remove test dependencies (hack data so tests don't interfere). ToDo: fix this the right way --- test/fixtures/comments.yml | 6 +++++- test/integration/requests/request_test.rb | 12 ++++++------ 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/test/fixtures/comments.yml b/test/fixtures/comments.yml index c656e5039..efbf59123 100644 --- a/test/fixtures/comments.yml +++ b/test/fixtures/comments.yml @@ -14,4 +14,8 @@ post_2_thanks_man: id: 3 post_id: 2 body: Thanks man. Great post. But what is JR? - author_id: 2 \ No newline at end of file + author_id: 2 + +rogue_comment: + body: Rogue Comment Here + author_id: 3 \ No newline at end of file diff --git a/test/integration/requests/request_test.rb b/test/integration/requests/request_test.rb index d944dd660..4050a7724 100644 --- a/test/integration/requests/request_test.rb +++ b/test/integration/requests/request_test.rb @@ -180,15 +180,15 @@ def test_put_update_association_has_one def test_patch_update_association_has_many_acts_as_set # Comments are acts_as_set=false so PUT/PATCH should respond with 403 - like = Comment.find_by(body: 'i liked it') - patch '/posts/3/links/comments', { 'data' => [{type: 'comments', id: like.id.to_s }]}.to_json, "CONTENT_TYPE" => JSONAPI::MEDIA_TYPE + rogue = Comment.find_by(body: 'Rogue Comment Here') + patch '/posts/5/links/comments', { 'data' => [{type: 'comments', id: rogue.id.to_s }]}.to_json, "CONTENT_TYPE" => JSONAPI::MEDIA_TYPE assert_equal 403, status end def test_post_update_association_has_many - like = Comment.find_by(body: 'i liked it') - post '/posts/3/links/comments', { 'data' => [{type: 'comments', id: like.id.to_s }]}.to_json, "CONTENT_TYPE" => JSONAPI::MEDIA_TYPE + rogue = Comment.find_by(body: 'Rogue Comment Here') + post '/posts/5/links/comments', { 'data' => [{type: 'comments', id: rogue.id.to_s }]}.to_json, "CONTENT_TYPE" => JSONAPI::MEDIA_TYPE assert_equal 204, status end @@ -196,8 +196,8 @@ def test_post_update_association_has_many def test_put_update_association_has_many_acts_as_set # Comments are acts_as_set=false so PUT/PATCH should respond with 403. Note: JR currently treats PUT and PATCH as equivalent - like = Comment.find_by(body: 'i liked it') - put '/posts/3/links/comments', { 'data' => [{type: 'comments', id: like.id.to_s }]}.to_json, "CONTENT_TYPE" => JSONAPI::MEDIA_TYPE + rogue = Comment.find_by(body: 'Rogue Comment Here') + put '/posts/5/links/comments', { 'data' => [{type: 'comments', id: rogue.id.to_s }]}.to_json, "CONTENT_TYPE" => JSONAPI::MEDIA_TYPE assert_equal 403, status end