Merge remote branch 'rails/master'

14 files changed, 99 insertions, 54 deletions

diff --git a/activerecord/lib/active_record/association_preload.rb b/activerecord/lib/active_record/association_preload.rb
index cbec5789fd..08601f8ef9 100644
--- a/activerecord/lib/active_record/association_preload.rb
+++ b/activerecord/lib/active_record/association_preload.rb
@@ -112,13 +112,13 @@ module ActiveRecord
         # Not all records have the same class, so group then preload
         # group on the reflection itself so that if various subclass share the same association then we do not split them
         # unnecessarily
-        records.group_by {|record| class_to_reflection[record.class] ||= record.class.reflections[association]}.each do |reflection, records|
+        records.group_by { |record| class_to_reflection[record.class] ||= record.class.reflections[association]}.each do |reflection, _records|
           raise ConfigurationError, "Association named '#{ association }' was not found; perhaps you misspelled it?" unless reflection
 
           # 'reflection.macro' can return 'belongs_to', 'has_many', etc. Thus,
           # the following could call 'preload_belongs_to_association',
           # 'preload_has_many_association', etc.
-          send("preload_#{reflection.macro}_association", records, reflection, preload_options)
+          send("preload_#{reflection.macro}_association", _records, reflection, preload_options)
         end
       end
 
@@ -378,7 +378,7 @@ module ActiveRecord
           :order => preload_options[:order] || options[:order]
         }
 
-        reflection.klass.unscoped.apply_finder_options(find_options).to_a
+        reflection.klass.scoped.apply_finder_options(find_options).to_a
       end
 
 
diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb
index d67df64f59..1b9b725dd4 100644
--- a/activerecord/lib/active_record/associations.rb
+++ b/activerecord/lib/active_record/associations.rb
@@ -3,6 +3,7 @@ require 'active_support/core_ext/enumerable'
 require 'active_support/core_ext/module/delegation'
 require 'active_support/core_ext/object/blank'
 require 'active_support/core_ext/string/conversions'
+require 'active_support/core_ext/module/remove_method'
 
 module ActiveRecord
   class InverseOfAssociationNotFoundError < ActiveRecordError #:nodoc:
@@ -1354,7 +1355,7 @@ module ActiveRecord
         end
 
         def association_accessor_methods(reflection, association_proxy_class)
-          define_method(reflection.name) do |*params|
+          redefine_method(reflection.name) do |*params|
             force_reload = params.first unless params.empty?
             association = association_instance_get(reflection.name)
 
@@ -1371,12 +1372,12 @@ module ActiveRecord
             association.target.nil? ? nil : association
           end
 
-          define_method("loaded_#{reflection.name}?") do
+          redefine_method("loaded_#{reflection.name}?") do
             association = association_instance_get(reflection.name)
             association && association.loaded?
           end
-
-          define_method("#{reflection.name}=") do |new_value|
+          
+          redefine_method("#{reflection.name}=") do |new_value|
             association = association_instance_get(reflection.name)
 
             if association.nil? || association.target != new_value
@@ -1386,8 +1387,8 @@ module ActiveRecord
             association.replace(new_value)
             association_instance_set(reflection.name, new_value.nil? ? nil : association)
           end
-
-          define_method("set_#{reflection.name}_target") do |target|
+          
+          redefine_method("set_#{reflection.name}_target") do |target|
             return if target.nil? and association_proxy_class == BelongsToAssociation
             association = association_proxy_class.new(self, reflection)
             association.target = target
@@ -1396,7 +1397,7 @@ module ActiveRecord
         end
 
         def collection_reader_method(reflection, association_proxy_class)
-          define_method(reflection.name) do |*params|
+          redefine_method(reflection.name) do |*params|
             force_reload = params.first unless params.empty?
             association = association_instance_get(reflection.name)
 
@@ -1409,8 +1410,8 @@ module ActiveRecord
 
             association
           end
-
-          define_method("#{reflection.name.to_s.singularize}_ids") do
+          
+          redefine_method("#{reflection.name.to_s.singularize}_ids") do
             if send(reflection.name).loaded? || reflection.options[:finder_sql]
               send(reflection.name).map(&:id)
             else
@@ -1430,14 +1431,14 @@ module ActiveRecord
           collection_reader_method(reflection, association_proxy_class)
 
           if writer
-            define_method("#{reflection.name}=") do |new_value|
+            redefine_method("#{reflection.name}=") do |new_value|
               # Loads proxy class instance (defined in collection_reader_method) if not already loaded
               association = send(reflection.name)
               association.replace(new_value)
               association
             end
