Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Descendants tracking error when cache_classes = true #35

Open
mockdeep opened this issue Jan 23, 2022 · 14 comments
Open

Descendants tracking error when cache_classes = true #35

mockdeep opened this issue Jan 23, 2022 · 14 comments

Comments

@mockdeep
Copy link

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
Copy link

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

@maestromac
Copy link

Same here.

@waiting-for-dev
Copy link

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
Copy link

@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 added a commit to peterberkenbosch/solidus that referenced this issue Mar 28, 2022
@waiting-for-dev
Copy link

@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
Copy link

That's much clearer, thanks.

waiting-for-dev added a commit to peterberkenbosch/solidus that referenced this issue Mar 31, 2022
waiting-for-dev added a commit to peterberkenbosch/solidus that referenced this issue Apr 1, 2022
peterberkenbosch pushed a commit to peterberkenbosch/solidus that referenced this issue Apr 5, 2022
peterberkenbosch pushed a commit to peterberkenbosch/solidus that referenced this issue Apr 8, 2022
mamhoff pushed a commit to mamhoff/solidus that referenced this issue Apr 19, 2022
@waiting-for-dev
Copy link

@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.

waiting-for-dev added a commit to nebulab/solidus that referenced this issue Apr 22, 2022
Rails' config setting `cache_classes` was recently set to `false` in the
dummy app (see
solidusio@ef9b5ab).
That was due to a misunderstanding, as Rails recommends it be true
except when `spring` is present. From
https://guides.rubyonrails.org/configuring.html#config-cache-classes:

> Controls whether or not application classes and modules should be
> reloaded if they change. When the cache is enabled (true), reloading
> will not occur. Defaults to false in the development environment, and
> true in production. In the test environment, the default is false if
> Spring is installed, true otherwise.

That wasn't such a big deal until now. However, if we try to use the new
Ruby 3.1 in conjunction with `cache_classes` in the test environment,
many errors related to STI classes not being found pop out.

However, having the correct `false` value on Rails 7 raises an error
sourced on the `with_model` gem. See
Casecommons/with_model#35.

Because of that, we remove the `with_model` dependency so that our path
to support Ruby 3.1 is unblocked. We take different decisions to remove
the occurrences of `with_model` usage on Solidus:

- Tests on `Spree::Api::ResourceController`. That was only used on
`Spree::Api::UsersController`. Therefore, it was no longer a valid
abstraction:
  - We removed the tests altogether.
  - We moved the missing pieces to `UsersController`.
  - We removed the soft deletable specific test and added one to `UsersController`.
  - We marked `ResourceController` as deprecated (not removing it just in case some store uses it).

- `Spree::Admin::ResourceController`, `Spree::CalculatedAdjustements`,
`Spree::Validations::DbMaximumLengthValidtor` &
`Spree::WalletPaymentSource`: we use a custom logic for the same
behavior provided by `with_model`.

- `Spree::SoftDeletable`: we use `Spree::Product` as a fixture.
waiting-for-dev added a commit to nebulab/solidus that referenced this issue Apr 22, 2022
Rails' config setting `cache_classes` was recently set to `false` in the
dummy app (see
solidusio@ef9b5ab).
That was due to a misunderstanding, as Rails recommends it be true
except when `spring` is present. From
https://guides.rubyonrails.org/configuring.html#config-cache-classes:

> Controls whether or not application classes and modules should be
> reloaded if they change. When the cache is enabled (true), reloading
> will not occur. Defaults to false in the development environment, and
> true in production. In the test environment, the default is false if
> Spring is installed, true otherwise.

That wasn't such a big deal until now. However, if we try to use the new
Ruby 3.1 in conjunction with `cache_classes` in the test environment,
many errors related to STI classes not being found pop out.

However, having the correct `false` value on Rails 7 raises an error
sourced on the `with_model` gem. See
Casecommons/with_model#35.

Because of that, we remove the `with_model` dependency so that our path
to support Ruby 3.1 is unblocked. We take different decisions to remove
the occurrences of `with_model` usage on Solidus:

