This project is archived and is in readonly mode.
[PATCH] ActiveRecord::Base#dup changed drastically in 3.0, should be brought back to 2.x behaviour
Reported by James MacAulay | November 4th, 2010 @ 04:15 PM
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.
Comments and changes to this ticket
-
James MacAulay November 4th, 2010 @ 04:19 PM
- no changes were found...
-
Aaron Patterson November 4th, 2010 @ 09:38 PM
- Assigned user set to Aaron Patterson
- Importance changed from to Low
-
Aaron Patterson November 23rd, 2010 @ 01:14 AM
- Milestone cleared.
@James, would you mind rebasing this patch from 3-0-stable, and I'll apply it?
-
Aaron Patterson December 15th, 2010 @ 10:06 PM
- State changed from new to incomplete
-
Aaron Patterson February 25th, 2011 @ 11:49 PM
- Milestone cleared.
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>