-
-            define_method("#{reflection.name.to_s.singularize}_ids=") do |new_value|
+            
+            redefine_method("#{reflection.name.to_s.singularize}_ids=") do |new_value|
               ids = (new_value || []).reject { |nid| nid.blank? }.map(&:to_i)
               send("#{reflection.name}=", reflection.klass.find(ids).index_by(&:id).values_at(*ids))
             end
@@ -1445,7 +1446,7 @@ module ActiveRecord
         end
 
         def association_constructor_method(constructor, reflection, association_proxy_class)
-          define_method("#{constructor}_#{reflection.name}") do |*params|
+          redefine_method("#{constructor}_#{reflection.name}") do |*params|
             attributees      = params.first unless params.empty?
             replace_existing = params[1].nil? ? true : params[1]
             association      = association_instance_get(reflection.name)
@@ -1486,8 +1487,8 @@ module ActiveRecord
         end
 
         def add_touch_callbacks(reflection, touch_attribute)
-          method_name = "belongs_to_touch_after_save_or_destroy_for_#{reflection.name}".to_sym
-          define_method(method_name) do
+          method_name = :"belongs_to_touch_after_save_or_destroy_for_#{reflection.name}"
+          redefine_method(method_name) do
             association = send(reflection.name)
 
             if touch_attribute == true
diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb
index f4e34657a5..f346a19a3a 100644
--- a/activerecord/lib/active_record/associations/association_collection.rb
+++ b/activerecord/lib/active_record/associations/association_collection.rb
@@ -218,9 +218,9 @@ module ActiveRecord
       # are actually removed from the database, that depends precisely on
       # +delete_records+. They are in any case removed from the collection.
       def delete(*records)
-        remove_records(records) do |records, old_records|
+        remove_records(records) do |_records, old_records|
           delete_records(old_records) if old_records.any?
-          records.each { |record| @target.delete(record) }
+          _records.each { |record| @target.delete(record) }
         end
       end
 
@@ -231,7 +231,7 @@ module ActiveRecord
       # ignoring the +:dependent+ option.
       def destroy(*records)
         records = find(records) if records.any? {|record| record.kind_of?(Fixnum) || record.kind_of?(String)}
-        remove_records(records) do |records, old_records|
+        remove_records(records) do |_records, old_records|
           old_records.each { |record| record.destroy }
         end
 
@@ -396,11 +396,12 @@ module ActiveRecord
                 if @target.is_a?(Array) && @target.any?
                   @target = find_target.map do |f|
                     i = @target.index(f)
-                    t = @target.delete_at(i) if i
-                    if t && t.changed?
-                      t
+                    if i
+                      @target.delete_at(i).tap do |t|
+                        keys = ["id"] + t.changes.keys + (f.attribute_names - t.attribute_names)
+                        t.attributes = f.attributes.except(*keys)
+                      end
                     else
-                      f.mark_for_destruction if t && t.marked_for_destruction?
                       f
                     end
                   end + @target
@@ -477,7 +478,14 @@ module ActiveRecord
           callback(:before_add, record)
           yield(record) if block_given?
           @target ||= [] unless loaded?
-          @target << record unless @reflection.options[:uniq] && @target.include?(record)
+          index = @target.index(record)
+          unless @reflection.options[:uniq] && index
+            if index
+              @target[index] = record
+            else
+             @target << record
+            end
+          end
           callback(:after_add, record)
           set_inverse_instance(record, @owner)
           record
diff --git a/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb b/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb
index c989c3536d..e61af93d1e 100644
--- a/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb
+++ b/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb
@@ -45,17 +45,23 @@ module ActiveRecord
           if @reflection.options[:insert_sql]
             @owner.connection.insert(interpolate_sql(@reflection.options[:insert_sql], record))
           else
-            relation = Arel::Table.new(@reflection.options[:join_table])
+            relation   = Arel::Table.new(@reflection.options[:join_table])
+            timestamps = record_timestamp_columns(record)
+            timezone   = record.send(:current_time_from_proper_timezone) if timestamps.any?
+
             attributes = columns.inject({}) do |attrs, column|
-              case column.name.to_s
+              name = column.name
+              case name.to_s
                 when @reflection.primary_key_name.to_s
-                  attrs[relation[column.name]] = owner_quoted_id
+                  attrs[relation[name]] = @owner.id
                 when @reflection.association_foreign_key.to_s
-                  attrs[relation[column.name]] = record.quoted_id
+                  attrs[relation[name]] = record.id
+                when *timestamps
+                  attrs[relation[name]] = timezone
                 else
-                  if record.has_attribute?(column.name)
-                    value = @owner.send(:quote_value, record[column.name], column)
-                    attrs[relation[column.name]] = value unless value.nil?
+                  if record.has_attribute?(name)
+                    value = @owner.send(:quote_value, record[name], column)
+                    attrs[relation[name]] = value unless value.nil?
                   end
               end
               attrs
