Commit 09b02836 authored by Dries's avatar Dries

- Patch #692044 by Damien Tournoud, effulgentsia: a site with statistics...

- Patch #692044 by Damien Tournoud, effulgentsia: a site with statistics module enabled is much slower in serving cached pages in D7 than in D6.
parent d4200e26
......@@ -1069,14 +1069,14 @@ function drupal_serve_page_from_cache(stdClass $cache) {
// drupal_add_http_headers(). Keys are mixed-case.
$default_headers = array();
foreach ($cache->headers as $name => $value) {
foreach ($cache->data['headers'] as $name => $value) {
// In the case of a 304 response, certain headers must be sent, and the
// remaining may not (see RFC 2616, section 10.3.5). Do not override
// headers set in hook_boot().
$name_lower = strtolower($name);
if (in_array($name_lower, array('content-location', 'expires', 'cache-control', 'vary')) && !isset($hook_boot_headers[$name_lower])) {
drupal_add_http_header($name, $value);
unset($cache->headers[$name]);
unset($cache->data['headers'][$name]);
}
}
......@@ -1106,7 +1106,7 @@ function drupal_serve_page_from_cache(stdClass $cache) {
}
// Send the remaining headers.
foreach ($cache->headers as $name => $value) {
foreach ($cache->data['headers'] as $name => $value) {
drupal_add_http_header($name, $value);
}
......@@ -1134,19 +1134,20 @@ function drupal_serve_page_from_cache(stdClass $cache) {
header('Vary: Accept-Encoding', FALSE);
// If page_compression is enabled, the cache contains gzipped data.
if ($return_compressed) {
// $cache->data is already gzip'ed, so make sure zlib.output_compression
// does not compress it once more.
// $cache->data['body'] is already gzip'ed, so make sure
// zlib.output_compression does not compress it once more.
ini_set('zlib.output_compression', '0');
header('Content-Encoding: gzip');
}
else {
// The client does not support compression, so unzip the data in the
// cache. Strip the gzip header and run uncompress.
$cache->data = gzinflate(substr(substr($cache->data, 10), 0, -8));
$cache->data['body'] = gzinflate(substr(substr($cache->data['body'], 10), 0, -8));
}
}
print $cache->data;
// Print the page.
print $cache->data['body'];
}
/**
......@@ -1638,6 +1639,48 @@ function drupal_get_messages($type = NULL, $clear_queue = TRUE) {
return array();
}
/**
* Get the title of the current page, for display on the page and in the title bar.
*
* @return
* The current page's title.
*/
function drupal_get_title() {
$title = drupal_set_title();
// During a bootstrap, menu.inc is not included and thus we cannot provide a title.
if (!isset($title) && function_exists('menu_get_active_title')) {
$title = check_plain(menu_get_active_title());
}
return $title;
}
/**
* Set the title of the current page, for display on the page and in the title bar.
*
* @param $title
* Optional string value to assign to the page title; or if set to NULL
* (default), leaves the current title unchanged.
* @param $output
* Optional flag - normally should be left as CHECK_PLAIN. Only set to
* PASS_THROUGH if you have already removed any possibly dangerous code
* from $title using a function like check_plain() or filter_xss(). With this
* flag the string will be passed through unchanged.
*
* @return
* The updated title of the current page.
*/
function drupal_set_title($title = NULL, $output = CHECK_PLAIN) {
$stored_title = &drupal_static(__FUNCTION__);
if (isset($title)) {
$stored_title = ($output == PASS_THROUGH) ? $title : check_plain($title);
}
return $stored_title;
}
/**
* Check to see if an IP address has been blocked.
*
......@@ -1990,6 +2033,9 @@ function _drupal_bootstrap_page_cache() {
// If there is a cached page, display it.
if (is_object($cache)) {
header('X-Drupal-Cache: HIT');
// Restore the metadata cached with the page.
$_GET['q'] = $cache->data['path'];
drupal_set_title($cache->data['title'], PASS_THROUGH);
date_default_timezone_set(drupal_get_user_timezone());
// If the skipping of the bootstrap hooks is not enforced, call
// hook_boot.
......@@ -2307,6 +2353,54 @@ function request_path() {
return $path;
}
/**
* Return a component of the current Drupal path.
*
* When viewing a page at the path "admin/structure/types", for example, arg(0)
* returns "admin", arg(1) returns "structure", and arg(2) returns "types".
*
* Avoid use of this function where possible, as resulting code is hard to read.
* In menu callback functions, attempt to use named arguments. See the explanation
* in menu.inc for how to construct callbacks that take arguments. When attempting
* to use this function to load an element from the current path, e.g. loading the
* node on a node page, please use menu_get_object() instead.
*
* @param $index
* The index of the component, where each component is separated by a '/'
* (forward-slash), and where the first component has an index of 0 (zero).
* @param $path
* A path to break into components. Defaults to the path of the current page.
*
* @return
* The component specified by $index, or NULL if the specified component was
* not found.
*/
function arg($index = NULL, $path = NULL) {
// Even though $arguments doesn't need to be resettable for any functional
// reasons (the result of explode() does not depend on any run-time
// information), it should be resettable anyway in case a module needs to
// free up the memory used by it.
// Use the advanced drupal_static() pattern, since this is called very often.
static $drupal_static_fast;
if (!isset($drupal_static_fast)) {
$drupal_static_fast['arguments'] = &drupal_static(__FUNCTION__);
}
$arguments = &$drupal_static_fast['arguments'];
if (!isset($path)) {
$path = $_GET['q'];
}
if (!isset($arguments[$path])) {
$arguments[$path] = explode('/', $path);
}
if (!isset($index)) {
return $arguments[$path];
}
if (isset($arguments[$path][$index])) {
return $arguments[$path][$index];
}
}
/**
* If Drupal is behind a reverse proxy, we use the X-Forwarded-For header
* instead of $_SERVER['REMOTE_ADDR'], which would be the IP address of
......
......@@ -24,7 +24,7 @@ function getMultiple(&$cids) {
return array();
}
function set($cid, $data, $expire = CACHE_PERMANENT, array $headers = NULL) {
function set($cid, $data, $expire = CACHE_PERMANENT) {
}
function clear($cid = NULL, $wildcard = FALSE) {
......
......@@ -133,11 +133,9 @@ function cache_get_multiple(array &$cids, $bin = 'cache') {
* general cache wipe.
* - A Unix timestamp: Indicates that the item should be kept at least until
* the given time, after which it behaves like CACHE_TEMPORARY.
* @param $headers
* A string containing HTTP header information for cached pages.
*/
function cache_set($cid, $data, $bin = 'cache', $expire = CACHE_PERMANENT, array $headers = NULL) {
return _cache_get_object($bin)->set($cid, $data, $expire, $headers);
function cache_set($cid, $data, $bin = 'cache', $expire = CACHE_PERMANENT) {
return _cache_get_object($bin)->set($cid, $data, $expire);
}
/**
......@@ -263,10 +261,8 @@ function getMultiple(&$cids);
* general cache wipe.
* - A Unix timestamp: Indicates that the item should be kept at least until
* the given time, after which it behaves like CACHE_TEMPORARY.
* @param $headers
* A string containing HTTP header information for cached pages.
*/
function set($cid, $data, $expire = CACHE_PERMANENT, array $headers = NULL);
function set($cid, $data, $expire = CACHE_PERMANENT);
/**
......@@ -312,7 +308,7 @@ function get($cid) {
try {
// Garbage collection necessary when enforcing a minimum cache lifetime.
$this->garbageCollection($this->bin);
$cache = db_query("SELECT data, created, headers, expire, serialized FROM {" . $this->bin . "} WHERE cid = :cid", array(':cid' => $cid))->fetchObject();
$cache = db_query("SELECT data, created, expire, serialized FROM {" . $this->bin . "} WHERE cid = :cid", array(':cid' => $cid))->fetchObject();
return $this->prepareItem($cache);
}
catch (Exception $e) {
......@@ -327,7 +323,7 @@ function getMultiple(&$cids) {
// Garbage collection necessary when enforcing a minimum cache lifetime.
$this->garbageCollection($this->bin);
$query = db_select($this->bin);
$query->fields($this->bin, array('cid', 'data', 'created', 'headers', 'expire', 'serialized'));
$query->fields($this->bin, array('cid', 'data', 'created', 'expire', 'serialized'));
$query->condition($this->bin . '.cid', $cids, 'IN');
$result = $query->execute();
$cache = array();
......@@ -401,19 +397,15 @@ protected function prepareItem($cache) {
if ($cache->serialized) {
$cache->data = unserialize($cache->data);
}
if (isset($cache->headers)) {
$cache->headers = unserialize($cache->headers);
}
return $cache;
}
function set($cid, $data, $expire = CACHE_PERMANENT, array $headers = NULL) {
function set($cid, $data, $expire = CACHE_PERMANENT) {
$fields = array(
'serialized' => 0,
'created' => REQUEST_TIME,
'expire' => $expire,
'headers' => isset($headers) ? serialize($headers) : NULL,
);
if (!is_string($data)) {
$fields['data'] = serialize($data);
......
......@@ -4518,28 +4518,32 @@ function drupal_page_set_cache() {
if (drupal_page_is_cacheable()) {
$cache = (object) array(
'cid' => $base_root . request_uri(),
'data' => ob_get_clean(),
'data' => array(
'path' => $_GET['q'],
'body' => ob_get_clean(),
'title' => drupal_get_title(),
'headers' => array(),
),
'expire' => CACHE_TEMPORARY,
'created' => REQUEST_TIME,
'headers' => array(),
);
// Restore preferred header names based on the lower-case names returned
// by drupal_get_http_header().
$header_names = _drupal_set_preferred_header_name();
foreach (drupal_get_http_header() as $name_lower => $value) {
$cache->headers[$header_names[$name_lower]] = $value;
$cache->data['headers'][$header_names[$name_lower]] = $value;
if ($name_lower == 'expires') {
// Use the actual timestamp from an Expires header if available.
$cache->expire = strtotime($value);
}
}
if ($cache->data) {
if ($cache->data['body']) {
if (variable_get('page_compression', TRUE) && extension_loaded('zlib')) {
$cache->data = gzencode($cache->data, 9, FORCE_GZIP);
$cache->data['body'] = gzencode($cache->data['body'], 9, FORCE_GZIP);
}
cache_set($cache->cid, $cache->data, 'cache_page', $cache->expire, $cache->headers);
cache_set($cache->cid, $cache->data, 'cache_page', $cache->expire);
}
return $cache;
}
......
......@@ -234,96 +234,6 @@ function drupal_get_normal_path($path, $path_language = NULL) {
return $path;
}
/**
* Return a component of the current Drupal path.
*
* When viewing a page at the path "admin/structure/types", for example, arg(0)
* returns "admin", arg(1) returns "structure", and arg(2) returns "types".
*
* Avoid use of this function where possible, as resulting code is hard to read.
* In menu callback functions, attempt to use named arguments. See the explanation
* in menu.inc for how to construct callbacks that take arguments. When attempting
* to use this function to load an element from the current path, e.g. loading the
* node on a node page, please use menu_get_object() instead.
*
* @param $index
* The index of the component, where each component is separated by a '/'
* (forward-slash), and where the first component has an index of 0 (zero).
* @param $path
* A path to break into components. Defaults to the path of the current page.
*
* @return
* The component specified by $index, or NULL if the specified component was
* not found.
*/
function arg($index = NULL, $path = NULL) {
// Even though $arguments doesn't need to be resettable for any functional
// reasons (the result of explode() does not depend on any run-time
// information), it should be resettable anyway in case a module needs to
// free up the memory used by it.
// Use the advanced drupal_static() pattern, since this is called very often.
static $drupal_static_fast;
if (!isset($drupal_static_fast)) {
$drupal_static_fast['arguments'] = &drupal_static(__FUNCTION__);
}
$arguments = &$drupal_static_fast['arguments'];
if (!isset($path)) {
$path = $_GET['q'];
}
if (!isset($arguments[$path])) {
$arguments[$path] = explode('/', $path);
}
if (!isset($index)) {
return $arguments[$path];
}
if (isset($arguments[$path][$index])) {
return $arguments[$path][$index];
}
}
/**
* Get the title of the current page, for display on the page and in the title bar.
*
* @return
* The current page's title.
*/
function drupal_get_title() {
$title = drupal_set_title();
// During a bootstrap, menu.inc is not included and thus we cannot provide a title.
if (!isset($title) && function_exists('menu_get_active_title')) {
$title = check_plain(menu_get_active_title());
}
return $title;
}
/**
* Set the title of the current page, for display on the page and in the title bar.
*
* @param $title
* Optional string value to assign to the page title; or if set to NULL
* (default), leaves the current title unchanged.
* @param $output
* Optional flag - normally should be left as CHECK_PLAIN. Only set to
* PASS_THROUGH if you have already removed any possibly dangerous code
* from $title using a function like check_plain() or filter_xss(). With this
* flag the string will be passed through unchanged.
*
* @return
* The updated title of the current page.
*/
function drupal_set_title($title = NULL, $output = CHECK_PLAIN) {
$stored_title = &drupal_static(__FUNCTION__);
if (isset($title)) {
$stored_title = ($output == PASS_THROUGH) ? $title : check_plain($title);
}
return $stored_title;
}
/**
* Check if the current page is the front page.
*
......
......@@ -51,7 +51,12 @@ function statistics_help($path, $arg) {
function statistics_exit() {
global $user;
drupal_bootstrap(DRUPAL_BOOTSTRAP_FULL);
// When serving cached pages with the 'page_cache_without_database'
// configuration, system variables need to be loaded. This is a major
// performance decrease for non-database page caches, but with Statistics
// module, it is likely to also have 'statistics_enable_access_log' enabled,
// in which case we need to bootstrap to the session phase anyway.
drupal_bootstrap(DRUPAL_BOOTSTRAP_VARIABLES);
if (variable_get('statistics_count_content_views', 0)) {
// We are counting content views.
......@@ -70,6 +75,7 @@ function statistics_exit() {
}
}
if (variable_get('statistics_enable_access_log', 0)) {
drupal_bootstrap(DRUPAL_BOOTSTRAP_SESSION);
// Log this page access.
db_insert('accesslog')
->fields(array(
......
......@@ -33,6 +33,69 @@ class StatisticsTestCase extends DrupalWebTestCase {
}
}
/**
* Tests that logging via statistics_exit() works for cached and uncached pages.
*
* Subclass DrupalWebTestCase rather than StatisticsTestCase, because we want
* to test requests from an anonymous user.
*/
class StatisticsLoggingTestCase extends DrupalWebTestCase {
public static function getInfo() {
return array(
'name' => 'Statistics logging tests',
'description' => 'Tests request logging for cached and uncached pages.',
'group' => 'Statistics'
);
}
function setUp() {
parent::setUp('statistics');
// Ensure we have a node page to access.
$this->node = $this->drupalCreateNode();
// Enable page caching.
variable_set('cache', TRUE);
// Enable access logging.
variable_set('statistics_enable_access_log', 1);
variable_set('statistics_count_content_views', 1);
// Clear the logs.
db_truncate('accesslog');
db_truncate('node_counter');
}
/**
* Verifies request logging for cached and uncached pages.
*/
function testLogging() {
$path = 'node/' . $this->node->nid;
$expected = array(
'title' => $this->node->title,
'path' => $path,
);
// Verify logging of an uncached page.
$this->drupalGet($path);
$this->assertIdentical($this->drupalGetHeader('X-Drupal-Cache'), 'MISS', t('Testing an uncached page.'));
$log = db_query('SELECT * FROM {accesslog}')->fetchAll(PDO::FETCH_ASSOC);
$this->assertTrue(is_array($log) && count($log) == 1, t('Page request was logged.'));
$this->assertEqual(array_intersect_key($log[0], $expected), $expected);
$node_counter = statistics_get($this->node->nid);
$this->assertIdentical($node_counter['totalcount'], '1');
// Verify logging of a cached page.
$this->drupalGet($path);
$this->assertIdentical($this->drupalGetHeader('X-Drupal-Cache'), 'HIT', t('Testing a cached page.'));
$log = db_query('SELECT * FROM {accesslog}')->fetchAll(PDO::FETCH_ASSOC);
$this->assertTrue(is_array($log) && count($log) == 2, t('Page request was logged.'));
$this->assertEqual(array_intersect_key($log[1], $expected), $expected);
$node_counter = statistics_get($this->node->nid);
$this->assertIdentical($node_counter['totalcount'], '2');
}
}
/**
* Tests that report pages render properly, and that access logging works.
*/
......
......@@ -634,11 +634,6 @@ function system_schema() {
'not null' => TRUE,
'default' => 0,
),
'headers' => array(
'description' => 'Any custom HTTP headers to be added to cached data.',
'type' => 'text',
'not null' => FALSE,
),
'serialized' => array(
'description' => 'A flag to indicate whether content is serialized (1) or not (0).',
'type' => 'int',
......@@ -2381,6 +2376,16 @@ function system_update_7053() {
->execute();
}
/**
* Remove {cache_*}.headers columns.
*/
function system_update_7054() {
$cache_tables = array('cache', 'cache_bootstrap', 'cache_filter', 'cache_form', 'cache_menu', 'cache_page', 'cache_path');
foreach ($cache_tables as $table) {
db_drop_field($table, 'headers');
}
}
/**
* @} End of "defgroup updates-6.x-to-7.x"
* The next series of updates should start at 8000.
......
......@@ -167,3 +167,10 @@ function update_update_7000() {
$queue = DrupalQueue::get('update_fetch_tasks');
$queue->createQueue();
}
/**
* Remove {cache_update}.headers columns.
*/
function update_update_7001() {
db_drop_field('cache_update', 'headers');
}
......@@ -653,9 +653,6 @@ function theme_update_last_check($variables) {
/**
* Store data in the private update status cache table.
*
* Note: this function completely ignores the {cache_update}.headers field
* since that is meaningless for the kinds of data we're caching.
*
* @param $cid
* The cache ID to save the data with.
* @param $data
......@@ -671,7 +668,6 @@ function _update_cache_set($cid, $data, $expire) {
$fields = array(
'created' => REQUEST_TIME,
'expire' => $expire,
'headers' => NULL,
);
if (!is_string($data)) {
$fields['data'] = serialize($data);
......
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