[2.0] Small refactorings on the cache drivers. Introduced flag to control...

[2.0] Small refactorings on the cache drivers. Introduced flag to control whether the cache driver should manage the cache keys since this is an advanced feature that is not always needed and can have negative side-effects (higher probability for cache slams).

lib/Doctrine/Common/Annotations/AnnotationReader.php

@@ -62,8 +62,8 @@ class AnnotationReader
     private $_cache;
     
     /**
-     * Constructor. Initiaizes a new AnnotationReader that uses the given 
-     * Cache provider to cache annotations.
+     * Constructor. Initializes a new AnnotationReader that uses the given 
+     * Cache provider.
      * 
      * @param Cache $cache The cache provider to use.
      */

lib/Doctrine/Common/Cache/AbstractCache.php

@@ -22,7 +22,7 @@
 namespace Doctrine\Common\Cache;
 
 /**
- * Abstract cache driver class
+ * Base class for cache driver implementations.
  *
  * @license http://www.opensource.org/licenses/lgpl-license.php LGPL
  * @link    www.doctrine-project.org
@@ -34,14 +34,44 @@ namespace Doctrine\Common\Cache;
  */
 abstract class AbstractCache implements Cache
 {
-    /* @var string $cacheIdsIndexId   The cache id to store the index of cache ids under */
+    /** @var string The cache id to store the index of cache ids under */
     private $_cacheIdsIndexId = 'doctrine_cache_ids';
 
-    /* @var string $namespace   The namespace to prefix all cache ids with */
+    /** @var string The namespace to prefix all cache ids with */
     private $_namespace = null;
+    
+    /** @var boolean Whether to manage cache keys or not. */
+    private $_manageCacheKeys = false;
+    
+    /**
+     * Sets whether cache keys should be managed by the cache driver
+     * separately from the cache entries. This allows more granular
+     * cache clearing through {@link deleteByPrefix}, {@link deleteByRegex},
+     * {@link deleteBySuffix} and some other operations such as {@link count}
+     * and {@link getIds}. Managing cache keys comes at the cost of a higher
+     * probability for cache slams due to the single cache key used for
+     * managing all other keys.
+     * 
+     * @param boolean $bool
+     */
+    public function setManageCacheKeys($bool)
+    {
+        $this->_manageCacheKeys = $bool;
+    }
+    
+    /**
+     * Checks whether cache keys are managed by this cache driver.
+     * 
+     * @return boolean
+     * @see setManageCacheKeys()
+     */
+    public function getManageCacheKeys()
+    {
+        return $this->_manageCacheKeys;
+    }
 
     /**
-     * Set the namespace to prefix all cache ids with
+     * Set the namespace to prefix all cache ids with.
      *
      * @param string $namespace
      * @return void
@@ -56,8 +86,7 @@ abstract class AbstractCache implements Cache
      */
     public function fetch($id)
     {
-        $id = $this->_getNamespacedId($id);
-        return $this->_doFetch($id);
+        return $this->_doFetch($this->_getNamespacedId($id));
     }
 
     /**
@@ -65,8 +94,7 @@ abstract class AbstractCache implements Cache
      */
     public function contains($id)
     {
-        $id = $this->_getNamespacedId($id);
-        return $this->_doContains($id);
+        return $this->_doContains($this->_getNamespacedId($id));
     }
 
     /**
@@ -76,7 +104,9 @@ abstract class AbstractCache implements Cache
     {
         $id = $this->_getNamespacedId($id);
         if ($this->_doSave($id, $data, $lifeTime)) {
-            $this->_saveId($id);
+            if ($this->_manageCacheKeys) {
+                $this->_saveId($id);
+            }
 
             return true;
         }
@@ -95,7 +125,9 @@ abstract class AbstractCache implements Cache
         }
 
         if ($this->_doDelete($id)) {
-            $this->_deleteId($id);
+            if ($this->_manageCacheKeys) {
+                $this->_deleteId($id);
+            }
 
             return true;
         }
@@ -109,6 +141,7 @@ abstract class AbstractCache implements Cache
      */
     public function deleteAll()
     {
+        $this->_errorIfCacheKeysNotManaged();
         $ids = $this->getIds();
         foreach ($ids as $id) {
             $this->delete($id);
@@ -124,6 +157,7 @@ abstract class AbstractCache implements Cache
      */
     public function deleteByRegex($regex)
     {
+        $this->_errorIfCacheKeysNotManaged();
         $deleted = array();
         $ids = $this->getIds();
         foreach ($ids as $id) {
@@ -143,6 +177,7 @@ abstract class AbstractCache implements Cache
      */
     public function deleteByPrefix($prefix)
     {
+        $this->_errorIfCacheKeysNotManaged();
         $deleted = array();
         $ids = $this->getIds();
         foreach ($ids as $id) {
@@ -162,6 +197,7 @@ abstract class AbstractCache implements Cache
      */
     public function deleteBySuffix($suffix)
     {
+        $this->_errorIfCacheKeysNotManaged();
         $deleted = array();
         $ids = $this->getIds();
         foreach ($ids as $id) {
@@ -180,6 +216,7 @@ abstract class AbstractCache implements Cache
      */
     public function count() 
     {
+        $this->_errorIfCacheKeysNotManaged();
         $ids = $this->getIds();
         return $ids ? count($ids) : 0;
     }
@@ -191,6 +228,7 @@ abstract class AbstractCache implements Cache
      */
     public function getIds()
     {
+        $this->_errorIfCacheKeysNotManaged();
         $ids = $this->fetch($this->_cacheIdsIndexId);
         return $ids ? $ids : array();
     }
@@ -243,6 +281,16 @@ abstract class AbstractCache implements Cache
         }
         return false;
     }
+    
+    /**
+     * @throws BadMethodCallException If the cache driver does not manage cache keys.
+     */
+    private function _errorIfCacheKeysNotManaged()
+    {
+        if ( ! $this->_manageCacheKeys) {
+            throw new \BadMethodCallException("Operation not supported if cache keys are not managed.");
+        }
+    }
 
     /**
      * Fetches an entry from the cache.

lib/Doctrine/DBAL/Connection.php

@@ -96,35 +96,28 @@ class Connection
      *
      * @var boolean
      */
-    protected $_isConnected = false;
+    private $_isConnected = false;
 
     /**
      * The transaction nesting level.
      *
      * @var integer
      */
-    protected $_transactionNestingLevel = 0;
+    private $_transactionNestingLevel = 0;
 
     /**
      * The currently active transaction isolation level.
      *
      * @var integer
      */
-    protected $_transactionIsolationLevel;
+    private $_transactionIsolationLevel;
 
     /**
      * The parameters used during creation of the Connection instance.
      *
      * @var array
      */
-    protected $_params = array();
-
-    /**
-     * The query count. Represents the number of executed database queries by the connection.
-     *
-     * @var integer
-     */
-    protected $_queryCount = 0;
+    private $_params = array();
 
     /**
      * The DatabasePlatform object that provides information about the
@@ -147,6 +140,13 @@ class Connection
      * @var Doctrine\DBAL\Driver
      */
     protected $_driver;
+    
+    /**
+     * Flag that indicates whether the current transaction is marked for rollback only.
+     * 
+     * @var boolean
+     */
+    private $_isRollbackOnly = false;
 
     /**
      * Initializes a new instance of the Connection class.
@@ -602,8 +602,6 @@ class Connection
             $stmt = $this->_conn->query($query);
         }
         
-        $this->_queryCount++;
-        
         return $stmt;
     }
 
@@ -629,22 +627,10 @@ class Connection
         } else {
             $result = $this->_conn->exec($query);
         }
-
-        $this->_queryCount++;
         
         return $result;
     }
 
-    /**
-     * Returns the number of queries executed by the connection.
-     *
-     * @return integer
-     */
-    public function getQueryCount()
-    {
-        return $this->_queryCount;
-    }
-
     /**
      * Returns the current transaction nesting level.
      *
@@ -702,7 +688,7 @@ class Connection
      * if trying to set a savepoint and there is no active transaction
      * a new transaction is being started.
      *
-     * @return boolean
+     * @return void
      */
     public function beginTransaction()
     {
@@ -713,8 +699,6 @@ class Connection
         }
         
         ++$this->_transactionNestingLevel;
-        
-        return true;
     }
 
     /**
@@ -722,13 +706,17 @@ class Connection
      * progress or release a savepoint. This function may only be called when
      * auto-committing is disabled, otherwise it will fail.
      *
-     * @return boolean FALSE if commit couldn't be performed, TRUE otherwise
+     * @return void
+     * @throws ConnectionException If the commit failed.
      */
     public function commit()
     {
         if ($this->_transactionNestingLevel == 0) {
             throw ConnectionException::commitFailedNoActiveTransaction();
         }
+        if ($this->_isRollbackOnly) {
+            throw ConnectionException::commitFailedRollbackOnly();
+        }
 
         $this->connect();
 
@@ -737,8 +725,6 @@ class Connection
         }
         
         --$this->_transactionNestingLevel;
-
-        return true;
     }
 
     /**
@@ -751,8 +737,7 @@ class Connection
      * eventlistener methods
      *
      * @param string $savepoint                 Name of a savepoint to rollback to.
-     * @throws Doctrine\DBAL\ConnectionException   If the rollback operation fails at database level.
-     * @return boolean                          FALSE if rollback couldn't be performed, TRUE otherwise.
+     * @throws ConnectionException   If the rollback operation fails at database level.
      */
     public function rollback()
     {
@@ -765,11 +750,10 @@ class Connection
         if ($this->_transactionNestingLevel == 1) {
             $this->_transactionNestingLevel = 0;
             $this->_conn->rollback();
+            $this->_isRollbackOnly = false;
         } else {
             --$this->_transactionNestingLevel;
         }
-
-        return true;
     }
 
     /**
@@ -798,4 +782,32 @@ class Connection
         
         return $this->_schemaManager;
     }
+    
+    /**
+     * Marks the current transaction so that the only possible
+     * outcome for the transaction to be rolled back.
+     * 
+     * @throws BadMethodCallException If no transaction is active.
+     */
+    public function setRollbackOnly()
+    {
+        if ($this->_transactionNestingLevel == 0) {
+            throw ConnectionException::noActiveTransaction();
+        }
+        $this->_isRollbackOnly = true;
+    }
+    
+    /**
+     * Check whether the current transaction is marked for rollback only.
+     * 
+     * @return boolean
+     * @throws BadMethodCallException If no transaction is active.
+     */
+    public function getRollbackOnly()
+    {
+        if ($this->_transactionNestingLevel == 0) {
+            throw ConnectionException::noActiveTransaction();
+        }
+        return $this->_isRollbackOnly;
+    }
 }

lib/Doctrine/ORM/AbstractQuery.php

@@ -262,14 +262,14 @@ abstract class AbstractQuery
     }
 
     /**
-     * Defines if the resultset cache is active or not.
+     * Defines if the result cache is active or not.
      *
      * @param boolean $expire Whether or not to force resultset cache expiration.
      * @return Doctrine_ORM_Query
      */
     public function setExpireResultCache($expire = true)
     {
-        $this->_expireResultCache = (bool) $expire;
+        $this->_expireResultCache = $expire;
 
         return $this;
     }
@@ -445,7 +445,7 @@ abstract class AbstractQuery
         // Check result cache
         if ($cacheDriver = $this->getResultCacheDriver()) {
             $id = $this->_getResultCacheId($params);
-            $cached = ($this->_expireResultCache) ? false : $cacheDriver->fetch($id);
+            $cached = $this->_expireResultCache ? false : $cacheDriver->fetch($id);
 
             if ($cached === false) {
                 // Cache miss.

lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php

@@ -91,7 +91,7 @@ class ClassMetadataInfo
     const GENERATOR_TYPE_IDENTITY = 4;
     /**
      * NONE means the class does not have a generated id. That means the class
-     * must have a natural id.
+     * must have a natural, manually assigned id.
      */
     const GENERATOR_TYPE_NONE = 5;
     /**
@@ -105,7 +105,7 @@ class ClassMetadataInfo
     /**
      * DEFERRED_EXPLICIT means that changes of entities are calculated at commit-time
      * by doing a property-by-property comparison with the original data. This will
-     * be done only for entities that were explicitly saved (through save() or cascade).
+     * be done only for entities that were explicitly saved (through persist() or a cascade).
      */
     const CHANGETRACKING_DEFERRED_EXPLICIT = 2;
     /**

lib/Doctrine/ORM/UnitOfWork.php

@@ -204,6 +204,7 @@ class UnitOfWork implements PropertyChangedListener
     private $_collectionPersisters = array();
 
     /**
+     * EXPERIMENTAL:
      * Flag for whether or not to make use of the C extension.
      *
      * @var boolean
@@ -315,6 +316,7 @@ class UnitOfWork implements PropertyChangedListener
 
             $conn->commit();
         } catch (\Exception $e) {
+            $conn->setRollbackOnly();
             $conn->rollback();
             $this->clear();
             throw $e;

tests/Doctrine/Tests/Common/Cache/ApcCacheTest.php

@@ -30,6 +30,7 @@ class ApcCacheTest extends \Doctrine\Tests\DoctrineTestCase
 
         // Test delete
         $cache->save('test_key2', 'test2');
+        
         $cache->delete('test_key2');
         $this->assertFalse($cache->contains('test_key2'));
     }

tests/Doctrine/Tests/Common/Cache/ArrayCacheTest.php

@@ -11,6 +11,7 @@ class ArrayCacheTest extends \Doctrine\Tests\DoctrineTestCase
     public function testArrayCacheDriver()
     {
         $cache = new ArrayCache();
+        $cache->setManageCacheKeys(true);
 
         // Test save
         $cache->save('test_key', 'testing this out');

tests/Doctrine/Tests/Common/Cache/CacheTest.php

@@ -11,6 +11,7 @@ class CacheTest extends \Doctrine\Tests\DoctrineTestCase
     public function testCount()
     {
         $cache = new ArrayCache();
+        $cache->setManageCacheKeys(true);
         $cache->save('test_key1', '1');
         $cache->save('test_key2', '2');
         $this->assertEquals($cache->count(), 2);
@@ -19,6 +20,7 @@ class CacheTest extends \Doctrine\Tests\DoctrineTestCase
     public function testDeleteAll()
     {
         $cache = new ArrayCache();
+        $cache->setManageCacheKeys(true);
         $cache->save('test_key1', '1');
         $cache->save('test_key2', '2');
         $cache->deleteAll();
@@ -29,6 +31,7 @@ class CacheTest extends \Doctrine\Tests\DoctrineTestCase
     public function testDeleteByRegex()
     {
         $cache = new ArrayCache();
+        $cache->setManageCacheKeys(true);
         $cache->save('test_key1', '1');
         $cache->save('test_key2', '2');
         $cache->deleteByRegex('/test_key[0-9]/');
@@ -39,6 +42,7 @@ class CacheTest extends \Doctrine\Tests\DoctrineTestCase
     public function testDeleteByPrefix()
     {
         $cache = new ArrayCache();
+        $cache->setManageCacheKeys(true);
         $cache->save('test_key1', '1');
         $cache->save('test_key2', '2');
         $cache->deleteByPrefix('test_key');
@@ -49,6 +53,7 @@ class CacheTest extends \Doctrine\Tests\DoctrineTestCase
     public function testDeleteBySuffix()
     {
         $cache = new ArrayCache();
+        $cache->setManageCacheKeys(true);
         $cache->save('1test_key', '1');
         $cache->save('2test_key', '2');
         $cache->deleteBySuffix('test_key');
@@ -59,6 +64,7 @@ class CacheTest extends \Doctrine\Tests\DoctrineTestCase
     public function testDeleteByWildcard()
     {
         $cache = new ArrayCache();
+        $cache->setManageCacheKeys(true);
         $cache->save('test_key1', '1');
         $cache->save('test_key2', '2');
         $cache->delete('test_key*');
@@ -69,6 +75,7 @@ class CacheTest extends \Doctrine\Tests\DoctrineTestCase
     public function testNamespace()
     {
         $cache = new ArrayCache();
+        $cache->setManageCacheKeys(true);
         $cache->setNamespace('test_');
         $cache->save('key1', 'test');
         $this->assertTrue($cache->contains('key1'));

tests/Doctrine/Tests/ORM/Functional/ManyToManyBidirectionalAssociationTest.php

@@ -90,7 +90,7 @@ class ManyToManyBidirectionalAssociationTest extends AbstractManyToManyAssociati
 
         $query = $this->_em->createQuery('SELECT c FROM Doctrine\Tests\Models\ECommerce\ECommerceCategory c order by c.id');
         $categories = $query->getResult();
-        $this->assertLoadingOfInverseSide($categories); 
+        $this->assertLoadingOfInverseSide($categories);
     }
 
     public function testLazyLoadsCollectionOnTheOwningSide()
@@ -107,7 +107,7 @@ class ManyToManyBidirectionalAssociationTest extends AbstractManyToManyAssociati
         $this->assertEquals(0, count($products[0]->getCategories()->unwrap()));
         $this->assertEquals(0, count($products[1]->getCategories()->unwrap()));
         
-        $this->assertLoadingOfOwningSide($products); 
+        $this->assertLoadingOfOwningSide($products);
     }
 
 
@@ -161,5 +161,13 @@ class ManyToManyBidirectionalAssociationTest extends AbstractManyToManyAssociati
     {
         $this->assertTrue($categories[0] instanceof ECommerceCategory);
         $this->assertTrue($categories[1] instanceof ECommerceCategory);
+        
+        $this->assertEquals(2, count($categories[0]->getProducts()));
+        $this->assertTrue($categories[0]->getProducts()->get(0) instanceof ECommerceProduct);
+        $this->assertTrue($categories[0]->getProducts()->get(1) instanceof ECommerceProduct);
+        
+        $this->assertEquals(2, count($categories[1]->getProducts()));
+        $this->assertTrue($categories[1]->getProducts()->get(0) instanceof ECommerceProduct);
+        $this->assertTrue($categories[1]->getProducts()->get(1) instanceof ECommerceProduct);
     }
 }

tests/Doctrine/Tests/ORM/Functional/QueryCacheTest.php

@@ -33,6 +33,7 @@ class QueryCacheTest extends \Doctrine\Tests\OrmFunctionalTestCase
 
         $query = $this->_em->createQuery('select ux from Doctrine\Tests\Models\CMS\CmsUser ux');
         $cache = new ArrayCache;
+        $cache->setManageCacheKeys(true);
         $query->setQueryCacheDriver($cache);
 		$this->assertEquals(0, $cache->count());
 		

tests/Doctrine/Tests/ORM/Functional/ResultCacheTest.php

@@ -31,14 +31,12 @@ class ResultCacheTest extends \Doctrine\Tests\OrmFunctionalTestCase
 
         $query = $this->_em->createQuery('select ux from Doctrine\Tests\Models\CMS\CmsUser ux');
         $cache = new ArrayCache;
+        $cache->setManageCacheKeys(true);
         $query->setResultCache($cache);
 		$this->assertEquals(0, $cache->count());
 		
-		$initialQueryCount = $this->_em->getConnection()->getQueryCount();
-		
         $users = $query->getResult();
 
-        $this->assertEquals($initialQueryCount + 1, $this->_em->getConnection()->getQueryCount());
        	$this->assertEquals(1, $cache->count());
         $this->assertEquals(1, count($users));
         $this->assertEquals('Roman', $users[0]->name);
@@ -50,7 +48,6 @@ class ResultCacheTest extends \Doctrine\Tests\OrmFunctionalTestCase
         
         $users = $query2->getResult();
 
-        $this->assertEquals($initialQueryCount + 1, $this->_em->getConnection()->getQueryCount());
        	$this->assertEquals(1, $cache->count());
         $this->assertEquals(1, count($users));
         $this->assertEquals('Roman', $users[0]->name);