cvs diff: Diffing modules/update
Index: modules/update/update.compare.inc
===================================================================
RCS file: /cvs/drupal/drupal/modules/update/update.compare.inc,v
retrieving revision 1.8.2.2
diff -u -p -r1.8.2.2 update.compare.inc
--- modules/update/update.compare.inc	28 Aug 2008 08:14:56 -0000	1.8.2.2
+++ modules/update/update.compare.inc	29 Apr 2009 10:20:25 -0000
@@ -16,9 +16,18 @@
  * that logic is only required when preparing the status report, not for
  * fetching the available release data.
  *
+ * This array is fairly expensive to construct, since it involves a lot of
+ * disk I/O, so we cache the results into the {cache_update} table using the
+ * 'update_project_projects' cache ID. However, since this is not the data
+ * about available updates fetched from the network, it is ok to invalidate it
+ * somewhat quickly. If we keep this data for very long, site administrators
+ * are more likely to see incorrect results if they upgrade to a newer version
+ * of a module or theme but do not visit certain pages that automatically
+ * clear this cache.
+ *
  * @see update_process_project_info()
  * @see update_calculate_project_data()
- *
+ * @see update_project_cache()
  */
 function update_get_projects() {
   static $projects = array();
@@ -29,8 +38,8 @@ function update_get_projects() {
       // Still empty, so we have to rebuild the cache.
       _update_process_info_list($projects, module_rebuild_cache(), 'module');
       _update_process_info_list($projects, system_theme_data(), 'theme');
-      // Set the projects array into the cache table.
-      cache_set('update_project_projects', $projects, 'cache_update', time() + 3600);
+      // Cache the site's project data for at most 1 hour.
+      _update_cache_set('update_project_projects', $projects, time() + 3600);
     }
   }
   return $projects;
