Changeset 3553

Timestamp:

02/09/06 09:17:40 (3 years ago)

Author:

nzkoz

Message:

* Fix pagination problems when using include
* Introduce Unit Tests for pagination
* Allow count to work with :include by using count distinct.

[Kevin Clark & Jeremy Hopple]

Files:

• trunk/actionpack/CHANGELOG (modified) (1 diff)
• trunk/actionpack/lib/action_controller/pagination.rb (modified) (2 diffs)
• trunk/actionpack/Rakefile (modified) (1 diff)
• trunk/actionpack/test/active_record_unit.rb (added)
• trunk/actionpack/test/activerecord (added)
• trunk/actionpack/test/activerecord/active_record_assertions_test.rb (added)
• trunk/actionpack/test/activerecord/pagination_test.rb (added)
• trunk/actionpack/test/controller/active_record_assertions_test.rb (deleted)
• trunk/actionpack/test/fixtures/companies.yml (added)
• trunk/actionpack/test/fixtures/company.rb (added)
• trunk/actionpack/test/fixtures/db_definitions (added)
• trunk/actionpack/test/fixtures/db_definitions/sqlite.sql (added)
• trunk/actionpack/test/fixtures/developer.rb (added)
• trunk/actionpack/test/fixtures/developers_projects.yml (added)
• trunk/actionpack/test/fixtures/developers.yml (added)
• trunk/actionpack/test/fixtures/project.rb (added)
• trunk/actionpack/test/fixtures/projects.yml (added)
• trunk/actionpack/test/fixtures/replies.yml (added)
• trunk/actionpack/test/fixtures/reply.rb (added)
• trunk/actionpack/test/fixtures/topic.rb (added)
• trunk/actionpack/test/fixtures/topics.yml (added)
• trunk/activerecord/CHANGELOG (modified) (1 diff)
• trunk/activerecord/lib/active_record/associations.rb (modified) (4 diffs)
• trunk/activerecord/lib/active_record/base.rb (modified) (1 diff)
• trunk/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb (modified) (1 diff)
• trunk/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb (modified) (1 diff)
• trunk/activerecord/test/associations_go_eager_test.rb (modified) (2 diffs)
• trunk/activerecord/test/base_test.rb (modified) (1 diff)

trunk/actionpack/CHANGELOG

@@ -1 +1 @@
-*SVN*
+
+* Fix problems with pagination and :include.  [Kevin Clark]
+
+* Add ActiveRecordTestCase for testing AR integration. [Kevin Clark]
+
+* Add Unit Tests for pagination [Kevin Clark]
-
-* Add :html option for specifying form tag options in form_for. [Sam Stephenson]

trunk/actionpack/lib/action_controller/pagination.rb

@@ -164 +164 @@
-    # the +model+ and given +conditions+. Override this method to implement a
-    # custom counter.
-
-
-
-
+
+
+
+
+
-    # Returns a collection of items for the given +model+ and +options[conditions]+,
-    # ordered by +options[order]+, for the current page in the given +paginator+.
@@ -186 +187 @@
-      klass = options[:class_name].constantize
-      page  = @params[options[:parameter]]
-
-
-
+
-      paginator = Paginator.new(self, count, options[:per_page], page)
-      collection = find_collection_for_pagination(klass, options, paginator)
-  
+
-      return paginator, collection 
-    end

trunk/actionpack/Rakefile

@@ -31 +31 @@
-  t.verbose = true
-}
+
+desc 'ActiveRecord Integration Tests'
+Rake::TestTask.new(:test_active_record_integration) do |t|
+  t.libs << "test"
+  t.test_files = Dir.glob("test/activerecord/*_test.rb")
+  t.verbose = true
+end
-
-

trunk/activerecord/CHANGELOG

@@ -1 +1 @@
-*SVN*
+
+* Fix problems with count when used with :include [Jeremy Hopple and Kevin Clark]
-
-* ActiveRecord::RecordInvalid now states which validations failed in its default error message [Tobias Luetke]

trunk/activerecord/lib/active_record/associations.rb

