bringing over latest from master

11 files changed, 193 insertions, 36 deletions

diff --git a/activemodel/lib/active_model/attribute_methods.rb b/activemodel/lib/active_model/attribute_methods.rb
index 817640b178..a43436e008 100644
--- a/activemodel/lib/active_model/attribute_methods.rb
+++ b/activemodel/lib/active_model/attribute_methods.rb
@@ -283,7 +283,7 @@ module ActiveModel
         @attribute_methods_generated = true
       end
 
-      # Removes all the preiously dynamically defined methods from the class
+      # Removes all the previously dynamically defined methods from the class
       def undefine_attribute_methods
         generated_attribute_methods.module_eval do
           instance_methods.each { |m| undef_method(m) }
diff --git a/activemodel/lib/active_model/dirty.rb b/activemodel/lib/active_model/dirty.rb
index 5ea7636427..2516377afd 100644
--- a/activemodel/lib/active_model/dirty.rb
+++ b/activemodel/lib/active_model/dirty.rb
@@ -37,12 +37,13 @@ module ActiveModel
   #     end
   #   
   #     def name=(val)
-  #       name_will_change!
+  #       name_will_change! unless val == @name
   #       @name = val
   #     end
   #   
   #     def save
   #       @previously_changed = changes
+  #       @changed_attributes.clear
   #     end
   #   
   #   end
@@ -77,12 +78,6 @@ module ActiveModel
   #   person.changed        # => ['name']
   #   person.changes        # => { 'name' => ['Bill', 'Bob'] }
   #
-  # Resetting an attribute returns it to its original state:
-  #   person.reset_name!    # => 'Bill'
-  #   person.changed?       # => false
-  #   person.name_changed?  # => false
-  #   person.name           # => 'Bill'
-  #
   # If an attribute is modified in-place then make use of <tt>[attribute_name]_will_change!</tt>
   # to mark that the attribute is changing. Otherwise ActiveModel can't track changes to 
   # in-place attributes.
diff --git a/activemodel/lib/active_model/errors.rb b/activemodel/lib/active_model/errors.rb
index f39678db83..272ddb1554 100644
--- a/activemodel/lib/active_model/errors.rb
+++ b/activemodel/lib/active_model/errors.rb
@@ -37,11 +37,11 @@ module ActiveModel
   #       send(attr)
   #     end
   #   
-  #     def ErrorsPerson.human_attribute_name(attr, options = {})
+  #     def Person.human_attribute_name(attr, options = {})
   #       attr
   #     end
   #   
-  #     def ErrorsPerson.lookup_ancestors
+  #     def Person.lookup_ancestors
   #       [self]
   #     end
   #   
@@ -86,11 +86,7 @@ module ActiveModel
     #   p.errors[:name]   # => ["can not be nil"]
     #   p.errors['name']  # => ["can not be nil"]
     def [](attribute)
-      if errors = get(attribute.to_sym)
-        errors
-      else
-        set(attribute.to_sym, [])
-      end
+      get(attribute.to_sym) || set(attribute.to_sym, [])
     end
 
     # Adds to the supplied attribute the supplied error message.
diff --git a/activemodel/lib/active_model/serialization.rb b/activemodel/lib/active_model/serialization.rb
index 5670ec74cb..e675937f4d 100644
--- a/activemodel/lib/active_model/serialization.rb
+++ b/activemodel/lib/active_model/serialization.rb
@@ -61,6 +61,8 @@ module ActiveModel
   #   person.serializable_hash   # => {"name"=>"Bob"}
   #   person.to_json             # => "{\"name\":\"Bob\"}"
   #   person.to_xml              # => "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n<serial-person...
+  #
+  # Valid options are <tt>:only</tt>, <tt>:except</tt> and <tt>:methods</tt> .  
   module Serialization
     def serializable_hash(options = nil)
       options ||= {}
diff --git a/activemodel/lib/active_model/serializers/json.rb b/activemodel/lib/active_model/serializers/json.rb
index 500b2399a3..e1dbc522de 100644
--- a/activemodel/lib/active_model/serializers/json.rb
+++ b/activemodel/lib/active_model/serializers/json.rb
@@ -19,8 +19,8 @@ module ActiveModel
       # passed through +options+.
       #
       # The option <tt>ActiveModel::Base.include_root_in_json</tt> controls the
-      # top-level behavior of <tt>to_json</tt>. It is <tt>true</tt> by default. When it is <tt>true</tt>,
-      # <tt>to_json</tt> will emit a single root node named after the object's type. For example:
+      # top-level behavior of +to_json+. If true (the default) +to_json+ will
+      # emit a single root node named after the object's type. For example:
       #
       #   konata = User.find(1)
       #   konata.to_json
@@ -32,11 +32,11 @@ module ActiveModel
       #   # => {"id": 1, "name": "Konata Izumi", "age": 16,
       #         "created_at": "2006/08/01", "awesome": true}
       #
-      # The remainder of the examples in this section assume include_root_in_json is set to
-      # <tt>false</tt>.
+      # The remainder of the examples in this section assume +include_root_in_json+
+      # is false.
       #