- Tests on `Spree::Api::ResourceController`. That was only used on
`Spree::Api::UsersController`. Therefore, it was no longer a valid
abstraction:
  - We removed the tests altogether.
  - We moved the missing pieces to `UsersController`.
  - We removed the soft deletable specific test and added one to `UsersController`.
  - We marked `ResourceController` as deprecated (not removing it just in case some store uses it).

- Tests on `Spree::Admin::ResourceController`, `Spree::CalculatedAdjustements`,
`Spree::Validations::DbMaximumLengthValidtor` &
`Spree::WalletPaymentSource`: we use a custom logic for the same
behavior provided by `with_model`.

- Tests on `Spree::SoftDeletable`: we use `Spree::Product` as a fixture.
waiting-for-dev added a commit to nebulab/solidus that referenced this issue Apr 22, 2022
Rails' config setting `cache_classes` was recently set to `false` in the
dummy app (see
solidusio@ef9b5ab).
That was due to a misunderstanding, as Rails recommends it be true
except when `spring` is present. From
https://guides.rubyonrails.org/configuring.html#config-cache-classes:

> Controls whether or not application classes and modules should be
> reloaded if they change. When the cache is enabled (true), reloading
> will not occur. Defaults to false in the development environment, and
> true in production. In the test environment, the default is false if
> Spring is installed, true otherwise.

That wasn't such a big deal until now. However, if we try to use the new
Ruby 3.1 in conjunction with `cache_classes` in the test environment,
many errors related to STI classes not being found pop out.

However, having the correct `false` value on Rails 7 raises an error
sourced on the `with_model` gem. See
Casecommons/with_model#35.

Because of that, we remove the `with_model` dependency so that our path
to support Ruby 3.1 is unblocked. We take different decisions to remove
the occurrences of `with_model` usage on Solidus:

- Tests on `Spree::Api::ResourceController`. That controller was only
used on `Spree::Api::UsersController`. Therefore, it was no longer a
valid abstraction:
  - We removed the tests altogether.
  - We moved the missing pieces to `UsersController`.
  - We removed the soft deletable specific test and added one to `UsersController`.
  - We marked `ResourceController` as deprecated (not removing it just in case some store uses it).

- Tests on `Spree::Admin::ResourceController`, `Spree::CalculatedAdjustements`,
`Spree::Validations::DbMaximumLengthValidtor` &
`Spree::WalletPaymentSource`: we use a custom logic for the same
behavior provided by `with_model`.

- Tests on `Spree::SoftDeletable`: we use `Spree::Product` as a fixture.
waiting-for-dev added a commit to nebulab/solidus that referenced this issue Apr 25, 2022
Rails' config setting `cache_classes` was recently set to `false` in the
dummy app (see
solidusio@ef9b5ab).
That was due to a misunderstanding, as Rails recommends it be true
except when `spring` is present. From
https://guides.rubyonrails.org/configuring.html#config-cache-classes:

> Controls whether or not application classes and modules should be
> reloaded if they change. When the cache is enabled (true), reloading
> will not occur. Defaults to false in the development environment, and
> true in production. In the test environment, the default is false if
> Spring is installed, true otherwise.

That wasn't such a big deal until now. However, if we try to use the new
Ruby 3.1 in conjunction with `cache_classes` in the test environment,
many errors related to STI classes not being found pop out.

However, having the correct `false` value on Rails 7 raises an error
sourced on the `with_model` gem. See
Casecommons/with_model#35.

Because of that, we remove the `with_model` dependency so that our path
to support Ruby 3.1 is unblocked. We take different decisions to remove
the occurrences of `with_model` usage on Solidus:

- Tests on `Spree::Api::ResourceController`. That controller was only
used on `Spree::Api::UsersController`. Therefore, it was no longer a
valid abstraction:
  - We removed the tests altogether.
  - We moved the missing pieces to `UsersController`.
  - We removed the soft deletable specific test and added one to `UsersController`.
  - We marked `ResourceController` as deprecated (not removing it just in case some store uses it).

