Ticket #10012: ar_joins_fixed_for_10011.diff

activerecord/test/associations/ar_joins_test.rb

@@ +1 @@
+require 'abstract_unit'
+require 'fixtures/post'
+require 'fixtures/comment'
+require 'fixtures/author'
+require 'fixtures/category'
+require 'fixtures/categorization'
+require 'fixtures/company'
+require 'fixtures/topic'
+require 'fixtures/reply'
+require 'fixtures/developer'
+require 'fixtures/project'
+
+class ArJoinsTest < Test::Unit::TestCase
+  fixtures :authors, :posts, :comments, :categories, :categories_posts, :people,
+           :developers, :projects, :developers_projects,
+           :categorizations, :companies, :accounts, :topics
+
+  def test_ar_joins
+    authors = Author.find(:all, :joins => :posts, :conditions => ['posts.type = ?', "Post"])
+    assert_not_equal(0 , authors.length)
+    authors.each do |author|
+      assert !(author.send(:instance_variables).include? "@posts")
+      assert(!author.readonly?, "non-string join value produced read-only result.")
+    end
+  end
+
+  def test_ar_joins_with_cascaded_two_levels
+    authors = Author.find(:all, :joins=>{:posts=>:comments})
+    assert_equal(2, authors.length)
+    authors.each do |author|
+      assert !(author.send(:instance_variables).include? "@posts")
+      assert(!author.readonly?, "non-string join value produced read-only result.")
+    end
+    authors = Author.find(:all, :joins=>{:posts=>:comments}, :conditions => ["comments.body = ?", "go crazy" ])
+    assert_equal(1, authors.length)
+    authors.each do |author|
+      assert !(author.send(:instance_variables).include? "@posts")
+      assert(!author.readonly?, "non-string join value produced read-only result.")
+    end
+  end
+
+
+  def test_ar_joins_with_complex_conditions
+    authors = Author.find(:all, :joins=>{:posts=>[:comments, :categories]},
+    :conditions => ["categories.name = ?  AND posts.title = ?", "General", "So I was thinking"] 
+    )
+    assert_equal(1, authors.length)
+    authors.each do |author|
+      assert !(author.send(:instance_variables).include? "@posts")
+      assert(!author.readonly?, "non-string join value produced read-only result.")
+    end
+    assert_equal("David", authors.first.name)
+  end   
+  
+  def test_ar_join_with_has_many_and_limit_and_scoped_and_explicit_conditions
+    Post.with_scope(:find => { :conditions => "1=1" }) do
+      posts = authors(:david).posts.find(:all, 
+        :joins    => :comments, 
+        :conditions => "comments.body like 'Normal%' OR comments.#{QUOTED_TYPE}= 'SpecialComment'",
+        :limit      => 2
+      )
+      assert_equal 2, posts.size
+      
+      count = Post.count(
+        :joins    => [ :comments, :author ], 
+        :conditions => "authors.name = 'David' AND (comments.body like 'Normal%' OR comments.#{QUOTED_TYPE}= 'SpecialComment')",
+        :limit      => 2
+      )
+      assert_equal count, posts.size
+    end
+  end
+  
+  def test_ar_join_with_scoped_order_using_association_limiting_without_explicit_scope
+    posts_with_explicit_order = Post.find(:all, :conditions => 'comments.id is not null', :joins => :comments, :order => 'posts.id DESC', :limit => 2)
+    posts_with_scoped_order = Post.with_scope(:find => {:order => 'posts.id DESC'}) do
+      Post.find(:all, :conditions => 'comments.id is not null', :joins => :comments, :limit => 2)
+    end
+    assert_equal posts_with_explicit_order, posts_with_scoped_order
+  end
+  
+  def test_scoped_find_include
+    # with the include, will retrieve only developers for the given project
+    scoped_developers = Developer.with_scope(:find => { :joins => :projects }) do
+      Developer.find(:all, :conditions => 'projects.id = 2')
+    end
+    assert scoped_developers.include?(developers(:david))
+    assert !scoped_developers.include?(developers(:jamis))
+    assert_equal 1, scoped_developers.size
+  end
+  
+  
+  def test_nested_scoped_find_ar_join
+    Developer.with_scope(:find => { :joins => :projects }) do
+      Developer.with_scope(:find => { :conditions => "projects.id = 2" }) do
+        assert_equal('David', Developer.find(:first).name)
+      end
+    end  
+  end
+
+  def test_nested_scoped_find_merged_ar_join
+    # :include's remain unique and don't "double up" when merging
+    Developer.with_scope(:find => { :joins => :projects, :conditions => "projects.id = 2" }) do
+      Developer.with_scope(:find => { :joins => :projects }) do
+        assert_equal 1, Developer.instance_eval('current_scoped_methods')[:find][:ar_joins].length
+        assert_equal('David', Developer.find(:first).name)
+      end
+    end
+    # the nested scope doesn't remove the first :include
+    Developer.with_scope(:find => { :joins => :projects, :conditions => "projects.id = 2" }) do
+      Developer.with_scope(:find => { :joins => [] }) do
+        assert_equal 1, Developer.instance_eval('current_scoped_methods')[:find][:ar_joins].length 
+        assert_equal('David', Developer.find(:first).name)
+      end
+    end  
+    # mixing array and symbol include's will merge correctly
+    Developer.with_scope(:find => { :joins => [:projects], :conditions => "projects.id = 2" }) do
+      Developer.with_scope(:find => { :joins => :projects }) do
+        assert_equal 1, Developer.instance_eval('current_scoped_methods')[:find][:ar_joins].length
+        assert_equal('David', Developer.find(:first).name)
+      end
+    end       
+  end 
+
+  def test_nested_scoped_find_replace_include
+    Developer.with_scope(:find => { :joins => :projects }) do
+      Developer.with_exclusive_scope(:find => { :joins => [] }) do
+        assert_equal 0, Developer.instance_eval('current_scoped_methods')[:find][:ar_joins].length
+      end
+    end  
+  end
+  
+#
+# Calculations
+#  
+  def test_count_with_ar_joins
+    assert_equal(2, Author.count(:joins => :posts, :conditions => ['posts.type = ?', "Post"]))
+    assert_equal(1, Author.count(:joins => :posts, :conditions => ['posts.type = ?', "SpecialPost"]))
+  end  
+  
+  def test_should_get_maximum_of_field_with_joins
+    assert_equal 50, Account.maximum(:credit_limit, :joins=> :firm, :conditions => "companies.name != 'Summit'")
+  end 
+  
+  def test_should_get_maximum_of_field_with_scoped_include
+    Account.with_scope :find => { :joins => :firm, :conditions => "companies.name != 'Summit'" } do
+      assert_equal 50, Account.maximum(:credit_limit)
+    end
+  end
+  
+  def test_should_not_modify_options_when_using_ar_joins_on_count
+    options = {:conditions => 'companies.id > 1', :joins => :firm}
+    options_copy = options.dup
+    
+    Account.count(:all, options)
+    assert_equal options_copy, options
+  end
+  
+end
+

