Skip to content

Commit

Permalink
Merge remote branch 'gramos/ares-missing-prefix-value'
Browse files Browse the repository at this point in the history
  • Loading branch information
josevalim committed Sep 27, 2010
2 parents 8aa3684 + f85b38a commit 8be911c
Show file tree
Hide file tree
Showing 5 changed files with 41 additions and 1 deletion.
22 changes: 22 additions & 0 deletions activeresource/lib/active_resource/base.rb
Expand Up @@ -166,6 +166,7 @@ module ActiveResource
# # GET http://api.people.com:3000/people/999.xml
# ryan = Person.find(999) # 404, raises ActiveResource::ResourceNotFound
#
#
# <tt>404</tt> 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:
#
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -621,6 +632,8 @@ def prefix(options={}) "#{prefix_call}" end
# # => /posts/5/comments/1.xml?active=1
#
def element_path(id, 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}/#{URI.escape id.to_s}.#{format.extension}#{query_string(query_options)}"
end
Expand Down Expand Up @@ -663,6 +676,7 @@ def new_element_path(prefix_options = {})
# # => /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
Expand Down Expand Up @@ -842,6 +856,14 @@ def exists?(id, options = {})
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
Expand Down
3 changes: 3 additions & 0 deletions activeresource/lib/active_resource/exceptions.rb
Expand Up @@ -36,6 +36,9 @@ class Redirection < ConnectionError # :nodoc:
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:

Expand Down
1 change: 1 addition & 0 deletions activeresource/test/abstract_unit.rb
Expand Up @@ -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
Expand Down
14 changes: 14 additions & 0 deletions activeresource/test/cases/base_test.rb
Expand Up @@ -475,6 +475,12 @@ def test_custom_element_path
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_required_prefix_param
assert_raise ActiveResource::MissingPrefixParam do
StreetAddress.element_path(1)
end
end

def test_module_element_path
assert_equal '/sounds/1.xml', Asset::Sound.element_path(1)
end
Expand Down Expand Up @@ -513,6 +519,12 @@ def test_custom_element_path_with_prefix_and_parameters
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)
Expand Down Expand Up @@ -560,6 +572,8 @@ def test_set_prefix_twice_should_clear_params
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

Expand Down
2 changes: 1 addition & 1 deletion activeresource/test/cases/finder_test.rb
Expand Up @@ -84,7 +84,7 @@ def test_last_with_params

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
Expand Down

0 comments on commit 8be911c

Please sign in to comment.