- Tests on `Spree::Admin::ResourceController`, `Spree::CalculatedAdjustements`,
`Spree::Validations::DbMaximumLengthValidtor` &
`Spree::WalletPaymentSource`: we use a custom logic for the same
behavior provided by `with_model`.

- Tests on `Spree::SoftDeletable`: we use `Spree::Product` as a fixture.
waiting-for-dev added a commit to solidusio/solidus that referenced this issue Apr 25, 2022
Rails' config setting `cache_classes` was recently set to `false` in the
dummy app (see
ef9b5ab).
That was due to a misunderstanding, as Rails recommends it be true
except when `spring` is present. From
https://guides.rubyonrails.org/configuring.html#config-cache-classes:

> Controls whether or not application classes and modules should be
> reloaded if they change. When the cache is enabled (true), reloading
> will not occur. Defaults to false in the development environment, and
> true in production. In the test environment, the default is false if
> Spring is installed, true otherwise.

That wasn't such a big deal until now. However, if we try to use the new
Ruby 3.1 in conjunction with `cache_classes` in the test environment,
many errors related to STI classes not being found pop out.

However, having the correct `false` value on Rails 7 raises an error
sourced on the `with_model` gem. See
Casecommons/with_model#35.

Because of that, we remove the `with_model` dependency so that our path
to support Ruby 3.1 is unblocked. We take different decisions to remove
the occurrences of `with_model` usage on Solidus:

- Tests on `Spree::Api::ResourceController`. That controller was only
used on `Spree::Api::UsersController`. Therefore, it was no longer a
valid abstraction:
  - We removed the tests altogether.
  - We moved the missing pieces to `UsersController`.
  - We removed the soft deletable specific test and added one to `UsersController`.
  - We marked `ResourceController` as deprecated (not removing it just in case some store uses it).

- Tests on `Spree::Admin::ResourceController`, `Spree::CalculatedAdjustements`,
`Spree::Validations::DbMaximumLengthValidtor` &
`Spree::WalletPaymentSource`: we use a custom logic for the same
behavior provided by `with_model`.

- Tests on `Spree::SoftDeletable`: we use `Spree::Product` as a fixture.
waiting-for-dev added a commit to solidusio/solidus that referenced this issue Apr 25, 2022
Rails' config setting `cache_classes` was recently set to `false` in the
dummy app (see
ef9b5ab).
That was due to a misunderstanding, as Rails recommends it be true
except when `spring` is present. From
https://guides.rubyonrails.org/configuring.html#config-cache-classes:

> Controls whether or not application classes and modules should be
> reloaded if they change. When the cache is enabled (true), reloading
> will not occur. Defaults to false in the development environment, and
> true in production. In the test environment, the default is false if
> Spring is installed, true otherwise.

That wasn't such a big deal until now. However, if we try to use the new
Ruby 3.1 in conjunction with `cache_classes` in the test environment,
many errors related to STI classes not being found pop out.

However, having the correct `false` value on Rails 7 raises an error
sourced on the `with_model` gem. See
Casecommons/with_model#35.

Because of that, we remove the `with_model` dependency so that our path
to support Ruby 3.1 is unblocked. We take different decisions to remove
the occurrences of `with_model` usage on Solidus:

- Tests on `Spree::Api::ResourceController`. That controller was only
used on `Spree::Api::UsersController`. Therefore, it was no longer a
valid abstraction:
  - We removed the tests altogether.
  - We moved the missing pieces to `UsersController`.
  - We removed the soft deletable specific test and added one to `UsersController`.
  - We marked `ResourceController` as deprecated (not removing it just in case some store uses it).

- Tests on `Spree::Admin::ResourceController`, `Spree::CalculatedAdjustements`,
`Spree::Validations::DbMaximumLengthValidtor` &
`Spree::WalletPaymentSource`: we use a custom logic for the same
behavior provided by `with_model`.

