From eda3820df5525f9e97c0a8dcb37dc40777e69751 Mon Sep 17 00:00:00 2001 From: Antonio Vilar Date: Sat, 1 Nov 2014 09:02:07 +0100 Subject: [PATCH 1/5] Used connection params in cache key generation --- lib/Doctrine/DBAL/Cache/QueryCacheProfile.php | 11 ++++++++--- lib/Doctrine/DBAL/Connection.php | 2 +- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/lib/Doctrine/DBAL/Cache/QueryCacheProfile.php b/lib/Doctrine/DBAL/Cache/QueryCacheProfile.php index 330eec91f23..550ffad1e44 100644 --- 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, $connectionParams = array()) { - $realCacheKey = $query . "-" . serialize($params) . "-" . serialize($types); + $realCacheKey = 'query=' . $query . + '¶ms=' . serialize($params) . + '&types=' . serialize($types) . + (!empty($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 index cb11afa905c..23889bb2394 100644 --- a/lib/Doctrine/DBAL/Connection.php +++ b/lib/Doctrine/DBAL/Connection.php @@ -873,7 +873,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)) { From 536d192faeaa6f8c9e39f7adcb7d7c9021df9ae9 Mon Sep 17 00:00:00 2001 From: Antonio Vilar Date: Sat, 8 Nov 2014 09:54:42 +0100 Subject: [PATCH 2/5] Added tests for Cache\QueryCacheProfile --- .../DBAL/Cache/QueryCacheProfileTest.php | 78 +++++++++++++++++++ 1 file changed, 78 insertions(+) create mode 100644 tests/Doctrine/Tests/DBAL/Cache/QueryCacheProfileTest.php diff --git a/tests/Doctrine/Tests/DBAL/Cache/QueryCacheProfileTest.php b/tests/Doctrine/Tests/DBAL/Cache/QueryCacheProfileTest.php new file mode 100644 index 00000000000..a1f6260a058 --- /dev/null +++ b/tests/Doctrine/Tests/DBAL/Cache/QueryCacheProfileTest.php @@ -0,0 +1,78 @@ +queryCacheProfile = new QueryCacheProfile(self::LIFETIME, self::CACHE_KEY); + } + + /** + * @test + */ + public function it_should_use_the_given_cache_key_if_present() + { + $query = 'SELECT * FROM foo WHERE bar = ?'; + $params = array(666); + $types = array(\PDO::PARAM_INT); + + $connectionParams = array( + 'dbname' => 'database_name', + 'user' => 'database_user', + 'password' => 'database_password', + 'host' => 'database_host', + 'driver' => 'database_driver' + ); + + $expectedRealCacheKey = 'query=SELECT * FROM foo WHERE bar = ?¶ms=a:1:{i:0;i:666;}&types=a:1:{i:0;' + . 'i:1;}a:5:{s:6:"dbname";s:13:"database_name";s:4:"user";s:13:"database_user";s:8:"password";s:17:"' + . 'database_password";s:4:"host";s:13:"database_host";s:6:"driver";s:15:"database_driver";}'; + + $cacheKeysResult = $this->queryCacheProfile->generateCacheKeys($query, $params, $types, $connectionParams); + + $this->assertEquals(self::CACHE_KEY, $cacheKeysResult[0], 'The returned cached key should match the given one'); + $this->assertEquals($expectedRealCacheKey, $cacheKeysResult[1]); + } + + /** + * @test + */ + public function it_should_generate_an_automatic_key_if_no_key_has_been_specified() + { + $query = 'SELECT * FROM foo WHERE bar = ?'; + $params = array(666); + $types = array(\PDO::PARAM_INT); + + $connectionParams = array( + 'dbname' => 'database_name', + 'user' => 'database_user', + 'password' => 'database_password', + 'host' => 'database_host', + 'driver' => 'database_driver' + ); + + $expectedRealCacheKey = 'query=SELECT * FROM foo WHERE bar = ?¶ms=a:1:{i:0;i:666;}&types=a:1:{i:0;' + . 'i:1;}a:5:{s:6:"dbname";s:13:"database_name";s:4:"user";s:13:"database_user";s:8:"password";s:17:"' + . 'database_password";s:4:"host";s:13:"database_host";s:6:"driver";s:15:"database_driver";}'; + + $this->queryCacheProfile = $this->queryCacheProfile->setCacheKey(null); + + $cacheKeysResult = $this->queryCacheProfile->generateCacheKeys($query, $params, $types, $connectionParams); + + $this->assertEquals('8f74136f51719d57f4da9b6d2ba955b42189bb81', $cacheKeysResult[0]); + $this->assertEquals($expectedRealCacheKey, $cacheKeysResult[1]); + } +} From dfbe7721b83db6144886481fa274e8a831971d96 Mon Sep 17 00:00:00 2001 From: Antonio Vilar Date: Sat, 8 Nov 2014 20:53:05 +0100 Subject: [PATCH 3/5] Added new tests for QueryCacheProfile and Connection --- .../DBAL/Cache/QueryCacheProfileTest.php | 110 ++++++++++++++---- tests/Doctrine/Tests/DBAL/ConnectionTest.php | 34 ++++++ 2 files changed, 124 insertions(+), 20 deletions(-) diff --git a/tests/Doctrine/Tests/DBAL/Cache/QueryCacheProfileTest.php b/tests/Doctrine/Tests/DBAL/Cache/QueryCacheProfileTest.php index a1f6260a058..76c0776c34e 100644 --- a/tests/Doctrine/Tests/DBAL/Cache/QueryCacheProfileTest.php +++ b/tests/Doctrine/Tests/DBAL/Cache/QueryCacheProfileTest.php @@ -20,10 +20,7 @@ protected function setUp() $this->queryCacheProfile = new QueryCacheProfile(self::LIFETIME, self::CACHE_KEY); } - /** - * @test - */ - public function it_should_use_the_given_cache_key_if_present() + public function testShouldUseTheGivenCacheKeyIfPresent() { $query = 'SELECT * FROM foo WHERE bar = ?'; $params = array(666); @@ -37,20 +34,43 @@ public function it_should_use_the_given_cache_key_if_present() 'driver' => 'database_driver' ); - $expectedRealCacheKey = 'query=SELECT * FROM foo WHERE bar = ?¶ms=a:1:{i:0;i:666;}&types=a:1:{i:0;' - . 'i:1;}a:5:{s:6:"dbname";s:13:"database_name";s:4:"user";s:13:"database_user";s:8:"password";s:17:"' - . 'database_password";s:4:"host";s:13:"database_host";s:6:"driver";s:15:"database_driver";}'; + $generatedKeys = $this->queryCacheProfile->generateCacheKeys( + $query, + $params, + $types, + $connectionParams + ); + + $this->assertEquals(self::CACHE_KEY, $generatedKeys[0], 'The returned cached key should match the given one'); + } + + public function testShouldGenerateAnAutomaticKeyIfNoKeyHasBeenGiven() + { + $query = 'SELECT * FROM foo WHERE bar = ?'; + $params = array(666); + $types = array(\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); - $cacheKeysResult = $this->queryCacheProfile->generateCacheKeys($query, $params, $types, $connectionParams); + $generatedKeys = $this->queryCacheProfile->generateCacheKeys( + $query, + $params, + $types, + $connectionParams + ); - $this->assertEquals(self::CACHE_KEY, $cacheKeysResult[0], 'The returned cached key should match the given one'); - $this->assertEquals($expectedRealCacheKey, $cacheKeysResult[1]); + $this->assertNotEmpty($generatedKeys[0], 'The generated cache key should not be empty'); } - /** - * @test - */ - public function it_should_generate_an_automatic_key_if_no_key_has_been_specified() + public function testShouldGenerateDifferentKeysForSameQueryAndParamsAndDifferentConnections() { $query = 'SELECT * FROM foo WHERE bar = ?'; $params = array(666); @@ -64,15 +84,65 @@ public function it_should_generate_an_automatic_key_if_no_key_has_been_specified 'driver' => 'database_driver' ); - $expectedRealCacheKey = 'query=SELECT * FROM foo WHERE bar = ?¶ms=a:1:{i:0;i:666;}&types=a:1:{i:0;' - . 'i:1;}a:5:{s:6:"dbname";s:13:"database_name";s:4:"user";s:13:"database_user";s:8:"password";s:17:"' - . 'database_password";s:4:"host";s:13:"database_host";s:6:"driver";s:15:"database_driver";}'; + $this->queryCacheProfile = $this->queryCacheProfile->setCacheKey(null); + + $generatedKeys = $this->queryCacheProfile->generateCacheKeys( + $query, + $params, + $types, + $connectionParams + ); + + $firstCacheKey = $generatedKeys[0]; + + $connectionParams['host'] = 'a_different_host'; + + $generatedKeys = $this->queryCacheProfile->generateCacheKeys( + $query, + $params, + $types, + $connectionParams + ); + + $secondCacheKey = $generatedKeys[0]; + + $this->assertNotEquals($firstCacheKey, $secondCacheKey, 'Cache keys should be different'); + } + + public function testShouldGenerateSameKeysIfNoneOfTheParamsChanges() + { + $query = 'SELECT * FROM foo WHERE bar = ?'; + $params = array(666); + $types = array(\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); - $cacheKeysResult = $this->queryCacheProfile->generateCacheKeys($query, $params, $types, $connectionParams); + $generatedKeys = $this->queryCacheProfile->generateCacheKeys( + $query, + $params, + $types, + $connectionParams + ); + + $firstCacheKey = $generatedKeys[0]; + + $generatedKeys = $this->queryCacheProfile->generateCacheKeys( + $query, + $params, + $types, + $connectionParams + ); + + $secondCacheKey = $generatedKeys[0]; - $this->assertEquals('8f74136f51719d57f4da9b6d2ba955b42189bb81', $cacheKeysResult[0]); - $this->assertEquals($expectedRealCacheKey, $cacheKeysResult[1]); + $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 index 77bc9e87e78..94f98eed8ad 100644 --- a/tests/Doctrine/Tests/DBAL/ConnectionTest.php +++ b/tests/Doctrine/Tests/DBAL/ConnectionTest.php @@ -482,4 +482,38 @@ public function testCallingDeleteWithNoDeletionCriteriaResultsInSqlWithoutWhereC $conn->delete('kittens', array()); } + + public function testConnectionParamsArePassedToTheQueryCacheProfileInExecuteCacheQuery() + { + $resultCacheDriverMock = $this->getMock('Doctrine\Common\Cache\Cache'); + + $resultCacheDriverMock->expects($this->any()) + ->method('fetch') + ->will($this->returnValue(array('realKey' => array()))); + + $query = 'SELECT * FROM foo WHERE bar = ?'; + $params = array(666); + $types = array(\PDO::PARAM_INT); + + $queryCacheProfileMock = $this->getMock('Doctrine\DBAL\Cache\QueryCacheProfile'); + + $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(array('cacheKey', 'realKey'))); + + $conn = new Connection( + $this->params, + $this->getMock('Doctrine\DBAL\Driver') + ); + + $result = $conn->executeCacheQuery($query, $params, $types, $queryCacheProfileMock); + + $this->assertInstanceOf('Doctrine\DBAL\Cache\ArrayStatement', $result); + } } From 73c8278bf6e80bede3a2e5adcc935ff716172717 Mon Sep 17 00:00:00 2001 From: Antonio Vilar Date: Mon, 10 Nov 2014 16:15:02 +0100 Subject: [PATCH 4/5] Some test tweaks --- lib/Doctrine/DBAL/Cache/QueryCacheProfile.php | 2 +- .../Tests/DBAL/Cache/QueryCacheProfileTest.php | 10 +++++++--- tests/Doctrine/Tests/DBAL/ConnectionTest.php | 9 +++++---- 3 files changed, 13 insertions(+), 8 deletions(-) diff --git a/lib/Doctrine/DBAL/Cache/QueryCacheProfile.php b/lib/Doctrine/DBAL/Cache/QueryCacheProfile.php index 550ffad1e44..7b6010b5b15 100644 --- a/lib/Doctrine/DBAL/Cache/QueryCacheProfile.php +++ b/lib/Doctrine/DBAL/Cache/QueryCacheProfile.php @@ -97,7 +97,7 @@ public function getCacheKey() * * @return array */ - public function generateCacheKeys($query, $params, $types, $connectionParams = array()) + public function generateCacheKeys($query, $params, $types, array $connectionParams = array()) { $realCacheKey = 'query=' . $query . '¶ms=' . serialize($params) . diff --git a/tests/Doctrine/Tests/DBAL/Cache/QueryCacheProfileTest.php b/tests/Doctrine/Tests/DBAL/Cache/QueryCacheProfileTest.php index 76c0776c34e..ea9f3a7487a 100644 --- a/tests/Doctrine/Tests/DBAL/Cache/QueryCacheProfileTest.php +++ b/tests/Doctrine/Tests/DBAL/Cache/QueryCacheProfileTest.php @@ -2,8 +2,6 @@ namespace Doctrine\Tests\DBAL\Cache; -require_once __DIR__ . '/../../TestInit.php'; - use Doctrine\DBAL\Cache\QueryCacheProfile; use Doctrine\Tests\DbalTestCase; @@ -41,7 +39,7 @@ public function testShouldUseTheGivenCacheKeyIfPresent() $connectionParams ); - $this->assertEquals(self::CACHE_KEY, $generatedKeys[0], 'The returned cached key should match the given one'); + $this->assertEquals(self::CACHE_KEY, $generatedKeys[0], 'The returned cache key should match the given one'); } public function testShouldGenerateAnAutomaticKeyIfNoKeyHasBeenGiven() @@ -67,6 +65,12 @@ public function testShouldGenerateAnAutomaticKeyIfNoKeyHasBeenGiven() $connectionParams ); + $this->assertNotEquals( + self::CACHE_KEY, + $generatedKeys[0], + 'The returned cache key should be generated automatically' + ); + $this->assertNotEmpty($generatedKeys[0], 'The generated cache key should not be empty'); } diff --git a/tests/Doctrine/Tests/DBAL/ConnectionTest.php b/tests/Doctrine/Tests/DBAL/ConnectionTest.php index 94f98eed8ad..9f5e91f7915 100644 --- a/tests/Doctrine/Tests/DBAL/ConnectionTest.php +++ b/tests/Doctrine/Tests/DBAL/ConnectionTest.php @@ -487,8 +487,9 @@ public function testConnectionParamsArePassedToTheQueryCacheProfileInExecuteCach { $resultCacheDriverMock = $this->getMock('Doctrine\Common\Cache\Cache'); - $resultCacheDriverMock->expects($this->any()) + $resultCacheDriverMock->expects($this->atLeastOnce()) ->method('fetch') + ->with('cacheKey') ->will($this->returnValue(array('realKey' => array()))); $query = 'SELECT * FROM foo WHERE bar = ?'; @@ -512,8 +513,8 @@ public function testConnectionParamsArePassedToTheQueryCacheProfileInExecuteCach $this->getMock('Doctrine\DBAL\Driver') ); - $result = $conn->executeCacheQuery($query, $params, $types, $queryCacheProfileMock); - - $this->assertInstanceOf('Doctrine\DBAL\Cache\ArrayStatement', $result); + $this->assertInstanceOf('Doctrine\DBAL\Cache\ArrayStatement', + $conn->executeCacheQuery($query, $params, $types, $queryCacheProfileMock) + ); } } From d7f6b4c03125c9f960b4043c96af331aa65b96c8 Mon Sep 17 00:00:00 2001 From: Antonio Vilar Date: Mon, 10 Nov 2014 16:59:51 +0100 Subject: [PATCH 5/5] Loaded bootstrap script in all phpunit travis configuration files --- tests/travis/mysql.travis.xml | 2 +- tests/travis/mysqli.travis.xml | 2 +- tests/travis/pgsql.travis.xml | 4 ++-- tests/travis/sqlite.travis.xml | 4 ++-- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/travis/mysql.travis.xml b/tests/travis/mysql.travis.xml index c80f4d29502..26b2d8afda9 100644 --- a/tests/travis/mysql.travis.xml +++ b/tests/travis/mysql.travis.xml @@ -1,5 +1,5 @@ - + diff --git a/tests/travis/mysqli.travis.xml b/tests/travis/mysqli.travis.xml index ace5df463d6..dea808414a9 100644 --- a/tests/travis/mysqli.travis.xml +++ b/tests/travis/mysqli.travis.xml @@ -1,5 +1,5 @@ - + diff --git a/tests/travis/pgsql.travis.xml b/tests/travis/pgsql.travis.xml index 1c34404f76d..9eb6bef3492 100644 --- a/tests/travis/pgsql.travis.xml +++ b/tests/travis/pgsql.travis.xml @@ -1,5 +1,5 @@ - + @@ -25,4 +25,4 @@ locking_functional - \ No newline at end of file + diff --git a/tests/travis/sqlite.travis.xml b/tests/travis/sqlite.travis.xml index 944f137a879..aafd496c90b 100644 --- a/tests/travis/sqlite.travis.xml +++ b/tests/travis/sqlite.travis.xml @@ -1,5 +1,5 @@ - + ./../Doctrine/Tests/DBAL @@ -11,4 +11,4 @@ locking_functional - \ No newline at end of file +