with_model icon indicating copy to clipboard operation
with_model copied to clipboard

Call of cleanup_descendants_tracking cleanup too much models

Open kakuzei opened this issue 3 years ago • 5 comments

Hello,

Regarding the following part of the code: https://github.com/Casecommons/with_model/blob/481e96bfb4b84f5ba92e0206a6280ed3f5a6d257/lib/with_model/model.rb#L59

This call has a side effect on our project: all the models that inherit ActiveRecord::Base are not available in the ActiveRecord::Base.descendants list after the upgrade to Rails 7.

I temporary handle the side effect by overriding the following line: ActiveSupport::DescendantsTracker.clear([ActiveRecord::Base]) ActiveSupport::DescendantsTracker.clear([@model])

Is there a reason to clear all models related to ActiveRecord::Base and not only the current model?

Regards

kakuzei avatar Mar 08 '22 10:03 kakuzei

There's no reason. I think your idea is a good one.

nertzy avatar Mar 16 '22 22:03 nertzy

To give a little more context, we were running into issues years ago on an old project where with_model models would still exist in the descendants after a test was finished. We cleared the tracked descendants as a quick way to work around this problem.

nertzy avatar Mar 16 '22 22:03 nertzy

Thank you for you answer. How do you want to proceed to make this change available for everyone?

kakuzei avatar Mar 21 '22 08:03 kakuzei

👍 Just hit this issue upgrading from Rails 6.1 -> 7.0. For those Googling, the specific error message is: DescendantsTracker.clear was disabled because config.cache_classes = true.

cvincent avatar Apr 15 '22 13:04 cvincent

This is causing strange errors with STI.

image

I have one test that uses WithModel; after that test run, the next test that uses any STI will fail because one or more classes are not present on the descendants collection.

I was only able to spot this error because I turned on config.cache_classes

marcocarvalho avatar Aug 30 '22 22:08 marcocarvalho

Fixed in #44

nertzy avatar Feb 06 '23 00:02 nertzy

@nertzy can you release a new version? I'm running into this as well. I've patched my own app for now with this in a Rails initializer, but a new version would be amazing.

# frozen_string_literal: true

if defined?(WithModel)
  raise 'Remove this WithModel backport' unless WithModel::VERSION == '2.1.6'

  # https://github.com/Casecommons/with_model/issues/36
  # https://github.com/Casecommons/with_model/blob/5fe21fb109f97daed6ce5a2a450beda54f355c86/lib/with_model/model.rb#L56-L66
  WithModel::Model.define_method(:cleanup_descendants_tracking) do
    if defined?(ActiveSupport::DescendantsTracker)
      if ActiveSupport::VERSION::MAJOR >= 7
        ActiveSupport::DescendantsTracker.clear([@model])
      else
        ActiveSupport::DescendantsTracker.class_variable_get(:@@direct_descendants).delete(ActiveRecord::Base)
      end
    elsif @model.superclass.respond_to?(:direct_descendants)
      @model.superclass.direct_descendants.delete(@model)
    end
  end
end

natematykiewicz avatar Sep 13 '23 21:09 natematykiewicz

@natematykiewicz Thanks for the bump, I just released version 2.1.7 with this fix.

nertzy avatar Sep 15 '23 21:09 nertzy