Permalink
Browse files

Swallow ResourceNotFound error on find_every

Active Record does not explode with RecordNotFound if you go looking for a
collection of objects - it just returns nil. Thus Active Resource should
also not explode.

After all - finding no objects that match a set of conditions is not
exceptional behaviour - unlike looking for a specific object with a given id
(which you'd expect to exist).

I've also added documentation to +find+ to reflect this.

Signed-off-by: Joshua Peek <josh@joshpeek.com>
  • Loading branch information...
taryn authored and josh committed Aug 19, 2009
1 parent 079ed8f commit 4dc05bc8a9824b9404cebecaba28f9f248f9995e
Showing with 42 additions and 10 deletions.
  1. +29 −10 activeresource/lib/active_resource/base.rb
  2. +13 −0 activeresource/test/cases/finder_test.rb
@@ -585,6 +585,19 @@ def create(attributes = {})
#
# StreetAddress.find(1, :params => { :person_id => 1 })
# # => GET /people/1/street_addresses/1.xml
+ #
+ # == Failure or missing data
+ # A failure to find the requested object raises a ResourceNotFound
+ # exception if the find was called with an id.
+ # With any other scope, find returns nil when no data is returned.
+ #
+ # Person.find(1)
+ # # => raises ResourcenotFound
+ #
+ # Person.find(:all)
+ # Person.find(:first)
+ # Person.find(:last)
+ # # => nil
def find(*arguments)
scope = arguments.slice!(0)
options = arguments.slice!(0) || {}
@@ -638,16 +651,22 @@ def exists?(id, options = {})
private
# Find every resource
def find_every(options)
- case from = options[:from]
- when Symbol
- instantiate_collection(get(from, options[:params]))
- when String
- path = "#{from}#{query_string(options[:params])}"
- instantiate_collection(connection.get(path, headers) || [])
- else
- prefix_options, query_options = split_options(options[:params])
- path = collection_path(prefix_options, query_options)
- instantiate_collection( (connection.get(path, headers) || []), prefix_options )
+ begin
+ case from = options[:from]
+ when Symbol
+ instantiate_collection(get(from, options[:params]))
+ when String
+ path = "#{from}#{query_string(options[:params])}"
+ instantiate_collection(connection.get(path, headers) || [])
+ else
+ prefix_options, query_options = split_options(options[:params])
+ path = collection_path(prefix_options, query_options)
+ instantiate_collection( (connection.get(path, headers) || []), prefix_options )
+ end
+ rescue ActiveResource::ResourceNotFound
+ # Swallowing ResourceNotFound exceptions and return nil - as per
+ # ActiveRecord.
+ nil
end
end
@@ -79,6 +79,7 @@ def setup
mock.get "/people/1/addresses.xml", {}, @addresses
mock.get "/people/1/addresses/1.xml", {}, @addy
mock.get "/people/1/addresses/2.xml", {}, nil, 404
+ mock.get "/people/2/addresses.xml", {}, nil, 404
mock.get "/people/2/addresses/1.xml", {}, nil, 404
mock.get "/people/Greg/addresses/1.xml", {}, @addy
mock.put "/people/1/addresses/1.xml", {}, nil, 204
@@ -142,6 +143,18 @@ def test_find_by_id_not_found
assert_raise(ActiveResource::ResourceNotFound) { StreetAddress.find(1) }
end
+ def test_find_all_sub_objects
+ all = StreetAddress.find(:all, :params => { :person_id => 1 })
+ assert_equal 1, all.size
+ assert_kind_of StreetAddress, all.first
+ end
+
+ def test_find_all_sub_objects_not_found
+ assert_nothing_raised do
+ addys = StreetAddress.find(:all, :params => { :person_id => 2 })
+ end
+ end
+
def test_find_all_by_from
ActiveResource::HttpMock.respond_to { |m| m.get "/companies/1/people.xml", {}, @people_david }

0 comments on commit 4dc05bc

Please sign in to comment.