@@ -780 +780 @@
-          end
-        end
+        
+        def count_with_associations(options = {})
+          reflections = reflect_on_included_associations(options[:include])
+          return count_by_sql(construct_counter_sql_with_included_associations(options, reflections))
+        end
-
-        def find_with_associations(options = {})
@@ -997 +1002 @@
-          )
-        end
+        
+        def construct_counter_sql_with_included_associations(options, reflections)
+          sql = "SELECT COUNT(DISTINCT #{table_name}.#{primary_key})"
+          
+          # A (slower) workaround if we're using a backend, like sqlite, that doesn't support COUNT DISTINCT.
+          if !Base.connection.supports_count_distinct?
+            sql = "SELECT COUNT(*) FROM (SELECT DISTINCT #{table_name}.#{primary_key}"
+          end
+          
+          sql << " FROM #{table_name} "
+          sql << reflections.collect { |reflection| association_join(reflection) }.to_s
+          sql << "#{options[:joins]} " if options[:joins]
+
+          add_conditions!(sql, options[:conditions])
+          add_sti_conditions!(sql, reflections)
+          add_limited_ids_condition!(sql, options, reflections) if !using_limitable_reflections?(reflections) && options[:limit]
+
+          add_limit!(sql, options) if using_limitable_reflections?(reflections)
+
+          if !Base.connection.supports_count_distinct?
+            sql << ")"
+          end
+
+          return sanitize_sql(sql)          
+        end
-
-        def construct_finder_sql_with_included_associations(options, schema_abbreviations, reflections)
@@ -1002 +1032 @@
-          sql << reflections.collect { |reflection| association_join(reflection) }.to_s
-          sql << "#{options[:joins]} " if options[:joins]
-
+
-          add_conditions!(sql, options[:conditions])
-          add_sti_conditions!(sql, reflections)
-
+, reflections
-
-          sql << "ORDER BY #{options[:order]} " if options[:order]
-
+
-          add_limit!(sql, options) if using_limitable_reflections?(reflections)
-
+
-          return sanitize_sql(sql)
-        end
-
-
-
+
+, reflections
+, reflections
-            sql << "#{condition_word(sql)} #{table_name}.#{primary_key} IN (#{id_list}) "
-          end
-        end
-
-
+
+, reflections
-          connection.select_values(
-
+, reflections
-            "#{name} Load IDs For Limited Eager Loading"
-          ).collect { |id| connection.quote(id) }.join(", ")
-        end
-
-
-
-
-
+
+
+
+
+
+
+
+
+
+
+
+
-          add_conditions!(sql, options[:conditions])
-          sql << "ORDER BY #{options[:order]} " if options[:order]
@@ -1043 +1080 @@
-          conditions.scan(/(\w+)\.\w+/).flatten.any? do |condition_table_name|
-            condition_table_name != table_name
+          end
+        end
+        
+        def include_eager_order?(options)
+          order = options[:order]
+          return false unless order
+          order.scan(/(\w+)\.\w+/).flatten.any? do |order_table_name|
+            order_table_name != table_name
-          end
-        end

trunk/activerecord/lib/active_record/base.rb

@@ -496 +496 @@
-      end
-
-
-
-
-
-
-
-
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
-      end
-

trunk/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb

@@ -38 +38 @@
-      def supports_migrations?
-        false
+      end
+      
+      # Does this adapter support using DISTINCT within COUNT?  This is +true+
+      # for all adapters except sqlite.
+      def supports_count_distinct?
+        true
-      end
-

trunk/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb

@@ -98 +98 @@
-      def supports_migrations? #:nodoc:
-        true
+      end
+      
+      def supports_count_distinct? #:nodoc:
+        false
-      end
-

trunk/activerecord/test/associations_go_eager_test.rb

@@ -121 +121 @@
-
-  def test_eager_with_has_many_and_limit_and_conditions_array_on_the_eagers
-
-
-
+
+
+
+
+
-  end
-
@@ -142 +144 @@
-
-  def test_eager_with_has_many_and_limit_and_conditions_on_the_eagers
-
-
-
-
-
-
-
+
+
+
+
+
+
+
+
+
+
+
+
+
-  end
-

trunk/activerecord/test/base_test.rb

@@ -1057 +1057 @@
-    end
-    assert_equal res, res2
+    
+    res3 = res + 1
+    assert_nothing_raised do
+      res3 = Post.count(:conditions => "posts.#{QUOTED_TYPE} = 'Post'",
+                        :joins => "LEFT JOIN comments ON posts.id=comments.post_id")
+    end
+    assert_equal res, res3
-  end
-