Commit 5cdf2cce authored by catch's avatar catch

Issue #2236781 by damiankloip, alexpott, xjm: Fixed ViewsData only writes one...

Issue #2236781 by damiankloip, alexpott, xjm: Fixed ViewsData only writes one per-table cache entry per request.
parent 272c5249
......@@ -38,12 +38,24 @@ class ViewsData {
protected $cacheBackend;
/**
* Storage for the data itself.
* Table data storage.
*
* This is used for explicitly requested tables.
*
* @var array
*/
protected $storage = array();
/**
* All table storage data loaded from cache.
*
* This is used when all data has been loaded from the cache to prevent
* further cache get calls when rebuilding all data or for single tables.
*
* @var array
*/
protected $allStorage = array();
/**
* Whether the data has been fully loaded in this request.
*
......@@ -115,8 +127,8 @@ public function get($key = NULL) {
if (!isset($this->storage[$key])) {
// Prepare a cache ID for get and set.
$cid = $this->baseCid . ':' . $key;
$from_cache = FALSE;
if ($data = $this->cacheGet($cid)) {
$this->storage[$key] = $data->data;
$from_cache = TRUE;
......@@ -124,18 +136,23 @@ public function get($key = NULL) {
// If there is no cached entry and data is not already fully loaded,
// rebuild. This will stop requests for invalid tables calling getData.
elseif (!$this->fullyLoaded) {
$this->storage = $this->getData();
$this->allStorage = $this->getData();
}
if (!$from_cache) {
if (!isset($this->storage[$key])) {
if (!isset($this->allStorage[$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();
$this->allStorage[$key] = array();
}
else {
$this->storage[$key] = $this->allStorage[$key];
}
// Create a cache entry for the requested table.
$this->cacheSet($cid, $this->storage[$key]);
$this->cacheSet($cid, $this->allStorage[$key]);
}
}
......@@ -143,11 +160,16 @@ public function get($key = NULL) {
}
else {
if (!$this->fullyLoaded) {
$this->storage = $this->getData();
$this->allStorage = $this->getData();
}
// Set storage from allStorage outside of the fullyLoaded check to prevent
// cache calls on requests that have requested all data to get a single
// tables data. Make sure $this->storage is populated in this case.
$this->storage = $this->allStorage;
}
return $this->storage;
return $this->allStorage;
}
/**
......@@ -286,6 +308,7 @@ public function fetchBaseTables() {
*/
public function clear() {
$this->storage = array();
$this->allStorage = array();
$this->fullyLoaded = FALSE;
Cache::deleteTags(array('views_data' => TRUE));
}
......
......@@ -16,7 +16,7 @@
*
* @see hook_views_data
*
* @see \Drupal\views\ViewsData
* @coversDefaultClass \Drupal\views\ViewsData
*/
class ViewsDataTest extends UnitTestCase {
......@@ -192,6 +192,7 @@ public function testGetOnFirstCall() {
public function testFullAndTableGetCache() {
$expected_views_data = $this->viewsData();
$table_name = 'views_test_data';
$table_name_2 = 'views_test_data_2';
$random_table_name = $this->randomName();
// Views data should be invoked twice due to the clear call.
......@@ -212,8 +213,7 @@ public function testFullAndTableGetCache() {
->will($this->returnValue(FALSE));
$this->cacheBackend->expects($this->at(1))
->method('set')
->with("views_data:en", $expected_views_data)
->will($this->returnValue(FALSE));
->with("views_data:en", $expected_views_data);
$this->cacheBackend->expects($this->at(2))
->method('get')
->with("views_data:$random_table_name:en")
......@@ -229,8 +229,7 @@ public function testFullAndTableGetCache() {
->will($this->returnValue(FALSE));
$this->cacheBackend->expects($this->at(6))
->method('set')
->with("views_data:en", $expected_views_data)
->will($this->returnValue(FALSE));
->with("views_data:en", $expected_views_data);
$this->cacheBackend->expects($this->at(7))
->method('get')
->with("views_data:$random_table_name:en")
......@@ -246,6 +245,10 @@ public function testFullAndTableGetCache() {
$views_data = $this->viewsData->get($table_name);
$this->assertSame($expected_views_data[$table_name], $views_data);
// Another table being requested should also come from the static cache.
$views_data = $this->viewsData->get($table_name_2);
$this->assertSame($expected_views_data[$table_name_2], $views_data);
$views_data = $this->viewsData->get($random_table_name);
$this->assertSame(array(), $views_data);
......@@ -254,6 +257,7 @@ public function testFullAndTableGetCache() {
// Get the views data again.
$this->viewsData->get();
$this->viewsData->get($table_name);
$this->viewsData->get($table_name_2);
$this->viewsData->get($random_table_name);
}
......@@ -578,4 +582,42 @@ public function testCacheCallsWithWarmCacheAndGetAllTables() {
}
}
/**
* Tests the cache calls for multiple tables without warm caches.
*
* @covers ::get
*/
public function testCacheCallsWithoutWarmCacheAndGetMultipleTables() {
$expected_views_data = $this->viewsData();
$table_name = 'views_test_data';
$table_name_2 = 'views_test_data_2';
// Setup a warm cache backend for all table data, but not single tables.
$this->cacheBackend->expects($this->at(0))
->method('get')
->with("views_data:$table_name:en")
->will($this->returnValue(FALSE));
$this->cacheBackend->expects($this->at(1))
->method('get')
->with('views_data:en')
->will($this->returnValue((object) array('data' => $expected_views_data)));
$this->cacheBackend->expects($this->at(2))
->method('set')
->with("views_data:$table_name:en", $expected_views_data[$table_name]);
$this->cacheBackend->expects($this->at(3))
->method('get')
->with("views_data:$table_name_2:en")
->will($this->returnValue(FALSE));
$this->cacheBackend->expects($this->at(4))
->method('set')
->with("views_data:$table_name_2:en", $expected_views_data[$table_name_2]);
$this->assertSame($expected_views_data[$table_name], $this->viewsData->get($table_name));
$this->assertSame($expected_views_data[$table_name_2], $this->viewsData->get($table_name_2));
// Should only be invoked the first time.
$this->assertSame($expected_views_data[$table_name], $this->viewsData->get($table_name));
$this->assertSame($expected_views_data[$table_name_2], $this->viewsData->get($table_name_2));
}
}
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