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.16
diff -u -p -r1.16 update.compare.inc
--- modules/update/update.compare.inc	22 Feb 2009 17:55:30 -0000	1.16
+++ modules/update/update.compare.inc	29 Apr 2009 00:35:03 -0000
@@ -29,8 +29,13 @@ 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', REQUEST_TIME + 3600);
+      // Set the projects array into the cache table. Since this is not the
+      // data we fetched about available updates, it is ok to invalidate it
+      // after an hour. We don't want this to persist too long, or if someone
+      // upgrades their version of a module, update status will be using stale
+      // data unless an admin visits one of the paths where this data is
+      // flushed (see update_project_cache()).
+      _update_cache_set('update_project_projects', $projects, REQUEST_TIME + 3600);
     }
   }
   return $projects;
@@ -550,8 +555,13 @@ 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', REQUEST_TIME + 3600);
+  // Set the projects array into the cache table. Since this is not the
+  // data we fetched about available updates, it is ok to invalidate it
+  // after an hour. We don't want this to persist too long, or if someone
+  // upgrades their version of a module, update status will be using stale
+  // data unless an admin visits one of the paths where this data is
+  // flushed (see update_project_cache()).
+  _update_cache_set('update_project_data', $projects, REQUEST_TIME + 3600);
   return $projects;
 }
 
@@ -567,6 +577,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'.
@@ -584,10 +601,10 @@ function update_project_cache($cid) {
   $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_invalidate_cache($cid);
   }
   else {
-    $cache = cache_get($cid, 'cache_update');
+    $cache = _update_cache_get($cid);
     if (!empty($cache->data) && $cache->expire > REQUEST_TIME) {
       $projects = $cache->data;
     }
Index: modules/update/update.fetch.inc
===================================================================
RCS file: /cvs/drupal/drupal/modules/update/update.fetch.inc,v
retrieving revision 1.16
diff -u -p -r1.16 update.fetch.inc
--- modules/update/update.fetch.inc	14 Mar 2009 23:01:37 -0000	1.16
+++ modules/update/update.fetch.inc	29 Apr 2009 00:35:03 -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_invalidate_cache('update_project_projects');
+  update_invalidate_cache('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_invalidate_cache('update_available_releases');
+  
   foreach ($projects as $key => $project) {
     $url = _update_build_fetch_url($project, $site_key);
     $xml = drupal_http_request($url);
@@ -51,7 +58,7 @@ function _update_refresh() {
   }
   if (!empty($available) && is_array($available)) {
     $frequency = variable_get('update_check_frequency', 1);
-    cache_set('update_info', $available, 'cache_update', REQUEST_TIME + (60 * 60 * 24 * $frequency));
+    _update_cache_set('update_available_releases', $available, REQUEST_TIME + (60 * 60 * 24 * $frequency));
     variable_set('update_last_check', REQUEST_TIME);
     watchdog('update', 'Fetched information about all available new releases and updates.', array(), WATCHDOG_NOTICE, l(t('view'), 'admin/reports/updates'));
   }
Index: modules/update/update.module
===================================================================
RCS file: /cvs/drupal/drupal/modules/update/update.module,v
retrieving revision 1.30
diff -u -p -r1.30 update.module
--- modules/update/update.module	22 Jan 2009 03:11:54 -0000	1.30
+++ modules/update/update.module	29 Apr 2009 00:35:03 -0000
@@ -277,9 +277,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') || ((REQUEST_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') || ((REQUEST_TIME - variable_get('update_last_check', 0)) > $interval)) {
     update_refresh();
     _update_cron_notify();
   }
@@ -354,8 +354,7 @@ function update_get_available($refresh =
       break;
     }
   }
-  if (!$needs_refresh && ($cache = cache_get('update_info', 'cache_update'))
-       && $cache->expire > REQUEST_TIME) {
+  if (!$needs_refresh && ($cache = _update_cache_get('update_available_releases')) && $cache->expire > REQUEST_TIME) {
     $available = $cache->data;
   }
   elseif ($needs_refresh || $refresh) {
@@ -368,24 +367,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() {
@@ -515,3 +496,113 @@ 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. When we relied on the core cache API, there were all sorts of
+ * potential problems that would result in attempting to fetch available
+ * update data all the time, including if a site sets a so-called "minimum
+ * cache lifetime" (which is both a minimum and a maximum), or if a site uses
+ * memcache.
+ *
+ * We continue to use 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 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 unless
+ *     explicitly told to using 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) {
+  $fields = array(
+    'created' => REQUEST_TIME,
+    'expire' => $expire,
+    'headers' => NULL,
+  );
+  if (!is_string($data)) {
+    $fields['data'] = serialize($data);
+    $fields['serialized'] = 1;
+  }
+  else {
+    $fields['data'] = $data;
+    $fields['serialized'] = 0;
+  }
+  db_merge('cache_update')
+    ->key(array('cid' => $cid))
+    ->fields($fields)
+    ->execute();
+}
+
+/** 
+ * 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_query("SELECT data, created, expire, serialized FROM {cache_update} WHERE cid = :cid", array(':cid' => $cid))->fetchObject();
+  if (isset($cache->data)) {
+    if ($cache->serialized) {
+      $cache->data = unserialize($cache->data);
+    }
+  }
+  return $cache;
+}
+
+/**
+ * Invalidates 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_invalidate_cache($cid = NULL) {
+  $query = db_delete('cache_update');
+  if (!empty($cid)) {
+    $query->condition('cid', $cid);
+  }
+  $query->execute();
+}
+
+/**
+ * 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.
+ * 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_invalidate_cache();
+  return array();
+}
+
+/**
+ * @} End of "defgroup update_status_cache".
+ */