clowne-rb/clowne

`dependent: :destroy` option on association destroys models related to source after cloning

Opened this issue · 3 comments

Associations that contain dependent: :destroy option will be destroyed on source object after the cloning process. It's easy to reproduce in your tests - just add
dependent: :destroy to has_one :image, class_name: 'AR::Image' and tests will fail.

I had debugged the issue a little bit and here is what I found:

This line is destroying association on source model:

  # Clowne::Adapters::ActiveRecord::Associations::HasOne
  record.__send__(:"#{association_name}=", child_clone)

Problem is that record.__send__(:"#{association_name}") is not nil, actually it's association of source object, so when we set new association to child_clone active record destroys source association because of 'dependent: :destroy' option.

To fix it, I think, we need to change the way how gem does initial copy of the object:

# Clowne::Adapters::Base
init_record = init_record(self.class.dup_record(source))

This line calls ActiveRecord's dup method, which duplicates a model + parent associations.

After a fresh look, I've realized what's going on:
I have a model with primary key :id and additional column uuid, my association looks like this:

has_one :image, dependent: :destroy, inverse_of: :post, foreign_key: :post_uuid, primary_key: :uuid

So when the cloning process starts, :id is set to nil, but uuid is not(I have used nullify :uuid), so newly created clone already has a relation of a source object and when clowne sets a cloned version of this association rails destroys original one due to dependent: :destroy option. I've fixed this problem by using init_as:

  init_as do |source|
    clone = source.dup
    clone.uuid = nil
    clone
  end

It would be great to check if an association has dependent option and if an association is already present before clone associations step then raise an error for such idiots like me :)
Thanks a lot for such nice gem!

Hi @dolhishev! Your case is interesting and unusual but make sense, yeh.
First of all, if this approach with two different columns (id and uuid) is used for all your models, you can define your adapter:

class UUIDActiveRecordAdapter < Clowne::Adapters::ActiveRecord
  class << self
    def dup_record(record)
       super.tap do |clone|
         clone.uuid = nil
       end
    end
  end
end

# and 
class BaseCloner < Clowne::Cloner
  adapter UUIDActiveRecordAdapter
end

class SomeModelCloner < BaseCloner
  # declarations
end

See here and here.

It would be great to check if an association has dependent option and if an association is already present before clone associations step then raise an error for such idiots like me :)

Hm, maybe is too rude to raise an exception but we can handle this case and put a warning message. I'll think about it.
And thanks for the report 👍