Issue #1547008 by Berdir, Sutharsan: Replace Update's cache system with the...

Issue #1547008 by Berdir, Sutharsan: Replace Update's cache system with the (expirable) key value store.

core/includes/update.inc

@@ -642,6 +642,44 @@ function update_fix_d8_requirements() {
     // picture upgrade path.
     update_module_enable(array('file'));
 
+    $schema = array(
+      'description' => 'Generic key/value storage table with an expiration.',
+      'fields' => array(
+        'collection' => array(
+          'description' => 'A named collection of key and value pairs.',
+          'type' => 'varchar',
+          'length' => 128,
+          'not null' => TRUE,
+          'default' => '',
+        ),
+        'name' => array(
+          // KEY is an SQL reserved word, so use 'name' as the key's field name.
+          'description' => 'The key of the key/value pair.',
+          'type' => 'varchar',
+          'length' => 128,
+          'not null' => TRUE,
+          'default' => '',
+        ),
+        'value' => array(
+          'description' => 'The value of the key/value pair.',
+          'type' => 'blob',
+          'not null' => TRUE,
+          'size' => 'big',
+        ),
+        'expire' => array(
+          'description' => 'The time since Unix epoch in seconds when this item expires. Defaults to the maximum possible time.',
+          'type' => 'int',
+          'not null' => TRUE,
+          'default' => 2147483647,
+        ),
+      ),
+      'primary key' => array('collection', 'name'),
+      'indexes' => array(
+        'all' => array('name', 'collection', 'expire'),
+      ),
+    );
+    db_create_table('key_value_expire', $schema);
+
     update_variable_set('update_d8_requirements', TRUE);
   }
 }

core/lib/Drupal.php

@@ -131,6 +131,19 @@ public static function database() {
     return static::$container->get('database');
   }
 
+  /**
+   * Returns an expirable key value store collection.
+   *
+   * @param string $collection
+   *   The name of the collection holding key and value pairs.
+   *
+   * @return \Drupal\Core\KeyValueStore\KeyValueStoreExpirableInterface
+   *   An expirable key value store collection.
+   */
+  public static function keyValueExpirable($collection) {
+    return static::$container->get('keyvalue.expirable')->get($collection);
+  }
+
   /**
    * Returns the locking layer instance.
    *

core/lib/Drupal/Core/CoreBundle.php

@@ -105,6 +105,14 @@ public function build(ContainerBuilder $container) {
     $container
       ->register('keyvalue.database', 'Drupal\Core\KeyValueStore\KeyValueDatabaseFactory')
       ->addArgument(new Reference('database'));
+    // Register the KeyValueStoreExpirable factory.
+    $container
+      ->register('keyvalue.expirable', 'Drupal\Core\KeyValueStore\KeyValueExpirableFactory')
+      ->addArgument(new Reference('service_container'));
+    $container
+      ->register('keyvalue.expirable.database', 'Drupal\Core\KeyValueStore\KeyValueDatabaseExpirableFactory')
+      ->addArgument(new Reference('database'))
+      ->addTag('needs_destruction');
 
     $container->register('settings', 'Drupal\Component\Utility\Settings')
       ->setFactoryClass('Drupal\Component\Utility\Settings')

core/lib/Drupal/Core/KeyValueStore/DatabaseStorage.php

@@ -125,4 +125,12 @@ public function deleteMultiple(array $keys) {
     while (count($keys));
   }
 
+  /**
+   * Implements Drupal\Core\KeyValueStore\KeyValueStoreInterface::deleteAll().
+   */
+  public function deleteAll() {
+    $this->connection->delete($this->table)
+      ->condition('collection', $this->collection)
+      ->execute();
+  }
 }

core/lib/Drupal/Core/KeyValueStore/DatabaseStorageExpirable.php

@@ -7,6 +7,7 @@
 
 namespace Drupal\Core\KeyValueStore;
 
+use Drupal\Core\DestructableInterface;
 use Drupal\Core\Database\Connection;
 use Drupal\Core\Database\Query\Merge;
 
@@ -16,7 +17,7 @@
  * This key/value store implementation uses the database to store key/value
  * data with an expire date.
  */
