Issue #3232018 by leksat, prudloff, acbramley, smustgrave, larowlan: Trigger a deprecation when using \Drupal\Core\Cache\RefinableCacheableDependencyTrait::addCacheableDependency with a non CacheableDependencyInterface object

8 files changed, 68 insertions, 9 deletions

diff --git a/core/lib/Drupal/Core/Access/AccessResult.php b/core/lib/Drupal/Core/Access/AccessResult.php
index 386aba4f09f..fe3a4ad52aa 100644
--- a/core/lib/Drupal/Core/Access/AccessResult.php
+++ b/core/lib/Drupal/Core/Access/AccessResult.php
@@ -406,8 +406,8 @@ abstract class AccessResult implements AccessResultInterface, RefinableCacheable
    * @return $this
    */
   public function inheritCacheability(AccessResultInterface $other) {
-    $this->addCacheableDependency($other);
     if ($other instanceof CacheableDependencyInterface) {
+      $this->addCacheableDependency($other);
       if ($this->getCacheMaxAge() !== 0 && $other->getCacheMaxAge() !== 0) {
         $this->setCacheMaxAge(Cache::mergeMaxAges($this->getCacheMaxAge(), $other->getCacheMaxAge()));
       }
@@ -415,6 +415,9 @@ abstract class AccessResult implements AccessResultInterface, RefinableCacheable
         $this->setCacheMaxAge($other->getCacheMaxAge());
       }
     }
+    else {
+      $this->setCacheMaxAge(0);
+    }
     return $this;
   }
 
diff --git a/core/lib/Drupal/Core/Cache/RefinableCacheableDependencyTrait.php b/core/lib/Drupal/Core/Cache/RefinableCacheableDependencyTrait.php
index fcbc11f8d1e..697d8d6c0cc 100644
--- a/core/lib/Drupal/Core/Cache/RefinableCacheableDependencyTrait.php
+++ b/core/lib/Drupal/Core/Cache/RefinableCacheableDependencyTrait.php
@@ -20,6 +20,7 @@ trait RefinableCacheableDependencyTrait {
     }
     else {
       // Not a cacheable dependency, this can not be cached.
+      @trigger_error(sprintf("Calling %s() with an object that doesn't implement %s is deprecated in drupal:11.2.0 and is required in drupal:12.0.0. See https://www.drupal.org/node/3232020", __METHOD__, CacheableDependencyInterface::class), E_USER_DEPRECATED);
       $this->cacheMaxAge = 0;
     }
     return $this;
diff --git a/core/modules/field_ui/src/Access/FieldReuseAccessCheck.php b/core/modules/field_ui/src/Access/FieldReuseAccessCheck.php
index db69c94a690..ecf71d563ba 100644
--- a/core/modules/field_ui/src/Access/FieldReuseAccessCheck.php
+++ b/core/modules/field_ui/src/Access/FieldReuseAccessCheck.php
@@ -4,6 +4,7 @@ namespace Drupal\field_ui\Access;
 
 use Drupal\Core\Access\AccessResult;
 use Drupal\Core\Access\AccessResultInterface;
+use Drupal\Core\Cache\CacheableDependencyInterface;
 use Drupal\Core\Entity\EntityFieldManagerInterface;
 use Drupal\Core\Entity\EntityTypeManagerInterface;
 use Drupal\Core\Field\FieldTypePluginManagerInterface;
@@ -81,7 +82,14 @@ class FieldReuseAccessCheck implements AccessInterface {
           $access = $access->orIf(AccessResult::allowedIfHasPermission($account, $permission));
         }
       }
-      $access->addCacheableDependency($this->entityFieldManager);
+      // @todo https://www.drupal.org/project/drupal/issues/3446507 Decide if
+      // this logic needs to be changed or removed.
+      if ($this->entityFieldManager instanceof CacheableDependencyInterface) {
+        $access->addCacheableDependency($this->entityFieldManager);
+      }
+      else {
+        $access->setCacheMaxAge(0);
+      }
     }
     return $access;
   }
diff --git a/core/modules/jsonapi/src/Normalizer/HttpExceptionNormalizer.php b/core/modules/jsonapi/src/Normalizer/HttpExceptionNormalizer.php
index 0a72e69f23f..1f983174845 100644
--- a/core/modules/jsonapi/src/Normalizer/HttpExceptionNormalizer.php
+++ b/core/modules/jsonapi/src/Normalizer/HttpExceptionNormalizer.php
@@ -2,6 +2,7 @@
 
 namespace Drupal\jsonapi\Normalizer;
 
+use Drupal\Core\Cache\CacheableDependencyInterface;
 use Drupal\Core\Cache\CacheableMetadata;
 use Drupal\Core\Session\AccountInterface;
 use Drupal\jsonapi\Normalizer\Value\HttpExceptionNormalizerValue;
