Skip to content
Browse files

rails@2075f39 introduced a regression in includes/preloades

by calling `read_attribute` on an association when preloading takes places, instead of using loaded records in `association.target`.

tl;dr

Records are not made properly available via `read_attribute` when preloding in simultaneous,
but value of `@loaded` is already set true, and records concatenated in `association.target` on an association object.
When `@loaded` is true we return an object of `AlreadyLoaded` in preload_for. In `AlreadyLoaded` to return preloaded
records we make wrong use of `read_attribute`, instead of `target` records.

The regression is fixed by making use of the loaded records in `association.target` when the preloading takes place.

Fixes #13437
  • Loading branch information...
1 parent 15e2eb4 commit bb17c3b2105da30ef3260c3b489da85e4865eaa5 @vipulnsward vipulnsward committed Dec 31, 2013
View
7 activerecord/CHANGELOG.md
@@ -1,3 +1,10 @@
+* Fixed regression on preload/includes with multiple arguments failing in certain conditions,
+ raising a NoMethodError internally by calling `reflect_on_association` for `NilClass:Class`.
+
+ Fixes #13437.
+
+ *Vipul A M*, *khustochka*
+
* Deprecated use of string argument as a configuration lookup in `ActiveRecord::Base.establish_connection`. Instead, a symbol must be given.
*José Valim*
View
2 activerecord/lib/active_record/associations/preloader.rb
@@ -183,7 +183,7 @@ def initialize(klass, owners, reflection, preload_scope)
def run(preloader); end
def preloaded_records
- owners.flat_map { |owner| owner.read_attribute reflection.name }
+ owners.flat_map { |owner| owner.association(reflection.name).target }
end
end
View
14 activerecord/test/cases/associations/cascaded_eager_loading_test.rb
@@ -174,4 +174,18 @@ def test_eager_association_loading_with_recursive_cascading_four_levels_has_and_
sink = Vertex.all.merge!(:includes=>{:sources=>{:sources=>{:sources=>:sources}}}, :order => 'vertices.id DESC').first
assert_equal vertices(:vertex_1), assert_no_queries { sink.sources.first.sources.first.sources.first.sources.first }
end
+
+ def test_eager_association_loading_with_cascaded_interdependent_one_level_and_two_levels
+ authors_relation = Author.all.merge!(:includes => [:comments, {:posts => :categorizations}], :order => "authors.id")
+ assert_nothing_raised do
+ authors_relation.to_a
+ end
+ authors = authors_relation.to_a
+ assert_equal 3, authors.size
+ assert_equal 10, authors[0].comments.size
+ assert_equal 1, authors[1].comments.size
+ assert_equal 5, authors[0].posts.size
+ assert_equal 3, authors[1].posts.size
+ assert_equal 3, authors[0].posts.collect { |post| post.categorizations.size }.inject(0) { |sum, i| sum+i }
+ end
end

0 comments on commit bb17c3b

Please sign in to comment.
Something went wrong with that request. Please try again.