-class DatabaseStorageExpirable extends DatabaseStorage implements KeyValueStoreExpirableInterface {
+class DatabaseStorageExpirable extends DatabaseStorage implements KeyValueStoreExpirableInterface, DestructableInterface {
 
   /**
    * The connection object for this storage.
@@ -55,15 +56,6 @@ public function __construct($collection, Connection $connection, $table = 'key_v
     parent::__construct($collection, $connection, $table);
   }
 
-  /**
-   * Performs garbage collection as needed when destructing the storage object.
-   */
-  public function __destruct() {
-    if ($this->needsGarbageCollection) {
-      $this->garbageCollection();
-    }
-  }
-
   /**
    * Implements Drupal\Core\KeyValueStore\KeyValueStoreInterface::getMultiple().
    */
@@ -158,4 +150,13 @@ protected function garbageCollection() {
       ->execute();
   }
 
+  /**
+   * Implements Drupal\Core\DestructableInterface::destruct().
+   */
+  public function destruct() {
+    if ($this->needsGarbageCollection) {
+      $this->garbageCollection();
+    }
+  }
+
 }

core/lib/Drupal/Core/KeyValueStore/KeyValueDatabaseExpirableFactory.php

@@ -7,6 +7,7 @@
 
 namespace Drupal\Core\KeyValueStore;
 
+use Drupal\Core\DestructableInterface;
 use Drupal\Core\Database\Connection;
 use Drupal\Core\Database\Database;
 use Drupal\Core\KeyValueStore\KeyValueDatabaseFactory;
@@ -14,7 +15,14 @@
 /**
  * Defines the key/value store factory for the database backend.
  */
-class KeyValueDatabaseExpirableFactory extends KeyValueDatabaseFactory {
+class KeyValueDatabaseExpirableFactory extends KeyValueDatabaseFactory implements DestructableInterface {
+
+  /**
+   * Holds references to each instantiation so they can be terminated.
+   *
+   * @var array
+   */
+  protected $storages;
 
   /**
    * Constructs a new key/value expirable database storage object for a given
@@ -28,6 +36,17 @@ class KeyValueDatabaseExpirableFactory extends KeyValueDatabaseFactory {
    *   A key/value store implementation for the given $collection.
    */
   public function get($collection) {
-    return new DatabaseStorageExpirable($collection, $this->connection);
+    $storage = new DatabaseStorageExpirable($collection, $this->connection);
+    $this->storages[] = $storage;
+    return $storage;
+  }
+
+  /**
+   * Implements Drupal\Core\DestructableInterface::terminate().
+   */
+  public function destruct() {
+    foreach ($this->storages as $storage) {
+      $storage->destruct();
+    }
   }
 }

core/lib/Drupal/Core/KeyValueStore/KeyValueStoreInterface.php

@@ -99,4 +99,9 @@ public function delete($key);
    */
   public function deleteMultiple(array $keys);
 
+  /**
+   * Deletes all items from the key/value store.
+   */
+  public function deleteAll();
+
 }

core/lib/Drupal/Core/KeyValueStore/MemoryStorage.php

@@ -81,4 +81,10 @@ public function deleteMultiple(array $keys) {
     }
   }
 
+  /**
+   * Implements Drupal\Core\KeyValueStore\KeyValueStoreInterface::deleteAll().
+   */
+  public function deleteAll() {
+    $this->data = array();
+  }
 }

core/modules/system/lib/Drupal/system/Tests/KeyValueStore/GarbageCollectionTest.php

@@ -62,10 +62,10 @@ public function testGarbageCollection() {
         ->execute();
     }
 
-    // Perform a new set operation and then manually unset the object to
+    // Perform a new set operation and then manually destruct the object to
     // trigger garbage collection.
     $store->setWithExpire('autumn', 'winter', rand(500, 1000000));