- Tests on `Spree::SoftDeletable`: we use `Spree::Product` as a fixture.
@Benoit-Baumann
Copy link

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
Copy link

stex commented May 6, 2022

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
Copy link

Thanks for the workaround @stex !

rmparr pushed a commit to rmparr/solidus that referenced this issue Jun 1, 2022
rmparr pushed a commit to rmparr/solidus that referenced this issue Jun 1, 2022
Rails' config setting `cache_classes` was recently set to `false` in the
dummy app (see
solidusio@ef9b5ab).
That was due to a misunderstanding, as Rails recommends it be true
except when `spring` is present. From
https://guides.rubyonrails.org/configuring.html#config-cache-classes:

> Controls whether or not application classes and modules should be
> reloaded if they change. When the cache is enabled (true), reloading
> will not occur. Defaults to false in the development environment, and
> true in production. In the test environment, the default is false if
> Spring is installed, true otherwise.

That wasn't such a big deal until now. However, if we try to use the new
Ruby 3.1 in conjunction with `cache_classes` in the test environment,
many errors related to STI classes not being found pop out.

However, having the correct `false` value on Rails 7 raises an error
sourced on the `with_model` gem. See
Casecommons/with_model#35.

Because of that, we remove the `with_model` dependency so that our path
to support Ruby 3.1 is unblocked. We take different decisions to remove
the occurrences of `with_model` usage on Solidus:

- Tests on `Spree::Api::ResourceController`. That controller was only
used on `Spree::Api::UsersController`. Therefore, it was no longer a
valid abstraction:
  - We removed the tests altogether.
  - We moved the missing pieces to `UsersController`.
  - We removed the soft deletable specific test and added one to `UsersController`.
  - We marked `ResourceController` as deprecated (not removing it just in case some store uses it).

- Tests on `Spree::Admin::ResourceController`, `Spree::CalculatedAdjustements`,
`Spree::Validations::DbMaximumLengthValidtor` &
`Spree::WalletPaymentSource`: we use a custom logic for the same
behavior provided by `with_model`.

- Tests on `Spree::SoftDeletable`: we use `Spree::Product` as a fixture.
rmparr pushed a commit to rmparr/solidus that referenced this issue Jun 1, 2022
rmparr pushed a commit to rmparr/solidus that referenced this issue Jun 1, 2022
Rails' config setting `cache_classes` was recently set to `false` in the
dummy app (see
solidusio@ef9b5ab).
That was due to a misunderstanding, as Rails recommends it be true
except when `spring` is present. From
https://guides.rubyonrails.org/configuring.html#config-cache-classes:

> Controls whether or not application classes and modules should be
> reloaded if they change. When the cache is enabled (true), reloading
> will not occur. Defaults to false in the development environment, and
> true in production. In the test environment, the default is false if
> Spring is installed, true otherwise.

That wasn't such a big deal until now. However, if we try to use the new
Ruby 3.1 in conjunction with `cache_classes` in the test environment,
many errors related to STI classes not being found pop out.

However, having the correct `false` value on Rails 7 raises an error
sourced on the `with_model` gem. See
Casecommons/with_model#35.

Because of that, we remove the `with_model` dependency so that our path
to support Ruby 3.1 is unblocked. We take different decisions to remove
the occurrences of `with_model` usage on Solidus:

- Tests on `Spree::Api::ResourceController`. That controller was only
used on `Spree::Api::UsersController`. Therefore, it was no longer a
valid abstraction:
  - We removed the tests altogether.
  - We moved the missing pieces to `UsersController`.
  - We removed the soft deletable specific test and added one to `UsersController`.
  - We marked `ResourceController` as deprecated (not removing it just in case some store uses it).

- Tests on `Spree::Admin::ResourceController`, `Spree::CalculatedAdjustements`,
`Spree::Validations::DbMaximumLengthValidtor` &
`Spree::WalletPaymentSource`: we use a custom logic for the same
behavior provided by `with_model`.

