diff --git a/.travis.yml b/.travis.yml index da6cda2..60f0be2 100644 --- a/.travis.yml +++ b/.travis.yml @@ -24,6 +24,8 @@ env: # force composer 1.8+ to use a specific folder as home - COMPOSER_HOME="$HOME/.composer/" + - DRUPAL_TI_DRUSH_VERSION="drush/drush:^9" + # Configuration variables. - DRUPAL_TI_MODULE_NAME="redis" - DRUPAL_TI_SIMPLETEST_GROUP="redis" @@ -62,7 +64,7 @@ env: - DRUPAL_TI_BEHAT_BROWSER="firefox" # Set Drupal version in which to run tests. - - DRUPAL_TI_CORE_BRANCH="8.7.x" + - DRUPAL_TI_CORE_BRANCH="8.8.x" # PHPUnit specific commandline arguments. - DRUPAL_TI_PHPUNIT_ARGS="--verbose --debug" diff --git a/redis.info.yml b/redis.info.yml index c5dd1a5..b227ae2 100644 --- a/redis.info.yml +++ b/redis.info.yml @@ -2,5 +2,5 @@ name: Redis description: Provide a module placeholder, for using as dependency for module that needs Redis. package: Performance type: module -core: 8.x +core_version_requirement: ^8.8 || ^9 configure: redis.admin_display diff --git a/src/Cache/CacheBase.php b/src/Cache/CacheBase.php index 4f33184..8014a6b 100644 --- a/src/Cache/CacheBase.php +++ b/src/Cache/CacheBase.php @@ -96,6 +96,13 @@ abstract class CacheBase implements CacheBackendInterface { */ protected $lastDeleteAll = NULL; + /** + * Delayed deletions for deletions during a transaction. + * + * @var string[] + */ + protected $delayedDeletions = []; + /** * Get TTL for CACHE_PERMANENT items. * @@ -144,6 +151,49 @@ public function delete($cid) { $this->deleteMultiple([$cid]); } + /** + * {@inheritdoc} + */ + public function deleteMultiple(array $cids) { + $in_transaction = \Drupal::database()->inTransaction(); + if ($in_transaction) { + if (empty($this->delayedDeletions)) { + \Drupal::database()->addRootTransactionEndCallback([$this, 'postRootTransactionCommit']); + } + $this->delayedDeletions = array_unique(array_merge($this->delayedDeletions, $cids)); + } + else { + $this->doDeleteMultiple($cids); + } + } + + /** + * Execute the deletion. + * + * This can be delayed to avoid race conditions. + * + * @param array $cids + * An array of cache IDs to delete. + * + * @see static::deleteMultiple() + */ + protected abstract function doDeleteMultiple(array $cids); + + /** + * Callback to be invoked after a database transaction gets committed. + * + * Invalidates all delayed cache deletions. + * + * @param bool $success + * Whether or not the transaction was successful. + */ + public function postRootTransactionCommit($success) { + if ($success) { + $this->doDeleteMultiple($this->delayedDeletions); + } + $this->delayedDeletions = []; + } + /** * {@inheritdoc} */ @@ -251,6 +301,11 @@ protected function expandEntry(array $values, $allow_invalid) { return FALSE; } + // Ignore items that are scheduled for deletion. + if (in_array($values['cid'], $this->delayedDeletions)) { + return FALSE; + } + $cache = (object) $values; $cache->tags = explode(' ', $cache->tags); diff --git a/src/Cache/PhpRedis.php b/src/Cache/PhpRedis.php index 84ad791..df3c5e1 100644 --- a/src/Cache/PhpRedis.php +++ b/src/Cache/PhpRedis.php @@ -102,10 +102,9 @@ public function set($cid, $data, $expire = Cache::PERMANENT, array $tags = []) { /** * {@inheritdoc} */ - public function deleteMultiple(array $cids) { + public function doDeleteMultiple(array $cids) { $keys = array_map([$this, 'getKey'], $cids); $this->client->del($keys); } - } diff --git a/src/Cache/Predis.php b/src/Cache/Predis.php index 6ccd781..3e75e95 100644 --- a/src/Cache/Predis.php +++ b/src/Cache/Predis.php @@ -102,12 +102,11 @@ public function set($cid, $data, $expire = Cache::PERMANENT, array $tags = []) { /** * {@inheritdoc} */ - public function deleteMultiple(array $cids) { + public function doDeleteMultiple(array $cids) { if (!empty($cids)) { $keys = array_map([$this, 'getKey'], $cids); $this->client->del($keys); } } - } diff --git a/src/Cache/RedisCacheTagsChecksum.php b/src/Cache/RedisCacheTagsChecksum.php index 9992a2f..cdbae31 100644 --- a/src/Cache/RedisCacheTagsChecksum.php +++ b/src/Cache/RedisCacheTagsChecksum.php @@ -3,6 +3,7 @@ namespace Drupal\redis\Cache; use Drupal\Core\Cache\CacheTagsChecksumInterface; +use Drupal\Core\Cache\CacheTagsChecksumTrait; use Drupal\Core\Cache\CacheTagsInvalidatorInterface; use Drupal\redis\ClientFactory; use Drupal\redis\RedisPrefixTrait; @@ -13,6 +14,7 @@ class RedisCacheTagsChecksum implements CacheTagsChecksumInterface, CacheTagsInvalidatorInterface { use RedisPrefixTrait; + use CacheTagsChecksumTrait; /** * Contains already loaded cache invalidations from the database. @@ -51,87 +53,36 @@ public function __construct(ClientFactory $factory) { /** * {@inheritdoc} */ - public function invalidateTags(array $tags) { - $keys_to_increment = []; - foreach ($tags as $tag) { - // Only invalidate tags once per request unless they are written again. - if (isset($this->invalidatedTags[$tag])) { - continue; + public function doInvalidateTags(array $tags) { + $keys = array_map([$this, 'getTagKey'], $tags); + + // We want to differentiate between PhpRedis and Redis clients. + if ($this->clientType === 'PhpRedis') { + $multi = $this->client->multi(); + foreach ($keys as $key) { + $multi->incr($key); } - $this->invalidatedTags[$tag] = TRUE; - unset($this->tagCache[$tag]); - $keys_to_increment[] = $this->getTagKey($tag); + $multi->exec(); } - if ($keys_to_increment) { - - // We want to differentiate between PhpRedis and Redis clients. - if ($this->clientType === 'PhpRedis') { - $multi = $this->client->multi(\Redis::PIPELINE); - foreach ($keys_to_increment as $key) { - $multi->incr($key); - } - $multi->exec(); - } - elseif ($this->clientType === 'Predis') { - - $pipe = $this->client->pipeline(); - foreach ($keys_to_increment as $key) { - $pipe->incr($key); - } - $pipe->execute(); - } - } - - } - - /** - * {@inheritdoc} - */ - public function getCurrentChecksum(array $tags) { - // Remove tags that were already invalidated during this request from the - // static caches so that another invalidation can occur later in the same - // request. Without that, written cache items would not be invalidated - // correctly. - foreach ($tags as $tag) { - unset($this->invalidatedTags[$tag]); - } - return $this->calculateChecksum($tags); - } - - /** - * {@inheritdoc} - */ - public function isValid($checksum, array $tags) { - return $checksum == $this->calculateChecksum($tags); - } + elseif ($this->clientType === 'Predis') { - /** - * {@inheritdoc} - */ - public function calculateChecksum(array $tags) { - $checksum = 0; - - $fetch = array_values(array_diff($tags, array_keys($this->tagCache))); - if ($fetch) { - $keys = array_map([$this, 'getTagKey'], $fetch); - foreach ($this->client->mget($keys) as $index => $invalidations) { - $this->tagCache[$fetch[$index]] = $invalidations ?: 0; + $pipe = $this->client->pipeline(); + foreach ($keys as $key) { + $pipe->incr($key); } + $pipe->execute(); } - - foreach ($tags as $tag) { - $checksum += $this->tagCache[$tag]; - } - - return $checksum; } /** * {@inheritdoc} */ - public function reset() { - $this->tagCache = []; - $this->invalidatedTags = []; + protected function getTagInvalidationCounts(array $tags) { + $keys = array_map([$this, 'getTagKey'], $tags); + // The mget command returns the values as an array with numeric keys, + // combine it with the tags array to get the expected return value and run + // it through intval() to convert to integers and FALSE to 0. + return array_map('intval', array_combine($tags, $this->client->mget($keys))); } /** @@ -147,4 +98,13 @@ protected function getTagKey($tag) { return $this->getPrefix() . ':cachetags:' . $tag; } + /** + * {@inheritdoc} + */ + protected function getDatabaseConnection() { + // This is not injected to avoid a dependency on the database in the + // critical path. It is only needed during cache tag invalidations. + return \Drupal::database(); + } + } diff --git a/tests/src/Functional/WebTest.php b/tests/src/Functional/WebTest.php index 2be7314..f608e62 100644 --- a/tests/src/Functional/WebTest.php +++ b/tests/src/Functional/WebTest.php @@ -26,6 +26,11 @@ class WebTest extends BrowserTestBase { */ public static $modules = ['redis', 'block']; + /** + * {@inheritdoc} + */ + protected $defaultTheme = 'classy'; + /** * {@inheritdoc} */