@@ -223,12 +232,23 @@ function update_process_project_info(&$p
  * version (e.g. 5.x-1.5-beta1, 5.x-1.5-beta2, and 5.x-1.5). Development
  * snapshots for a given major version are always listed last.
  *
+ * The results of this function are expensive to compute, especially on sites
+ * with lots of modules or themes, since it involves a lot of comparisons and
+ * other operations. Therefore, we cache the results into the {cache_update}
+ * table using the 'update_project_data' cache ID. However, since this is not
+ * the data about available updates fetched from the network, it is ok to
+ * invalidate it somewhat quickly. If we keep this data for very long, site
+ * administrators are more likely to see incorrect results if they upgrade to
+ * a newer version of a module or theme but do not visit certain pages that
+ * automatically clear this cache.
+ *
  * @param $available
  *  Array of data about available project releases.
  *
  * @see update_get_available()
  * @see update_get_projects()
  * @see update_process_project_info()
+ * @see update_project_cache()
  */
 function update_calculate_project_data($available) {
   // Retrieve the projects from cache, if present.
@@ -444,7 +464,7 @@ function update_calculate_project_data($
 
         // If we're running a dev snapshot and have a timestamp, stop
         // searching for security updates once we hit an official release
-        // older than what we've got.  Allow 100 seconds of leeway to handle
+        // older than what we've got. Allow 100 seconds of leeway to handle
         // differences between the datestamp in the .info file and the
         // timestamp of the tarball itself (which are usually off by 1 or 2
         // seconds) so that we don't flag that as a new release.
@@ -550,8 +570,8 @@ function update_calculate_project_data($
   // projects or releases).
   drupal_alter('update_status', $projects);
 
-  // Set the projects array into the cache table.
-  cache_set('update_project_data', $projects, 'cache_update', time() + 3600);
+  // Cache the site's update status for at most 1 hour.
+  _update_cache_set('update_project_data', $projects, time() + 3600);
   return $projects;
 }
 
@@ -567,6 +587,13 @@ function update_calculate_project_data($
  * administration pages, since we should always recompute the most current
  * values on any of those pages.
  *
+ * Note: while both of these arrays are expensive to compute (in terms of disk
+ * I/O and some fairly heavy CPU processing), neither of these is the actual
+ * data about available updates that we have to fetch over the network from
+ * updates.drupal.org. That information is stored with the
+ * 'update_available_releases' cache ID -- it needs to persist longer than 1
+ * hour and never get invalidated just by visiting a page on the site.
+ *
  * @param $cid
  *   The cache id of data to return from the cache. Valid options are
  *   'update_project_data' and 'update_project_projects'.
@@ -579,15 +606,15 @@ function update_calculate_project_data($
 function update_project_cache($cid) {
   $projects = array();
 
-  // In some cases, we must clear the cache.  Rather than do so on a time
-  // basis, we check for specific paths.
+  // On certain paths, we should clear the cache and recompute the projects or
+  // update status of the site to avoid presenting stale information.
   $q = $_GET['q'];
   $paths = array('admin/build/modules', 'admin/build/themes', 'admin/reports', 'admin/reports/updates', 'admin/reports/status', 'admin/reports/updates/check');
   if (in_array($q, $paths)) {
-    cache_clear_all($cid, 'cache_update');
+    _update_cache_clear($cid);
   }
   else {
-    $cache = cache_get($cid, 'cache_update');
+    $cache = _update_cache_get($cid);
     if (!empty($cache->data) && $cache->expire > time()) {
       $projects = $cache->data;
     }
Index: modules/update/update.fetch.inc
===================================================================
RCS file: /cvs/drupal/drupal/modules/update/update.fetch.inc,v
retrieving revision 1.7.2.2
diff -u -p -r1.7.2.2 update.fetch.inc
--- modules/update/update.fetch.inc	14 Jan 2009 21:36:16 -0000	1.7.2.2
+++ modules/update/update.fetch.inc	29 Apr 2009 10:20:25 -0000
@@ -27,17 +27,24 @@ function _update_refresh() {
   module_load_include('inc', 'update', 'update.compare');
 
   // Since we're fetching new available update data, we want to clear
-  // everything in our cache, to ensure we recompute the status. Note that
-  // this does not cause update_get_projects() to be recomputed twice in the
-  // same page load (e.g. when manually checking) since that function stashes
-  // its answer in a static array.
-  update_invalidate_cache();
+  // our cache of both the projects we care about, and the current update
+  // status of the site. We do *not* want to clear the cache of available
+  // releases just yet, since that data (even if it's stale) can be useful
+  // during update_get_projects(); for example, to modules that implement
+  // hook_system_info_alter() such as cvs_deploy.
+  _update_cache_clear('update_project_projects');
+  _update_cache_clear('update_project_data');
 
   $available = array();
   $data = array();
   $site_key = md5($base_url . drupal_get_private_key());
   $projects = update_get_projects();
 
+  // Now that we have the list of projects, we should also clear our cache of
+  // available release data, since even if we fail to fetch new data, we need
+  // to clear out the stale data at this point.
+  _update_cache_clear('update_available_releases');
+  
   foreach ($projects as $key => $project) {
     $url = _update_build_fetch_url($project, $site_key);
     $xml = drupal_http_request($url);
@@ -52,7 +59,7 @@ function _update_refresh() {
   }
   if (!empty($available) && is_array($available)) {
     $frequency = variable_get('update_check_frequency', 1);
-    cache_set('update_info', $available, 'cache_update', time() + (60 * 60 * 24 * $frequency));
+    _update_cache_set('update_available_releases', $available, time() + (60 * 60 * 24 * $frequency));
     variable_set('update_last_check', time());
     watchdog('update', 'Fetched information about all available new releases and updates.', array(), WATCHDOG_NOTICE, l(t('view'), 'admin/reports/updates'));
   }
@@ -101,7 +108,7 @@ function _update_build_fetch_url($projec
  * Perform any notifications that should be done once cron fetches new data.
  *
  * This method checks the status of the site using the new data and depending
- * on the configuration of the site, notifys administrators via email if there
+ * on the configuration of the site, notifies administrators via email if there
  * are new releases or missing security updates.
  *
  * @see update_requirements()
Index: modules/update/update.module
===================================================================
RCS file: /cvs/drupal/drupal/modules/update/update.module,v
retrieving revision 1.17.2.1
diff -u -p -r1.17.2.1 update.module
--- modules/update/update.module	23 Sep 2008 10:19:02 -0000	1.17.2.1
+++ modules/update/update.module	29 Apr 2009 10:20:26 -0000
@@ -168,7 +168,7 @@ function update_theme() {
 }
 
 /**
- * Implementation of hook_requirements.
+ * Implementation of hook_requirements().
  *
  * @return
  *   An array describing the status of the site regarding available updates.
@@ -286,9 +286,9 @@ function _update_requirement_check($proj
 function update_cron() {
   $frequency = variable_get('update_check_frequency', 1);
   $interval = 60 * 60 * 24 * $frequency;
-  // Cron should check for updates if there is no update data cached or if the configured
-  // update interval has elapsed.
-  if (!cache_get('update_info', 'cache_update') || ((time() - variable_get('update_last_check', 0)) > $interval)) {
+  // Cron should check for updates if there is no update data cached or if the
+  // configured update interval has elapsed.
+  if (!_update_cache_get('update_available_releases') || ((time() - variable_get('update_last_check', 0)) > $interval)) {
     update_refresh();
     _update_cron_notify();
   }
@@ -353,8 +353,7 @@ function update_get_available($refresh =
       break;
     }
   }
-  if (!$needs_refresh && ($cache = cache_get('update_info', 'cache_update'))
-       && $cache->expire > time()) {
+  if (!$needs_refresh && ($cache = _update_cache_get('update_available_releases')) && $cache->expire > time()) {
     $available = $cache->data;
   }
   elseif ($needs_refresh || $refresh) {
@@ -367,24 +366,6 @@ function update_get_available($refresh =
 }
 
 /**
- * Implementation of hook_flush_caches().
- *
- * The function update.php (among others) calls this hook to flush the caches.
- * Since we're running update.php, we are likely to install a new version of
- * something, in which case, we want to check for available update data again.
- */
-function update_flush_caches() {
-  return array('cache_update');
-}
-
-/**
- * Invalidates any cached data relating to update status.
- */
-function update_invalidate_cache() {
-  cache_clear_all('*', 'cache_update', TRUE);
-}
-
-/**
  * Wrapper to load the include file and then refresh the release data.
  */
 function update_refresh() {
@@ -514,3 +495,115 @@ function _update_project_status_sort($a,
   $b_status = $b['status'] > 0 ? $b['status'] : (-10 * $b['status']);
   return $a_status - $b_status;
 }
+
+/**
+ * @defgroup update_status_cache Private update status cache system
+ * @{
+ *
+ * We specifically do NOT use the core cache API for saving the fetched data
+ * about available updates. It is vitally important that this cache is only
+ * cleared when we're populating it after successfully fetching new available
+ * update data. Usage of the core cache API results in all sorts of potential
+ * problems that would result in attempting to fetch available update data all
+ * the time, including if a site has a "minimum cache lifetime" (which is both
+ * a minimum and a maximum) defined, or if a site uses memcache or another
+ * plug-able cache system that assumes volatile caches.
+ *
+ * Update module still uses the {cache_update} table, but instead of using
+ * cache_set(), cache_get(), and cache_clear_all(), there are private helper
+ * functions that implement these same basic tasks but ensure that the cache
+ * is not prematurely cleared, and that the data is always stored in the
+ * database, even if memcache or another cache backend is in use.
+ */
+
+/**
+ * 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
+ *   The data to store.
+ * @param $expire
+ *   One of the following values:
+ *   - CACHE_PERMANENT: Indicates that the item should never be removed except
+ *     by explicitly using _update_cache_clear() or update_invalidate_cache().
+ *   - A Unix timestamp: Indicates that the item should be kept at least until
+ *     the given time, after which it will be invalidated.
+ */
+function _update_cache_set($cid, $data, $expire) {
+  $serialized = 0;
+  if (is_object($data) || is_array($data)) {
+    $data = serialize($data);
+    $serialized = 1;
+  }
+  $created = time();
+  db_query("UPDATE {cache_update} SET data = %b, created = %d, expire = %d, serialized = %d WHERE cid = '%s'", $data, $created, $expire, $serialized, $cid);
+  if (!db_affected_rows()) {
+    @db_query("INSERT INTO {cache_update} (cid, data, created, expire, serialized) VALUES ('%s', %b, %d, %d, %d)", $cid, $data, $created, $expire, $serialized);
+  }
+}
+
+/** 
+ * Retrieve data from the private update status cache table.
+ *
+ * @param $cid
+ *   The cache ID to retrieve.
+ * @return
+ *   The data for the given cache ID, or NULL if the ID was not found.
+ */
+function _update_cache_get($cid) {
+  $cache = db_fetch_object(db_query("SELECT data, created, expire, serialized FROM {cache_update} WHERE cid = '%s'", $cid));
+  if (isset($cache->data)) {
+    $cache->data = db_decode_blob($cache->data);
+    if ($cache->serialized) {
+      $cache->data = unserialize($cache->data);
+    }
+  }
+  return $cache;
+}
+
+/**
+ * Invalidates specific cached data relating to update status.
+ *
+ * @param $cid
+ *   Optional cache ID of the record to clear from the private update module
+ *   cache. If empty, all records will be cleared from the table.
+ */
+function _update_cache_clear($cid = NULL) {
+  if (empty($cid)) {
+    db_query("DELETE FROM {cache_update}");
+  }
+  else {
+    db_query("DELETE FROM {cache_update} WHERE cid = '%s'", $cid);
+  }
+}
+
+/**
+ * Implementation of hook_flush_caches().
+ *
+ * Called from update.php (among others) to flush the caches.
+ * Since we're running update.php, we are likely to install a new version of
+ * something, in which case, we want to check for available update data again.
+ * However, because we have our own caching system, we need to directly clear
+ * the database table ourselves at this point and return nothing, for example,
+ * on sites that use memcache where cache_clear_all() won't know how to purge
+ * this data.
+ */
+function update_flush_caches() {
+  _update_cache_clear();
+  return array();
+}
+
+/**
+ * Invalidates all cached data relating to update status.
+ */
+function update_invalidate_cache() {
+  _update_cache_clear();
+}
+
+/**
+ * @} End of "defgroup update_status_cache".
+ */