- Tests on `Spree::SoftDeletable`: we use `Spree::Product` as a fixture.
cpfergus1 pushed a commit to cpfergus1/solidus that referenced this issue Aug 25, 2022
cpfergus1 pushed a commit to cpfergus1/solidus that referenced this issue Aug 25, 2022
Rails' config setting `cache_classes` was recently set to `false` in the
dummy app (see
solidusio@ef9b5ab).
That was due to a misunderstanding, as Rails recommends it be true
except when `spring` is present. From
https://guides.rubyonrails.org/configuring.html#config-cache-classes:

> Controls whether or not application classes and modules should be
> reloaded if they change. When the cache is enabled (true), reloading
> will not occur. Defaults to false in the development environment, and
> true in production. In the test environment, the default is false if
> Spring is installed, true otherwise.

That wasn't such a big deal until now. However, if we try to use the new
Ruby 3.1 in conjunction with `cache_classes` in the test environment,
many errors related to STI classes not being found pop out.

However, having the correct `false` value on Rails 7 raises an error
sourced on the `with_model` gem. See
Casecommons/with_model#35.

Because of that, we remove the `with_model` dependency so that our path
to support Ruby 3.1 is unblocked. We take different decisions to remove
the occurrences of `with_model` usage on Solidus:

- Tests on `Spree::Api::ResourceController`. That controller was only
used on `Spree::Api::UsersController`. Therefore, it was no longer a
valid abstraction:
  - We removed the tests altogether.
  - We moved the missing pieces to `UsersController`.
  - We removed the soft deletable specific test and added one to `UsersController`.
  - We marked `ResourceController` as deprecated (not removing it just in case some store uses it).

- Tests on `Spree::Admin::ResourceController`, `Spree::CalculatedAdjustements`,
`Spree::Validations::DbMaximumLengthValidtor` &
`Spree::WalletPaymentSource`: we use a custom logic for the same
behavior provided by `with_model`.

- Tests on `Spree::SoftDeletable`: we use `Spree::Product` as a fixture.
@nertzy
Copy link
Collaborator

nertzy commented Feb 6, 2023

Does #44 fix this issue?

@Benoit-Baumann
Copy link

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

waiting-for-dev added a commit to solidusio/solidus_graphql_api that referenced this issue Aug 10, 2023
with_model is not compatible with Rails 7 [1]. We remove the dependency
and use custom logic in the specs to create tables and models.

[1] - Casecommons/with_model#35

[skip ci]
waiting-for-dev added a commit to solidusio/solidus_graphql_api that referenced this issue Aug 11, 2023
with_model is not compatible with Rails 7 [1]. We remove the dependency
and use custom logic in the specs to create tables and models.

[1] - Casecommons/with_model#35

[skip ci]
waiting-for-dev added a commit to solidusio/solidus_graphql_api that referenced this issue Aug 14, 2023
with_model is not compatible with Rails 7 [1]. We remove the dependency
and use custom logic in the specs to create tables and models.

[1] - Casecommons/with_model#35

[skip ci]
waiting-for-dev added a commit to solidusio/solidus_graphql_api that referenced this issue Aug 15, 2023
with_model is not compatible with Rails 7 [1]. We remove the dependency
and use custom logic in the specs to create tables and models.

We need to skip tests on MySQL because of https://api.rubyonrails.org/classes/ActiveRecord/Transactions/ClassMethods.html#caveats:

> If you're on MySQL, then do not use Data Definition Language (DDL)
> operations in nested transactions blocks that are emulated with
> savepoints. That is, do not execute statements like 'CREATE TABLE'
> inside such blocks. This is because MySQL automatically releases all
> savepoints upon executing a DDL operation. When transaction is finished
> and tries to release the savepoint it created earlier, a database error
> will occur because the savepoint has already been automatically
> released.

[1] - Casecommons/with_model#35

[skip ci]
@swishstache
Copy link

@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: rails/rails@cb82f5f

@markedmondson
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants