Implement CachingExecutor using cache TTL, deprecate old CachedExecutor

src/Query/CachedExecutor.php

@@ -4,6 +4,10 @@
 
 use React\Dns\Model\Message;
 
+/**
+ * @deprecated unused, exists for BC only
+ * @see CachingExecutor
+ */
 class CachedExecutor implements ExecutorInterface
 {
     private $executor;

src/Query/CachingExecutor.php

@@ -0,0 +1,84 @@
+<?php
+
+namespace React\Dns\Query;
+
+use React\Cache\CacheInterface;
+use React\Dns\Model\Message;
+use React\Promise\Promise;
+
+class CachingExecutor implements ExecutorInterface
+{
+    /**
+     * Default TTL for negative responses (NXDOMAIN etc.).
+     *
+     * @internal
+     */
+    const TTL = 60;
+
+    private $executor;
+    private $cache;
+
+    public function __construct(ExecutorInterface $executor, CacheInterface $cache)
+    {
+        $this->executor = $executor;
+        $this->cache = $cache;
+    }
+
+    public function query($nameserver, Query $query)
+    {
+        $id = $query->name . ':' . $query->type . ':' . $query->class;
+        $cache = $this->cache;
+        $that = $this;
+        $executor = $this->executor;
+
+        $pending = $cache->get($id);
+        return new Promise(function ($resolve, $reject) use ($nameserver, $query, $id, $cache, $executor, &$pending, $that) {
+            $pending->then(
+                function ($message) use ($nameserver, $query, $id, $cache, $executor, &$pending, $that) {
+                    // return cached response message on cache hit
+                    if ($message !== null) {
+                        return $message;
+                    }
+
+                    // perform DNS lookup if not already cached
+                    return $pending = $executor->query($nameserver, $query)->then(
+                        function (Message $message) use ($cache, $id, $that) {
+                            // DNS response message received => store in cache when not truncated and return
+                            if (!$message->header->isTruncated()) {
+                                $cache->set($id, $message, $that->ttl($message));
+                            }
+
+                            return $message;
+                        }
+                    );
+                }
+            )->then($resolve, $reject);
+        }, function ($_, $reject) use (&$pending, $query) {
+            $reject(new \RuntimeException('DNS query for ' . $query->name . ' has been cancelled'));
+            $pending->cancel();
+        });
+    }
+
+    /**
+     * @param Message $message
+     * @return int
+     * @internal
+     */
+    public function ttl(Message $message)
+    {
+        // select TTL from answers (should all be the same), use smallest value if available
+        // @link https://tools.ietf.org/html/rfc2181#section-5.2
+        $ttl = null;
+        foreach ($message->answers as $answer) {
+            if ($ttl === null || $answer->ttl < $ttl) {
+                $ttl = $answer->ttl;
+            }
+        }
+
+        if ($ttl === null) {
+            $ttl = self::TTL;
+        }
+
+        return $ttl;
+    }
+}

src/Query/RecordBag.php

@@ -4,6 +4,10 @@
 
 use React\Dns\Model\Record;
 
+/**
+ * @deprecated unused, exists for BC only
+ * @see CachingExecutor
+ */
 class RecordBag
 {
     private $records = array();

src/Query/RecordCache.php

@@ -10,6 +10,9 @@
 
 /**
  * Wraps an underlying cache interface and exposes only cached DNS data
+ *
+ * @deprecated unused, exists for BC only
+ * @see CachingExecutor
  */
 class RecordCache
 {

src/Resolver/Factory.php

@@ -5,11 +5,10 @@
 use React\Cache\ArrayCache;
 use React\Cache\CacheInterface;
 use React\Dns\Config\HostsFile;
-use React\Dns\Query\CachedExecutor;
+use React\Dns\Query\CachingExecutor;
 use React\Dns\Query\CoopExecutor;
 use React\Dns\Query\ExecutorInterface;
 use React\Dns\Query\HostsFileExecutor;
-use React\Dns\Query\RecordCache;
 use React\Dns\Query\RetryExecutor;
 use React\Dns\Query\TimeoutExecutor;
 use React\Dns\Query\UdpTransportExecutor;
@@ -84,7 +83,7 @@ protected function createRetryExecutor(LoopInterface $loop)
 
     protected function createCachedExecutor(LoopInterface $loop, CacheInterface $cache)
     {
-        return new CachedExecutor($this->createRetryExecutor($loop), new RecordCache($cache));
+        return new CachingExecutor($this->createRetryExecutor($loop), $cache);
     }
 
     protected function addPortToServerIfMissing($nameserver)

tests/Query/CachingExecutorTest.php

@@ -0,0 +1,183 @@
+<?php
+
+namespace React\Tests\Dns\Query;
+
+use React\Dns\Model\Message;
+use React\Dns\Query\CachingExecutor;
+use React\Dns\Query\Query;
+use React\Promise\Promise;
+use React\Tests\Dns\TestCase;
+use React\Promise\Deferred;
+use React\Dns\Model\Record;
+
+class CachingExecutorTest extends TestCase
+{
+    public function testQueryWillReturnPendingPromiseWhenCacheIsPendingWithoutSendingQueryToFallbackExecutor()
+    {
+        $fallback = $this->getMockBuilder('React\Dns\Query\ExecutorInterface')->getMock();
+        $fallback->expects($this->never())->method('query');
+
+        $cache = $this->getMockBuilder('React\Cache\CacheInterface')->getMock();
+        $cache->expects($this->once())->method('get')->with('reactphp.org:1:1')->willReturn(new Promise(function () { }));
+
+        $executor = new CachingExecutor($fallback, $cache);
+
+        $query = new Query('reactphp.org', Message::TYPE_A, Message::CLASS_IN);
+
+        $promise = $executor->query('8.8.8.8', $query);
+
+        $promise->then($this->expectCallableNever(), $this->expectCallableNever());
+    }
+
+    public function testQueryWillReturnPendingPromiseWhenCacheReturnsMissAndWillSendSameQueryToFallbackExecutor()
+    {
+        $query = new Query('reactphp.org', Message::TYPE_A, Message::CLASS_IN);
+
+        $fallback = $this->getMockBuilder('React\Dns\Query\ExecutorInterface')->getMock();
+        $fallback->expects($this->once())->method('query')->with('8.8.8.8', $query)->willReturn(new Promise(function () { }));
+
+        $cache = $this->getMockBuilder('React\Cache\CacheInterface')->getMock();
+        $cache->expects($this->once())->method('get')->willReturn(\React\Promise\resolve(null));
+
+        $executor = new CachingExecutor($fallback, $cache);
+
+        $promise = $executor->query('8.8.8.8', $query);
+
+        $promise->then($this->expectCallableNever(), $this->expectCallableNever());
+    }
+
+    public function testQueryWillReturnResolvedPromiseWhenCacheReturnsHitWithoutSendingQueryToFallbackExecutor()
+    {
+        $fallback = $this->getMockBuilder('React\Dns\Query\ExecutorInterface')->getMock();
+        $fallback->expects($this->never())->method('query');
+
+        $message = new Message();
+        $cache = $this->getMockBuilder('React\Cache\CacheInterface')->getMock();
+        $cache->expects($this->once())->method('get')->with('reactphp.org:1:1')->willReturn(\React\Promise\resolve($message));
+
+        $executor = new CachingExecutor($fallback, $cache);
+
+        $query = new Query('reactphp.org', Message::TYPE_A, Message::CLASS_IN);
+
+        $promise = $executor->query('8.8.8.8', $query);
+
+        $promise->then($this->expectCallableOnceWith($message), $this->expectCallableNever());
+    }
+
+    public function testQueryWillReturnResolvedPromiseWhenCacheReturnsMissAndFallbackExecutorResolvesAndSaveMessageToCacheWithMinimumTtlFromRecord()
+    {
+        $message = new Message();
+        $message->answers[] = new Record('reactphp.org', Message::TYPE_A, Message::CLASS_IN, 3700, '127.0.0.1');
+        $message->answers[] = new Record('reactphp.org', Message::TYPE_A, Message::CLASS_IN, 3600, '127.0.0.1');
+        $fallback = $this->getMockBuilder('React\Dns\Query\ExecutorInterface')->getMock();
+        $fallback->expects($this->once())->method('query')->willReturn(\React\Promise\resolve($message));
+
+        $cache = $this->getMockBuilder('React\Cache\CacheInterface')->getMock();
+        $cache->expects($this->once())->method('get')->with('reactphp.org:1:1')->willReturn(\React\Promise\resolve(null));
+        $cache->expects($this->once())->method('set')->with('reactphp.org:1:1', $message, 3600);
+
+        $executor = new CachingExecutor($fallback, $cache);
+
+        $query = new Query('reactphp.org', Message::TYPE_A, Message::CLASS_IN);
+
+        $promise = $executor->query('8.8.8.8', $query);
+
+        $promise->then($this->expectCallableOnceWith($message), $this->expectCallableNever());
+    }
+
+    public function testQueryWillReturnResolvedPromiseWhenCacheReturnsMissAndFallbackExecutorResolvesAndSaveMessageToCacheWithDefaultTtl()
+    {
+        $message = new Message();
+        $fallback = $this->getMockBuilder('React\Dns\Query\ExecutorInterface')->getMock();
+        $fallback->expects($this->once())->method('query')->willReturn(\React\Promise\resolve($message));
+
+        $cache = $this->getMockBuilder('React\Cache\CacheInterface')->getMock();
+        $cache->expects($this->once())->method('get')->with('reactphp.org:1:1')->willReturn(\React\Promise\resolve(null));
+        $cache->expects($this->once())->method('set')->with('reactphp.org:1:1', $message, 60);
+
+        $executor = new CachingExecutor($fallback, $cache);
+
+        $query = new Query('reactphp.org', Message::TYPE_A, Message::CLASS_IN);
+
+        $promise = $executor->query('8.8.8.8', $query);
+
+        $promise->then($this->expectCallableOnceWith($message), $this->expectCallableNever());
+    }
+
+    public function testQueryWillReturnResolvedPromiseWhenCacheReturnsMissAndFallbackExecutorResolvesWithTruncatedResponseButShouldNotSaveTruncatedMessageToCache()
+    {
+        $message = new Message();
+        $message->header->set('tc', 1);
+        $fallback = $this->getMockBuilder('React\Dns\Query\ExecutorInterface')->getMock();
+        $fallback->expects($this->once())->method('query')->willReturn(\React\Promise\resolve($message));
+
+        $cache = $this->getMockBuilder('React\Cache\CacheInterface')->getMock();
+        $cache->expects($this->once())->method('get')->with('reactphp.org:1:1')->willReturn(\React\Promise\resolve(null));
+        $cache->expects($this->never())->method('set');
+
+        $executor = new CachingExecutor($fallback, $cache);
+
+        $query = new Query('reactphp.org', Message::TYPE_A, Message::CLASS_IN);
+
+        $promise = $executor->query('8.8.8.8', $query);
+
+        $promise->then($this->expectCallableOnceWith($message), $this->expectCallableNever());
+    }
+
+    public function testQueryWillReturnRejectedPromiseWhenCacheReturnsMissAndFallbackExecutorRejects()
+    {
+        $query = new Query('reactphp.org', Message::TYPE_A, Message::CLASS_IN);
+
+        $fallback = $this->getMockBuilder('React\Dns\Query\ExecutorInterface')->getMock();
+        $fallback->expects($this->once())->method('query')->willReturn(\React\Promise\reject(new \RuntimeException()));
+
+        $cache = $this->getMockBuilder('React\Cache\CacheInterface')->getMock();
+        $cache->expects($this->once())->method('get')->willReturn(\React\Promise\resolve(null));
+
+        $executor = new CachingExecutor($fallback, $cache);
+
+        $promise = $executor->query('8.8.8.8', $query);
+
+        $promise->then($this->expectCallableNever(), $this->expectCallableOnceWith($this->isInstanceOf('RuntimeException')));
+    }
+
+    public function testCancelQueryWillReturnRejectedPromiseAndCancelPendingPromiseFromCache()
+    {
+        $fallback = $this->getMockBuilder('React\Dns\Query\ExecutorInterface')->getMock();
+        $fallback->expects($this->never())->method('query');
+
+        $pending = new Promise(function () { }, $this->expectCallableOnce());
+        $cache = $this->getMockBuilder('React\Cache\CacheInterface')->getMock();
+        $cache->expects($this->once())->method('get')->with('reactphp.org:1:1')->willReturn($pending);
+
+        $executor = new CachingExecutor($fallback, $cache);
+
+        $query = new Query('reactphp.org', Message::TYPE_A, Message::CLASS_IN);
+
+        $promise = $executor->query('8.8.8.8', $query);
+        $promise->cancel();
+
+        $promise->then($this->expectCallableNever(), $this->expectCallableOnceWith($this->isInstanceOf('RuntimeException')));
+    }
+
+    public function testCancelQueryWillReturnRejectedPromiseAndCancelPendingPromiseFromFallbackExecutorWhenCacheReturnsMiss()
+    {
+        $pending = new Promise(function () { }, $this->expectCallableOnce());
+        $fallback = $this->getMockBuilder('React\Dns\Query\ExecutorInterface')->getMock();
+        $fallback->expects($this->once())->method('query')->willReturn($pending);
+
+        $deferred = new Deferred();
+        $cache = $this->getMockBuilder('React\Cache\CacheInterface')->getMock();
+        $cache->expects($this->once())->method('get')->with('reactphp.org:1:1')->willReturn($deferred->promise());
+
+        $executor = new CachingExecutor($fallback, $cache);
+
+        $query = new Query('reactphp.org', Message::TYPE_A, Message::CLASS_IN);
+
+        $promise = $executor->query('8.8.8.8', $query);
+        $deferred->resolve(null);
+        $promise->cancel();
+
+        $promise->then($this->expectCallableNever(), $this->expectCallableOnceWith($this->isInstanceOf('RuntimeException')));
+    }
+}

tests/Resolver/FactoryTest.php

@@ -32,7 +32,7 @@ public function createWithoutPortShouldCreateResolverWithDefaultPort()
     }
 
     /** @test */
-    public function createCachedShouldCreateResolverWithCachedExecutor()
+    public function createCachedShouldCreateResolverWithCachingExecutor()
     {
         $loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock();
 
@@ -41,15 +41,13 @@ public function createCachedShouldCreateResolverWithCachedExecutor()
 
         $this->assertInstanceOf('React\Dns\Resolver\Resolver', $resolver);
         $executor = $this->getResolverPrivateExecutor($resolver);
-        $this->assertInstanceOf('React\Dns\Query\CachedExecutor', $executor);
-        $recordCache = $this->getCachedExecutorPrivateMemberValue($executor, 'cache');
-        $recordCacheCache = $this->getRecordCachePrivateMemberValue($recordCache, 'cache');
-        $this->assertInstanceOf('React\Cache\CacheInterface', $recordCacheCache);
-        $this->assertInstanceOf('React\Cache\ArrayCache', $recordCacheCache);
+        $this->assertInstanceOf('React\Dns\Query\CachingExecutor', $executor);
+        $cache = $this->getCachingExecutorPrivateMemberValue($executor, 'cache');
+        $this->assertInstanceOf('React\Cache\ArrayCache', $cache);
     }
 
     /** @test */
-    public function createCachedShouldCreateResolverWithCachedExecutorWithCustomCache()
+    public function createCachedShouldCreateResolverWithCachingExecutorWithCustomCache()
     {
         $cache = $this->getMockBuilder('React\Cache\CacheInterface')->getMock();
         $loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock();
@@ -59,11 +57,9 @@ public function createCachedShouldCreateResolverWithCachedExecutorWithCustomCach
 
         $this->assertInstanceOf('React\Dns\Resolver\Resolver', $resolver);
         $executor = $this->getResolverPrivateExecutor($resolver);
-        $this->assertInstanceOf('React\Dns\Query\CachedExecutor', $executor);
-        $recordCache = $this->getCachedExecutorPrivateMemberValue($executor, 'cache');
-        $recordCacheCache = $this->getRecordCachePrivateMemberValue($recordCache, 'cache');
-        $this->assertInstanceOf('React\Cache\CacheInterface', $recordCacheCache);
-        $this->assertSame($cache, $recordCacheCache);
+        $this->assertInstanceOf('React\Dns\Query\CachingExecutor', $executor);
+        $cacheProperty = $this->getCachingExecutorPrivateMemberValue($executor, 'cache');
+        $this->assertSame($cache, $cacheProperty);
     }
 
     /**
@@ -115,16 +111,9 @@ private function getResolverPrivateMemberValue($resolver, $field)
         return $reflector->getValue($resolver);
     }
 
-    private function getCachedExecutorPrivateMemberValue($resolver, $field)
+    private function getCachingExecutorPrivateMemberValue($resolver, $field)
     {
-        $reflector = new \ReflectionProperty('React\Dns\Query\CachedExecutor', $field);
-        $reflector->setAccessible(true);
-        return $reflector->getValue($resolver);
-    }
-
-    private function getRecordCachePrivateMemberValue($resolver, $field)
-    {
-        $reflector = new \ReflectionProperty('React\Dns\Query\RecordCache', $field);
+        $reflector = new \ReflectionProperty('React\Dns\Query\CachingExecutor', $field);
         $reflector->setAccessible(true);
         return $reflector->getValue($resolver);
     }