Merge pull request #29 from WyriHaximus/follow-up-on-TTL-pr

Follow up on TTL PR putting some finishing touches on it
WyriHaximus · May 11, 2021 · 6df3b03 · 6df3b03
2 parents d7f0709 + 6609e74
commit 6df3b03
Show file tree

Hide file tree

Showing 5 changed files with 41 additions and 28 deletions.
diff --git a/src/CacheItem.php b/src/CacheItem.php
@@ -5,9 +5,10 @@
 namespace WyriHaximus\React\Cache;
 
 /**
- * one item in a cache. Only used locally within the cache
+ * One item in a cache. Only used locally within the cache
+ * @internal
  */
-class CacheItem
+final class CacheItem
 {
     /** @var mixed the data to be cached */
     private $data;
@@ -25,14 +26,13 @@ public function __construct($data, float $expiresAtTime = null)
         $this->expiresAt = $expiresAtTime;
     }
 
-    public function getExpiresAt(): ?float
+    public function expiresAt(): ?float
     {
         return $this->expiresAt;
     }
 
     /**
      * @param float|null $now current time
-     * @return bool
      */
     public function hasExpired(float $now = null): bool
     {
@@ -42,7 +42,7 @@ public function hasExpired(float $now = null): bool
     /**
      * @return mixed
      */
-    public function getData()
+    public function data()
     {
         return $this->data;
     }

diff --git a/src/Filesystem.php b/src/Filesystem.php
@@ -6,12 +6,12 @@
 use React\Filesystem\FilesystemInterface as ReactFilesystem;
 use React\Filesystem\Node\FileInterface;
 use React\Filesystem\Node\NodeInterface;
-use function React\Promise\all;
 use React\Promise\Promise;
 use React\Promise\PromiseInterface;
+use Throwable;
+use function React\Promise\all;
 use function React\Promise\reject;
 use function React\Promise\resolve;
-use Throwable;
 
 final class Filesystem implements CacheInterface
 {
@@ -46,28 +46,30 @@ public function __construct(ReactFilesystem $filesystem, string $path)
     /**
      * @param  string           $key
      * @param  null|mixed       $default
-     * @return PromiseInterface
      */
     public function get($key, $default = null): PromiseInterface
     {
         return $this->has($key)->then(function (bool $has) use ($key, $default) {
             if ($has === true) {
                 return $this->getFile($key)
-                            ->getContents()
-                            ->then(
-                                function (CacheItem $cacheItem) use ($key, $default) {
-                                    if ($cacheItem->hasExpired($this->now())) {
-                                        return $this->getFile($key)
-                                                    ->remove()
-                                                    ->then(
-                                                        function () use ($default) {
-                                                            return resolve($default);
-                                                        }
-                                                    );
-                                    }
-                                    return resolve(unserialize($cacheItem->getData()));
-                                }
-                            );
+                    ->getContents()
+                    ->then(static function (string $contents) {
+                        return unserialize($contents);
+                    })
+                    ->then(
+                        function (CacheItem $cacheItem) use ($key, $default) {
+                            if ($cacheItem->hasExpired($this->now())) {
+                                return $this->getFile($key)
+                                    ->remove()
+                                    ->then(
+                                        function () use ($default) {
+                                            return resolve($default);
+                                        }
+                                    );
+                            }
+                            return resolve($cacheItem->data());
+                        }
+                    );
             }
 
             return resolve($default);

diff --git a/tests/CacheItemTest.php b/tests/CacheItemTest.php
@@ -13,8 +13,8 @@ public function testConstructor()
     {
         $time    = 123456;
         $subject = new CacheItem($data = ['abc' => 123], $time);
-        self::assertEquals($data, $subject->getData());
-        self::assertEquals($time, $subject->getExpiresAt());
+        self::assertEquals($data, $subject->data());
+        self::assertEquals($time, $subject->expiresAt());
 
         self::assertTrue($subject->hasExpired($time + 1));
         self::assertFalse($subject->hasExpired($time));

diff --git a/tests/FilesystemTest.php b/tests/FilesystemTest.php
@@ -2,6 +2,7 @@
 
 namespace WyriHaximus\Tests\React\Cache;
 
+use WyriHaximus\React\Cache\CacheItem;
 use function Clue\React\Block\await;
 use React\EventLoop\Factory;
 use React\Filesystem\FilesystemInterface as ReactFilesystem;
@@ -39,7 +40,7 @@ public function testGet(): void
         $file = $this->prophesize(FileInterface::class);
         $this->filesystem->file($prefix . $key)->shouldBeCalled()->willReturn($file->reveal());
         $file->exists()->shouldBeCalled()->willReturn(new FulfilledPromise());
-        $file->getContents()->shouldBeCalled()->willReturn(new FulfilledPromise($value));
+        $file->getContents()->shouldBeCalled()->willReturn(new FulfilledPromise(serialize(new CacheItem($value))));
         $promise = (new Filesystem($this->filesystem->reveal(), $prefix))->get($key);
         $this->assertInstanceOf(PromiseInterface::class, $promise);
         $result = await($promise, Factory::create());
@@ -66,7 +67,7 @@ public function testSet(): void
         $key = 'key';
         $value = 'value';
         $file = $this->prophesize(FileInterface::class);
-        $file->putContents($value)->shouldBeCalled()->willReturn(resolve(true));
+        $file->putContents(serialize(new CacheItem($value)))->shouldBeCalled()->willReturn(resolve(true));
         $this->filesystem->file($prefix . $key)->shouldBeCalled()->willReturn($file->reveal());
         (new Filesystem($this->filesystem->reveal(), $prefix))->set($key, $value);
     }
@@ -78,7 +79,7 @@ public function testSetMakeDirectory(): void
         $dirKey = 'path/to';
         $value = 'value';
         $file = $this->prophesize(FileInterface::class);
-        $file->putContents($value)->shouldBeCalled()->willReturn(resolve(true));
+        $file->putContents(serialize(new CacheItem($value)))->shouldBeCalled()->willReturn(resolve(true));
         $dir = $this->prophesize(DirectoryInterface::class);
         $dir->createRecursive()->shouldBeCalled()->willReturn(new FulfilledPromise());
         $this->filesystem->file($prefix . $key)->shouldBeCalled()->willReturn($file->reveal());

diff --git a/tests/FunctionalTest.php b/tests/FunctionalTest.php
@@ -34,6 +34,16 @@ public function testHasNot(): void
         self::assertFalse($this->await($this->filesystem->has($fileName), $this->loop));
     }
 
+    public function testGettingExpiredItem(): void
+    {
+        $fileName = 'file.name';
+        $default = 'Sober!';
+
+        $this->await($this->filesystem->set($fileName, 'Alcohol!', 1.234), $this->loop);
+        sleep(3);
+        self::assertSame($default, $this->await($this->filesystem->get($fileName, $default), $this->loop));
+    }
+
     public function testCannotDeleteNonExistingItem(): void
     {
         $fileName = 'file.name';