Casecommons / with_model

Dynamically build an Active Record model (with table) within a test context
http://www.casebook.net
MIT License
167 stars 18 forks source link

Descendants tracking error when `cache_classes = true` #35

Open mockdeep opened 2 years ago

mockdeep commented 2 years ago

We're getting an error when we have cache_classes = true on CI with Rails 7.0.1:

 RuntimeError:
              DescendantsTracker.clear was disabled because config.cache_classes = true
            # /home/circleci/app/vendor/bundle/ruby/2.7.0/gems/activesupport-7.0.1/lib/active_support/descendants_tracker.rb:119:in `clear'
            # /home/circleci/app/vendor/bundle/ruby/2.7.0/gems/with_model-2.1.6/lib/with_model/model.rb:59:in `cleanup_descendants_tracking'
            # /home/circleci/app/vendor/bundle/ruby/2.7.0/gems/with_model-2.1.6/lib/with_model/model.rb:38:in `destroy'
            # /home/circleci/app/vendor/bundle/ruby/2.7.0/gems/with_model-2.1.6/lib/with_model.rb:53:in `block in setup_object'
airblade commented 2 years ago

Me too. config.cache_classes = true is the default in Rails 7.

maestromac commented 2 years ago

Same here.

waiting-for-dev commented 2 years ago

See https://guides.rubyonrails.org/upgrading_ruby_on_rails.html#upgrading-from-rails-6-1-to-rails-7-0-spring. It needs to be explicitly set to false from Rails 7 when Spring is not used.

airblade commented 2 years ago

@waiting-for-dev I read that the opposite way, i.e. it needs to be set to false in Rails 7 when Spring is used.

waiting-for-dev commented 2 years ago

@waiting-for-dev I read that the opposite way, i.e. it needs to be set to false in Rails 7 when Spring is used.

Yeah, that wording is confusing. See here instead https://guides.rubyonrails.org/configuring.html#config-cache-classes

airblade commented 2 years ago

That's much clearer, thanks.

waiting-for-dev commented 2 years ago

@airblade, in the end, I think you're right. It should be true in the test environment, and it should only be false when Spring is present, as it'll handle cache by itself. So, yeah, I think this should be fixed from with_model end.

Benoit-Baumann commented 2 years ago

I have the same error with Rails 7.0.2 and the new default config.cache_classes = true.

Any chance to have this issue fixed @nertzy ?

stex commented 2 years ago

It doesn't fix the actual issue, but I added a local monkey-patch to get with_model running with Rails 7, maybe it's of use to someone else.

# spec/support/with_model.rb

module WithModel
  class Model
    # Workaround for https://github.com/Casecommons/with_model/issues/35
    def cleanup_descendants_tracking
      if defined?(ActiveSupport::DescendantsTracker) && !Rails.application.config.cache_classes
        ActiveSupport::DescendantsTracker.clear([@model])
      elsif @model.superclass.respond_to?(:direct_descendants)
        @model.superclass.subclasses.delete(@model)
      end
    end
  end
end
Benoit-Baumann commented 2 years ago

Thanks for the workaround @Stex !

nertzy commented 1 year ago

Does #44 fix this issue?

Benoit-Baumann commented 1 year ago

Unfortunately no, I still get the same error... Stex's workaround works like a charm, though.

swishstache commented 8 months ago

@nertzy is there a chance of some variant of @stex 's fix above being incorporated into a future release? I'm not sure what he means by "... doesn't fix the actual issue... ". It would appear calling ActiveSupport::DescendantsTracker.clear under Rails >= 7 with config.cache_classes set to true (the default for the test environment) will no longer work. What else could we do other than delete from subclasses?

For those looking, here's the commit to 7.0.0 that is the root of this: https://github.com/rails/rails/commit/cb82f5f0a438c7be64ca1c0633556f6a535138b6

markedmondson commented 7 months ago

Hey @nertzy if you're happy to add me as an owner on Rubygems I can get this out.