Merge branch 'master' of github.com:lifo/docrails

16 files changed, 159 insertions, 71 deletions

diff --git a/activemodel/CHANGELOG.md b/activemodel/CHANGELOG.md
index 3d3c61ed1c..e8602ecbcf 100644
--- a/activemodel/CHANGELOG.md
+++ b/activemodel/CHANGELOG.md
@@ -1,3 +1,18 @@
+*   Fix `has_secure_password` to honor bcrypt-ruby's cost attribute.
+
+    *T.J. Schuck*
+
+*   Updated the `ActiveModel::Dirty#changed_attributes` method to be indifferent between using
+    symbols and strings as keys.
+
+    *William Myers*
+
+*   Added new API methods `reset_changes` and `changed_applied` to `ActiveModel::Dirty`
+    that control changes state. Previsously you needed to update internal
+    instance variables, but now API methods are available.
+
+    *Bogdan Gusiev*
+
 *   Fix has_secure_password. `password_confirmation` validations are triggered
     even if no `password_confirmation` is set.
 
diff --git a/activemodel/activemodel.gemspec b/activemodel/activemodel.gemspec
index 51655fe3da..11e755649c 100644
--- a/activemodel/activemodel.gemspec
+++ b/activemodel/activemodel.gemspec
@@ -5,7 +5,7 @@ Gem::Specification.new do |s|
   s.name        = 'activemodel'
   s.version     = version
   s.summary     = 'A toolkit for building modeling frameworks (part of Rails).'
-  s.description = 'A toolkit for building modeling frameworks like Active Record. Rich support for attributes, callbacks, validations, observers, serialization, internationalization, and testing.'
+  s.description = 'A toolkit for building modeling frameworks like Active Record. Rich support for attributes, callbacks, validations, serialization, internationalization, and testing.'
 
   s.required_ruby_version = '>= 1.9.3'
 
diff --git a/activemodel/examples/validations.rb b/activemodel/examples/validations.rb
index c94cd17e18..b8e74acd5e 100644
--- a/activemodel/examples/validations.rb
+++ b/activemodel/examples/validations.rb
@@ -1,3 +1,4 @@
+require File.expand_path('../../../load_paths', __FILE__)
 require 'active_model'
 
 class Person
diff --git a/activemodel/lib/active_model/dirty.rb b/activemodel/lib/active_model/dirty.rb
index ea5ddf71de..c5f1b3f11a 100644
--- a/activemodel/lib/active_model/dirty.rb
+++ b/activemodel/lib/active_model/dirty.rb
@@ -14,13 +14,9 @@ module ActiveModel
   #   track.
   # * Call <tt>attr_name_will_change!</tt> before each change to the tracked
   #   attribute.
-  #
-  # If you wish to also track previous changes on save or update, you need to
-  # add:
-  #
-  #   @previously_changed = changes
-  #
-  # inside of your save or update method.
+  # * Call <tt>changes_applied</tt> after the changes are persisted.
+  # * Call <tt>reset_changes</tt> when you want to reset the changes
+  #   information.
   #
   # A minimal implementation could be:
   #
@@ -39,8 +35,12 @@ module ActiveModel
   #     end
   #
   #     def save
-  #       @previously_changed = changes
-  #       @changed_attributes.clear
+  #       # do persistence work
+  #       changes_applied
+  #     end
+  #
+  #     def reload!
+  #       reset_changes
   #     end
   #   end
   #
@@ -65,6 +65,12 @@ module ActiveModel
   #   person.changed?       # => false
   #   person.name_changed?  # => false
   #
+  # Reset the changes:
+  #
+  #   person.previous_changes # => {"name" => ["Uncle Bob", "Bill"]}
+  #   person.reload!
+  #   person.previous_changes # => {}
+  #
   # Assigning the same value leaves the attribute unchanged:
   #
   #   person.name = 'Bill'
@@ -129,7 +135,7 @@ module ActiveModel
     #   person.save
     #   person.previous_changes # => {"name" => ["bob", "robert"]}
     def previous_changes
-      @previously_changed
+      @previously_changed ||= {}
     end
 
     # Returns a hash of the attributes with unsaved changes indicating their original
