Drupal
/
drupal
mirror of https://git.drupalcode.org/project/drupal.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

Issue #1962578 by Berdir, damiankloip, dawehner, xjm: Fixed ViewsDataCache writes multiple times in __destruct().

8.0.x
catch 2013-04-10 10:35:19 +01:00
parent 879589c8ac
commit 75f3ef0601
4 changed files with 237 additions and 47 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

100
core/lib/Drupal/Core/Cache/MemoryCounterBackend.php Normal file
View File

@ -0,0 +1,100 @@
<?php
/**
* @file
* Contains \Drupal\Core\Cache\MemoryCounterBackend.
*/
namespace Drupal\Core\Cache;
/**
* Defines a memory cache implementation that counts set and get calls.
*
* This can be used to mock a cache backend where one needs to know how
* many times a cache entry was set or requested.
*
* @todo On the longrun this backend should be replaced by phpunit mock objects.
*
*/
class MemoryCounterBackend extends MemoryBackend {
/**
* Stores a list of cache cid calls keyed by function name.
*
* @var array
*/
protected $counter = array();
/**
* Implements \Drupal\Core\Cache\CacheBackendInterface::get().
*/
public function get($cid, $allow_invalid = FALSE) {
$this->increaseCounter(__FUNCTION__, $cid);
return parent::get($cid, $allow_invalid);
}
/**
* Implements \Drupal\Core\Cache\CacheBackendInterface::set().
*/
public function set($cid, $data, $expire = CacheBackendInterface::CACHE_PERMANENT, array $tags = array()) {
$this->increaseCounter(__FUNCTION__, $cid);
parent::set($cid, $data, $expire, $tags);
}
/**
* Implements \Drupal\Core\Cache\CacheBackendInterface::delete().
*/
public function delete($cid) {
$this->increaseCounter(__FUNCTION__, $cid);
parent::delete($cid);
}
/**
* Increase the counter for a function with a certain cid.
*
* @param string $function
* The called function.
*
* @param string $cid
* The cache ID of the cache entry to increase the counter.
*/
protected function increaseCounter($function, $cid) {
if (!isset($this->counter[$function][$cid])) {
$this->counter[$function][$cid] = 1;
}
else {
$this->counter[$function][$cid]++;
}
}
/**
* Returns the call counter for the get, set and delete methods.
*
* @param string $method
* (optional) The name of the method to return the call counter for.
* @param string $cid
* (optional) The name of the cache id to return the call counter for.
*
* @return int|array
* An integer if both method and cid is given, an array otherwise.
*/
public function getCounter($method = NULL, $cid = NULL) {
if ($method && $cid) {
return isset($this->counter[$method][$cid]) ? $this->counter[$method][$cid] : 0;
}
elseif ($method) {
return isset($this->counter[$method]) ? $this->counter[$method] : array();
}
else {
return $this->counter;
}
}
/**
* Resets the call counter.
*/
public function resetCounter() {
$this->counter = array();
}
}

119
core/modules/views/lib/Drupal/views/Tests/ViewsDataTest.php
View File