-      # Without any +options+, the returned JSON string will include all
-      # the model's attributes. For example:
+      # Without any +options+, the returned JSON string will include all the model's
+      # attributes. For example:
       #
       #   konata = User.find(1)
       #   konata.to_json
@@ -52,14 +52,14 @@ module ActiveModel
       #   konata.to_json(:except => [ :id, :created_at, :age ])
       #   # => {"name": "Konata Izumi", "awesome": true}
       #
-      # To include any methods on the model, use <tt>:methods</tt>.
+      # To include the result of some method calls on the model use <tt>:methods</tt>:
       #
       #   konata.to_json(:methods => :permalink)
       #   # => {"id": 1, "name": "Konata Izumi", "age": 16,
       #         "created_at": "2006/08/01", "awesome": true,
       #         "permalink": "1-konata-izumi"}
       #
-      # To include associations, use <tt>:include</tt>.
+      # To include associations use <tt>:include</tt>:
       #
       #   konata.to_json(:include => :posts)
       #   # => {"id": 1, "name": "Konata Izumi", "age": 16,
@@ -67,7 +67,7 @@ module ActiveModel
       #         "posts": [{"id": 1, "author_id": 1, "title": "Welcome to the weblog"},
       #                   {"id": 2, author_id: 1, "title": "So I was thinking"}]}
       #
-      # 2nd level and higher order associations work as well:
+      # Second level and higher order associations work as well:
       #
       #   konata.to_json(:include => { :posts => {
       #                                  :include => { :comments => {
diff --git a/activemodel/lib/active_model/validations.rb b/activemodel/lib/active_model/validations.rb
index 1a58d4c4fb..3407c59e7a 100644
--- a/activemodel/lib/active_model/validations.rb
+++ b/activemodel/lib/active_model/validations.rb
@@ -118,11 +118,13 @@ module ActiveModel
       #   end
       #
       def validate(*args, &block)
-        options = args.last
-        if options.is_a?(Hash) && options.key?(:on)
+        options = args.extract_options!
+        if options.key?(:on)
+          options = options.dup
           options[:if] = Array.wrap(options[:if])
           options[:if] << "validation_context == :#{options[:on]}"
         end
+        args << options
         set_callback(:validate, *args, &block)
       end
 
diff --git a/activemodel/lib/active_model/validations/length.rb b/activemodel/lib/active_model/validations/length.rb
index c8a77ad666..a7af4f2b4d 100644
--- a/activemodel/lib/active_model/validations/length.rb
+++ b/activemodel/lib/active_model/validations/length.rb
@@ -40,8 +40,6 @@ module ActiveModel
 
         CHECKS.each do |key, validity_check|
           next unless check_value = options[key]
-          default_message = options[MESSAGES[key]]
-          options[:message] ||= default_message if default_message
 
           valid_value = if key == :maximum
             value.nil? || value.size.send(validity_check, check_value)
@@ -51,8 +49,13 @@ module ActiveModel
 
           next if valid_value
 
-          record.errors.add(attribute, MESSAGES[key],
-                            options.except(*RESERVED_OPTIONS).merge!(:count => check_value))
+          errors_options = options.except(*RESERVED_OPTIONS)
+          errors_options[:count] = check_value
+
+          default_message = options[MESSAGES[key]]
+          errors_options[:message] ||= default_message if default_message
+
+          record.errors.add(attribute, MESSAGES[key], errors_options)
         end
       end
     end
diff --git a/activemodel/lib/active_model/validator.rb b/activemodel/lib/active_model/validator.rb
index 52192d5988..163124d531 100644
--- a/activemodel/lib/active_model/validator.rb
+++ b/activemodel/lib/active_model/validator.rb
@@ -111,7 +111,7 @@ module ActiveModel #:nodoc:
 
     # Accepts options that will be made available through the +options+ reader.
     def initialize(options)
-      @options = options
+      @options = options.freeze
     end
 
     # Return the kind for this validator.
diff --git a/activemodel/test/cases/dirty_test.rb b/activemodel/test/cases/dirty_test.rb
index e1a35be384..858ae9cb69 100644
--- a/activemodel/test/cases/dirty_test.rb
+++ b/activemodel/test/cases/dirty_test.rb
@@ -3,10 +3,11 @@ require "cases/helper"
 class DirtyTest < ActiveModel::TestCase
   class DirtyModel
     include ActiveModel::Dirty
-    define_attribute_methods [:name]
+    define_attribute_methods [:name, :color]
 
     def initialize
       @name = nil
+      @color = nil
     end
 
     def name
@@ -17,13 +18,92 @@ class DirtyTest < ActiveModel::TestCase
       name_will_change!
       @name = val
     end
+
+    def color
+      @color
+    end
+
+    def color=(val)
+      color_will_change! unless val == @color
+      @color = val
+    end
+
+    def save
+      @previously_changed = changes
+      @changed_attributes.clear
+    end
+  end
+
+  setup do
+    @model = DirtyModel.new
+  end
+
+  test "setting attribute will result in change" do
+    assert !@model.changed?
+    assert !@model.name_changed?
+    @model.name = "Ringo"
+    assert @model.changed?
+    assert @model.name_changed?
+  end
+
+  test "list of changed attributes" do
+    assert_equal [], @model.changed
+    @model.name = "Paul"
+    assert_equal ['name'], @model.changed
+  end
+
+  test "changes to attribute values" do
+    assert !@model.changes['name']
+    @model.name = "John"
+    assert_equal [nil, "John"], @model.changes['name']
   end
 
   test "changes accessible through both strings and symbols" do
