Code review

CHANGELOG.md

@@ -30,3 +30,5 @@
 - [2026-01-16] DamImpr: test method with depth array greater than 1 [#38](https://github.com/DamImpr/cache-multi-layer/pull/38) 
 
 - [2026-03-25] DamImpr: Fix code review [#40](https://github.com/DamImpr/cache-multi-layer/pull/40) 
+
+- [2026-05-31] DamImpr: Code review [#41](https://github.com/DamImpr/cache-multi-layer/pull/41) 

Dockerfile

@@ -11,12 +11,12 @@ RUN apk add --no-cache \
     autoconf \
     libc-dev \
     && pecl install apcu redis memcache \
+    && rm -rf /tmp/pear \
     && docker-php-ext-enable apcu redis memcache \
     && apk del .build-deps
 
 COPY --from=composer:2.8 /usr/bin/composer /usr/bin/composer
-RUN echo "apc.enable_cli=1" >> /usr/local/etc/php/php.ini
-RUN echo "apc.enable=1" >> /usr/local/etc/php/php.ini
+RUN printf "apc.enable_cli=1\napc.enable=1\n" >> /usr/local/etc/php/php.ini
 WORKDIR /app
 
 COPY ./src ./src

commands

@@ -30,7 +30,7 @@ case "$1" in
         docker compose down
         ;;
     *)
-        echo "comands allowed"
+        echo "commands allowed"
         echo " - test-sw"
         echo " - update-vendor"
         echo " - php-cs-fixer"

docker-compose.yml

@@ -17,15 +17,25 @@ services:
       - cache-multi-layer-network
     depends_on:
       redis-server:
-        condition: service_started
+        condition: service_healthy
       memcache-server:
-        condition: service_started
+        condition: service_healthy
   redis-server:
     image: redis
+    healthcheck:
+      test: ["CMD", "redis-cli", "ping"]
+      interval: 5s
+      timeout: 3s
+      retries: 5
     networks:
       - cache-multi-layer-network
   memcache-server:
     image: memcached
+    healthcheck:
+      test: ["CMD-SHELL", "bash -c 'exec 3<>/dev/tcp/localhost/11211'"]
+      interval: 5s
+      timeout: 3s
+      retries: 5
     networks:
       - cache-multi-layer-network
 networks:

entrypoint.sh

@@ -21,7 +21,7 @@ case "$1" in
         exec sh
         ;;
     *)
-        echo "comands allowed"
+        echo "commands allowed"
         echo " - test-sw"
         echo " - update-vendor"
         echo " - php-cs-fix"

src/Exception/CacheConnectionException.php

@@ -0,0 +1,14 @@
+<?php
+
+namespace CacheMultiLayer\Exception;
+
+use Exception;
+
+/**
+ * Exception thrown if an error occurs when connecting to the cache.
+ *
+ * @author Damiano Improta <code@damianoimprota.it> aka Drizella
+ */
+class CacheConnectionException extends \Exception
+{
+}

src/Service/ApcuCache.php

@@ -63,8 +63,15 @@ public function increment(string $key, ?int $ttl = null): int|false
     public function getRemainingTTL(string $key): ?int
     {
         $keyInfo = apcu_key_info($this->getEffectiveKey($key));
-
-        return null !== $keyInfo ? $keyInfo['ttl'] : null;
+        if (null === $keyInfo) {
+            return null;
+        }
+        $result = $keyInfo['creation_time'] + $keyInfo['ttl'] - time();
+        if ($result < 0) {
+            return null;
+        }
+
+        return $result;
     }
 
     #[\Override]

src/Service/CacheManagerImpl.php

@@ -53,8 +53,11 @@ public function get(string $key): int|float|string|Cacheable|array|null
             return null;
         }
         $ttlRemaining = $this->caches[$i - 1]->getRemainingTTL($key);
+        if ($ttlRemaining <= 0) {
+            $ttlRemaining = null;
+        }
         for ($j = $i - 2; $j >= 0; --$j) {
-            $ttl = min($this->caches[$j]->getTtl(), $ttlRemaining);
+            $ttl = null !== $ttlRemaining ? min($this->caches[$j]->getTtl(), $ttlRemaining) : null;
             $this->caches[$j]->set($key, $data, $ttl);
         }
 

src/Service/MemcacheCache.php

@@ -3,8 +3,8 @@
 namespace CacheMultiLayer\Service;
 
 use CacheMultiLayer\Enum\CacheEnum;
+use CacheMultiLayer\Exception\CacheConnectionException;
 use CacheMultiLayer\Interface\Cacheable;
