From fd0dcf94100a16b11f7e535c10f8b31e107ec150 Mon Sep 17 00:00:00 2001 From: Nathaniel Catchpole Date: Fri, 9 May 2014 14:57:55 +0100 Subject: [PATCH] Issue #2256877 by znerol: Remove the API function to check if a cache bin is empty. --- core/lib/Drupal/Core/Cache/BackendChain.php | 13 ---- .../Core/Cache/CacheBackendInterface.php | 11 --- .../lib/Drupal/Core/Cache/DatabaseBackend.php | 18 ----- core/lib/Drupal/Core/Cache/MemoryBackend.php | 7 -- core/lib/Drupal/Core/Cache/NullBackend.php | 7 -- core/lib/Drupal/Core/Cache/PhpBackend.php | 8 -- .../Drupal/filter/Tests/FilterAdminTest.php | 73 ++++++++++++++++++- .../Cache/GenericCacheBackendUnitTestBase.php | 23 ++---- core/modules/toolbar/toolbar.module | 15 ++-- .../BackendChainImplementationUnitTest.php | 23 ++---- .../Tests/Core/Cache/NullBackendTest.php | 1 - 11 files changed, 89 insertions(+), 110 deletions(-) diff --git a/core/lib/Drupal/Core/Cache/BackendChain.php b/core/lib/Drupal/Core/Cache/BackendChain.php index 2ea11187e87..0cea14251d2 100644 --- a/core/lib/Drupal/Core/Cache/BackendChain.php +++ b/core/lib/Drupal/Core/Cache/BackendChain.php @@ -221,19 +221,6 @@ class BackendChain implements CacheBackendInterface { } } - /** - * Implements Drupal\Core\Cache\CacheBackendInterface::isEmpty(). - */ - public function isEmpty() { - foreach ($this->backends as $backend) { - if (!$backend->isEmpty()) { - return FALSE; - } - } - - return TRUE; - } - /** * {@inheritdoc} */ diff --git a/core/lib/Drupal/Core/Cache/CacheBackendInterface.php b/core/lib/Drupal/Core/Cache/CacheBackendInterface.php index f6049805f5e..796573ced34 100644 --- a/core/lib/Drupal/Core/Cache/CacheBackendInterface.php +++ b/core/lib/Drupal/Core/Cache/CacheBackendInterface.php @@ -257,15 +257,4 @@ interface CacheBackendInterface { * Remove a cache bin. */ public function removeBin(); - - /** - * Checks if a cache bin is empty. - * - * A cache bin is considered empty if it does not contain any valid data for - * any cache ID. - * - * @return - * TRUE if the cache bin specified is empty. - */ - public function isEmpty(); } diff --git a/core/lib/Drupal/Core/Cache/DatabaseBackend.php b/core/lib/Drupal/Core/Cache/DatabaseBackend.php index 3ae9eacf90d..3f15f43358f 100644 --- a/core/lib/Drupal/Core/Cache/DatabaseBackend.php +++ b/core/lib/Drupal/Core/Cache/DatabaseBackend.php @@ -493,24 +493,6 @@ class DatabaseBackend implements CacheBackendInterface { return $checksum; } - /** - * Implements Drupal\Core\Cache\CacheBackendInterface::isEmpty(). - */ - public function isEmpty() { - $this->garbageCollection(); - $query = $this->connection->select($this->bin); - $query->addExpression('1'); - try { - $result = $query->range(0, 1) - ->execute() - ->fetchField(); - } - catch (\Exception $e) { - $this->catchException($e); - } - return empty($result); - } - /** * {@inheritdoc} */ diff --git a/core/lib/Drupal/Core/Cache/MemoryBackend.php b/core/lib/Drupal/Core/Cache/MemoryBackend.php index ff65a34b24c..8cd759cebf2 100644 --- a/core/lib/Drupal/Core/Cache/MemoryBackend.php +++ b/core/lib/Drupal/Core/Cache/MemoryBackend.php @@ -215,13 +215,6 @@ class MemoryBackend implements CacheBackendInterface { return $flat_tags; } - /** - * Implements Drupal\Core\Cache\CacheBackendInterface::isEmpty(). - */ - public function isEmpty() { - return empty($this->cache); - } - /** * Implements Drupal\Core\Cache\CacheBackendInterface::garbageCollection() */ diff --git a/core/lib/Drupal/Core/Cache/NullBackend.php b/core/lib/Drupal/Core/Cache/NullBackend.php index 2d5664d3274..d27bc13f73f 100644 --- a/core/lib/Drupal/Core/Cache/NullBackend.php +++ b/core/lib/Drupal/Core/Cache/NullBackend.php @@ -99,13 +99,6 @@ class NullBackend implements CacheBackendInterface { */ public function garbageCollection() {} - /** - * Implements Drupal\Core\Cache\CacheBackendInterface::isEmpty(). - */ - public function isEmpty() { - return TRUE; - } - /** * {@inheritdoc} */ diff --git a/core/lib/Drupal/Core/Cache/PhpBackend.php b/core/lib/Drupal/Core/Cache/PhpBackend.php index 23ed324e6c2..646b2c9029b 100644 --- a/core/lib/Drupal/Core/Cache/PhpBackend.php +++ b/core/lib/Drupal/Core/Cache/PhpBackend.php @@ -231,14 +231,6 @@ class PhpBackend implements CacheBackendInterface { return $flat_tags; } - /** - * {@inheritdoc} - */ - public function isEmpty() { - $names = $this->storage()->listAll(); - return empty($names); - } - /** * {@inheritdoc} */ diff --git a/core/modules/filter/lib/Drupal/filter/Tests/FilterAdminTest.php b/core/modules/filter/lib/Drupal/filter/Tests/FilterAdminTest.php index 38ef433c896..d7869f75f74 100644 --- a/core/modules/filter/lib/Drupal/filter/Tests/FilterAdminTest.php +++ b/core/modules/filter/lib/Drupal/filter/Tests/FilterAdminTest.php @@ -8,6 +8,7 @@ namespace Drupal\filter\Tests; use Drupal\simpletest\WebTestBase; +use Drupal\filter\Plugin\FilterInterface; /** * Tests the administrative functionality of the Filter module. @@ -154,8 +155,6 @@ class FilterAdminTest extends WebTestBase { $this->drupalGet('admin/config/content/formats/manage/' . $restricted); $this->assertFieldByName('filters[filter_html][settings][allowed_html]', $edit['filters[filter_html][settings][allowed_html]'], 'Allowed HTML tag added.'); - $this->assertTrue(\Drupal::cache('filter')->isEmpty(), 'Cache cleared.'); - $elements = $this->xpath('//select[@name=:first]/following::select[@name=:second]', array( ':first' => 'filters[' . $first_filter . '][weight]', ':second' => 'filters[' . $second_filter . '][weight]', @@ -302,4 +301,74 @@ class FilterAdminTest extends WebTestBase { $this->drupalPostForm('admin/config/content/formats/manage/basic_html', $edit, t('Save configuration')); $this->assertNoRaw(t('The text format %format has been updated.', array('%format' => 'Basic HTML'))); } + + /** + * Tests that changing filter properties clears the filter cache. + */ + public function testFilterAdminClearsFilterCache() { + $restricted = 'restricted_html'; + $original_markup = '

