Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Wrong behavior of ActiveModel::Errors#dup is causing regressions on applications using Rails 3-2-stable #4492

Merged
merged 1 commit into from

2 participants

@pkondzior

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.

@pkondzior pkondzior 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.
7021184
@drogus drogus merged commit ed35f37 into from
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jan 17, 2012
  1. @pkondzior

    Fix ActiveModel::Errors#dup

    pkondzior authored
    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.
This page is out of date. Refresh to see the latest.
View
15 activemodel/lib/active_model/errors.rb
@@ -79,6 +79,19 @@ def initialize(base)
@messages = ActiveSupport::OrderedHash.new
end
+ def initialize_dup(other)
+ @messages = other.messages.dup
+ end
+
+ # Backport dup from 1.9 so that #initialize_dup gets called
+ unless Object.respond_to?(:initialize_dup)
+ def dup # :nodoc:
+ copy = super
+ copy.initialize_dup(self)
+ copy
+ end
+ end
+
# Clear the messages
def clear
messages.clear
@@ -119,7 +132,7 @@ def [](attribute)
# p.errors[:name] = "must be set"
# p.errors[:name] # => ['must be set']
def []=(attribute, error)
- self[attribute.to_sym] << error
+ self[attribute] << error
end
# Iterates through each error key, value pair in the error messages hash.
View
8 activemodel/test/cases/errors_test.rb
@@ -46,6 +46,14 @@ def test_has_key?
assert errors.has_key?(:foo), 'errors should have key :foo'
end
+ def test_dup
+ errors = ActiveModel::Errors.new(self)
+ errors[:foo] = 'bar'
+ errors_dup = errors.dup
+ errors_dup[:bar] = 'omg'
+ assert_not_same errors_dup.messages, errors.messages
+ end
+
test "should return true if no errors" do
person = Person.new
person.errors[:foo]
Something went wrong with that request. Please try again.