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>

2 files changed, 42 insertions, 10 deletions

diff --git a/activeresource/lib/active_resource/base.rb b/activeresource/lib/active_resource/base.rb
index 88de8b1c66..293ba75ee0 100644
--- a/activeresource/lib/active_resource/base.rb
+++ b/activeresource/lib/active_resource/base.rb
@@ -585,6 +585,19 @@ module ActiveResource
       #
       #   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 @@ module ActiveResource
       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
 
diff --git a/activeresource/test/cases/finder_test.rb b/activeresource/test/cases/finder_test.rb
index a3083101db..38f7de96ac 100644
--- a/activeresource/test/cases/finder_test.rb
+++ b/activeresource/test/cases/finder_test.rb
@@ -79,6 +79,7 @@ class FinderTest < Test::Unit::TestCase
       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 @@ class FinderTest < Test::Unit::TestCase
     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 }