@@ -117,6 +123,14 @@ module ActiveRecord
             build_record(attributes, &block)
           end
         end
+
+        def record_timestamp_columns(record)
+          if record.record_timestamps
+            record.send(:all_timestamp_attributes).map(&:to_s)
+          else
+            []
+          end
+        end
     end
   end
 end
diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
index be8d1bd76b..6072481411 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
@@ -199,11 +199,14 @@ module ActiveRecord
 
         def log(sql, name)
           name ||= "SQL"
-          result = nil
-          ActiveSupport::Notifications.instrument("sql.active_record",
-            :sql => sql, :name => name, :connection_id => self.object_id) do
-            @runtime += Benchmark.ms { result = yield }
+          instrumenter = ActiveSupport::Notifications.instrumenter
+
+          result = instrumenter.instrument("sql.active_record",
+            :sql => sql, :name => name, :connection_id => object_id) do
+            yield
           end
+          @runtime += instrumenter.elapsed
+
           result
         rescue Exception => e
           message = "#{e.class.name}: #{e.message}: #{sql}"
diff --git a/activerecord/lib/active_record/fixtures.rb b/activerecord/lib/active_record/fixtures.rb
index 5bf43b3a72..657303fd14 100644
--- a/activerecord/lib/active_record/fixtures.rb
+++ b/activerecord/lib/active_record/fixtures.rb
@@ -871,7 +871,7 @@ module ActiveRecord
         table_names.each do |table_name|
           table_name = table_name.to_s.tr('./', '_')
 
-          define_method(table_name) do |*fixtures|
+          redefine_method(table_name) do |*fixtures|
             force_reload = fixtures.pop if fixtures.last == true || fixtures.last == :reload
 
             @fixture_cache[table_name] ||= {}
diff --git a/activerecord/lib/active_record/log_subscriber.rb b/activerecord/lib/active_record/log_subscriber.rb
index 71065f9908..278e192e59 100644
--- a/activerecord/lib/active_record/log_subscriber.rb
+++ b/activerecord/lib/active_record/log_subscriber.rb
@@ -6,14 +6,16 @@ module ActiveRecord
     end
 
     def sql(event)
+      return unless logger.debug?
+
       name = '%s (%.1fms)' % [event.payload[:name], event.duration]
       sql  = event.payload[:sql].squeeze(' ')
 
       if odd?
-        name = color(name, :cyan, true)
+        name = color(name, CYAN, true)
         sql  = color(sql, nil, true)
       else
-        name = color(name, :magenta, true)
+        name = color(name, MAGENTA, true)
       end
 
       debug "  #{name}  #{sql}"
@@ -29,4 +31,4 @@ module ActiveRecord
   end
 end
 
-ActiveRecord::LogSubscriber.attach_to :active_record
\ No newline at end of file
+ActiveRecord::LogSubscriber.attach_to :active_record
diff --git a/activerecord/lib/active_record/named_scope.rb b/activerecord/lib/active_record/named_scope.rb
index 849ec9c884..417ff4b5eb 100644
--- a/activerecord/lib/active_record/named_scope.rb
+++ b/activerecord/lib/active_record/named_scope.rb
@@ -26,7 +26,7 @@ module ActiveRecord
       # You can define a \scope that applies to all finders using 
       # ActiveRecord::Base.default_scope.
       def scoped(options = nil)
-        if options.present?
+        if options
           scoped.apply_finder_options(options)
         else
           current_scoped_methods ? relation.merge(current_scoped_methods) : relation.clone
@@ -105,7 +105,7 @@ module ActiveRecord
           extension ? relation.extending(extension) : relation
         end
 
-        singleton_class.send :define_method, name, &scopes[name]
+        singleton_class.send(:redefine_method, name, &scopes[name])
       end
 
       def named_scope(*args, &block)
diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb
index 68f5be63d6..b587abd5d0 100644
--- a/activerecord/lib/active_record/persistence.rb
+++ b/activerecord/lib/active_record/persistence.rb
@@ -112,6 +112,8 @@ module ActiveRecord
     # * does not work on attr_accessor attributes. The attribute that is being updated must be column name.
     #
     def update_attribute(name, value)
+      raise ActiveRecordError, "#{name.to_s} is marked as readonly" if self.class.readonly_attributes.include? name.to_s
+
       changes = record_update_timestamps || {}
 
       if name