@@ -139,7 +145,7 @@ module ActiveModel
     #   person.name = 'robert'
     #   person.changed_attributes # => {"name" => "bob"}
     def changed_attributes
-      @changed_attributes ||= {}
+      @changed_attributes ||= ActiveSupport::HashWithIndifferentAccess.new
     end
 
     # Handle <tt>*_changed?</tt> for +method_missing+.
@@ -154,6 +160,18 @@ module ActiveModel
 
     private
 
+      # Removes current changes and makes them accessible through +previous_changes+.
+      def changes_applied
+        @previously_changed = changes
+        @changed_attributes = {}
+      end
+
+      # Removes all dirty data: current changes and previous changes
+      def reset_changes
+        @previously_changed = {}
+        @changed_attributes = {}
+      end
+
       # Handle <tt>*_change</tt> for +method_missing+.
       def attribute_change(attr)
         [changed_attributes[attr], __send__(attr)] if attribute_changed?(attr)
diff --git a/activemodel/lib/active_model/naming.rb b/activemodel/lib/active_model/naming.rb
index bc9edf4a56..11ebfe6cc0 100644
--- a/activemodel/lib/active_model/naming.rb
+++ b/activemodel/lib/active_model/naming.rb
@@ -262,10 +262,10 @@ module ActiveModel
     # namespaced models regarding whether it's inside isolated engine.
     #
     #   # For isolated engine:
-    #   ActiveModel::Naming.singular_route_key(Blog::Post) #=> post
+    #   ActiveModel::Naming.singular_route_key(Blog::Post) # => "post"
     #
     #   # For shared engine:
-    #   ActiveModel::Naming.singular_route_key(Blog::Post) #=> blog_post
+    #   ActiveModel::Naming.singular_route_key(Blog::Post) # => "blog_post"
     def self.singular_route_key(record_or_class)
       model_name_from_record_or_class(record_or_class).singular_route_key
     end
@@ -274,10 +274,10 @@ module ActiveModel
     # namespaced models regarding whether it's inside isolated engine.
     #
     #   # For isolated engine:
-    #   ActiveModel::Naming.route_key(Blog::Post) #=> posts
+    #   ActiveModel::Naming.route_key(Blog::Post) # => "posts"
     #
     #   # For shared engine:
-    #   ActiveModel::Naming.route_key(Blog::Post) #=> blog_posts
+    #   ActiveModel::Naming.route_key(Blog::Post) # => "blog_posts"
     #
     # The route key also considers if the noun is uncountable and, in
     # such cases, automatically appends _index.
@@ -289,10 +289,10 @@ module ActiveModel
     # namespaced models regarding whether it's inside isolated engine.
     #
     #   # For isolated engine:
-    #   ActiveModel::Naming.param_key(Blog::Post) #=> post
+    #   ActiveModel::Naming.param_key(Blog::Post) # => "post"
     #
     #   # For shared engine:
-    #   ActiveModel::Naming.param_key(Blog::Post) #=> blog_post
+    #   ActiveModel::Naming.param_key(Blog::Post) # => "blog_post"
     def self.param_key(record_or_class)
       model_name_from_record_or_class(record_or_class).param_key
     end
diff --git a/activemodel/lib/active_model/secure_password.rb b/activemodel/lib/active_model/secure_password.rb
index 8b9ac97bbb..7e694b5c50 100644
--- a/activemodel/lib/active_model/secure_password.rb
+++ b/activemodel/lib/active_model/secure_password.rb
@@ -20,9 +20,9 @@ module ActiveModel
       # value to the password_confirmation attribute and the validation
       # will not be triggered.
       #
-      # You need to add bcrypt-ruby (~> 3.1.0) to Gemfile to use #has_secure_password:
+      # You need to add bcrypt-ruby (~> 3.1.2) to Gemfile to use #has_secure_password:
       #
-      #   gem 'bcrypt-ruby', '~> 3.1.0'
+      #   gem 'bcrypt-ruby', '~> 3.1.2'
       #
       # Example using Active Record (which automatically includes ActiveModel::SecurePassword):
       #
@@ -46,7 +46,6 @@ module ActiveModel
         # This is to avoid ActiveModel (and by extension the entire framework)
         # being dependent on a binary library.
         begin