Small headers

small headers are allowed in restricted html by default'; + + // Check that the filter cache is empty for the test markup. + $cid = $this->computeFilterCacheId($original_markup, $restricted, '', TRUE); + $this->assertFalse(\Drupal::cache('filter')->get($cid)); + + // Check that the filter cache gets populated when check_markup is called. + $actual_markup = check_markup($original_markup, $restricted, '', TRUE); + $this->assertTrue(\Drupal::cache('filter')->get($cid)); + $this->assertIdentical(strpos($actual_markup, '

'), 0, 'The h4 tag is present in the resulting markup'); + + // Edit the restricted filter format. + $edit = array(); + $edit['filters[filter_html][settings][allowed_html]'] = ' '; + $this->drupalPostForm('admin/config/content/formats/manage/' . $restricted, $edit, t('Save configuration')); + $this->assertUrl('admin/config/content/formats'); + $this->drupalGet('admin/config/content/formats/manage/' . $restricted); + $this->assertFieldByName('filters[filter_html][settings][allowed_html]', $edit['filters[filter_html][settings][allowed_html]'], 'Allowed HTML tag added.'); + + // Check that the filter cache is empty after the format was changed. + $this->assertFalse(\Drupal::cache('filter')->get($cid)); + + // Check that after changind the filter, the changes are reflected in the + // filtered markup. + $actual_markup = check_markup($original_markup, $restricted, '', TRUE); + $this->assertIdentical(strpos($actual_markup, '

