Merge branch 'master' into loofah

Conflicts:
	actionpack/CHANGELOG.md

17 files changed, 112 insertions, 56 deletions

diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb
index 1c5a737696..d3b9b8251a 100644
--- a/activerecord/lib/active_record/associations.rb
+++ b/activerecord/lib/active_record/associations.rb
@@ -1209,7 +1209,7 @@ module ActiveRecord
       # Option examples:
       #   has_many :comments, -> { order "posted_on" }
       #   has_many :comments, -> { includes :author }
-      #   has_many :people, -> { where("deleted = 0").order("name") }, class_name: "Person"
+      #   has_many :people, -> { where(deleted: false).order("name") }, class_name: "Person"
       #   has_many :tracks, -> { order "position" }, dependent: :destroy
       #   has_many :comments, dependent: :nullify
       #   has_many :tags, as: :taggable
@@ -1435,7 +1435,7 @@ module ActiveRecord
       #   belongs_to :firm, foreign_key: "client_of"
       #   belongs_to :person, primary_key: "name", foreign_key: "person_name"
       #   belongs_to :author, class_name: "Person", foreign_key: "author_id"
-      #   belongs_to :valid_coupon, ->(o) { where "discounts > #{o.payments_count}" },
+      #   belongs_to :valid_coupon, ->(o) { where "discounts > ?", o.payments_count },
       #                             class_name: "Coupon", foreign_key: "coupon_id"
       #   belongs_to :attachable, polymorphic: true
       #   belongs_to :project, readonly: true
diff --git a/activerecord/lib/active_record/associations/has_many_association.rb b/activerecord/lib/active_record/associations/has_many_association.rb
index 79c3d2b0f5..1413efaf7f 100644
--- a/activerecord/lib/active_record/associations/has_many_association.rb
+++ b/activerecord/lib/active_record/associations/has_many_association.rb
@@ -103,7 +103,7 @@ module ActiveRecord
           if has_cached_counter?(reflection)
             counter = cached_counter_attribute_name(reflection)
             owner[counter] += difference
-            owner.changed_attributes.delete(counter) # eww
+            owner.send(:clear_attribute_changes, counter) # eww
           end
         end
 
diff --git a/activerecord/lib/active_record/associations/preloader/through_association.rb b/activerecord/lib/active_record/associations/preloader/through_association.rb
index 1fed7f74e7..d57da366bd 100644
--- a/activerecord/lib/active_record/associations/preloader/through_association.rb
+++ b/activerecord/lib/active_record/associations/preloader/through_association.rb
@@ -63,7 +63,7 @@ module ActiveRecord
           should_reset = (through_scope != through_reflection.klass.unscoped) ||
              (reflection.options[:source_type] && through_reflection.collection?)
 
