Merge branch 'fix/#713-prevent-result-cache-key-collisions'

Close #713 DBAL-1030
doctrine · May 3, 2017 · 232d585 · 232d585
2 parents 1b3c36b + c23d80a
commit 232d585
Show file tree

Hide file tree

Showing 7 changed files with 205 additions and 13 deletions.
diff --git a/lib/Doctrine/DBAL/Cache/QueryCacheProfile.php b/lib/Doctrine/DBAL/Cache/QueryCacheProfile.php
@@ -88,17 +88,22 @@ public function getCacheKey()
     }
 
     /**
-     * Generates the real cache key from query, params and types.
+     * Generates the real cache key from query, params, types and connection parameters.
      *
      * @param string $query
      * @param array  $params
      * @param array  $types
+     * @param array  $connectionParams
      *
      * @return array
      */
-    public function generateCacheKeys($query, $params, $types)
+    public function generateCacheKeys($query, $params, $types, array $connectionParams = [])
     {
-        $realCacheKey = $query . "-" . serialize($params) . "-" . serialize($types);
+        $realCacheKey = 'query=' . $query .
+            '&params=' . serialize($params) .
+            '&types=' . serialize($types) .
+            '&connectionParams=' . serialize($connectionParams);
+
         // should the key be automatically generated using the inputs or is the cache key set?
         if ($this->cacheKey === null) {
             $cacheKey = sha1($realCacheKey);

diff --git a/lib/Doctrine/DBAL/Connection.php b/lib/Doctrine/DBAL/Connection.php
@@ -879,7 +879,7 @@ public function executeCacheQuery($query, $params, $types, QueryCacheProfile $qc
             throw CacheException::noResultDriverConfigured();
         }
 
-        list($cacheKey, $realKey) = $qcp->generateCacheKeys($query, $params, $types);
+        list($cacheKey, $realKey) = $qcp->generateCacheKeys($query, $params, $types, $this->getParams());
 
         // fetch the row pointers entry
         if ($data = $resultCache->fetch($cacheKey)) {

diff --git a/tests/Doctrine/Tests/DBAL/Cache/QueryCacheProfileTest.php b/tests/Doctrine/Tests/DBAL/Cache/QueryCacheProfileTest.php
@@ -0,0 +1,145 @@
+<?php
+
+namespace Doctrine\Tests\DBAL\Cache;
+
+use Doctrine\DBAL\Cache\QueryCacheProfile;
+use Doctrine\Tests\DbalTestCase;
+use PDO;
+
+class QueryCacheProfileTest extends DbalTestCase
+{
+    const LIFETIME = 3600;
+    const CACHE_KEY = 'user_specified_cache_key';
+
+    /** @var QueryCacheProfile */
+    private $queryCacheProfile;
+
+    protected function setUp()
+    {
+        $this->queryCacheProfile = new QueryCacheProfile(self::LIFETIME, self::CACHE_KEY);
+    }
+
+    public function testShouldUseTheGivenCacheKeyIfPresent()
+    {
+        $query  = 'SELECT * FROM foo WHERE bar = ?';
+        $params = [666];
+        $types  = [PDO::PARAM_INT];
+
+        $connectionParams = array(
+            'dbname'   => 'database_name',
+            'user'     => 'database_user',
+            'password' => 'database_password',
+            'host'     => 'database_host',
+            'driver'   => 'database_driver'
+        );
+
+        list($cacheKey) = $this->queryCacheProfile->generateCacheKeys(
+            $query,
+            $params,
+            $types,
+            $connectionParams
+        );
+
+        $this->assertEquals(self::CACHE_KEY, $cacheKey, 'The returned cache key should match the given one');
+    }
+
+    public function testShouldGenerateAnAutomaticKeyIfNoKeyHasBeenGiven()
+    {
+        $query  = 'SELECT * FROM foo WHERE bar = ?';
+        $params = [666];
+        $types  = [PDO::PARAM_INT];
+
+        $connectionParams = array(
+            'dbname'   => 'database_name',
+            'user'     => 'database_user',
+            'password' => 'database_password',
+            'host'     => 'database_host',
+            'driver'   => 'database_driver'
+        );
+
+        $this->queryCacheProfile = $this->queryCacheProfile->setCacheKey(null);
+
+        list($cacheKey) = $this->queryCacheProfile->generateCacheKeys(
+            $query,
+            $params,
+            $types,
+            $connectionParams
+        );
+
+        $this->assertNotEquals(
+            self::CACHE_KEY,
+            $cacheKey,
+            'The returned cache key should be generated automatically'
+        );
+
+        $this->assertNotEmpty($cacheKey, 'The generated cache key should not be empty');
+    }
+
+    public function testShouldGenerateDifferentKeysForSameQueryAndParamsAndDifferentConnections()
+    {
+        $query  = 'SELECT * FROM foo WHERE bar = ?';
+        $params = [666];
+        $types  = [PDO::PARAM_INT];
+
+        $connectionParams = array(
+            'dbname'   => 'database_name',
+            'user'     => 'database_user',
+            'password' => 'database_password',
+            'host'     => 'database_host',
+            'driver'   => 'database_driver'
+        );
+
+        $this->queryCacheProfile = $this->queryCacheProfile->setCacheKey(null);
+
+        list($firstCacheKey) = $this->queryCacheProfile->generateCacheKeys(
+            $query,
+            $params,
+            $types,
+            $connectionParams
+        );
+
+        $connectionParams['host'] = 'a_different_host';
+
+        list($secondCacheKey) = $this->queryCacheProfile->generateCacheKeys(
+            $query,
+            $params,
+            $types,
+            $connectionParams
+        );
+
+        $this->assertNotEquals($firstCacheKey, $secondCacheKey, 'Cache keys should be different');
+    }
+
+    public function testShouldGenerateSameKeysIfNoneOfTheParamsChanges()
+    {
+        $query  = 'SELECT * FROM foo WHERE bar = ?';
+        $params = [666];
+        $types  = [PDO::PARAM_INT];
+
+        $connectionParams = array(
+            'dbname'   => 'database_name',
+            'user'     => 'database_user',
+            'password' => 'database_password',
+            'host'     => 'database_host',
+            'driver'   => 'database_driver'
+        );
+
+        $this->queryCacheProfile = $this->queryCacheProfile->setCacheKey(null);
+
+        list($firstCacheKey) = $this->queryCacheProfile->generateCacheKeys(
+            $query,
+            $params,
+            $types,
+            $connectionParams
+        );
+
+        list($secondCacheKey) = $this->queryCacheProfile->generateCacheKeys(
+            $query,
+            $params,
+            $types,
+            $connectionParams
+        );
+
+        $this->assertEquals($firstCacheKey, $secondCacheKey, 'Cache keys should be the same');
+    }
+}
diff --git a/tests/Doctrine/Tests/DBAL/ConnectionTest.php b/tests/Doctrine/Tests/DBAL/ConnectionTest.php
@@ -2,12 +2,16 @@
 
 namespace Doctrine\Tests\DBAL;
 
+use Doctrine\Common\Cache\Cache;
 use Doctrine\Common\EventManager;
+use Doctrine\DBAL\Cache\QueryCacheProfile;
 use Doctrine\DBAL\Configuration;
 use Doctrine\DBAL\Connection;
+use Doctrine\DBAL\Driver;
 use Doctrine\DBAL\Events;
 use Doctrine\Tests\Mocks\DriverConnectionMock;
 use Doctrine\Tests\Mocks\DriverMock;
+use Doctrine\DBAL\Cache\ArrayStatement;
 
 class ConnectionTest extends \Doctrine\Tests\DbalTestCase
 {
@@ -670,4 +674,42 @@ public function testPlatformDetectionIsTriggerOnlyOnceOnRetrievingPlatform()
 
         $this->assertSame($platformMock, $connection->getDatabasePlatform());
     }
+
+    public function testConnectionParamsArePassedToTheQueryCacheProfileInExecuteCacheQuery()
+    {
+        $resultCacheDriverMock = $this->createMock(Cache::class);
+
+        $resultCacheDriverMock
+            ->expects($this->atLeastOnce())
+            ->method('fetch')
+            ->with('cacheKey')
+            ->will($this->returnValue(['realKey' => []]));
+
+        $query  = 'SELECT * FROM foo WHERE bar = ?';
+        $params = [666];
+        $types  = [\PDO::PARAM_INT];
+
+        /* @var $queryCacheProfileMock QueryCacheProfile|\PHPUnit_Framework_MockObject_MockObject */
+        $queryCacheProfileMock = $this->createMock(QueryCacheProfile::class);
+
+        $queryCacheProfileMock
+            ->expects($this->any())
+            ->method('getResultCacheDriver')
+            ->will($this->returnValue($resultCacheDriverMock));
+
+        // This is our main expectation
+        $queryCacheProfileMock
+            ->expects($this->once())
+            ->method('generateCacheKeys')
+            ->with($query, $params, $types, $this->params)
+            ->will($this->returnValue(['cacheKey', 'realKey']));
+
+        /* @var $driver Driver */
+        $driver = $this->createMock(Driver::class);
+
+        $this->assertInstanceOf(
+            ArrayStatement::class,
+            (new Connection($this->params, $driver))->executeCacheQuery($query, $params, $types, $queryCacheProfileMock)
+        );
+    }
 }
diff --git a/tests/Doctrine/Tests/DBAL/Schema/DB2SchemaManagerTest.php b/tests/Doctrine/Tests/DBAL/Schema/DB2SchemaManagerTest.php
@@ -27,13 +27,13 @@ final class DB2SchemaManagerTest extends \PHPUnit_Framework_TestCase
     protected function setUp()
     {
         $eventManager = new EventManager();
-        $driverMock = $this->getMock(Driver::class);
-        $platform = $this->getMock(DB2Platform::class);
-        $this->conn = $this->getMock(
-            Connection::class,
-            ['fetchAll'],
-            [['platform' => $platform], $driverMock, new Configuration(), $eventManager]
-        );
+        $driverMock = $this->createMock(Driver::class);
+        $platform = $this->createMock(DB2Platform::class);
+        $this->conn = $this
+            ->getMockBuilder(Connection::class)
+            ->setMethods(['fetchAll'])
+            ->setConstructorArgs([['platform' => $platform], $driverMock, new Configuration(), $eventManager])
+            ->getMock();
         $this->manager = new DB2SchemaManager($this->conn);
     }
 

diff --git a/tests/travis/pgsql.travis.xml b/tests/travis/pgsql.travis.xml
@@ -34,4 +34,4 @@
             <group>locking_functional</group>
         </exclude>
     </groups>
-</phpunit>
+</phpunit>
diff --git a/tests/travis/sqlite.travis.xml b/tests/travis/sqlite.travis.xml
@@ -21,4 +21,4 @@
             <group>locking_functional</group>
         </exclude>
     </groups>
-</phpunit>
+</phpunit>