Problem/Motivation

The logic of RefinableCacheableDependencyTrait::addCacheableDependency() has been updated in #3232018: Trigger a deprecation when using \Drupal\Core\Cache\RefinableCacheableDependencyTrait::addCacheableDependency with a non CacheableDependencyInterface object.

If the passed object is NOT an instance of CacheableDependencyInterface
Previous behavior: max-age is set to 0
Future behavior: an exception is thrown

In order to get rid of the deprecation notice and preserve the old behavior, one bit of LayoutBuilderAccessCheck::access() code was changed from

$access->addCacheableDependency($section_storage);

to

if ($section_storage instanceof CacheableDependencyInterface) {
  $access->addCacheableDependency($section_storage);
}
else {
  $access->setCacheMaxAge(0);
}

However, it was noticed that SimpleConfigSectionStorage is the only class that does not implement CacheableDependencyInterface.

Maybe SimpleConfigSectionStorage should be extended to implement CacheableDependencyInterface? Then the if condition could be removed.

Steps to reproduce

Proposed resolution

Remaining tasks

- Make a decision
- Update the code?

User interface changes

None

API changes

None

Data model changes

None

Release notes snippet

Issue fork drupal-3446509

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

Leksat created an issue. See original summary.

catch’s picture

I think this would be a good idea.

danielveza’s picture

Assigned: Unassigned » danielveza

This should only apply to SectionStorage plugins that DO NOT extend SectionStorageBase and instead implement the SectionStorageInterface.

I searched through Drupal contrib projects and can't find any examples. I agree that we should add CacheableDependencyInterface to SimpleConfigSectionStorage and remove the if. I'll start on that and see how tests look

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.