-          gem 'bcrypt-ruby', '~> 3.1.0'
           require 'bcrypt'
         rescue LoadError
           $stderr.puts "You don't have bcrypt-ruby installed in your application. Please add it to your Gemfile and run bundle install"
@@ -103,7 +102,7 @@ module ActiveModel
       def password=(unencrypted_password)
         unless unencrypted_password.blank?
           @password = unencrypted_password
-          cost = ActiveModel::SecurePassword.min_cost ? BCrypt::Engine::MIN_COST : BCrypt::Engine::DEFAULT_COST
+          cost = ActiveModel::SecurePassword.min_cost ? BCrypt::Engine::MIN_COST : BCrypt::Engine.cost
           self.password_digest = BCrypt::Password.create(unencrypted_password, cost: cost)
         end
       end
diff --git a/activemodel/lib/active_model/validations/clusivity.rb b/activemodel/lib/active_model/validations/clusivity.rb
index 1c35cb7c35..bad9e4f9a9 100644
--- a/activemodel/lib/active_model/validations/clusivity.rb
+++ b/activemodel/lib/active_model/validations/clusivity.rb
@@ -30,12 +30,21 @@ module ActiveModel
         @delimiter ||= options[:in] || options[:within]
       end
 
-      # In Ruby 1.9 <tt>Range#include?</tt> on non-numeric ranges checks all possible values in the
-      # range for equality, which is slower but more accurate. <tt>Range#cover?</tt> uses
-      # the previous logic of comparing a value with the range endpoints, which is fast
-      # but is only accurate on numeric ranges.
+      # In Ruby 1.9 <tt>Range#include?</tt> on non-number-or-time-ish ranges checks all
+      # possible values in the range for equality, which is slower but more accurate.
+      # <tt>Range#cover?</tt> uses the previous logic of comparing a value with the range
+      # endpoints, which is fast but is only accurate on Numeric, Time, or DateTime ranges.
       def inclusion_method(enumerable)
-        (enumerable.is_a?(Range) && enumerable.first.is_a?(Numeric)) ? :cover? : :include?
+        if enumerable.is_a? Range
+          case enumerable.first
+          when Numeric, Time, DateTime
+            :cover?
+          else
+            :include?
+          end
+        else
+          :include?
+        end
       end
     end
   end
diff --git a/activemodel/lib/active_model/validations/format.rb b/activemodel/lib/active_model/validations/format.rb
index be7cae588f..f0fe22438f 100644
--- a/activemodel/lib/active_model/validations/format.rb
+++ b/activemodel/lib/active_model/validations/format.rb
@@ -17,8 +17,8 @@ module ActiveModel
           raise ArgumentError, "Either :with or :without must be supplied (but not both)"
         end
 
-        check_options_validity(options, :with)
-        check_options_validity(options, :without)
+        check_options_validity :with
+        check_options_validity :without
       end
 
       private
@@ -32,21 +32,23 @@ module ActiveModel
         record.errors.add(attribute, :invalid, options.except(name).merge!(value: value))
       end
 
-      def regexp_using_multiline_anchors?(regexp)
-        regexp.source.start_with?("^") ||
-          (regexp.source.end_with?("$") && !regexp.source.end_with?("\\$"))
+      def check_options_validity(name)
+        if option = options[name]
+          if option.is_a?(Regexp)
+            if options[:multiline] != true && regexp_using_multiline_anchors?(option)
+              raise ArgumentError, "The provided regular expression is using multiline anchors (^ or $), " \
+              "which may present a security risk. Did you mean to use \\A and \\z, or forgot to add the " \
+              ":multiline => true option?"
+            end
+          elsif !option.respond_to?(:call)
+            raise ArgumentError, "A regular expression or a proc or lambda must be supplied as :#{name}"
+          end
+        end
       end
 
-      def check_options_validity(options, name)
-        option = options[name]
-        if option && !option.is_a?(Regexp) && !option.respond_to?(:call)
-          raise ArgumentError, "A regular expression or a proc or lambda must be supplied as :#{name}"
-        elsif option && option.is_a?(Regexp) &&
-              regexp_using_multiline_anchors?(option) && options[:multiline] != true
-          raise ArgumentError, "The provided regular expression is using multiline anchors (^ or $), " \
-          "which may present a security risk. Did you mean to use \\A and \\z, or forgot to add the " \
-          ":multiline => true option?"
-        end
+      def regexp_using_multiline_anchors?(regexp)
+        source = regexp.source
+        source.start_with?("^") || (source.end_with?("$") && !source.end_with?("\\$"))
       end
     end
 