diff --git a/activerecord/lib/active_record/railtie.rb b/activerecord/lib/active_record/railtie.rb
index eff51a7c87..78fdb77216 100644
--- a/activerecord/lib/active_record/railtie.rb
+++ b/activerecord/lib/active_record/railtie.rb
@@ -22,6 +22,12 @@ module ActiveRecord
       load "active_record/railties/databases.rake"
     end
 
+    # When loading console, force ActiveRecord to be loaded to avoid cross
+    # references when loading a constant for the first time.
+    console do
+      ActiveRecord::Base
+    end
+
     initializer "active_record.initialize_timezone" do
       ActiveSupport.on_load(:active_record) do
         self.time_zone_aware_attributes = true
diff --git a/activerecord/lib/active_record/railties/databases.rake b/activerecord/lib/active_record/railties/databases.rake
index 5024787c3c..2c17c74ab4 100644
--- a/activerecord/lib/active_record/railties/databases.rake
+++ b/activerecord/lib/active_record/railties/databases.rake
@@ -274,7 +274,7 @@ namespace :db do
   task :setup => [ 'db:create', 'db:schema:load', 'db:seed' ]
 
   desc 'Load the seed data from db/seeds.rb'
-  task :seed => :environment do
+  task :seed => 'db:abort_if_pending_migrations' do
     seed_file = File.join(Rails.root, 'db', 'seeds.rb')
     load(seed_file) if File.exist?(seed_file)
   end
diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb
index 7499100f55..86a210d2be 100644
--- a/activerecord/lib/active_record/relation.rb
+++ b/activerecord/lib/active_record/relation.rb
@@ -13,14 +13,15 @@ module ActiveRecord
     delegate :to_xml, :to_yaml, :length, :collect, :map, :each, :all?, :include?, :to => :to_a
     delegate :insert, :to => :arel
 
-    attr_reader :table, :klass
+    attr_reader :table, :klass, :loaded
     attr_accessor :extensions
+    alias :loaded? :loaded
 
     def initialize(klass, table)
       @klass, @table = klass, table
 
       @implicit_readonly = nil
-      @loaded            = nil
+      @loaded            = false
 
       SINGLE_VALUE_METHODS.each {|v| instance_variable_set(:"@#{v}_value", nil)}
       (ASSOCIATION_METHODS + MULTI_VALUE_METHODS).each {|v| instance_variable_set(:"@#{v}_values", [])}
@@ -292,10 +293,6 @@ module ActiveRecord
       where(@klass.primary_key => id_or_array).delete_all
     end
 
-    def loaded?
-      @loaded
-    end
-
     def reload
       reset
       to_a # force reload
diff --git a/activerecord/lib/active_record/session_store.rb b/activerecord/lib/active_record/session_store.rb
index df2f429c5d..7ea7fb5c51 100644
--- a/activerecord/lib/active_record/session_store.rb
+++ b/activerecord/lib/active_record/session_store.rb
@@ -294,7 +294,6 @@ module ActiveRecord
     private
       def get_session(env, sid)
         Base.silence do
-          sid ||= generate_sid
           session = find_session(sid)
           env[SESSION_RECORD_KEY] = session
           [sid, session.data]
diff --git a/activerecord/lib/active_record/timestamp.rb b/activerecord/lib/active_record/timestamp.rb
index 341cc87be5..6c1e376745 100644
--- a/activerecord/lib/active_record/timestamp.rb
+++ b/activerecord/lib/active_record/timestamp.rb
@@ -39,8 +39,9 @@ module ActiveRecord
       if record_timestamps
         current_time = current_time_from_proper_timezone
 
-        write_attribute('created_at', current_time) if respond_to?(:created_at) && created_at.nil?
-        write_attribute('created_on', current_time) if respond_to?(:created_on) && created_on.nil?
+        timestamp_attributes_for_create.each do |column|
+          write_attribute(column.to_s, current_time) if respond_to?(column) && self.send(column).nil?
+        end
 
         timestamp_attributes_for_update_in_model.each do |column|
           write_attribute(column.to_s, current_time) if self.send(column).nil?
@@ -65,7 +66,19 @@ module ActiveRecord
     end
 
     def timestamp_attributes_for_update_in_model #:nodoc:
-      [:updated_at, :updated_on].select { |elem| respond_to?(elem) }
+      timestamp_attributes_for_update.select { |elem| respond_to?(elem) }
+    end
+
+    def timestamp_attributes_for_update #:nodoc:
+      [:updated_at, :updated_on]
+    end
+
+    def timestamp_attributes_for_create #:nodoc:
+      [:created_at, :created_on]
+    end
+
+    def all_timestamp_attributes #:nodoc:
+      timestamp_attributes_for_update + timestamp_attributes_for_create
     end
     
     def current_time_from_proper_timezone #:nodoc: