Merge branch 'master' into nested_has_many_through

15 files changed, 147 insertions, 67 deletions

diff --git a/activerecord/lib/active_record.rb b/activerecord/lib/active_record.rb
index e2f2508ae8..f692e5ac89 100644
--- a/activerecord/lib/active_record.rb
+++ b/activerecord/lib/active_record.rb
@@ -118,7 +118,7 @@ module ActiveRecord
 end
 
 ActiveSupport.on_load(:active_record) do
-  Arel::Table.engine = Arel::Sql::Engine.new(self)
+  Arel::Table.engine = self
 end
 
 I18n.load_path << File.dirname(__FILE__) + '/active_record/locale/en.yml'
diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb
index 91e0a9f2f8..cb2d9e0a79 100644
--- a/activerecord/lib/active_record/associations/association_collection.rb
+++ b/activerecord/lib/active_record/associations/association_collection.rb
@@ -363,6 +363,7 @@ module ActiveRecord
 
       def include?(record)
         return false unless record.is_a?(@reflection.klass)
+        return include_in_memory?(record) if record.new_record?
         load_target if @reflection.options[:finder_sql] && !loaded?
         return @target.include?(record) if loaded?
         exists?(record)
@@ -554,6 +555,17 @@ module ActiveRecord
           args.first.kind_of?(Hash) || !(loaded? || @owner.new_record? || @reflection.options[:finder_sql] ||
                                          @target.any? { |record| record.new_record? } || args.first.kind_of?(Integer))
         end
+
+        def include_in_memory?(record)
+          if @reflection.is_a?(ActiveRecord::Reflection::ThroughReflection)
+            @owner.send(proxy_reflection.through_reflection.name.to_sym).any? do |source|
+              target = source.send(proxy_reflection.source_reflection.name)
+              target.respond_to?(:include?) ? target.include?(record) : target == record
+            end
+          else
+            @target.include?(record)
+          end
+        end
     end
   end
 end
diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb
index 2157a0aded..ff6be4ff19 100644
--- a/activerecord/lib/active_record/base.rb
+++ b/activerecord/lib/active_record/base.rb
@@ -834,7 +834,7 @@ module ActiveRecord #:nodoc:
           if self == ActiveRecord::Base
             Arel::Table.engine
           else
-            connection_handler.connection_pools[name] ? Arel::Sql::Engine.new(self) : superclass.arel_engine
+            connection_handler.connection_pools[name] ? self : superclass.arel_engine
           end
         end
       end
@@ -884,13 +884,9 @@ module ActiveRecord #:nodoc:
         # single-table inheritance model that makes it possible to create
         # objects of different types from the same table.
         def instantiate(record)
-          find_sti_class(record[inheritance_column]).allocate.instance_eval do
-            @attributes, @attributes_cache, @previously_changed, @changed_attributes = record, {}, {}, {}
-            @new_record = @readonly = @destroyed = @marked_for_destruction = false
-            _run_find_callbacks
-            _run_initialize_callbacks
-            self
-          end
+          model = find_sti_class(record[inheritance_column]).allocate
+          model.init_with('attributes' => record)
+          model
         end
 
         def find_sti_class(type_name)
@@ -1274,8 +1270,10 @@ MSG
           attrs = expand_hash_conditions_for_aggregates(attrs)
 
           table = Arel::Table.new(self.table_name, :engine => arel_engine, :as => default_table_name)
-          builder = PredicateBuilder.new(arel_engine)
-          builder.build_from_hash(attrs, table).map{ |b| b.to_sql }.join(' AND ')
+          viz = Arel::Visitors.for(arel_engine)
+          PredicateBuilder.build_from_hash(arel_engine, attrs, table).map { |b|
+            viz.accept b
+          }.join(' AND ')
         end
         alias_method :sanitize_sql_hash, :sanitize_sql_hash_for_conditions
 
@@ -1416,6 +1414,24 @@ MSG
         populate_with_current_scope_attributes
       end
 