diff --git a/activemodel/lib/active_model/validations/numericality.rb b/activemodel/lib/active_model/validations/numericality.rb
index c6abe45f4a..c8d3236463 100644
--- a/activemodel/lib/active_model/validations/numericality.rb
+++ b/activemodel/lib/active_model/validations/numericality.rb
@@ -11,8 +11,9 @@ module ActiveModel
       def check_validity!
         keys = CHECKS.keys - [:odd, :even]
         options.slice(*keys).each do |option, value|
-          next if value.is_a?(Numeric) || value.is_a?(Proc) || value.is_a?(Symbol)
-          raise ArgumentError, ":#{option} must be a number, a symbol or a proc"
+          unless value.is_a?(Numeric) || value.is_a?(Proc) || value.is_a?(Symbol)
+            raise ArgumentError, ":#{option} must be a number, a symbol or a proc"
+          end
         end
       end
 
@@ -43,11 +44,15 @@ module ActiveModel
               record.errors.add(attr_name, option, filtered_options(value))
             end
           else
-            option_value = option_value.call(record) if option_value.is_a?(Proc)
-            option_value = record.send(option_value) if option_value.is_a?(Symbol)
+            case option_value
+            when Proc
+              option_value = option_value.call(record)
+            when Symbol
+              option_value = record.send(option_value)
+            end
 
             unless value.send(CHECKS[option], option_value)
-              record.errors.add(attr_name, option, filtered_options(value).merge(count: option_value))
+              record.errors.add(attr_name, option, filtered_options(value).merge!(count: option_value))
             end
           end
         end
@@ -56,16 +61,9 @@ module ActiveModel
     protected
 
       def parse_raw_value_as_a_number(raw_value)
-        case raw_value
-        when /\A0[xX]/
-          nil
-        else
-          begin
-            Kernel.Float(raw_value)
-          rescue ArgumentError, TypeError
-            nil
-          end
-        end
+        Kernel.Float(raw_value) if raw_value !~ /\A0[xX]/
+      rescue ArgumentError, TypeError
+        nil
       end
 
       def parse_raw_value_as_an_integer(raw_value)
@@ -73,7 +71,9 @@ module ActiveModel
       end
 
       def filtered_options(value)
-        options.except(*RESERVED_OPTIONS).merge!(value: value)
+        filtered = options.except(*RESERVED_OPTIONS)
+        filtered[:value] = value
+        filtered
       end
     end
 
diff --git a/activemodel/lib/active_model/validator.rb b/activemodel/lib/active_model/validator.rb
index 690856aee1..e19b8e0a9e 100644
--- a/activemodel/lib/active_model/validator.rb
+++ b/activemodel/lib/active_model/validator.rb
@@ -109,7 +109,7 @@ module ActiveModel
       deprecated_setup(options)
     end
 
-    # Return the kind for this validator.
+    # Returns the kind for this validator.
     #
     #   PresenceValidator.new.kind   # => :presence
     #   UniquenessValidator.new.kind # => :uniqueness
diff --git a/activemodel/test/cases/dirty_test.rb b/activemodel/test/cases/dirty_test.rb
index ba45089cca..a90d0b1299 100644
--- a/activemodel/test/cases/dirty_test.rb
+++ b/activemodel/test/cases/dirty_test.rb
@@ -3,11 +3,12 @@ require "cases/helper"
 class DirtyTest < ActiveModel::TestCase
   class DirtyModel
     include ActiveModel::Dirty
-    define_attribute_methods :name, :color
+    define_attribute_methods :name, :color, :size
 
     def initialize
       @name = nil
       @color = nil
+      @size = nil
     end
 
     def name
@@ -28,9 +29,17 @@ class DirtyTest < ActiveModel::TestCase
       @color = val
     end
 