-use Memcache;
 
 /**
  * MEMCACHE cache implementation.
@@ -79,8 +79,9 @@ public function getRemainingTTL(string $key): ?int
         if (empty($val)) {
             return null;
         }
+        $res = $val['expires_at'] - time();
 
-        return $val['expires_at'] - time();
+        return $res >= 0 ? $res : null;
     }
 
     #[\Override]
@@ -144,7 +145,7 @@ protected function __construct(int $ttl, array $configuration = [])
             }
 
             if (!$resultConnection) {
-                throw new \Exception('Connection not found');
+                throw new CacheConnectionException('Connection not found');
             }
         }
 

src/Service/PRedisCache.php

@@ -5,6 +5,7 @@
 use CacheMultiLayer\Enum\CacheEnum;
 use CacheMultiLayer\Interface\Cacheable;
 use Predis\Client as PredisClient;
+use Predis\Response\ServerException;
 
 /**
  * PREDIS cache implementation.
@@ -16,7 +17,12 @@ class PRedisCache extends Cache
     #[\Override]
     public function decrement(string $key, ?int $ttl = null): int|false
     {
-        $value = $this->predisClient->decr($this->getEffectiveKey($key));
+        try {
+            $value = $this->predisClient->decr($this->getEffectiveKey($key));
+        } catch (ServerException) {
+            // value has not numeric value
+            return false;
+        }
         if (empty($this->getRemainingTTL($key))) {
             $this->predisClient->expire($this->getEffectiveKey($key), $this->getTtlToUse($ttl));
         }
@@ -48,7 +54,12 @@ public function set(string $key, int|float|string|Cacheable|array $val, ?int $tt
     #[\Override]
     public function increment(string $key, ?int $ttl = null): int|false
     {
-        $value = $this->predisClient->incr($this->getEffectiveKey($key));
+        try {
+            $value = $this->predisClient->incr($this->getEffectiveKey($key));
+        } catch (ServerException) {
+            // value has not numeric value
+            return false;
+        }
         if (empty($this->getRemainingTTL($key))) {
             $this->predisClient->expire($this->getEffectiveKey($key), $this->getTtlToUse($ttl));
         }
@@ -65,13 +76,13 @@ public function clear(string $key): bool
     #[\Override]
     public function clearAllCache(): bool
     {
-        return null !== $this->predisClient->flushall();
+        return 'OK' === (string) $this->predisClient->flushall();
     }
 
     #[\Override]
     public function getRemainingTTL(string $key): ?int
     {
-        $ttl = $this->predisClient->ttl($key);
+        $ttl = $this->predisClient->ttl($this->getEffectiveKey($key));
 
         return $ttl >= 0 ? $ttl : null;
     }

src/Service/RedisCache.php

@@ -64,7 +64,7 @@ public function clear(string $key): bool
     #[\Override]
     public function clearAllCache(): bool
     {
-        return null !== $this->redis->flushall();
+        return (bool) $this->redis->flushall();
     }
 
     #[\Override]
@@ -78,7 +78,7 @@ public function getRemainingTTL(string $key): ?int
     #[\Override]
     public function isConnected(): bool
     {
-        return null !== $this->redis->ping();
+        return (bool) $this->redis->ping();
     }
 
     #[\Override]

tests/AbstractCache.php

@@ -176,16 +176,35 @@ public function testEmptyDecrement(): void
         $this->assertEquals($expected, $actual);
     }
 
+    public function testFailStringIncrement(): void
+    {
+        $key = 'test_fail_increment';
+        $value = 'foo';
+        $this->cache->set($key, $value);
+        $actual = $this->cache->increment($key);
+        $this->assertFalse($actual);
+    }
+
+    public function testFailStringDecrement(): void
+    {
+        $key = 'test_fail_decrement';
+        $value = 'foo';
+        $this->cache->set($key, $value);
+        $actual = $this->cache->decrement($key);
+        $this->assertFalse($actual);
+    }
+
     public function testRemainingTTL(): void
     {
-        $key = 'test_clear';
-        $x = 1;
-        $res = $this->cache->set($key, $x);
+        $key = 'test_remaining_ttl';
+        $val = 1;
+        $ttl = 10;
+        $res = $this->cache->set($key, $val, $ttl);
         $this->assertTrue($res);
         sleep(2);
-        $ttl = $this->cache->getRemainingTTL($key);
-        $this->assertNotNull($ttl);
-        $this->assertLessThanOrEqual(60, $ttl);
+        $ttlActual = $this->cache->getRemainingTTL($key);
+        $this->assertNotNull($ttlActual);
+        $this->assertLessThan(60, $ttlActual);
     }
 
     public function testIsConnected(): void

tests/AbstractCacheManager.php

@@ -2,6 +2,9 @@
 
 namespace CacheMultiLayer\Tests;
 
+use CacheMultiLayer\Enum\CacheEnum;
+use CacheMultiLayer\Service\Cache;
+use CacheMultiLayer\Service\CacheConfiguration;
 use CacheMultiLayer\Service\CacheManager;
 use CacheMultiLayer\Tests\Entity\Foo;
 use PHPUnit\Framework\TestCase;
@@ -15,7 +18,6 @@
 class AbstractCacheManager extends TestCase
 {
     private ?CacheManager $cacheManager = null;
-
     private ?Foo $foo = null;
 
     final public function setCacheManager(?CacheManager $cacheManager): void
@@ -156,4 +158,114 @@ public function testClearAllCache(): void
         $val2 = $this->cacheManager->get($key2);
         $this->assertNull($val2);
     }
+
+    public function testEmptyCache(): void
+    {
+        $key = 'foo';
+        $val = 'bar';
+        $res = CacheManager::factory()->set($key, $val);
+        $this->assertFalse($res);
+    }
+
+    public function testDuplicateCache(): void
+    {
+        $cc = CacheManager::factory();
+        $appendTrue = $cc->appendCache(Cache::factory(CacheEnum::APCU, 10));
+        $appendFalse = $cc->appendCache(Cache::factory(CacheEnum::APCU, 10));
+        $this->assertTrue($appendTrue);
+        $this->assertFalse($appendFalse);
+    }
+
+    public function testDuplicateConfig(): void
+    {
+        $cc = new CacheConfiguration();
+        $appendTrue = $cc->appendCacheLevel(CacheEnum::APCU, 10);
+        $appendFalse = $cc->appendCacheLevel(CacheEnum::APCU, 10);
+        $this->assertTrue($appendTrue);
+        $this->assertFalse($appendFalse);
+    }
+
+    public function testArrayDepth(): void
+    {
+        $x = [1, 2, 3, null, [
+            1, 2, 3, null, [
+                1, 2, 3, null,
+            ],
+        ],
+        ];
+        $key = 'test_array_depth_manager';
+        $res = $this->cacheManager->set($key, $x);
+        $this->assertTrue($res);
+        $val = $this->cacheManager->get($key);
+        $this->testRecursiveArray($x, $val);
+    }
+
+    public function testEmptyIncrement(): void
+    {
+        $key = 'test_empty_increment_manager';
+        $expected = 1;
+        $resultSet = $this->cacheManager->increment($key);
+        foreach ($resultSet as $cacheKey => $actual) {
+            $this->assertEquals($expected, $actual, 'cache current '.$cacheKey);
+        }
+    }
+
+    public function testEmptyDecrement(): void
+    {
+        $key = 'test_empty_decrement_manager';
+        $expected = -1;
+        $resultSet = $this->cacheManager->decrement($key);
+        foreach ($resultSet as $cacheKey => $actual) {
+            $this->assertEquals($expected, $actual, 'cache current '.$cacheKey);
+        }
+    }
+
+    public function testFailStringIncrement(): void
+    {
+        $key = 'test_fail_increment_manager';
+        $value = 'foo';
+        $this->cacheManager->set($key, $value);
+        $resultSet = $this->cacheManager->increment($key);
+        foreach ($resultSet as $cacheKey => $actual) {
+            $this->assertFalse($actual, 'cache current '.$cacheKey);
+        }
+    }
+
+    public function testFailStringDecrement(): void
+    {
+        $key = 'test_fail_decrement_manager';
+        $value = 'foo';
+        $this->cacheManager->set($key, $value);
+        $resultSet = $this->cacheManager->decrement($key);
+        foreach ($resultSet as $cacheKey => $actual) {
+            $this->assertFalse($actual, 'cache current '.$cacheKey);
+        }
+    }
+
+    public function testRemainingTTL(): void
+    {
+        $key = 'test_remaining_ttl_manager';
+        $val = 1;
+        $ttl = 10;
+        $res = $this->cacheManager->set($key, $val, $ttl);
+        $this->assertTrue($res);
+        sleep(2);
+        $resultSet = $this->cacheManager->getRemainingTTL($key);
+        foreach ($resultSet as $cacheKey => $actual) {
+            $this->assertNotNull($actual, 'cache current '.$cacheKey);
+            $this->assertLessThan(60, $actual, 'cache current '.$cacheKey);
+        }
+    }
+
+    private function testRecursiveArray(array $actual, array $expected): void
+    {
+        foreach ($expected as $key => $value) {
+            $this->assertArrayHasKey($key, $actual);
+            if (is_array($value)) {
+                $this->testRecursiveArray($value, $actual[$key]);
+            } else {
+                $this->assertEquals($value, $actual[$key]);
+            }
+        }
+    }
 }