activerecord/lib/active_record/calculations.rb

@@ -15 +15 @@
-      # The third approach, count using options, accepts an option hash as the only parameter. The options are:
-      #
-      # * <tt>:conditions</tt>: An SQL fragment like "administrator = 1" or [ "user_name = ?", username ]. See conditions in the intro.
-
-
+
+
+
+
+
-      # * <tt>:include</tt>: Named associations that should be loaded alongside using LEFT OUTER JOINs. The symbols named refer
-      #   to already defined associations. When using named associations count returns the number DISTINCT items for the model you're counting.
-      #   See eager loading under Associations.
@@ -109 +112 @@
-      #   Person.minimum(:age, :conditions => ['last_name != ?', 'Drake']) # Selects the minimum age for everyone with a last name other than 'Drake'
-      #   Person.minimum(:age, :having => 'min(age) > 17', :group => :last_name) # Selects the minimum age for any family without any minors
-      def calculate(operation, column_name, options = {})
+        options, ar_joins = *extract_ar_join_from_options(options)
-        validate_calculation_options(operation, options)
+        options[:ar_joins] = ar_joins if ar_joins
-        column_name     = options[:select] if options[:select]
-        column_name     = '*' if column_name == :all
-        column          = column_for column_name
@@ -150 +155 @@
-          options = options.symbolize_keys
-
-          scope           = scope(:find)
+          if scope &&scope[:ar_joins]
+            scope = scope.dup
+            options = options.dup
+            options[:ar_joins] = scope.delete(:ar_joins)
+          end
-          merged_includes = merge_includes(scope ? scope[:include] : [], options[:include])
+          merged_includes = merge_includes(merged_includes, options[:ar_joins])
-          aggregate_alias = column_alias_for(operation, column_name)
-
-          if operation == 'count'
@@ -173 +184 @@
-          sql << " FROM (SELECT DISTINCT #{column_name}" if use_workaround
-          sql << " FROM #{table_name} "
-          if merged_includes.any?
-
+, options[:ar_joins]
-            sql << join_dependency.join_associations.collect{|join| join.association_join }.join
-          end
-          add_joins!(sql, options, scope)

