aaronmallen/activeinteractor

Rails 6.1 changes how ActiveModels are deep_dup'ed

dark-panda opened this issue · 2 comments

Current Behavior

#deep_dup is used in ActiveInteractor::Interactor::Perform on @context to dup the whole structure. In versions of Rails prior to 6.1, this would not recurse down into the attributes. However, it seems that the following change implements an #initialize_dup on ActiveModel::Attributes that causes the attributes themselves to be duped, which has an unfortunate side effect on any code relying on the old behaviour: because attributes are now being deeply dup'ed, complex objects like ActiveRecords and ActiveModels are initialized weirdly. If you pass either of these object types to an ActiveInteractor, you get the dup'ed version of that object, and not the object itself, which is really not what you want. Otherwise, you'll end up with things like a dup'ed ActiveRecord object that contains a new record that's a dup of an existing record, rather than the record you actually want to pass along.

The breaking commit is here:

rails/rails@199e4e9

I cannot find this change documented, but it's a pretty important one to anyone using ActiveModel::Attributes. I would go so far as to say that this is a bug in Rails, or at least the change in behaviour should have been documented, as it is unexpected based on previous bevhaviour.

Expected Behaviour

These attributes should not be deeply dup'ed in this context.

Steps to Reproduce

  1. Pass a persisted ActiveRecord to an ActiveInteractor and inspect the result after the #deep_dup is performed.

Additional Comments

Dup'ing may have to be done in a more manual way in order to dup @context and @options to avoid unexpectedly dup'ing the contents of a context, which includes ActiveModel::Attributes.

I added the following to my base ApplicationInteractorContext class from which I inherit all of my interactor contexts and it restores the previous behaviour. Now I just have a bunch of other specs failing for different reasons altogether.

def initialize_dup(other)
  avoid_attributes_dup = @attributes
  @attributes = nil
  super
ensure
  @attributes = avoid_attributes_dup
end

Not exactly an ideal fix, but it does work.

Not sure if this is related, but I've been struck in an infinite loops after the upgrade to 6.1.

I have scoped down the issue to merging errors

def merge_errors!(other)
errors.merge!(other.errors)
failure_errors.merge!(other.errors)
failure_errors.merge!(other.failure_errors)
end

Seems like self and other have the same reference which causes an infinite loop. I have verified that by applying the following patch

def merge_errors!(other)
  failure_errors.merge!(other.errors)

  return if self.object_id == other.object_id

  errors.merge!(other.errors)
  failure_errors.merge!(other.failure_errors)   
end