+    def size
+      @size
+    end
+
+    def size=(val)
+      attribute_will_change!(:size) unless val == @size
+      @size = val
+    end
+
     def save
-      @previously_changed = changes
-      @changed_attributes.clear
+      changes_applied
     end
   end
 
@@ -125,4 +134,9 @@ class DirtyTest < ActiveModel::TestCase
     assert_equal ["Otto", "Mr. Manfredgensonton"], @model.name_change
     assert_equal @model.name_was, "Otto"
   end
+
+  test "using attribute_will_change! with a symbol" do
+    @model.size = 1
+    assert @model.size_changed?
+  end
 end
diff --git a/activemodel/test/cases/secure_password_test.rb b/activemodel/test/cases/secure_password_test.rb
index 98e5c747d5..41d0b2263e 100644
--- a/activemodel/test/cases/secure_password_test.rb
+++ b/activemodel/test/cases/secure_password_test.rb
@@ -82,6 +82,14 @@ class SecurePasswordTest < ActiveModel::TestCase
     assert_equal BCrypt::Engine::DEFAULT_COST, @user.password_digest.cost
   end
 
+  test "Password digest cost honors bcrypt cost attribute when min_cost is false" do
+    ActiveModel::SecurePassword.min_cost = false
+    BCrypt::Engine.cost = 5
+
+    @user.password = "secret"
+    assert_equal BCrypt::Engine.cost, @user.password_digest.cost
+  end
+
   test "Password digest cost can be set to bcrypt min cost to speed up tests" do
     ActiveModel::SecurePassword.min_cost = true
 
diff --git a/activemodel/test/cases/serializers/json_serialization_test.rb b/activemodel/test/cases/serializers/json_serialization_test.rb
index e4f5e61e91..bc185c737f 100644
--- a/activemodel/test/cases/serializers/json_serialization_test.rb
+++ b/activemodel/test/cases/serializers/json_serialization_test.rb
@@ -198,7 +198,7 @@ class JsonSerializationTest < ActiveModel::TestCase
     assert_no_match %r{"preferences":}, json
   end
 
-  test "custom as_json options should be extendible" do
+  test "custom as_json options should be extensible" do
     def @contact.as_json(options = {}); super(options.merge(only: [:name])); end
     json = @contact.to_json
 
diff --git a/activemodel/test/cases/validations/conditional_validation_test.rb b/activemodel/test/cases/validations/conditional_validation_test.rb
index 41a4c33727..5049d6dd61 100644
--- a/activemodel/test/cases/validations/conditional_validation_test.rb
+++ b/activemodel/test/cases/validations/conditional_validation_test.rb
@@ -23,7 +23,7 @@ class ConditionalValidationTest < ActiveModel::TestCase
     Topic.validates_length_of(:title, maximum: 5, too_long: "hoo %{count}", unless: :condition_is_true)
     t = Topic.new("title" => "uhohuhoh", "content" => "whatever")
     assert t.valid?
-    assert t.errors[:title].empty?
+    assert_empty t.errors[:title]
   end
 
   def test_if_validation_using_method_false
@@ -31,7 +31,7 @@ class ConditionalValidationTest < ActiveModel::TestCase
     Topic.validates_length_of(:title, maximum: 5, too_long: "hoo %{count}", if: :condition_is_true_but_its_not)
     t = Topic.new("title" => "uhohuhoh", "content" => "whatever")
     assert t.valid?
-    assert t.errors[:title].empty?
+    assert_empty t.errors[:title]
   end
 
   def test_unless_validation_using_method_false
@@ -57,7 +57,7 @@ class ConditionalValidationTest < ActiveModel::TestCase
     Topic.validates_length_of(:title, maximum: 5, too_long: "hoo %{count}", unless: "a = 1; a == 1")
     t = Topic.new("title" => "uhohuhoh", "content" => "whatever")
     assert t.valid?
-    assert t.errors[:title].empty?
+    assert_empty t.errors[:title]
   end
 
   def test_if_validation_using_string_false
@@ -65,7 +65,7 @@ class ConditionalValidationTest < ActiveModel::TestCase
     Topic.validates_length_of(:title, maximum: 5, too_long: "hoo %{count}", if: "false")
     t = Topic.new("title" => "uhohuhoh", "content" => "whatever")
     assert t.valid?