+      # Initialize an empty model object from +coder+.  +coder+ must contain
+      # the attributes necessary for initializing an empty model object.  For
+      # example:
+      #
+      #   class Post < ActiveRecord::Base
+      #   end
+      #
+      #   post = Post.allocate
+      #   post.init_with('attributes' => { 'title' => 'hello world' })
+      #   post.title # => 'hello world'
+      def init_with(coder)
+        @attributes = coder['attributes']
+        @attributes_cache, @previously_changed, @changed_attributes = {}, {}, {}
+        @new_record = @readonly = @destroyed = @marked_for_destruction = false
+        _run_find_callbacks
+        _run_initialize_callbacks
+      end
+
       # Returns a String, which Action Pack uses for constructing an URL to this
       # object. The default implementation returns this record's id as a String,
       # or nil if this record's unsaved.
diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
index 194842a9a0..5f14284615 100644
--- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
@@ -975,7 +975,7 @@ module ActiveRecord
         def select(sql, name = nil)
           fields, rows = select_raw(sql, name)
           rows.map do |row|
-            Hash[*fields.zip(row).flatten]
+            Hash[fields.zip(row)]
           end
         end
 
@@ -1017,11 +1017,11 @@ module ActiveRecord
         end
 
         def extract_pg_identifier_from_name(name)
