Skip to content

Fix #5797. Error calling dup method on AR model with serialized field #5810

Merged
merged 1 commit into from May 30, 2012

4 participants

@kennyj
kennyj commented Apr 11, 2012

When we call the initialize_attributes method in serialization.rb, sometimes it is serialized, sometimes it is not serialized.

And for consistency I also changed the initialize_attributes's signiture in optimistic.rb.

Closes #5797

@gmcnaughton

Any activity on this? We're still stuck on 3.2.1 due to being unable to dup() objects with serialized fields.

@rafaelfranca rafaelfranca and 1 other commented on an outdated diff May 30, 2012
.../lib/active_record/attribute_methods/serialization.rb
@@ -72,12 +72,13 @@ def serialize(attr_name, class_name = Object)
self.serialized_attributes = serialized_attributes.merge(attr_name.to_s => coder)
end
- def initialize_attributes(attributes) #:nodoc:
- super
+ def initialize_attributes(attributes, options = {}) #:nodoc:
+ serialized = (!options.has_key?(:serialized) || options.delete(:serialized)) ? :serialized : :unserialized
@rafaelfranca
Ruby on Rails member
rafaelfranca added a note May 30, 2012

Why check for the key?

@kennyj
kennyj added a note May 30, 2012

It's compatibility.
If we don't have key, we should assign true to serialized variable.

@rafaelfranca
Ruby on Rails member
rafaelfranca added a note May 30, 2012

How about options.delete(:serialized) { true }?

@kennyj
kennyj added a note May 30, 2012

oops.. sorry. we must assign :serialized or :unserialized to serialized variable.

@rafaelfranca
Ruby on Rails member
rafaelfranca added a note May 30, 2012

I mean:

serialized = (options.delete(:serialized) { true }) ? :serialized : :unserialized
@kennyj
kennyj added a note May 30, 2012

cool ! I agree.
Test is still green :-)

@kennyj
kennyj added a note May 30, 2012

I rebased and updated this :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@kennyj kennyj Fix #5797. Error calling dup method on AR model with serialized field d09b2fd
@tenderlove tenderlove commented on the diff May 30, 2012
.../lib/active_record/attribute_methods/serialization.rb
@@ -72,12 +72,13 @@ def serialize(attr_name, class_name = Object)
self.serialized_attributes = serialized_attributes.merge(attr_name.to_s => coder)
end
- def initialize_attributes(attributes) #:nodoc:
- super
+ def initialize_attributes(attributes, options = {}) #:nodoc:
+ serialized = (options.delete(:serialized) { true }) ? :serialized : :unserialized
@tenderlove
Ruby on Rails member
tenderlove added a note May 30, 2012

Are you sure we should mutate options? Maybe options.fetch(:serialized, true) instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@tenderlove tenderlove merged commit 1b07522 into rails:master May 30, 2012
@rafaelfranca
Ruby on Rails member

@gmcnaughton Backported to 3-2-stable at c470001

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.