@ -7,6 +7,9 @@
namespace Drupal\views\Tests;
use Drupal\Core\Cache\MemoryCounterBackend;
use Drupal\views\ViewsDataCache;
/**
* Tests the fetching of views data.
*
@ -28,6 +31,13 @@ class ViewsDataTest extends ViewUnitTestBase {
*/
protected $count = 0;
/**
* The memory backend to use for the test.
*
* @var \Drupal\Core\Cache\MemoryCounterBackend
*/
protected $memoryCounterBackend;
public static function getInfo() {
return array(
'name' => 'Table Data',
@ -39,8 +49,10 @@ class ViewsDataTest extends ViewUnitTestBase {
protected function setUp() {
parent::setUp();
$this->viewsDataCache = $this->container->get('views.views_data');
$this->memoryCounterBackend = new MemoryCounterBackend('views_info');
$this->state = $this->container->get('state');
$this->initViewsDataCache();
}
/**
@ -117,6 +129,111 @@ class ViewsDataTest extends ViewUnitTestBase {
$this->assertCountIncrement(FALSE);
}
/**
* Ensures that cache entries are only set and get when necessary.
*/
public function testCacheRequests() {
// Request the same table 5 times. The caches are empty at this point, so
// what will happen is that it will first check for a cache entry for the
// given table, get a cache miss, then try the cache entry for all tables,
// which does not exist yet either. As a result, it rebuilds the information
// and writes a cache entry for all tables and the requested table.
$table_name = 'views_test_data';
for ($i = 0; $i < 5; $i++) {
$this->viewsDataCache->get($table_name);
}
// Assert cache set and get calls.
$this->assertEqual($this->memoryCounterBackend->getCounter('get', 'views_data:views_test_data:en'), 1, 'Requested the cache for the table-specific cache entry.');
$this->assertEqual($this->memoryCounterBackend->getCounter('get', 'views_data:en'), 1, 'Requested the cache for all tables.');
$this->assertEqual($this->memoryCounterBackend->getCounter('set', 'views_data:views_test_data:en'), 1, 'Wrote the cache for the requested once.');
$this->assertEqual($this->memoryCounterBackend->getCounter('set', 'views_data:en'), 1, 'Wrote the cache for the all tables once.');
// Re-initialize the views data cache to simulate a new request and repeat.
// We have a warm cache now, so this will only request the tables-specific
// cache entry and return that.
$this->initViewsDataCache();
for ($i = 0; $i < 5; $i++) {
$this->viewsDataCache->get($table_name);
}
// Assert cache set and get calls.
$this->assertEqual($this->memoryCounterBackend->getCounter('get', 'views_data:views_test_data:en'), 1, 'Requested the cache for the table-specific cache entry.');
$this->assertEqual($this->memoryCounterBackend->getCounter('get', 'views_data:en'), 0, 'Did not request to load the cache entry for all tables.');
$this->assertEqual($this->memoryCounterBackend->getCounter('set', 'views_data:views_test_data:en'), 0, 'Did not write the cache for the requested table.');
$this->assertEqual($this->memoryCounterBackend->getCounter('set', 'views_data:en'), 0, 'Did not write the cache for all tables.');
// Re-initialize the views data cache to simulate a new request and request
// a different table. This will fail to get a table specific cache entry,
// load the cache entry for all tables and save a cache entry for this table
// but not all.
$this->initViewsDataCache();
$another_table_name = 'views';
for ($i = 0; $i < 5; $i++) {
$this->viewsDataCache->get($another_table_name);
}
// Assert cache set and get calls.
$this->assertEqual($this->memoryCounterBackend->getCounter('get', 'views_data:views:en'), 1, 'Requested the cache for the table-specific cache entry.');
$this->assertEqual($this->memoryCounterBackend->getCounter('get', 'views_data:en'), 1, 'Requested the cache for all tables.');
$this->assertEqual($this->memoryCounterBackend->getCounter('set', 'views_data:views:en'), 1, 'Wrote the cache for the requested once.');
$this->assertEqual($this->memoryCounterBackend->getCounter('set', 'views_data:en'), 0, 'Did not write the cache for all tables.');
// Re-initialize the views data cache to simulate a new request and request
// a non-existing table. This will result in the same cache requests as we
// explicitly write an empty cache entry for non-existing tables to avoid
// unecessary requests in those situations. We do have to load the cache
// entry for all tables to check if the table does exist or not.
$this->initViewsDataCache();
$non_existing_table = $this->randomName();
for ($i = 0; $i < 5; $i++) {
$this->viewsDataCache->get($non_existing_table);
}
// Assert cache set and get calls.
$this->assertEqual($this->memoryCounterBackend->getCounter('get', "views_data:$non_existing_table:en"), 1, 'Requested the cache for the table-specific cache entry.');
$this->assertEqual($this->memoryCounterBackend->getCounter('get', 'views_data:en'), 1, 'Requested the cache for all tables.');
$this->assertEqual($this->memoryCounterBackend->getCounter('set', "views_data:$non_existing_table:en"), 1, 'Wrote the cache for the requested once.');
$this->assertEqual($this->memoryCounterBackend->getCounter('set', 'views_data:en'), 0, 'Did not write the cache for all tables.');
// Re-initialize the views data cache to simulate a new request and request
// the same non-existing table. This will load the table-specific cache
// entry and return the stored empty array for that.
$this->initViewsDataCache();
for ($i = 0; $i < 5; $i++) {
$this->viewsDataCache->get($non_existing_table);
}
// Assert cache set and get calls.
$this->assertEqual($this->memoryCounterBackend->getCounter('get', "views_data:$non_existing_table:en"), 1, 'Requested the cache for the table-specific cache entry.');
$this->assertEqual($this->memoryCounterBackend->getCounter('get', 'views_data:en'), 0, 'Did not request to load the cache entry for all tables.');
$this->assertEqual($this->memoryCounterBackend->getCounter('set', "views_data:$non_existing_table:en"), 0, 'Did not write the cache for the requested table.');
$this->assertEqual($this->memoryCounterBackend->getCounter('set', 'views_data:en'), 0, 'Did not write the cache for all tables.');
// Re-initialize the views data cache and repeat with no specified table.
// This should only load the cache entry for all tables.
$this->initViewsDataCache();
for ($i = 0; $i < 5; $i++) {
$this->viewsDataCache->get();
}
// This only requested the full information. No other cache requests should
// have been made.
$this->assertEqual($this->memoryCounterBackend->getCounter('get', 'views_data:views_test_data:en'), 0);
$this->assertEqual($this->memoryCounterBackend->getCounter('get', 'views_data:en'), 1);
$this->assertEqual($this->memoryCounterBackend->getCounter('set', 'views_data:views_test_data:en'), 0);
$this->assertEqual($this->memoryCounterBackend->getCounter('set', 'views_data:en'), 0);
}
/**
* Initializes a new ViewsDataCache instance and resets the cache backend.
*/
protected function initViewsDataCache() {
$this->memoryCounterBackend->resetCounter();
$this->viewsDataCache = new ViewsDataCache($this->memoryCounterBackend, $this->container->get('config.factory'), $this->container->get('module_handler'));
}
/**
* Starts a count for hook_views_data being invoked.
*/

63
core/modules/views/lib/Drupal/views/ViewsDataCache.php
View File

@ -10,12 +10,16 @@ namespace Drupal\views;
use Drupal\Core\Cache\CacheBackendInterface;
use Drupal\Core\Config\ConfigFactory;
use Drupal\Core\Extension\ModuleHandlerInterface;
use Drupal\Core\DestructableInterface;
/**
* Class to manage and lazy load cached views data.
*
* If a table is requested and cannot be loaded from cache, all data is then
* requested from cache. A table-specific cache entry will then be created for
* the requested table based on this cached data. Table data is only rebuilt
* when no cache entry for all table data can be retrieved.
*/
class ViewsDataCache implements DestructableInterface {
class ViewsDataCache {
/**
* The base cache ID to use.
@ -38,13 +42,6 @@ class ViewsDataCache implements DestructableInterface {
*/
protected $storage = array();
/**
* An array of requested tables.
*
* @var array
*/
protected $requestedTables = array();
/**
* Whether the data has been fully loaded in this request.
*
@ -52,14 +49,6 @@ class ViewsDataCache implements DestructableInterface {
*/
protected $fullyLoaded = FALSE;
/**
* Whether views data has been rebuilt. This is set when getData() doesn't
* return anything from cache.
*
* @var bool
*/
protected $rebuildAll = FALSE;
/**
* Whether or not to skip data caching and rebuild data each time.
*
@ -111,11 +100,11 @@ class ViewsDataCache implements DestructableInterface {
*/
public function get($key = NULL) {
if ($key) {
$from_cache = FALSE;
if (!isset($this->storage[$key])) {
// Prepare a cache ID.
$cid = $this->baseCid . ':' . $key;
$from_cache = FALSE;
if ($data = $this->cacheGet($cid)) {
$this->storage[$key] = $data->data;
$from_cache = TRUE;
@ -125,15 +114,19 @@ class ViewsDataCache implements DestructableInterface {
elseif (!$this->fullyLoaded) {
$this->storage = $this->getData();
}
}
if (isset($this->storage[$key])) {
if (!$from_cache) {
// Add this table to a list of requested tables, as it's table cache
// entry was not found.
array_push($this->requestedTables, $key);
if (!isset($this->storage[$key])) {
// Write an empty cache entry if no information for that table
// exists to avoid repeated cache get calls for this table and
// prevent loading all tables unnecessarily.
$this->storage[$key] = array();
}
// Create a cache entry for the requested table.
$this->cacheBackend->set($this->prepareCid($cid), $this->storage[$key]);
}
}
if (isset($this->storage[$key])) {
return $this->storage[$key];
}
@ -200,8 +193,8 @@ class ViewsDataCache implements DestructableInterface {
$this->processEntityTypes($data);
// Set as TRUE, so all table data will be cached.
$this->rebuildAll = TRUE;
// Keep a record with all data.
$this->cacheBackend->set($this->prepareCid($this->baseCid), $data);
return $data;
}
@ -268,24 +261,6 @@ class ViewsDataCache implements DestructableInterface {
return $tables;
}
/**
* Implements \Drupal\Core\DestructableInterface::destruct().
*/
public function destruct() {
if (!empty($this->storage) && !$this->skipCache) {
if ($this->rebuildAll) {
// Keep a record with all data.
$this->cacheBackend->set($this->prepareCid($this->baseCid), $this->storage);
}
// Save data in seperate, per table cache entries.
foreach ($this->requestedTables as $table) {
$cid = $this->baseCid . ':' . $table;
$this->cacheBackend->set($this->prepareCid($cid), $this->storage[$table]);
}
}
}
/**
* Clears the class storage and cache.
*/

2
core/modules/views/views.services.yml
View File

@ -58,8 +58,6 @@ services:
arguments: [wizard, '%container.namespaces%']
views.views_data:
class: Drupal\views\ViewsDataCache
tags:
- { name: needs_destruction }
arguments: ['@cache.views_info', '@config.factory', '@module_handler']
views.executable:
class: Drupal\views\ViewExecutableFactory