Skip to content

Commit

Permalink
Throwing a RecordNotFound exception when a record is scanned using the
Browse files Browse the repository at this point in the history
inverse_of option. I've also refactored the code for raising a
RecordNotFound exception when searching for records with ids.
  • Loading branch information
wangjohn committed Apr 1, 2013
1 parent 93302dc commit 6c5032f
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 12 deletions.
Expand Up @@ -79,8 +79,20 @@ def find(*args)
if block_given? if block_given?
load_target.find(*args) { |*block_args| yield(*block_args) } load_target.find(*args) { |*block_args| yield(*block_args) }
else else
if options[:finder_sql] || options[:inverse_of] if options[:finder_sql]
find_by_scan(*args) 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 else
scope.find(*args) scope.find(*args)
end end
Expand Down
35 changes: 24 additions & 11 deletions activerecord/lib/active_record/relation/finder_methods.rb
Expand Up @@ -176,6 +176,28 @@ def exists?(conditions = :none)
false false
end 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 protected


def find_with_associations def find_with_associations
Expand Down Expand Up @@ -259,11 +281,7 @@ def find_one(id)
relation.bind_values += [[column, id]] relation.bind_values += [[column, id]]
record = relation.take record = relation.take


unless record raise_record_not_found_exception!(id, 0, 1) unless record
conditions = arel.where_sql
conditions = " [#{conditions}]" if conditions
raise RecordNotFound, "Couldn't find #{@klass.name} with #{primary_key}=#{id}#{conditions}"
end


record record
end end
Expand All @@ -286,12 +304,7 @@ def find_some(ids)
if result.size == expected_size if result.size == expected_size
result result
else else
conditions = arel.where_sql raise_record_not_found_exception!(ids, result.size, expected_size)
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
end end
end end


Expand Down
18 changes: 18 additions & 0 deletions activerecord/test/cases/associations/inverse_associations_test.rb
Expand Up @@ -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" 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 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 def test_trying_to_use_inverses_that_dont_exist_should_raise_an_error
assert_raise(ActiveRecord::InverseOfAssociationNotFoundError) { Man.first.secret_interests } assert_raise(ActiveRecord::InverseOfAssociationNotFoundError) { Man.first.secret_interests }
end end
Expand Down

0 comments on commit 6c5032f

Please sign in to comment.