-          # Dont cache the association - we would only be caching a subset
+          # Don't cache the association - we would only be caching a subset
           if should_reset
             owners.each { |owner|
               owner.association(association_name).reset
diff --git a/activerecord/lib/active_record/attribute.rb b/activerecord/lib/active_record/attribute.rb
index 6d38224830..8cc1904575 100644
--- a/activerecord/lib/active_record/attribute.rb
+++ b/activerecord/lib/active_record/attribute.rb
@@ -30,10 +30,14 @@ module ActiveRecord
 
     def value
       # `defined?` is cheaper than `||=` when we get back falsy values
-      @value = type_cast(value_before_type_cast) unless defined?(@value)
+      @value = original_value unless defined?(@value)
       @value
     end
 
+    def original_value
+      type_cast(value_before_type_cast)
+    end
+
     def value_for_database
       type.type_cast_for_database(value)
     end
@@ -54,7 +58,7 @@ module ActiveRecord
       self.class.from_database(name, value, type)
     end
 
-    def type_cast
+    def type_cast(*)
       raise NotImplementedError
     end
 
@@ -62,6 +66,13 @@ module ActiveRecord
       true
     end
 
+    def ==(other)
+      self.class == other.class &&
+        name == other.name &&
+        value_before_type_cast == other.value_before_type_cast &&
+        type == other.type
+    end
+
     protected
 
     def initialize_dup(other)
diff --git a/activerecord/lib/active_record/attribute_methods.rb b/activerecord/lib/active_record/attribute_methods.rb
index a2bb78dfcc..1ff28ceccc 100644
--- a/activerecord/lib/active_record/attribute_methods.rb
+++ b/activerecord/lib/active_record/attribute_methods.rb
@@ -57,6 +57,8 @@ module ActiveRecord
       end
     end
 
+    class GeneratedAttributeMethods < Module; end # :nodoc:
+
     module ClassMethods
       def inherited(child_class) #:nodoc:
         child_class.initialize_generated_modules
@@ -64,7 +66,7 @@ module ActiveRecord
       end
 
       def initialize_generated_modules # :nodoc:
-        @generated_attribute_methods = Module.new { extend Mutex_m }
+        @generated_attribute_methods = GeneratedAttributeMethods.new { extend Mutex_m }
         @attribute_methods_generated = false
         include @generated_attribute_methods
       end
@@ -113,10 +115,11 @@ module ActiveRecord
         if superclass == Base
           super
         else
-          # If B < A and A defines its own attribute method, then we don't want to overwrite that.
-          defined = method_defined_within?(method_name, superclass, superclass.generated_attribute_methods)
-          base_defined = Base.method_defined?(method_name) || Base.private_method_defined?(method_name)
-          defined && !base_defined || super
+          # If ThisClass < ... < SomeSuperClass < ... < Base and SomeSuperClass
+          # defines its own attribute method, then we don't want to overwrite that.
+          defined = method_defined_within?(method_name, superclass, Base) &&
+            ! superclass.instance_method(method_name).owner.is_a?(GeneratedAttributeMethods)
+          defined || super
         end
       end
 
@@ -279,9 +282,9 @@ module ActiveRecord
     end
 
     # Returns an <tt>#inspect</tt>-like string for the value of the
-    # attribute +attr_name+. String attributes are truncated upto 50
+    # attribute +attr_name+. String attributes are truncated up to 50
     # characters, Date and Time attributes are returned in the
-    # <tt>:db</tt> format, Array attributes are truncated upto 10 values.
+    # <tt>:db</tt> format, Array attributes are truncated up to 10 values.
     # Other attributes return the value of <tt>#inspect</tt> without
     # modification.
     #
diff --git a/activerecord/lib/active_record/attribute_methods/dirty.rb b/activerecord/lib/active_record/attribute_methods/dirty.rb
index b58295a106..d3f4e51c33 100644
--- a/activerecord/lib/active_record/attribute_methods/dirty.rb
+++ b/activerecord/lib/active_record/attribute_methods/dirty.rb
@@ -51,14 +51,6 @@ module ActiveRecord
         super | changed_in_place
       end
 
-      def attribute_changed?(attr_name, options = {})
-        result = super
-        # We can't change "from" something in place. Only setters can define
-        # "from" and "to"
-        result ||= changed_in_place?(attr_name) unless options.key?(:from)
-        result
-      end
-
       def changes_applied
         super
         store_original_raw_attributes
@@ -69,12 +61,16 @@ module ActiveRecord
         original_raw_attributes.clear
       end
 
+      def changed_attributes
+        super.reverse_merge(attributes_changed_in_place).freeze
+      end
+
       private
 
       def calculate_changes_from_defaults
         @changed_attributes = nil
         self.class.column_defaults.each do |attr, orig_value|
-          changed_attributes[attr] = orig_value if _field_changed?(attr, orig_value)
+          set_attribute_was(attr, orig_value) if _field_changed?(attr, orig_value)
         end
       end
 
@@ -100,9 +96,9 @@ module ActiveRecord
 
       def save_changed_attribute(attr, old_value)
         if attribute_changed?(attr)
-          changed_attributes.delete(attr) unless _field_changed?(attr, old_value)
+          clear_attribute_changes(attr) unless _field_changed?(attr, old_value)
         else
-          changed_attributes[attr] = old_value if _field_changed?(attr, old_value)
+          set_attribute_was(attr, old_value) if _field_changed?(attr, old_value)
         end
       end
 
@@ -132,6 +128,13 @@ module ActiveRecord
         @attributes[attr].changed_from?(old_value)
       end
 
+      def attributes_changed_in_place
+        changed_in_place.each_with_object({}) do |attr_name, h|
+          orig = @attributes[attr_name].original_value
+          h[attr_name] = orig
+        end
+      end
+
       def changed_in_place
         self.class.attribute_names.select do |attr_name|
           changed_in_place?(attr_name)
diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb
index e44ccb7d81..9e07e9a5c4 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb
@@ -56,6 +56,19 @@ module ActiveRecord
       end
     end
 
+    module TimestampDefaultDeprecation # :nodoc:
+      def emit_warning_if_null_unspecified(options)
+        return if options.key?(:null)
+
+        ActiveSupport::Deprecation.warn(<<-MESSAGE.strip_heredoc)
+        `timestamp` was called without specifying an option for `null`. In Rails
+        5.0, this behavior will change to `null: false`. You should manually
+        specify `null: true` to prevent the behavior of your existing migrations
+        from changing.
+        MESSAGE
+      end
+    end
+
     # Represents the schema of an SQL table in an abstract way. This class
     # provides methods for manipulating the schema representation.
     #
@@ -77,6 +90,8 @@ module ActiveRecord
     # The table definitions
     # The Columns are stored as a ColumnDefinition in the +columns+ attribute.
     class TableDefinition
+      include TimestampDefaultDeprecation
+
       # An array of ColumnDefinition objects, representing the column changes
       # that have been defined.
       attr_accessor :indexes
@@ -276,6 +291,7 @@ module ActiveRecord
       # <tt>:updated_at</tt> to the table.
       def timestamps(*args)
         options = args.extract_options!
+        emit_warning_if_null_unspecified(options)
         column(:created_at, :datetime, options)
         column(:updated_at, :datetime, options)
       end
@@ -405,6 +421,8 @@ module ActiveRecord
     #   end
     #
     class Table
+      include TimestampDefaultDeprecation
+
       def initialize(table_name, base)
         @table_name = table_name
         @base = base
@@ -452,8 +470,9 @@ module ActiveRecord
       # Adds timestamps (+created_at+ and +updated_at+) columns to the table. See SchemaStatements#add_timestamps
       #
       #  t.timestamps
-      def timestamps
-        @base.add_timestamps(@table_name)
+      def timestamps(options = {})
+        emit_warning_if_null_unspecified(options)
+        @base.add_timestamps(@table_name, options)
       end
 
       # Changes the column's definition according to the new options.
@@ -559,6 +578,5 @@ module ActiveRecord
           @base.native_database_types
         end
     end
-
   end
 end
diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb
index 10753defc2..7105df1ee4 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb
@@ -43,13 +43,14 @@ module ActiveRecord
       #   index_exists?(:suppliers, :company_id, name: "idx_company_id")
       #
       def index_exists?(table_name, column_name, options = {})
-        column_names = Array(column_name)
-        index_name = options.key?(:name) ? options[:name].to_s : index_name(table_name, :column => column_names)
-        if options[:unique]
-          indexes(table_name).any?{ |i| i.unique && i.name == index_name }
-        else
-          indexes(table_name).any?{ |i| i.name == index_name }
-        end
+        column_names = Array(column_name).map(&:to_s)
+        index_name = options.key?(:name) ? options[:name].to_s : index_name(table_name, column: column_names)
+        checks = []
+        checks << lambda { |i| i.name == index_name }
+        checks << lambda { |i| i.columns == column_names }
+        checks << lambda { |i| i.unique } if options[:unique]
+
+        indexes(table_name).any? { |i| checks.all? { |check| check[i] } }
       end
 
       # Returns an array of Column objects for the table specified by +table_name+.
@@ -838,9 +839,9 @@ module ActiveRecord
       #
       #   add_timestamps(:suppliers)
       #
-      def add_timestamps(table_name)
-        add_column table_name, :created_at, :datetime
-        add_column table_name, :updated_at, :datetime
+      def add_timestamps(table_name, options = {})
+        add_column table_name, :created_at, :datetime, options
+        add_column table_name, :updated_at, :datetime, options
       end
 
       # Removes the timestamp columns (+created_at+ and +updated_at+) from the table definition.
diff --git a/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb b/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb
index 4a7f2aaca8..8f06cf3a1f 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb
@@ -111,6 +111,8 @@ module ActiveRecord
       def commit
         super
         connection.release_savepoint(savepoint_name)
+        parent = connection.transaction_manager.current_transaction
+        records.each { |r| parent.add_record(r) }
       end
 
       def full_rollback?; false; end
@@ -169,16 +171,14 @@ module ActiveRecord
         transaction = begin_transaction options
         yield
       rescue Exception => error
-        transaction.rollback if transaction
+        rollback_transaction if transaction
         raise
       ensure
         begin
-          transaction.commit unless error
+          commit_transaction unless error
         rescue Exception
           transaction.rollback
           raise
-        ensure
-          @stack.pop if transaction
         end
       end
 
diff --git a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb
index e5417a9556..a1c370b05d 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb
@@ -764,8 +764,8 @@ module ActiveRecord
         "DROP INDEX #{index_name}"
       end
 
-      def add_timestamps_sql(table_name)
-        [add_column_sql(table_name, :created_at, :datetime), add_column_sql(table_name, :updated_at, :datetime)]
+      def add_timestamps_sql(table_name, options = {})
+        [add_column_sql(table_name, :created_at, :datetime, options), add_column_sql(table_name, :updated_at, :datetime, options)]
       end
 
       def remove_timestamps_sql(table_name)
diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/oid/jsonb.rb b/activerecord/lib/active_record/connection_adapters/postgresql/oid/jsonb.rb
index 34ed32ad35..380c50fc14 100644
--- a/activerecord/lib/active_record/connection_adapters/postgresql/oid/jsonb.rb
+++ b/activerecord/lib/active_record/connection_adapters/postgresql/oid/jsonb.rb
@@ -12,7 +12,7 @@ module ActiveRecord
             # roundtripping jsonb columns. This causes some false positives for
             # the comparison here. Therefore, we need to parse and re-dump the
             # raw value here to ensure the insignificant whitespaces are
-            # consitent with our encoder's output.
+            # consistent with our encoder's output.
             raw_old_value = type_cast_for_database(type_cast_from_database(raw_old_value))
             super(raw_old_value, new_value)
           end
diff --git a/activerecord/lib/active_record/enum.rb b/activerecord/lib/active_record/enum.rb
index f0ee433d0b..5958373e88 100644
--- a/activerecord/lib/active_record/enum.rb
+++ b/activerecord/lib/active_record/enum.rb
@@ -145,11 +145,11 @@ module ActiveRecord
                   value = read_attribute(attr_name)
                   if attribute_changed?(attr_name)
                     if mapping[old] == value
-                      changed_attributes.delete(attr_name)
+                      clear_attribute_changes([attr_name])
                     end
                   else
                     if old != value
-                      changed_attributes[attr_name] = mapping.key old
+                      set_attribute_was(attr_name, mapping.key(old))
                     end
                   end
                 else
diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb
index 51b1931ed5..755ff2b2f1 100644
--- a/activerecord/lib/active_record/persistence.rb
+++ b/activerecord/lib/active_record/persistence.rb
@@ -466,7 +466,7 @@ module ActiveRecord
 
         changes[self.class.locking_column] = increment_lock if locking_enabled?
 
-        changed_attributes.except!(*changes.keys)
+        clear_attribute_changes(changes.keys)
         primary_key = self.class.primary_key
         self.class.unscoped.where(primary_key => self[primary_key]).update_all(changes) == 1
       else
diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb
index 1262b2c291..c8f382ad78 100644
--- a/activerecord/lib/active_record/relation/query_methods.rb
+++ b/activerecord/lib/active_record/relation/query_methods.rb
@@ -952,9 +952,7 @@ WARNING
         self.bind_values += bind_values
 
         attributes = @klass.send(:expand_hash_conditions_for_aggregates, tmp_opts)
-        attributes.values.grep(ActiveRecord::Relation) do |rel|
-          self.bind_values += rel.bind_values
-        end
+        add_relations_to_bind_values(attributes)
 
         PredicateBuilder.build_from_hash(klass, attributes, table)
       else
@@ -1137,5 +1135,19 @@ WARNING
         raise ArgumentError, "The method .#{method_name}() must contain arguments."
       end
     end
+
+    # This function is recursive just for better readablity.
+    # #where argument doesn't support more than one level nested hash in real world.
+    def add_relations_to_bind_values(attributes)
+      if attributes.is_a?(Hash)
+        attributes.each_value do |value|
+          if value.is_a?(ActiveRecord::Relation)
+            self.bind_values += value.bind_values
+          else
+            add_relations_to_bind_values(value)
+          end
+        end
+      end
+    end
   end
 end
diff --git a/activerecord/lib/active_record/timestamp.rb b/activerecord/lib/active_record/timestamp.rb
index ddf3e1804c..936a18d99a 100644
--- a/activerecord/lib/active_record/timestamp.rb
+++ b/activerecord/lib/active_record/timestamp.rb
@@ -47,8 +47,9 @@ module ActiveRecord
         current_time = current_time_from_proper_timezone
 
         all_timestamp_attributes.each do |column|
-          if respond_to?(column) && respond_to?("#{column}=") && self.send(column).nil?
-            write_attribute(column.to_s, current_time)
+          column = column.to_s
+          if has_attribute?(column) && !attribute_present?(column)
+            write_attribute(column, current_time)
           end
         end
       end
@@ -113,7 +114,7 @@ module ActiveRecord
     def clear_timestamp_attributes
       all_timestamp_attributes_in_model.each do |attribute_name|
         self[attribute_name] = nil
-        changed_attributes.delete(attribute_name)
+        clear_attribute_changes([attribute_name])
       end
     end
   end
diff --git a/activerecord/lib/active_record/type/value.rb b/activerecord/lib/active_record/type/value.rb
index e0a783fb45..9456a4a56c 100644
--- a/activerecord/lib/active_record/type/value.rb
+++ b/activerecord/lib/active_record/type/value.rb
@@ -69,13 +69,20 @@ module ActiveRecord
       end
 
       # Determines whether the mutable value has been modified since it was
-      # read. Returns +false+ by default. This method should not need to be
-      # overriden directly. Types which return a mutable value should include
+      # read. Returns +false+ by default. This method should not be overridden
+      # directly. Types which return a mutable value should include
       # +Type::Mutable+, which will define this method.
       def changed_in_place?(*)
         false
       end
 
+      def ==(other)
+        self.class == other.class &&
+          precision == other.precision &&
+          scale == other.scale &&
+          limit == other.limit
+      end
+
       private
 
       def type_cast(value)
diff --git a/activerecord/lib/rails/generators/active_record/migration/templates/create_table_migration.rb b/activerecord/lib/rails/generators/active_record/migration/templates/create_table_migration.rb
index fd94a2d038..f7bf6987c4 100644
--- a/activerecord/lib/rails/generators/active_record/migration/templates/create_table_migration.rb
+++ b/activerecord/lib/rails/generators/active_record/migration/templates/create_table_migration.rb
@@ -9,7 +9,7 @@ class <%= migration_class_name %> < ActiveRecord::Migration
 <% end -%>
 <% end -%>
 <% if options[:timestamps] %>
-      t.timestamps
+      t.timestamps null: false
 <% end -%>
     end
 <% attributes_with_index.each do |attribute| -%>