This project is archived and is in readonly mode.

#3608 ✓resolved
Jorge Dias

OrderedHash serialization doesn't work in AR

Reported by Jorge Dias | December 23rd, 2009 @ 01:32 PM | in 2.3.6

When you serailize an OrderedHash in ActiveRecord, when you retreive the value, it doesn't initialize the object, so you get errors on OrderedHash, because the @keys variable is nil.

undefined method each' for nil:NilClass

/opt/ruby-enterprise/lib/ruby/gems/1.8/gems/activesupport-2.3.5/lib/active_support/ordered_hash.rb:97:in each' /opt/ruby-enterprise/lib/ruby/1.8/yaml/rubytypes.rb:40:into_yaml' /opt/ruby-enterprise/lib/ruby/1.8/yaml/rubytypes.rb:39:in map' /opt/ruby-enterprise/lib/ruby/1.8/yaml/rubytypes.rb:39:into_yaml' /opt/ruby-enterprise/lib/ruby/1.8/yaml.rb:391:in call' /opt/ruby-enterprise/lib/ruby/1.8/yaml.rb:391:inemit' /opt/ruby-enterprise/lib/ruby/1.8/yaml.rb:391:in quick_emit' /opt/ruby-enterprise/lib/ruby/1.8/yaml/rubytypes.rb:38:into_yaml' /opt/ruby-enterprise/lib/ruby/gems/1.8/gems/activerecord-2.3.5/lib/active_record/connection_adapters/abstract/quoting.rb:31:in quote_before_arext' /opt/ruby-enterprise/lib/ruby/gems/1.8/gems/ar-extensions-0.9.2/lib/ar-extensions/finders.rb:9:inquote' /opt/ruby-enterprise/lib/ruby/gems/1.8/gems/activerecord-2.3.5/lib/active_record/connection_adapters/mysql_adapter.rb:236:in quo! te' /opt/ruby-enterprise/lib/ruby/gems/1.8/gems/activerecord-2.3.5/lib/active_record/base.rb:2992:inattributes_with_quotes' /opt/ruby-enterprise/lib/ruby/gems/1.8/gems/activerecord-2.3.5/lib/active_record/base.rb:2983:in each' /opt/ruby-enterprise/lib/ruby/gems/1.8/gems/activerecord-2.3.5/lib/active_record/base.rb:2983:inattributes_with_quotes' /opt/ruby-enterprise/lib/ruby/gems/1.8/gems/activerecord-2.3.5/lib/active_record/base.rb:2881:in update_without_lock' /opt/ruby-enterprise/lib/ruby/gems/1.8/gems/activerecord-2.3.5/lib/active_record/locking/optimistic.rb:70:inupdate_without_dirty' /opt/ruby-enterprise/lib/ruby/gems/1.8/gems/activerecord-2.3.5/lib/active_record/dirty.rb:146:in update_without_timestamps' /opt/ruby-enterprise/lib/ruby/gems/1.8/gems/activerecord-2.3.5/lib/active_record/timestamp.rb:64:inupdate_without_callbacks' /opt/ruby-enterprise/lib/ruby/gems/1.8/gems/activerecord-2.3.5/lib/active_record/callbacks.rb! :282:in update' /opt/ruby-enterprise/lib/ruby/gems/1.8/g! ems/activerecord-2.3.5/lib/active_record/base.rb:2874:increate_or_update_without_callbacks'

/opt/ruby-enterprise/lib/ruby/gems/1.8/gems/activerecord-2.3.5/lib/active_record/callbacks.rb:250:in create_or_update' /opt/ruby-enterprise/lib/ruby/gems/1.8/gems/activerecord-2.3.5/lib/active_record/base.rb:2538:insave_without_validation' /opt/ruby-enterprise/lib/ruby/gems/1.8/gems/activerecord-2.3.5/lib/active_record/validations.rb:1078:in save_without_dirty' /opt/ruby-enterprise/lib/ruby/gems/1.8/gems/activerecord-2.3.5/lib/active_record/dirty.rb:79:insave_without_transactions' /opt/ruby-enterprise/lib/ruby/gems/1.8/gems/activerecord-2.3.5/lib/active_record/transactions.rb:229:in send' /opt/ruby-enterprise/lib/ruby/gems/1.8/gems/activerecord-2.3.5/lib/active_record/transactions.rb:229:inwith_transaction_returning_status' /opt/ruby-enterprise/lib/ruby/gems/1.8/gems/activerecord-2.3.5/lib/active_record/connection_adapters/abstract/databa! se_statements.rb:136:in transaction' /opt/ruby-enterprise/lib/ruby/gems/1.8/gems/activerecord-2.3.5/lib/active_record/transactions.rb:182:intransaction'