-    assert t.errors[:title].empty?
+    assert_empty t.errors[:title]
   end
 
   def test_unless_validation_using_string_false
@@ -93,7 +93,7 @@ class ConditionalValidationTest < ActiveModel::TestCase
       unless: Proc.new { |r| r.content.size > 4 })
     t = Topic.new("title" => "uhohuhoh", "content" => "whatever")
     assert t.valid?
-    assert t.errors[:title].empty?
+    assert_empty t.errors[:title]
   end
 
   def test_if_validation_using_block_false
@@ -102,7 +102,7 @@ class ConditionalValidationTest < ActiveModel::TestCase
       if: Proc.new { |r| r.title != "uhohuhoh"})
     t = Topic.new("title" => "uhohuhoh", "content" => "whatever")
     assert t.valid?
-    assert t.errors[:title].empty?
+    assert_empty t.errors[:title]
   end
 
   def test_unless_validation_using_block_false
@@ -124,7 +124,7 @@ class ConditionalValidationTest < ActiveModel::TestCase
 
     t = Topic.new
     assert t.invalid?, "A topic without a title should not be valid"
-    assert t.errors[:author_name].empty?, "A topic without an 'important' title should not require an author"
+    assert_empty t.errors[:author_name], "A topic without an 'important' title should not require an author"
 
     t.title = "Just a title"
     assert t.valid?, "A topic with a basic title should be valid"
diff --git a/activemodel/test/cases/validations/inclusion_validation_test.rb b/activemodel/test/cases/validations/inclusion_validation_test.rb
index 01a373d85d..8b90856869 100644
--- a/activemodel/test/cases/validations/inclusion_validation_test.rb
+++ b/activemodel/test/cases/validations/inclusion_validation_test.rb
@@ -1,5 +1,6 @@
 # encoding: utf-8
 require 'cases/helper'
+require 'active_support/all'
 
 require 'models/topic'
 require 'models/person'
@@ -20,6 +21,27 @@ class InclusionValidationTest < ActiveModel::TestCase
     assert Topic.new("title" => "bbb", "content" => "abc").valid?
   end
 
+  def test_validates_inclusion_of_time_range
+    Topic.validates_inclusion_of(:created_at, in: 1.year.ago..Time.now)
+    assert Topic.new(title: 'aaa', created_at: 2.years.ago).invalid?
+    assert Topic.new(title: 'aaa', created_at: 3.months.ago).valid?
+    assert Topic.new(title: 'aaa', created_at: 37.weeks.from_now).invalid?
+  end
+
+  def test_validates_inclusion_of_date_range
+    Topic.validates_inclusion_of(:created_at, in: 1.year.until(Date.today)..Date.today)
+    assert Topic.new(title: 'aaa', created_at: 2.years.until(Date.today)).invalid?
+    assert Topic.new(title: 'aaa', created_at: 3.months.until(Date.today)).valid?
+    assert Topic.new(title: 'aaa', created_at: 37.weeks.since(Date.today)).invalid?
+  end
+
+  def test_validates_inclusion_of_date_time_range
+    Topic.validates_inclusion_of(:created_at, in: 1.year.until(DateTime.current)..DateTime.current)
+    assert Topic.new(title: 'aaa', created_at: 2.years.until(DateTime.current)).invalid?
+    assert Topic.new(title: 'aaa', created_at: 3.months.until(DateTime.current)).valid?
+    assert Topic.new(title: 'aaa', created_at: 37.weeks.since(DateTime.current)).invalid?
+  end
+
   def test_validates_inclusion_of
     Topic.validates_inclusion_of(:title, in: %w( a b c d e f g ))
 
diff --git a/activemodel/test/models/topic.rb b/activemodel/test/models/topic.rb
index c9af78f595..1411a093e9 100644
--- a/activemodel/test/models/topic.rb
+++ b/activemodel/test/models/topic.rb
@@ -6,7 +6,7 @@ class Topic
     super | [ :message ]
   end
 
-  attr_accessor :title, :author_name, :content, :approved
+  attr_accessor :title, :author_name, :content, :approved, :created_at
   attr_accessor :after_validation_performed
 
   after_validation :perform_after_validation