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
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
Comment #2
catchI think this would be a good idea.
Comment #3
danielvezaThis 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