From a71e07d61e8bb69788b7120a5cf0d3620c6094be Mon Sep 17 00:00:00 2001 From: Gaston Ramos Date: Mon, 13 Sep 2010 17:05:52 -0300 Subject: - elmenth_path raise an ActiveResource::MissingPrefixParam exception when prefix_options does not has all required prefix_options ex: class StreetAddress < ActiveResource::Base self.site = "http://37s.sunrise.i:3000/people/:person_id/" end StreetAddress.element_path(1) # => ActiveResource::MissingPrefixParam --- activeresource/lib/active_resource/base.rb | 6 ++++++ activeresource/lib/active_resource/exceptions.rb | 3 +++ activeresource/test/cases/base_test.rb | 8 ++++++++ activeresource/test/cases/finder_test.rb | 2 +- 4 files changed, 18 insertions(+), 1 deletion(-) diff --git a/activeresource/lib/active_resource/base.rb b/activeresource/lib/active_resource/base.rb index d31db9f0ba..80af7e7adf 100644 --- a/activeresource/lib/active_resource/base.rb +++ b/activeresource/lib/active_resource/base.rb @@ -621,6 +621,12 @@ module ActiveResource # # => /posts/5/comments/1.xml?active=1 # def element_path(id, prefix_options = {}, query_options = nil) + + p_options = HashWithIndifferentAccess.new(prefix_options) + prefix_parameters.each do |p| + raise MissingPrefixParam if p_options[p].blank? + end + prefix_options, query_options = split_options(prefix_options) if query_options.nil? "#{prefix(prefix_options)}#{collection_name}/#{URI.escape id.to_s}.#{format.extension}#{query_string(query_options)}" end diff --git a/activeresource/lib/active_resource/exceptions.rb b/activeresource/lib/active_resource/exceptions.rb index 0f4549fd73..6b953b28ad 100644 --- a/activeresource/lib/active_resource/exceptions.rb +++ b/activeresource/lib/active_resource/exceptions.rb @@ -36,6 +36,9 @@ module ActiveResource def to_s; response['Location'] ? "#{super} => #{response['Location']}" : super; end end + # Raised when ... + class MissingPrefixParam < ArgumentError; end # :nodoc: + # 4xx Client Error class ClientError < ConnectionError; end # :nodoc: diff --git a/activeresource/test/cases/base_test.rb b/activeresource/test/cases/base_test.rb index 6fabeeebcd..eb1a747c0a 100644 --- a/activeresource/test/cases/base_test.rb +++ b/activeresource/test/cases/base_test.rb @@ -475,6 +475,12 @@ class BaseTest < Test::Unit::TestCase assert_equal '/people/ann%20mary/addresses/ann%20mary.xml', StreetAddress.element_path(:'ann mary', 'person_id' => 'ann mary') end + def test_custom_element_path_without_parent_id + assert_raise ActiveResource::MissingPrefixParam do + assert_equal '/people/1/addresses/1.xml', StreetAddress.element_path(1) + end + end + def test_module_element_path assert_equal '/sounds/1.xml', Asset::Sound.element_path(1) end @@ -560,6 +566,8 @@ class BaseTest < Test::Unit::TestCase assert_equal Set.new([:the_param1]), person_class.prefix_parameters person_class.prefix = "the_prefix/:the_param2" assert_equal Set.new([:the_param2]), person_class.prefix_parameters + person_class.prefix = "the_prefix/:the_param1/other_prefix/:the_param2" + assert_equal Set.new([:the_param2, :the_param1]), person_class.prefix_parameters end end diff --git a/activeresource/test/cases/finder_test.rb b/activeresource/test/cases/finder_test.rb index fd09ef46d7..ebb783996d 100644 --- a/activeresource/test/cases/finder_test.rb +++ b/activeresource/test/cases/finder_test.rb @@ -84,7 +84,7 @@ class FinderTest < Test::Unit::TestCase def test_find_by_id_not_found assert_raise(ActiveResource::ResourceNotFound) { Person.find(99) } - assert_raise(ActiveResource::ResourceNotFound) { StreetAddress.find(1) } + assert_raise(ActiveResource::ResourceNotFound) { StreetAddress.find(99, :params => {:person_id => 1}) } end def test_find_all_sub_objects -- cgit v1.2.3 From 30fb3638cce6fd27d003c7e0c4f689a0f1e28177 Mon Sep 17 00:00:00 2001 From: Gaston Ramos Date: Mon, 13 Sep 2010 17:14:37 -0300 Subject: - refactoring, move prefix_options check to a custom method --- activeresource/lib/active_resource/base.rb | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/activeresource/lib/active_resource/base.rb b/activeresource/lib/active_resource/base.rb index 80af7e7adf..911ae37b35 100644 --- a/activeresource/lib/active_resource/base.rb +++ b/activeresource/lib/active_resource/base.rb @@ -621,11 +621,7 @@ module ActiveResource # # => /posts/5/comments/1.xml?active=1 # def element_path(id, prefix_options = {}, query_options = nil) - - p_options = HashWithIndifferentAccess.new(prefix_options) - prefix_parameters.each do |p| - raise MissingPrefixParam if p_options[p].blank? - end + check_prefix_options(prefix_options) prefix_options, query_options = split_options(prefix_options) if query_options.nil? "#{prefix(prefix_options)}#{collection_name}/#{URI.escape id.to_s}.#{format.extension}#{query_string(query_options)}" @@ -848,6 +844,14 @@ module ActiveResource end private + + def check_prefix_options(prefix_options) + p_options = HashWithIndifferentAccess.new(prefix_options) + prefix_parameters.each do |p| + raise(MissingPrefixParam, "#{p} prefix_option is missing") if p_options[p].blank? + end + end + # Find every resource def find_every(options) begin -- cgit v1.2.3 From 9363931f349d783058688c2c14a31856116125e0 Mon Sep 17 00:00:00 2001 From: Gaston Ramos Date: Mon, 13 Sep 2010 17:21:49 -0300 Subject: - better name for prefix param test case --- activeresource/test/cases/base_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/activeresource/test/cases/base_test.rb b/activeresource/test/cases/base_test.rb index eb1a747c0a..f813ce2181 100644 --- a/activeresource/test/cases/base_test.rb +++ b/activeresource/test/cases/base_test.rb @@ -475,7 +475,7 @@ class BaseTest < Test::Unit::TestCase assert_equal '/people/ann%20mary/addresses/ann%20mary.xml', StreetAddress.element_path(:'ann mary', 'person_id' => 'ann mary') end - def test_custom_element_path_without_parent_id + def test_custom_element_path_without_required_prefix_param assert_raise ActiveResource::MissingPrefixParam do assert_equal '/people/1/addresses/1.xml', StreetAddress.element_path(1) end -- cgit v1.2.3 From 823a8e6e66b6e63583f569fdbc2e9cffa9430bf8 Mon Sep 17 00:00:00 2001 From: Gaston Ramos Date: Mon, 13 Sep 2010 17:33:58 -0300 Subject: - check prefix options in collection_path --- activeresource/lib/active_resource/base.rb | 1 + activeresource/test/cases/base_test.rb | 8 +++++++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/activeresource/lib/active_resource/base.rb b/activeresource/lib/active_resource/base.rb index 911ae37b35..9442eba8af 100644 --- a/activeresource/lib/active_resource/base.rb +++ b/activeresource/lib/active_resource/base.rb @@ -665,6 +665,7 @@ module ActiveResource # # => /posts/5/comments.xml?active=1 # def collection_path(prefix_options = {}, query_options = nil) + check_prefix_options(prefix_options) prefix_options, query_options = split_options(prefix_options) if query_options.nil? "#{prefix(prefix_options)}#{collection_name}.#{format.extension}#{query_string(query_options)}" end diff --git a/activeresource/test/cases/base_test.rb b/activeresource/test/cases/base_test.rb index f813ce2181..18c732b2ab 100644 --- a/activeresource/test/cases/base_test.rb +++ b/activeresource/test/cases/base_test.rb @@ -477,7 +477,7 @@ class BaseTest < Test::Unit::TestCase def test_custom_element_path_without_required_prefix_param assert_raise ActiveResource::MissingPrefixParam do - assert_equal '/people/1/addresses/1.xml', StreetAddress.element_path(1) + StreetAddress.element_path(1) end end @@ -519,6 +519,12 @@ class BaseTest < Test::Unit::TestCase assert_equal '/people/1/addresses/1.xml?type=work', StreetAddress.element_path(1, {:person_id => 1}, {:type => 'work'}) end + def test_custom_collection_path_without_required_prefix_param + assert_raise ActiveResource::MissingPrefixParam do + StreetAddress.collection_path + end + end + def test_custom_collection_path assert_equal '/people/1/addresses.xml', StreetAddress.collection_path(:person_id => 1) assert_equal '/people/1/addresses.xml', StreetAddress.collection_path('person_id' => 1) -- cgit v1.2.3 From e405dbcb4896054d517c6e0cb8b737e5e632c617 Mon Sep 17 00:00:00 2001 From: Gaston Ramos Date: Mon, 13 Sep 2010 19:34:46 -0300 Subject: - update exceptions documentation --- activeresource/lib/active_resource/base.rb | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/activeresource/lib/active_resource/base.rb b/activeresource/lib/active_resource/base.rb index 9442eba8af..6135ce4ec1 100644 --- a/activeresource/lib/active_resource/base.rb +++ b/activeresource/lib/active_resource/base.rb @@ -166,6 +166,7 @@ module ActiveResource # # GET http://api.people.com:3000/people/999.xml # ryan = Person.find(999) # 404, raises ActiveResource::ResourceNotFound # + # # 404 is just one of the HTTP error response codes that Active Resource will handle with its own exception. The # following HTTP response codes will also result in these exceptions: # @@ -194,6 +195,16 @@ module ActiveResource # redirect_to :action => 'new' # end # + # When a GET is requested for a nested resource and you don't provide the prefix_param + # an ActiveResource::MissingPrefixParam will be raised. + # + # class Comment < ActiveResource::Base + # self.site = "http://someip.com/posts/:post_id/" + # end + # + # Comment.find(1) + # # => ActiveResource::MissingPrefixParam: post_id prefix_option is missing + # # === Validation errors # # Active Resource supports validations on resources and will return errors if any of these validations fail -- cgit v1.2.3 From f85b38a36b68488e94e9950fd15ec29a21bf6917 Mon Sep 17 00:00:00 2001 From: Gaston Ramos Date: Fri, 17 Sep 2010 21:24:04 -0300 Subject: - added mock to test ActiveResource::MissingPrefixParam in finder_test --- activeresource/test/abstract_unit.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/activeresource/test/abstract_unit.rb b/activeresource/test/abstract_unit.rb index 129efeb879..544aede002 100644 --- a/activeresource/test/abstract_unit.rb +++ b/activeresource/test/abstract_unit.rb @@ -97,6 +97,7 @@ def setup_response mock.put "/people/1/addresses/1.xml", {}, nil, 204 mock.delete "/people/1/addresses/1.xml", {}, nil, 200 mock.post "/people/1/addresses.xml", {}, nil, 201, 'Location' => '/people/1/addresses/5' + mock.get "/people/1/addresses/99.xml", {}, nil, 404 mock.get "/people//addresses.xml", {}, nil, 404 mock.get "/people//addresses/1.xml", {}, nil, 404 mock.put "/people//addresses/1.xml", {}, nil, 404 -- cgit v1.2.3