Commit 9435942a authored by alexpott's avatar alexpott

Issue #2526326 by borisson_, Berdir, Wim Leers, Xano: Update CacheableMetadata...

Issue #2526326 by borisson_, Berdir, Wim Leers, Xano: Update CacheableMetadata & AccessResult to use RefinableCacheableDependency(Interface|Trait)
parent 9deeb7e9
......@@ -8,6 +8,8 @@
use Drupal\Core\Cache\Cache;
use Drupal\Core\Cache\CacheableDependencyInterface;
use Drupal\Core\Cache\RefinableCacheableDependencyInterface;
use Drupal\Core\Cache\RefinableCacheableDependencyTrait;
use Drupal\Core\Config\ConfigBase;
use Drupal\Core\Entity\EntityInterface;
use Drupal\Core\Session\AccountInterface;
......@@ -25,47 +27,10 @@
*
* When using ::orIf() and ::andIf(), cacheability metadata will be merged
* accordingly as well.
*
* @todo Use RefinableCacheableDependencyInterface and the corresponding trait in
* https://www.drupal.org/node/2526326.
*/
abstract class AccessResult implements AccessResultInterface, CacheableDependencyInterface {
/**
* The cache context IDs (to vary a cache item ID based on active contexts).
*
* @see \Drupal\Core\Cache\Context\CacheContextInterface
* @see \Drupal\Core\Cache\Context\CacheContextsManager::convertTokensToKeys()
*
* @var string[]
*/
protected $contexts;
/**
* The cache tags.
*
* @var array
*/
protected $tags;
/**
* The maximum caching time in seconds.
*
* @var int
*/
protected $maxAge;
abstract class AccessResult implements AccessResultInterface, RefinableCacheableDependencyInterface {
/**
* Constructs a new AccessResult object.
*/
public function __construct() {
$this->resetCacheContexts()
->resetCacheTags()
// Max-age must be non-zero for an access result to be cacheable.
// Typically, cache items are invalidated via associated cache tags, not
// via a maximum age.
->setCacheMaxAge(Cache::PERMANENT);
}
use RefinableCacheableDependencyTrait;
/**
* Creates an AccessResultInterface object with isNeutral() === TRUE.
......@@ -215,35 +180,21 @@ public function isNeutral() {
* {@inheritdoc}
*/
public function getCacheContexts() {
sort($this->contexts);
return $this->contexts;
return $this->cacheContexts;
}
/**
* {@inheritdoc}
*/
public function getCacheTags() {
return $this->tags;
return $this->cacheTags;
}
/**
* {@inheritdoc}
*/
public function getCacheMaxAge() {
return $this->maxAge;
}
/**
* Adds cache contexts associated with the access result.
*
* @param string[] $contexts
* An array of cache context IDs, used to generate a cache ID.
*
* @return $this
*/
public function addCacheContexts(array $contexts) {
$this->contexts = array_unique(array_merge($this->contexts, $contexts));
return $this;
return $this->cacheMaxAge;
}
/**
......@@ -252,20 +203,7 @@ public function addCacheContexts(array $contexts) {
* @return $this
*/
public function resetCacheContexts() {
$this->contexts = array();
return $this;
}
/**
* Adds cache tags associated with the access result.
*
* @param array $tags
* An array of cache tags.
*
* @return $this
*/
public function addCacheTags(array $tags) {
$this->tags = Cache::mergeTags($this->tags, $tags);
$this->cacheContexts = [];
return $this;
}
......@@ -275,7 +213,7 @@ public function addCacheTags(array $tags) {
* @return $this
*/
public function resetCacheTags() {
$this->tags = array();
$this->cacheTags = [];
return $this;
}
......@@ -288,7 +226,7 @@ public function resetCacheTags() {
* @return $this
*/
public function setCacheMaxAge($max_age) {
$this->maxAge = $max_age;
$this->cacheMaxAge = $max_age;
return $this;
}
......@@ -342,28 +280,6 @@ public function cacheUntilConfigurationChanges(ConfigBase $configuration) {
return $this->addCacheableDependency($configuration);
}
/**
* Adds a dependency on an object: merges its cacheability metadata.
*
* @param \Drupal\Core\Cache\CacheableDependencyInterface|object $other_object
* The dependency. If the object implements CacheableDependencyInterface,
* then its cacheability metadata will be used. Otherwise, the passed in
* object must be assumed to be uncacheable, so max-age 0 is set.
*
* @return $this
*/
public function addCacheableDependency($other_object) {
if ($other_object instanceof CacheableDependencyInterface) {
$this->contexts = Cache::mergeContexts($this->contexts, $other_object->getCacheContexts());
$this->tags = Cache::mergeTags($this->tags, $other_object->getCacheTags());
$this->maxAge = Cache::mergeMaxAges($this->maxAge, $other_object->getCacheMaxAge());
}
else {
$this->maxAge = 0;
}
return $this;
}
/**
* {@inheritdoc}
*/
......@@ -452,12 +368,19 @@ public function andIf(AccessResultInterface $other) {
/**
* Inherits the cacheability of the other access result, if any.
*
* inheritCacheability() differs from addCacheableDependency() in how it
* handles max-age, because it is designed to inherit the cacheability of the
* second operand in the andIf() and orIf() operations. There, the situation
* "allowed, max-age=0 OR allowed, max-age=1000" needs to yield max-age 1000
* as the end result.
*
* @param \Drupal\Core\Access\AccessResultInterface $other
* The other access result, whose cacheability (if any) to inherit.
*
* @return $this
*/
public function inheritCacheability(AccessResultInterface $other) {
$this->addCacheableDependency($other);
if ($other instanceof CacheableDependencyInterface) {
if ($this->getCacheMaxAge() !== 0 && $other->getCacheMaxAge() !== 0) {
$this->setCacheMaxAge(Cache::mergeMaxAges($this->getCacheMaxAge(), $other->getCacheMaxAge()));
......@@ -465,14 +388,6 @@ public function inheritCacheability(AccessResultInterface $other) {
else {
$this->setCacheMaxAge($other->getCacheMaxAge());
}
$this->addCacheContexts($other->getCacheContexts());
$this->addCacheTags($other->getCacheTags());
}
// If any of the access results don't provide cacheability metadata, then
// we cannot cache the combined access result, for we may not make
// assumptions.
else {
$this->setCacheMaxAge(0);
}
return $this;
}
......
......@@ -11,50 +11,16 @@
*
* @ingroup cache
*
* @todo Use RefinableCacheableDependencyInterface and the corresponding trait in
* https://www.drupal.org/node/2526326.
*/
class CacheableMetadata implements CacheableDependencyInterface {
class CacheableMetadata implements RefinableCacheableDependencyInterface {
/**
* Cache contexts.
*
* @var string[]
*/
protected $contexts = [];
/**
* Cache tags.
*
* @var string[]
*/
protected $tags = [];
/**
* Cache max-age.
*
* @var int
*/
protected $maxAge = Cache::PERMANENT;
use RefinableCacheableDependencyTrait;
/**
* {@inheritdoc}
*/
public function getCacheTags() {
return $this->tags;
}
/**
* Adds cache tags.
*
* @param string[] $cache_tags
* The cache tags to be added.
*
* @return $this
*/
public function addCacheTags(array $cache_tags) {
$this->tags = Cache::mergeTags($this->tags, $cache_tags);
return $this;
return $this->cacheTags;
}
/**
......@@ -66,7 +32,7 @@ public function addCacheTags(array $cache_tags) {
* @return $this
*/
public function setCacheTags(array $cache_tags) {
$this->tags = $cache_tags;
$this->cacheTags = $cache_tags;
return $this;
}
......@@ -74,20 +40,7 @@ public function setCacheTags(array $cache_tags) {
* {@inheritdoc}
*/
public function getCacheContexts() {
return $this->contexts;
}
/**
* Adds cache contexts.
*
* @param string[] $cache_contexts
* The cache contexts to be added.
*
* @return $this
*/
public function addCacheContexts(array $cache_contexts) {
$this->contexts = Cache::mergeContexts($this->contexts, $cache_contexts);
return $this;
return $this->cacheContexts;
}
/**
......@@ -99,7 +52,7 @@ public function addCacheContexts(array $cache_contexts) {
* @return $this
*/
public function setCacheContexts(array $cache_contexts) {
$this->contexts = $cache_contexts;
$this->cacheContexts = $cache_contexts;
return $this;
}
......@@ -107,7 +60,7 @@ public function setCacheContexts(array $cache_contexts) {
* {@inheritdoc}
*/
public function getCacheMaxAge() {
return $this->maxAge;
return $this->cacheMaxAge;
}
/**
......@@ -128,36 +81,7 @@ public function setCacheMaxAge($max_age) {
throw new \InvalidArgumentException('$max_age must be an integer');
}
$this->maxAge = $max_age;
return $this;
}
/**
* Adds a dependency on an object: merges its cacheability metadata.
*
* @param \Drupal\Core\Cache\CacheableDependencyInterface|mixed $other_object
* The dependency. If the object implements CacheableDependencyInterface,
* then its cacheability metadata will be used. Otherwise, the passed in
* object must be assumed to be uncacheable, so max-age 0 is set.
*
* @return $this
*/
public function addCacheableDependency($other_object) {
if ($other_object instanceof CacheableDependencyInterface) {
$this->addCacheTags($other_object->getCacheTags());
$this->addCacheContexts($other_object->getCacheContexts());
if ($this->maxAge === Cache::PERMANENT) {
$this->maxAge = $other_object->getCacheMaxAge();
}
elseif (($max_age = $other_object->getCacheMaxAge()) && $max_age !== Cache::PERMANENT) {
$this->maxAge = Cache::mergeMaxAges($this->maxAge, $max_age);
}
}
else {
// Not a cacheable dependency, this can not be cached.
$this->maxAge = 0;
}
$this->cacheMaxAge = $max_age;
return $this;
}
......@@ -175,34 +99,34 @@ public function merge(CacheableMetadata $other) {
// This is called many times per request, so avoid merging unless absolutely
// necessary.
if (empty($this->contexts)) {
$result->contexts = $other->contexts;
if (empty($this->cacheContexts)) {
$result->cacheContexts = $other->cacheContexts;
}
elseif (empty($other->contexts)) {
$result->contexts = $this->contexts;
elseif (empty($other->cacheContexts)) {
$result->cacheContexts = $this->cacheContexts;
}
else {
$result->contexts = Cache::mergeContexts($this->contexts, $other->contexts);
$result->cacheContexts = Cache::mergeContexts($this->cacheContexts, $other->cacheContexts);
}
if (empty($this->tags)) {
$result->tags = $other->tags;
if (empty($this->cacheTags)) {
$result->cacheTags = $other->cacheTags;
}
elseif (empty($other->tags)) {
$result->tags = $this->tags;
elseif (empty($other->cacheTags)) {
$result->cacheTags = $this->cacheTags;
}
else {
$result->tags = Cache::mergeTags($this->tags, $other->tags);
$result->cacheTags = Cache::mergeTags($this->cacheTags, $other->cacheTags);
}
if ($this->maxAge === Cache::PERMANENT) {
$result->maxAge = $other->maxAge;
if ($this->cacheMaxAge === Cache::PERMANENT) {
$result->cacheMaxAge = $other->cacheMaxAge;
}
elseif ($other->maxAge === Cache::PERMANENT) {
$result->maxAge = $this->maxAge;
elseif ($other->cacheMaxAge === Cache::PERMANENT) {
$result->cacheMaxAge = $this->cacheMaxAge;
}
else {
$result->maxAge = Cache::mergeMaxAges($this->maxAge, $other->maxAge);
$result->cacheMaxAge = Cache::mergeMaxAges($this->cacheMaxAge, $other->cacheMaxAge);
}
return $result;
}
......@@ -214,9 +138,9 @@ public function merge(CacheableMetadata $other) {
* A render array.
*/
public function applyTo(array &$build) {
$build['#cache']['contexts'] = $this->contexts;
$build['#cache']['tags'] = $this->tags;
$build['#cache']['max-age'] = $this->maxAge;
$build['#cache']['contexts'] = $this->cacheContexts;
$build['#cache']['tags'] = $this->cacheTags;
$build['#cache']['max-age'] = $this->cacheMaxAge;
}
/**
......@@ -229,9 +153,9 @@ public function applyTo(array &$build) {
*/
public static function createFromRenderArray(array $build) {
$meta = new static();
$meta->contexts = (isset($build['#cache']['contexts'])) ? $build['#cache']['contexts'] : [];
$meta->tags = (isset($build['#cache']['tags'])) ? $build['#cache']['tags'] : [];
$meta->maxAge = (isset($build['#cache']['max-age'])) ? $build['#cache']['max-age'] : Cache::PERMANENT;
$meta->cacheContexts = (isset($build['#cache']['contexts'])) ? $build['#cache']['contexts'] : [];
$meta->cacheTags = (isset($build['#cache']['tags'])) ? $build['#cache']['tags'] : [];
$meta->cacheMaxAge = (isset($build['#cache']['max-age'])) ? $build['#cache']['max-age'] : Cache::PERMANENT;
return $meta;
}
......@@ -249,16 +173,16 @@ public static function createFromRenderArray(array $build) {
public static function createFromObject($object) {
if ($object instanceof CacheableDependencyInterface) {
$meta = new static();
$meta->contexts = $object->getCacheContexts();
$meta->tags = $object->getCacheTags();
$meta->maxAge = $object->getCacheMaxAge();
$meta->cacheContexts = $object->getCacheContexts();
$meta->cacheTags = $object->getCacheTags();
$meta->cacheMaxAge = $object->getCacheMaxAge();
return $meta;
}
// Objects that don't implement CacheableDependencyInterface must be assumed
// to be uncacheable, so set max-age 0.
$meta = new static();
$meta->maxAge = 0;
$meta->cacheMaxAge = 0;
return $meta;
}
......
......@@ -44,7 +44,7 @@ public function addCacheableDependency($other_object) {
}
else {
// Not a cacheable dependency, this can not be cached.
$this->maxAge = 0;
$this->cacheMaxAge = 0;
}
return $this;
}
......@@ -53,7 +53,9 @@ public function addCacheableDependency($other_object) {
* {@inheritdoc}
*/
public function addCacheContexts(array $cache_contexts) {
$this->cacheContexts = Cache::mergeContexts($this->cacheContexts, $cache_contexts);
if ($cache_contexts) {
$this->cacheContexts = Cache::mergeContexts($this->cacheContexts, $cache_contexts);
}
return $this;
}
......@@ -61,7 +63,9 @@ public function addCacheContexts(array $cache_contexts) {
* {@inheritdoc}
*/
public function addCacheTags(array $cache_tags) {
$this->cacheTags = Cache::mergeTags($this->cacheTags, $cache_tags);
if ($cache_tags) {
$this->cacheTags = Cache::mergeTags($this->cacheTags, $cache_tags);
}
return $this;
}
......
......@@ -10,6 +10,7 @@
use Drupal\content_translation\Access\ContentTranslationManageAccessCheck;
use Drupal\Core\Access\AccessResult;
use Drupal\Core\DependencyInjection\ContainerBuilder;
use Drupal\Core\Cache\Cache;
use Drupal\Core\Language\Language;
use Drupal\Tests\UnitTestCase;
use Symfony\Component\Routing\Route;
......@@ -96,6 +97,12 @@ public function testCreateAccess() {
->method('getTranslationLanguages')
->with()
->will($this->returnValue(array()));
$entity->expects($this->once())
->method('getCacheContexts')
->willReturn([]);
$entity->expects($this->once())
->method('getCacheMaxAge')
->willReturn(Cache::PERMANENT);
$entity->expects($this->once())
->method('getCacheTags')
->will($this->returnValue(array('node:1337')));
......
......@@ -8,10 +8,10 @@
namespace Drupal\Tests\quickedit\Unit\Access;
use Drupal\Core\Access\AccessResult;
use Drupal\Core\Cache\Context\CacheContextsManager;
use Drupal\Core\DependencyInjection\Container;
use Drupal\quickedit\Access\EditEntityFieldAccessCheck;
use Drupal\Tests\UnitTestCase;
use Drupal\field\FieldStorageConfigInterface;
use Drupal\Core\Entity\EntityInterface;
use Drupal\Core\Language\LanguageInterface;
/**
......@@ -33,6 +33,11 @@ class EditEntityFieldAccessCheckTest extends UnitTestCase {
*/
protected function setUp() {
$this->editAccessCheck = new EditEntityFieldAccessCheck();
$cache_contexts_manager = $this->prophesize(CacheContextsManager::class)->reveal();
$container = new Container();
$container->set('cache_contexts_manager', $cache_contexts_manager);
\Drupal::setContainer($container);
}
/**
......@@ -41,34 +46,11 @@ protected function setUp() {
* @see \Drupal\Tests\edit\Unit\quickedit\Access\EditEntityFieldAccessCheckTest::testAccess()
*/
public function providerTestAccess() {
$editable_entity = $this->createMockEntity();
$editable_entity->expects($this->any())
->method('access')
->will($this->returnValue(AccessResult::allowed()->cachePerPermissions()));
$non_editable_entity = $this->createMockEntity();
$non_editable_entity->expects($this->any())
->method('access')
->will($this->returnValue(AccessResult::neutral()->cachePerPermissions()));
$field_storage_with_access = $this->getMockBuilder('Drupal\field\Entity\FieldStorageConfig')
->disableOriginalConstructor()
->getMock();
$field_storage_with_access->expects($this->any())
->method('access')
->will($this->returnValue(AccessResult::allowed()));
$field_storage_without_access = $this->getMockBuilder('Drupal\field\Entity\FieldStorageConfig')
->disableOriginalConstructor()
->getMock();
$field_storage_without_access->expects($this->any())
->method('access')
->will($this->returnValue(AccessResult::neutral()));
$data = array();
$data[] = array($editable_entity, $field_storage_with_access, AccessResult::allowed()->cachePerPermissions());
$data[] = array($non_editable_entity, $field_storage_with_access, AccessResult::neutral()->cachePerPermissions());
$data[] = array($editable_entity, $field_storage_without_access, AccessResult::neutral()->cachePerPermissions());
$data[] = array($non_editable_entity, $field_storage_without_access, AccessResult::neutral()->cachePerPermissions());
$data[] = array(TRUE, TRUE, AccessResult::allowed());
$data[] = array(FALSE, TRUE, AccessResult::neutral());
$data[] = array(TRUE, FALSE, AccessResult::neutral());
$data[] = array(FALSE, FALSE, AccessResult::neutral());
return $data;
}
......@@ -76,16 +58,28 @@ public function providerTestAccess() {
/**
* Tests the method for checking access to routes.
*
* @param \Drupal\Core\Entity\EntityInterface $entity
* A mocked entity.
* @param \Drupal\field\FieldStorageConfigInterface $field_storage
* A mocked field storage.
* @param bool|null $expected_result
* @param bool $entity_is_editable
* Whether the subject entity is editable.
* @param bool $field_storage_is_accessible
* Whether the user has access to the field storage entity.
* @param \Drupal\Core\Access\AccessResult $expected_result
* The expected result of the access call.
*
* @dataProvider providerTestAccess
*/
public function testAccess(EntityInterface $entity, FieldStorageConfigInterface $field_storage = NULL, $expected_result) {
public function testAccess($entity_is_editable, $field_storage_is_accessible, AccessResult $expected_result) {
$entity = $this->createMockEntity();
$entity->expects($this->any())
->method('access')
->willReturn(AccessResult::allowedIf($entity_is_editable)->cachePerPermissions());
$field_storage = $this->getMock('Drupal\field\FieldStorageConfigInterface');
$field_storage->expects($this->any())
->method('access')
->willReturn(AccessResult::allowedIf($field_storage_is_accessible));
$expected_result->cachePerPermissions();
$field_name = 'valid';
$entity_with_field = clone $entity;
$entity_with_field->expects($this->any())
......
......@@ -100,7 +100,10 @@ protected function setUp() {
*
* @dataProvider providerTestBuildCacheability
*/
public function testBuildCacheability($description, $tree, $expected_build) {
public function testBuildCacheability($description, $tree, $expected_build, $access, array $access_cache_contexts = []) {
if ($access !== NULL) {
$access->addCacheContexts($access_cache_contexts);
}
$build = $this->menuLinkTree->build($tree);
sort($expected_build['#cache']['contexts']);
$this->assertEquals($expected_build, $build, $description);
......@@ -175,13 +178,12 @@ public function providerTestBuildCacheability() {
'description' => 'Empty tree.',
'tree' => [],
'expected_build' => $base_expected_build_empty,
'access' => NULL,
'access_cache_contexts' => [],
];
for ($i = 0; $i < count($access_scenarios); $i++) {
list($access, $access_cache_contexts) = $access_scenarios[$i];
if ($access !== NULL) {
$access->addCacheContexts($access_cache_contexts);
}
for ($j = 0; $j < count($links_scenarios); $j++) {
$links = $links_scenarios[$j];
......@@ -203,6 +205,8 @@ public function providerTestBuildCacheability() {
'description' => "Single-item tree; access=$i; link=$j.",
'tree' => $tree,
'expected_build' => $expected_build,
'access' => $access,
'access_cache_contexts' => $access_cache_contexts,
];
// Single-level tree.
......@@ -221,6 +225,8 @@ public function providerTestBuildCacheability() {
'description' => "Single-level tree; access=$i; link=$j.",
'tree' => $tree,
'expected_build' => $expected_build,
'access' => $access,
'access_cache_contexts' => $access_cache_contexts,
];
// Multi-level tree.
......@@ -251,6 +257,8 @@ public function providerTestBuildCacheability() {
'description' => "Multi-level tree; access=$i; link=$j.",
'tree' => $tree,
'expected_build' => $expected_build,
'access' => $access,
'access_cache_contexts' => $access_cache_contexts,
];
}
}
......
......@@ -11,6 +11,8 @@
use Drupal\Tests\UnitTestCase;
use Drupal\user\Access\PermissionAccessCheck;
use Symfony\Component\Routing\Route;
use Drupal\Core\Cache\Context\CacheContextsManager;
use Symfony\Component\DependencyInjection\ContainerBuilder;
/**
* @coversDefaultClass \Drupal\user\Access\PermissionAccessCheck
......@@ -26,12 +28,24 @@ class PermissionAccessCheckTest extends UnitTestCase {
*/
public $accessCheck;
/**
* The dependency injection container.
*
* @var \Symfony\Component\DependencyInjection\ContainerBuilder
*/
protected $container;
/**
* {@inheritdoc}
*/
protected function setUp() {
parent::setUp();
$this->container = new ContainerBuilder();
$cache_contexts_manager = $this->prophesize(CacheContextsManager::class)->reveal();
$this->container->set('cache_contexts_manager', $cache_contexts_manager);
\Drupal::setContainer($this->container);
$this->accessCheck = new PermissionAccessCheck();
}
......@@ -41,15 +55,13 @@ protected function setUp() {
* @return array
*/
public function providerTestAccess() {
$allowed = AccessResult::allowedIf(TRUE)->addCacheContexts(['user.permissions']);
$neutral = AccessResult::allowedIf(FALSE)->addCacheContexts(['user.permissions']);
return [
[[], AccessResult::allowedIf(FALSE)],
[['_permission' => 'allowed'], $allowed],
[['_permission' => 'denied'], $neutral],
[['_permission' => 'allowed+denied'], $allowed],
[['_permission' => 'allowed+denied+other'], $allowed],
[['_permission' => 'allowed,denied'], $neutral],
[[], FALSE],
[['_permission' => 'allowed'], TRUE, ['user.permissions']],
[['_permission' => 'denied'], FALSE, ['user.permissions']],
[['_permission' => 'allowed+denied'], TRUE, ['user.permissions']],
[['_permission' => 'allowed+denied+other'], TRUE, ['user.permissions']],
[['_permission' => 'allowed,denied'], FALSE, ['user.permissions']],
];
}
......@@ -59,7 +71,8 @@ public function providerTestAccess() {
* @dataProvider providerTestAccess
* @covers ::access
*/
public function testAccess($requirements, $access) {
public function testAccess($requirements, $access, array $contexts = []) {
$access_result = AccessResult::allowedIf($access)->addCacheContexts($contexts);
$user = $this->getMock('Drupal\Core\Session\AccountInterface');
$user->expects($this->any())
->method('hasPermission')
......@@ -71,7 +84,7 @@ public function testAccess($requirements, $access) {
));
$route = new Route('', [], $requirements);