Respect record TTL values from RRset for cache TTL

Author: clue

src/Query/CachingExecutor.php

@@ -9,8 +9,7 @@
 class CachingExecutor implements ExecutorInterface
 {
     /**
-     * Initial implementation uses a fixed TTL for postive DNS responses as well
-     * as negative responses (NXDOMAIN etc.).
+     * Default TTL for negative responses (NXDOMAIN etc.).
      *
      * @internal
      */
@@ -29,23 +28,24 @@ 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) {
+        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) {
+                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) {
+                        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, CachingExecutor::TTL);
+                                $cache->set($id, $message, $that->ttl($message));
                             }
 
                             return $message;
@@ -58,4 +58,27 @@ function (Message $message) use ($cache, $id) {
             $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;
+    }
 }

tests/Query/CachingExecutorTest.php

@@ -8,6 +8,7 @@
 use React\Promise\Promise;
 use React\Tests\Dns\TestCase;
 use React\Promise\Deferred;
+use React\Dns\Model\Record;
 
 class CachingExecutorTest extends TestCase
 {
@@ -63,7 +64,28 @@ public function testQueryWillReturnResolvedPromiseWhenCacheReturnsHitWithoutSend
         $promise->then($this->expectCallableOnceWith($message), $this->expectCallableNever());
     }
 
-    public function testQueryWillReturnResolvedPromiseWhenCacheReturnsMissAndFallbackExecutorResolvesAndSaveMessageToCache()
+    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();