-          match_data = name[0,1] == '"' ? name.match(/\"([^\"]+)\"/) : name.match(/([^\.]+)/)
+          match_data = name.start_with?('"') ? name.match(/\"([^\"]+)\"/) : name.match(/([^\.]+)/)
 
           if match_data
-            rest = name[match_data[0].length..-1]
-            rest = rest[1..-1] if rest[0,1] == "."
+            rest = name[match_data[0].length, name.length]
+            rest = rest[1, rest.length] if rest.start_with? "."
             [match_data[1], (rest.length > 0 ? rest : nil)]
           end
         end
diff --git a/activerecord/lib/active_record/migration.rb b/activerecord/lib/active_record/migration.rb
index e708b3fbcf..9ac18f9939 100644
--- a/activerecord/lib/active_record/migration.rb
+++ b/activerecord/lib/active_record/migration.rb
@@ -419,9 +419,12 @@ module ActiveRecord
 
   # MigrationProxy is used to defer loading of the actual migration classes
   # until they are needed
-  class MigrationProxy
+  class MigrationProxy < Struct.new(:name, :version, :filename, :scope)
 
-    attr_accessor :name, :version, :filename, :scope
+    def initialize(name, version, filename, scope)
+      super
+      @migration = nil
+    end
 
     delegate :migrate, :announce, :write, :to=>:migration
 
@@ -504,26 +507,21 @@ module ActiveRecord
       def migrations(path)
         files = Dir["#{path}/[0-9]*_*.rb"]
 
-        migrations = files.inject([]) do |klasses, file|
+        seen = Hash.new false
+
+        migrations = files.map do |file|
           version, name, scope = file.scan(/([0-9]+)_([_a-z0-9]*)\.?([_a-z0-9]*)?.rb/).first
 
           raise IllegalMigrationNameError.new(file) unless version
           version = version.to_i
+          name = name.camelize
 
-          if klasses.detect { |m| m.version == version }
-            raise DuplicateMigrationVersionError.new(version)
-          end
+          raise DuplicateMigrationVersionError.new(version) if seen[version]
+          raise DuplicateMigrationNameError.new(name) if seen[[name, scope]]
 
-          if klasses.detect { |m| m.name == name.camelize && m.scope == scope }
-            raise DuplicateMigrationNameError.new(name.camelize)
-          end
+          seen[version] = seen[[name, scope]] = true
 
-          migration = MigrationProxy.new
-          migration.name     = name.camelize
-          migration.version  = version
-          migration.filename = file
-          migration.scope    = scope
-          klasses << migration
+          MigrationProxy.new(name, version, file, scope)
         end
 
         migrations.sort_by(&:version)
@@ -570,7 +568,7 @@ module ActiveRecord
       current = migrations.detect { |m| m.version == current_version }
       target = migrations.detect { |m| m.version == @target_version }
 
-      if target.nil? && !@target_version.nil? && @target_version > 0
+      if target.nil? && @target_version && @target_version > 0
         raise UnknownMigrationVersionError.new(@target_version)
       end
 
@@ -579,16 +577,18 @@ module ActiveRecord
       runnable = migrations[start..finish]
 
       # skip the last migration if we're headed down, but not ALL the way down
-      runnable.pop if down? && !target.nil?
+      runnable.pop if down? && target
 
       runnable.each do |migration|
         Base.logger.info "Migrating to #{migration.name} (#{migration.version})" if Base.logger
 
+        seen = migrated.include?(migration.version.to_i)
+
         # On our way up, we skip migrating the ones we've already migrated
-        next if up? && migrated.include?(migration.version.to_i)
+        next if up? && seen
 
         # On our way down, we skip reverting the ones we've never migrated
-        if down? && !migrated.include?(migration.version.to_i)
+        if down? && !seen
           migration.announce 'never migrated, skipping'; migration.write
           next
         end
diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb
index 03862c78e4..d79ef78b4d 100644
--- a/activerecord/lib/active_record/relation/calculations.rb
+++ b/activerecord/lib/active_record/relation/calculations.rb
@@ -183,12 +183,16 @@ module ActiveRecord
       end
     end
 
-    def execute_simple_calculation(operation, column_name, distinct) #:nodoc:
-      column = if @klass.column_names.include?(column_name.to_s)
+    def aggregate_column(column_name)
+      if @klass.column_names.include?(column_name.to_s)
         Arel::Attribute.new(@klass.unscoped.table, column_name)
       else
-        Arel::SqlLiteral.new(column_name == :all ? "*" : column_name.to_s)
+        Arel.sql(column_name == :all ? "*" : column_name.to_s)
       end
+    end
+
+    def execute_simple_calculation(operation, column_name, distinct) #:nodoc:
+      column = aggregate_column(column_name)
 
       # Postgresql doesn't like ORDER BY when there are no GROUP BY
       relation = except(:order)
@@ -209,18 +213,17 @@ module ActiveRecord
 
       group = @klass.connection.adapter_name == 'FrontBase' ? group_alias : group_field
 
-      aggregate_alias = column_alias_for(operation, column_name)
-
-      select_statement = if operation == 'count' && column_name == :all
-        ["COUNT(*) AS count_all"]
+      if operation == 'count' && column_name == :all
+        aggregate_alias = 'count_all'
       else
-        [Arel::Attribute.new(@klass.unscoped.table, column_name).send(operation).as(aggregate_alias)]
+        aggregate_alias = column_alias_for(operation, column_name)
       end
 
-      select_statement <<  "#{group_field} AS #{group_alias}"
-
       relation = except(:group).group(group)
-      relation.select_values = select_statement
+      relation.select_values = [
+        aggregate_column(column_name).send(operation).as(aggregate_alias),
+        "#{group_field} AS #{group_alias}"
+      ]
 
       calculated_data = @klass.connection.select_all(relation.to_sql)
 
diff --git a/activerecord/lib/active_record/relation/predicate_builder.rb b/activerecord/lib/active_record/relation/predicate_builder.rb
index 0d1307d87e..c5428dccd6 100644
--- a/activerecord/lib/active_record/relation/predicate_builder.rb
+++ b/activerecord/lib/active_record/relation/predicate_builder.rb
@@ -1,23 +1,18 @@
 module ActiveRecord
-  class PredicateBuilder
-
-    def initialize(engine)
-      @engine = engine
-    end
-
-    def build_from_hash(attributes, default_table)
+  class PredicateBuilder # :nodoc:
+    def self.build_from_hash(engine, attributes, default_table)
       predicates = attributes.map do |column, value|
         table = default_table
 
         if value.is_a?(Hash)
-          table = Arel::Table.new(column, :engine => @engine)
-          build_from_hash(value, table)
+          table = Arel::Table.new(column, :engine => engine)
+          build_from_hash(engine, value, table)
         else
           column = column.to_s
 
           if column.include?('.')
             table_name, column = column.split('.', 2)
-            table = Arel::Table.new(table_name, :engine => @engine)
+            table = Arel::Table.new(table_name, :engine => engine)
           end
 
           attribute = table[column] || Arel::Attribute.new(table, column)
@@ -38,6 +33,5 @@ module ActiveRecord
 
       predicates.flatten
     end
-
   end
 end
diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb
index acc42faf7d..f314ff861f 100644
--- a/activerecord/lib/active_record/relation/query_methods.rb
+++ b/activerecord/lib/active_record/relation/query_methods.rb
@@ -196,20 +196,20 @@ module ActiveRecord
       arel
     end
 
+    private
+
     def build_where(opts, other = [])
       case opts
       when String, Array
         [@klass.send(:sanitize_sql, other.empty? ? opts : ([opts] + other))]
       when Hash
         attributes = @klass.send(:expand_hash_conditions_for_aggregates, opts)
-        PredicateBuilder.new(table.engine).build_from_hash(attributes, table)
+        PredicateBuilder.build_from_hash(table.engine, attributes, table)
       else
         [opts]
       end
     end
 
-    private
-
     def build_joins(relation, joins)
       association_joins = []
 
diff --git a/activerecord/lib/active_record/schema_dumper.rb b/activerecord/lib/active_record/schema_dumper.rb
index 6faa88ab78..f5331bb8a9 100644
--- a/activerecord/lib/active_record/schema_dumper.rb
+++ b/activerecord/lib/active_record/schema_dumper.rb
@@ -176,13 +176,15 @@ HEADER
       def indexes(table, stream)
         if (indexes = @connection.indexes(table)).any?
           add_index_statements = indexes.map do |index|
-            statement_parts = [ ('add_index ' + index.table.inspect) ]
-            statement_parts << index.columns.inspect
-            statement_parts << (':name => ' + index.name.inspect)
+            statement_parts = [
+              ('add_index ' + index.table.inspect),
+              index.columns.inspect,
+              (':name => ' + index.name.inspect),
+            ]
             statement_parts << ':unique => true' if index.unique
 
-            index_lengths = index.lengths.compact if index.lengths.is_a?(Array)
-            statement_parts << (':length => ' + Hash[*index.columns.zip(index.lengths).flatten].inspect) if index_lengths.present?
+            index_lengths = (index.lengths || []).compact
+            statement_parts << (':length => ' + Hash[index.columns.zip(index.lengths)].inspect) unless index_lengths.empty?
 
             '  ' + statement_parts.join(', ')
           end
diff --git a/activerecord/lib/active_record/session_store.rb b/activerecord/lib/active_record/session_store.rb
index 01cc14b8d6..3fc596e02a 100644
--- a/activerecord/lib/active_record/session_store.rb
+++ b/activerecord/lib/active_record/session_store.rb
@@ -288,6 +288,7 @@ module ActiveRecord
     self.session_class = Session
 
     SESSION_RECORD_KEY = 'rack.session.record'
+    ENV_SESSION_OPTIONS_KEY = Rack::Session::Abstract::ENV_SESSION_OPTIONS_KEY
 
     private
       def get_session(env, sid)
@@ -299,7 +300,7 @@ module ActiveRecord
         end
       end
 
-      def set_session(env, sid, session_data)
+      def set_session(env, sid, session_data, options)
         Base.silence do
           record = get_session_model(env, sid)
           record.data = session_data
@@ -316,12 +317,14 @@ module ActiveRecord
         sid
       end
 
-      def destroy(env)
+      def destroy_session(env, session_id, options)
         if sid = current_session_id(env)
           Base.silence do
             get_session_model(env, sid).destroy
           end
         end
+
+        generate_sid unless options[:drop]
       end
 
       def get_session_model(env, sid)
diff --git a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb
index c0be7dfdcc..7e070e1746 100644
--- a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb
+++ b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb
@@ -858,4 +858,10 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase
     assert_equal new_developer.name, "Marcelo"
     assert_equal new_developer.salary, 90_000
   end
+
+  def test_include_method_in_has_and_belongs_to_many_association_should_return_true_for_instance_added_with_build
+    project = Project.new
+    developer = project.developers.build
+    assert project.developers.include?(developer)
+  end
 end
diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb
index 720b7fc386..c9f00fd737 100644
--- a/activerecord/test/cases/associations/has_many_associations_test.rb
+++ b/activerecord/test/cases/associations/has_many_associations_test.rb
@@ -1267,4 +1267,10 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
     assert_equal new_comment.type, "SpecialComment"
     assert_equal new_comment.post_id, posts(:welcome).id
   end
+
+  def test_include_method_in_has_many_association_should_return_true_for_instance_added_with_build
+    post = Post.new
+    comment = post.comments.build
+    assert post.comments.include?(comment)
+  end
 end
diff --git a/activerecord/test/cases/associations/has_many_through_associations_test.rb b/activerecord/test/cases/associations/has_many_through_associations_test.rb
index 0dac633852..4b9f49f1ec 100644
--- a/activerecord/test/cases/associations/has_many_through_associations_test.rb
+++ b/activerecord/test/cases/associations/has_many_through_associations_test.rb
@@ -435,4 +435,18 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase
     assert_equal new_subscriber.nick, "marklazz"
     assert_equal new_subscriber.name, "Marcelo Giorgi"
   end
+
+  def test_include_method_in_association_through_should_return_true_for_instance_added_with_build
+    person = Person.new
+    reference = person.references.build
+    job = reference.build_job
+    assert person.jobs.include?(job)
+  end
+
+  def test_include_method_in_association_through_should_return_true_for_instance_added_with_nested_builds
+    author = Author.new
+    post = author.posts.build
+    comment = post.comments.build
+    assert author.comments.include?(comment)
+  end
 end
diff --git a/activerecord/test/cases/calculations_test.rb b/activerecord/test/cases/calculations_test.rb
index afef31396e..7ec40906d4 100644
--- a/activerecord/test/cases/calculations_test.rb
+++ b/activerecord/test/cases/calculations_test.rb
@@ -275,6 +275,17 @@ class CalculationsTest < ActiveRecord::TestCase
     assert_equal 2, Account.count(:firm_id, :conditions => "credit_limit = 50 AND firm_id IS NOT NULL")
   end
 
+  def test_should_count_field_in_joined_table
+    assert_equal 5, Account.count('companies.id', :joins => :firm)
+    assert_equal 4, Account.count('companies.id', :joins => :firm, :distinct => true)
+  end
+
+  def test_should_count_field_in_joined_table_with_group_by
+    c = Account.count('companies.id', :group => 'accounts.firm_id', :joins => :firm)
+
+    [1,6,2,9].each { |firm_id| assert c.keys.include?(firm_id) }
+  end
+
   def test_count_with_no_parameters_isnt_deprecated
     assert_not_deprecated { Account.count }
   end
@@ -335,5 +346,4 @@ class CalculationsTest < ActiveRecord::TestCase
   def test_from_option_with_table_different_than_class
     assert_equal Account.count(:all), Company.count(:all, :from => 'accounts')
   end
-
 end
diff --git a/activerecord/test/cases/inheritance_test.rb b/activerecord/test/cases/inheritance_test.rb
index 8c09fc4d59..31679b2efe 100644
--- a/activerecord/test/cases/inheritance_test.rb
+++ b/activerecord/test/cases/inheritance_test.rb
@@ -14,6 +14,20 @@ class InheritanceTest < ActiveRecord::TestCase
     ActiveRecord::Base.store_full_sti_class = old
   end
 
+  def test_class_with_blank_sti_name
+    company = Company.find(:first)
+    company = company.clone
+    company.extend(Module.new {
+      def read_attribute(name)
+        return '  ' if name == 'type'
+        super
+      end
+    })
+    company.save!
+    company = Company.find(:all).find { |x| x.id == company.id }
+    assert_equal '  ', company.type
+  end
+
   def test_class_without_store_full_sti_class_returns_demodulized_name
     old = ActiveRecord::Base.store_full_sti_class
     ActiveRecord::Base.store_full_sti_class = false