From 93b93df1147414211af6baf4028c9d6954ebdbbd Mon Sep 17 00:00:00 2001 From: "Dmitriy.trt" Date: Fri, 15 Jun 2012 01:30:36 +0700 Subject: [PATCH 1/2] Issue #1055616: Fix cached output for different input with the same results --- plugins/views_plugin_cache.inc | 53 ++++++++++++++++---------------- tests/views_cache.test | 65 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 92 insertions(+), 26 deletions(-) diff --git a/plugins/views_plugin_cache.inc b/plugins/views_plugin_cache.inc index 969c843..756ed9f 100644 --- a/plugins/views_plugin_cache.inc +++ b/plugins/views_plugin_cache.inc @@ -243,56 +243,57 @@ class views_plugin_cache extends views_plugin { } } - function get_results_key() { + /** + * Returns cache key. + * + * @param string $cache_type + * @param array $key_data + * Additional data for cache segmentation and/or overrides for default + * segmentation. + * + * @return string + */ + function get_key($cache_type, $key_data = array()) { global $user; - if (!isset($this->_results_key)) { + $key_data += array( + 'roles' => array_keys($user->roles), + 'super-user' => $user->uid == 1, // special caching for super user. + 'language' => $GLOBALS['language']->language, + 'base_url' => $GLOBALS['base_url'], + ); + if (empty($key_data['build_info'])) { $build_info = $this->view->build_info; - - $query_plugin = $this->view->display_handler->get_plugin('query'); - foreach (array('query','count_query') as $index) { // If the default query back-end is used generate SQL query strings from // the query objects. if ($build_info[$index] instanceof SelectQueryInterface) { $query = clone $build_info[$index]; $query->preExecute(); - $build_info[$index] = (string)$query; - } - } - $key_data = array( - 'build_info' => $build_info, - 'roles' => array_keys($user->roles), - 'super-user' => $user->uid == 1, // special caching for super user. - 'language' => $GLOBALS['language']->language, - 'base_url' => $GLOBALS['base_url'], - ); - foreach (array('exposed_info', 'page', 'sort', 'order', 'items_per_page', 'offset') as $key) { - if (isset($_GET[$key])) { - $key_data[$key] = $_GET[$key]; + $key_data['build_info'][$index] = strtr($query, $query->getArguments()); } } + } + $key = $this->view->name . ':' . $this->display->id . ':' . $cache_type . ':' . md5(serialize($key_data)); + return $key; + } - $this->_results_key = $this->view->name . ':' . $this->display->id . ':results:' . md5(serialize($key_data)); + function get_results_key() { + if (!isset($this->_results_key)) { + $this->_results_key = $this->get_key('results'); } return $this->_results_key; } function get_output_key() { - global $user; if (!isset($this->_output_key)) { $key_data = array( 'result' => $this->view->result, - 'roles' => array_keys($user->roles), - 'super-user' => $user->uid == 1, // special caching for super user. 'theme' => $GLOBALS['theme'], - 'language' => $GLOBALS['language']->language, - 'base_url' => $GLOBALS['base_url'], ); - - $this->_output_key = $this->view->name . ':' . $this->display->id . ':output:' . md5(serialize($key_data)); + $this->_output_key = $this->get_key('output', $key_data); } return $this->_output_key; diff --git a/tests/views_cache.test b/tests/views_cache.test index df80453..cdb678b 100644 --- a/tests/views_cache.test +++ b/tests/views_cache.test @@ -208,4 +208,69 @@ class ViewsCacheTest extends ViewsSqlTest { } + /** + * Test caching of different exposed filter values with the same view result. + * + * Make sure the output is different. + */ + function testExposedFilterSameResultsCaching() { + // Create the view with time-based cache with hour lifetimes and add exposed + // filter to it with "Starts with" operator. + $view = $this->getBasicView(); + $view->set_display(); + $view->display_handler->override_option('cache', array( + 'type' => 'time', + 'results_lifespan' => '3600', + 'output_lifespan' => '3600', + )); + $view->display_handler->override_option('filters', array( + 'name' => array( + 'id' => 'name', + 'table' => 'views_test', + 'field' => 'name', + 'relationship' => 'none', + 'operator' => 'starts', + 'exposed' => TRUE, + 'expose' => array( + 'operator_id' => 'name_op', + 'operator' => 'name_op', + 'identifier' => 'name', + ), + ), + )); + + // Clone the view before setting exposed input. + $clone = $view->copy(); + + // Pass "Rin" to the exposed filter and check that only one row returned. + $view->set_exposed_input(array( + 'name' => 'Rin', + )); + $this->executeView($view); + $first_result = $view->result; + $first_output = $view->render(); + $this->assertEqual(1, count($first_result), t('The number of rows returned by the first view match.')); + + // Pass full "Ringo" to the exposed filter at the second time and make sure + // results are the same. + $clone->set_exposed_input(array( + 'name' => 'Ringo', + )); + $this->executeView($clone); + $second_result = $clone->result; + $second_output = $clone->render(); + $this->assertEqual($first_result, $second_result, t('Results of both views are the same.')); + + // Check that output is not the same and it contains full "Ringo" word in + // default value of exposed input. + $this->assertNotEqual($first_output, $second_output, t('Output of the second view is different.')); + $document = new DOMDocument(); + $html = '
' . $second_output . '
'; + $this->assertTrue($document->loadHTML($html), t('HTML has been loaded to DOM parser.')); + $xpath = new DOMXPath($document); + $this->assertEqual("Ringo", $xpath->evaluate('string(//input[@name="name"]/@value)'), t('Input field of exposed filter has the second value.')); + + $view->destroy(); + $clone->destroy(); + } } -- 1.7.10 From 2caa5a287547cba8b7e7b17c8ccc4655d0348174 Mon Sep 17 00:00:00 2001 From: "Dmitriy.trt" Date: Fri, 15 Jun 2012 18:54:00 +0700 Subject: [PATCH 2/2] Issue #1055616: add limits to the query on the query building step We're going to use full query string as a part of the cache key, so the limits must be in query before we try to get data from the cache. --- plugins/views_plugin_query_default.inc | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/plugins/views_plugin_query_default.inc b/plugins/views_plugin_query_default.inc index 0f1a1fe..8cb9494 100644 --- a/plugins/views_plugin_query_default.inc +++ b/plugins/views_plugin_query_default.inc @@ -1353,6 +1353,16 @@ class views_plugin_query_default extends views_plugin_query { // Add all query substitutions as metadata. $query->addMetaData('views_substitutions', module_invoke_all('views_query_substitutions', $this)); + if (!$get_count) { + if (!empty($this->limit) || !empty($this->offset)) { + // We can't have an offset without a limit, so provide a very large limit + // instead. + $limit = intval(!empty($this->limit) ? $this->limit : 999999); + $offset = intval(!empty($this->offset) ? $this->offset : 0); + $query->range($offset, $limit); + } + } + return $query; } @@ -1457,16 +1467,8 @@ class views_plugin_query_default extends views_plugin_query { $this->pager->execute_count_query($count_query); } - // Let the pager modify the query to add limits. $this->pager->pre_execute($query); - if (!empty($this->limit) || !empty($this->offset)) { - // We can't have an offset without a limit, so provide a very large limit instead. - $limit = intval(!empty($this->limit) ? $this->limit : 999999); - $offset = intval(!empty($this->offset) ? $this->offset : 0); - $query->range($offset, $limit); - } - $result = $query->execute(); $view->result = array(); -- 1.7.10