Fix expiry calculation

composer.json

@@ -1,10 +1,11 @@
 {
   "name": "drupal/redis",
   "type": "drupal-module",
+  "description": "Integration of Drupal with the Redis key-value store.",
   "suggest": {
     "predis/predis": "^1.1.1"
   },
-  "license": "GPL-2.0",
+  "license": "GPL-2.0-or-later",
   "autoload": {
     "psr-4": {
       "Drupal\\redis\\": "src"

redis.info.yml

@@ -4,3 +4,4 @@ package: Performance
 type: module
 core_version_requirement: ^8.8 || ^9
 configure: redis.admin_display
+php: 7.1.0

src/Cache/CacheBase.php

@@ -2,7 +2,7 @@
 
 namespace Drupal\redis\Cache;
 
-use \DateInterval;
+use DateInterval;
 use Drupal\Component\Assertion\Inspector;
 use Drupal\Component\Serialization\SerializationInterface;
 use Drupal\Core\Cache\Cache;
@@ -221,21 +221,27 @@ public function getKey($cid = NULL) {
   }
 
   /**
-   * Calculate the correct expiration time.
+   * Calculate the correct ttl value for redis.
    *
    * @param int $expire
    *   The expiration time provided for the cache set.
    *
    * @return int
-   *   The default expiration if expire is PERMANENT or higher than the default.
-   *   May return negative values if the item is already expired.
+   *   The default TTL if expire is PERMANENT or higher than the default.
+   *   Otherwise, the adjusted lifetime of the cache. May return negative values
+   *   if the item is already expired.
    */
   protected function getExpiration($expire) {
-    if ($expire == Cache::PERMANENT || $expire > $this->permTtl) {
+    if ($expire == Cache::PERMANENT) {
       return $this->permTtl;
     }
-    return $expire - \Drupal::time()->getRequestTime();
+    $expire_ttl = $expire - \Drupal::time()->getRequestTime();
+    if ($expire_ttl > $this->permTtl) {
+      return $this->permTtl;
+    }
+    return $expire_ttl;
   }
+
   /**
    * Return the key for the tag used to specify the bin of cache-entries.
    */
@@ -267,7 +273,7 @@ public function setPermTtl($ttl = NULL) {
         else {
           if ($iv = DateInterval::createFromDateString($ttl)) {
             // http://stackoverflow.com/questions/14277611/convert-dateinterval-object-to-seconds-in-php
-            $this->permTtl = ($iv->y * 31536000 + $iv->m * 2592000 + $iv->days * 86400 + $iv->h * 3600 + $iv->i * 60 + $iv->s);
+            $this->permTtl = ($iv->y * 31536000 + $iv->m * 2592000 + $iv->d * 86400 + $iv->h * 3600 + $iv->i * 60 + $iv->s);
           }
           else {
             // Log error about invalid ttl.

src/Cache/RedisCacheTagsChecksum.php

@@ -82,7 +82,8 @@ protected function getTagInvalidationCounts(array $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)));
+    $values = $this->client->mget($keys);
+    return $values ? array_map('intval', array_combine($tags, $values)) : [];
   }
 
   /**

tests/src/Kernel/Cache/CacheBaseTest.php

@@ -0,0 +1,79 @@
+<?php
+
+namespace Drupal\Test\redis\Kernel\Cache;
+
+use Drupal\Component\Serialization\SerializationInterface;
+use Drupal\Core\Cache\Cache;
+use Drupal\Core\Site\Settings;
+use Drupal\KernelTests\KernelTestBase;
+use Drupal\redis\Cache\CacheBase;
+
+/**
+ * @coversDefaultClass \Drupal\redis\Cache\CacheBase
+ */
+class CacheBaseTest extends KernelTestBase {
+
+  private function getMockCache() {
+    return new class ('testbin', $this->prophesize(SerializationInterface::class)
+      ->reveal()) extends CacheBase {
+
+      public function getMultiple(&$cids, $allow_invalid = FALSE) {
+      }
+
+      public function set($cid, $data, $expire = Cache::PERMANENT, array $tags = []) {
+        return $this->getExpiration($expire);
+      }
+
+      protected function doDeleteMultiple(array $cids) {
+      }
+    };
+  }
+
+  /**
+   * @covers ::getPermTtl
+   */
+  public function testSet() {
+    $class = $this->getMockCache();
+    $class->setPermTtl();
+    $this->assertSame(CacheBase::LIFETIME_PERM_DEFAULT, $class->getPermTtl());
+    $class->setPermTtl(5);
+    $this->assertSame(5, $class->getPermTtl());
+    new Settings(['redis.settings' => ['perm_ttl_testbin' => 15]]);
+    $class->setPermTtl();
+    $this->assertSame(15, $class->getPermTtl());
+    new Settings(['redis.settings' => ['perm_ttl_testbin' => '1 year']]);
+    $class->setPermTtl();
+    $this->assertSame(31536000, $class->getPermTtl());
+    new Settings(['redis.settings' => ['perm_ttl_testbin' => '1 month']]);
+    $class->setPermTtl();
+    $this->assertSame(2592000, $class->getPermTtl());
+    // TODO Fix this.
+    new Settings(['redis.settings' => ['perm_ttl_testbin' => '1 day']]);
+    $class->setPermTtl();
+    $this->assertSame(86400, $class->getPermTtl());
+    new Settings(['redis.settings' => ['perm_ttl_testbin' => '1 hour']]);
+    $class->setPermTtl();
+    $this->assertSame(3600, $class->getPermTtl());
+    new Settings(['redis.settings' => ['perm_ttl_testbin' => '1 minute']]);
+    $class->setPermTtl();
+    $this->assertSame(60, $class->getPermTtl());
+    new Settings(['redis.settings' => ['perm_ttl_testbin' => '1 minute 15 seconds']]);
+    $class->setPermTtl();
+    $this->assertSame(75, $class->getPermTtl());
+  }
+
+  /**
+   * @covers ::getExpiration
+   */
+  public function testGetExpiration() {
+    $class = $this->getMockCache();
+    $class->setPermTtl(CacheBase::LIFETIME_PERM_DEFAULT);
+
+    $time = \Drupal::time()->getRequestTime();
+    $this->assertSame(CacheBase::LIFETIME_PERM_DEFAULT, $class->set('a', 'a', Cache::PERMANENT), 'Cache::PERMANENT uses permTtl');
+    $this->assertSame(CacheBase::LIFETIME_PERM_DEFAULT, $class->set('a', 'a', $time + CacheBase::LIFETIME_PERM_DEFAULT), 'expire same as permTtl');
+    $this->assertSame(CacheBase::LIFETIME_PERM_DEFAULT, $class->set('a', 'a', $time + CacheBase::LIFETIME_PERM_DEFAULT + 5), 'expire bigger than permTtl uses permTtl');
+    $this->assertSame(60, $class->set('a', 'a', $time + 60), 'smaller offsets return correct TTL');
+  }
+
+}

tests/src/Kernel/RedisCacheTest.php

@@ -21,7 +21,7 @@ class RedisCacheTest extends GenericCacheBackendUnitTestBase {
    *
    * @var array
    */
-  public static $modules = ['system', 'redis'];
+  protected static $modules = ['system', 'redis'];
 
   public function register(ContainerBuilder $container) {
     self::setUpSettings();

tests/src/Kernel/RedisFloodTest.php

@@ -20,7 +20,7 @@ class RedisFloodTest extends KernelTestBase {
    *
    * @var array
    */
-  public static $modules = ['redis'];
+  protected static $modules = ['redis'];
 
   /**
    * Test flood control.

tests/src/Kernel/RedisLockTest.php

@@ -41,7 +41,7 @@ public function register(ContainerBuilder $container) {
   /**
    * {@inheritdoc}
    */
-  protected function setUp() {
+  protected function setUp(): void {
     parent::setUp();
     $this->lock = $this->container->get('lock');
   }

tests/src/Kernel/RedisQueueTest.php

@@ -20,7 +20,7 @@ class RedisQueueTest extends CoreQueueTest {
    *
    * @var array
    */
-  public static $modules = ['redis'];
+  protected static $modules = ['redis'];
 
   /**
    * Tests Redis non-blocking queue.