Commit 72b0180f authored by catch's avatar catch

Issue #2454649 by Aki Tendo, dawehner: Cache Optimization and hardening --...

Issue #2454649 by Aki Tendo, dawehner: Cache Optimization and hardening -- [PP-1] Use assert() instead of exceptions in Cache::merge(Tags|Contexts)
parent 8d196a79
......@@ -166,7 +166,7 @@ protected function prepareItem($cache, $allow_invalid) {
* {@inheritdoc}
*/
public function set($cid, $data, $expire = CacheBackendInterface::CACHE_PERMANENT, array $tags = array()) {
Cache::validateTags($tags);
assert('\Drupal\Component\Assertion\Inspector::assertAllStrings($tags)', 'Cache tags must be strings.');
$tags = array_unique($tags);
$cache = new \stdClass();
$cache->cid = $cid;
......
......@@ -34,7 +34,7 @@ class Cache {
*/
public static function mergeContexts(array $a = [], array $b = []) {
$cache_contexts = array_unique(array_merge($a, $b));
\Drupal::service('cache_contexts_manager')->validateTokens($cache_contexts);
assert('\Drupal::service(\'cache_contexts_manager\')->assertValidTokens($cache_contexts)');
sort($cache_contexts);
return $cache_contexts;
}
......@@ -59,8 +59,9 @@ public static function mergeContexts(array $a = [], array $b = []) {
* The merged array of cache tags.
*/
public static function mergeTags(array $a = [], array $b = []) {
assert('\Drupal\Component\Assertion\Inspector::assertAllStrings($a) && \Drupal\Component\Assertion\Inspector::assertAllStrings($b)', 'Cache tags must be valid strings');
$cache_tags = array_unique(array_merge($a, $b));
static::validateTags($cache_tags);
sort($cache_tags);
return $cache_tags;
}
......@@ -99,6 +100,9 @@ public static function mergeMaxAges($a = Cache::PERMANENT, $b = Cache::PERMANENT
* @param string[] $tags
* An array of cache tags.
*
* @deprecated
* Use assert('\Drupal\Component\Assertion\Inspector::assertAllStrings($tags)');
*
* @throws \LogicException
*/
public static function validateTags(array $tags) {
......
......@@ -115,7 +115,7 @@
* (optional) The tags to specify for the cache item.
*/
public function __construct($cid, CacheBackendInterface $cache, LockBackendInterface $lock, array $tags = array()) {
Cache::validateTags($tags);
assert('\Drupal\Component\Assertion\Inspector::assertAllStrings($tags)', 'Cache tags must be strings.');
$this->cid = $cid;
$this->cache = $cache;
$this->tags = $tags;
......
......@@ -27,8 +27,7 @@ class CacheTagsInvalidator implements CacheTagsInvalidatorInterface {
* {@inheritdoc}
*/
public function invalidateTags(array $tags) {
// Validate the tags.
Cache::validateTags($tags);
assert('Drupal\Component\Assertion\Inspector::assertAllStrings($tags)', 'Cache tags must be strings.');
// Notify all added cache tags invalidators.
foreach ($this->invalidators as $invalidator) {
......
......@@ -103,11 +103,9 @@ public function getLabels($include_calculated_cache_contexts = FALSE) {
* The ContextCacheKeys object containing the converted cache keys and
* cacheability metadata.
*
* @throws \LogicException
* Thrown if any of the context tokens or parameters are not valid.
*/
public function convertTokensToKeys(array $context_tokens) {
$this->validateTokens($context_tokens);
assert('$this->assertValidTokens($context_tokens)');
$cacheable_metadata = new CacheableMetadata();
$optimized_tokens = $this->optimizeTokens($context_tokens);
// Iterate over cache contexts that have been optimized away and get their
......@@ -299,4 +297,33 @@ public function validateTokens(array $context_tokens = []) {
}
}
/**
* Asserts the context tokens are valid
*
* Similar to ::validateTokens, this method returns boolean TRUE when the
* context tokens are valid, and FALSE when they are not instead of returning
* NULL when they are valid and throwing a \LogicException when they are not.
* This function should be used with the assert() statement.
*
* @param mixed $context_tokens
* Variable to be examined - should be array of context_tokens.
*
* @return bool
* TRUE if context_tokens is an array of valid tokens.
*/
public function assertValidTokens($context_tokens) {
if (!is_array($context_tokens)) {
return FALSE;
}
try {
$this->validateTokens($context_tokens);
}
catch (\LogicException $e) {
return FALSE;
}
return TRUE;
}
}
......@@ -199,7 +199,7 @@ protected function doSetMultiple(array $items) {
'tags' => array(),
);
Cache::validateTags($item['tags']);
assert('\Drupal\Component\Assertion\Inspector::assertAllStrings($item[\'tags\'])', 'Cache Tags must be strings.');
$item['tags'] = array_unique($item['tags']);
// Sort the cache tags so that they are stored consistently in the DB.
sort($item['tags']);
......
......@@ -107,7 +107,7 @@ protected function prepareItem($cache, $allow_invalid) {
* Implements Drupal\Core\Cache\CacheBackendInterface::set().
*/
public function set($cid, $data, $expire = Cache::PERMANENT, array $tags = array()) {
Cache::validateTags($tags);
assert('\Drupal\Component\Assertion\Inspector::assertAllStrings($tags)', 'Cache Tags must be strings.');
$tags = array_unique($tags);
// Sort the cache tags so that they are stored consistently in the database.
sort($tags);
......
......@@ -148,7 +148,7 @@ protected function prepareItem($cache, $allow_invalid) {
* {@inheritdoc}
*/
public function set($cid, $data, $expire = Cache::PERMANENT, array $tags = array()) {
Cache::validateTags($tags);
assert('\Drupal\Component\Assertion\Inspector::assertAllStrings($tags)', 'Cache Tags must be strings.');
$item = (object) array(
'cid' => $cid,
'data' => $data,
......
......@@ -154,7 +154,7 @@ public function __construct($subdir, \Traversable $namespaces, ModuleHandlerInte
* definitions should be cleared along with other, related cache entries.
*/
public function setCacheBackend(CacheBackendInterface $cache_backend, $cache_key, array $cache_tags = array()) {
Cache::validateTags($cache_tags);
assert('\Drupal\Component\Assertion\Inspector::assertAllStrings($cache_tags)', 'Cache Tags must be strings.');
$this->cacheBackend = $cache_backend;
$this->cacheKey = $cache_key;
$this->cacheTags = $cache_tags;
......
......@@ -57,9 +57,8 @@ public function setUpDisplayVariant($configuration = array(), $definition = arra
->getMock();
$container->set('cache_contexts_manager', $cache_context_manager);
$cache_context_manager->expects($this->any())
->method('validateTokens')
->with([])
->willReturn([]);
->method('assertValidTokens')
->willReturn(TRUE);
\Drupal::setContainer($container);
$this->blockRepository = $this->getMock('Drupal\block\BlockRepositoryInterface');
......
......@@ -40,6 +40,7 @@ protected function setUp() {
$this->cacheContextsManager = $this->getMockBuilder('Drupal\Core\Cache\Context\CacheContextsManager')
->disableOriginalConstructor()
->getMock();
$this->cacheContextsManager->method('assertValidTokens')->willReturn(TRUE);
$container = new ContainerBuilder();
$container->set('cache_contexts_manager', $this->cacheContextsManager);
......
......@@ -27,8 +27,7 @@ public function setUp() {
$cache_contexts_manager = $this->getMockBuilder('Drupal\Core\Cache\Context\CacheContextsManager')
->disableOriginalConstructor()
->getMock();
$cache_contexts_manager->expects($this->any())
->method('validate_tokens');
$cache_contexts_manager->method('assertValidTokens')->willReturn(TRUE);
$container = new Container();
$container->set('cache_contexts_manager', $cache_contexts_manager);
\Drupal::setContainer($container);
......
......@@ -28,8 +28,7 @@ public function setUp() {
$cache_contexts_manager = $this->getMockBuilder('Drupal\Core\Cache\Context\CacheContextsManager')
->disableOriginalConstructor()
->getMock();
$cache_contexts_manager->expects($this->any())
->method('validate_tokens');
$cache_contexts_manager->method('assertValidTokens')->willReturn(TRUE);
$container = new Container();
$container->set('cache_contexts_manager', $cache_contexts_manager);
\Drupal::setContainer($container);
......
......@@ -27,8 +27,7 @@ public function setUp() {
$cache_contexts_manager = $this->getMockBuilder('Drupal\Core\Cache\Context\CacheContextsManager')
->disableOriginalConstructor()
->getMock();
$cache_contexts_manager->expects($this->any())
->method('validate_tokens');
$cache_contexts_manager->method('assertValidTokens')->willReturn(TRUE);
$container = new Container();
$container->set('cache_contexts_manager', $cache_contexts_manager);
\Drupal::setContainer($container);
......
......@@ -60,6 +60,7 @@ protected function setUp() {
$cache_contexts_manager = $this->getMockBuilder('Drupal\Core\Cache\Context\CacheContextsManager')
->disableOriginalConstructor()
->getMock();
$cache_contexts_manager->method('assertValidTokens')->willReturn(TRUE);
$container = new ContainerBuilder();
$container->set('cache_contexts_manager', $cache_contexts_manager);
\Drupal::setContainer($container);
......
......@@ -34,7 +34,9 @@ class EditEntityFieldAccessCheckTest extends UnitTestCase {
protected function setUp() {
$this->editAccessCheck = new EditEntityFieldAccessCheck();
$cache_contexts_manager = $this->prophesize(CacheContextsManager::class)->reveal();
$cache_contexts_manager = $this->prophesize(CacheContextsManager::class);
$cache_contexts_manager->assertValidTokens()->willReturn(TRUE);
$cache_contexts_manager->reveal();
$container = new Container();
$container->set('cache_contexts_manager', $cache_contexts_manager);
\Drupal::setContainer($container);
......
......@@ -969,6 +969,10 @@ public function run(array $methods = array()) {
$this->httpAuthCredentials = $username . ':' . $password;
}
// Force assertion failures to be thrown as AssertionError for PHP 5 & 7
// compatibility.
\Drupal\Component\Assertion\Handle::register();
set_error_handler(array($this, 'errorHandler'));
// Iterate through all the methods in this class, unless a specific list of
// methods to run was passed.
......
......@@ -218,13 +218,13 @@ public function testSetGet() {
$this->assertEqual('value', $backend->get('TEST8')->data);
$this->assertFalse($backend->get('test8'));
// Calling ::set() with invalid cache tags.
// Calling ::set() with invalid cache tags. This should fail an assertion.
try {
$backend->set('exception_test', 'value', Cache::PERMANENT, ['node' => [3, 5, 7]]);
$this->fail('::set() was called with invalid cache tags, no exception was thrown.');
$backend->set('assertion_test', 'value', Cache::PERMANENT, ['node' => [3, 5, 7]]);
$this->fail('::set() was called with invalid cache tags, runtime assertion did not fail.');
}
catch (\LogicException $e) {
$this->pass('::set() was called with invalid cache tags, an exception was thrown.');
catch (\AssertionError $e) {
$this->pass('::set() was called with invalid cache tags, runtime assertion failed.');
}
}
......@@ -412,7 +412,8 @@ public function testSetMultiple() {
$this->assertEqual($cached['cid_5']->data, $items['cid_5']['data'], 'New cache item set correctly.');
// Calling ::setMultiple() with invalid cache tags.
// Calling ::setMultiple() with invalid cache tags. This should fail an
// assertion.
try {
$items = [
'exception_test_1' => array('data' => 1, 'tags' => []),
......@@ -420,10 +421,10 @@ public function testSetMultiple() {
'exception_test_3' => array('data' => 3, 'tags' => ['node' => [3, 5, 7]]),
];
$backend->setMultiple($items);
$this->fail('::setMultiple() was called with invalid cache tags, no exception was thrown.');
$this->fail('::setMultiple() was called with invalid cache tags, runtime assertion did not fail.');
}
catch (\LogicException $e) {
$this->pass('::setMultiple() was called with invalid cache tags, an exception was thrown.');
catch (\AssertionError $e) {
$this->pass('::setMultiple() was called with invalid cache tags, runtime assertion failed.');
}
}
......
......@@ -124,6 +124,7 @@ protected function setUp() {
$cache_contexts_manager = $this->getMockBuilder('Drupal\Core\Cache\Context\CacheContextsManager')
->disableOriginalConstructor()
->getMock();
$cache_contexts_manager->method('assertValidTokens')->willReturn(TRUE);
$cache_contexts_manager->expects($this->any())
->method('validate_tokens');
$container = new Container();
......
......@@ -47,6 +47,7 @@ protected function setUp() {
$cache_contexts_manager = $this->getMockBuilder('Drupal\Core\Cache\Context\CacheContextsManager')
->disableOriginalConstructor()
->getMock();
$cache_contexts_manager->method('assertValidTokens')->willReturn(TRUE);
$container = new ContainerBuilder();
$container->set('cache_contexts_manager', $cache_contexts_manager);
\Drupal::setContainer($container);
......
......@@ -42,7 +42,9 @@ protected function setUp() {
parent::setUp();
$this->container = new ContainerBuilder();
$cache_contexts_manager = $this->prophesize(CacheContextsManager::class)->reveal();
$cache_contexts_manager = $this->prophesize(CacheContextsManager::class);
$cache_contexts_manager->assertValidTokens()->willReturn(TRUE);
$cache_contexts_manager->reveal();
$this->container->set('cache_contexts_manager', $cache_contexts_manager);
\Drupal::setContainer($this->container);
......
......@@ -64,7 +64,9 @@ class UserAccessControlHandlerTest extends UnitTestCase {
public function setUp() {
parent::setUp();
$cache_contexts_manager = $this->prophesize(CacheContextsManager::class)->reveal();
$cache_contexts_manager = $this->prophesize(CacheContextsManager::class);
$cache_contexts_manager->assertValidTokens()->willReturn(TRUE);
$cache_contexts_manager->reveal();
$container = new Container();
$container->set('cache_contexts_manager', $cache_contexts_manager);
\Drupal::setContainer($container);
......
......@@ -31,12 +31,36 @@ public function testAssertTraversable() {
* Tests asserting all members are strings.
*
* @covers ::assertAllStrings
* @dataProvider providerTestAssertAllStrings
*/
public function testAssertAllStrings() {
$this->assertTrue(Inspector::assertAllStrings([]));
$this->assertTrue(Inspector::assertAllStrings(['foo', 'bar']));
$this->assertFalse(Inspector::assertAllStrings('foo'));
$this->assertFalse(Inspector::assertAllStrings(['foo', new StringObject()]));
public function testAssertAllStrings($input, $expected) {
$this->assertSame($expected, Inspector::assertAllStrings($input));
}
public function providerTestAssertAllStrings() {
$data = [
'empty-array' => [[], TRUE],
'array-with-strings' => [['foo', 'bar'], TRUE],
'string' => ['foo', FALSE],
'array-with-strings-with-colon' => [['foo', 'bar', 'llama:2001988', 'baz', 'llama:14031991'], TRUE],
'with-FALSE' => [[FALSE], FALSE],
'with-TRUE' => [[TRUE], FALSE],
'with-string-and-boolean' => [['foo', FALSE], FALSE],
'with-NULL' => [[NULL], FALSE],
'string-with-NULL' => [['foo', NULL], FALSE],
'integer' => [[1337], FALSE],
'string-and-integer' => [['foo', 1337], FALSE],
'double' => [[3.14], FALSE],
'string-and-double' => [['foo', 3.14], FALSE],
'array' => [[[]], FALSE],
'string-and-array' => [['foo', []], FALSE],
'string-and-nested-array' => [['foo', ['bar']], FALSE],
'object' => [[new \stdClass()], FALSE],
'string-and-object' => [['foo', new StringObject()], FALSE],
];
return $data;
}
/**
......
......@@ -38,6 +38,7 @@ protected function setUp() {
->disableOriginalConstructor()
->getMock();
$this->cacheContextsManager->method('assertValidTokens')->willReturn(TRUE);
$container = new ContainerBuilder();
$container->set('cache_contexts_manager', $this->cacheContextsManager);
\Drupal::setContainer($container);
......
......@@ -57,7 +57,9 @@ protected function setUp() {
$this->breadcrumb = new Breadcrumb();
$this->container = new ContainerBuilder();
$cache_contexts_manager = $this->prophesize(CacheContextsManager::class)->reveal();
$cache_contexts_manager = $this->prophesize(CacheContextsManager::class);
$cache_contexts_manager->assertValidTokens()->willReturn(TRUE);
$cache_contexts_manager->reveal();
$this->container->set('cache_contexts_manager', $cache_contexts_manager);
\Drupal::setContainer($this->container);
}
......
......@@ -20,8 +20,7 @@ class CacheTagsInvalidatorTest extends UnitTestCase {
/**
* @covers ::invalidateTags
*
* @expectedException \LogicException
* @expectedExceptionMessage Cache tags must be strings, array given.
* @expectedException \AssertionError
*/
public function testInvalidateTagsWithInvalidTags() {
$cache_tags_invalidator = new CacheTagsInvalidator();
......
......@@ -7,6 +7,7 @@
namespace Drupal\Tests\Core\Cache;
use Drupal\Component\Assertion\Inspector;
use Drupal\Core\Cache\Cache;
use Drupal\Tests\UnitTestCase;
use Symfony\Component\DependencyInjection\ContainerBuilder;
......@@ -59,6 +60,7 @@ public function testValidateTags(array $tags, $expected_exception_message) {
$this->assertNull(Cache::validateTags($tags));
}
/**
* Provides a list of pairs of cache tags arrays to be merged.
*
......
......@@ -35,6 +35,8 @@ public function testMerge(CacheableMetadata $a, CacheableMetadata $b, CacheableM
$cache_contexts_manager = $this->getMockBuilder('Drupal\Core\Cache\Context\CacheContextsManager')
->disableOriginalConstructor()
->getMock();
$cache_contexts_manager->method('assertValidTokens')->willReturn(TRUE);
$container = new ContainerBuilder();
$container->set('cache_contexts_manager', $cache_contexts_manager);
\Drupal::setContainer($container);
......@@ -57,6 +59,7 @@ public function testAddCacheableDependency(CacheableMetadata $a, CacheableMetada
$cache_contexts_manager = $this->getMockBuilder('Drupal\Core\Cache\Context\CacheContextsManager')
->disableOriginalConstructor()
->getMock();
$cache_contexts_manager->method('assertValidTokens')->willReturn(TRUE);
$container = new ContainerBuilder();
$container->set('cache_contexts_manager', $cache_contexts_manager);
\Drupal::setContainer($container);
......
......@@ -106,8 +106,7 @@ public function testConvertTokensToKeys() {
/**
* @covers ::convertTokensToKeys
*
* @expectedException \LogicException
* @expectedExceptionMessage "non-cache-context" is not a valid cache context ID.
* @expectedException \AssertionError
*/
public function testInvalidContext() {
$container = $this->getMockContainer();
......
......@@ -35,7 +35,9 @@ class EntityCreateAccessCheckTest extends UnitTestCase {
protected function setUp() {
parent::setUp();
$cache_contexts_manager = $this->prophesize(CacheContextsManager::class)->reveal();
$cache_contexts_manager = $this->prophesize(CacheContextsManager::class);
$cache_contexts_manager->assertValidTokens()->willReturn(TRUE);
$cache_contexts_manager->reveal();
$container = new Container();
$container->set('cache_contexts_manager', $cache_contexts_manager);
\Drupal::setContainer($container);
......
......@@ -518,6 +518,8 @@ public function testCacheContexts() {
$cache_contexts_manager = $this->getMockBuilder('Drupal\Core\Cache\Context\CacheContextsManager')
->disableOriginalConstructor()
->getMock();
$cache_contexts_manager->method('assertValidTokens')->willReturn(TRUE);
$container = new ContainerBuilder();
$container->set('cache_contexts_manager', $cache_contexts_manager);
\Drupal::setContainer($container);
......
......@@ -81,7 +81,10 @@ protected function setUp() {
$this->defaultMenuTreeManipulators = new DefaultMenuLinkTreeManipulators($this->accessManager, $this->currentUser, $this->queryFactory);
$cache_contexts_manager = $this->prophesize(CacheContextsManager::class)->reveal();
$cache_contexts_manager = $this->prophesize(CacheContextsManager::class);
$cache_contexts_manager->assertValidTokens()->willReturn(TRUE);
$cache_contexts_manager->reveal();
$container = new Container();
$container->set('cache_contexts_manager', $cache_contexts_manager);
\Drupal::setContainer($container);
......
......@@ -488,6 +488,10 @@ protected function setupNullCacheabilityMetadataValidation() {
$cache_context_manager = $this->prophesize(CacheContextsManager::class);
foreach ([NULL, ['user.permissions'], ['route'], ['route', 'context.example1'], ['context.example1', 'route'], ['context.example1', 'route', 'context.example2'], ['context.example1', 'context.example2', 'route'], ['context.example1', 'context.example2', 'route', 'user.permissions']] as $argument) {
$cache_context_manager->assertValidTokens($argument)->willReturn(TRUE);
}
$container->set('cache_contexts_manager', $cache_context_manager->reveal());
\Drupal::setContainer($container);
}
......
......@@ -60,6 +60,7 @@ public function testMerge(BubbleableMetadata $a, CacheableMetadata $b, Bubbleabl
$cache_contexts_manager = $this->getMockBuilder('Drupal\Core\Cache\Context\CacheContextsManager')
->disableOriginalConstructor()
->getMock();
$cache_contexts_manager->method('assertValidTokens')->willReturn(TRUE);
$container = new ContainerBuilder();
$container->set('cache_contexts_manager', $cache_contexts_manager);
$container->set('renderer', $renderer);
......@@ -664,6 +665,7 @@ public function testAddCacheableDependency(BubbleableMetadata $a, $b, Bubbleable
$cache_contexts_manager = $this->getMockBuilder('Drupal\Core\Cache\Context\CacheContextsManager')
->disableOriginalConstructor()
->getMock();
$cache_contexts_manager->method('assertValidTokens')->willReturn(TRUE);
$container = new ContainerBuilder();
$container->set('cache_contexts_manager', $cache_contexts_manager);
\Drupal::setContainer($container);
......
......@@ -144,6 +144,7 @@ protected function setUp() {
$this->cacheContextsManager = $this->getMockBuilder('Drupal\Core\Cache\Context\CacheContextsManager')
->disableOriginalConstructor()
->getMock();
$this->cacheContextsManager->method('assertValidTokens')->willReturn(TRUE);
$current_user_role = &$this->currentUserRole;
$this->cacheContextsManager->expects($this->any())
->method('convertTokensToKeys')
......
......@@ -145,7 +145,9 @@ public function roleAccessProvider() {
* @dataProvider roleAccessProvider
*/
public function testRoleAccess($path, $grant_accounts, $deny_accounts) {
$cache_contexts_manager = $this->prophesize(CacheContextsManager::class)->reveal();
$cache_contexts_manager = $this->prophesize(CacheContextsManager::class);
$cache_contexts_manager->assertValidTokens()->willReturn(TRUE);
$cache_contexts_manager->reveal();
$container = new Container();
$container->set('cache_contexts_manager', $cache_contexts_manager);
\Drupal::setContainer($container);
......
......@@ -63,6 +63,7 @@ protected function setUp() {
$cache_contexts_manager = $this->getMockBuilder('Drupal\Core\Cache\Context\CacheContextsManager')
->disableOriginalConstructor()
->getMock();
$cache_contexts_manager->method('assertValidTokens')->willReturn(TRUE);
$container = new ContainerBuilder();
$container->set('cache_contexts_manager', $cache_contexts_manager);
\Drupal::setContainer($container);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment