Issue #1191290 by klausi, David_Rothstein, Fabianx: system_cron() should not invoke hook_flush_caches() on every cron run

4 files changed, 58 insertions, 2 deletions

diff --git a/modules/system/system.module b/modules/system/system.module
index 39de758ed..362bdd445 100644
--- a/modules/system/system.module
+++ b/modules/system/system.module
@@ -3056,8 +3056,20 @@ function system_cron() {
     }
   }
 
-  $core = array('cache', 'cache_path', 'cache_filter', 'cache_page', 'cache_form', 'cache_menu');
-  $cache_tables = array_merge(module_invoke_all('flush_caches'), $core);
+  // Delete expired cache entries.
+  // Avoid invoking hook_flush_cashes() on every cron run because some modules
+  // use this hook to perform expensive rebuilding operations (which are only
+  // designed to happen on full cache clears), rather than just returning a
+  // list of cache tables to be cleared.
+  $cache_object = cache_get('system_cache_tables');
+  if (empty($cache_object)) {
+    $core = array('cache', 'cache_path', 'cache_filter', 'cache_page', 'cache_form', 'cache_menu');
+    $cache_tables = array_merge(module_invoke_all('flush_caches'), $core);
+    cache_set('system_cache_tables', $cache_tables);
+  }
+  else {
+    $cache_tables = $cache_object->data;
+  }
   foreach ($cache_tables as $table) {
     cache_clear_all(NULL, $table);
   }
diff --git a/modules/system/system.test b/modules/system/system.test
index 2865bbb25..bc764dde5 100644
--- a/modules/system/system.test
+++ b/modules/system/system.test
@@ -905,6 +905,29 @@ class CronRunTestCase extends DrupalWebTestCase {
     $result = variable_get('common_test_cron');
     $this->assertEqual($result, 'success', 'Cron correctly handles exceptions thrown during hook_cron() invocations.');
   }
+
+  /**
+   * Tests that hook_flush_caches() is not invoked on every single cron run.
+   *
+   * @see system_cron()
+   */
+  public function testCronCacheExpiration() {
+    module_enable(array('system_cron_test'));
+    variable_del('system_cron_test_flush_caches');
+
+    // Invoke cron the first time: hook_flush_caches() should be called and then
+    // get cached.
+    drupal_cron_run();
+    $this->assertEqual(variable_get('system_cron_test_flush_caches'), 1, 'hook_flush_caches() was invoked the first time.');
+    $cache = cache_get('system_cache_tables');
+    $this->assertEqual(empty($cache), FALSE, 'Cache is filled with cache table data.');
+
+    // Run cron again and ensure that hook_flush_caches() is not called.
+    variable_del('system_cron_test_flush_caches');
+    drupal_cron_run();
+    $this->assertNull(variable_get('system_cron_test_flush_caches'), 'hook_flush_caches() was not invoked the second time.');
+  }
+
 }
 
 /**
diff --git a/modules/system/tests/system_cron_test.info b/modules/system/tests/system_cron_test.info
new file mode 100644
index 000000000..3d171ccc4
--- /dev/null
+++ b/modules/system/tests/system_cron_test.info
@@ -0,0 +1,6 @@
+name = System Cron Test
+description = 'Support module for testing the system_cron().'
+package = Testing
+version = VERSION
+core = 7.x
+hidden = TRUE
diff --git a/modules/system/tests/system_cron_test.module b/modules/system/tests/system_cron_test.module
new file mode 100644
index 000000000..9ef80e231
--- /dev/null
+++ b/modules/system/tests/system_cron_test.module
@@ -0,0 +1,15 @@
+<?php
+
+/**
+ * @file
+ * Helper module for CronRunTestCase::testCronCacheExpiration().
+ */
+
+/**
+ * Implements hook_flush_caches().
+ */
+function system_cron_test_flush_caches() {
+  // Set a variable to indicate that this hook was invoked.
+  variable_set('system_cron_test_flush_caches', 1);
+  return array();
+}