[PATCH] ActiveRecord::Base#dup changed drastically in 3.0, should be brought back to 2.x behaviour

[lighthouse-import]

Imported from Lighthouse. Original ticket at: http://rails.lighthouseapp.com/projects/8994/tickets/5917
Created by James MacAulay - 2010-11-04 16:15:12 UTC

The problem comes from ticket #3164, which ended up as commit 6361d42.

Its concern was with AR::B#clone, but it had the side effect of giving dup almost exactly the same behaviour as clone, since both methods call initialize_copy as a hook in ruby.

As a result, these are the main changes to duped records in Rails 3.0:

• new_record is set to true
• dirty tracking is set so that all attributes with non-default values are considered changed

Because dup itself also has a custom implementation which duplicates the attributes hash after initialize_copy is called, the behaviour of clone to remove the primary key was not carried over to dup. This combination of things means that, for example, you can't save a dup of a persisted record:

    >> Shop.first.dup.save
    ActiveRecord::RecordNotUnique: Mysql::Error: Duplicate entry '2' for key 1: INSERT INTO `shops` ...

The error is raised because Active Record looks at new_record, sees that it's true, decides to use an INSERT instead of an UPDATE, and the INSERT tries to save a primary key which already exists.

In Rails 2.x, clone was used to create copies intended to be saved as new records, while dup was a simpler operation which made a more direct (but still shallow) copy of the object. Both methods would remove the frozen state of the original.

Notes on the patch:

The provided patch brings dup back in line with its old behaviour, while maintaining the improvements made to clone which still seem to make sense. One minor improvement to clone was made as well, to set its previous_changes to an empty hash (since it doesn't make any sense for it to keep the hash from the original). Tests have been added both to establish new behaviour and clarify existing behaviour. Many of the dup tests were just copied from the clone tests with minor changes.

The weirdest aspect of the patch is that it checks RUBY_VERSION in a few places. This is ugly, I know. Basically I wanted people to still be able to override initialize_copy in the same way that the original ticket intended, while also providing different behaviour for clone and dup. I thought an initialize_dup method would be a good thing to have, and then found that ruby 1.9 already has it. (Ruby 1.9 will first look for initialize_clone or initialize_dup as appropriate, and only fall back to initialize_copy if the more specific hook is not implemented.) So the checks on RUBY_VERSION are there so that initialize_copy is run exactly once, followed by initialize_clone/initialize_dup exactly once, regardless of ruby version. If the checks weren't there, then one method or another would be called twice, dependent on ruby version.

I can think of a few alternate ways to handle this, by sacrificing a little bit of backwards compatibility and/or ruby version compatibility for the sake of cleaner code.

[lighthouse-import]

Imported from Lighthouse.
Comment by Jeff Kreeftmeijer - 2010-11-08 08:27:42 UTC

Automatic cleanup of spam.

[lighthouse-import]

Imported from Lighthouse.
Comment by Aaron Patterson - 2010-11-23 01:14:33 UTC

@james, would you mind rebasing this patch from 3-0-stable, and I'll apply it?

[lighthouse-import]

Imported from Lighthouse.
Comment by Santiago Pastorino - 2011-02-12 21:58:22 UTC

[bulk edit]

[lighthouse-import]

Attachments saved to Gist: http://gist.github.com/971722