From 6c5032f53de9f4ba1669ff9c9eea25c70bf3af33 Mon Sep 17 00:00:00 2001
From: wangjohn <wangjohn@mit.edu>
Date: Sun, 31 Mar 2013 21:23:53 -0400
Subject: [PATCH] Throwing a RecordNotFound exception when a record is scanned
 using the inverse_of option. I've also refactored the code for raising a
 RecordNotFound exception when searching for records with ids.

---
 .../associations/collection_association.rb    | 14 +++++++-
 .../active_record/relation/finder_methods.rb  | 35 +++++++++++++------
 .../associations/inverse_associations_test.rb | 18 ++++++++++
 3 files changed, 55 insertions(+), 12 deletions(-)

diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb
index 2385c90c1aaf0..79e626f67ed87 100644
--- a/activerecord/lib/active_record/associations/collection_association.rb
+++ b/activerecord/lib/active_record/associations/collection_association.rb
@@ -79,8 +79,20 @@ def find(*args)
         if block_given?
           load_target.find(*args) { |*block_args| yield(*block_args) }
         else
-          if options[:finder_sql] || options[:inverse_of]
+          if options[:finder_sql]
             find_by_scan(*args)
+          elsif options[:inverse_of]
+            args = args.flatten
+            raise RecordNotFound, "Must specify an id to find" if args.blank?
+
+            result = find_by_scan(*args)
+
+            result_size = Array(result).size
+            if !result || result_size != args.size
+              scope.raise_record_not_found_exception!(args, result_size, args.size)
+            else
+              result
+            end
           else
             scope.find(*args)
           end
diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb
index e2685d0478441..a03d4bfeffb44 100644
--- a/activerecord/lib/active_record/relation/finder_methods.rb
+++ b/activerecord/lib/active_record/relation/finder_methods.rb
@@ -176,6 +176,28 @@ def exists?(conditions = :none)
       false
     end
 
+    # This method is called whenever no records are found with either a single
+    # id or multiple ids and raises a +ActiveRecord::RecordNotFound+ exception.
+    #
+    # The error message is different depending on whether a single id or
+    # multiple ids are provided. If multiple ids are provided, then the number
+    # of results obtained should be provided in the +result_size+ argument and
+    # the expected number of results should be provided in the +expected_size+
+    # argument.
+    def raise_record_not_found_exception!(ids, result_size, expected_size) #:nodoc:
+      conditions = arel.where_sql
+      conditions = " [#{conditions}]" if conditions
+
+      if Array(ids).size == 1
+        error = "Couldn't find #{@klass.name} with #{primary_key}=#{ids}#{conditions}"
+      else
+        error = "Couldn't find all #{@klass.name.pluralize} with IDs "
+        error << "(#{ids.join(", ")})#{conditions} (found #{result_size} results, but was looking for #{expected_size})"
+      end
+
+      raise RecordNotFound, error
+    end
+
     protected
 
     def find_with_associations
@@ -259,11 +281,7 @@ def find_one(id)
       relation.bind_values += [[column, id]]
       record = relation.take
 
-      unless record
-        conditions = arel.where_sql
-        conditions = " [#{conditions}]" if conditions
-        raise RecordNotFound, "Couldn't find #{@klass.name} with #{primary_key}=#{id}#{conditions}"
-      end
+      raise_record_not_found_exception!(id, 0, 1) unless record
 
       record
     end
@@ -286,12 +304,7 @@ def find_some(ids)
       if result.size == expected_size
         result
       else
-        conditions = arel.where_sql
-        conditions = " [#{conditions}]" if conditions
-
-        error = "Couldn't find all #{@klass.name.pluralize} with IDs "
-        error << "(#{ids.join(", ")})#{conditions} (found #{result.size} results, but was looking for #{expected_size})"
-        raise RecordNotFound, error
+        raise_record_not_found_exception!(ids, result.size, expected_size)
       end
     end
 
diff --git a/activerecord/test/cases/associations/inverse_associations_test.rb b/activerecord/test/cases/associations/inverse_associations_test.rb
index c8cad84013ec1..89d2876c6c29b 100644
--- a/activerecord/test/cases/associations/inverse_associations_test.rb
+++ b/activerecord/test/cases/associations/inverse_associations_test.rb
@@ -303,6 +303,24 @@ def test_parent_instance_should_find_child_instance_using_child_instance_id_when
     assert_equal man.name, man.interests.find(interest.id).man.name, "The name of the man should match after the child name is changed"
   end
 
+  def test_raise_record_not_found_error_when_invalid_ids_are_passed
+    man = Man.create!
+    interest = Interest.create!(man: man)
+
+    invalid_id = 2394823094892348920348523452345
+    assert_raise(ActiveRecord::RecordNotFound) { man.interests.find(invalid_id) }
+
+    invalid_ids = [8432342, 2390102913, 2453245234523452]
+    assert_raise(ActiveRecord::RecordNotFound) { man.interests.find(invalid_ids) }
+  end
+
+  def test_raise_record_not_found_error_when_no_ids_are_passed
+    man = Man.create!
+    interest = Interest.create!(man: man)
+
+    assert_raise(ActiveRecord::RecordNotFound) { man.interests.find() }
+  end
+
   def test_trying_to_use_inverses_that_dont_exist_should_raise_an_error
     assert_raise(ActiveRecord::InverseOfAssociationNotFoundError) { Man.first.secret_interests }
   end