Skip to content
This repository
Browse code

Fix ActiveModel::Errors#dup

Since ActiveModel::Errors instance keeps all error messages as hash
we should duplicate this object as well.

Previously ActiveModel::Errors was a subclass of ActiveSupport::OrderedHash,
which results in different behavior on dup, this may result in regression for
people relying on it.

Because Rails 3.2 stills supports Ruby 1.8.7 in order to properly fix this
regression we need to backport #initialize_dup.
  • Loading branch information...
commit 5da6b6ed672cead5ece53ecab248fe3fdcc3503a 1 parent f34e5a7
Paweł Kondzior authored January 16, 2012 drogus committed January 23, 2012
15  activemodel/lib/active_model/errors.rb
@@ -79,6 +79,19 @@ def initialize(base)
79 79
       @messages = ActiveSupport::OrderedHash.new
80 80
     end
81 81
 
  82
+    def initialize_dup(other)
  83
+      @messages = other.messages.dup
  84
+    end
  85
+
  86
+    # Backport dup from 1.9 so that #initialize_dup gets called
  87
+    unless Object.respond_to?(:initialize_dup)
  88
+      def dup # :nodoc:
  89
+        copy = super
  90
+        copy.initialize_dup(self)
  91
+        copy
  92
+      end
  93
+    end
  94
+
82 95
     # Clear the messages
83 96
     def clear
84 97
       messages.clear
@@ -118,7 +131,7 @@ def [](attribute)
118 131
     #   p.errors[:name] = "must be set"
119 132
     #   p.errors[:name] # => ['must be set']
120 133
     def []=(attribute, error)
121  
-      self[attribute.to_sym] << error
  134
+      self[attribute] << error
122 135
     end
123 136
 
124 137
     # Iterates through each error key, value pair in the error messages hash.
8  activemodel/test/cases/errors_test.rb
@@ -40,6 +40,14 @@ def test_include?
40 40
     assert errors.include?(:foo), 'errors should include :foo'
41 41
   end
42 42
 
  43
+  def test_dup
  44
+    errors = ActiveModel::Errors.new(self)
  45
+    errors[:foo] = 'bar'
  46
+    errors_dup = errors.dup
  47
+    errors_dup[:bar] = 'omg'
  48
+    assert_not_same errors_dup.messages, errors.messages
  49
+  end
  50
+
43 51
   test "should return true if no errors" do
44 52
     person = Person.new
45 53
     person.errors[:foo]

0 notes on commit 5da6b6e

Please sign in to comment.
Something went wrong with that request. Please try again.