From 2d6b6d492f9757ee19d992f90d73b99235f58609 Mon Sep 17 00:00:00 2001 From: Dries Buytaert <dries@buytaert.net> Date: Tue, 15 Dec 2009 08:45:32 +0000 Subject: [PATCH] - Patch #566494 by Dave Reid, chx, JoshuaRogers, David_Rothstein, Rob Loach, TheRec, moshe weitzman: cron image does a full bootstrap on every page request so changing to a Javascript-based solution instead. Important performance fix -- what were we smoking? ;-) --- includes/common.inc | 8 +++- modules/system/system.admin.inc | 2 +- modules/system/system.cron.js | 20 ++++++++ modules/system/system.js | 1 - modules/system/system.module | 81 ++++++++++++++------------------- modules/system/system.test | 74 ++++++++++++++++++------------ 6 files changed, 105 insertions(+), 81 deletions(-) create mode 100644 modules/system/system.cron.js diff --git a/includes/common.inc b/includes/common.inc index 8ff82045223b..27ffaa411d8f 100644 --- a/includes/common.inc +++ b/includes/common.inc @@ -3082,7 +3082,7 @@ function base_path() { * which on normal pages is up through the preprocess step of theme('html'). * Adding a link will overwrite a prior link with the exact same 'rel' and * 'href' attributes. - * + * * @param $attributes * Associative array of element attributes including 'href' and 'rel'. * @param $header @@ -3482,7 +3482,7 @@ function drupal_load_stylesheet_content($contents, $optimize = FALSE) { $contents = preg_replace('{ (?<=\\\\\*/)([^/\*]+/\*)([^\*/]+\*/) # Add a backslash also at the end ie-mac hack comment, so the next pass will not touch it. # The added backshlash does not affect the effectiveness of the hack. - }x', '\1\\\\\2', $contents); + }x', '\1\\\\\2', $contents); $contents = preg_replace('< \s*([@{}:;,]|\)\s|\s\()\s* | # Remove whitespace around separators, but keep space around parentheses. /\*[^*\\\\]*\*+([^/*][^*]*\*+)*/ | # Remove comments that are not CSS hacks. @@ -4571,6 +4571,10 @@ function drupal_page_set_cache() { $header_names = _drupal_set_preferred_header_name(); foreach (drupal_get_http_header() as $name_lower => $value) { $cache->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) { diff --git a/modules/system/system.admin.inc b/modules/system/system.admin.inc index d0b3a6226507..1d326da9563e 100644 --- a/modules/system/system.admin.inc +++ b/modules/system/system.admin.inc @@ -1496,7 +1496,7 @@ function system_site_information_settings_submit($form, &$form_state) { // Clear caches when the cron threshold is changed to ensure that the cron // image is not contained in cached pages. $cron_threshold = variable_get('cron_safe_threshold', DRUPAL_CRON_DEFAULT_THRESHOLD); - if (($cron_threshold > 0 && $form_state['input']['cron_safe_threshold'] == 0) || ($cron_threshold == 0 && $form_state['input']['cron_safe_threshold'] > 0)) { + if ($cron_threshold != $form_state['input']['cron_safe_threshold']) { cache_clear_all(); } } diff --git a/modules/system/system.cron.js b/modules/system/system.cron.js new file mode 100644 index 000000000000..01809b096e59 --- /dev/null +++ b/modules/system/system.cron.js @@ -0,0 +1,20 @@ +// $Id$ +(function ($) { + +/** + * Checks to see if the cron should be automatically run. + */ +Drupal.behaviors.cronCheck = { + attach: function(context, settings) { + if (settings.cronCheck || false) { + $('body').once('cron-check', function() { + // Only execute the cron check if its the right time. + if (Math.round(new Date().getTime() / 1000.0) > settings.cronCheck) { + $.get(settings.basePath + 'system/run-cron-check'); + } + }); + } + } +}; + +})(jQuery); diff --git a/modules/system/system.js b/modules/system/system.js index b0ae974d2cae..0b474deb9dd4 100644 --- a/modules/system/system.js +++ b/modules/system/system.js @@ -211,4 +211,3 @@ Drupal.behaviors.machineReadableValue = { }; })(jQuery); - diff --git a/modules/system/system.module b/modules/system/system.module index ddb970b19679..dc39bcca589d 100644 --- a/modules/system/system.module +++ b/modules/system/system.module @@ -190,9 +190,6 @@ function system_theme() { 'system_compact_link' => array( 'variables' => array(), ), - 'system_run_cron_image' => array( - 'variables' => array('image_path' => NULL), - ), 'system_date_time_settings' => array( 'render element' => 'form', 'file' => 'system.admin.inc', @@ -530,10 +527,10 @@ function system_menu() { 'type' => MENU_CALLBACK, 'file' => 'system.admin.inc', ); - $items['system/run-cron-image'] = array( + $items['system/run-cron-check'] = array( 'title' => 'Execute cron', - 'page callback' => 'system_run_cron_image', - 'access callback' => 'system_run_cron_image_access', + 'page callback' => 'system_run_cron_check', + 'access callback' => 'system_run_cron_check_access', 'type' => MENU_CALLBACK, 'file' => 'system.admin.inc', ); @@ -2420,6 +2417,10 @@ function system_region_list($theme_key, $show = REGIONS_ALL) { if (empty($list[$theme_key][$show])) { $themes = list_themes(); + if (!isset($themes[$theme_key])) { + $list[$theme_key][$show] = array(); + return $list[$theme_key][$show]; + } $info = $themes[$theme_key]->info; // If requested, suppress hidden regions. @see block_admin_display_form(). foreach ($info['regions'] as $name => $label) { @@ -3066,17 +3067,23 @@ function system_retrieve_file($url, $destination = NULL, $overwrite = TRUE) { */ function system_page_build(&$page) { // Automatic cron runs. - // @see system_run_cron_image() - if (system_run_cron_image_access()) { + $cron_threshold = variable_get('cron_safe_threshold', DRUPAL_CRON_DEFAULT_THRESHOLD); + if ($cron_threshold) { $page['page_bottom']['run_cron'] = array( // Trigger cron run via AJAX. '#attached' => array( 'js' => array( - '(function($){ $.get(' . drupal_json_encode(url('system/run-cron-image')) . '); })(jQuery);' => array('type' => 'inline', 'scope' => 'header'), + drupal_get_path('module', 'system') . '/system.cron.js' => array('weight' => JS_DEFAULT - 5), + array( + 'data' => array( + // Add the timestamp for the next automatic cron run. + 'cronCheck' => variable_get('cron_last', 0) + $cron_threshold, + ), + 'type' => 'setting', + ), + ), ), - // Trigger cron run for clients not supporting JavaScript (fall-back). - '#markup' => theme('system_run_cron_image', array('image_path' => 'system/run-cron-image')), ); } } @@ -3097,27 +3104,18 @@ function system_page_alter(&$page) { } /** - * Menu callback; executes cron via an image callback. + * Menu callback; executes cron via an AJAX callback. * * This callback runs cron in a separate HTTP request to prevent "mysterious" - * slow-downs of regular HTTP requests. It is either invoked via an AJAX request - * (if the client's browser supports JavaScript) or by an IMG tag directly in - * the page output (for clients not supporting JavaScript). For the latter case, - * we need to output a transparent 1x1 image, so the browser does not render the - * image's alternate text or a 'missing image placeholder'. The AJAX request + * slow-downs of regular HTTP requests. It is invoked via an AJAX request and * does not process the returned output. * * @see system_page_alter() - * @see theme_system_run_cron_image() - * @see system_run_cron_image_access() + * @see system_run_cron_check_access() */ -function system_run_cron_image() { - drupal_page_is_cacheable(FALSE); - - // Output a transparent 1x1 image to the browser; required for clients not - // supporting JavaScript. - drupal_add_http_header('Content-Type', 'image/gif'); - echo "\x47\x49\x46\x38\x39\x61\x1\x0\x1\x0\x80\xff\x0\xc0\xc0\xc0\x0\x0\x0\x21\xf9\x4\x1\x0\x0\x0\x0\x2c\x0\x0\x0\x0\x1\x0\x1\x0\x0\x2\x2\x44\x1\x0\x3b"; +function system_run_cron_check() { + $cron_run = FALSE; + $cron_threshold = variable_get('cron_safe_threshold', DRUPAL_CRON_DEFAULT_THRESHOLD); // Cron threshold semaphore is used to avoid errors every time the image // callback is displayed when a previous cron is still running. @@ -3133,19 +3131,23 @@ function system_run_cron_image() { } } else { - // Run cron automatically if it has never run or threshold was crossed. $cron_last = variable_get('cron_last', NULL); - $cron_threshold = variable_get('cron_safe_threshold', DRUPAL_CRON_DEFAULT_THRESHOLD); + // Run cron automatically if it has never run or threshold was crossed. if (!isset($cron_last) || (REQUEST_TIME - $cron_last > $cron_threshold)) { // Lock cron threshold semaphore. variable_set('cron_threshold_semaphore', REQUEST_TIME); - drupal_cron_run(); + $cron_run = drupal_cron_run(); // Release the cron threshold semaphore. variable_del('cron_threshold_semaphore'); } } - drupal_exit(); + // Add an expires header with the time of the next cron run. + $cron_last = variable_get('cron_last', NULL); + drupal_add_http_header('Expires', gmdate(DATE_RFC1123, $cron_last + $cron_threshold)); + + drupal_json_output(array('cron_run' => $cron_run)); + drupal_page_footer(); } /** @@ -3156,28 +3158,13 @@ function system_run_cron_image() { * @return * TRUE if cron threshold is enabled, FALSE otherwise. * - * @see system_run_cron_image() + * @see system_run_cron_check() * @see system_page_alter() */ -function system_run_cron_image_access() { +function system_run_cron_check_access() { return variable_get('cron_safe_threshold', DRUPAL_CRON_DEFAULT_THRESHOLD) > 0; } -/** - * Display image used to run cron automatically. - * - * Renders an image pointing to the automatic cron run menu callback for - * graceful degradation when Javascript is not available. The wrapping NOSCRIPT - * tag ensures that only browsers not supporting JavaScript render the image. - * - * @see system_page_alter() - * @see system_run_cron_image() - * @ingroup themeable - */ -function theme_system_run_cron_image($variables) { - return '<noscript><div id="system-cron-image">' . theme('image', array('path' => $variables['image_path'], 'getsize' => FALSE)) . '</div></noscript>'; -} - /** * Get the list of available date types and attributes. * diff --git a/modules/system/system.test b/modules/system/system.test index 18b9f8807d04..9ff7f42af1d4 100644 --- a/modules/system/system.test +++ b/modules/system/system.test @@ -415,47 +415,61 @@ class CronRunTestCase extends DrupalWebTestCase { } /** - * Follow every image paths in the previously retrieved content. - */ - function drupalGetAllImages() { - foreach ($this->xpath('//img') as $image) { - $this->drupalGet($this->getAbsoluteUrl($image['src'])); - } - } - - /** - * Ensure that the cron image callback to run it automatically is working. + * Ensure that the automatic cron run callback is working. * * In these tests we do not use REQUEST_TIME to track start time, because we * need the exact time when cron is triggered. */ - function testCronThreshold() { + function testAutomaticCron() { // Ensure cron does not run when the cron threshold is enabled and was // not passed. - $start_cron_last = time(); - variable_set('cron_last', $start_cron_last); - variable_set('cron_safe_threshold', 10); + $cron_last = time(); + $cron_safe_threshold = 100; + variable_set('cron_last', $cron_last); + variable_set('cron_safe_threshold', $cron_safe_threshold); $this->drupalGet(''); - // Follow every image path on the page. - $this->drupalGetAllImages(); - $this->assertTrue($start_cron_last == variable_get('cron_last', NULL), t('Cron does not run when the cron threshold is not passed.')); + $this->assertRaw('"cronCheck":' . ($cron_last + $cron_safe_threshold)); + $this->drupalGet('system/run-cron-check'); + $this->assertExpiresHeader($cron_last + $cron_safe_threshold); + $this->assertTrue($cron_last == variable_get('cron_last', NULL), t('Cron does not run when the cron threshold is not passed.')); // Test if cron runs when the cron threshold was passed. - $start_cron_last = time() - 15; - variable_set('cron_last', $start_cron_last); + $cron_last = time() - 200; + variable_set('cron_last', $cron_last); $this->drupalGet(''); - // Follow every image path on the page. - $this->drupalGetAllImages(); - $this->assertTrue(variable_get('cron_last', NULL) > $start_cron_last, t('Cron runs when the cron threshold is passed.')); - - // Test if cron does not run when the cron threshold was is disabled. - $start_cron_last = time() - 15; - variable_set('cron_safe_threshold', 0); - variable_set('cron_last', $start_cron_last); + $this->assertRaw('"cronCheck":' . ($cron_last + $cron_safe_threshold)); + sleep(1); + $this->drupalGet('system/run-cron-check'); + $this->assertExpiresHeader(variable_get('cron_last', NULL) + $cron_safe_threshold); + $this->assertTrue($cron_last < variable_get('cron_last', NULL), t('Cron runs when the cron threshold is passed.')); + + // Disable the cron threshold through the interface. + $admin_user = $this->drupalCreateUser(array('administer site configuration')); + $this->drupalLogin($admin_user); + $this->drupalPost('admin/config/system/site-information', array('cron_safe_threshold' => 0), t('Save configuration')); + $this->assertText(t('The configuration options have been saved.')); + $this->drupalLogout(); + + // Test if cron does not run when the cron threshold is disabled. + $cron_last = time() - 200; + variable_set('cron_last', $cron_last); $this->drupalGet(''); - // Follow every image path on the page. - $this->drupalGetAllImages(); - $this->assertTrue($start_cron_last == variable_get('cron_last', NULL), t('Cron does not run when the cron threshold is disabled.')); + $this->assertNoRaw('cronCheck'); + $this->drupalGet('system/run-cron-check'); + $this->assertResponse(403); + $this->assertTrue($cron_last == variable_get('cron_last', NULL), t('Cron does not run when the cron threshold is disabled.')); + } + + /** + * Assert that the Expires header is a specific timestamp. + * + * @param $timestamp + * The timestamp value to match against the header. + */ + private function assertExpiresHeader($timestamp) { + $expires = $this->drupalGetHeader('Expires'); + $expires = strtotime($expires); + $this->assertEqual($expires, $timestamp, t('Expires header expected @expected got @actual.', array('@expected' => $timestamp, '@actual' => $expires))); } /** -- GitLab