From 15e2da656f41af0124f7577858536f3b65462ad5 Mon Sep 17 00:00:00 2001 From: Dominic Cleal Date: Thu, 6 Oct 2016 13:47:26 +0100 Subject: Restore RecordNotFound when *_ids= can't find records by ID 9c9fb19 changed the behaviour of the _ids= setters for associations to raise an AssociationTypeMismatch when unknown IDs are given: Class: Message: <"Developer(#43811860) expected, got NilClass(#16732720)"> This restores the original ActiveRecord::RecordNotFound exception with a much clearer error message: Class: Message: <"Couldn't find all Developers with 'id': (1, -9999) [WHERE \"contracts\".\"company_id\" = ?] (found 1 results, but was looking for 2)"> Fixes #25719 --- activerecord/CHANGELOG.md | 5 +++++ .../lib/active_record/associations/collection_association.rb | 8 ++++++-- activerecord/lib/active_record/relation/finder_methods.rb | 8 ++++---- .../test/cases/associations/has_many_through_associations_test.rb | 3 ++- 4 files changed, 17 insertions(+), 7 deletions(-) diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 0b9a2f9943..f4218cc110 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,8 @@ +* Raise ActiveRecord::RecordNotFound from collection `*_ids` setters + for unknown IDs with a better error message. + + *Dominic Cleal* + * For PostgreSQL >= 9.4 use `pgcrypto`'s `gen_random_uuid()` instead of `uuid-ossp`'s UUID generation function. diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb index b2cf4713bb..99e6696c66 100644 --- a/activerecord/lib/active_record/associations/collection_association.rb +++ b/activerecord/lib/active_record/associations/collection_association.rb @@ -73,8 +73,12 @@ module ActiveRecord ids.map! { |i| pk_type.cast(i) } records = klass.where(reflection.association_primary_key => ids).index_by do |r| r.send(reflection.association_primary_key) - end.values_at(*ids) - replace(records) + end.values_at(*ids).compact + if records.size != ids.size + scope.raise_record_not_found_exception!(ids, records.size, ids.size, reflection.association_primary_key) + else + replace(records) + end end def reset diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb index 55ded4c6d0..93c8722aa3 100644 --- a/activerecord/lib/active_record/relation/finder_methods.rb +++ b/activerecord/lib/active_record/relation/finder_methods.rb @@ -345,7 +345,7 @@ module ActiveRecord # 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 = nil, result_size = nil, expected_size = nil) # :nodoc: + def raise_record_not_found_exception!(ids = nil, result_size = nil, expected_size = nil, key = primary_key) # :nodoc: conditions = arel.where_sql(@klass.arel_engine) conditions = " [#{conditions}]" if conditions name = @klass.name @@ -355,10 +355,10 @@ module ActiveRecord error << " with#{conditions}" if conditions raise RecordNotFound.new(error, name) elsif Array(ids).size == 1 - error = "Couldn't find #{name} with '#{primary_key}'=#{ids}#{conditions}" - raise RecordNotFound.new(error, name, primary_key, ids) + error = "Couldn't find #{name} with '#{key}'=#{ids}#{conditions}" + raise RecordNotFound.new(error, name, key, ids) else - error = "Couldn't find all #{name.pluralize} with '#{primary_key}': " + error = "Couldn't find all #{name.pluralize} with '#{key}': " error << "(#{ids.join(", ")})#{conditions} (found #{result_size} results, but was looking for #{expected_size})" raise RecordNotFound.new(error, name, primary_key, ids) diff --git a/activerecord/test/cases/associations/has_many_through_associations_test.rb b/activerecord/test/cases/associations/has_many_through_associations_test.rb index c2239ac03a..08a22a58b1 100644 --- a/activerecord/test/cases/associations/has_many_through_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_through_associations_test.rb @@ -886,7 +886,8 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase def test_collection_singular_ids_setter_raises_exception_when_invalid_ids_set company = companies(:rails_core) ids = [Developer.first.id, -9999] - assert_raises(ActiveRecord::AssociationTypeMismatch) { company.developer_ids = ids } + e = assert_raises(ActiveRecord::RecordNotFound) { company.developer_ids = ids } + assert_match(/Couldn't find all Developers with 'id'/, e.message) end def test_build_a_model_from_hm_through_association_with_where_clause -- cgit v1.2.3