@@ -43,7 +44,12 @@ class HttpExceptionNormalizer extends NormalizerBase {
    */
   public function normalize($object, $format = NULL, array $context = []): array|string|int|float|bool|\ArrayObject|NULL {
     $cacheability = new CacheableMetadata();
-    $cacheability->addCacheableDependency($object);
+    if ($object instanceof CacheableDependencyInterface) {
+      $cacheability->addCacheableDependency($object);
+    }
+    else {
+      $cacheability->setCacheMaxAge(0);
+    }
 
     $cacheability->addCacheTags(['config:system.logging']);
     if (\Drupal::config('system.logging')->get('error_level') === ERROR_REPORTING_DISPLAY_VERBOSE) {
diff --git a/core/modules/layout_builder/src/Access/LayoutBuilderAccessCheck.php b/core/modules/layout_builder/src/Access/LayoutBuilderAccessCheck.php
index 671265499a7..cf104f5840d 100644
--- a/core/modules/layout_builder/src/Access/LayoutBuilderAccessCheck.php
+++ b/core/modules/layout_builder/src/Access/LayoutBuilderAccessCheck.php
@@ -3,6 +3,7 @@
 namespace Drupal\layout_builder\Access;
 
 use Drupal\Core\Access\AccessResult;
+use Drupal\Core\Cache\CacheableDependencyInterface;
 use Drupal\Core\Cache\RefinableCacheableDependencyInterface;
 use Drupal\Core\Routing\Access\AccessInterface;
 use Drupal\Core\Session\AccountInterface;
@@ -43,7 +44,14 @@ class LayoutBuilderAccessCheck implements AccessInterface {
     }
 
     if ($access instanceof RefinableCacheableDependencyInterface) {
-      $access->addCacheableDependency($section_storage);
+      // @todo https://www.drupal.org/project/drupal/issues/3446509 Decide if
+      // this logic needs to be changed.
+      if ($section_storage instanceof CacheableDependencyInterface) {
+        $access->addCacheableDependency($section_storage);
+      }
+      else {
+        $access->setCacheMaxAge(0);
+      }
     }
     return $access;
   }
diff --git a/core/tests/Drupal/Tests/Core/Access/AccessResultTest.php b/core/tests/Drupal/Tests/Core/Access/AccessResultTest.php
index 829f5a2a7a3..e44218405bb 100644
--- a/core/tests/Drupal/Tests/Core/Access/AccessResultTest.php
+++ b/core/tests/Drupal/Tests/Core/Access/AccessResultTest.php
@@ -523,10 +523,6 @@ class AccessResultTest extends UnitTestCase {
     $verify($a, $tags);
     $b = AccessResult::neutral()->addCacheableDependency($node);
     $verify($b, $tags, ['user'], 600);
-
-    $non_cacheable_dependency = new \stdClass();
-    $non_cacheable = AccessResult::neutral()->addCacheableDependency($non_cacheable_dependency);
-    $verify($non_cacheable, [], [], 0);
   }
 
   /**
diff --git a/core/tests/Drupal/Tests/Core/Cache/RefinableCacheableDependencyTraitTest.php b/core/tests/Drupal/Tests/Core/Cache/RefinableCacheableDependencyTraitTest.php
new file mode 100644
index 00000000000..3ee009f6a10
--- /dev/null
+++ b/core/tests/Drupal/Tests/Core/Cache/RefinableCacheableDependencyTraitTest.php
@@ -0,0 +1,28 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Drupal\Tests\Core\Cache;
+
+use Drupal\Core\Cache\RefinableCacheableDependencyInterface;
+use Drupal\Core\Cache\RefinableCacheableDependencyTrait;
+use Drupal\Tests\UnitTestCase;
+
+/**
+ * Tests the cache RefinableCacheableDependencyTrait.
+ *
+ * @group Cache
+ */
+class RefinableCacheableDependencyTraitTest extends UnitTestCase implements RefinableCacheableDependencyInterface {
+
+  use RefinableCacheableDependencyTrait;
+
+  /**
+   * @group legacy
+   */
+  public function testNonCacheableDependencyAddDeprecation(): void {
+    $this->expectDeprecation("Calling Drupal\Core\Cache\RefinableCacheableDependencyTrait::addCacheableDependency() with an object that doesn't implement Drupal\Core\Cache\CacheableDependencyInterface is deprecated in drupal:11.2.0 and is required in drupal:12.0.0. See https://www.drupal.org/node/3232020");
+    $this->addCacheableDependency(new \stdClass());
+  }
+
+}
diff --git a/core/tests/Drupal/Tests/Core/Menu/LocalActionManagerTest.php b/core/tests/Drupal/Tests/Core/Menu/LocalActionManagerTest.php
index 973b69f159f..08fa2eceaf5 100644
--- a/core/tests/Drupal/Tests/Core/Menu/LocalActionManagerTest.php
+++ b/core/tests/Drupal/Tests/Core/Menu/LocalActionManagerTest.php
@@ -162,7 +162,16 @@ class LocalActionManagerTest extends UnitTestCase {
       ->willReturn($plugin_definitions);
     $map = [];
     foreach ($plugin_definitions as $plugin_id => $plugin_definition) {
-      $plugin = $this->createMock('Drupal\Core\Menu\LocalActionInterface');
+      $plugin = $this->createMock('Drupal\Core\Menu\LocalActionDefault');
+      $plugin->expects($this->any())
+        ->method('getCacheContexts')
+        ->willReturn([]);
+      $plugin->expects($this->any())
+        ->method('getCacheTags')
+        ->willReturn([]);
+      $plugin->expects($this->any())
+        ->method('getCacheMaxAge')
+        ->willReturn(0);
       $plugin->expects($this->any())
         ->method('getRouteName')
         ->willReturn($plugin_definition['route_name']);