Changeset 8109

Timestamp:

11/07/07 15:07:39 (6 months ago)

Author:

david

Message:

Address shortcomings of changeset [8054] [protocool]

Files:

• trunk/activerecord/lib/active_record/associations.rb (modified) (8 diffs)
• trunk/activerecord/lib/active_record/base.rb (modified) (6 diffs)
• trunk/activerecord/lib/active_record/calculations.rb (modified) (4 diffs)
• trunk/activerecord/test/associations/ar_joins_test.rb (modified) (1 diff)
• trunk/activerecord/test/associations/inner_join_association_test.rb (added)

trunk/activerecord/lib/active_record/associations.rb

@@ -487 +487 @@
-    # 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
-    #
@@ -1178 +1122 @@
-        def find_with_associations(options = {})
-          catch :invalid_query do
-
-
-
-
-
-
-
+
-            rows = select_all_rows(options, join_dependency)
-            return join_dependency.instantiate(rows)
@@ -1438 +1376 @@
-          attr_reader :joins, :reflections, :table_aliases
-
-, ar_joins = nil
+
-            @joins                 = [JoinBase.new(base, joins)]
-            @ar_joins              = ar_joins
-            @associations          = associations
-            @reflections           = []
@@ -1464 +1401 @@
-                @base_records_in_order << (@base_records_hash[primary_id] = join_base.instantiate(row))
-              end
- unless @ar_joins
-
- unless @ar_joins
+
+
+
-            return @base_records_in_order
-          end
@@ -1508 +1445 @@
-                  raise ConfigurationError, "Association named '#{ associations }' was not found; perhaps you misspelled it?"
-                  @reflections << reflection
-(@ar_joins ? ARJoinAssociation : JoinAssociation).new(reflection, self
+build_join_association(reflection
-                when Array
-                  associations.each do |association|
@@ -1521 +1458 @@
-                  raise ConfigurationError, associations.inspect
-              end
+            end
+
+            # overridden in InnerJoinDependency subclass
+            def build_join_association(reflection, parent)
+              JoinAssociation.new(reflection, self, parent)
-            end
-
@@ -1787 +1729 @@
-              def interpolate_sql(sql)
-                instance_eval("%@#{sql.gsub('@', '\@')}@") 
-
-
-
+
+
+
+
-              def join_type
-                "LEFT OUTER JOIN"
-              end
-
-
-
+
+
+
+
+
+
+
+
+
+
-            private
-              def join_type
@@ -1802 +1752 @@
-          end
-        end
+
-    end
-  end

trunk/activerecord/lib/active_record/base.rb

@@ -381 +381 @@
-      # * <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.
-Either an SQL fragment for additional joins like "LEFT JOIN comments ON comments.post_id = id".
- or names associations in the same form used for the :include option
-If the value is a string, then t
+An SQL fragment for additional joins like "LEFT JOIN comments ON comments.post_id = id"
+Accepts named associations in the form of :include, which will perform an INNER JOIN on the associated table(s)
+T
-      #   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.
@@ -431 +430 @@
-      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
@@ -1039 +1029 @@
-        end
-
-        # If options contains :joins, with a non-string value
-        #  remove it from options
-        # return the updated or unchanged options, and the ar_join value or nil
-        def extract_ar_join_from_options(options)
-          new_options = options.dup
-          join_option = new_options.delete(:joins)
-          (join_option && !join_option.kind_of?(String)) ? [new_options, join_option] : [options, nil]
-        end
-
-        def find_every(options)
-|| scoped?(:find, :ar_joins) || (options[:ar_joins]) 
+
-            find_with_associations(options) : 
-            find_by_sql(construct_finder_sql(options))
@@ -1247 +1228 @@
-          scope = scope(:find) if :auto == scope
-          join = (scope && scope[:joins]) || options[:joins]
-
+
+
+
+
+
+
+
-        end
-
@@ -1473 +1460 @@
-          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
-
@@ -1492 +1473 @@
-                      if key == :conditions && merge
-                        hash[method][key] = [params[key], hash[method][key]].collect{ |sql| "( %s )" % sanitize_sql(sql) }.join(" AND ")
-([:include, :ar_joins].include?(key))
+key == :include
-                        hash[method][key] = merge_includes(hash[method][key], params[key]).uniq
-                      else

trunk/activerecord/lib/active_record/calculations.rb

@@ -16 +16 @@
-      #
-      # * <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.
@@ -113 +110 @@
-      #   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
@@ -155 +150 @@
-          options = options.symbolize_keys
-
-
-
-
-
-
-
+
-          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)
-
@@ -185 +174 @@
-          sql << " FROM #{connection.quote_table_name(table_name)} "
-          if merged_includes.any?
-, options[:ar_joins]
+
-            sql << join_dependency.join_associations.collect{|join| join.association_join }.join
-          end

trunk/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