...

Comments and changes to this ticket

  • Jorge Dias

    Jorge Dias December 23rd, 2009 @ 01:33 PM

    • Tag changed from active_record, hash, serialization to active_record, active_support, hash, serialization
  • Serguei Filimonov

    Serguei Filimonov December 24th, 2009 @ 09:33 PM

    • Tag changed from active_record, active_support, hash, serialization to active_record, active_support, hash, serialization, yaml

    Hey I tried this in both 2-3 stable and master branches. Can't reproduce it. Could you provide perhaps a small code snippet that fails to run because of this?

  • Prem Sichanugrist (sikachu)

    Prem Sichanugrist (sikachu) December 25th, 2009 @ 03:49 PM

    I wonder that this problem arise from using ar-extensions plugin?

    Can you try to remove the plugin first, and see if the problem still exists?

  • Dave Ungerer

    Dave Ungerer December 25th, 2009 @ 07:01 PM

    I'm getting the same error, and I don't use ar-extensions. The error happens only in Rails 2.3.5, not in 2.3.4.

    I have an AR model with a serialized hash attribute, which itself contains some nested hashes. The error below occurs when saving the model. Hope that helps. I'll see if I can get a stand-alone code snippet to replicate this if it remains outstanding for too long.

    NoMethodError: You have a nil object when you didn't expect it!
    You might have expected an instance of Array.
    The error occurred while evaluating nil.each
    /opt/ruby-enterprise-1.8.7-2009.10/lib/ruby/1.8/yaml/rubytypes.rb:40:in `to_yaml'
    /opt/ruby-enterprise-1.8.7-2009.10/lib/ruby/1.8/yaml/rubytypes.rb:39:in `map'
    /opt/ruby-enterprise-1.8.7-2009.10/lib/ruby/1.8/yaml/rubytypes.rb:39:in `to_yaml'
    /opt/ruby-enterprise-1.8.7-2009.10/lib/ruby/1.8/yaml.rb:391:in `call'
    /opt/ruby-enterprise-1.8.7-2009.10/lib/ruby/1.8/yaml.rb:391:in `emit'
    /opt/ruby-enterprise-1.8.7-2009.10/lib/ruby/1.8/yaml.rb:391:in `quick_emit'
    /opt/ruby-enterprise-1.8.7-2009.10/lib/ruby/1.8/yaml/rubytypes.rb:38:in `to_yaml'
    /opt/ruby-enterprise-1.8.7-2009.10/lib/ruby/1.8/yaml/rubytypes.rb:18:in `node_export'
    /opt/ruby-enterprise-1.8.7-2009.10/lib/ruby/1.8/yaml/rubytypes.rb:18:in `add'
    /opt/ruby-enterprise-1.8.7-2009.10/lib/ruby/1.8/yaml/rubytypes.rb:18:in `to_yaml'
    /opt/ruby-enterprise-1.8.7-2009.10/lib/ruby/1.8/yaml/rubytypes.rb:17:in `each'
    /opt/ruby-enterprise-1.8.7-2009.10/lib/ruby/1.8/yaml/rubytypes.rb:17:in `to_yaml'
    /opt/ruby-enterprise-1.8.7-2009.10/lib/ruby/1.8/yaml/rubytypes.rb:16:in `map'
    /opt/ruby-enterprise-1.8.7-2009.10/lib/ruby/1.8/yaml/rubytypes.rb:16:in `to_yaml'
    /opt/ruby-enterprise-1.8.7-2009.10/lib/ruby/1.8/yaml.rb:391:in `call'
    /opt/ruby-enterprise-1.8.7-2009.10/lib/ruby/1.8/yaml.rb:391:in `emit'
    /opt/ruby-enterprise-1.8.7-2009.10/lib/ruby/1.8/yaml.rb:391:in `quick_emit'
    /opt/ruby-enterprise-1.8.7-2009.10/lib/ruby/1.8/yaml/rubytypes.rb:15:in `to_yaml'
    /opt/ruby-enterprise-1.8.7-2009.10/lib/ruby/1.8/yaml/rubytypes.rb:18:in `node_export'
    /opt/ruby-enterprise-1.8.7-2009.10/lib/ruby/1.8/yaml/rubytypes.rb:18:in `add'
    /opt/ruby-enterprise-1.8.7-2009.10/lib/ruby/1.8/yaml/rubytypes.rb:18:in `to_yaml'
    /opt/ruby-enterprise-1.8.7-2009.10/lib/ruby/1.8/yaml/rubytypes.rb:17:in `each'
    /opt/ruby-enterprise-1.8.7-2009.10/lib/ruby/1.8/yaml/rubytypes.rb:17:in `to_yaml'
    /opt/ruby-enterprise-1.8.7-2009.10/lib/ruby/1.8/yaml/rubytypes.rb:16:in `map'
    /opt/ruby-enterprise-1.8.7-2009.10/lib/ruby/1.8/yaml/rubytypes.rb:16:in `to_yaml'
    /opt/ruby-enterprise-1.8.7-2009.10/lib/ruby/1.8/yaml.rb:391:in `call'
    /opt/ruby-enterprise-1.8.7-2009.10/lib/ruby/1.8/yaml.rb:391:in `emit'
    /opt/ruby-enterprise-1.8.7-2009.10/lib/ruby/1.8/yaml.rb:391:in `quick_emit'
    /opt/ruby-enterprise-1.8.7-2009.10/lib/ruby/1.8/yaml/rubytypes.rb:15:in `to_yaml'
    /opt/ruby-enterprise-1.8.7-2009.10/lib/ruby/1.8/yaml/rubytypes.rb:41:in `node_export'
    /opt/ruby-enterprise-1.8.7-2009.10/lib/ruby/1.8/yaml/rubytypes.rb:41:in `add'
    /opt/ruby-enterprise-1.8.7-2009.10/lib/ruby/1.8/yaml/rubytypes.rb:41:in `to_yaml'
    /opt/ruby-enterprise-1.8.7-2009.10/lib/ruby/1.8/yaml/rubytypes.rb:40:in `each'
    /opt/ruby-enterprise-1.8.7-2009.10/lib/ruby/1.8/yaml/rubytypes.rb:40:in `to_yaml'
    /opt/ruby-enterprise-1.8.7-2009.10/lib/ruby/1.8/yaml/rubytypes.rb:39:in `map'
    /opt/ruby-enterprise-1.8.7-2009.10/lib/ruby/1.8/yaml/rubytypes.rb:39:in `to_yaml'
    /opt/ruby-enterprise-1.8.7-2009.10/lib/ruby/1.8/yaml.rb:391:in `call'
    /opt/ruby-enterprise-1.8.7-2009.10/lib/ruby/1.8/yaml.rb:391:in `emit'
    /opt/ruby-enterprise-1.8.7-2009.10/lib/ruby/1.8/yaml.rb:391:in `quick_emit'
    /opt/ruby-enterprise-1.8.7-2009.10/lib/ruby/1.8/yaml/rubytypes.rb:38:in `to_yaml'
    
  • Dave Ungerer

    Dave Ungerer January 15th, 2010 @ 11:53 AM

    I can now confirm that this bug doesn't happen under Ruby 1.9.1. That leaves 2 possibilities that I can see:

    1. It's a bug in Ruby Enterprise Edition (since both bug reports used it)
    2. It's a general 1.8 incompatibility introduced when fixing 1.9 compatibility. (since the bug was introduced in Rails 2.3.5)

    I'm not going to bother installing and testing with MRI 1.8.7 to narrow down the possiblities, because it's not a deployment option for me. Will just upgrade to Ruby 1.9.1 instead - been needing an excuse.

  • Gregor Schmidt

    Gregor Schmidt January 25th, 2010 @ 06:55 PM

    I am seeing this bug as well. The test case on http://gist.github.com/286087 is the shortest one, I could come up with.

    It fails with activesupport -v 2.3.5 on

    • Ruby 1.8.6-p287
    • Ruby 1.8.7-p248
    • JRuby 1.4
    • JRuby HEAD

    As far as I can tell, there's no direct relation to AR - it just uses the broken functionality of AS.

  • Martin Gamsjaeger (snusnu)

    Martin Gamsjaeger (snusnu) January 26th, 2010 @ 03:17 AM

    • Tag changed from active_record, active_support, hash, serialization, yaml to active_record, active_support, datamapper, hash, serialization, yaml

    I can confirm this too. I don't use AR, I'm a datamapper user, but it happens when I run specs for dm-serializer from branches of dm-core and dm-more that work with active_support

    http://github.com/snusnu/dm-core/tree/active_support
    http://github.com/snusnu/dm-more/tree/active_support

    Now the cool thing is that this is the only failing spec for the whole of dm-core and dm-more specs, when running with active_support instead of extlib.

    I would really appreciate a fix for this, since that would mean that datamapper is 100% compatible with active_support and my patches could safely be pulled into dm-core and dm-more repositories. This, in combination with http://github.com/dkubb/rails3_datamapper would mean that datamapper is fully usable with current rails master.

    Here's what I get:

    undefined method `each' for nil:NilClass
    /Users/snusnu/.rvm/gems/ree/1.8.7%rails3/gems/activesupport-3.0.pre/lib/active_support/ordered_hash.rb:97:in `each'
    
  • Dan Kubb (dkubb)

    Dan Kubb (dkubb) January 26th, 2010 @ 06:45 AM

    Based on Gregor's test case reduction, it looks like YAML.load is creating the OrderedHash object without calling #initialize (or for that matter the allocate or new class methods).

    Here's a simple work-around I found that causes Gregor's test to pass:

      class ActiveSupport::OrderedHash
        def yaml_initialize(*)
          initialize()  # initialize @keys
          super
        end
      end
    

    I would submit a patch, but I wanted to understand why YAML is doing this. Is it a bug, or was it on purpose? I mean, I can understand why it might not call new directly, but why is it not calling allocate? (and how is it doing that... is there some part of the MRI C API that allows it to allocate a Ruby object without calling any of the normal methods?) I can't tell if it's bypassing things on purpose, with a valid reason, or not.

    I also couldn't really find any documentation on yaml_initialize. I found it by looking through the source. It seems to be reasonably well used in other parts of YAML, but I was cautious to recommend using it unless my usage is valid.

  • Gregor Schmidt

    Gregor Schmidt January 26th, 2010 @ 09:17 AM

    Okay, thanks for the work-around. It already solves my immediate problem. But unfortunately it does not restore the key ordering within the hash, which would be the main feature of this class, I guess.

    I have attached a patch against 2-3-stable containing two failing test cases. One of them is fixed by the above patch, the other is still broken.

    Regarding your concerns about the usage of hash_initialize: I guess it is save to use this feature. It's used all over the place in YAML so I would not expect it to disappear.

  • Gregor Schmidt

    Gregor Schmidt January 26th, 2010 @ 11:07 AM

    So, I finally got a little closer.

    First: the above test cases fail both with Dan's workaround.

    Second: I managed to find two alternative fixes for the underlying problem. One uses the "yaml_initialize" method, the other uses "add_domain_type" which seems to be the official way to serialize and deserialize custom objects with syck.

    I have attached complete patches against 2-3-stable for both implementations.

  • Martin Gamsjaeger (snusnu)

    Martin Gamsjaeger (snusnu) January 26th, 2010 @ 02:55 PM

    • State changed from “new” to “verified”

    Thx a lot Gregor! I tried both your patches, both of them apply cleanly against current rails master too, and both of them solve the failing spec in dm-more/dm-serializer. Now if only I knew which one is better suited for being applied, I would vote for one of them. Since I don't know it, I'll leave that up for others to decide :)

    Again, thx Gregor for taking the time to investigate this!

    PS: I'm not sure which milestone to assign, since I guess we definitely want to have that in 3.0 but in 2.x too. I hope changing the Ticket state to verified is ok, since I can definitely verify the original bug and the fix, but I'm new to the rules of rails' ticket handling (probably that's also the reason why I left Priority to be low for the time being :)

  • Dan Kubb (dkubb)

    Dan Kubb (dkubb) January 26th, 2010 @ 06:07 PM

    I should also note that the YAML spec has defined an ordered map type that may fit this use case more closely. It's defined as an "Ordered sequence of key:value pairs without duplicates".

  • Dan Kubb (dkubb)

    Dan Kubb (dkubb) January 26th, 2010 @ 06:12 PM

    Another consideration that I had is should we make it so that this works the same between Ruby 1.8 and 1.9? (sorry for the extra post, I can't edit the comment I just submitted)

    From the way AS::OrderedHash is defined, it looks like it delegates straight to the Hash if Ruby 1.9 is being used. That would bypass the patched code Gregor submitted and just rely on whatever defaults the Ruby 1.9 Hash has. Depending on which approach is used, I think you'd want it to work the same between Ruby 1.8 and 1.9, so that if something was serialized with 1.8 it would be readable with someone using AS::OrderedHash with 1.9, and vice versa.

  • Gregor Schmidt

    Gregor Schmidt January 27th, 2010 @ 10:52 AM

    First of all, thanks for the reviews, Martin and Dan.

    Dan's remarks made me think again about the implementation strategy. As far as I can tell, there is only one way to allow an OrderedHash serialized in 1.9 to be correctly deserialized in 1.8 - we have to tag it specifically to be an ordered one. Therefore I now also subclassed Hash in 1.9 and - for all ruby version - added a custom serialization matching YAML's built-in ordered map. This way, anything expicitly instantiated as AS::OrderedHash will be serialized as !!omap and is loaded into an OrderedHash on the other side - full round trip. I think, this would not be possible using the "yaml_initialize"-strategy - therefore I dropped this implementation.

    The only problem, that may arise is, if other libraries start defining custom deserializations for !!omap - this would be a reason to not use "add_builtin_type" but "add_domain_type" with something like "ActiveSupport,2010/orderedHash".

    As far as I can tell, the attached patch passes on

    • 1.8.5
    • 1.8.6
    • 1.8.7
    • 1.9.1
    • JRuby 1.4.0

    I would welcome any feedback.

  • Martin Gamsjaeger (snusnu)

    Martin Gamsjaeger (snusnu) January 27th, 2010 @ 05:26 PM

    • State changed from “verified” to “incomplete”

    Gregor,

    Again, thx for doing the work on this. I just tried your new patch - which btw looks like a good way of solving the problem to me - but unfortunately I got (different) failures in dm-more/dm-serializer specs. Here's what I get now:

    expected: {"name"=>["Name must be at least 2 characters long"], "solar_system_id"=>["Solar system must not be blank"]},
         got: {"name"=>"Name must be at least 2 characters long", "solar_system_id"=>"Solar system must not be blank"} (using ==)
    

    So it seems there's some problem with serializing (array) values? Oh and btw, active_support tests all pass with your patch.

  • Jeremy Kemper

    Jeremy Kemper January 27th, 2010 @ 09:50 PM

    • Assigned user set to “Jeremy Kemper”
    • Milestone set to 2.3.6
    +    ActiveSupport::OrderedHash[*val.map(&:to_a).flatten]
    

    Note that flatten is recursive. This will over-flatten array values.

  • Gregor Schmidt

    Gregor Schmidt January 27th, 2010 @ 10:37 PM

    Thanks for the hints. I hope, I've got it right this time.

    I've added a test with a nested array. It was failing with my latest patch. I then fixed the "flatten" problem. Please have a look at the newly attached patch.

  • Gregor Schmidt
  • Martin Gamsjaeger (snusnu)

    Martin Gamsjaeger (snusnu) January 27th, 2010 @ 11:04 PM

    • State changed from “incomplete” to “verified”

    Yes! Now all dm-core and dm-more specs pass with active_support :) Also, active_support tests pass for me. I should probably add that I haven't tested the scenario of serializing with 1.8 and deserializing with 1.9 (and vice versa), but I can confirm that the issues I previously had are now fixed.

    Thx for taking the time guys!

  • Repository

    Repository January 27th, 2010 @ 11:31 PM

    • State changed from “verified” to “committed”

    (from [57337cd74db0596a1e9f8eb0e6df0bb8ccf53dd2]) Adding custom yaml (de-)serialization for OrderedHash

    [#3608 state:committed]

    Signed-off-by: Jeremy Kemper jeremy@bitsweat.net
    http://github.com/rails/rails/commit/57337cd74db0596a1e9f8eb0e6df0b...

  • Repository
  • Martin Gamsjaeger (snusnu)

    Martin Gamsjaeger (snusnu) January 28th, 2010 @ 01:10 AM

    • State changed from “committed” to “incomplete”

    Jeremy,

    OrderedHash now requires yaml, and in order to be properly cherry pickable, ordered_hash.rb should require 'yaml'. (My datamapper sample app won't boot otherwise). The obviously trivial patch can be found at:

    http://github.com/snusnu/rails/commit/7e5379189e4398d91c77fb5eea119...

  • Jeremy Kemper

    Jeremy Kemper January 28th, 2010 @ 02:14 AM

    • State changed from “incomplete” to “resolved”

    Thanks Martin!

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

<h2 style="font-size: 14px">Tickets have moved to Github</h2>

The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>

Referenced by

Pages