'), FALSE, 'The h4 tag is not present in the resulting markup'); + } + + + /** + * Computes the cache-key for the given text just like check_markup(). + * + * Note that this is copied over from check_markup(). + * + * @return string|NULL + * The cache-key used to store the text in the filter cache. + */ + protected function computeFilterCacheId($text, $format_id = NULL, $langcode = '', $cache = FALSE, $filter_types_to_skip = array()) { + if (!isset($format_id)) { + $format_id = filter_fallback_format(); + } + // If the requested text format does not exist, the text cannot be filtered. + if (!$format = entity_load('filter_format', $format_id)) { + return; + } + + // Prevent FilterInterface::TYPE_HTML_RESTRICTOR from being skipped. + if (in_array(FilterInterface::TYPE_HTML_RESTRICTOR, $filter_types_to_skip)) { + $filter_types_to_skip = array_diff($filter_types_to_skip, array(FilterInterface::TYPE_HTML_RESTRICTOR)); + } + + // When certain filters should be skipped, don't perform caching. + if ($filter_types_to_skip) { + $cache = FALSE; + } + + // Compute the cache key if the text is cacheable. + $cache = $cache && !empty($format->cache); + $cache_id = ''; + if ($cache) { + return $format->format . ':' . $langcode . ':' . hash('sha256', $text); + } + } + } diff --git a/core/modules/system/lib/Drupal/system/Tests/Cache/GenericCacheBackendUnitTestBase.php b/core/modules/system/lib/Drupal/system/Tests/Cache/GenericCacheBackendUnitTestBase.php index b390429bd26..bd89e2d6ae8 100644 --- a/core/modules/system/lib/Drupal/system/Tests/Cache/GenericCacheBackendUnitTestBase.php +++ b/core/modules/system/lib/Drupal/system/Tests/Cache/GenericCacheBackendUnitTestBase.php @@ -334,21 +334,6 @@ abstract class GenericCacheBackendUnitTestBase extends DrupalUnitTestBase { $this->assertEqual($cached['cid_5']->tags, array('test:a', 'test:b')); } - /** - * Tests Drupal\Core\Cache\CacheBackendInterface::isEmpty(). - */ - public function testIsEmpty() { - $backend = $this->getCacheBackend(); - - $this->assertTrue($backend->isEmpty(), "Backend is empty."); - - $backend->set('pony', "Shetland"); - $this->assertFalse($backend->isEmpty(), "Backend is not empty."); - - $backend->delete('pony'); - $this->assertTrue($backend->isEmpty(), "Backend is empty."); - } - /** * Test Drupal\Core\Cache\CacheBackendInterface::delete() and * Drupal\Core\Cache\CacheBackendInterface::deleteMultiple(). @@ -456,17 +441,18 @@ abstract class GenericCacheBackendUnitTestBase extends DrupalUnitTestBase { */ public function testDeleteAll() { $backend = $this->getCacheBackend(); + $unrelated = $this->getCacheBackend('bootstrap'); // Set both expiring and permanent keys. $backend->set('test1', 1, Cache::PERMANENT); $backend->set('test2', 3, time() + 1000); + $unrelated->set('test3', 4, Cache::PERMANENT); $backend->deleteAll(); - $this->assertTrue($backend->isEmpty(), "Backend is empty after deleteAll()."); - $this->assertFalse($backend->get('test1'), 'First key has been deleted.'); $this->assertFalse($backend->get('test2'), 'Second key has been deleted.'); + $this->assertTrue($unrelated->get('test3'), 'Item in other bin is preserved.'); } /** @@ -563,15 +549,18 @@ abstract class GenericCacheBackendUnitTestBase extends DrupalUnitTestBase { */ public function testInvalidateAll() { $backend = $this->getCacheBackend(); + $unrelated = $this->getCacheBackend('bootstrap'); // Set both expiring and permanent keys. $backend->set('test1', 1, Cache::PERMANENT); $backend->set('test2', 3, time() + 1000); + $unrelated->set('test3', 4, Cache::PERMANENT); $backend->invalidateAll(); $this->assertFalse($backend->get('test1'), 'First key has been invalidated.'); $this->assertFalse($backend->get('test2'), 'Second key has been invalidated.'); + $this->assertTrue($unrelated->get('test3'), 'Item in other bin is preserved.'); $this->assertTrue($backend->get('test1', TRUE), 'First key has not been deleted.'); $this->assertTrue($backend->get('test2', TRUE), 'Second key has not been deleted.'); } diff --git a/core/modules/toolbar/toolbar.module b/core/modules/toolbar/toolbar.module index 4d810d6b126..3599c5944b1 100644 --- a/core/modules/toolbar/toolbar.module +++ b/core/modules/toolbar/toolbar.module @@ -584,14 +584,11 @@ function _toolbar_get_user_cid($uid, $langcode) { * (optional) The user ID whose toolbar cache entry to clear. */ function _toolbar_clear_user_cache($uid = NULL) { - $cache = \Drupal::cache('toolbar'); - if (!$cache->isEmpty()) { - // Clear by the 'user' tag in order to delete all caches, in any language, - // associated with this user. - if (isset($uid)) { - Cache::deleteTags(array('user' => array($uid))); - } else { - $cache->deleteAll(); - } + // Clear by the 'user' tag in order to delete all caches, in any language, + // associated with this user. + if (isset($uid)) { + Cache::deleteTags(array('user' => array($uid))); + } else { + \Drupal::cache('toolbar')->deleteAll(); } } diff --git a/core/tests/Drupal/Tests/Core/Cache/BackendChainImplementationUnitTest.php b/core/tests/Drupal/Tests/Core/Cache/BackendChainImplementationUnitTest.php index 75c0a977581..1cf617b0a5d 100644 --- a/core/tests/Drupal/Tests/Core/Cache/BackendChainImplementationUnitTest.php +++ b/core/tests/Drupal/Tests/Core/Cache/BackendChainImplementationUnitTest.php @@ -203,20 +203,6 @@ class BackendChainImplementationUnitTest extends UnitTestCase { $this->assertSame(232, $cached->data, 'And value has been kept'); } - /** - * Test that the chain is not empty when at least one backend has data. - */ - public function testNotEmptyIfOneBackendHasTheKey() { - $this->assertFalse($this->chain->isEmpty(), 'Chain is not empty'); - - // This is the only test that needs to start with an empty chain. - $this->chain->deleteAll(); - $this->assertTrue($this->chain->isEmpty(), 'Chain have been emptied by the deleteAll() call'); - - $this->secondBackend->set('test', 5); - $this->assertFalse($this->chain->isEmpty(), 'Chain is not empty anymore now that the second backend has something'); - } - /** * Test that the delete all operation is propagated to all backends in the chain. */ @@ -226,9 +212,12 @@ class BackendChainImplementationUnitTest extends UnitTestCase { $this->chain->set('test2', 3, time() + 1000); $this->chain->deleteAll(); - $this->assertTrue($this->firstBackend->isEmpty(), 'First backend is empty after delete all.'); - $this->assertTrue($this->secondBackend->isEmpty(), 'Second backend is empty after delete all.'); - $this->assertTrue($this->thirdBackend->isEmpty(), 'Third backend is empty after delete all.'); + $this->assertFalse($this->firstBackend->get('test1'), 'First key has been deleted in first backend.'); + $this->assertFalse($this->firstBackend->get('test2'), 'Second key has been deleted in first backend.'); + $this->assertFalse($this->secondBackend->get('test1'), 'First key has been deleted in second backend.'); + $this->assertFalse($this->secondBackend->get('test2'), 'Second key has been deleted in second backend.'); + $this->assertFalse($this->thirdBackend->get('test1'), 'First key has been deleted in third backend.'); + $this->assertFalse($this->thirdBackend->get('test2'), 'Second key has been deleted in third backend.'); } /** diff --git a/core/tests/Drupal/Tests/Core/Cache/NullBackendTest.php b/core/tests/Drupal/Tests/Core/Cache/NullBackendTest.php index fc3a7000430..bb7e6ac655c 100644 --- a/core/tests/Drupal/Tests/Core/Cache/NullBackendTest.php +++ b/core/tests/Drupal/Tests/Core/Cache/NullBackendTest.php @@ -35,7 +35,6 @@ class NullBackendTest extends UnitTestCase { $value = $this->randomName(); $null_cache->set($key, $value); - $this->assertTrue($null_cache->isEmpty()); $this->assertFalse($null_cache->get($key)); } }