activerecord/lib/active_record/associations.rb

@@ -486 +486 @@
-    # 
-    # When eager loaded, conditions are interpolated in the context of the model class, not the model instance.  Conditions are lazily interpolated
-    # before the actual model exists.
-
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
-    # == Table Aliasing
-    #
-    # ActiveRecord uses table aliasing in the case that a table is referenced multiple times in a join.  If a table is referenced only once,
@@ -1121 +1177 @@
-        
-        def find_with_associations(options = {})
-          catch :invalid_query do
-
+
+
+
+
+
+
+
-            rows = select_all_rows(options, join_dependency)
-            return join_dependency.instantiate(rows)
-          end
@@ -1375 +1437 @@
-        class JoinDependency # :nodoc:
-          attr_reader :joins, :reflections, :table_aliases
-
-
+, ar_joins = nil
-            @joins                 = [JoinBase.new(base, joins)]
+            @ar_joins              = ar_joins
-            @associations          = associations
-            @reflections           = []
-            @base_records_hash     = {}
@@ -1400 +1463 @@
-              unless @base_records_hash[primary_id]
-                @base_records_in_order << (@base_records_hash[primary_id] = join_base.instantiate(row))
-              end
-
+ unless @ar_joins
-            end
-
+ unless @ar_joins
-            return @base_records_in_order
-          end
-
@@ -1444 +1507 @@
-                  reflection = parent.reflections[associations.to_s.intern] or
-                  raise ConfigurationError, "Association named '#{ associations }' was not found; perhaps you misspelled it?"
-                  @reflections << reflection
-JoinAssociation
+(@ar_joins ? ARJoinAssociation : JoinAssociation)
-                when Array
-                  associations.each do |association|
-                    build(association, parent)
@@ -1548 +1611 @@
-            end
-          end
-
-
+     
-            attr_reader :reflection, :parent, :aliased_table_name, :aliased_prefix, :aliased_join_table_name, :parent_table_name
-            delegate    :options, :klass, :through_reflection, :source_reflection, :to => :reflection
-
@@ -1591 +1654 @@
-                end
-              end
-            end
-
+
-            def association_join
-              join = case reflection.macro
-                when :has_and_belongs_to_many
-LEFT OUTER JOIN
+#{join_type}
-                     table_alias_for(options[:join_table], aliased_join_table_name),
-                     aliased_join_table_name,
-                     options[:foreign_key] || reflection.active_record.to_s.foreign_key,
-                     parent.aliased_table_name, reflection.active_record.primary_key] +
-LEFT OUTER JOIN
+#{join_type}
-                     table_name_and_alias, aliased_table_name, klass.primary_key,
-                     aliased_join_table_name, options[:association_foreign_key] || klass.to_s.foreign_key
-                     ]
@@ -1658 +1721 @@
-                        end
-                      end
-
-LEFT OUTER JOIN
+#{join_type}
-                        table_alias_for(through_reflection.klass.table_name, aliased_join_table_name),
-                        parent.aliased_table_name, reflection.active_record.connection.quote_column_name(parent.primary_key),
-                        aliased_join_table_name, reflection.active_record.connection.quote_column_name(jt_foreign_key), 
-                        jt_as_extra, jt_source_extra, jt_sti_extra
-                      ] +
-LEFT OUTER JOIN
+#{join_type}
-                        table_name_and_alias, 
-                        aliased_table_name, reflection.active_record.connection.quote_column_name(first_key),
-                        aliased_join_table_name, reflection.active_record.connection.quote_column_name(second_key),
@@ -1672 +1735 @@
-                      ]
-
-                    when reflection.options[:as] && [:has_many, :has_one].include?(reflection.macro)
-LEFT OUTER JOIN
+#{join_type}
-                        table_name_and_alias,
-                        aliased_table_name, "#{reflection.options[:as]}_id",
-                        parent.aliased_table_name, parent.primary_key,
@@ -1681 +1744 @@
-                      ]
-                    else
-                      foreign_key = options[:foreign_key] || reflection.active_record.name.foreign_key
-LEFT OUTER JOIN
+#{join_type}
-                        table_name_and_alias,
-                        aliased_table_name, foreign_key,
-                        parent.aliased_table_name, parent.primary_key
-                      ]
-                  end
-                when :belongs_to
-LEFT OUTER JOIN
+#{join_type}
-                     table_name_and_alias, aliased_table_name, reflection.klass.primary_key,
-                     parent.aliased_table_name, options[:foreign_key] || klass.to_s.foreign_key
-                    ]
@@ -1723 +1786 @@
-
-              def interpolate_sql(sql)
-                instance_eval("%@#{sql.gsub('@', '\@')}@") 
-
+
+
+
+
+
+
+
-          end
+          class ARJoinAssociation < JoinAssociation 
+            private
+              def join_type
+                "INNER JOIN"
+              end
+          end
-        end
-    end
-  end