-    model = DirtyModel.new
-    model.name = "David"
-    assert_not_nil model.changes[:name]
-    assert_not_nil model.changes['name']
+    @model.name = "David"
+    assert_not_nil @model.changes[:name]
+    assert_not_nil @model.changes['name']
+  end
+
+  test "attribute mutation" do
+    @model.instance_variable_set("@name", "Yam")
+    assert !@model.name_changed?
+    @model.name.replace("Hadad")
+    assert !@model.name_changed?
+    @model.name_will_change!
+    @model.name.replace("Baal")
+    assert @model.name_changed?
+  end
+
+  test "resetting attribute" do
+    @model.name = "Bob"
+    @model.reset_name!
+    assert_nil @model.name
+    #assert !@model.name_changed #Doesn't work yet
+  end
+
+  test "setting color to same value should not result in change being recorded" do
+    @model.color = "red"
+    assert @model.color_changed?
+    @model.save
+    assert !@model.color_changed?
+    assert !@model.changed?
+    @model.color = "red"
+    assert !@model.color_changed?
+    assert !@model.changed?
+  end
+
+  test "saving should reset model's changed status" do
+    @model.name = "Alf"
+    assert @model.changed?
+    @model.save
+    assert !@model.changed?
+    assert !@model.name_changed?
+  end
+
+  test "saving should preserve previous changes" do
+    @model.name = "Jericho Cane"
+    @model.save
+    assert_equal [nil, "Jericho Cane"], @model.previous_changes['name']
   end
 
 end
diff --git a/activemodel/test/cases/errors_test.rb b/activemodel/test/cases/errors_test.rb
new file mode 100644
index 0000000000..79b45bb298
--- /dev/null
+++ b/activemodel/test/cases/errors_test.rb
@@ -0,0 +1,65 @@
+require "cases/helper"
+
+class ErrorsTest < ActiveModel::TestCase
+  class Person
+    extend ActiveModel::Naming
+    def initialize
+      @errors = ActiveModel::Errors.new(self)
+    end
+
+    attr_accessor :name
+    attr_reader   :errors
+
+    def validate!
+      errors.add(:name, "can not be nil") if name == nil
+    end
+
+    def read_attribute_for_validation(attr)
+      send(attr)
+    end
+
+    def self.human_attribute_name(attr, options = {})
+      attr
+    end
+
+    def self.lookup_ancestors
+      [self]
+    end
+
+  end
+
+  test "method validate! should work" do
+    person = Person.new
+    person.validate!
+    assert_equal ["name can not be nil"], person.errors.full_messages
+    assert_equal ["can not be nil"], person.errors[:name]
+
+  end
+
+  test 'should be able to assign error' do
+    person = Person.new
+    person.errors[:name] = 'should not be nil'
+    assert_equal ["should not be nil"], person.errors[:name]
+  end
+
+  test 'should be able to add an error on an attribute' do
+    person = Person.new
+    person.errors.add(:name, "can not be blank")
+    assert_equal ["can not be blank"], person.errors[:name]
+  end
+
+  test 'should respond to size' do
+    person = Person.new
+    person.errors.add(:name, "can not be blank")
+    assert_equal 1, person.errors.size
+  end
+
+  test 'to_a should return an array' do
+    person = Person.new
+    person.errors.add(:name, "can not be blank")
+    person.errors.add(:name, "can not be nil")
+    assert_equal ["name can not be blank", "name can not be nil"], person.errors.to_a
+
+  end
+
+end
diff --git a/activemodel/test/cases/validations/length_validation_test.rb b/activemodel/test/cases/validations/length_validation_test.rb
index 012c5a2f37..1e6180a938 100644
--- a/activemodel/test/cases/validations/length_validation_test.rb
+++ b/activemodel/test/cases/validations/length_validation_test.rb
@@ -229,6 +229,20 @@ class LengthValidationTest < ActiveModel::TestCase
     assert_equal ["hoo 5"], t.errors["title"]
   end
 
+  def test_validates_length_of_custom_errors_for_both_too_short_and_too_long
+    Topic.validates_length_of :title, :minimum => 3, :maximum => 5, :too_short => 'too short', :too_long => 'too long'
+
+    t = Topic.new(:title => 'a')
+    assert t.invalid?
+    assert t.errors[:title].any?
+    assert_equal ['too short'], t.errors['title']
+
+    t = Topic.new(:title => 'aaaaaa')
+    assert t.invalid?
+    assert t.errors[:title].any?
+    assert_equal ['too long'], t.errors['title']
+  end
+
   def test_validates_length_of_custom_errors_for_is_with_message
     Topic.validates_length_of( :title, :is=>5, :message=>"boo %{count}" )
     t = Topic.new("title" => "uhohuhoh", "content" => "whatever")