Fixed order of loading in eager associations

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@1229 5ecf4fe2-1ee6-0310-87b1-e25e094e27de

activerecord/CHANGELOG

@@ -1,4 +1,6 @@
-*SVN*
+*1.10.1* (20th April, 2005)
+
+* Fixed order of loading in eager associations
 
 * Fixed stray comma when using eager loading and ordering together from has_many associations #1143
 

activerecord/Rakefile

@@ -8,7 +8,7 @@ require 'rake/contrib/rubyforgepublisher'
 
 PKG_BUILD     = ENV['PKG_BUILD'] ? '.' + ENV['PKG_BUILD'] : ''
 PKG_NAME      = 'activerecord'
-PKG_VERSION   = '1.10.0' + PKG_BUILD
+PKG_VERSION   = '1.10.1' + PKG_BUILD
 PKG_FILE_NAME = "#{PKG_NAME}-#{PKG_VERSION}"
 
 RELEASE_NAME  = "REL #{PKG_VERSION}"

activerecord/lib/active_record/associations.rb

@@ -147,7 +147,10 @@ module ActiveRecord
     # :limit property and it will be ignored if attempted.
     #
     # Also have in mind that since the eager loading is pulling from multiple tables, you'll have to disambiguate any column references
-    # in both conditions and orders. So :order => "posts.id DESC" will work while :order => "id DESC" will not.
+    # in both conditions and orders. So :order => "posts.id DESC" will work while :order => "id DESC" will not. This may require that
+    # you alter the :order and :conditions on the association definitions themselves.
+    #
+    # It's currently not possible to use eager loading on multiple associations from the same table.
     #
     # == Modules
     #
@@ -690,24 +693,26 @@ module ActiveRecord
           primary_key_table    = generate_primary_key_table(reflections, schema_abbreviations)
 
           rows        = select_all_rows(options, schema_abbreviations, reflections)
-          records     = { }        
+          records, records_in_order     = { }, []
           primary_key = primary_key_table[table_name]
           
           for row in rows
             id = row[primary_key]
-            records[id] ||= instantiate(extract_record(schema_abbreviations, table_name, row))
-          
+            records_in_order << (records[id] = instantiate(extract_record(schema_abbreviations, table_name, row))) unless records[id]
+            record = records[id]
+
             reflections.each do |reflection|
               next unless row[primary_key_table[reflection.table_name]]
               
               case reflection.macro
                 when :has_many, :has_and_belongs_to_many
-                  records[id].send(reflection.name)
-                  records[id].instance_variable_get("@#{reflection.name}").target.push(
-                    reflection.klass.send(:instantiate, extract_record(schema_abbreviations, reflection.table_name, row))
-                  )
+                  collection = record.send(reflection.name)
+                  collection.loaded
+
+                  association = reflection.klass.send(:instantiate, extract_record(schema_abbreviations, reflection.table_name, row))                  
+                  collection.target.push(association) unless collection.target.include?(association)
                 when :has_one, :belongs_to
-                  records[id].send(
+                  record.send(
                     "#{reflection.name}=", 
                     reflection.klass.send(:instantiate, extract_record(schema_abbreviations, reflection.table_name, row))
                   )
@@ -715,9 +720,10 @@ module ActiveRecord
             end
           end
           
-          return records.values
+          return records_in_order
         end
 
+
         def reflect_on_included_associations(associations)
           [ associations ].flatten.collect { |association| reflect_on_association(association) }
         end

activerecord/lib/active_record/associations/association_proxy.rb

@@ -32,6 +32,10 @@ module ActiveRecord
         @loaded
       end
       
+      def loaded
+        @loaded = true
+      end
+      
       def target
         @target
       end

activerecord/test/associations_go_eager_test.rb

@@ -17,6 +17,13 @@ class EagerAssociationTest < Test::Unit::TestCase
     assert post.comments.include?(@greetings)
   end
 
+  def test_with_ordering
+    posts = Post.find(:all, :include => :comments, :order => "posts.id DESC")
+    assert_equal @authorless, posts[0]
+    assert_equal @thinking, posts[1]
+    assert_equal @welcome, posts[2]
+  end
+  
   def test_loading_with_multiple_associations
     posts = Post.find(:all, :include => [ :comments, :author, :categories ], :order => "posts.id")
     assert_equal 2, posts.first.comments.size
@@ -25,7 +32,7 @@ class EagerAssociationTest < Test::Unit::TestCase
   end
 
   def test_loading_from_an_association
-    posts = @david.posts.find(:all, :include => :comments, :order => "posts.id DESC")
+    posts = @david.posts.find(:all, :include => :comments, :order => "posts.id")
     assert_equal 2, posts.first.comments.size
   end