activerecord/lib/active_record/base.rb

@@ -380 +380 @@
-      # * <tt>:group</tt>: An attribute name by which the result should be grouped. Uses the GROUP BY SQL-clause.
-      # * <tt>:limit</tt>: An integer determining the limit on the number of rows that should be returned.
-      # * <tt>:offset</tt>: An integer determining the offset from where the rows should be fetched. So at 5, it would skip rows 0 through 4.
-
-
+
+
+
-      #   Pass :readonly => false to override.
+      #   See adding joins for associations under Association.
-      # * <tt>:include</tt>: Names associations that should be loaded alongside using LEFT OUTER JOINs. The symbols named refer
-      #   to already defined associations. See eager loading under Associations.
-      # * <tt>:select</tt>: By default, this is * as in SELECT * FROM, but can be changed if you for example want to do a join, but not
@@ -428 +430 @@
-      #   end
-      def find(*args)
-        options = args.extract_options!
+        # Note:  we extract any :joins option with a non-string value from the options, and turn it into
+        #  an internal option :ar_joins.  This allows code called from her to find the ar_joins, and
+        #  it bypasses marking the result as read_only.
+        #  A normal string join marks the result as read-only because it contains attributes from joined tables
+        #  which are not in the base table and therefore prevent the result from being saved.
+        #  In the case of an ar_join, the JoinDependency created to instantiate the results eliminates these
+        #  bogus attributes.  See JoinDependency#instantiate, and JoinBase#instantiate in associations.rb.
+        options, ar_joins = *extract_ar_join_from_options(options)
-        validate_find_options(options)
-        set_readonly_option!(options)
+        options[:ar_joins] = ar_joins if ar_joins
-
-        case args.first
-          when :first then find_initial(options)
@@ -1019 +1030 @@
-          options.update(:limit => 1) unless options[:include]
-          find_every(options).first
-        end
-
+
+
+
+
+
+
+
+
+
+
-        def find_every(options)
-
+|| scoped?(:find, :ar_joins) || (options[:ar_joins]) 
-            find_with_associations(options) : 
-            find_by_sql(construct_finder_sql(options))
-
@@ -1445 +1465 @@
-
-          if f = method_scoping[:find]
-            f.assert_valid_keys(VALID_FIND_OPTIONS)
+            # see note about :joins and :ar_joins in ActiveRecord::Base#find
+            f, ar_joins = *extract_ar_join_from_options(f)
-            set_readonly_option! f
+            if ar_joins
+              f[:ar_joins] = ar_joins
+              method_scoping[:find] = f 
+            end                       
-          end
-
-          # Merge scopings
@@ -1458 +1484 @@
-                      merge = hash[method][key] && params[key] # merge if both scopes have the same key
-                      if key == :conditions && merge
-                        hash[method][key] = [params[key], hash[method][key]].collect{ |sql| "( %s )" % sanitize_sql(sql) }.join(" AND ")
-key == :include
+([:include, :ar_joins].include?(key))
-                        hash[method][key] = merge_includes(hash[method][key], params[key]).uniq
-                      else
-                        hash[method][key] = hash[method][key] || params[key]
@@ -1473 +1499 @@
-              hash
-            end
-          end
-
+
-          self.scoped_methods << method_scoping
-
-          begin