-    unset($store);
+    $store->destruct();
 
     // Query the database and confirm that the stale records were deleted.
     $result = db_query(

core/modules/system/system.install

@@ -1735,44 +1735,7 @@ function system_update_8022() {
  * Create the 'key_value_expire' table.
  */
 function system_update_8023() {
-  $table = array(
-    'description' => 'Generic key/value storage table with an expiration.',
-    'fields' => array(
-      'collection' => array(
-        'description' => 'A named collection of key and value pairs.',
-        'type' => 'varchar',
-        'length' => 128,
-        'not null' => TRUE,
-        'default' => '',
-      ),
-      'name' => array(
-        // KEY is an SQL reserved word, so use 'name' as the key's field name.
-        'description' => 'The key of the key/value pair.',
-        'type' => 'varchar',
-        'length' => 128,
-        'not null' => TRUE,
-        'default' => '',
-      ),
-      'value' => array(
-        'description' => 'The value of the key/value pair.',
-        'type' => 'blob',
-        'not null' => TRUE,
-        'size' => 'big',
-      ),
-      'expire' => array(
-        'description' => 'The time since Unix epoch in seconds when this item expires. Defaults to the maximum possible time.',
-        'type' => 'int',
-        'not null' => TRUE,
-        'default' => 2147483647,
-      ),
-    ),
-    'primary key' => array('collection', 'name'),
-    'indexes' => array(
-      'all' => array('name', 'collection', 'expire'),
-    ),
-  );
-
-  db_create_table('key_value_expire', $table);
+  // Moved to update_fix_d8_requirements() as it is required early.
 }
 
 /**

core/modules/system/theme.api.php

@@ -234,5 +234,5 @@ function hook_themes_enabled($theme_list) {
  */
 function hook_themes_disabled($theme_list) {
  // Clear all update module caches.
-  _update_cache_clear();
+  update_storage_clear();
 }

core/modules/update/lib/Drupal/update/Tests/UpdateCoreTest.php

@@ -211,8 +211,8 @@ function testFetchTasks() {
     update_create_fetch_task($projecta);
     $this->assertEqual($queue->numberOfItems(), 2, 'Queue still contains two items');
 
-    // Clear cache and try again.
-    _update_cache_clear();
+    // Clear storage and try again.
+    update_storage_clear();
     drupal_static_reset('_update_create_fetch_task');
     update_create_fetch_task($projecta);
     $this->assertEqual($queue->numberOfItems(), 2, 'Queue contains two items');

core/modules/update/lib/Drupal/update/UpdateSettingsForm.php

@@ -106,9 +106,9 @@ public function validateForm(array &$form, array &$form_state) {
   public function submitForm(array &$form, array &$form_state) {
     $config = $this->configFactory->get('update.settings');
      // See if the update_check_disabled setting is being changed, and if so,
-    // invalidate all cached update status data.
+    // invalidate all update status data.
     if ($form_state['values']['update_check_disabled'] != $config->get('check.disabled_extensions')) {
-      _update_cache_clear();
+      update_storage_clear();
     }
 
     $config

core/modules/update/update.authorize.inc

@@ -178,7 +178,7 @@ function update_authorize_batch_copy_project($project, $updater_name, $local_url
  *
  * This processes the results and stashes them into SESSION such that
  * authorize.php will render a report. Also responsible for putting the site
- * back online and clearing the update status cache after a successful update.
+ * back online and clearing the update status storage after a successful update.
  *
  * @param $success
  *   TRUE if the batch operation was successful; FALSE if there were errors.
@@ -193,8 +193,8 @@ function update_authorize_update_batch_finished($success, $results) {
   }
   $offline = config('system.maintenance')->get('enabled');
   if ($success) {
-    // Now that the update completed, we need to clear the cache of available
-    // update data and recompute our status, so prevent show bogus results.
+    // Now that the update completed, we need to clear the available update data
+    // and recompute our status, so prevent show bogus results.
     _update_authorize_clear_update_status();
 
     // Take the site out of maintenance mode if it was previously that way.
@@ -315,25 +315,19 @@ function _update_batch_create_message(&$project_results, $message, $success = TR
 }
 
 /**
- * Clears cached available update status data.
+ * Clears available update status data.
  *
  * Since this function is run at such a low bootstrap level, the Update Manager
- * module is not loaded. So, we can't just call _update_cache_clear(). However,
- * the database is bootstrapped, so we can do a query ourselves to clear out
- * what we want to clear.
+ * module is not loaded. So, we can't just call update_storage_clear(). However,
+ * the key-value backend is available, so we just call that.
  *
- * Note that we do not want to just truncate the table, since that would remove
- * items related to currently pending fetch attempts.
+ * Note that we do not want to delete  items related to currently pending fetch
+ * attempts.
  *
  * @see update_authorize_update_batch_finished()
- * @see _update_cache_clear()
+ * @see update_storage_clear()
  */
 function _update_authorize_clear_update_status() {
-  $query = db_delete('cache_update');
-  $query->condition(
-    db_or()
-    ->condition('cid', 'update_project_%', 'LIKE')
-    ->condition('cid', 'available_releases::%', 'LIKE')
-  );
-  $query->execute();
+  Drupal::keyValueExpirable('update')->deleteAll();
+  Drupal::keyValueExpirable('update_available_release')->deleteAll();
 }

core/modules/update/update.compare.inc

@@ -16,13 +16,12 @@
  * 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
+ * I/O, so we store the results. However, since this is not the data about
  * available updates fetched from the network, it is acceptable 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.
+ * data.
  *
  * @return
  *   An associative array of currently enabled projects keyed by the
@@ -51,15 +50,15 @@
  *
  * @see update_process_project_info()
  * @see update_calculate_project_data()
- * @see update_project_cache()
+ * @see update_project_storage()
  */
 function update_get_projects() {
   $projects = &drupal_static(__FUNCTION__, array());
   if (empty($projects)) {
-    // Retrieve the projects from cache, if present.
-    $projects = update_project_cache('update_project_projects');
+    // Retrieve the projects from storage, if present.
+    $projects = update_project_storage('update_project_projects');
     if (empty($projects)) {
-      // Still empty, so we have to rebuild the cache.
+      // Still empty, so we have to rebuild.
       $module_data = system_rebuild_module_data();
       $theme_data = system_rebuild_theme_data();
       update_process_info_list($projects, $module_data, 'module', TRUE);
@@ -70,8 +69,8 @@ function update_get_projects() {
       }
       // Allow other modules to alter projects before fetching and comparing.
       drupal_alter('update_projects', $projects);
-      // Cache the site's project data for at most 1 hour.
-      _update_cache_set('update_project_projects', $projects, REQUEST_TIME + 3600);
+      // Store the site's project data for at most 1 hour.
+      Drupal::keyValueExpirable('update')->setWithExpire('update_project_projects', $projects, 3600);
     }
   }
   return $projects;
@@ -90,7 +89,7 @@ function update_get_projects() {
  * 'Hidden' themes are ignored only if they have no enabled sub-themes.
  * This function also records the latest change time on the .info.yml
  * files for each module or theme, which is important data which is used when
- * deciding if the cached available update data should be invalidated.
+ * deciding if the available update data should be invalidated.
  *
  * @param $projects
  *   Reference to the array of project data of what's installed on this site.
@@ -318,13 +317,12 @@ function update_process_project_info(&$projects) {
  *
  * 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
+ * other operations. Therefore, we store the results. 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.
+ * automatically clear this.
  *
  * @param array $available
  *   Data about available project releases.
@@ -335,13 +333,13 @@ function update_process_project_info(&$projects) {
  * @see update_get_available()
  * @see update_get_projects()
  * @see update_process_project_info()
- * @see update_project_cache()
+ * @see update_project_storage()
  */
 function update_calculate_project_data($available) {
-  // Retrieve the projects from cache, if present.
-  $projects = update_project_cache('update_project_data');
-  // If $projects is empty, then the cache must be rebuilt.
-  // Otherwise, return the cached data and skip the rest of the function.
+  // Retrieve the projects from storage, if present.
+  $projects = update_project_storage('update_project_data');
+  // If $projects is empty, then the data must be rebuilt.
+  // Otherwise, return the data and skip the rest of the function.
   if (!empty($projects)) {
     return $projects;
   }
@@ -361,8 +359,8 @@ function update_calculate_project_data($available) {
   // projects or releases).
   drupal_alter('update_status', $projects);
 
-  // Cache the site's update status for at most 1 hour.
-  _update_cache_set('update_project_data', $projects, REQUEST_TIME + 3600);
+  // Store the site's update status for at most 1 hour.
+  Drupal::keyValueExpirable('update')->setWithExpire('update_project_data', $projects, 3600);
   return $projects;
 }
 
@@ -752,37 +750,36 @@ function update_calculate_project_update_status(&$project_data, $available) {
 }
 
 /**
- * Retrieves data from {cache_update} or empties the cache when necessary.
+ * Retrieves update storage data or empties it.
  *
  * Two very expensive arrays computed by this module are the list of all
- * installed modules and themes (and .info.yml data, project associations, etc),
- * and the current status of the site relative to the currently available
- * releases. These two arrays are cached in the {cache_update} table and used
- * whenever possible. The cache is cleared whenever the administrator visits the
- * status report, available updates report, or the module or theme
- * administration pages, since we should always recompute the most current
- * values on any of those pages.
+ * installed modules and themes (and .info.yml data, project associations, etc), and
+ * the current status of the site relative to the currently available releases.
+ * These two arrays are stored and used whenever possible. The data is cleared
+ * whenever the administrator visits the status report, available updates
+ * report, or the module or theme 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
+ * updates.drupal.org. That information is stored in the
+ * 'update_available_releases' collection -- 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'.
+ * @param $key
+ *   The key of data to return. Valid options are 'update_project_data' and
+ *   'update_project_projects'.
  *
  * @return
- *   The cached value of the $projects array generated by
+ *   The stored value of the $projects array generated by
  *   update_calculate_project_data() or update_get_projects(), or an empty array
- *   when the cache is cleared.
+ *   when the storage is cleared.
  */
-function update_project_cache($cid) {
+function update_project_storage($key) {
   $projects = array();
 
-  // On certain paths, we should clear the cache and recompute the projects for
+  // On certain paths, we should clear the data and recompute the projects for
   // update status of the site to avoid presenting stale information.
   $paths = array(
     'admin/modules',
@@ -796,13 +793,10 @@ function update_project_cache($cid) {
     'admin/reports/updates/check',
   );
   if (in_array(current_path(), $paths)) {
-    _update_cache_clear($cid);
+    Drupal::keyValueExpirable('update')->delete($key);
   }
   else {
-    $cache = _update_cache_get($cid);
-    if (!empty($cache->data) && $cache->expire > REQUEST_TIME) {
-      $projects = $cache->data;
-    }
+    $projects = Drupal::keyValueExpirable('update')->get($key);
   }
   return $projects;
 }

core/modules/update/update.fetch.inc

@@ -117,8 +117,8 @@ function _update_fetch_data() {
 /**
  * Processes a task to fetch available update data for a single project.
  *
- * Once the release history XML data is downloaded, it is parsed and saved into
- * the {cache_update} table in an entry just for that project.
+ * Once the release history XML data is downloaded, it is parsed and saved in an
+ * entry just for that project.
  *
  * @param $project
  *   Associative array of information about the project to fetch data for.
@@ -132,13 +132,11 @@ function _update_process_fetch_task($project) {
   $fail = &drupal_static(__FUNCTION__, array());
   // This can be in the middle of a long-running batch, so REQUEST_TIME won't
   // necessarily be valid.
-  $now = time();
+  $request_time_difference = time() - REQUEST_TIME;
   if (empty($fail)) {
     // If we have valid data about release history XML servers that we have
-    // failed to fetch from on previous attempts, load that from the cache.
-    if (($cache = _update_cache_get('fetch_failures')) && ($cache->expire > $now)) {
-      $fail = $cache->data;
-    }
+    // failed to fetch from on previous attempts, load that.
+    $fail = Drupal::keyValueExpirable('update')->get('fetch_failures');
   }
 
   $max_fetch_attempts = $update_config->get('fetch.max_attempts');
@@ -179,43 +177,43 @@ function _update_process_fetch_task($project) {
   }
 
   $frequency = $update_config->get('check.interval_days');
-  $cid = 'available_releases::' . $project_name;
-  _update_cache_set($cid, $available, $now + (60 * 60 * 24 * $frequency));
+  $available['last_fetch'] = REQUEST_TIME + $request_time_difference;
+  Drupal::keyValueExpirable('update_available_releases')->setWithExpire($project_name, $available, $request_time_difference + (60 * 60 * 24 * $frequency));
 
   // Stash the $fail data back in the DB for the next 5 minutes.
-  _update_cache_set('fetch_failures', $fail, $now + (60 * 5));
+  Drupal::keyValueExpirable('update')->setWithExpire('fetch_failures', $fail, $request_time_difference + (60 * 5));
 
   // Whether this worked or not, we did just (try to) check for updates.
-  state()->set('update.last_check', $now);
+  state()->set('update.last_check', REQUEST_TIME + $request_time_difference);
 
   // Now that we processed the fetch task for this project, clear out the
-  // record in {cache_update} for this task so we're willing to fetch again.
-  _update_cache_clear('fetch_task::' . $project_name);
+  // record for this task so we're willing to fetch again.
+  drupal_container()->get('keyvalue')->get('update_fetch_task')->delete($project_name);
 
   return $success;
 }
 
 /**
- * Clears out all the cached available update data and initiates re-fetching.
+ * Clears out all the available update data and initiates re-fetching.
  */
 function _update_refresh() {
   module_load_include('inc', 'update', 'update.compare');
 
   // Since we're fetching new available update data, we want to clear
-  // 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
+  // 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');
+  Drupal::keyValueExpirable('update')->delete('update_project_projects');
+  Drupal::keyValueExpirable('update')->delete('update_project_data');
 
   $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('available_releases::', TRUE);
+  // Now that we have the list of projects, we should also clear the available
+  // release data, since even if we fail to fetch new data, we need to clear
+  // out the stale data at this point.
+  Drupal::keyValueExpirable('update_available_releases')->deleteAll();