Refactor BasicEnvironment

Remove \Countable, add tests.

lib/Worker/BasicEnvironment.php

@@ -3,17 +3,12 @@
 namespace Amp\Parallel\Worker;
 
 use Amp\Loop;
+use Amp\Struct;
 
 class BasicEnvironment implements Environment {
     /** @var array */
     private $data = [];
 
-    /** @var array */
-    private $ttl = [];
-
-    /** @var array */
-    private $expire = [];
-
     /** @var \SplPriorityQueue */
     private $queue;
 
@@ -28,14 +23,27 @@ class BasicEnvironment implements Environment {
             while (!$this->queue->isEmpty()) {
                 $key = $this->queue->top();
 
-                if (isset($this->expire[$key])) {
-                    if ($time <= $this->expire[$key]) {
-                        break;
-                    }
-
-                    unset($this->data[$key], $this->expire[$key], $this->ttl[$key]);
+                if (!isset($this->data[$key])) {
+                    // Item removed.
+                    $this->queue->extract();
+                    continue;
                 }
 
+                $struct = $this->data[$key];
+
+                if ($struct->expire === 0) {
+                    // Item was set again without a TTL.
+                    $this->queue->extract();
+                    continue;
+                }
+
+                if ($time < $struct->expire) {
+                    // Item at top has not expired, break out of loop.
+                    break;
+                }
+
+                unset($this->data[$key]);
+
                 $this->queue->extract();
             }
 
@@ -63,49 +71,62 @@ class BasicEnvironment implements Environment {
      * @return mixed|null Returns null if the key does not exist.
      */
     public function get(string $key) {
-        if (isset($this->ttl[$key]) && 0 !== $this->ttl[$key]) {
-            $this->expire[$key] = time() + $this->ttl[$key];
-            $this->queue->insert($key, -$this->expire[$key]);
+        if (!isset($this->data[$key])) {
+            return null;
         }
 
-        return isset($this->data[$key]) ? $this->data[$key] : null;
+        $struct = $this->data[$key];
+
+        if ($struct->ttl !== null) {
+            $struct->expire = \time() + $struct->ttl;
+            $this->queue->insert($key, -$struct->expire);
+        }
+
+        return $struct->data;
     }
 
     /**
      * @param string $key
      * @param mixed $value Using null for the value deletes the key.
-     * @param int $ttl Number of seconds until data is automatically deleted. Use 0 for unlimited TTL.
+     * @param int $ttl Number of seconds until data is automatically deleted. Use null for unlimited TTL.
+     *
+     * @throws \Error If the time-to-live is not a positive integer.
      */
-    public function set(string $key, $value, int $ttl = 0) {
-        if (null === $value) {
+    public function set(string $key, $value, int $ttl = null) {
+        if ($value === null) {
             $this->delete($key);
             return;
         }
 
-        $ttl = (int) $ttl;
-        if (0 > $ttl) {
-            $ttl = 0;
+        if ($ttl !== null && $ttl <= 0) {
+            throw new \Error("The time-to-live must be a positive integer or null");
         }
 
-        if (0 !== $ttl) {
-            $this->ttl[$key] = $ttl;
-            $this->expire[$key] = time() + $ttl;
-            $this->queue->insert($key, -$this->expire[$key]);
+        $struct = new class {
+            use Struct;
+            public $data;
+            public $expire = 0;
+            public $ttl;
+        };
+
+        $struct->data = $value;
+
+        if ($ttl !== null) {
+            $struct->ttl = $ttl;
+            $struct->expire = \time() + $ttl;
+            $this->queue->insert($key, -$struct->expire);
 
             Loop::enable($this->timer);
-        } else {
-            unset($this->expire[$key], $this->ttl[$key]);
         }
 
-        $this->data[$key] = $value;
+        $this->data[$key] = $struct;
     }
 
     /**
      * @param string $key
      */
     public function delete(string $key) {
-        $key = (string) $key;
-        unset($this->data[$key], $this->expire[$key], $this->ttl[$key]);
+        unset($this->data[$key]);
     }
 
     /**
@@ -131,7 +152,7 @@ class BasicEnvironment implements Environment {
     }
 
     /**
-     * Alias of set() with $ttl = 0.
+     * Alias of set() with $ttl = null.
      *
      * @param string $key
      * @param mixed $value
@@ -149,20 +170,11 @@ class BasicEnvironment implements Environment {
         $this->delete($key);
     }
 
-    /**
-     * @return int
-     */
-    public function count(): int {
-        return count($this->data);
-    }
-
     /**
      * Removes all values.
      */
     public function clear() {
         $this->data = [];
-        $this->expire = [];
-        $this->ttl = [];
 
         Loop::disable($this->timer);
         $this->queue = new \SplPriorityQueue;

lib/Worker/Environment.php

@@ -2,7 +2,7 @@
 
 namespace Amp\Parallel\Worker;
 
-interface Environment extends \ArrayAccess, \Countable {
+interface Environment extends \ArrayAccess {
     /**
      * @param string $key
      *
@@ -22,7 +22,7 @@ interface Environment extends \ArrayAccess, \Countable {
      * @param mixed $value Using null for the value deletes the key.
      * @param int $ttl Number of seconds until data is automatically deleted. Use 0 for unlimited TTL.
      */
-    public function set(string $key, $value, int $ttl = 0);
+    public function set(string $key, $value, int $ttl = null);
 
     /**
      * @param string $key

test/Worker/BasicEnvironmentTest.php

@@ -0,0 +1,92 @@
+<?php
+
+namespace Amp\Parallel\Test\Worker;
+
+use Amp\Delayed;
+use Amp\Loop;
+use Amp\Parallel\Worker\BasicEnvironment;
+use Amp\PHPUnit\TestCase;
+
+class BasicEnvironmentTest extends TestCase {
+    public function testBasicOperations() {
+        $environment = new BasicEnvironment;
+        $key = "key";
+
+        $this->assertFalse($environment->exists($key));
+        $this->assertNull($environment->get($key));
+
+        $environment->set($key, 1);
+        $this->assertTrue($environment->exists($key));
+        $this->assertSame(1, $environment->get($key));
+
+        $environment->set($key, 2);
+        $this->assertSame(2, $environment->get($key));
+
+        $environment->delete($key);
+        $this->assertFalse($environment->exists($key));
+        $this->assertNull($environment->get($key));
+    }
+
+    public function testArrayAccess() {
+        $environment = new BasicEnvironment;
+        $key = "key";
+
+        $this->assertFalse(isset($environment[$key]));
+        $this->assertNull($environment[$key]);
+
+        $environment[$key] = 1;
+        $this->assertTrue(isset($environment[$key]));
+        $this->assertSame(1, $environment[$key]);
+
+        $environment[$key] = 2;
+        $this->assertSame(2, $environment[$key]);
+
+        unset($environment[$key]);
+        $this->assertFalse(isset($environment[$key]));
+        $this->assertNull($environment[$key]);
+    }
+
+    public function testClear() {
+        $environment = new BasicEnvironment;
+
+        $environment->set("key1", 1);
+        $environment->set("key2", 2);
+
+        $environment->clear();
+
+        $this->assertFalse($environment->exists("key1"));
+        $this->assertFalse($environment->exists("key2"));
+    }
+
+    public function testTtl() {
+        Loop::run(function () {
+            $environment = new BasicEnvironment;
+            $key = "key";
+
+            $environment->set($key, 1, 2);
+
+            yield new Delayed(3000);
+
+            $this->assertFalse($environment->exists($key));
+        });
+    }
+
+    /**
+     * @depends testTtl
+     */
+    public function testRemovingTtl() {
+        Loop::run(function () {
+            $environment = new BasicEnvironment;
+            $key = "key";
+
+            $environment->set($key, 1, 1);
+
+            $environment->set($key, 2);
+
+            yield new Delayed(2000);
+
+            $this->assertTrue($environment->exists($key));
+            $this->assertSame(2, $environment->get($key));
+        });
+    }
+}