From edd590f833a81f31f54e747811a378a9ca21b718 Mon Sep 17 00:00:00 2001 From: Deni Date: Tue, 20 Dec 2011 20:09:12 +0400 Subject: [PATCH 01/24] Added tests for TransactionalMatcher --- .../Transactions/TransactionalMatcherTest.php | 162 +++++++++++++++++- ...lException.php => TransactionException.php | 0 Transactions/TransactionalMatcher.php | 3 +- 3 files changed, 159 insertions(+), 6 deletions(-) rename TransactionalException.php => TransactionException.php (100%) diff --git a/Tests/Transactions/TransactionalMatcherTest.php b/Tests/Transactions/TransactionalMatcherTest.php index 80632cd..d123faa 100644 --- a/Tests/Transactions/TransactionalMatcherTest.php +++ b/Tests/Transactions/TransactionalMatcherTest.php @@ -2,19 +2,171 @@ namespace SimpleThings\TransactionalBundle\Tests\Transactions; +use Symfony\Component\HttpFoundation\Request; +use SimpleThings\TransactionalBundle\Annotations\Transactional; +use SimpleThings\TransactionalBundle\Transactions\TransactionalMatcher; +use SimpleThings\TransactionalBundle\Transactions\TransactionDefinition; + class TransactionalMatcherTest extends \PHPUnit_Framework_TestCase { private $reader; - private $matcher; - public function setUp() + /** + * @dataProvider getPatterns + */ + public function testMatchPattern($pattern, $method, $matched) { - $this->reader = $this->getMock('Doctrine\Common\Annotations\Reader'); + $pattern = array( + 'pattern' => $pattern, + 'methods' => array('POST', 'PUT'), + 'conn' => array('orm.default'), + 'isolation' => TransactionDefinition::ISOLATION_DEFAULT, + 'propagation' => TransactionDefinition::PROPAGATION_REQUIRED, + 'noRollbackFor' => array(), + 'subrequest' => false, + ); + + $matcher = new TransactionalMatcher(array($pattern), array(), $this->reader); + $request = Request::create('/foo', $method); + $controller = new TestController(); + + $definition = $matcher->match($request, array($controller, 'fooAction')); + + if ($matched) { + $expectedDefinition = new TransactionDefinition(array( + 'orm.default' => array( + 'isolation' => TransactionDefinition::ISOLATION_DEFAULT, + 'propagation' => TransactionDefinition::PROPAGATION_REQUIRED, + 'noRollbackFor' => array(), + 'subrequest' => false, + ) + )); + $this->assertEquals($expectedDefinition, $definition); + } else { + $this->assertFalse($definition); + } } - public function testMatch() + public function testMatchClassAnnotation() { - $this->matcher = new TransactionalMatcher(array('/foo/bar'), array(), $this->reader); + $defaults = array( + 'isolation' => TransactionDefinition::ISOLATION_DEFAULT, + 'propagation' => TransactionDefinition::PROPAGATION_REQUIRED, + 'noRollbackFor' => array(), + ); + + $matcher = new TransactionalMatcher(array(), $defaults, $this->reader); + $request = Request::create('/foo', 'GET'); + $controller = new TestController(); + + $this->reader->expects($this->once()) + ->method('getClassAnnotation') + ->will($this->returnValue( + new Transactional(array( + 'methods' => array('GET'), + 'subrequest' => true, + 'conn' => array('orm.default'), + )) + )); + + $definition = $matcher->match($request, array($controller, 'fooAction')); + + $expectedDefinition = new TransactionDefinition(array( + 'orm.default' => array( + 'isolation' => TransactionDefinition::ISOLATION_DEFAULT, + 'propagation' => TransactionDefinition::PROPAGATION_REQUIRED, + 'noRollbackFor' => array(), + 'subrequest' => true, + ) + )); + $this->assertEquals($expectedDefinition, $definition); + } + + public function testMatchMethodAnnotation() + { + $defaults = array( + 'isolation' => TransactionDefinition::ISOLATION_DEFAULT, + 'propagation' => TransactionDefinition::PROPAGATION_REQUIRED, + 'noRollbackFor' => array(), + ); + + $matcher = new TransactionalMatcher(array(), $defaults, $this->reader); + $request = Request::create('/foo', 'GET'); + $controller = new TestController(); + + $this->reader->expects($this->once()) + ->method('getClassAnnotation'); + $this->reader->expects($this->once()) + ->method('getMethodAnnotation') + ->will($this->returnValue( + new Transactional(array( + 'methods' => array('GET'), + 'subrequest' => true, + 'conn' => array('orm.default'), + )) + )); + + $definition = $matcher->match($request, array($controller, 'fooAction')); + + $expectedDefinition = new TransactionDefinition(array( + 'orm.default' => array( + 'isolation' => TransactionDefinition::ISOLATION_DEFAULT, + 'propagation' => TransactionDefinition::PROPAGATION_REQUIRED, + 'noRollbackFor' => array(), + 'subrequest' => true, + ) + )); + $this->assertEquals($expectedDefinition, $definition); + } + + /** + * @expectedException \SimpleThings\TransactionalBundle\TransactionException + */ + public function testThrowExceptionWhenStoreDuplicateConnectionMatch() + { + $pattern = array( + 'pattern' => '.*', + 'methods' => array('GET'), + 'conn' => array('orm.default'), + 'isolation' => TransactionDefinition::ISOLATION_DEFAULT, + 'propagation' => TransactionDefinition::PROPAGATION_REQUIRED, + 'noRollbackFor' => array(), + 'subrequest' => false, + ); + $this->reader->expects($this->once()) + ->method('getClassAnnotation') + ->will($this->returnValue( + new Transactional(array( + 'methods' => array('GET'), + 'subrequest' => true, + 'conn' => array('orm.default'), + )) + )); + + $matcher = new TransactionalMatcher(array($pattern), array(), $this->reader); + $request = Request::create('/foo', 'GET'); + + $matcher->match($request, array(new TestController(), 'fooAction')); + } + + public function getPatterns() + { + return array( + array('.*', 'POST', true), + array('SimpleThings\\\\(.+)Controller::(.+)Action', 'POST', true), + array('SimpleThings\\\\TransactionalBundle\\\\Tests\\\\Transactions\\\\TestController::fooAction', 'POST', true), + array('.*', 'GET', false), + array('SimpleThings\\\\(.+)Controller::barAction', 'POST', false), + ); + } + + protected function setUp() + { + $this->reader = $this->getMock('Doctrine\Common\Annotations\Reader'); } } +class TestController +{ + public function fooAction() {} +} \ No newline at end of file diff --git a/TransactionalException.php b/TransactionException.php similarity index 100% rename from TransactionalException.php rename to TransactionException.php diff --git a/Transactions/TransactionalMatcher.php b/Transactions/TransactionalMatcher.php index e480764..f4e7e34 100644 --- a/Transactions/TransactionalMatcher.php +++ b/Transactions/TransactionalMatcher.php @@ -16,6 +16,7 @@ use Symfony\Component\HttpFoundation\Request; use Doctrine\Common\Annotations\Reader; +use SimpleThings\TransactionalBundle\TransactionException; class TransactionalMatcher { @@ -113,7 +114,7 @@ private function matchAnnotations($subject, $method, $controller, $action) $annotData = array_merge($this->defaults, array_filter((array)$txAnnot, function($v) { return $v !== null; })); if (in_array($method, $annotData['methods'])) { - $this->storeMatch($connections, $annotData); + $this->storeMatch($subject, $method, $annotData); } } From 84aa8391f0019ddb62bca3217f242836f356f114 Mon Sep 17 00:00:00 2001 From: Deni Date: Tue, 20 Dec 2011 20:12:46 +0400 Subject: [PATCH 02/24] Fixed a class name --- Transactions/Doctrine/DBALTransactionManager.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Transactions/Doctrine/DBALTransactionManager.php b/Transactions/Doctrine/DBALTransactionManager.php index 755dd15..3a7c31b 100644 --- a/Transactions/Doctrine/DBALTransactionManager.php +++ b/Transactions/Doctrine/DBALTransactionManager.php @@ -22,7 +22,7 @@ * * @author Benjamin Eberlei */ -class DoctrineDBALTransactionManager implements TransactionManagerInterface +class DBALTransactionManager implements TransactionManagerInterface { private $conn; From 284913bdbb6425061d88716bcd823dc4fd32f8b0 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Fri, 6 Jan 2012 23:55:35 +0100 Subject: [PATCH 03/24] Start refactoring away from wrapper --- Controller/ControllerListener.php | 91 ++++++++++++++++--- Controller/TransactionalControllerWrapper.php | 12 +-- 2 files changed, 85 insertions(+), 18 deletions(-) diff --git a/Controller/ControllerListener.php b/Controller/ControllerListener.php index f628d9f..d7ba86e 100644 --- a/Controller/ControllerListener.php +++ b/Controller/ControllerListener.php @@ -20,10 +20,21 @@ use SimpleThings\TransactionalBundle\Controller\TransactionalControllerWrapper; use SimpleThings\TransactionalBundle\Transactions\TransactionalMatcher; +/** + * Transactional controller listener. + * + * Checks if the request should run in transactional scope during the + * onCoreController event. Transactions are opened for every manager matches + * the criteria configured. + * + * Depending on the success or failure of the request the open transactions are + * either rolled back or committed. + */ class ControllerListener { private $container; private $matcher; + private $logger; public function __construct(ContainerInterface $container, TransactionalMatcher $matcher) { @@ -37,25 +48,81 @@ public function onCoreController(FilterControllerEvent $event) $def = $this->matcher->match($request, $event->getController()); if ($def) { - - list($controller, $action) = $event->getController(); - $txManagers = array(); foreach ($def->getConnections() AS $txConnName) { if (($def->isInvokedOnSubrequest($txConnName) === true || $event->getRequestType() == HttpKernelInterface::SUB_REQUEST)) { - $id = "simple_things_transactional.tx.".$txConnName; - if (!$this->container->has($id)) { - throw new \InvalidArgumentException( - "A transactional manager by name of '".$txConnName."' was requested, but does not exist." - ); - } - $txManagers[$txConnName] = $this->container->get($id); + $this->getTransaction($txConnName)->beginTransaction(); + $txManagers[] = $txConnName; } } - $controller = new TransactionalControllerWrapper($controller, $txManagers, $def, $this->container->get('logger')); - $event->setController(array($controller, $action)); + if ($txManagers && $this->logger) { + $this->logger->info("[TransactionBundle] Started transactions for " . implode(", ", $txManagers)); + } + + $request->attributes->set('_transactions', $txManagers); + } + } + + private function getTransaction($name) + { + $id = "simple_things_transactional.tx.".$name; + if (!$this->container->has($id)) { + throw new \InvalidArgumentException( + "A transactional manager by name of '".$name."' was requested, but does not exist." + ); + } + return $this->container->get($id); + } + + public function onKernelResponse(FilterResponseEvent $event) + { + $request = $event->getRequest(); + $response = $event->getResponse(); + + if (!$request->attributes->has('_transactions')) { + return; + } + + $txManagers = $request->attributes->get('_transactions'); + if ($response->getStatusCode() >= 500) { + $this->rollBack($txManagers); + } else { + $this->commit($txManagers); + } + } + + public function onKernelException(GetResponseForExceptionEvent $event) + { + $request = $event->getRequest(); + + if (!$request->attributes->has('_transactions')) { + return; } + $txManagers = $request->attributes->get('_transactions'); + $this->rollBack($txManagers); + } + + private function commit($txManagers) + { + foreach ($txManagers AS $txConnName) { + $this->getTransaction($txConnName)->commit(); + } + + if ($this->logger) { + $this->logger->info("[TransactionBundle] Committed transactions for " . implode(", ", array_keys($txManagers))); + } + } + + private function rollBack($txManagers) + { + foreach ($txManagers AS $txConnName) { + $this->getTransaction($txConnName)->rollback(); + } + + if ($this->logger) { + $this->logger->info("[TransactionBundle] Aborted transactions for " . implode(", ", array_keys($this->txManagers))); + } } } diff --git a/Controller/TransactionalControllerWrapper.php b/Controller/TransactionalControllerWrapper.php index e3e80b4..81ef5d6 100644 --- a/Controller/TransactionalControllerWrapper.php +++ b/Controller/TransactionalControllerWrapper.php @@ -23,7 +23,7 @@ class TransactionalControllerWrapper private $txManagers = array(); private $def; private $logger; - + /** * @param array $controller * @param array $txManagers @@ -35,12 +35,12 @@ public function __construct($controller, array $txManagers, TransactionDefinitio $this->def = $definition; $this->logger = $logger; } - + public function getController() { return $this->controller; } - + public function __call($method, $args) { foreach ($this->txManagers AS $txManager) { @@ -49,10 +49,10 @@ public function __call($method, $args) if ($this->logger) { $this->logger->info("[TransactionBundle] Started transactions for " . implode(", ", array_keys($this->txManagers))); } - + try { $response = call_user_func_array(array($this->controller, $method), $args); - + foreach ($this->txManagers AS $txName => $txManager) { $txManager->commit(); } @@ -74,4 +74,4 @@ public function __call($method, $args) throw $e; } } -} \ No newline at end of file +} From a2f255e694e480d75a58e82d7e637b4018253b50 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Sat, 7 Jan 2012 13:22:42 +0100 Subject: [PATCH 04/24] Checkpoint Refactoring. Removed all old Managers, reworked TransactionManagerInterface, added TransactionStatus and fixed ControllerListener to work with new API --- Controller/ControllerListener.php | 30 ++--- Controller/ControllerResolver.php | 29 ----- Controller/TraceableControllerResolver.php | 30 ----- Controller/TransactionalControllerWrapper.php | 77 ------------ .../Transactions/TransactionalMatcherTest.php | 67 +++++------ .../Doctrine/CouchDBTransactionManager.php | 44 ------- .../Doctrine/DBALTransactionManager.php | 48 -------- .../EntityManagerTransactionManager.php | 48 -------- .../Doctrine/MongoDBTransactionManager.php | 44 ------- Transactions/TransactionDefinition.php | 110 +++++++++++++++--- Transactions/TransactionManagerInterface.php | 38 +++++- Transactions/TransactionStatus.php | 67 +++++++++++ Transactions/TransactionalMatcher.php | 64 +++++----- 13 files changed, 274 insertions(+), 422 deletions(-) delete mode 100644 Controller/ControllerResolver.php delete mode 100644 Controller/TraceableControllerResolver.php delete mode 100644 Controller/TransactionalControllerWrapper.php delete mode 100644 Transactions/Doctrine/CouchDBTransactionManager.php delete mode 100644 Transactions/Doctrine/DBALTransactionManager.php delete mode 100644 Transactions/Doctrine/EntityManagerTransactionManager.php delete mode 100644 Transactions/Doctrine/MongoDBTransactionManager.php create mode 100644 Transactions/TransactionStatus.php diff --git a/Controller/ControllerListener.php b/Controller/ControllerListener.php index d7ba86e..7a25e63 100644 --- a/Controller/ControllerListener.php +++ b/Controller/ControllerListener.php @@ -19,6 +19,7 @@ use Symfony\Component\DependencyInjection\ContainerInterface; use SimpleThings\TransactionalBundle\Controller\TransactionalControllerWrapper; use SimpleThings\TransactionalBundle\Transactions\TransactionalMatcher; +use Symfony\Component\HttpKernel\Log\LoggerInterface; /** * Transactional controller listener. @@ -36,35 +37,34 @@ class ControllerListener private $matcher; private $logger; - public function __construct(ContainerInterface $container, TransactionalMatcher $matcher) + public function __construct(ContainerInterface $container, TransactionalMatcher $matcher, LoggerInterface $logger = null) { $this->container = $container; $this->matcher = $matcher; + $this->logger = $logger; } public function onCoreController(FilterControllerEvent $event) { $request = $event->getRequest(); - $def = $this->matcher->match($request, $event->getController()); + $definitions = $this->matcher->match($request, $event->getController()); - if ($def) { + if ($definitions) { $txManagers = array(); - foreach ($def->getConnections() AS $txConnName) { - if (($def->isInvokedOnSubrequest($txConnName) === true || $event->getRequestType() == HttpKernelInterface::SUB_REQUEST)) { - $this->getTransaction($txConnName)->beginTransaction(); - $txManagers[] = $txConnName; - } + foreach ($definitions as $def) { + $managerName = $def->getManagerName(); + $txManagers[$managerName] = $this->getTransactionManager($managerName)->getTransaction($def) } if ($txManagers && $this->logger) { - $this->logger->info("[TransactionBundle] Started transactions for " . implode(", ", $txManagers)); + $this->logger->info("[TransactionBundle] Started transactions for " . implode(", ", array_keys($txManagers))); } $request->attributes->set('_transactions', $txManagers); } } - private function getTransaction($name) + private function getTransactionManager($name) { $id = "simple_things_transactional.tx.".$name; if (!$this->container->has($id)) { @@ -106,8 +106,8 @@ public function onKernelException(GetResponseForExceptionEvent $event) private function commit($txManagers) { - foreach ($txManagers AS $txConnName) { - $this->getTransaction($txConnName)->commit(); + foreach ($txManagers AS $managerName = $txStatus) { + $this->getTransaction($managerName)->commit($txStatus); } if ($this->logger) { @@ -117,12 +117,12 @@ private function commit($txManagers) private function rollBack($txManagers) { - foreach ($txManagers AS $txConnName) { - $this->getTransaction($txConnName)->rollback(); + foreach ($txManagers AS $managerName = $txStatus) { + $this->getTransaction($managerName)->rollBack($txStatus); } if ($this->logger) { - $this->logger->info("[TransactionBundle] Aborted transactions for " . implode(", ", array_keys($this->txManagers))); + $this->logger->info("[TransactionBundle] Aborted transactions for " . implode(", ", array_keys($txManagers))); } } } diff --git a/Controller/ControllerResolver.php b/Controller/ControllerResolver.php deleted file mode 100644 index 8b53fb4..0000000 --- a/Controller/ControllerResolver.php +++ /dev/null @@ -1,29 +0,0 @@ -getController(); - } - return parent::getArguments($request, $controller); - } -} \ No newline at end of file diff --git a/Controller/TraceableControllerResolver.php b/Controller/TraceableControllerResolver.php deleted file mode 100644 index c010dfb..0000000 --- a/Controller/TraceableControllerResolver.php +++ /dev/null @@ -1,30 +0,0 @@ -getController(); - } - - return parent::getArguments($request, $controller); - } -} \ No newline at end of file diff --git a/Controller/TransactionalControllerWrapper.php b/Controller/TransactionalControllerWrapper.php deleted file mode 100644 index 81ef5d6..0000000 --- a/Controller/TransactionalControllerWrapper.php +++ /dev/null @@ -1,77 +0,0 @@ -controller = $controller; - $this->txManagers = $txManagers; - $this->def = $definition; - $this->logger = $logger; - } - - public function getController() - { - return $this->controller; - } - - public function __call($method, $args) - { - foreach ($this->txManagers AS $txManager) { - $txManager->beginTransaction(); - } - if ($this->logger) { - $this->logger->info("[TransactionBundle] Started transactions for " . implode(", ", array_keys($this->txManagers))); - } - - try { - $response = call_user_func_array(array($this->controller, $method), $args); - - foreach ($this->txManagers AS $txName => $txManager) { - $txManager->commit(); - } - - if ($this->logger) { - $this->logger->info("[TransactionBundle] Committed transactions for " . implode(", ", array_keys($this->txManagers))); - } - - return $response; - } catch(\Exception $e) { - foreach ($this->txManagers AS $txName => $txManager) { - if (!in_array(get_class($e), $this->def->getNoRollbackFor($txName))) { - $txManager->rollBack(); - } - } - if ($this->logger) { - $this->logger->info("[TransactionBundle] Aborted transactions for " . implode(", ", array_keys($this->txManagers))); - } - throw $e; - } - } -} diff --git a/Tests/Transactions/TransactionalMatcherTest.php b/Tests/Transactions/TransactionalMatcherTest.php index d123faa..d58666a 100644 --- a/Tests/Transactions/TransactionalMatcherTest.php +++ b/Tests/Transactions/TransactionalMatcherTest.php @@ -14,7 +14,7 @@ class TransactionalMatcherTest extends \PHPUnit_Framework_TestCase /** * @dataProvider getPatterns */ - public function testMatchPattern($pattern, $method, $matched) + public function testMatchPattern($pattern, $method, $matched, $readOnly) { $pattern = array( 'pattern' => $pattern, @@ -30,20 +30,15 @@ public function testMatchPattern($pattern, $method, $matched) $request = Request::create('/foo', $method); $controller = new TestController(); - $definition = $matcher->match($request, array($controller, 'fooAction')); + $definitions = $matcher->match($request, array($controller, 'fooAction')); if ($matched) { - $expectedDefinition = new TransactionDefinition(array( - 'orm.default' => array( - 'isolation' => TransactionDefinition::ISOLATION_DEFAULT, - 'propagation' => TransactionDefinition::PROPAGATION_REQUIRED, - 'noRollbackFor' => array(), - 'subrequest' => false, - ) - )); - $this->assertEquals($expectedDefinition, $definition); + $this->assertInternalType('array', $definitions); + $this->assertCount(1, $definitions); + $this->assertEquals('orm.default', $definitions[0]->getManagerName()); + $this->assertEquals($readOnly, $definitions[0]->getReadOnly()); } else { - $this->assertFalse($definition); + $this->assertCount(0, $definitions); } } @@ -64,22 +59,20 @@ public function testMatchClassAnnotation() ->will($this->returnValue( new Transactional(array( 'methods' => array('GET'), - 'subrequest' => true, 'conn' => array('orm.default'), )) )); $definition = $matcher->match($request, array($controller, 'fooAction')); - $expectedDefinition = new TransactionDefinition(array( - 'orm.default' => array( - 'isolation' => TransactionDefinition::ISOLATION_DEFAULT, - 'propagation' => TransactionDefinition::PROPAGATION_REQUIRED, - 'noRollbackFor' => array(), - 'subrequest' => true, - ) - )); - $this->assertEquals($expectedDefinition, $definition); + $expectedDefinition = new TransactionDefinition( + 'orm.default', + TransactionDefinition::PROPAGATION_REQUIRED, + TransactionDefinition::ISOLATION_DEFAULT, + false, + array() + ); + $this->assertEquals($expectedDefinition, $definition[0]); } public function testMatchMethodAnnotation() @@ -101,22 +94,20 @@ public function testMatchMethodAnnotation() ->will($this->returnValue( new Transactional(array( 'methods' => array('GET'), - 'subrequest' => true, 'conn' => array('orm.default'), )) )); $definition = $matcher->match($request, array($controller, 'fooAction')); - $expectedDefinition = new TransactionDefinition(array( - 'orm.default' => array( - 'isolation' => TransactionDefinition::ISOLATION_DEFAULT, - 'propagation' => TransactionDefinition::PROPAGATION_REQUIRED, - 'noRollbackFor' => array(), - 'subrequest' => true, - ) - )); - $this->assertEquals($expectedDefinition, $definition); + $expectedDefinition = new TransactionDefinition( + 'orm.default', + TransactionDefinition::PROPAGATION_REQUIRED, + TransactionDefinition::ISOLATION_DEFAULT, + false, + array() + ); + $this->assertEquals($expectedDefinition, $definition[0]); } /** @@ -152,11 +143,11 @@ public function testThrowExceptionWhenStoreDuplicateConnectionMatch() public function getPatterns() { return array( - array('.*', 'POST', true), - array('SimpleThings\\\\(.+)Controller::(.+)Action', 'POST', true), - array('SimpleThings\\\\TransactionalBundle\\\\Tests\\\\Transactions\\\\TestController::fooAction', 'POST', true), - array('.*', 'GET', false), - array('SimpleThings\\\\(.+)Controller::barAction', 'POST', false), + array('.*', 'POST', true, false), + array('SimpleThings\\\\(.+)Controller::(.+)Action', 'POST', true, false), + array('SimpleThings\\\\TransactionalBundle\\\\Tests\\\\Transactions\\\\TestController::fooAction', 'POST', true, false), + array('.*', 'GET', true, true), + array('SimpleThings\\\\(.+)Controller::barAction', 'POST', false, false), ); } @@ -169,4 +160,4 @@ protected function setUp() class TestController { public function fooAction() {} -} \ No newline at end of file +} diff --git a/Transactions/Doctrine/CouchDBTransactionManager.php b/Transactions/Doctrine/CouchDBTransactionManager.php deleted file mode 100644 index c9b8e73..0000000 --- a/Transactions/Doctrine/CouchDBTransactionManager.php +++ /dev/null @@ -1,44 +0,0 @@ -dm = $dm; - } - - public function beginTransaction() - { - - } - - public function commit() - { - $this->dm->flush(); - } - - public function rollBack() - { - - } - -} \ No newline at end of file diff --git a/Transactions/Doctrine/DBALTransactionManager.php b/Transactions/Doctrine/DBALTransactionManager.php deleted file mode 100644 index 3a7c31b..0000000 --- a/Transactions/Doctrine/DBALTransactionManager.php +++ /dev/null @@ -1,48 +0,0 @@ -conn = $conn; - } - - public function beginTransaction() - { - $this->conn->beginTransaction(); - } - - public function commit() - { - $this->conn->commit(); - } - - public function rollBack() - { - $this->conn->rollBack(); - } -} \ No newline at end of file diff --git a/Transactions/Doctrine/EntityManagerTransactionManager.php b/Transactions/Doctrine/EntityManagerTransactionManager.php deleted file mode 100644 index 29b200f..0000000 --- a/Transactions/Doctrine/EntityManagerTransactionManager.php +++ /dev/null @@ -1,48 +0,0 @@ -doctrineRegistry = $doctrineRegistry; - $this->name = $name; - } - - public function beginTransaction() - { - $this->doctrineRegistry->getManager($this->name)->beginTransaction(); - } - - public function commit() - { - $em = $this->doctrineRegistry->getManager($this->name); - $em->flush(); - $em->commit(); - } - - public function rollBack() - { - $this->doctrineRegistry->getManager($this->name)->rollback(); - $this->doctrineRegistry->resetEntityManager($this->name); - } -} \ No newline at end of file diff --git a/Transactions/Doctrine/MongoDBTransactionManager.php b/Transactions/Doctrine/MongoDBTransactionManager.php deleted file mode 100644 index 8b31330..0000000 --- a/Transactions/Doctrine/MongoDBTransactionManager.php +++ /dev/null @@ -1,44 +0,0 @@ -dm = $dm; - } - - public function beginTransaction() - { - - } - - public function commit() - { - $this->dm->flush(); - } - - public function rollBack() - { - - } - -} \ No newline at end of file diff --git a/Transactions/TransactionDefinition.php b/Transactions/TransactionDefinition.php index e150f4e..80c5048 100644 --- a/Transactions/TransactionDefinition.php +++ b/Transactions/TransactionDefinition.php @@ -16,9 +16,37 @@ class TransactionDefinition { + /** + * A transaction definition of this kind doesnt mind if its nested inside + * another transaction or not and does not start a transaction on its own. + * + * This is the default behavior. + * + * @var int + */ const PROPAGATION_SUPPORTS = 1; + + /** + * A transaction is required. If a transaction + * is already open for the transaction manager it will be re-used. + * + * @var int + */ const PROPAGATION_REQUIRED = 2; + + /** + * A NEW transaction is required. When the transaction is finished the old + * higher level transaction will be restored. + * + * @var int + */ const PROPAGATION_REQUIRES_NEW = 3; + + /** + * Throws an exception if a transaction is open. + * + * @var int + */ const PROPAGATION_NEVER = 4; const ISOLATION_DEFAULT = 0; @@ -27,35 +55,87 @@ class TransactionDefinition const ISOLATION_REPEATABLE_READ = 3; const ISOLATION_SERIALIZABLE = 4; - private $connections = array(); + /** + * @var string + */ + private $managerName; + + /** + * @var int + */ + private $isolationLevel; - public function __construct(array $connections) + /** + * @var bool + */ + private $readOnly; + + /** + * @var int + */ + private $propagation; + + /** + * @var array + */ + private $noRollbackFor = array(); + + public function __construct($managerName, $propagation, $isolationLevel, $readOnly = false, $noRollbackFor = array()) { - return $this->connections = $connections; + $this->managerName = $managerName; + $this->propagation = $propagation; + $this->isolationLevel = $isolationLevel; + $this->readOnly = $readOnly; + $this->noRollbackFor = $noRollbackFor; } - public function getConnections() + /** + * Get propagation. + * + * @return propagation. + */ + public function getPropagation() { - return array_keys($this->connections); + return $this->propagation; } - public function getIsolationLevel($connection) + /** + * Get readOnly. + * + * @return readOnly. + */ + public function getReadOnly() { - return $this->connections[$connection]['isolation']; + return $this->readOnly; } - public function getPropagation($connection) + /** + * Get isolationLevel. + * + * @return isolationLevel. + */ + public function getIsolationLevel() { - return $this->connections[$connection]['propagation']; + return $this->isolationLevel; } - public function getNoRollbackFor($connection) + /** + * Get managerName. + * + * @return managerName. + */ + public function getManagerName() { - return $this->connections[$connection]['noRollbackFor']; + return $this->managerName; } - - public function isInvokedOnSubrequest($connection) + + /** + * Get noRollbackFor. + * + * @return noRollbackFor. + */ + public function getNoRollbackFor() { - return $this->connections[$connection]['subrequest']; + return $this->noRollbackFor; } -} \ No newline at end of file +} diff --git a/Transactions/TransactionManagerInterface.php b/Transactions/TransactionManagerInterface.php index 0d4b032..7e6b7c7 100644 --- a/Transactions/TransactionManagerInterface.php +++ b/Transactions/TransactionManagerInterface.php @@ -15,15 +15,41 @@ /** * Wraps a transactional service into a common interface - * + * * @author Benjamin Eberlei */ interface TransactionManagerInterface { + /** + * Get a transaction status object. + * + * 1. Returns a new transaction if none was opened with this manager yet. + * 2. Returns a previous transaction if the propagation is REQUIRED. + * 3. Returns a new transaction if the propagation is REQUIRES_NEW. + * + * @return TransactionDefinition + */ + function getTransaction(TransactionDefintion $def); - function beginTransaction(); - - function commit(); + /** + * Commit the transaction inside the status object. + * + * Depending on the Transaction#isRollBackOnly status this method commits + * or rollbacks the transaction wrapped inside the status. If an error + * happens during commit the original exception of the underlying + * connection is thrown from this method. + * + * @throws Exception + * @param TransactionStatus $status + * @return void + */ + function commit(TransactionStatus $status); - function rollBack(); -} \ No newline at end of file + /** + * Rollback the transaction inside the status object. + * + * @param TransactionStatus $status + * @return void + */ + function rollBack(TransactionStatus $status); +} diff --git a/Transactions/TransactionStatus.php b/Transactions/TransactionStatus.php new file mode 100644 index 0000000..8494a05 --- /dev/null +++ b/Transactions/TransactionStatus.php @@ -0,0 +1,67 @@ +cache[$subject][$method])) { - $this->matchPatterns($subject, $method); - $this->matchAnnotations($subject, $method, $controller, $action); + $this->cache[$subject] = array(); + $this->matchPatterns($subject); + $this->matchAnnotations($subject, $controller, $action); + } - if (!isset($this->cache[$subject][$method])) { - $this->cache[$subject][$method] = false; - } else { - $this->cache[$subject][$method] = new TransactionDefinition($this->cache[$subject][$method]); - } + $definitions = array(); + foreach ($this->cache[$subject] as $connectionName => $definition) { + $definitions[] = new TransactionDefinition( + $definition['managerName'], + $definition['propagation'], + $definition['isolation'], + ! in_array($method, $definition['methods']) + ); } - return $this->cache[$subject][$method]; + + return $definitions; } /** * Match transactional patterns. * * @param string $subject - * @param string $method */ - private function matchPatterns($subject, $method) + private function matchPatterns($subject) { foreach ($this->patterns AS $pattern) { - if (in_array($method, $pattern['methods']) && preg_match('(' . $pattern['pattern'] . ')', $subject)) { - $this->storeMatch($subject, $method, $pattern); + if (preg_match('(' . $pattern['pattern'] . ')', $subject)) { + $this->storeMatch($subject, $pattern); } } } @@ -98,12 +112,11 @@ private function matchPatterns($subject, $method) * Match annotations on controllers for transactional behavior. * * @param string $subject - * @param string $method * @param object $controller * @param string $action * @return void */ - private function matchAnnotations($subject, $method, $controller, $action) + private function matchAnnotations($subject, $controller, $action) { if ($this->reader === null) { return; @@ -112,33 +125,28 @@ private function matchAnnotations($subject, $method, $controller, $action) $reflClass = new \ReflectionObject($controller); if ($txAnnot = $this->reader->getClassAnnotation($reflClass, 'SimpleThings\TransactionalBundle\Annotations\Transactional')) { $annotData = array_merge($this->defaults, array_filter((array)$txAnnot, function($v) { return $v !== null; })); - - if (in_array($method, $annotData['methods'])) { - $this->storeMatch($subject, $method, $annotData); - } + $this->storeMatch($subject, $annotData); } if ($txAnnot = $this->reader->getMethodAnnotation($reflClass->getMethod($action), 'SimpleThings\TransactionalBundle\Annotations\Transactional')) { $annotData = array_merge($this->defaults, array_filter((array)$txAnnot, function($v) { return $v !== null; })); - - if (in_array($method, $annotData['methods'])) { - $this->storeMatch($subject, $method, $annotData); - } + $this->storeMatch($subject, $annotData); } } - private function storeMatch($subject, $method, $pattern) + private function storeMatch($subject, $pattern) { - foreach ($pattern['conn'] AS $connectionName) { - if (isset($this->cache[$subject][$method][$connectionName])) { - throw TransactionException::duplicateConnectionMatch($connectionName, $pattern); + foreach ($pattern['conn'] AS $managerName) { + if (isset($this->cache[$subject][$managerName])) { + throw TransactionException::duplicateConnectionMatch($managerName, $pattern); } - $this->cache[$subject][$method][$connectionName] = array( + $this->cache[$subject][$managerName] = array( + 'managerName' => $managerName, 'isolation' => $pattern['isolation'], 'propagation' => $pattern['propagation'], 'noRollbackFor' => $pattern['noRollbackFor'], - 'subrequest' => $pattern['subrequest'], + 'methods' => $pattern['methods'], ); } } From 0a52ba753e8994f3a30ef0d900c3bfbb51ce437e Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Sat, 7 Jan 2012 13:32:37 +0100 Subject: [PATCH 05/24] Remove Request dependency from TransactionalMatcher --- Tests/Transactions/TransactionalMatcherTest.php | 9 +++------ Transactions/TransactionalMatcher.php | 5 ++--- 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/Tests/Transactions/TransactionalMatcherTest.php b/Tests/Transactions/TransactionalMatcherTest.php index d58666a..67eae74 100644 --- a/Tests/Transactions/TransactionalMatcherTest.php +++ b/Tests/Transactions/TransactionalMatcherTest.php @@ -27,10 +27,9 @@ public function testMatchPattern($pattern, $method, $matched, $readOnly) ); $matcher = new TransactionalMatcher(array($pattern), array(), $this->reader); - $request = Request::create('/foo', $method); $controller = new TestController(); - $definitions = $matcher->match($request, array($controller, 'fooAction')); + $definitions = $matcher->match($method, array($controller, 'fooAction')); if ($matched) { $this->assertInternalType('array', $definitions); @@ -51,7 +50,6 @@ public function testMatchClassAnnotation() ); $matcher = new TransactionalMatcher(array(), $defaults, $this->reader); - $request = Request::create('/foo', 'GET'); $controller = new TestController(); $this->reader->expects($this->once()) @@ -63,7 +61,7 @@ public function testMatchClassAnnotation() )) )); - $definition = $matcher->match($request, array($controller, 'fooAction')); + $definition = $matcher->match('GET', array($controller, 'fooAction')); $expectedDefinition = new TransactionDefinition( 'orm.default', @@ -84,7 +82,6 @@ public function testMatchMethodAnnotation() ); $matcher = new TransactionalMatcher(array(), $defaults, $this->reader); - $request = Request::create('/foo', 'GET'); $controller = new TestController(); $this->reader->expects($this->once()) @@ -98,7 +95,7 @@ public function testMatchMethodAnnotation() )) )); - $definition = $matcher->match($request, array($controller, 'fooAction')); + $definition = $matcher->match('GET', array($controller, 'fooAction')); $expectedDefinition = new TransactionDefinition( 'orm.default', diff --git a/Transactions/TransactionalMatcher.php b/Transactions/TransactionalMatcher.php index e74f82b..fb1f964 100644 --- a/Transactions/TransactionalMatcher.php +++ b/Transactions/TransactionalMatcher.php @@ -59,16 +59,15 @@ public function __construct(array $patterns, array $defaults = array(), Reader $ * Important: Only Controller as services or Class#Action method * controllers can be transactional. * - * @param Request $request + * @param string $method HTTP Method * @param mixed $controllerCallback * @return TransactionDefinition|false */ - public function match(Request $request, $controllerCallback) + public function match($method, $controllerCallback) { if (!is_array($controllerCallback)) { return false; } - $method = $request->getMethod(); list($controller, $action) = $controllerCallback; $class = get_class($controller); From f6b794feb282c02e166c8cce5922da51de0396d3 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Sat, 7 Jan 2012 23:35:21 +0100 Subject: [PATCH 06/24] Implement and test AbstractTransactionManager, Rename ControllerListener to HttpTransactionsListener --- ...tener.php => HttpTransactionsListener.php} | 12 +- .../SimpleThingsTransactionalExtension.php | 9 +- Resources/config/services.xml | 15 +- .../AbstractTransactionManagerTest.php | 152 ++++++++++++++++++ Transactions/AbstractTransactionManager.php | 128 +++++++++++++++ Transactions/TransactionManagerInterface.php | 2 +- Transactions/TransactionStatus.php | 4 +- phpunit.xml.dist | 1 + 8 files changed, 302 insertions(+), 21 deletions(-) rename Controller/{ControllerListener.php => HttpTransactionsListener.php} (90%) create mode 100644 Tests/Transactions/AbstractTransactionManagerTest.php create mode 100644 Transactions/AbstractTransactionManager.php diff --git a/Controller/ControllerListener.php b/Controller/HttpTransactionsListener.php similarity index 90% rename from Controller/ControllerListener.php rename to Controller/HttpTransactionsListener.php index 7a25e63..c86f830 100644 --- a/Controller/ControllerListener.php +++ b/Controller/HttpTransactionsListener.php @@ -31,7 +31,7 @@ * Depending on the success or failure of the request the open transactions are * either rolled back or committed. */ -class ControllerListener +class HttpTransactionsListener { private $container; private $matcher; @@ -47,13 +47,15 @@ public function __construct(ContainerInterface $container, TransactionalMatcher public function onCoreController(FilterControllerEvent $event) { $request = $event->getRequest(); - $definitions = $this->matcher->match($request, $event->getController()); + $definitions = $this->matcher->match($request->getMethod(), $event->getController()); if ($definitions) { $txManagers = array(); foreach ($definitions as $def) { $managerName = $def->getManagerName(); - $txManagers[$managerName] = $this->getTransactionManager($managerName)->getTransaction($def) + if ($txStatus = $this->getTransactionManager($managerName)->getTransaction($def)) { + $txManagers[$managerName] = $txStatus; + } } if ($txManagers && $this->logger) { @@ -106,7 +108,7 @@ public function onKernelException(GetResponseForExceptionEvent $event) private function commit($txManagers) { - foreach ($txManagers AS $managerName = $txStatus) { + foreach ($txManagers AS $managerName => $txStatus) { $this->getTransaction($managerName)->commit($txStatus); } @@ -117,7 +119,7 @@ private function commit($txManagers) private function rollBack($txManagers) { - foreach ($txManagers AS $managerName = $txStatus) { + foreach ($txManagers AS $managerName => $txStatus) { $this->getTransaction($managerName)->rollBack($txStatus); } diff --git a/DependencyInjection/SimpleThingsTransactionalExtension.php b/DependencyInjection/SimpleThingsTransactionalExtension.php index 5e7946d..0838970 100644 --- a/DependencyInjection/SimpleThingsTransactionalExtension.php +++ b/DependencyInjection/SimpleThingsTransactionalExtension.php @@ -52,7 +52,6 @@ public function load(array $configs, ContainerBuilder $builder) 'isolation' => TransactionDefinition::ISOLATION_DEFAULT, 'noRollbackFor' => array(), 'methods' => array('POST', 'PUT', 'DELETE', 'PATCH'), - 'subrequest' => false, ), $config['defaults']); if (isset($config['auto_transactional']) && $config['auto_transactional']) { @@ -83,7 +82,7 @@ public function load(array $configs, ContainerBuilder $builder) )->setArguments(array(new Reference($service))); } } - + if ($builder->hasParameter('doctrine.entity_managers')) { foreach ($builder->getParameter('doctrine.entity_managers') AS $alias => $service) { $builder->setDefinition( @@ -92,7 +91,7 @@ public function load(array $configs, ContainerBuilder $builder) )->setArguments(array(new Reference('doctrine'), $alias)); } } - + if ($builder->hasParameter('doctrine_couchdb.document_managers')) { foreach ($builder->getParameter('doctrine_couchdb.document_managers') AS $alias => $service) { $builder->setDefinition( @@ -101,7 +100,7 @@ public function load(array $configs, ContainerBuilder $builder) )->setArguments(array(new Reference($service))); } } - + if ($builder->hasParameter('doctrine_mongodb.document_managers')) { foreach ($builder->getParameter('doctrine_mongodb.document_managers') AS $alias => $service) { $builder->setDefinition( @@ -111,4 +110,4 @@ public function load(array $configs, ContainerBuilder $builder) } } } -} \ No newline at end of file +} diff --git a/Resources/config/services.xml b/Resources/config/services.xml index eb3e7ce..9e67144 100644 --- a/Resources/config/services.xml +++ b/Resources/config/services.xml @@ -9,28 +9,27 @@ SimpleThings\TransactionalBundle\Transactions\Doctrine\EntityManagerTransactionManager SimpleThings\TransactionalBundle\Transactions\Doctrine\MongoDBTransactionManager SimpleThings\TransactionalBundle\Transactions\Doctrine\CouchDBTransactionManager - SimpleThings\TransactionalBundle\Controller\ControllerListener + SimpleThings\TransactionalBundle\Controller\HttpTransactionsListener SimpleThings\TransactionalBundle\Transactions\TransactionalMatcher SimpleThings\TransactionalBundle\Controller\ControllerResolver SimpleThings\TransactionalBundle\Controller\TraceableControllerResolver - + - - + + - - + + - - \ No newline at end of file + diff --git a/Tests/Transactions/AbstractTransactionManagerTest.php b/Tests/Transactions/AbstractTransactionManagerTest.php new file mode 100644 index 0000000..fabc91a --- /dev/null +++ b/Tests/Transactions/AbstractTransactionManagerTest.php @@ -0,0 +1,152 @@ +manager = $this->getMock( + 'SimpleThings\TransactionalBundle\Transactions\AbstractTransactionManager', + array('doBeginTransaction', 'doCommit', 'doRollBack') + ); + } + + private function getTxStatusMock() + { + return $this->getMock('SimpleThings\TransactionalBundle\Transactions\TransactionStatus'); + } + + public function testGetTransaction() + { + $txStatus = $this->getTxStatusMock(); + $this->manager->expects($this->once())->method('doBeginTransaction')->will($this->returnValue($txStatus)); + $def = new TransactionDefinition("test", TransactionDefinition::PROPAGATION_REQUIRED, TransactionDefinition::ISOLATION_DEFAULT, true, array()); + $actualStatus = $this->manager->getTransaction($def); + + $this->assertSame($txStatus, $actualStatus); + } + + public function testGetNeverTransaction() + { + $def = new TransactionDefinition("test", TransactionDefinition::PROPAGATION_NEVER, TransactionDefinition::ISOLATION_DEFAULT, true, array()); + $actualStatus = $this->manager->getTransaction($def); + + $this->assertNull($actualStatus); + } + + public function testGetTransactionNeverButOpen() + { + $txStatus = $this->getTxStatusMock(); + $this->manager->expects($this->once())->method('doBeginTransaction')->will($this->returnValue($txStatus)); + + $def1 = new TransactionDefinition("test", TransactionDefinition::PROPAGATION_REQUIRED, TransactionDefinition::ISOLATION_DEFAULT, true, array()); + $def2 = new TransactionDefinition("test", TransactionDefinition::PROPAGATION_NEVER, TransactionDefinition::ISOLATION_DEFAULT, true, array()); + + $actualStatus1 = $this->manager->getTransaction($def1); + $this->setExpectedException("SimpleThings\TransactionalBundle\TransactionException"); + $actualStatus2 = $this->manager->getTransaction($def2); + } + + public function testGetRequireTransactionTwice() + { + $txStatus = $this->getTxStatusMock(); + $this->manager->expects($this->once())->method('doBeginTransaction')->will($this->returnValue($txStatus)); + + $def = new TransactionDefinition("test", TransactionDefinition::PROPAGATION_REQUIRED, TransactionDefinition::ISOLATION_DEFAULT, true, array()); + $actualStatus1 = $this->manager->getTransaction($def); + $actualStatus2 = $this->manager->getTransaction($def); + + $this->assertSame($actualStatus1, $actualStatus2); + } + + public function testGetRequiredIsolationLevelMissmatch() + { + $txStatus = $this->getTxStatusMock(); + $this->manager->expects($this->once())->method('doBeginTransaction')->will($this->returnValue($txStatus)); + + $def1 = new TransactionDefinition("test", TransactionDefinition::PROPAGATION_REQUIRED, TransactionDefinition::ISOLATION_DEFAULT, true, array()); + $def2 = new TransactionDefinition("test", TransactionDefinition::PROPAGATION_REQUIRED, TransactionDefinition::ISOLATION_REPEATABLE_READ, true, array()); + + $actualStatus1 = $this->manager->getTransaction($def1); + + $this->setExpectedException("SimpleThings\TransactionalBundle\TransactionException"); + $actualStatus2 = $this->manager->getTransaction($def2); + } + + public function testGetReadOnlyMissMatch() + { + $txStatus = $this->getTxStatusMock(); + $txStatus->expects($this->once())->method('isReadOnly')->will($this->returnValue(true)); + $this->manager->expects($this->once())->method('doBeginTransaction')->will($this->returnValue($txStatus)); + + $def1 = new TransactionDefinition("test", TransactionDefinition::PROPAGATION_REQUIRED, TransactionDefinition::ISOLATION_DEFAULT, true, array()); + $def2 = new TransactionDefinition("test", TransactionDefinition::PROPAGATION_REQUIRED, TransactionDefinition::ISOLATION_DEFAULT, false, array()); + + $actualStatus1 = $this->manager->getTransaction($def1); + + $this->setExpectedException("SimpleThings\TransactionalBundle\TransactionException"); + $actualStatus2 = $this->manager->getTransaction($def2); + } + + public function testGetTransactionPropagationSupports() + { + $this->manager->expects($this->never())->method('doBeginTransaction'); + + $def = new TransactionDefinition("test", TransactionDefinition::PROPAGATION_SUPPORTS, TransactionDefinition::ISOLATION_DEFAULT, true, array()); + + $status = $this->manager->getTransaction($def); + $this->assertNull($status); + } + + public function testGetTransactionPropagationSupportsNestedInRequired() + { + $txStatus = $this->getTxStatusMock(); + $this->manager->expects($this->once())->method('doBeginTransaction')->will($this->returnValue($txStatus)); + + $def1 = new TransactionDefinition("test", TransactionDefinition::PROPAGATION_REQUIRED, TransactionDefinition::ISOLATION_DEFAULT, true, array()); + $def2 = new TransactionDefinition("test", TransactionDefinition::PROPAGATION_SUPPORTS, TransactionDefinition::ISOLATION_DEFAULT, false, array()); + + $actualStatus1 = $this->manager->getTransaction($def1); + $actualStatus2 = $this->manager->getTransaction($def2); + + $this->assertSame($actualStatus1, $actualStatus2); + } + + public function testGetTransactionRequiresNew() + { + $txStatus1 = $this->getTxStatusMock(); + $txStatus2 = $this->getTxStatusMock(); + $this->manager->expects($this->at(0))->method('doBeginTransaction')->will($this->returnValue($txStatus1)); + $this->manager->expects($this->at(1))->method('doBeginTransaction')->will($this->returnValue($txStatus2)); + + $def1 = new TransactionDefinition("test", TransactionDefinition::PROPAGATION_REQUIRED, TransactionDefinition::ISOLATION_DEFAULT, true, array()); + $def2 = new TransactionDefinition("test", TransactionDefinition::PROPAGATION_REQUIRES_NEW, TransactionDefinition::ISOLATION_DEFAULT, false, array()); + + $actualStatus1 = $this->manager->getTransaction($def1); + $actualStatus2 = $this->manager->getTransaction($def2); + + $this->assertNotSame($actualStatus1, $actualStatus2); + } + + private function createDefinition($propagation, $isolation, $readOnly = false) + { + return new TransactionDefinition("test", $propagation, $isolation, $readOnly, array()); + } +} + diff --git a/Transactions/AbstractTransactionManager.php b/Transactions/AbstractTransactionManager.php new file mode 100644 index 0000000..a034bed --- /dev/null +++ b/Transactions/AbstractTransactionManager.php @@ -0,0 +1,128 @@ +transactions = new \SplObjectStorage(); + } + + abstract protected function doBeginTransaction(TransactionDefinition $def); + + abstract protected function doCommit(TransactionStatus $def); + + abstract protected function doRollBack(TransactionStatus $def); + + protected function beginTransaction(TransactionDefinition $def) + { + $status = $this->doBeginTransaction($def); + $this->transactions->attach($status); + $this->transactions[$status] = $def; + $this->currentTxStatus = $status; + return $status; + } + + public function getTransaction(TransactionDefinition $def) + { + switch ($def->getPropagation()) { + case TransactionDefinition::PROPAGATION_REQUIRES_NEW: + $status = $this->beginTransaction($def); + break; + case TransactionDefinition::PROPAGATION_NEVER: + if (count($this->transactions)) { + throw new TransactionException("Controller does not want to run in transaction, but one is open."); + } + return null; + case TransactionDefinition::PROPAGATION_REQUIRED: + $openTransactionDef = $this->getCurrentTransactionDef(); + $status = $this->getCurrentTransaction(); + if ($openTransactionDef) { + if ($def->getIsolationLevel() != $openTransactionDef->getIsolationLevel()) { + throw new TransactionException("Trying to re-use transaction that has different isolation level than the already active one."); + } + + if ($status->isReadOnly() && ! $def->getReadOnly()) { + throw new TransactionException("Cannot reuse readonly transaction when requesting a read/write transaction."); + } + } + + if (!$status) { + $status = $this->beginTransaction($def); + } + break; + case TransactionDefinition::PROPAGATION_SUPPORTS: + default: + $status = $this->getCurrentTransaction(); + break; + } + return $status; + } + + public function commit(TransactionStatus $status) + { + if ($status->isCompleted()) { + throw new TransactionException("Cannot commit an already completed transaction."); + } else if (!$this->transactions->contains($status)) { + throw new TransactionException("Cannot commit a detached transaction. It may have been committed before or belongs to another transaction manager"); + } + + if ($status->isRollBackOnly()) { + return $this->rollBack($status); + } + + $this->doCommit($status); + $this->transactions->detach($status); + } + + public function rollBack(TransactionStatus $status) + { + if ($status->isCompleted()) { + throw new TransactionException("Cannot rollback an already completed transaction."); + } else if (!$this->transactions->contains($status)) { + throw new TransactionException("Cannot rollback a detached transaction. It may have been committed/rollbacked before or belongs to another transaction manager"); + } + + $this->doRollBack($status); + $this->transactions->detach($status); + } + + private function getCurrentTransaction() + { + return $this->currentTxStatus; + } + + private function getCurrentTransactionDef() + { + if ($this->currentTxStatus) { + return $this->transactions[$this->currentTxStatus]; + } + return null; + } +} + + diff --git a/Transactions/TransactionManagerInterface.php b/Transactions/TransactionManagerInterface.php index 7e6b7c7..e7cd138 100644 --- a/Transactions/TransactionManagerInterface.php +++ b/Transactions/TransactionManagerInterface.php @@ -29,7 +29,7 @@ interface TransactionManagerInterface * * @return TransactionDefinition */ - function getTransaction(TransactionDefintion $def); + function getTransaction(TransactionDefinition $def); /** * Commit the transaction inside the status object. diff --git a/Transactions/TransactionStatus.php b/Transactions/TransactionStatus.php index 8494a05..9cc542f 100644 --- a/Transactions/TransactionStatus.php +++ b/Transactions/TransactionStatus.php @@ -11,7 +11,7 @@ * to kontakt@beberlei.de so I can send you a copy immediately. */ -namespace SimpleThingsTransactionalBundle\Transactions; +namespace SimpleThings\TransactionalBundle\Transactions; /** * Contains information about the current state of a transaction. @@ -55,7 +55,7 @@ function isCompleted(); * * @return bool */ - function hasSavepoint() + function hasSavepoint(); /** * Commit the transaction at this point. diff --git a/phpunit.xml.dist b/phpunit.xml.dist index a2a0bc8..5ba6b7d 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -13,6 +13,7 @@ ./Resources ./Tests + ./vendor From 24f3b8276e9fb8c250abe02004f1ea50994de248 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Sun, 8 Jan 2012 11:14:35 +0100 Subject: [PATCH 07/24] Move all non-configuration Files under Transactions namespace. Testing of AbstractTransactionManager, Introduced TransactionsRegistry to simplify HttpTransactionsListener --- Controller/HttpTransactionsListener.php | 65 ++++------ .../HttpTransactionsListenerTest.php | 14 +++ .../AbstractTransactionManagerTest.php | 89 +++++++++++--- .../Http/HttpTransactionsListenerTest.php | 23 ++++ .../{ => Http}/TransactionalMatcherTest.php | 11 +- Transactions/AbstractTransactionManager.php | 45 +++---- .../Annotations}/Transactional.php | 9 +- .../Http/HttpTransactionsListener.php | 115 ++++++++++++++++++ .../{ => Http}/TransactionalMatcher.php | 3 +- Transactions/TransactionsRegistry.php | 71 +++++++++++ 10 files changed, 349 insertions(+), 96 deletions(-) create mode 100644 Tests/Controller/HttpTransactionsListenerTest.php create mode 100644 Tests/Transactions/Http/HttpTransactionsListenerTest.php rename Tests/Transactions/{ => Http}/TransactionalMatcherTest.php (93%) rename {Annotations => Transactions/Annotations}/Transactional.php (80%) create mode 100644 Transactions/Http/HttpTransactionsListener.php rename Transactions/{ => Http}/TransactionalMatcher.php (97%) create mode 100644 Transactions/TransactionsRegistry.php diff --git a/Controller/HttpTransactionsListener.php b/Controller/HttpTransactionsListener.php index c86f830..d51496d 100644 --- a/Controller/HttpTransactionsListener.php +++ b/Controller/HttpTransactionsListener.php @@ -15,11 +15,13 @@ namespace SimpleThings\TransactionalBundle\Controller; use Symfony\Component\HttpKernel\Event\FilterControllerEvent; +use Symfony\Component\HttpKernel\Event\FilterResponseEvent; +use Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent; use Symfony\Component\HttpKernel\HttpKernelInterface; -use Symfony\Component\DependencyInjection\ContainerInterface; -use SimpleThings\TransactionalBundle\Controller\TransactionalControllerWrapper; -use SimpleThings\TransactionalBundle\Transactions\TransactionalMatcher; use Symfony\Component\HttpKernel\Log\LoggerInterface; +use Symfony\Component\HttpFoundation\Exceptions\NotFoundHttpException; +use SimpleThings\TransactionalBundle\Transactions\TransactionalMatcher; +use SimpleThings\TransactionalBundle\Transactions\TransactionsRegistry; /** * Transactional controller listener. @@ -33,13 +35,13 @@ */ class HttpTransactionsListener { - private $container; + private $registry; private $matcher; private $logger; - public function __construct(ContainerInterface $container, TransactionalMatcher $matcher, LoggerInterface $logger = null) + public function __construct(TransactionsRegistry $registry, TransactionalMatcher $matcher, LoggerInterface $logger = null) { - $this->container = $container; + $this->registry = $registry; $this->matcher = $matcher; $this->logger = $logger; } @@ -48,33 +50,13 @@ public function onCoreController(FilterControllerEvent $event) { $request = $event->getRequest(); $definitions = $this->matcher->match($request->getMethod(), $event->getController()); + $txManagers = $this->registry->getTransactions($definitions); - if ($definitions) { - $txManagers = array(); - foreach ($definitions as $def) { - $managerName = $def->getManagerName(); - if ($txStatus = $this->getTransactionManager($managerName)->getTransaction($def)) { - $txManagers[$managerName] = $txStatus; - } - } - - if ($txManagers && $this->logger) { - $this->logger->info("[TransactionBundle] Started transactions for " . implode(", ", array_keys($txManagers))); - } - - $request->attributes->set('_transactions', $txManagers); + if ($txManagers && $this->logger) { + $this->logger->info("[TransactionBundle] Started transactions for " . implode(", ", array_keys($txManagers))); } - } - private function getTransactionManager($name) - { - $id = "simple_things_transactional.tx.".$name; - if (!$this->container->has($id)) { - throw new \InvalidArgumentException( - "A transactional manager by name of '".$name."' was requested, but does not exist." - ); - } - return $this->container->get($id); + $request->attributes->set('_transactions', $txManagers); } public function onKernelResponse(FilterResponseEvent $event) @@ -87,7 +69,7 @@ public function onKernelResponse(FilterResponseEvent $event) } $txManagers = $request->attributes->get('_transactions'); - if ($response->getStatusCode() >= 500) { + if ($response->getStatusCode() >= 400 && $response->getStatusCode() != 404) { $this->rollBack($txManagers); } else { $this->commit($txManagers); @@ -97,34 +79,37 @@ public function onKernelResponse(FilterResponseEvent $event) public function onKernelException(GetResponseForExceptionEvent $event) { $request = $event->getRequest(); + $ex = $event->getException(); if (!$request->attributes->has('_transactions')) { return; } $txManagers = $request->attributes->get('_transactions'); - $this->rollBack($txManagers); + + if ($ex instanceof NotFoundHttpException) { + $this->registry->commit($txManagers); + } else { + $this->registry->rollBack($txManagers); + } } private function commit($txManagers) { - foreach ($txManagers AS $managerName => $txStatus) { - $this->getTransaction($managerName)->commit($txStatus); - } + $this->registry->commit($txManagers); - if ($this->logger) { + if ($txManagers && $this->logger) { $this->logger->info("[TransactionBundle] Committed transactions for " . implode(", ", array_keys($txManagers))); } } private function rollBack($txManagers) { - foreach ($txManagers AS $managerName => $txStatus) { - $this->getTransaction($managerName)->rollBack($txStatus); - } + $this->registry->rollBack($txManagers); - if ($this->logger) { + if ($txManagers && $this->logger) { $this->logger->info("[TransactionBundle] Aborted transactions for " . implode(", ", array_keys($txManagers))); } } } + diff --git a/Tests/Controller/HttpTransactionsListenerTest.php b/Tests/Controller/HttpTransactionsListenerTest.php new file mode 100644 index 0000000..aa47da3 --- /dev/null +++ b/Tests/Controller/HttpTransactionsListenerTest.php @@ -0,0 +1,14 @@ +getTxStatusMock(); $this->manager->expects($this->once())->method('doBeginTransaction')->will($this->returnValue($txStatus)); - $def = new TransactionDefinition("test", TransactionDefinition::PROPAGATION_REQUIRED, TransactionDefinition::ISOLATION_DEFAULT, true, array()); + $def = $this->createDefinition(TransactionDefinition::PROPAGATION_REQUIRED, TransactionDefinition::ISOLATION_DEFAULT); $actualStatus = $this->manager->getTransaction($def); $this->assertSame($txStatus, $actualStatus); @@ -44,7 +44,7 @@ public function testGetTransaction() public function testGetNeverTransaction() { - $def = new TransactionDefinition("test", TransactionDefinition::PROPAGATION_NEVER, TransactionDefinition::ISOLATION_DEFAULT, true, array()); + $def = $this->createDefinition(TransactionDefinition::PROPAGATION_NEVER, TransactionDefinition::ISOLATION_DEFAULT); $actualStatus = $this->manager->getTransaction($def); $this->assertNull($actualStatus); @@ -55,8 +55,8 @@ public function testGetTransactionNeverButOpen() $txStatus = $this->getTxStatusMock(); $this->manager->expects($this->once())->method('doBeginTransaction')->will($this->returnValue($txStatus)); - $def1 = new TransactionDefinition("test", TransactionDefinition::PROPAGATION_REQUIRED, TransactionDefinition::ISOLATION_DEFAULT, true, array()); - $def2 = new TransactionDefinition("test", TransactionDefinition::PROPAGATION_NEVER, TransactionDefinition::ISOLATION_DEFAULT, true, array()); + $def1 = $this->createDefinition(TransactionDefinition::PROPAGATION_REQUIRED, TransactionDefinition::ISOLATION_DEFAULT); + $def2 = $this->createDefinition(TransactionDefinition::PROPAGATION_NEVER, TransactionDefinition::ISOLATION_DEFAULT); $actualStatus1 = $this->manager->getTransaction($def1); $this->setExpectedException("SimpleThings\TransactionalBundle\TransactionException"); @@ -68,7 +68,7 @@ public function testGetRequireTransactionTwice() $txStatus = $this->getTxStatusMock(); $this->manager->expects($this->once())->method('doBeginTransaction')->will($this->returnValue($txStatus)); - $def = new TransactionDefinition("test", TransactionDefinition::PROPAGATION_REQUIRED, TransactionDefinition::ISOLATION_DEFAULT, true, array()); + $def = $this->createDefinition(TransactionDefinition::PROPAGATION_REQUIRED, TransactionDefinition::ISOLATION_DEFAULT); $actualStatus1 = $this->manager->getTransaction($def); $actualStatus2 = $this->manager->getTransaction($def); @@ -80,8 +80,8 @@ public function testGetRequiredIsolationLevelMissmatch() $txStatus = $this->getTxStatusMock(); $this->manager->expects($this->once())->method('doBeginTransaction')->will($this->returnValue($txStatus)); - $def1 = new TransactionDefinition("test", TransactionDefinition::PROPAGATION_REQUIRED, TransactionDefinition::ISOLATION_DEFAULT, true, array()); - $def2 = new TransactionDefinition("test", TransactionDefinition::PROPAGATION_REQUIRED, TransactionDefinition::ISOLATION_REPEATABLE_READ, true, array()); + $def1 = $this->createDefinition(TransactionDefinition::PROPAGATION_REQUIRED, TransactionDefinition::ISOLATION_DEFAULT); + $def2 = $this->createDefinition(TransactionDefinition::PROPAGATION_REQUIRED, TransactionDefinition::ISOLATION_REPEATABLE_READ); $actualStatus1 = $this->manager->getTransaction($def1); @@ -95,8 +95,8 @@ public function testGetReadOnlyMissMatch() $txStatus->expects($this->once())->method('isReadOnly')->will($this->returnValue(true)); $this->manager->expects($this->once())->method('doBeginTransaction')->will($this->returnValue($txStatus)); - $def1 = new TransactionDefinition("test", TransactionDefinition::PROPAGATION_REQUIRED, TransactionDefinition::ISOLATION_DEFAULT, true, array()); - $def2 = new TransactionDefinition("test", TransactionDefinition::PROPAGATION_REQUIRED, TransactionDefinition::ISOLATION_DEFAULT, false, array()); + $def1 = $this->createDefinition(TransactionDefinition::PROPAGATION_REQUIRED, TransactionDefinition::ISOLATION_DEFAULT); + $def2 = $this->createDefinition(TransactionDefinition::PROPAGATION_REQUIRED, TransactionDefinition::ISOLATION_DEFAULT); $actualStatus1 = $this->manager->getTransaction($def1); @@ -108,7 +108,7 @@ public function testGetTransactionPropagationSupports() { $this->manager->expects($this->never())->method('doBeginTransaction'); - $def = new TransactionDefinition("test", TransactionDefinition::PROPAGATION_SUPPORTS, TransactionDefinition::ISOLATION_DEFAULT, true, array()); + $def = $this->createDefinition(TransactionDefinition::PROPAGATION_SUPPORTS, TransactionDefinition::ISOLATION_DEFAULT); $status = $this->manager->getTransaction($def); $this->assertNull($status); @@ -119,8 +119,8 @@ public function testGetTransactionPropagationSupportsNestedInRequired() $txStatus = $this->getTxStatusMock(); $this->manager->expects($this->once())->method('doBeginTransaction')->will($this->returnValue($txStatus)); - $def1 = new TransactionDefinition("test", TransactionDefinition::PROPAGATION_REQUIRED, TransactionDefinition::ISOLATION_DEFAULT, true, array()); - $def2 = new TransactionDefinition("test", TransactionDefinition::PROPAGATION_SUPPORTS, TransactionDefinition::ISOLATION_DEFAULT, false, array()); + $def1 = $this->createDefinition(TransactionDefinition::PROPAGATION_REQUIRED, TransactionDefinition::ISOLATION_DEFAULT); + $def2 = $this->createDefinition(TransactionDefinition::PROPAGATION_SUPPORTS, TransactionDefinition::ISOLATION_DEFAULT); $actualStatus1 = $this->manager->getTransaction($def1); $actualStatus2 = $this->manager->getTransaction($def2); @@ -135,8 +135,8 @@ public function testGetTransactionRequiresNew() $this->manager->expects($this->at(0))->method('doBeginTransaction')->will($this->returnValue($txStatus1)); $this->manager->expects($this->at(1))->method('doBeginTransaction')->will($this->returnValue($txStatus2)); - $def1 = new TransactionDefinition("test", TransactionDefinition::PROPAGATION_REQUIRED, TransactionDefinition::ISOLATION_DEFAULT, true, array()); - $def2 = new TransactionDefinition("test", TransactionDefinition::PROPAGATION_REQUIRES_NEW, TransactionDefinition::ISOLATION_DEFAULT, false, array()); + $def1 = $this->createDefinition(TransactionDefinition::PROPAGATION_REQUIRED, TransactionDefinition::ISOLATION_DEFAULT); + $def2 = $this->createDefinition(TransactionDefinition::PROPAGATION_REQUIRES_NEW, TransactionDefinition::ISOLATION_DEFAULT); $actualStatus1 = $this->manager->getTransaction($def1); $actualStatus2 = $this->manager->getTransaction($def2); @@ -144,6 +144,67 @@ public function testGetTransactionRequiresNew() $this->assertNotSame($actualStatus1, $actualStatus2); } + public function testCommit() + { + $txStatus1 = $this->getTxStatusMock(); + $this->manager->expects($this->at(0))->method('doBeginTransaction')->will($this->returnValue($txStatus1)); + $this->manager->expects($this->at(1))->method('doCommit'); + $def1 = $this->createDefinition(TransactionDefinition::PROPAGATION_REQUIRED, TransactionDefinition::ISOLATION_DEFAULT); + + $actualStatus1 = $this->manager->getTransaction($def1); + + $this->manager->commit($actualStatus1); + } + + public function testCommitRecommitException() + { + $txStatus1 = $this->getTxStatusMock(); + $this->manager->expects($this->at(0))->method('doBeginTransaction')->will($this->returnValue($txStatus1)); + $this->manager->expects($this->at(1))->method('doCommit'); + $def1 = $this->createDefinition(TransactionDefinition::PROPAGATION_REQUIRED, TransactionDefinition::ISOLATION_DEFAULT); + + $actualStatus1 = $this->manager->getTransaction($def1); + + $this->manager->commit($actualStatus1); + + $this->setExpectedException("SimpleThings\TransactionalBundle\TransactionException"); + $this->manager->commit($actualStatus1); + } + + public function testCommitRollbackOnly() + { + $txStatus1 = $this->getTxStatusMock(); + $txStatus1->expects($this->once())->method('isRollBackOnly')->will($this->returnValue(true)); + + $this->manager->expects($this->at(0))->method('doBeginTransaction')->will($this->returnValue($txStatus1)); + $this->manager->expects($this->at(1))->method('doRollBack'); + $def1 = $this->createDefinition(TransactionDefinition::PROPAGATION_REQUIRED, TransactionDefinition::ISOLATION_DEFAULT); + + $actualStatus1 = $this->manager->getTransaction($def1); + $this->manager->commit($actualStatus1); + } + + public function testCommitRequiresNew() + { + $txStatus1 = $this->getTxStatusMock(); + $txStatus1->expects($this->once())->method('isRollBackOnly')->will($this->returnValue(true)); + $txStatus2 = $this->getTxStatusMock(); + + $this->manager->expects($this->at(0))->method('doBeginTransaction')->will($this->returnValue($txStatus1)); + $this->manager->expects($this->at(1))->method('doBeginTransaction')->will($this->returnValue($txStatus2)); + $this->manager->expects($this->at(2))->method('doCommit')->with($this->equalTo($txStatus2)); + $this->manager->expects($this->at(2))->method('doCommit')->with($this->equalTo($txStatus1)); + + $def1 = $this->createDefinition(TransactionDefinition::PROPAGATION_REQUIRED, TransactionDefinition::ISOLATION_DEFAULT); + $def2 = $this->createDefinition(TransactionDefinition::PROPAGATION_REQUIRES_NEW, TransactionDefinition::ISOLATION_DEFAULT); + + $actualStatus1 = $this->manager->getTransaction($def1); + + $actualStatus2 = $this->manager->getTransaction($def2); + $this->manager->commit($actualStatus2); + $this->manager->commit($actualStatus1); + } + private function createDefinition($propagation, $isolation, $readOnly = false) { return new TransactionDefinition("test", $propagation, $isolation, $readOnly, array()); diff --git a/Tests/Transactions/Http/HttpTransactionsListenerTest.php b/Tests/Transactions/Http/HttpTransactionsListenerTest.php new file mode 100644 index 0000000..01e4057 --- /dev/null +++ b/Tests/Transactions/Http/HttpTransactionsListenerTest.php @@ -0,0 +1,23 @@ + TransactionDefinition::ISOLATION_DEFAULT, 'propagation' => TransactionDefinition::PROPAGATION_REQUIRED, 'noRollbackFor' => array(), - 'subrequest' => false, ); $matcher = new TransactionalMatcher(array($pattern), array(), $this->reader); @@ -119,14 +118,12 @@ public function testThrowExceptionWhenStoreDuplicateConnectionMatch() 'isolation' => TransactionDefinition::ISOLATION_DEFAULT, 'propagation' => TransactionDefinition::PROPAGATION_REQUIRED, 'noRollbackFor' => array(), - 'subrequest' => false, ); $this->reader->expects($this->once()) ->method('getClassAnnotation') ->will($this->returnValue( new Transactional(array( 'methods' => array('GET'), - 'subrequest' => true, 'conn' => array('orm.default'), )) )); @@ -142,7 +139,7 @@ public function getPatterns() return array( array('.*', 'POST', true, false), array('SimpleThings\\\\(.+)Controller::(.+)Action', 'POST', true, false), - array('SimpleThings\\\\TransactionalBundle\\\\Tests\\\\Transactions\\\\TestController::fooAction', 'POST', true, false), + array('SimpleThings\\\\TransactionalBundle\\\\Tests\\\\Transactions\\\\Http\\\\TestController::fooAction', 'POST', true, false), array('.*', 'GET', true, true), array('SimpleThings\\\\(.+)Controller::barAction', 'POST', false, false), ); diff --git a/Transactions/AbstractTransactionManager.php b/Transactions/AbstractTransactionManager.php index a034bed..c3013e1 100644 --- a/Transactions/AbstractTransactionManager.php +++ b/Transactions/AbstractTransactionManager.php @@ -18,20 +18,10 @@ abstract class AbstractTransactionManager implements TransactionManagerInterface { /** - * @var SplStack + * @var array */ private $transactions = array(); - /** - * @var TransactionStatus - */ - private $currentTxStatus; - - public function __construct() - { - $this->transactions = new \SplObjectStorage(); - } - abstract protected function doBeginTransaction(TransactionDefinition $def); abstract protected function doCommit(TransactionStatus $def); @@ -41,9 +31,11 @@ abstract protected function doRollBack(TransactionStatus $def); protected function beginTransaction(TransactionDefinition $def) { $status = $this->doBeginTransaction($def); - $this->transactions->attach($status); - $this->transactions[$status] = $def; - $this->currentTxStatus = $status; + $oid = spl_object_hash($status); + $this->transactions[$oid] = array( + 'status' => $status, + 'def' => $def, + ); return $status; } @@ -85,43 +77,42 @@ public function getTransaction(TransactionDefinition $def) public function commit(TransactionStatus $status) { + if ($status->isRollBackOnly()) { + return $this->rollBack($status); + } + if ($status->isCompleted()) { throw new TransactionException("Cannot commit an already completed transaction."); - } else if (!$this->transactions->contains($status)) { + } else if (!isset($this->transactions[spl_object_hash($status)])) { throw new TransactionException("Cannot commit a detached transaction. It may have been committed before or belongs to another transaction manager"); } - if ($status->isRollBackOnly()) { - return $this->rollBack($status); - } - $this->doCommit($status); - $this->transactions->detach($status); + unset($this->transactions[spl_object_hash($status)]); } public function rollBack(TransactionStatus $status) { if ($status->isCompleted()) { throw new TransactionException("Cannot rollback an already completed transaction."); - } else if (!$this->transactions->contains($status)) { + } else if (!isset($this->transactions[spl_object_hash($status)])) { throw new TransactionException("Cannot rollback a detached transaction. It may have been committed/rollbacked before or belongs to another transaction manager"); } $this->doRollBack($status); - $this->transactions->detach($status); + unset($this->transactions[spl_object_hash($status)]); } private function getCurrentTransaction() { - return $this->currentTxStatus; + $tx = end($this->transactions); + return $tx['status']; } private function getCurrentTransactionDef() { - if ($this->currentTxStatus) { - return $this->transactions[$this->currentTxStatus]; - } - return null; + $tx = end($this->transactions); + return $tx['def']; } } diff --git a/Annotations/Transactional.php b/Transactions/Annotations/Transactional.php similarity index 80% rename from Annotations/Transactional.php rename to Transactions/Annotations/Transactional.php index 6a51d23..a46b97f 100644 --- a/Annotations/Transactional.php +++ b/Transactions/Annotations/Transactional.php @@ -12,10 +12,9 @@ * to kontakt@beberlei.de so I can send you a copy immediately. */ -namespace SimpleThings\TransactionalBundle\Annotations; +namespace SimpleThings\TransactionalBundle\Transactions\Annotations; use Doctrine\Common\Annotations\Annotation; -use SimpleThings\TransactionalBundle\Transactions\TransactionDefinition; /** * @Annotation @@ -42,9 +41,5 @@ class Transactional extends Annotation * @var array */ public $methods = null; +} - /** - * @var bool - */ - public $subrequest = null; -} \ No newline at end of file diff --git a/Transactions/Http/HttpTransactionsListener.php b/Transactions/Http/HttpTransactionsListener.php new file mode 100644 index 0000000..0bc31a9 --- /dev/null +++ b/Transactions/Http/HttpTransactionsListener.php @@ -0,0 +1,115 @@ +registry = $registry; + $this->matcher = $matcher; + $this->logger = $logger; + } + + public function onCoreController(FilterControllerEvent $event) + { + $request = $event->getRequest(); + $definitions = $this->matcher->match($request->getMethod(), $event->getController()); + $txManagers = $this->registry->getTransactions($definitions); + + if ($txManagers && $this->logger) { + $this->logger->info("[TransactionBundle] Started transactions for " . implode(", ", array_keys($txManagers))); + } + + $request->attributes->set('_transactions', $txManagers); + } + + public function onKernelResponse(FilterResponseEvent $event) + { + $request = $event->getRequest(); + $response = $event->getResponse(); + + if (!$request->attributes->has('_transactions')) { + return; + } + + $txManagers = $request->attributes->get('_transactions'); + if ($response->getStatusCode() >= 400 && $response->getStatusCode() != 404) { + $this->rollBack($txManagers); + } else { + $this->commit($txManagers); + } + } + + public function onKernelException(GetResponseForExceptionEvent $event) + { + $request = $event->getRequest(); + $ex = $event->getException(); + + if (!$request->attributes->has('_transactions')) { + return; + } + + $txManagers = $request->attributes->get('_transactions'); + + if ($ex instanceof NotFoundHttpException) { + $this->registry->commit($txManagers); + } else { + $this->registry->rollBack($txManagers); + } + } + + private function commit($txManagers) + { + $this->registry->commit($txManagers); + + if ($txManagers && $this->logger) { + $this->logger->info("[TransactionBundle] Committed transactions for " . implode(", ", array_keys($txManagers))); + } + } + + private function rollBack($txManagers) + { + $this->registry->rollBack($txManagers); + + if ($txManagers && $this->logger) { + $this->logger->info("[TransactionBundle] Aborted transactions for " . implode(", ", array_keys($txManagers))); + } + } +} + diff --git a/Transactions/TransactionalMatcher.php b/Transactions/Http/TransactionalMatcher.php similarity index 97% rename from Transactions/TransactionalMatcher.php rename to Transactions/Http/TransactionalMatcher.php index fb1f964..a1081ed 100644 --- a/Transactions/TransactionalMatcher.php +++ b/Transactions/Http/TransactionalMatcher.php @@ -12,11 +12,12 @@ * to kontakt@beberlei.de so I can send you a copy immediately. */ -namespace SimpleThings\TransactionalBundle\Transactions; +namespace SimpleThings\TransactionalBundle\Transactions\Http; use Symfony\Component\HttpFoundation\Request; use Doctrine\Common\Annotations\Reader; use SimpleThings\TransactionalBundle\TransactionException; +use SimpleThings\TransactionalBundle\Transactions\TransactionDefinition; /** * TransactionalMatcher finds the transaction definitions for matched diff --git a/Transactions/TransactionsRegistry.php b/Transactions/TransactionsRegistry.php new file mode 100644 index 0000000..f67ffa0 --- /dev/null +++ b/Transactions/TransactionsRegistry.php @@ -0,0 +1,71 @@ +container = $container; + } + + public function getTransactions(array $definitions) + { + $txManagers = array(); + foreach ($definitions as $def) { + $managerName = $def->getManagerName(); + if ($txStatus = $this->getTransactionManager($managerName)->getTransaction($def)) { + $txManagers[$managerName] = $txStatus; + } + } + return $txManagers; + } + + public function commit(array $statuses) + { + foreach ($txManagers AS $managerName => $txStatus) { + $this->getTransactionManager($managerName)->commit($txStatus); + } + } + + public function rollBack(array $statuses) + { + foreach ($txManagers AS $managerName => $txStatus) { + $this->getTransactionManager($managerName)->rollBack($txStatus); + } + } + + private function getTransactionManager($name) + { + $id = "simple_things_transactional.tx.".$name; + if (!$this->container->has($id)) { + throw new \InvalidArgumentException( + "A transactional manager by name of '".$name."' was requested, but does not exist." + ); + } + return $this->container->get($id); + } +} + From e44abbb3a699fb763005ddd524da5eaf37f1ae4a Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Sun, 8 Jan 2012 17:40:08 +0100 Subject: [PATCH 08/24] Implement ObjectTransactionManager and Status. Started writing DESIGN_DOCS to discuss the problems with REQUIRES_NEW transaction type. Add Tests and refactored HttpTransactionsListener. --- Doctrine/ObjectTransactionManager.php | 71 ++++++++++++ Doctrine/ObjectTransactionStatus.php | 107 ++++++++++++++++++ Resources/docs/DESIGN_DOCS | 79 +++++++++++++ .../Http/HttpTransactionsListenerTest.php | 30 +++++ .../Http/HttpTransactionsListener.php | 1 - Transactions/TransactionsRegistry.php | 4 +- 6 files changed, 290 insertions(+), 2 deletions(-) create mode 100644 Doctrine/ObjectTransactionManager.php create mode 100644 Doctrine/ObjectTransactionStatus.php create mode 100644 Resources/docs/DESIGN_DOCS diff --git a/Doctrine/ObjectTransactionManager.php b/Doctrine/ObjectTransactionManager.php new file mode 100644 index 0000000..0056498 --- /dev/null +++ b/Doctrine/ObjectTransactionManager.php @@ -0,0 +1,71 @@ +registry = $registry; + } + + protected function doBeginTransaction(TransactionDefinition $def) + { + $parts = split(".", $def->getManagerName()); + $name = end($parts); + + $manager = $this->registry->getManager($name); + // this could be an already instantiated manager, check and reset if + // necessary. + if ($this->managers->contains($manager)) { + $this->registry->resetManager($name); + return $this->doBeginTransaction($def); + } + $this->managers->add($manager); + + return $this->createTxStatus($manager, $def); + } + + protected function doCommit(TransactionStatus $def) + { + $this->managers->detach($manager); + $def->commit(); + } + + protected function doRollBack(TransactionStatus $def) + { + $this->managers->detach($manager); + $def->rollBack(); + } + + protected function createTxStatus($manager, $def) + { + return new ObjectTransactionStatus($manager, $def); + } +} + diff --git a/Doctrine/ObjectTransactionStatus.php b/Doctrine/ObjectTransactionStatus.php new file mode 100644 index 0000000..12797dc --- /dev/null +++ b/Doctrine/ObjectTransactionStatus.php @@ -0,0 +1,107 @@ +manager = $manager; + $this->def = $def; + } + + /** + * Checks if the transaction is read-only. + * + * A read-only transaction does not commit changes to the database when + * commit is called. It allows the underlying transaction manager to + * perform optimizations to this regard if possible. + * + * @return bool + */ + public function isReadOnly() + { + return $this->def->getReadOnly(); + } + + /** + * Check if transaction is broken and has to be rolled back at this point. + * + * @return bool + */ + public function isRollBackOnly() + { + return $this->rollbackOnly; + } + + /** + * Mark the transaction as rollback-only + * + * @return void + */ + public function setRollBackOnly() + { + $this->rollbackOnly = true; + } + + /** + * Check if this transaction was committed already. + * + * @return bool + */ + public function isCompleted() + { + return $this->completed; + } + + /** + * Check if this transaction has savepoints. + * + * @return bool + */ + public function hasSavepoint() + { + return false; + } + + /** + * Commit the transaction at this point. + * + * @return void + */ + public function commit() + { + $this>completed = true; + $this->manager->flush(); + } + + /** + * Rollback the transaction at this point marking it as complete. + * + * @return void + */ + public function rollBack() + { + $this->completed = true; + $this->manager->clear(); + } +} + diff --git a/Resources/docs/DESIGN_DOCS b/Resources/docs/DESIGN_DOCS new file mode 100644 index 0000000..bd196c3 --- /dev/null +++ b/Resources/docs/DESIGN_DOCS @@ -0,0 +1,79 @@ +Design Document: Transaction Management in Symfony +================================================== + +Symfony is a HTTP-framework. The framework transaction management should be based +on this request/response semantic. A transaction should be opened for every request +and committed when the response is sent and no error occured. Currently Symfony does +not handle transactions at all, leaving this to the developer. + +HTTP Transaction Management +--------------------------- + +A transaction for all involved database resources of a controller should be started +in the kernel controller event. The kernel.response and kernel.exception events handle +successes and errors respectively. Different types of exceptions should either lead to +commit or rollback of the transaction. + +Since Symfony supports sub-requests we have to decide how to handle subrequests: + +1. Re-use existing transaction scope. If the subrequest transaction fails the + parent requests transaction is rolled back as well. + + This is useful for subrequests that are either not failing by design or + that explicitly should rollback the parent transactions as well. + +2. Don't care about transactions (re-use existing, or don't open a transaction) + This is useful for view slots (render) that only need the database for read access. + +3. Start their own sub-transaction. This poses several problems: The existing transactional + service could already be injected into several services that are container scoped and could be + resused in the action that "requires new" transaction. Additionally we have to handle + the transaction stack and "suspend" and "resume" transactions correctly. + + This type is useful if subrequests do data-manipulation that should never affect other transactions, + or if several subrequests are started (slots) that should run independently of each other. + +4. Manually + + No Transaction should be started already and the user wants to manually handle all the transactions. + This is the current Symfony default (with no support for transactions). It is however sometimes necessary + in complex scenarios to manually handle transactions. + +New Transaction in Sub-Requests +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +Staring new transactions in sub-requests creates a bunch of problems that have to be solved: + +1. How to guarantee that all services in the sub-request contain references to the connection + with the new transaction? +2. How to suspend and resume the previous transaction if the sub-request is completed. + +First we have to evaluate what we want from this feature: + +1. Independence of sub-request transactions from any other sources. The sub-request has to complete or rollback alone. +2. Independence of the parent requests from the sub-request, which maybe unstable with regard to tranaction success. + +In short: A REQUIRES_NEW transaction should behave as if it were a master-request. No effects of the transaction should +be visible outside of it. + +Solution 1: Transactional Scope +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +We may need a container scope "transactional" for this. Problems with this are: + +* Say service X is used in the REQUIRES_NEW controller. Then it has to be marked as "transactional" scope. +* If it depends on any connection these have to be marked as "transactional" too. +* Every service depending on any connection is then required to be "transactional". + +How can we break up this problem from point 2 to 3? We want services to be container scoped by default. + +Problems: + +* We have to automatically implicitly move all services depending on connections from container to transactional scope. +* Transaction scope has to be the parent of request scope. + +Solution 2: Connection Proxies +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +Each connection is a proxy to the real connection and returns the right/new connection. + diff --git a/Tests/Transactions/Http/HttpTransactionsListenerTest.php b/Tests/Transactions/Http/HttpTransactionsListenerTest.php index 01e4057..aaf5453 100644 --- a/Tests/Transactions/Http/HttpTransactionsListenerTest.php +++ b/Tests/Transactions/Http/HttpTransactionsListenerTest.php @@ -13,11 +13,41 @@ namespace SimpleThings\TransactionalBundle\Tests\Transactions\Http; +use SimpleThings\TransactionalBundle\Transactions\Http\HttpTransactionsListener; +use SimpleThings\TransactionalBundle\Transactions\TransactionDefinition; +use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\HttpKernel\Event\FilterControllerEvent; +use Symfony\Component\HttpKernel\Event\FilterResponseEvent; +use Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent; + class HttpTransactionsListenerTest extends \PHPUnit_Framework_TestCase { + private $matcher; + private $registry; + private $listener; + public function setUp() { + $this->matcher = $this->getMock('SimpleThings\TransactionalBundle\Transactions\Http\TransactionalMatcher', array(), array(), '', false); + $this->registry = $this->getMock('SimpleThings\TransactionalBundle\Transactions\TransactionsRegistry', array(), array(), '', false); + $this->listener = new HttpTransactionsListener($this->registry, $this->matcher); + } + + public function testOnCoreController() + { + $kernel = $this->getMock('Symfony\Component\HttpKernel\HttpKernelInterface'); + $request = Request::create('/foo', 'POST'); + $event = new FilterControllerEvent($kernel, array(__CLASS__, 'testOnCoreController'), $request, null); + + $txStatus = $this->getMock('SimpleThings\TransactionalBundle\Transactions\TransactionStatus'); + $def = new TransactionDefinition('name', 1, 1, false, array()); + + $this->matcher->expects($this->once())->method('match')->will($this->returnValue(array($def))); + $this->registry->expects($this->once())->method('getTransactions')->with($this->equalTo(array($def)))->will($this->returnValue(array($txStatus))); + + $this->listener->onCoreController($event); + $this->assertSame(array($txStatus), $request->attributes->get('_transactions')); } } diff --git a/Transactions/Http/HttpTransactionsListener.php b/Transactions/Http/HttpTransactionsListener.php index 0bc31a9..6cf5c16 100644 --- a/Transactions/Http/HttpTransactionsListener.php +++ b/Transactions/Http/HttpTransactionsListener.php @@ -20,7 +20,6 @@ use Symfony\Component\HttpKernel\HttpKernelInterface; use Symfony\Component\HttpKernel\Log\LoggerInterface; use Symfony\Component\HttpFoundation\Exceptions\NotFoundHttpException; -use SimpleThings\TransactionalBundle\Transactions\TransactionalMatcher; use SimpleThings\TransactionalBundle\Transactions\TransactionsRegistry; /** diff --git a/Transactions/TransactionsRegistry.php b/Transactions/TransactionsRegistry.php index f67ffa0..d835a55 100644 --- a/Transactions/TransactionsRegistry.php +++ b/Transactions/TransactionsRegistry.php @@ -25,8 +25,10 @@ class TransactionsRegistry { private $container; + private $connectionServices; + private $connections = array(); - public function __construct(ContainerInterface $container) + public function __construct(ContainerInterface $container, $connectionServices = array()) { $this->container = $container; } From bec3deb5ddeea2b497414dd0ad526d90baad9130 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Sun, 8 Jan 2012 17:42:39 +0100 Subject: [PATCH 09/24] Remove HttpTransactionsListener in Controller folder --- Controller/HttpTransactionsListener.php | 115 ------------------------ 1 file changed, 115 deletions(-) delete mode 100644 Controller/HttpTransactionsListener.php diff --git a/Controller/HttpTransactionsListener.php b/Controller/HttpTransactionsListener.php deleted file mode 100644 index d51496d..0000000 --- a/Controller/HttpTransactionsListener.php +++ /dev/null @@ -1,115 +0,0 @@ -registry = $registry; - $this->matcher = $matcher; - $this->logger = $logger; - } - - public function onCoreController(FilterControllerEvent $event) - { - $request = $event->getRequest(); - $definitions = $this->matcher->match($request->getMethod(), $event->getController()); - $txManagers = $this->registry->getTransactions($definitions); - - if ($txManagers && $this->logger) { - $this->logger->info("[TransactionBundle] Started transactions for " . implode(", ", array_keys($txManagers))); - } - - $request->attributes->set('_transactions', $txManagers); - } - - public function onKernelResponse(FilterResponseEvent $event) - { - $request = $event->getRequest(); - $response = $event->getResponse(); - - if (!$request->attributes->has('_transactions')) { - return; - } - - $txManagers = $request->attributes->get('_transactions'); - if ($response->getStatusCode() >= 400 && $response->getStatusCode() != 404) { - $this->rollBack($txManagers); - } else { - $this->commit($txManagers); - } - } - - public function onKernelException(GetResponseForExceptionEvent $event) - { - $request = $event->getRequest(); - $ex = $event->getException(); - - if (!$request->attributes->has('_transactions')) { - return; - } - - $txManagers = $request->attributes->get('_transactions'); - - if ($ex instanceof NotFoundHttpException) { - $this->registry->commit($txManagers); - } else { - $this->registry->rollBack($txManagers); - } - } - - private function commit($txManagers) - { - $this->registry->commit($txManagers); - - if ($txManagers && $this->logger) { - $this->logger->info("[TransactionBundle] Committed transactions for " . implode(", ", array_keys($txManagers))); - } - } - - private function rollBack($txManagers) - { - $this->registry->rollBack($txManagers); - - if ($txManagers && $this->logger) { - $this->logger->info("[TransactionBundle] Aborted transactions for " . implode(", ", array_keys($txManagers))); - } - } -} - From 6b5f83d60561154ce51ed804f25140466396f8a5 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Sun, 8 Jan 2012 21:10:49 +0100 Subject: [PATCH 10/24] Refactor towards Transactional Scope --- .../CompilerPass/DetectConnectionPass.php | 67 +++++++++++++++++ .../CompilerPass/TransactionalScopePass.php | 59 +++++++++++++++ README.markdown | 72 ++++++++++++------- Resources/config/services.xml | 23 ++++-- Resources/docs/DESIGN_DOCS | 18 +++++ SimpleThingsTransactionalBundle.php | 25 ++++++- .../AbstractTransactionManagerTest.php | 3 +- .../Http/TransactionalMatcherTest.php | 10 +-- Transactions/AbstractTransactionManager.php | 23 +++++- Transactions/Annotations/Transactional.php | 2 +- Transactions/Form/RollbackInvalidForm.php | 49 +++++++++++++ .../Form/RollbackInvalidFormExtension.php | 31 ++++++++ .../Form/RollbackInvalidFormValidator.php | 49 +++++++++++++ Transactions/Http/TransactionalMatcher.php | 18 +++-- Transactions/ScopeHandler.php | 61 ++++++++++++++++ Transactions/TransactionsRegistry.php | 15 +++- 16 files changed, 472 insertions(+), 53 deletions(-) create mode 100644 DependencyInjection/CompilerPass/DetectConnectionPass.php create mode 100644 DependencyInjection/CompilerPass/TransactionalScopePass.php create mode 100644 Transactions/Form/RollbackInvalidForm.php create mode 100644 Transactions/Form/RollbackInvalidFormExtension.php create mode 100644 Transactions/Form/RollbackInvalidFormValidator.php create mode 100644 Transactions/ScopeHandler.php diff --git a/DependencyInjection/CompilerPass/DetectConnectionPass.php b/DependencyInjection/CompilerPass/DetectConnectionPass.php new file mode 100644 index 0000000..7ea4d90 --- /dev/null +++ b/DependencyInjection/CompilerPass/DetectConnectionPass.php @@ -0,0 +1,67 @@ +hasParameter('doctrine.connections')) { + foreach ($builder->getParameter('doctrine.connections') AS $alias => $service) { + $builder->setDefinition( + 'simple_things_transactional.tx.dbal.'.$alias, + new DefinitionDecorator('simple_things_transactional.manager.dbal') + )->setArguments(array(new Reference($service))); + } + } + + if ($builder->hasParameter('doctrine.entity_managers')) { + foreach ($builder->getParameter('doctrine.entity_managers') AS $alias => $service) { + $builder->setDefinition( + 'simple_things_transactional.tx.orm.'.$alias, + new DefinitionDecorator('simple_things_transactional.manager.orm') + )->setArguments(array(new Reference('doctrine'), $alias)); + } + } + + if ($builder->hasParameter('doctrine_couchdb.document_managers')) { + foreach ($builder->getParameter('doctrine_couchdb.document_managers') AS $alias => $service) { + $builder->setDefinition( + 'simple_things_transactional.tx.couchdb.'.$alias, + new DefinitionDecorator('simple_things_transactional.manager.object_manager') + )->setArguments(array(new Reference($service))); + } + } + + if ($builder->hasParameter('doctrine_mongodb.document_managers')) { + foreach ($builder->getParameter('doctrine_mongodb.document_managers') AS $alias => $service) { + $builder->setDefinition( + 'simple_things_transactional.tx.mongodb.'.$alias, + new DefinitionDecorator('simple_things_transactional.manager.object_manager') + )->setArguments(array(new Reference($service))); + } + } + */ + // add tags as well for external resources (Propel, raw-PDO whatever) + } +} diff --git a/DependencyInjection/CompilerPass/TransactionalScopePass.php b/DependencyInjection/CompilerPass/TransactionalScopePass.php new file mode 100644 index 0000000..ee055e8 --- /dev/null +++ b/DependencyInjection/CompilerPass/TransactionalScopePass.php @@ -0,0 +1,59 @@ + + */ +class TransactionalScopePass implements CompilerPassInterface +{ + public function process(ContainerBuilder $container) + { + $connectionServices = $container->getParameter('simple_things_transactional.connection_serices'); + $graph = $container->getCompiler()->getServiceReferenceGraph(); + + $visited = array(); + foreach ($connectionServices as $connectionServiceId) { + $this->changeScopeTransactional($connectionServiceId, $visited); + } + } + + private function changeScopeTransactional($serviceId, $visited) + { + if (isset($visited[$serviceId])) { + return; + } + $visited[$serviceId] = true; + + $node = $graph->getNode($serviceId); + $def = $container->getDefinition($serviceId); + + if ($def->getScope() == ContainerInterface::SCOPE_CONTAINER) { + $def->setScope('transactional'); + } + + foreach ($node->getOutNodes() as $outNode) { + $this->changeScopeTransactional($outNode->getId(), $visited); + } + } +} + diff --git a/README.markdown b/README.markdown index 223fec3..0fd3f18 100644 --- a/README.markdown +++ b/README.markdown @@ -25,9 +25,9 @@ creates a service that implements a transactions manager interface: interface TransactionManagerInterface { - function beginTransaction(); - function commit(); - function rollBack(); + function getTransaction(TransactionDefinition $def); + function commit(TransactionStatus $status); + function rollBack(TransactionStatus $status); } With the transactional bundle the following workflow is applied to an action that is marked @@ -56,7 +56,7 @@ is as simple as configuring the transactional managers name in the app/config/co simple_things_transactional: auto_transactional: true defaults: - conn: ["orm.default"] + conn: "orm.default" With this configuration every POST, PUT, DELETE and PATCH request is wrapped inside a transaction of the given connection. There is no way to disable this behavior except by throwing an exception. GET requests that need to write a transaction @@ -73,7 +73,7 @@ If a transaction is started for a connection multiple times then an exception is simple_things_transactional: defaults: - conn: ["mongodb.default"] + conn: "mongodb.default" methods: ["POST", "PUT", "DELETE", "PATCH"] patterns: fos_user: @@ -81,66 +81,85 @@ If a transaction is started for a connection multiple times then an exception is # not giving conn: uses the default propagation: REQUIRES_NEW noRollbackFor: ["NotFoundHttpException"] - subrequest: true acme: pattern: "Acme(.*)" - conn: ["orm.default", "couchdb.default"] - subrequest: false + conn: "orm.default" acme_logging: pattern: "Acme\DemoBundle\Controller\IndexController::logAction" - conn: ["dbal.other"] + isolation: READ_UNCOMMITTED + conn: "orm.other" methods: ["GET"] ### Annotations -You can also configure transactional behavior with annotations. The configuration for annotations is as simple as: +You can also configure transactional behavior with annotations. Enabling annotations is simple: simple_things_transactional: annotations: true -The previous `Acme\DemoBundle\Controller\IndexController` then looks like: +The previous `Acme\DemoBundle\Controller\IndexController` can then be configured by adding: namespace Acme\DemoBundle\Controller; - use SimpleThings\TransactionalBundle\Annotations AS Tx; + use SimpleThings\TransactionalBundle\Transactions\Annotations AS Tx; /** - * @Tx\Transactional(conn={"orm.default", "couchdb.default"}) + * @Tx\Transactional(conn="orm.default") */ class IndexController { + public function indexAction() + { + + } + /** - * @Tx\Transactional(conn={"orm.other"}, methods: {"GET"}) + * @Tx\Transactional(conn="orm.other", methods: {"GET"}) */ public function demoAction() { - + // both orm.default and orm.other are transactions here } } -## Example +## Doctrine ORM Example + +This example assumes: -Using the previous routes as example here is a sample action that does not require any calls to EntityManager::flush anymore. +* You are using FrameworkExtraBundle and converters +* You automatically use HTTP semantics for tx management. + +See this controller for a Form Edit/Display. + +* It will commit all the changes automatically when a post request occurs. +* To rollback the transaction when a form error occurs the transactional + bundle automatically registers a form validator and sets all current + transactions to 'rollback only'. + +Here is the code: class PostController extends Controller { - public function editAction($id) + /** + * @ParamConverter("post", class="AcmeBlogBundle:Post") + * @Template + */ + public function editAction(Post $post, Request $request) { - $em = $this->container->get('doctrine.orm.default_entity_manager'); - $post = $em->find('Post', $id); + $form = $this->createForm(new PostType(), $post); - if ($this->container->get('request')->getMethod() == 'POST') { - $post->modifyState(); - // no need to call $em->flush(), the flush is executed in a transactional wrapper + if ($request->getMethod() == 'POST') { + $form->bindRequest($request); - return $this->redirect($this->generateUrl("view_post", array("id" => $post->getId())); + if ($form->isValid()) { + return $this->redirect($this->generateUrl("view_post", array("id" => $post->getId())); + } } - return $this->render("MyBlogBundle:Post:edit.html.twig", array()); + return array('form' => $form->createView()); } } -`EntityManager#flush()` is only called when the requet is using the POST-method. ## Installation @@ -175,4 +194,3 @@ Using the previous routes as example here is a sample action that does not requi * Implement Propagation * Implement Isolation -* Try to evaluate if hooking into exception_handler is a killing exceptions from controllers more gracefully and not having them loose the stack trace. diff --git a/Resources/config/services.xml b/Resources/config/services.xml index 9e67144..a687463 100644 --- a/Resources/config/services.xml +++ b/Resources/config/services.xml @@ -9,10 +9,10 @@ SimpleThings\TransactionalBundle\Transactions\Doctrine\EntityManagerTransactionManager SimpleThings\TransactionalBundle\Transactions\Doctrine\MongoDBTransactionManager SimpleThings\TransactionalBundle\Transactions\Doctrine\CouchDBTransactionManager - SimpleThings\TransactionalBundle\Controller\HttpTransactionsListener - SimpleThings\TransactionalBundle\Transactions\TransactionalMatcher - SimpleThings\TransactionalBundle\Controller\ControllerResolver - SimpleThings\TransactionalBundle\Controller\TraceableControllerResolver + SimpleThings\TransactionalBundle\Transactions\Http\HttpTransactionsListener + SimpleThings\TransactionalBundle\Transactions\Http\TransactionalMatcher + SimpleThings\TransactionalBundle\Transactions\TransactionsRegistry + @@ -22,14 +22,25 @@ - + + - + + + + + + + %simple_things_transactional.connection_services% + + + + diff --git a/Resources/docs/DESIGN_DOCS b/Resources/docs/DESIGN_DOCS index bd196c3..2ca65ce 100644 --- a/Resources/docs/DESIGN_DOCS +++ b/Resources/docs/DESIGN_DOCS @@ -77,3 +77,21 @@ Solution 2: Connection Proxies Each connection is a proxy to the real connection and returns the right/new connection. +Combined Transactions +--------------------- + +What if one controller requires a transaction for several different resources. + +* Do commits have to be synchronized? (Two-phase commit) - This is very complex +* Do rollbacks have to be synchronized (if one is isRollbackOnly, all others rollback as well) + +For the beginning I would say that transactions can fail and commit independently. + +If a user requires synchronization then he should implement his own transaction manager and +implement two-phase-commit manually. + +Read Only +--------- + +Transactions are bound to the HTTP Verb semantics. By default GET requests create Read-Only transactions. +Read-only transactions never commit at the end of the response/transaction cycle. diff --git a/SimpleThingsTransactionalBundle.php b/SimpleThingsTransactionalBundle.php index b3bd5ef..722bd17 100644 --- a/SimpleThingsTransactionalBundle.php +++ b/SimpleThingsTransactionalBundle.php @@ -15,8 +15,29 @@ namespace SimpleThings\TransactionalBundle; use Symfony\Component\HttpKernel\Bundle\Bundle; +use Symfony\Component\DependencyInjection\ContainerBuilder; +use Symfony\Component\DependencyInjection\Scope; +use SimpleThings\TransactionalBundle\DependencyInjection\CompilerPass\TransactionalScopePass; +use SimpleThings\TransactionalBundle\DependencyInjection\CompilerPass\DetectConnectionsPass; class SimpleThingsTransactionalBundle extends Bundle { - -} \ No newline at end of file + public function boot() + { + $this->getContainer()->enterScope('transactional'); + } + + public function build(ContainerBuilder $container) + { + parent::build($container); + + $container->addScope(new Scope('transactional')); + $container->addCompilerPass(new TransactionalScopePass()); + $container->addCompilerPass(new DetectConnectionsPass()); + } + + public function shutdown() + { + $this->getContainer()->leaveScope('transactional'); + } +} diff --git a/Tests/Transactions/AbstractTransactionManagerTest.php b/Tests/Transactions/AbstractTransactionManagerTest.php index 1054ed5..beb856a 100644 --- a/Tests/Transactions/AbstractTransactionManagerTest.php +++ b/Tests/Transactions/AbstractTransactionManagerTest.php @@ -23,7 +23,8 @@ public function setUp() { $this->manager = $this->getMock( 'SimpleThings\TransactionalBundle\Transactions\AbstractTransactionManager', - array('doBeginTransaction', 'doCommit', 'doRollBack') + array('doBeginTransaction', 'doCommit', 'doRollBack'), + array($this->getMock('SimpleThings\TransactionalBundle\Transactions\ScopeHandler', array(), array(), '', false)) ); } diff --git a/Tests/Transactions/Http/TransactionalMatcherTest.php b/Tests/Transactions/Http/TransactionalMatcherTest.php index 1d4a096..e41ca54 100644 --- a/Tests/Transactions/Http/TransactionalMatcherTest.php +++ b/Tests/Transactions/Http/TransactionalMatcherTest.php @@ -19,7 +19,7 @@ public function testMatchPattern($pattern, $method, $matched, $readOnly) $pattern = array( 'pattern' => $pattern, 'methods' => array('POST', 'PUT'), - 'conn' => array('orm.default'), + 'conn' => 'orm.default', 'isolation' => TransactionDefinition::ISOLATION_DEFAULT, 'propagation' => TransactionDefinition::PROPAGATION_REQUIRED, 'noRollbackFor' => array(), @@ -56,7 +56,7 @@ public function testMatchClassAnnotation() ->will($this->returnValue( new Transactional(array( 'methods' => array('GET'), - 'conn' => array('orm.default'), + 'conn' => 'orm.default', )) )); @@ -90,7 +90,7 @@ public function testMatchMethodAnnotation() ->will($this->returnValue( new Transactional(array( 'methods' => array('GET'), - 'conn' => array('orm.default'), + 'conn' => 'orm.default', )) )); @@ -114,7 +114,7 @@ public function testThrowExceptionWhenStoreDuplicateConnectionMatch() $pattern = array( 'pattern' => '.*', 'methods' => array('GET'), - 'conn' => array('orm.default'), + 'conn' => 'orm.default', 'isolation' => TransactionDefinition::ISOLATION_DEFAULT, 'propagation' => TransactionDefinition::PROPAGATION_REQUIRED, 'noRollbackFor' => array(), @@ -124,7 +124,7 @@ public function testThrowExceptionWhenStoreDuplicateConnectionMatch() ->will($this->returnValue( new Transactional(array( 'methods' => array('GET'), - 'conn' => array('orm.default'), + 'conn' => 'orm.default', )) )); diff --git a/Transactions/AbstractTransactionManager.php b/Transactions/AbstractTransactionManager.php index c3013e1..0f25b6e 100644 --- a/Transactions/AbstractTransactionManager.php +++ b/Transactions/AbstractTransactionManager.php @@ -22,6 +22,16 @@ abstract class AbstractTransactionManager implements TransactionManagerInterface */ private $transactions = array(); + /** + * @var ScopeHandler + */ + private $scope; + + public function __construct(ScopeHandler $scope) + { + $this->scope = $scope; + } + abstract protected function doBeginTransaction(TransactionDefinition $def); abstract protected function doCommit(TransactionStatus $def); @@ -43,6 +53,7 @@ public function getTransaction(TransactionDefinition $def) { switch ($def->getPropagation()) { case TransactionDefinition::PROPAGATION_REQUIRES_NEW: + $this->scope->enterScope(); $status = $this->beginTransaction($def); break; case TransactionDefinition::PROPAGATION_NEVER: @@ -87,8 +98,8 @@ public function commit(TransactionStatus $status) throw new TransactionException("Cannot commit a detached transaction. It may have been committed before or belongs to another transaction manager"); } + $this->cleanupAfterTransaction($status); $this->doCommit($status); - unset($this->transactions[spl_object_hash($status)]); } public function rollBack(TransactionStatus $status) @@ -99,7 +110,17 @@ public function rollBack(TransactionStatus $status) throw new TransactionException("Cannot rollback a detached transaction. It may have been committed/rollbacked before or belongs to another transaction manager"); } + $this->cleanupAfterTransaction($status); $this->doRollBack($status); + } + + private function cleanupAfterTransaction($status) + { + $def = $this->transactions[spl_object_hash($status)]['def']; + if ($def->getPropagation() == TransactionDefinition::PROPAGATION_REQUIRES_NEW) { + $this->scope->leaveScope(); + } + unset($this->transactions[spl_object_hash($status)]); } diff --git a/Transactions/Annotations/Transactional.php b/Transactions/Annotations/Transactional.php index a46b97f..872d2e0 100644 --- a/Transactions/Annotations/Transactional.php +++ b/Transactions/Annotations/Transactional.php @@ -22,7 +22,7 @@ class Transactional extends Annotation { /** - * @var array + * @var string */ public $conn = null; /** diff --git a/Transactions/Form/RollbackInvalidForm.php b/Transactions/Form/RollbackInvalidForm.php new file mode 100644 index 0000000..8a8e393 --- /dev/null +++ b/Transactions/Form/RollbackInvalidForm.php @@ -0,0 +1,49 @@ + + */ +class RollbackInvalidFormValidator implements FormValidatorInterface +{ + private $container; + + public function __construct($container) + { + $this->container = $container; + } + + public function validate(FormInterface $form) + { + if (!$this->container->has('request')) { + return; + } + + $request = $this->container->get('request'); + if (!$form->isValid()) { + foreach ($request->attributes->get('_transactions') as $tx) { + $tx->setRollbackOnly(); + } + } + } +} + diff --git a/Transactions/Form/RollbackInvalidFormExtension.php b/Transactions/Form/RollbackInvalidFormExtension.php new file mode 100644 index 0000000..18c983d --- /dev/null +++ b/Transactions/Form/RollbackInvalidFormExtension.php @@ -0,0 +1,31 @@ +addValidator(new RollbackInvalidFormValidator()); + } + + public function getExtendedType() + { + return 'form'; + } +} + diff --git a/Transactions/Form/RollbackInvalidFormValidator.php b/Transactions/Form/RollbackInvalidFormValidator.php new file mode 100644 index 0000000..8a8e393 --- /dev/null +++ b/Transactions/Form/RollbackInvalidFormValidator.php @@ -0,0 +1,49 @@ + + */ +class RollbackInvalidFormValidator implements FormValidatorInterface +{ + private $container; + + public function __construct($container) + { + $this->container = $container; + } + + public function validate(FormInterface $form) + { + if (!$this->container->has('request')) { + return; + } + + $request = $this->container->get('request'); + if (!$form->isValid()) { + foreach ($request->attributes->get('_transactions') as $tx) { + $tx->setRollbackOnly(); + } + } + } +} + diff --git a/Transactions/Http/TransactionalMatcher.php b/Transactions/Http/TransactionalMatcher.php index a1081ed..ccd0bd4 100644 --- a/Transactions/Http/TransactionalMatcher.php +++ b/Transactions/Http/TransactionalMatcher.php @@ -82,7 +82,12 @@ public function match($method, $controllerCallback) } $definitions = array(); + $requireNew = false; foreach ($this->cache[$subject] as $connectionName => $definition) { + if ($definition['propagation'] == TransactionDefinition::PROPAGATION_REQUIRES_NEW) { + + } + $definitions[] = new TransactionDefinition( $definition['managerName'], $definition['propagation'], @@ -136,18 +141,17 @@ private function matchAnnotations($subject, $controller, $action) private function storeMatch($subject, $pattern) { - foreach ($pattern['conn'] AS $managerName) { - if (isset($this->cache[$subject][$managerName])) { - throw TransactionException::duplicateConnectionMatch($managerName, $pattern); - } + $managerName = $pattern['conn']; + if (isset($this->cache[$subject][$managerName])) { + throw TransactionException::duplicateConnectionMatch($managerName, $pattern); + } - $this->cache[$subject][$managerName] = array( + $this->cache[$subject][$managerName] = array( 'managerName' => $managerName, 'isolation' => $pattern['isolation'], 'propagation' => $pattern['propagation'], 'noRollbackFor' => $pattern['noRollbackFor'], 'methods' => $pattern['methods'], - ); - } + ); } } diff --git a/Transactions/ScopeHandler.php b/Transactions/ScopeHandler.php new file mode 100644 index 0000000..472eacc --- /dev/null +++ b/Transactions/ScopeHandler.php @@ -0,0 +1,61 @@ +container = $container; + } + + public function enterScope() + { + $this->levels[] = $this->currentLevel; + $this->currentLevel = 0; + $this->container->enterScope('transactional'); + } + + public function leaveScope() + { + if ($this->currentLevel > 0) { + throw new TransactionException("Cannot leave transaction scope that still has levels."); + } + $this->currentLevel = array_pop($this->levels); + $this->container->leaveScope('transactional'); + } + + public function increaseNestingLevel() + { + $this->currentLevel++; + } + + public function decreaseNestingLevel() + { + $this->currentLevel--; + } + + public function getNestingLevel() + { + return $this->level; + } +} + diff --git a/Transactions/TransactionsRegistry.php b/Transactions/TransactionsRegistry.php index d835a55..50061fc 100644 --- a/Transactions/TransactionsRegistry.php +++ b/Transactions/TransactionsRegistry.php @@ -14,6 +14,7 @@ namespace SimpleThings\TransactionalBundle\Transactions; use Symfony\Component\DependencyInjection\ContainerInterface; +use SimpleThings\TransactionalBundle\TransactionException; /** * Registry for mass-operations on transactions. @@ -26,17 +27,25 @@ class TransactionsRegistry { private $container; private $connectionServices; - private $connections = array(); public function __construct(ContainerInterface $container, $connectionServices = array()) { $this->container = $container; + $this->connectionServices = $connectionServices; } public function getTransactions(array $definitions) { $txManagers = array(); + $requiresNew = false; foreach ($definitions as $def) { + if ($def->getPropagation() == TransactionDefinition::PROPAGATION_REQUIRES_NEW) { + if ($requiresNew) { + throw new TransactionException("Cannot have more than one connection to require a new transaction per request."); + } + $requiresNew = true; + } + $managerName = $def->getManagerName(); if ($txStatus = $this->getTransactionManager($managerName)->getTransaction($def)) { $txManagers[$managerName] = $txStatus; @@ -47,14 +56,14 @@ public function getTransactions(array $definitions) public function commit(array $statuses) { - foreach ($txManagers AS $managerName => $txStatus) { + foreach (array_reverse($txManagers) AS $managerName => $txStatus) { $this->getTransactionManager($managerName)->commit($txStatus); } } public function rollBack(array $statuses) { - foreach ($txManagers AS $managerName => $txStatus) { + foreach (array_reverse($txManagers) AS $managerName => $txStatus) { $this->getTransactionManager($managerName)->rollBack($txStatus); } } From 7f8e51ff3014923214cdfd5496535f75d68ffe65 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Sun, 8 Jan 2012 22:56:33 +0100 Subject: [PATCH 11/24] Add Functional Test, work on configuration, add OrmTransactionManager. --- .../CompilerPass/DetectConnectionPass.php | 20 +++- .../SimpleThingsTransactionalExtension.php | 37 ------ Doctrine/ObjectTransactionManager.php | 22 +--- Doctrine/ObjectTransactionStatus.php | 2 + Doctrine/OrmTransactionManager.php | 47 ++++++++ Doctrine/OrmTransactionStatus.php | 108 +++++++++++++++++ Resources/config/services.xml | 10 +- Tests/Functional/TransactionalKernelTest.php | 111 ++++++++++++++++++ Transactions/AbstractTransactionManager.php | 20 ++++ Transactions/Form/RollbackInvalidForm.php | 49 -------- .../Http/HttpTransactionsListener.php | 1 + Transactions/Http/TransactionalMatcher.php | 22 ++-- Transactions/TransactionsRegistry.php | 4 +- 13 files changed, 325 insertions(+), 128 deletions(-) create mode 100644 Doctrine/OrmTransactionManager.php create mode 100644 Doctrine/OrmTransactionStatus.php create mode 100644 Tests/Functional/TransactionalKernelTest.php delete mode 100644 Transactions/Form/RollbackInvalidForm.php diff --git a/DependencyInjection/CompilerPass/DetectConnectionPass.php b/DependencyInjection/CompilerPass/DetectConnectionPass.php index 7ea4d90..3223e1f 100644 --- a/DependencyInjection/CompilerPass/DetectConnectionPass.php +++ b/DependencyInjection/CompilerPass/DetectConnectionPass.php @@ -20,7 +20,6 @@ /** * Detects connections and registers the transaction manager services. - * */ class DetectConnectionsPass implements CompilerPassInterface { @@ -34,34 +33,45 @@ public function process(ContainerBuilder $builder) )->setArguments(array(new Reference($service))); } } +*/ + $connectionServices = array(); if ($builder->hasParameter('doctrine.entity_managers')) { foreach ($builder->getParameter('doctrine.entity_managers') AS $alias => $service) { + $connectionServices[] = 'doctrine.orm.' . $alias . '_entity_manager'; + $builder->setAlias( + 'simple_things_transactional.connections.orm.' . $alias, + 'doctrine.orm.' . $alias . '_entity_manager' + ); $builder->setDefinition( 'simple_things_transactional.tx.orm.'.$alias, new DefinitionDecorator('simple_things_transactional.manager.orm') - )->setArguments(array(new Reference('doctrine'), $alias)); + )->setArguments(array(new Reference('service_container'))); } } if ($builder->hasParameter('doctrine_couchdb.document_managers')) { foreach ($builder->getParameter('doctrine_couchdb.document_managers') AS $alias => $service) { + $connectionServices[] = 'doctrine_couchdb.odm.' . $alias . '_document_manager'; + $builder->setAlias('simple_things_transactional.connections.couchdb.' . $alias $builder->setDefinition( 'simple_things_transactional.tx.couchdb.'.$alias, new DefinitionDecorator('simple_things_transactional.manager.object_manager') - )->setArguments(array(new Reference($service))); + )->setArguments(array(new Reference('service_container'))); } } if ($builder->hasParameter('doctrine_mongodb.document_managers')) { foreach ($builder->getParameter('doctrine_mongodb.document_managers') AS $alias => $service) { + $connectionServices[] = 'doctrine_mongodb.odm.' . $alias . '_document_manager'; + $builder->setAlias('simple_things_transactional.connections.mongodb.' . $alias $builder->setDefinition( 'simple_things_transactional.tx.mongodb.'.$alias, new DefinitionDecorator('simple_things_transactional.manager.object_manager') - )->setArguments(array(new Reference($service))); + )->setArguments(array(new Reference('service_container'))); } } - */ // add tags as well for external resources (Propel, raw-PDO whatever) + $container->setParameter('simple_things_transactional.connection_services', $connectionServices); } } diff --git a/DependencyInjection/SimpleThingsTransactionalExtension.php b/DependencyInjection/SimpleThingsTransactionalExtension.php index 0838970..588e818 100644 --- a/DependencyInjection/SimpleThingsTransactionalExtension.php +++ b/DependencyInjection/SimpleThingsTransactionalExtension.php @@ -18,8 +18,6 @@ use Symfony\Component\DependencyInjection\Loader\XmlFileLoader; use Symfony\Component\HttpKernel\DependencyInjection\Extension; use Symfony\Component\Config\FileLocator; -use Symfony\Component\DependencyInjection\Reference; -use Symfony\Component\DependencyInjection\DefinitionDecorator; use SimpleThings\TransactionalBundle\Transactions\TransactionDefinition; class SimpleThingsTransactionalExtension extends Extension @@ -74,40 +72,5 @@ public function load(array $configs, ContainerBuilder $builder) $def = $builder->getDefinition('simple_things_transactional.transactional_matcher'); $def->setArguments($args); - if ($builder->hasParameter('doctrine.connections')) { - foreach ($builder->getParameter('doctrine.connections') AS $alias => $service) { - $builder->setDefinition( - 'simple_things_transactional.tx.dbal.'.$alias, - new DefinitionDecorator('simple_things_transactional.manager.dbal') - )->setArguments(array(new Reference($service))); - } - } - - if ($builder->hasParameter('doctrine.entity_managers')) { - foreach ($builder->getParameter('doctrine.entity_managers') AS $alias => $service) { - $builder->setDefinition( - 'simple_things_transactional.tx.orm.'.$alias, - new DefinitionDecorator('simple_things_transactional.manager.orm') - )->setArguments(array(new Reference('doctrine'), $alias)); - } - } - - if ($builder->hasParameter('doctrine_couchdb.document_managers')) { - foreach ($builder->getParameter('doctrine_couchdb.document_managers') AS $alias => $service) { - $builder->setDefinition( - 'simple_things_transactional.tx.couchdb.'.$alias, - new DefinitionDecorator('simple_things_transactional.manager.couchdb') - )->setArguments(array(new Reference($service))); - } - } - - if ($builder->hasParameter('doctrine_mongodb.document_managers')) { - foreach ($builder->getParameter('doctrine_mongodb.document_managers') AS $alias => $service) { - $builder->setDefinition( - 'simple_things_transactional.tx.mongodb.'.$alias, - new DefinitionDecorator('simple_things_transactional.manager.mongodb') - )->setArguments(array(new Reference($service))); - } - } } } diff --git a/Doctrine/ObjectTransactionManager.php b/Doctrine/ObjectTransactionManager.php index 0056498..5ce4c40 100644 --- a/Doctrine/ObjectTransactionManager.php +++ b/Doctrine/ObjectTransactionManager.php @@ -26,40 +26,26 @@ */ class ObjectTransactionManager extends AbstractTransactionManager { - private $registry; - private $managers; + private $container; - public function __construct(TransactionsRegistry $registry) + public function __construct($container) { - $this->registry = $registry; + $this->container = $container; } protected function doBeginTransaction(TransactionDefinition $def) { - $parts = split(".", $def->getManagerName()); - $name = end($parts); - - $manager = $this->registry->getManager($name); - // this could be an already instantiated manager, check and reset if - // necessary. - if ($this->managers->contains($manager)) { - $this->registry->resetManager($name); - return $this->doBeginTransaction($def); - } - $this->managers->add($manager); - + $manager = $container->get('simple_things_transactional.connections.' . $def->getManagerName()); return $this->createTxStatus($manager, $def); } protected function doCommit(TransactionStatus $def) { - $this->managers->detach($manager); $def->commit(); } protected function doRollBack(TransactionStatus $def) { - $this->managers->detach($manager); $def->rollBack(); } diff --git a/Doctrine/ObjectTransactionStatus.php b/Doctrine/ObjectTransactionStatus.php index 12797dc..1a13df3 100644 --- a/Doctrine/ObjectTransactionStatus.php +++ b/Doctrine/ObjectTransactionStatus.php @@ -14,6 +14,8 @@ namespace SimpleThingsTransactionalBundle\Doctrine; use SimpleThings\TransactionalBundle\Transactions\TransactionStatus; +use SimpleThings\TransactionalBundle\Transactions\TransactionDefinition; +use Doctrine\Common\Persistence\ObjectManager; class ObjectTransactionStatus implements TransactionStatus { diff --git a/Doctrine/OrmTransactionManager.php b/Doctrine/OrmTransactionManager.php new file mode 100644 index 0000000..f4e2f1e --- /dev/null +++ b/Doctrine/OrmTransactionManager.php @@ -0,0 +1,47 @@ +container = $container; + } + + protected function doBeginTransaction(TransactionDefinition $def) + { + $manager = $container->get('simple_things_transactional.connections.' . $def->getManagerName()); + $manager->beginTransaction(); + return $this->createTxStatus($manager, $def); + } + + protected function doCommit(TransactionStatus $def) + { + $def->commit(); + } + + protected function doRollBack(TransactionStatus $def) + { + $def->rollBack(); + } + + protected function createTxStatus($manager, $def) + { + return new OrmTransactionStatus($manager, $def); + } +} + diff --git a/Doctrine/OrmTransactionStatus.php b/Doctrine/OrmTransactionStatus.php new file mode 100644 index 0000000..6cfd446 --- /dev/null +++ b/Doctrine/OrmTransactionStatus.php @@ -0,0 +1,108 @@ +manager = $manager; + $this->def = $def; + } + + /** + * Checks if the transaction is read-only. + * + * A read-only transaction does not commit changes to the database when + * commit is called. It allows the underlying transaction manager to + * perform optimizations to this regard if possible. + * + * @return bool + */ + public function isReadOnly() + { + return $this->def->getReadOnly(); + } + + /** + * Check if transaction is broken and has to be rolled back at this point. + * + * @return bool + */ + public function isRollBackOnly() + { + return $this->manager->getConnection()->isRollbackOnly(); + } + + /** + * Mark the transaction as rollback-only + * + * @return void + */ + public function setRollBackOnly() + { + $this->manager->getConnection()->setRollbackOnly(true); + } + + /** + * Check if this transaction was committed already. + * + * @return bool + */ + public function isCompleted() + { + return $this->completed; + } + + /** + * Check if this transaction has savepoints. + * + * @return bool + */ + public function hasSavepoint() + { + return false; + } + + /** + * Commit the transaction at this point. + * + * @return void + */ + public function commit() + { + $this>completed = true; + $this->manager->flush(); + $this->manager->commit(); + } + + /** + * Rollback the transaction at this point marking it as complete. + * + * @return void + */ + public function rollBack() + { + $this->completed = true; + $this->manager->rollBack(); + $this->manager->clear(); + } +} + diff --git a/Resources/config/services.xml b/Resources/config/services.xml index a687463..eacc7e9 100644 --- a/Resources/config/services.xml +++ b/Resources/config/services.xml @@ -6,20 +6,18 @@ SimpleThings\TransactionalBundle\Transactions\Doctrine\DBALTransactionManager - SimpleThings\TransactionalBundle\Transactions\Doctrine\EntityManagerTransactionManager - SimpleThings\TransactionalBundle\Transactions\Doctrine\MongoDBTransactionManager - SimpleThings\TransactionalBundle\Transactions\Doctrine\CouchDBTransactionManager + SimpleThings\TransactionalBundle\Transactions\Doctrine\EntityTransactionManager + SimpleThings\TransactionalBundle\Transactions\Doctrine\ObjectTransactionManager SimpleThings\TransactionalBundle\Transactions\Http\HttpTransactionsListener SimpleThings\TransactionalBundle\Transactions\Http\TransactionalMatcher SimpleThings\TransactionalBundle\Transactions\TransactionsRegistry - + - - + diff --git a/Tests/Functional/TransactionalKernelTest.php b/Tests/Functional/TransactionalKernelTest.php new file mode 100644 index 0000000..9b17014 --- /dev/null +++ b/Tests/Functional/TransactionalKernelTest.php @@ -0,0 +1,111 @@ +getMock('Symfony\Component\HttpKernel\Controller\ControllerResolverInterface'); + $resolver->expects($this->once())->method('getController')->will($this->returnValue(array(new TestController, 'indexAction'))); + $resolver->expects($this->once())->method('getArguments')->will($this->returnValue(array())); + + $txStatus1 = $this->getMock('SimpleThings\TransactionalBundle\Transactions\TransactionStatus'); + $manager = $this->getMock( + 'SimpleThings\TransactionalBundle\Transactions\AbstractTransactionManager', + array('doBeginTransaction', 'doCommit', 'doRollBack'), + array($this->getMock('SimpleThings\TransactionalBundle\Transactions\ScopeHandler', array(), array(), '', false)) + ); + $manager->expects($this->at(0))->method('doBeginTransaction')->will($this->returnValue($txStatus1)); + $manager->expects($this->at(1))->method('doCommit'); + + $this->logger = new StackLogger; + $container = $this->getMock('Symfony\Component\DependencyInjection\ContainerInterface'); + $container->expects($this->any())->method('has')->with($this->equalTo('simple_things_transactional.tx.dbal.default'))->will($this->returnValue(true)); + $container->expects($this->any())->method('get')->with($this->equalTo('simple_things_transactional.tx.dbal.default'))->will($this->returnValue($manager)); + + $registry = new TransactionsRegistry($container); + $matcher = new TransactionalMatcher(array(), array( + 'conn' => 'dbal.default', + 'methods' => array('POST'), + 'propagation' => TransactionDefinition::PROPAGATION_REQUIRED, + 'isolation' => TransactionDefinition::ISOLATION_DEFAULT, + )); + $txListener = new HttpTransactionsListener($registry, $matcher, $this->logger); + $dispatcher = new EventDispatcher(); + $dispatcher->addListener("kernel.controller", array($txListener, 'onCoreController')); + $dispatcher->addListener("kernel.exception", array($txListener, 'onKernelException')); + $dispatcher->addListener("kernel.response", array($txListener, 'onKernelResponse')); + $this->kernel = new HttpKernel($dispatcher, $resolver); + } + + public function testGetRequest() + { + $request = Request::create('/foo', 'GET'); + $this->kernel->handle($request); + + $this->assertEquals(array( + "[TransactionBundle] Started transactions for dbal.default", + "[TransactionBundle] Committed transactions for dbal.default" + ), $this->logger->logs); + } + + public function testPostRequest() + { + $request = Request::create('/foo', 'POST'); + $this->kernel->handle($request); + + $this->assertEquals(array( + "[TransactionBundle] Started transactions for dbal.default", + "[TransactionBundle] Committed transactions for dbal.default" + ), $this->logger->logs); + } +} + +class StackLogger extends NullLogger +{ + public $logs = array(); + + public function info($message, array $contexts = array()) + { + $this->logs[] = $message; + } +} + +class TestController +{ + public function indexAction() + { + return new Response('data', 200); + } + + public function subAction() + { + return new Response('data', 200); + } +} + diff --git a/Transactions/AbstractTransactionManager.php b/Transactions/AbstractTransactionManager.php index 0f25b6e..1222dc7 100644 --- a/Transactions/AbstractTransactionManager.php +++ b/Transactions/AbstractTransactionManager.php @@ -62,6 +62,7 @@ public function getTransaction(TransactionDefinition $def) } return null; case TransactionDefinition::PROPAGATION_REQUIRED: + $this->scope->increaseNestingLevel(); $openTransactionDef = $this->getCurrentTransactionDef(); $status = $this->getCurrentTransaction(); if ($openTransactionDef) { @@ -80,6 +81,7 @@ public function getTransaction(TransactionDefinition $def) break; case TransactionDefinition::PROPAGATION_SUPPORTS: default: + $this->scope->increaseNestingLevel(); $status = $this->getCurrentTransaction(); break; } @@ -98,7 +100,16 @@ public function commit(TransactionStatus $status) throw new TransactionException("Cannot commit a detached transaction. It may have been committed before or belongs to another transaction manager"); } + $this->scope->decreaseNestingLevel(); + if ($this->scope->getNestingLevel() > 0) { + return; + } + $this->cleanupAfterTransaction($status); + if ($status->isReadOnly()) { + return; + } + $this->doCommit($status); } @@ -110,7 +121,16 @@ public function rollBack(TransactionStatus $status) throw new TransactionException("Cannot rollback a detached transaction. It may have been committed/rollbacked before or belongs to another transaction manager"); } + $this->scope->decreaseNestingLevel(); + if ($this->scope->getNestingLevel() > 0) { + return; + } + $this->cleanupAfterTransaction($status); + if ($status->isReadOnly()) { + return; + } + $this->doRollBack($status); } diff --git a/Transactions/Form/RollbackInvalidForm.php b/Transactions/Form/RollbackInvalidForm.php deleted file mode 100644 index 8a8e393..0000000 --- a/Transactions/Form/RollbackInvalidForm.php +++ /dev/null @@ -1,49 +0,0 @@ - - */ -class RollbackInvalidFormValidator implements FormValidatorInterface -{ - private $container; - - public function __construct($container) - { - $this->container = $container; - } - - public function validate(FormInterface $form) - { - if (!$this->container->has('request')) { - return; - } - - $request = $this->container->get('request'); - if (!$form->isValid()) { - foreach ($request->attributes->get('_transactions') as $tx) { - $tx->setRollbackOnly(); - } - } - } -} - diff --git a/Transactions/Http/HttpTransactionsListener.php b/Transactions/Http/HttpTransactionsListener.php index 6cf5c16..3795078 100644 --- a/Transactions/Http/HttpTransactionsListener.php +++ b/Transactions/Http/HttpTransactionsListener.php @@ -79,6 +79,7 @@ public function onKernelException(GetResponseForExceptionEvent $event) { $request = $event->getRequest(); $ex = $event->getException(); + var_dump($ex->getMessage()); if (!$request->attributes->has('_transactions')) { return; diff --git a/Transactions/Http/TransactionalMatcher.php b/Transactions/Http/TransactionalMatcher.php index ccd0bd4..915bf02 100644 --- a/Transactions/Http/TransactionalMatcher.php +++ b/Transactions/Http/TransactionalMatcher.php @@ -79,20 +79,20 @@ public function match($method, $controllerCallback) $this->cache[$subject] = array(); $this->matchPatterns($subject); $this->matchAnnotations($subject, $controller, $action); + + if (!$this->cache[$subject] && $this->defaults) { + $this->cache[$subject][$this->defaults['conn']] = $this->defaults; + } } $definitions = array(); $requireNew = false; foreach ($this->cache[$subject] as $connectionName => $definition) { - if ($definition['propagation'] == TransactionDefinition::PROPAGATION_REQUIRES_NEW) { - - } - $definitions[] = new TransactionDefinition( - $definition['managerName'], + $definition['conn'], $definition['propagation'], $definition['isolation'], - ! in_array($method, $definition['methods']) + ! in_array($method, (array)$definition['methods']) ); } @@ -141,13 +141,13 @@ private function matchAnnotations($subject, $controller, $action) private function storeMatch($subject, $pattern) { - $managerName = $pattern['conn']; - if (isset($this->cache[$subject][$managerName])) { - throw TransactionException::duplicateConnectionMatch($managerName, $pattern); + $conn = $pattern['conn']; + if (isset($this->cache[$subject][$conn])) { + throw TransactionException::duplicateConnectionMatch($conn, $pattern); } - $this->cache[$subject][$managerName] = array( - 'managerName' => $managerName, + $this->cache[$subject][$conn] = array( + 'conn' => $conn, 'isolation' => $pattern['isolation'], 'propagation' => $pattern['propagation'], 'noRollbackFor' => $pattern['noRollbackFor'], diff --git a/Transactions/TransactionsRegistry.php b/Transactions/TransactionsRegistry.php index 50061fc..254924e 100644 --- a/Transactions/TransactionsRegistry.php +++ b/Transactions/TransactionsRegistry.php @@ -56,14 +56,14 @@ public function getTransactions(array $definitions) public function commit(array $statuses) { - foreach (array_reverse($txManagers) AS $managerName => $txStatus) { + foreach (array_reverse($statuses) AS $managerName => $txStatus) { $this->getTransactionManager($managerName)->commit($txStatus); } } public function rollBack(array $statuses) { - foreach (array_reverse($txManagers) AS $managerName => $txStatus) { + foreach (array_reverse($statuses) AS $managerName => $txStatus) { $this->getTransactionManager($managerName)->rollBack($txStatus); } } From b7a05ac90b2a6627b796042df4ce40be391004e9 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Sun, 8 Jan 2012 23:04:18 +0100 Subject: [PATCH 12/24] Typo --- Transactions/ScopeHandler.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Transactions/ScopeHandler.php b/Transactions/ScopeHandler.php index 472eacc..3888fe2 100644 --- a/Transactions/ScopeHandler.php +++ b/Transactions/ScopeHandler.php @@ -55,7 +55,7 @@ public function decreaseNestingLevel() public function getNestingLevel() { - return $this->level; + return $this->currentLevel; } } From b11b03f6be2d6a1a4ad9ec08b29b720db8965e9d Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Sat, 21 Jan 2012 19:48:35 +0100 Subject: [PATCH 13/24] Simplified transactions code by allowing only one manager per request. Renamed a bunch of TransactionDefinition constants and general cleanup. --- .../SimpleThingsTransactionalExtension.php | 2 +- Doctrine/DBALTransactionManager.php | 65 ++++++++ ...onStatus.php => DBALTransactionStatus.php} | 41 ++--- Doctrine/ObjectTransactionManager.php | 22 +-- Doctrine/ObjectTransactionStatus.php | 23 +-- Doctrine/OrmTransactionManager.php | 29 ++-- LICENSE | 9 ++ .../HttpTransactionsListenerTest.php | 14 -- Tests/Functional/EndToEndTest.php | 147 ++++++++++++++++++ Tests/Functional/TransactionalKernelTest.php | 10 +- .../AbstractTransactionManagerTest.php | 34 ++-- .../Http/HttpTransactionsListenerTest.php | 6 +- .../Http/TransactionalMatcherTest.php | 53 ++----- Transactions/AbstractTransactionManager.php | 69 ++++---- .../Http/HttpTransactionsListener.php | 52 ++++--- Transactions/Http/TransactionalMatcher.php | 37 ++--- Transactions/ScopeHandler.php | 8 +- Transactions/TransactionDefinition.php | 25 +-- Transactions/TransactionStatus.php | 13 +- Transactions/TransactionsRegistry.php | 39 ++--- composer.json | 3 +- 21 files changed, 427 insertions(+), 274 deletions(-) create mode 100644 Doctrine/DBALTransactionManager.php rename Doctrine/{OrmTransactionStatus.php => DBALTransactionStatus.php} (67%) create mode 100644 LICENSE delete mode 100644 Tests/Controller/HttpTransactionsListenerTest.php create mode 100644 Tests/Functional/EndToEndTest.php diff --git a/DependencyInjection/SimpleThingsTransactionalExtension.php b/DependencyInjection/SimpleThingsTransactionalExtension.php index 588e818..616ddb1 100644 --- a/DependencyInjection/SimpleThingsTransactionalExtension.php +++ b/DependencyInjection/SimpleThingsTransactionalExtension.php @@ -46,7 +46,7 @@ public function load(array $configs, ContainerBuilder $builder) $config['defaults'] = array_merge(array( 'conn' => array(), 'pattern' => '.*', - 'propagation' => TransactionDefinition::PROPAGATION_REQUIRED, + 'propagation' => TransactionDefinition::PROPAGATION_JOINED, 'isolation' => TransactionDefinition::ISOLATION_DEFAULT, 'noRollbackFor' => array(), 'methods' => array('POST', 'PUT', 'DELETE', 'PATCH'), diff --git a/Doctrine/DBALTransactionManager.php b/Doctrine/DBALTransactionManager.php new file mode 100644 index 0000000..c1cceea --- /dev/null +++ b/Doctrine/DBALTransactionManager.php @@ -0,0 +1,65 @@ +container = $container; + parent::__construct($scopeHandler); + } + + protected function createTxStatus($conn, $def) + { + return new DBALTransactionStatus($conn, $def); + } + + protected function doBeginTransaction(TransactionDefinition $def) + { + $conn = $this->container->get('simple_things_transactional.connections.' . $def->getManagerName()); + $conn->beginTransaction(); + return $this->createTxStatus($conn, $def); + } + + protected function doCommit(TransactionStatus $status) + { + $conn = $status->getWrappedConnection(); + $conn->commit(); + $status->markCompleted(); + } + + protected function doRollBack(TransactionStatus $status) + { + $conn = $status->getWrappedConnection(); + $conn->rollback(); + $status->markCompleted(); + } +} + diff --git a/Doctrine/OrmTransactionStatus.php b/Doctrine/DBALTransactionStatus.php similarity index 67% rename from Doctrine/OrmTransactionStatus.php rename to Doctrine/DBALTransactionStatus.php index 6cfd446..666a838 100644 --- a/Doctrine/OrmTransactionStatus.php +++ b/Doctrine/DBALTransactionStatus.php @@ -1,6 +1,6 @@ manager = $manager; + $this->conn = $conn; $this->def = $def; } + public function getIsolationLevel() + { + return $this->def->getIsolationLevel(); + } + /** * Checks if the transaction is read-only. * @@ -48,7 +55,7 @@ public function isReadOnly() */ public function isRollBackOnly() { - return $this->manager->getConnection()->isRollbackOnly(); + $this->conn->isRollBackOnly(); } /** @@ -58,7 +65,7 @@ public function isRollBackOnly() */ public function setRollBackOnly() { - $this->manager->getConnection()->setRollbackOnly(true); + $this->conn->setRollBackOnly(true); } /** @@ -81,28 +88,14 @@ public function hasSavepoint() return false; } - /** - * Commit the transaction at this point. - * - * @return void - */ - public function commit() + public function getWrappedConnection() { - $this>completed = true; - $this->manager->flush(); - $this->manager->commit(); + return $this->conn; } - /** - * Rollback the transaction at this point marking it as complete. - * - * @return void - */ - public function rollBack() + public function markCompleted() { $this->completed = true; - $this->manager->rollBack(); - $this->manager->clear(); } } diff --git a/Doctrine/ObjectTransactionManager.php b/Doctrine/ObjectTransactionManager.php index 5ce4c40..3eaa10b 100644 --- a/Doctrine/ObjectTransactionManager.php +++ b/Doctrine/ObjectTransactionManager.php @@ -26,32 +26,34 @@ */ class ObjectTransactionManager extends AbstractTransactionManager { - private $container; + protected $container; public function __construct($container) { $this->container = $container; } - protected function doBeginTransaction(TransactionDefinition $def) + protected function createTxStatus($manager, $def) { - $manager = $container->get('simple_things_transactional.connections.' . $def->getManagerName()); - return $this->createTxStatus($manager, $def); + return new ObjectTransactionStatus($manager, $def); } - protected function doCommit(TransactionStatus $def) + protected function doBeginTransaction(TransactionDefinition $def) { - $def->commit(); + $manager = $this->container->get('simple_things_transactional.connections.' . $def->getManagerName()); + return $this->createTxStatus($manager, $def); } - protected function doRollBack(TransactionStatus $def) + protected function doCommit(TransactionStatus $status) { - $def->rollBack(); + $manager = $status->getWrappedConnection(); + $manager->flush(); + $status->markCompleted(); } - protected function createTxStatus($manager, $def) + protected function doRollBack(TransactionStatus $status) { - return new ObjectTransactionStatus($manager, $def); + $status->markCompleted(); } } diff --git a/Doctrine/ObjectTransactionStatus.php b/Doctrine/ObjectTransactionStatus.php index 1a13df3..6bb3e13 100644 --- a/Doctrine/ObjectTransactionStatus.php +++ b/Doctrine/ObjectTransactionStatus.php @@ -30,6 +30,11 @@ public function __construct(ObjectManager $manager, TransactionDefinition $def) $this->def = $def; } + public function getIsolationLevel() + { + return $this->def->getIsolationLevel(); + } + /** * Checks if the transaction is read-only. * @@ -84,26 +89,14 @@ public function hasSavepoint() return false; } - /** - * Commit the transaction at this point. - * - * @return void - */ - public function commit() + public function getWrappedConnection() { - $this>completed = true; - $this->manager->flush(); + return $this->manager; } - /** - * Rollback the transaction at this point marking it as complete. - * - * @return void - */ - public function rollBack() + public function markCompleted() { $this->completed = true; - $this->manager->clear(); } } diff --git a/Doctrine/OrmTransactionManager.php b/Doctrine/OrmTransactionManager.php index f4e2f1e..c6009c3 100644 --- a/Doctrine/OrmTransactionManager.php +++ b/Doctrine/OrmTransactionManager.php @@ -13,35 +13,30 @@ namespace SimpleThings\TransactionalBundle\Doctrine; -class OrmTransactionManager extends AbstractTransactionManager +class OrmTransactionManager extends ObjectTransactionManager { - private $container; - - public function __construct($container) - { - $this->container = $container; - } - protected function doBeginTransaction(TransactionDefinition $def) { - $manager = $container->get('simple_things_transactional.connections.' . $def->getManagerName()); + $manager = $this->container->get('simple_things_transactional.connections.' . $def->getManagerName()); $manager->beginTransaction(); return $this->createTxStatus($manager, $def); } - protected function doCommit(TransactionStatus $def) + protected function doCommit(TransactionStatus $status) { - $def->commit(); - } + $manager = $status->getWrappedConnection(); + $manager->flush(); + $manager->commit(); - protected function doRollBack(TransactionStatus $def) - { - $def->rollBack(); + $status->markCompleted(); } - protected function createTxStatus($manager, $def) + protected function doRollBack(TransactionStatus $status) { - return new OrmTransactionStatus($manager, $def); + $manager = $status->getWrappedConnection(); + $manager->rollBack(); + + $status->markCompleted(); } } diff --git a/LICENSE b/LICENSE new file mode 100644 index 0000000..ce654c1 --- /dev/null +++ b/LICENSE @@ -0,0 +1,9 @@ +Copyright (c) 2012, Benjamin Eberlei +All rights reserved. + +Redistribution and use in source and binary forms, with or without modification, are permitted provided that the following conditions are met: + +* Redistributions of source code must retain the above copyright notice, this list of conditions and the following disclaimer. +* Redistributions in binary form must reproduce the above copyright notice, this list of conditions and the following disclaimer in the documentation and/or other materials provided with the distribution. + +THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. diff --git a/Tests/Controller/HttpTransactionsListenerTest.php b/Tests/Controller/HttpTransactionsListenerTest.php deleted file mode 100644 index aa47da3..0000000 --- a/Tests/Controller/HttpTransactionsListenerTest.php +++ /dev/null @@ -1,14 +0,0 @@ -conn = DriverManager::getConnection(array('driver' => 'pdo_sqlite', 'memory' => true)); + $table = new \Doctrine\DBAL\Schema\Table("testdata"); + $table->addColumn('id', 'integer', array('auto_increment' => true)); + $table->addColumn('val', 'string'); + $table->setPrimaryKey(array('id')); + + $conn->getSchemaManager()->createTable($table); + + $this->logger = new FunctionalStackLogger; + + $container = new ContainerBuilder(); + $container->addScope(new Scope('transactional')); + $container->addScope(new Scope('request')); + + $container->set('doctrine.dbal.default_connection', $conn); + $container->set('simple_things_transactional.connections.dbal.default', $conn); + $scope = new ScopeHandler($container); + + $txManager = new DBALTransactionManager($container, $scope); + $container->set('simple_things_transactional.tx.dbal.default', $txManager); + + $resolver = $this->getMock('Symfony\Component\HttpKernel\Controller\ControllerResolverInterface'); + $resolver->expects($this->at(0))->method('getController')->will($this->returnValue(array(new DBALTestController($container), 'firstAction'))); + $resolver->expects($this->at(2))->method('getController')->will($this->returnValue(array(new DBALTestController($container), 'secondAction'))); + $resolver->expects($this->any())->method('getArguments')->will($this->returnValue(array())); + + $registry = new TransactionsRegistry($container); + $matcher = new TransactionalMatcher(array(), array( + 'conn' => 'dbal.default', + 'methods' => array('POST'), + 'propagation' => TransactionDefinition::PROPAGATION_ISOLATED, + 'isolation' => TransactionDefinition::ISOLATION_DEFAULT, + )); + + $txListener = new HttpTransactionsListener($registry, $matcher, $this->logger); + $dispatcher = new EventDispatcher(); + $dispatcher->addListener("kernel.controller", array($txListener, 'onCoreController')); + $dispatcher->addListener("kernel.exception", array($txListener, 'onKernelException')); + $dispatcher->addListener("kernel.response", array($txListener, 'onKernelResponse')); + $this->kernel = new HttpKernel($dispatcher, $container, $resolver); + $container->set('http_kernel', $this->kernel); + } + + public function testGetRequest() + { + $request = Request::create('/foo', 'GET'); + $this->kernel->handle($request); + + $this->assertEquals(array( + "[TransactionBundle] Started transaction for dbal.default", + "[TransactionBundle] Committed transaction for dbal.default" + ), $this->logger->logs); + + var_dump($this->conn->fetchAll("SELECT * FROM testdata")); + } + + public function testPostRequest() + { + $request = Request::create('/foo', 'POST'); + $this->kernel->handle($request); + + $this->assertEquals(array( + "[TransactionBundle] Started transaction for dbal.default", + "[TransactionBundle] Committed transaction for dbal.default" + ), $this->logger->logs); + var_dump($this->conn->fetchAll("SELECT * FROM testdata")); + } +} + +class FunctionalStackLogger extends NullLogger +{ + public $logs = array(); + + public function info($message, array $contexts = array()) + { + $this->logs[] = $message; + } +} + +class DBALTestController +{ + private $container; + + public function __construct($container) + { + $this->container = $container; + } + + public function firstAction() + { + $conn = $this->container->get('doctrine.dbal.default_connection'); + $conn->insert("testdata", array("val" => "foo")); + + try { + $this->container->get('http_kernel')->forward('DBALTestController:secondAxtion'); + } catch(\Exception $e) {} + + return new Response('data', 200); + } + + public function secondAction() + { + $conn = $this->container->get('doctrine.dbal.default_connection'); + $conn->insert("testdata", array("val" => "foo")); + + throw new \InvalidArgumentException("blablabla!"); + } +} + diff --git a/Tests/Functional/TransactionalKernelTest.php b/Tests/Functional/TransactionalKernelTest.php index 9b17014..6fb7fef 100644 --- a/Tests/Functional/TransactionalKernelTest.php +++ b/Tests/Functional/TransactionalKernelTest.php @@ -52,7 +52,7 @@ public function setUp() $matcher = new TransactionalMatcher(array(), array( 'conn' => 'dbal.default', 'methods' => array('POST'), - 'propagation' => TransactionDefinition::PROPAGATION_REQUIRED, + 'propagation' => TransactionDefinition::PROPAGATION_JOINED, 'isolation' => TransactionDefinition::ISOLATION_DEFAULT, )); $txListener = new HttpTransactionsListener($registry, $matcher, $this->logger); @@ -69,8 +69,8 @@ public function testGetRequest() $this->kernel->handle($request); $this->assertEquals(array( - "[TransactionBundle] Started transactions for dbal.default", - "[TransactionBundle] Committed transactions for dbal.default" + "[TransactionBundle] Started transaction for dbal.default", + "[TransactionBundle] Committed transaction for dbal.default" ), $this->logger->logs); } @@ -80,8 +80,8 @@ public function testPostRequest() $this->kernel->handle($request); $this->assertEquals(array( - "[TransactionBundle] Started transactions for dbal.default", - "[TransactionBundle] Committed transactions for dbal.default" + "[TransactionBundle] Started transaction for dbal.default", + "[TransactionBundle] Committed transaction for dbal.default" ), $this->logger->logs); } } diff --git a/Tests/Transactions/AbstractTransactionManagerTest.php b/Tests/Transactions/AbstractTransactionManagerTest.php index beb856a..9a05077 100644 --- a/Tests/Transactions/AbstractTransactionManagerTest.php +++ b/Tests/Transactions/AbstractTransactionManagerTest.php @@ -37,7 +37,7 @@ public function testGetTransaction() { $txStatus = $this->getTxStatusMock(); $this->manager->expects($this->once())->method('doBeginTransaction')->will($this->returnValue($txStatus)); - $def = $this->createDefinition(TransactionDefinition::PROPAGATION_REQUIRED, TransactionDefinition::ISOLATION_DEFAULT); + $def = $this->createDefinition(TransactionDefinition::PROPAGATION_JOINED, TransactionDefinition::ISOLATION_DEFAULT); $actualStatus = $this->manager->getTransaction($def); $this->assertSame($txStatus, $actualStatus); @@ -45,7 +45,7 @@ public function testGetTransaction() public function testGetNeverTransaction() { - $def = $this->createDefinition(TransactionDefinition::PROPAGATION_NEVER, TransactionDefinition::ISOLATION_DEFAULT); + $def = $this->createDefinition(TransactionDefinition::PROPAGATION_MANUAL, TransactionDefinition::ISOLATION_DEFAULT); $actualStatus = $this->manager->getTransaction($def); $this->assertNull($actualStatus); @@ -56,8 +56,8 @@ public function testGetTransactionNeverButOpen() $txStatus = $this->getTxStatusMock(); $this->manager->expects($this->once())->method('doBeginTransaction')->will($this->returnValue($txStatus)); - $def1 = $this->createDefinition(TransactionDefinition::PROPAGATION_REQUIRED, TransactionDefinition::ISOLATION_DEFAULT); - $def2 = $this->createDefinition(TransactionDefinition::PROPAGATION_NEVER, TransactionDefinition::ISOLATION_DEFAULT); + $def1 = $this->createDefinition(TransactionDefinition::PROPAGATION_JOINED, TransactionDefinition::ISOLATION_DEFAULT); + $def2 = $this->createDefinition(TransactionDefinition::PROPAGATION_MANUAL, TransactionDefinition::ISOLATION_DEFAULT); $actualStatus1 = $this->manager->getTransaction($def1); $this->setExpectedException("SimpleThings\TransactionalBundle\TransactionException"); @@ -69,7 +69,7 @@ public function testGetRequireTransactionTwice() $txStatus = $this->getTxStatusMock(); $this->manager->expects($this->once())->method('doBeginTransaction')->will($this->returnValue($txStatus)); - $def = $this->createDefinition(TransactionDefinition::PROPAGATION_REQUIRED, TransactionDefinition::ISOLATION_DEFAULT); + $def = $this->createDefinition(TransactionDefinition::PROPAGATION_JOINED, TransactionDefinition::ISOLATION_DEFAULT); $actualStatus1 = $this->manager->getTransaction($def); $actualStatus2 = $this->manager->getTransaction($def); @@ -81,8 +81,8 @@ public function testGetRequiredIsolationLevelMissmatch() $txStatus = $this->getTxStatusMock(); $this->manager->expects($this->once())->method('doBeginTransaction')->will($this->returnValue($txStatus)); - $def1 = $this->createDefinition(TransactionDefinition::PROPAGATION_REQUIRED, TransactionDefinition::ISOLATION_DEFAULT); - $def2 = $this->createDefinition(TransactionDefinition::PROPAGATION_REQUIRED, TransactionDefinition::ISOLATION_REPEATABLE_READ); + $def1 = $this->createDefinition(TransactionDefinition::PROPAGATION_JOINED, TransactionDefinition::ISOLATION_DEFAULT); + $def2 = $this->createDefinition(TransactionDefinition::PROPAGATION_JOINED, TransactionDefinition::ISOLATION_REPEATABLE_READ); $actualStatus1 = $this->manager->getTransaction($def1); @@ -96,8 +96,8 @@ public function testGetReadOnlyMissMatch() $txStatus->expects($this->once())->method('isReadOnly')->will($this->returnValue(true)); $this->manager->expects($this->once())->method('doBeginTransaction')->will($this->returnValue($txStatus)); - $def1 = $this->createDefinition(TransactionDefinition::PROPAGATION_REQUIRED, TransactionDefinition::ISOLATION_DEFAULT); - $def2 = $this->createDefinition(TransactionDefinition::PROPAGATION_REQUIRED, TransactionDefinition::ISOLATION_DEFAULT); + $def1 = $this->createDefinition(TransactionDefinition::PROPAGATION_JOINED, TransactionDefinition::ISOLATION_DEFAULT); + $def2 = $this->createDefinition(TransactionDefinition::PROPAGATION_JOINED, TransactionDefinition::ISOLATION_DEFAULT); $actualStatus1 = $this->manager->getTransaction($def1); @@ -120,7 +120,7 @@ public function testGetTransactionPropagationSupportsNestedInRequired() $txStatus = $this->getTxStatusMock(); $this->manager->expects($this->once())->method('doBeginTransaction')->will($this->returnValue($txStatus)); - $def1 = $this->createDefinition(TransactionDefinition::PROPAGATION_REQUIRED, TransactionDefinition::ISOLATION_DEFAULT); + $def1 = $this->createDefinition(TransactionDefinition::PROPAGATION_JOINED, TransactionDefinition::ISOLATION_DEFAULT); $def2 = $this->createDefinition(TransactionDefinition::PROPAGATION_SUPPORTS, TransactionDefinition::ISOLATION_DEFAULT); $actualStatus1 = $this->manager->getTransaction($def1); @@ -136,8 +136,8 @@ public function testGetTransactionRequiresNew() $this->manager->expects($this->at(0))->method('doBeginTransaction')->will($this->returnValue($txStatus1)); $this->manager->expects($this->at(1))->method('doBeginTransaction')->will($this->returnValue($txStatus2)); - $def1 = $this->createDefinition(TransactionDefinition::PROPAGATION_REQUIRED, TransactionDefinition::ISOLATION_DEFAULT); - $def2 = $this->createDefinition(TransactionDefinition::PROPAGATION_REQUIRES_NEW, TransactionDefinition::ISOLATION_DEFAULT); + $def1 = $this->createDefinition(TransactionDefinition::PROPAGATION_JOINED, TransactionDefinition::ISOLATION_DEFAULT); + $def2 = $this->createDefinition(TransactionDefinition::PROPAGATION_ISOLATED, TransactionDefinition::ISOLATION_DEFAULT); $actualStatus1 = $this->manager->getTransaction($def1); $actualStatus2 = $this->manager->getTransaction($def2); @@ -150,7 +150,7 @@ public function testCommit() $txStatus1 = $this->getTxStatusMock(); $this->manager->expects($this->at(0))->method('doBeginTransaction')->will($this->returnValue($txStatus1)); $this->manager->expects($this->at(1))->method('doCommit'); - $def1 = $this->createDefinition(TransactionDefinition::PROPAGATION_REQUIRED, TransactionDefinition::ISOLATION_DEFAULT); + $def1 = $this->createDefinition(TransactionDefinition::PROPAGATION_JOINED, TransactionDefinition::ISOLATION_DEFAULT); $actualStatus1 = $this->manager->getTransaction($def1); @@ -162,7 +162,7 @@ public function testCommitRecommitException() $txStatus1 = $this->getTxStatusMock(); $this->manager->expects($this->at(0))->method('doBeginTransaction')->will($this->returnValue($txStatus1)); $this->manager->expects($this->at(1))->method('doCommit'); - $def1 = $this->createDefinition(TransactionDefinition::PROPAGATION_REQUIRED, TransactionDefinition::ISOLATION_DEFAULT); + $def1 = $this->createDefinition(TransactionDefinition::PROPAGATION_JOINED, TransactionDefinition::ISOLATION_DEFAULT); $actualStatus1 = $this->manager->getTransaction($def1); @@ -179,7 +179,7 @@ public function testCommitRollbackOnly() $this->manager->expects($this->at(0))->method('doBeginTransaction')->will($this->returnValue($txStatus1)); $this->manager->expects($this->at(1))->method('doRollBack'); - $def1 = $this->createDefinition(TransactionDefinition::PROPAGATION_REQUIRED, TransactionDefinition::ISOLATION_DEFAULT); + $def1 = $this->createDefinition(TransactionDefinition::PROPAGATION_JOINED, TransactionDefinition::ISOLATION_DEFAULT); $actualStatus1 = $this->manager->getTransaction($def1); $this->manager->commit($actualStatus1); @@ -196,8 +196,8 @@ public function testCommitRequiresNew() $this->manager->expects($this->at(2))->method('doCommit')->with($this->equalTo($txStatus2)); $this->manager->expects($this->at(2))->method('doCommit')->with($this->equalTo($txStatus1)); - $def1 = $this->createDefinition(TransactionDefinition::PROPAGATION_REQUIRED, TransactionDefinition::ISOLATION_DEFAULT); - $def2 = $this->createDefinition(TransactionDefinition::PROPAGATION_REQUIRES_NEW, TransactionDefinition::ISOLATION_DEFAULT); + $def1 = $this->createDefinition(TransactionDefinition::PROPAGATION_JOINED, TransactionDefinition::ISOLATION_DEFAULT); + $def2 = $this->createDefinition(TransactionDefinition::PROPAGATION_ISOLATED, TransactionDefinition::ISOLATION_DEFAULT); $actualStatus1 = $this->manager->getTransaction($def1); diff --git a/Tests/Transactions/Http/HttpTransactionsListenerTest.php b/Tests/Transactions/Http/HttpTransactionsListenerTest.php index aaf5453..463c286 100644 --- a/Tests/Transactions/Http/HttpTransactionsListenerTest.php +++ b/Tests/Transactions/Http/HttpTransactionsListenerTest.php @@ -42,12 +42,12 @@ public function testOnCoreController() $txStatus = $this->getMock('SimpleThings\TransactionalBundle\Transactions\TransactionStatus'); $def = new TransactionDefinition('name', 1, 1, false, array()); - $this->matcher->expects($this->once())->method('match')->will($this->returnValue(array($def))); - $this->registry->expects($this->once())->method('getTransactions')->with($this->equalTo(array($def)))->will($this->returnValue(array($txStatus))); + $this->matcher->expects($this->once())->method('match')->will($this->returnValue($def)); + $this->registry->expects($this->once())->method('getTransaction')->with($this->equalTo($def))->will($this->returnValue($txStatus)); $this->listener->onCoreController($event); - $this->assertSame(array($txStatus), $request->attributes->get('_transactions')); + $this->assertSame($txStatus, $request->attributes->get('_transaction')); } } diff --git a/Tests/Transactions/Http/TransactionalMatcherTest.php b/Tests/Transactions/Http/TransactionalMatcherTest.php index e41ca54..39c1544 100644 --- a/Tests/Transactions/Http/TransactionalMatcherTest.php +++ b/Tests/Transactions/Http/TransactionalMatcherTest.php @@ -21,22 +21,21 @@ public function testMatchPattern($pattern, $method, $matched, $readOnly) 'methods' => array('POST', 'PUT'), 'conn' => 'orm.default', 'isolation' => TransactionDefinition::ISOLATION_DEFAULT, - 'propagation' => TransactionDefinition::PROPAGATION_REQUIRED, + 'propagation' => TransactionDefinition::PROPAGATION_JOINED, 'noRollbackFor' => array(), ); $matcher = new TransactionalMatcher(array($pattern), array(), $this->reader); $controller = new TestController(); - $definitions = $matcher->match($method, array($controller, 'fooAction')); + $definition = $matcher->match($method, array($controller, 'fooAction')); if ($matched) { - $this->assertInternalType('array', $definitions); - $this->assertCount(1, $definitions); - $this->assertEquals('orm.default', $definitions[0]->getManagerName()); - $this->assertEquals($readOnly, $definitions[0]->getReadOnly()); + $this->assertInstanceOf('SimpleThings\TransactionalBundle\Transactions\TransactionDefinition', $definition); + $this->assertEquals('orm.default', $definition->getManagerName()); + $this->assertEquals($readOnly, $definition->getReadOnly()); } else { - $this->assertCount(0, $definitions); + $this->assertFalse($definition); } } @@ -44,7 +43,7 @@ public function testMatchClassAnnotation() { $defaults = array( 'isolation' => TransactionDefinition::ISOLATION_DEFAULT, - 'propagation' => TransactionDefinition::PROPAGATION_REQUIRED, + 'propagation' => TransactionDefinition::PROPAGATION_JOINED, 'noRollbackFor' => array(), ); @@ -64,19 +63,19 @@ public function testMatchClassAnnotation() $expectedDefinition = new TransactionDefinition( 'orm.default', - TransactionDefinition::PROPAGATION_REQUIRED, + TransactionDefinition::PROPAGATION_JOINED, TransactionDefinition::ISOLATION_DEFAULT, false, array() ); - $this->assertEquals($expectedDefinition, $definition[0]); + $this->assertEquals($expectedDefinition, $definition); } public function testMatchMethodAnnotation() { $defaults = array( 'isolation' => TransactionDefinition::ISOLATION_DEFAULT, - 'propagation' => TransactionDefinition::PROPAGATION_REQUIRED, + 'propagation' => TransactionDefinition::PROPAGATION_JOINED, 'noRollbackFor' => array(), ); @@ -98,40 +97,12 @@ public function testMatchMethodAnnotation() $expectedDefinition = new TransactionDefinition( 'orm.default', - TransactionDefinition::PROPAGATION_REQUIRED, + TransactionDefinition::PROPAGATION_JOINED, TransactionDefinition::ISOLATION_DEFAULT, false, array() ); - $this->assertEquals($expectedDefinition, $definition[0]); - } - - /** - * @expectedException \SimpleThings\TransactionalBundle\TransactionException - */ - public function testThrowExceptionWhenStoreDuplicateConnectionMatch() - { - $pattern = array( - 'pattern' => '.*', - 'methods' => array('GET'), - 'conn' => 'orm.default', - 'isolation' => TransactionDefinition::ISOLATION_DEFAULT, - 'propagation' => TransactionDefinition::PROPAGATION_REQUIRED, - 'noRollbackFor' => array(), - ); - $this->reader->expects($this->once()) - ->method('getClassAnnotation') - ->will($this->returnValue( - new Transactional(array( - 'methods' => array('GET'), - 'conn' => 'orm.default', - )) - )); - - $matcher = new TransactionalMatcher(array($pattern), array(), $this->reader); - $request = Request::create('/foo', 'GET'); - - $matcher->match($request, array(new TestController(), 'fooAction')); + $this->assertEquals($expectedDefinition, $definition); } public function getPatterns() diff --git a/Transactions/AbstractTransactionManager.php b/Transactions/AbstractTransactionManager.php index 1222dc7..e9439ce 100644 --- a/Transactions/AbstractTransactionManager.php +++ b/Transactions/AbstractTransactionManager.php @@ -22,6 +22,11 @@ abstract class AbstractTransactionManager implements TransactionManagerInterface */ private $transactions = array(); + /** + * @var TrannsactionStatus + */ + private $currentTransaction = null; + /** * @var ScopeHandler */ @@ -41,48 +46,45 @@ abstract protected function doRollBack(TransactionStatus $def); protected function beginTransaction(TransactionDefinition $def) { $status = $this->doBeginTransaction($def); - $oid = spl_object_hash($status); - $this->transactions[$oid] = array( - 'status' => $status, - 'def' => $def, - ); + $this->transactions[] = $this->currentTransaction = $status; return $status; } public function getTransaction(TransactionDefinition $def) { switch ($def->getPropagation()) { - case TransactionDefinition::PROPAGATION_REQUIRES_NEW: + case TransactionDefinition::PROPAGATION_ISOLATED: $this->scope->enterScope(); $status = $this->beginTransaction($def); break; - case TransactionDefinition::PROPAGATION_NEVER: + case TransactionDefinition::PROPAGATION_MANUAL: if (count($this->transactions)) { - throw new TransactionException("Controller does not want to run in transaction, but one is open."); + throw new TransactionException("Controller does not want to run inside any transaction, but there is one open."); } return null; - case TransactionDefinition::PROPAGATION_REQUIRED: + case TransactionDefinition::PROPAGATION_JOINED: + $this->scope->enterScope(); $this->scope->increaseNestingLevel(); - $openTransactionDef = $this->getCurrentTransactionDef(); - $status = $this->getCurrentTransaction(); - if ($openTransactionDef) { - if ($def->getIsolationLevel() != $openTransactionDef->getIsolationLevel()) { + if ($this->currentTransaction) { + if ($def->getIsolationLevel() != $this->currentTransaction->getIsolationLevel()) { throw new TransactionException("Trying to re-use transaction that has different isolation level than the already active one."); } - if ($status->isReadOnly() && ! $def->getReadOnly()) { + if ($this->currentTransaction->isReadOnly() && ! $def->getReadOnly()) { throw new TransactionException("Cannot reuse readonly transaction when requesting a read/write transaction."); } - } - - if (!$status) { + $status = $this->currentTransaction; + } else { $status = $this->beginTransaction($def); } + break; case TransactionDefinition::PROPAGATION_SUPPORTS: default: - $this->scope->increaseNestingLevel(); - $status = $this->getCurrentTransaction(); + if ($this->currentTransaction) { + $this->scope->increaseNestingLevel(); + } + $status = $this->currentTransaction; break; } return $status; @@ -96,8 +98,8 @@ public function commit(TransactionStatus $status) if ($status->isCompleted()) { throw new TransactionException("Cannot commit an already completed transaction."); - } else if (!isset($this->transactions[spl_object_hash($status)])) { - throw new TransactionException("Cannot commit a detached transaction. It may have been committed before or belongs to another transaction manager"); + } else if ($this->currentTransaction !== $status) { + throw new TransactionException("Cannot commit transaction that is not the currently active. The order of your transaction was messed up."); } $this->scope->decreaseNestingLevel(); @@ -117,8 +119,8 @@ public function rollBack(TransactionStatus $status) { if ($status->isCompleted()) { throw new TransactionException("Cannot rollback an already completed transaction."); - } else if (!isset($this->transactions[spl_object_hash($status)])) { - throw new TransactionException("Cannot rollback a detached transaction. It may have been committed/rollbacked before or belongs to another transaction manager"); + } else if ($this->currentTransaction !== $status) { + throw new TransactionException("Cannot commit transaction that is not the currently active. The order of your transaction was messed up."); } $this->scope->decreaseNestingLevel(); @@ -136,24 +138,9 @@ public function rollBack(TransactionStatus $status) private function cleanupAfterTransaction($status) { - $def = $this->transactions[spl_object_hash($status)]['def']; - if ($def->getPropagation() == TransactionDefinition::PROPAGATION_REQUIRES_NEW) { - $this->scope->leaveScope(); - } - - unset($this->transactions[spl_object_hash($status)]); - } - - private function getCurrentTransaction() - { - $tx = end($this->transactions); - return $tx['status']; - } - - private function getCurrentTransactionDef() - { - $tx = end($this->transactions); - return $tx['def']; + $this->scope->leaveScope(); + array_pop($this->transactions); + $this->currentTransaction = end($this->transactions); } } diff --git a/Transactions/Http/HttpTransactionsListener.php b/Transactions/Http/HttpTransactionsListener.php index 3795078..0de51cf 100644 --- a/Transactions/Http/HttpTransactionsListener.php +++ b/Transactions/Http/HttpTransactionsListener.php @@ -48,14 +48,18 @@ public function __construct(TransactionsRegistry $registry, TransactionalMatcher public function onCoreController(FilterControllerEvent $event) { $request = $event->getRequest(); - $definitions = $this->matcher->match($request->getMethod(), $event->getController()); - $txManagers = $this->registry->getTransactions($definitions); - - if ($txManagers && $this->logger) { - $this->logger->info("[TransactionBundle] Started transactions for " . implode(", ", array_keys($txManagers))); + $definition = $this->matcher->match($request->getMethod(), $event->getController()); + if (!$definition) { + return; } - $request->attributes->set('_transactions', $txManagers); + $txManager = $this->registry->getTransaction($definition); + $request->attributes->set('_transaction', $txManager); + $request->attributes->set('_transaction_def', $definition); + + if ($txManager && $this->logger) { + $this->logger->info("[TransactionBundle] Started transaction for " . $definition->getManagerName()); + } } public function onKernelResponse(FilterResponseEvent $event) @@ -63,15 +67,16 @@ public function onKernelResponse(FilterResponseEvent $event) $request = $event->getRequest(); $response = $event->getResponse(); - if (!$request->attributes->has('_transactions')) { + $txStatus = $request->attributes->get('_transaction'); + if ($txStatus === null) { return; } + $txDef = $request->attributes->get('_transaction_def'); - $txManagers = $request->attributes->get('_transactions'); if ($response->getStatusCode() >= 400 && $response->getStatusCode() != 404) { - $this->rollBack($txManagers); + $this->rollBack($txStatus, $txDef); } else { - $this->commit($txManagers); + $this->commit($txStatus, $txDef); } } @@ -79,36 +84,35 @@ public function onKernelException(GetResponseForExceptionEvent $event) { $request = $event->getRequest(); $ex = $event->getException(); - var_dump($ex->getMessage()); - if (!$request->attributes->has('_transactions')) { + $txStatus = $request->attributes->get('_transaction'); + if ($txStatus === null) { return; } - - $txManagers = $request->attributes->get('_transactions'); + $txDef = $request->attributes->get('_transaction_def'); if ($ex instanceof NotFoundHttpException) { - $this->registry->commit($txManagers); + $this->registry->commit($txStatus); } else { - $this->registry->rollBack($txManagers); + $this->registry->rollBack($txStatus); } } - private function commit($txManagers) + private function commit($txStatus, $txDefinition) { - $this->registry->commit($txManagers); + $this->registry->commit($txStatus); - if ($txManagers && $this->logger) { - $this->logger->info("[TransactionBundle] Committed transactions for " . implode(", ", array_keys($txManagers))); + if ($this->logger) { + $this->logger->info("[TransactionBundle] Committed transaction for " . $txDefinition->getManagerName()); } } - private function rollBack($txManagers) + private function rollBack($txStatus, $txDefinition) { - $this->registry->rollBack($txManagers); + $this->registry->rollBack($txStatus); - if ($txManagers && $this->logger) { - $this->logger->info("[TransactionBundle] Aborted transactions for " . implode(", ", array_keys($txManagers))); + if ($this->logger) { + $this->logger->info("[TransactionBundle] Aborted transaction for " . $txDefinition->getManagerName()); } } } diff --git a/Transactions/Http/TransactionalMatcher.php b/Transactions/Http/TransactionalMatcher.php index 915bf02..5eca661 100644 --- a/Transactions/Http/TransactionalMatcher.php +++ b/Transactions/Http/TransactionalMatcher.php @@ -58,7 +58,7 @@ public function __construct(array $patterns, array $defaults = array(), Reader $ * Match if he current controller/action should be transactional or not. * * Important: Only Controller as services or Class#Action method - * controllers can be transactional. + * controllers can be transactional. Closures or function calls can't. * * @param string $method HTTP Method * @param mixed $controllerCallback @@ -75,20 +75,20 @@ public function match($method, $controllerCallback) $subject = $class . "::" . $action; - if (!isset($this->cache[$subject][$method])) { - $this->cache[$subject] = array(); + if (!isset($this->cache[$subject])) { + $this->cache[$subject] = false; $this->matchPatterns($subject); $this->matchAnnotations($subject, $controller, $action); if (!$this->cache[$subject] && $this->defaults) { - $this->cache[$subject][$this->defaults['conn']] = $this->defaults; + $this->cache[$subject] = $this->defaults; } } - $definitions = array(); - $requireNew = false; - foreach ($this->cache[$subject] as $connectionName => $definition) { - $definitions[] = new TransactionDefinition( + if ($this->cache[$subject]) { + $definition = $this->cache[$subject]; + + return new TransactionDefinition( $definition['conn'], $definition['propagation'], $definition['isolation'], @@ -96,7 +96,7 @@ public function match($method, $controllerCallback) ); } - return $definitions; + return false; } /** @@ -141,17 +141,12 @@ private function matchAnnotations($subject, $controller, $action) private function storeMatch($subject, $pattern) { - $conn = $pattern['conn']; - if (isset($this->cache[$subject][$conn])) { - throw TransactionException::duplicateConnectionMatch($conn, $pattern); - } - - $this->cache[$subject][$conn] = array( - 'conn' => $conn, - 'isolation' => $pattern['isolation'], - 'propagation' => $pattern['propagation'], - 'noRollbackFor' => $pattern['noRollbackFor'], - 'methods' => $pattern['methods'], - ); + $this->cache[$subject] = array( + 'conn' => $pattern['conn'], + 'isolation' => $pattern['isolation'], + 'propagation' => $pattern['propagation'], + 'noRollbackFor' => $pattern['noRollbackFor'], + 'methods' => $pattern['methods'], + ); } } diff --git a/Transactions/ScopeHandler.php b/Transactions/ScopeHandler.php index 3888fe2..a399558 100644 --- a/Transactions/ScopeHandler.php +++ b/Transactions/ScopeHandler.php @@ -29,9 +29,11 @@ public function __construct($container) public function enterScope() { + if ($this->levels) { + $this->container->enterScope('transactional'); + } $this->levels[] = $this->currentLevel; $this->currentLevel = 0; - $this->container->enterScope('transactional'); } public function leaveScope() @@ -40,7 +42,9 @@ public function leaveScope() throw new TransactionException("Cannot leave transaction scope that still has levels."); } $this->currentLevel = array_pop($this->levels); - $this->container->leaveScope('transactional'); + if ($this->levels) { + $this->container->leaveScope('transactional'); + } } public function increaseNestingLevel() diff --git a/Transactions/TransactionDefinition.php b/Transactions/TransactionDefinition.php index 80c5048..cbb8d68 100644 --- a/Transactions/TransactionDefinition.php +++ b/Transactions/TransactionDefinition.php @@ -14,40 +14,47 @@ namespace SimpleThings\TransactionalBundle\Transactions; +/** + * Describes the properties of a transaction + * + * @author Benjamin Eberlei + */ class TransactionDefinition { /** * A transaction definition of this kind doesnt mind if its nested inside * another transaction or not and does not start a transaction on its own. * - * This is the default behavior. - * * @var int */ const PROPAGATION_SUPPORTS = 1; /** - * A transaction is required. If a transaction - * is already open for the transaction manager it will be re-used. + * Joins into an existing transaction or opens a new one. + * + * This is the default behavior. * * @var int */ - const PROPAGATION_REQUIRED = 2; + const PROPAGATION_JOINED = 2; /** * A NEW transaction is required. When the transaction is finished the old - * higher level transaction will be restored. + * higher level transaction will be restored. This mode may open new + * database connection leading to additional resources being used by your + * script. * * @var int */ - const PROPAGATION_REQUIRES_NEW = 3; + const PROPAGATION_ISOLATED = 3; /** - * Throws an exception if a transaction is open. + * Throws an exception if a transaction is open. Doesn't open a transaction + * itself. Leaves transaction management to the user. * * @var int */ - const PROPAGATION_NEVER = 4; + const PROPAGATION_MANUAL = 4; const ISOLATION_DEFAULT = 0; const ISOLATION_READ_UNCOMMITTED = 1; diff --git a/Transactions/TransactionStatus.php b/Transactions/TransactionStatus.php index 9cc542f..12866b2 100644 --- a/Transactions/TransactionStatus.php +++ b/Transactions/TransactionStatus.php @@ -18,6 +18,13 @@ */ interface TransactionStatus { + /** + * Get the Isolation level of this transaction + * + * @return int + */ + function getIsolationLevel(); + /** * Checks if the transaction is read-only. * @@ -58,10 +65,10 @@ function isCompleted(); function hasSavepoint(); /** - * Commit the transaction at this point. + * Return the connection object that is wrapped in this status. * - * @return void + * @return object */ - function commit(); + function getWrappedConnection(); } diff --git a/Transactions/TransactionsRegistry.php b/Transactions/TransactionsRegistry.php index 254924e..9a44183 100644 --- a/Transactions/TransactionsRegistry.php +++ b/Transactions/TransactionsRegistry.php @@ -23,10 +23,11 @@ * request, however many are supported by this facade. The Transactions passed * here should always be in the same request, not from different requests. */ -class TransactionsRegistry +class TransactionsRegistry implements TransactionManagerInterface { private $container; private $connectionServices; + private $transactions; public function __construct(ContainerInterface $container, $connectionServices = array()) { @@ -34,38 +35,24 @@ public function __construct(ContainerInterface $container, $connectionServices = $this->connectionServices = $connectionServices; } - public function getTransactions(array $definitions) + public function getTransaction(TransactionDefinition $definition) { - $txManagers = array(); - $requiresNew = false; - foreach ($definitions as $def) { - if ($def->getPropagation() == TransactionDefinition::PROPAGATION_REQUIRES_NEW) { - if ($requiresNew) { - throw new TransactionException("Cannot have more than one connection to require a new transaction per request."); - } - $requiresNew = true; - } - - $managerName = $def->getManagerName(); - if ($txStatus = $this->getTransactionManager($managerName)->getTransaction($def)) { - $txManagers[$managerName] = $txStatus; - } - } - return $txManagers; + $managerName = $definition->getManagerName(); + $status = $this->getTransactionManager($managerName)->getTransaction($definition); + $this->transactions[spl_object_hash($status)] = $managerName; + return $status; } - public function commit(array $statuses) + public function commit(TransactionStatus $status) { - foreach (array_reverse($statuses) AS $managerName => $txStatus) { - $this->getTransactionManager($managerName)->commit($txStatus); - } + $managerName = $this->transactions[spl_object_hash($status)]; + $this->getTransactionManager($managerName)->commit($status); } - public function rollBack(array $statuses) + public function rollBack(TransactionStatus $status) { - foreach (array_reverse($statuses) AS $managerName => $txStatus) { - $this->getTransactionManager($managerName)->rollBack($txStatus); - } + $managerName = $this->transactions[spl_object_hash($status)]; + $this->getTransactionManager($managerName)->rollBack($status); } private function getTransactionManager($name) diff --git a/composer.json b/composer.json index ce6c161..922b942 100644 --- a/composer.json +++ b/composer.json @@ -10,7 +10,8 @@ }], "require": { "php": ">=5.3.0", - "symfony/symfony" : ">=2.0" + "symfony/symfony" : ">=2.0", + "doctrine/dbal": ">=2.0" }, "autoload": { "psr-0": { From ca422515d3527f10591cd143d55d19b3b12f064d Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Thu, 26 Jan 2012 20:35:31 +0100 Subject: [PATCH 14/24] Get rid of the PROPAGATION, ISOLATION overhead and back to simple again. --- .../SimpleThingsTransactionalExtension.php | 2 - Doctrine/DBALTransactionManager.php | 56 +++-- Doctrine/DBALTransactionStatus.php | 17 +- Doctrine/ObjectTransactionManager.php | 2 +- Doctrine/OrmTransactionManager.php | 2 +- Tests/Functional/EndToEndTest.php | 27 ++- Tests/Functional/TransactionalKernelTest.php | 12 +- .../AbstractTransactionManagerTest.php | 214 ------------------ .../Http/TransactionalMatcherTest.php | 12 +- Transactions/AbstractTransactionManager.php | 147 ------------ .../Http/HttpTransactionsListener.php | 12 +- Transactions/Http/TransactionalMatcher.php | 4 - Transactions/TransactionDefinition.php | 87 +------ Transactions/TransactionManagerInterface.php | 4 - Transactions/TransactionStatus.php | 14 -- Transactions/TransactionsRegistry.php | 14 +- 16 files changed, 86 insertions(+), 540 deletions(-) delete mode 100644 Tests/Transactions/AbstractTransactionManagerTest.php delete mode 100644 Transactions/AbstractTransactionManager.php diff --git a/DependencyInjection/SimpleThingsTransactionalExtension.php b/DependencyInjection/SimpleThingsTransactionalExtension.php index 616ddb1..4a4d3f7 100644 --- a/DependencyInjection/SimpleThingsTransactionalExtension.php +++ b/DependencyInjection/SimpleThingsTransactionalExtension.php @@ -46,8 +46,6 @@ public function load(array $configs, ContainerBuilder $builder) $config['defaults'] = array_merge(array( 'conn' => array(), 'pattern' => '.*', - 'propagation' => TransactionDefinition::PROPAGATION_JOINED, - 'isolation' => TransactionDefinition::ISOLATION_DEFAULT, 'noRollbackFor' => array(), 'methods' => array('POST', 'PUT', 'DELETE', 'PATCH'), ), $config['defaults']); diff --git a/Doctrine/DBALTransactionManager.php b/Doctrine/DBALTransactionManager.php index c1cceea..7af0167 100644 --- a/Doctrine/DBALTransactionManager.php +++ b/Doctrine/DBALTransactionManager.php @@ -15,7 +15,7 @@ use SimpleThings\TransactionalBundle\Transactions\TransactionStatus; use SimpleThings\TransactionalBundle\Transactions\TransactionDefinition; -use SimpleThings\TransactionalBundle\Transactions\AbstractTransactionManager; +use SimpleThings\TransactionalBundle\Transactions\TransactionManagerInterface; /** * Doctrine Object TransactionManager for any Doctrine ObjectManager. @@ -26,40 +26,64 @@ * resetting the service and reconstituting the "previous" manager when the * transaction is committed or rolled back. */ -class DBALTransactionManager extends AbstractTransactionManager +class DBALTransactionManager implements TransactionManagerInterface { + /** + * @var ContainerInterface + */ protected $container; - public function __construct($container, $scopeHandler) + public function __construct($container) { $this->container = $container; - parent::__construct($scopeHandler); } - protected function createTxStatus($conn, $def) + /** + * Get a transaction status object. + * + * @return TransactionDefinition + */ + public function getTransaction(TransactionDefinition $def) { - return new DBALTransactionStatus($conn, $def); - } - - protected function doBeginTransaction(TransactionDefinition $def) - { - $conn = $this->container->get('simple_things_transactional.connections.' . $def->getManagerName()); + $conn = $this->container->get('simple_things_transactional.connections.' . $def->getConnectionName()); $conn->beginTransaction(); - return $this->createTxStatus($conn, $def); + return new DBALTransactionStatus($conn, $def); } - protected function doCommit(TransactionStatus $status) + /** + * Commit the transaction inside the status object. + * + * Depending on the Transaction#isRollBackOnly status this method commits + * or rollbacks the transaction wrapped inside the status. If an error + * happens during commit the original exception of the underlying + * connection is thrown from this method. + * + * @throws Exception + * @param TransactionStatus $status + * @return void + */ + public function commit(TransactionStatus $status) { $conn = $status->getWrappedConnection(); $conn->commit(); - $status->markCompleted(); + if ($conn->getTransactionNestingLevel() == 0) { + $status->markCompleted(); + } } - protected function doRollBack(TransactionStatus $status) + /** + * Rollback the transaction inside the status object. + * + * @param TransactionStatus $status + * @return void + */ + public function rollBack(TransactionStatus $status) { $conn = $status->getWrappedConnection(); $conn->rollback(); - $status->markCompleted(); + if ($conn->getTransactionNestingLevel() == 0) { + $status->markCompleted(); + } } } diff --git a/Doctrine/DBALTransactionStatus.php b/Doctrine/DBALTransactionStatus.php index 666a838..1151e2a 100644 --- a/Doctrine/DBALTransactionStatus.php +++ b/Doctrine/DBALTransactionStatus.php @@ -1,6 +1,6 @@ def = $def; } - public function getIsolationLevel() - { - return $this->def->getIsolationLevel(); - } - /** * Checks if the transaction is read-only. * @@ -78,16 +73,6 @@ public function isCompleted() return $this->completed; } - /** - * Check if this transaction has savepoints. - * - * @return bool - */ - public function hasSavepoint() - { - return false; - } - public function getWrappedConnection() { return $this->conn; diff --git a/Doctrine/ObjectTransactionManager.php b/Doctrine/ObjectTransactionManager.php index 3eaa10b..a92b180 100644 --- a/Doctrine/ObjectTransactionManager.php +++ b/Doctrine/ObjectTransactionManager.php @@ -40,7 +40,7 @@ protected function createTxStatus($manager, $def) protected function doBeginTransaction(TransactionDefinition $def) { - $manager = $this->container->get('simple_things_transactional.connections.' . $def->getManagerName()); + $manager = $this->container->get('simple_things_transactional.connections.' . $def->getConnectionName()); return $this->createTxStatus($manager, $def); } diff --git a/Doctrine/OrmTransactionManager.php b/Doctrine/OrmTransactionManager.php index c6009c3..2b086d3 100644 --- a/Doctrine/OrmTransactionManager.php +++ b/Doctrine/OrmTransactionManager.php @@ -17,7 +17,7 @@ class OrmTransactionManager extends ObjectTransactionManager { protected function doBeginTransaction(TransactionDefinition $def) { - $manager = $this->container->get('simple_things_transactional.connections.' . $def->getManagerName()); + $manager = $this->container->get('simple_things_transactional.connections.' . $def->getConnectionName()); $manager->beginTransaction(); return $this->createTxStatus($manager, $def); } diff --git a/Tests/Functional/EndToEndTest.php b/Tests/Functional/EndToEndTest.php index e5c3b84..13a7fd1 100644 --- a/Tests/Functional/EndToEndTest.php +++ b/Tests/Functional/EndToEndTest.php @@ -18,6 +18,7 @@ use Symfony\Component\HttpFoundation\Response; use Symfony\Component\HttpKernel\Log\NullLogger; use Symfony\Component\DependencyInjection\ContainerBuilder; +use Symfony\Component\DependencyInjection\Definition; use Symfony\Component\DependencyInjection\Scope; use Symfony\Bundle\FrameworkBundle\HttpKernel; use SimpleThings\TransactionalBundle\Transactions\Http\HttpTransactionsListener; @@ -36,6 +37,12 @@ class EndToEndTest extends \PHPUnit_Framework_TestCase public function setUp() { + $definition = new Definition('Doctrine\DBAL\Connection'); + $definition->setFactoryClass('Doctrine\DBAL\DriverManager'); + $definition->setFactoryMethod('getConnection'); + $definition->setArguments(array(array('driver' => 'pdo_sqlite', 'memory' => true))); + $definition->setScope('transactional'); + $conn = $this->conn = DriverManager::getConnection(array('driver' => 'pdo_sqlite', 'memory' => true)); $table = new \Doctrine\DBAL\Schema\Table("testdata"); $table->addColumn('id', 'integer', array('auto_increment' => true)); @@ -47,9 +54,11 @@ public function setUp() $this->logger = new FunctionalStackLogger; $container = new ContainerBuilder(); + $container->setDefinition('simple_things_transactional.connections.dbal.default', $definition); $container->addScope(new Scope('transactional')); $container->addScope(new Scope('request')); + // set does not work here, get has to work! $container->set('doctrine.dbal.default_connection', $conn); $container->set('simple_things_transactional.connections.dbal.default', $conn); $scope = new ScopeHandler($container); @@ -66,8 +75,6 @@ public function setUp() $matcher = new TransactionalMatcher(array(), array( 'conn' => 'dbal.default', 'methods' => array('POST'), - 'propagation' => TransactionDefinition::PROPAGATION_ISOLATED, - 'isolation' => TransactionDefinition::ISOLATION_DEFAULT, )); $txListener = new HttpTransactionsListener($registry, $matcher, $this->logger); @@ -86,10 +93,12 @@ public function testGetRequest() $this->assertEquals(array( "[TransactionBundle] Started transaction for dbal.default", + "[TransactionBundle] Started transaction for dbal.default", + "[TransactionBundle] Committed transaction for dbal.default", "[TransactionBundle] Committed transaction for dbal.default" ), $this->logger->logs); - var_dump($this->conn->fetchAll("SELECT * FROM testdata")); + $this->assertEquals(0, count($this->conn->fetchAll("SELECT * FROM testdata"))); } public function testPostRequest() @@ -99,9 +108,12 @@ public function testPostRequest() $this->assertEquals(array( "[TransactionBundle] Started transaction for dbal.default", + "[TransactionBundle] Started transaction for dbal.default", + "[TransactionBundle] Committed transaction for dbal.default", "[TransactionBundle] Committed transaction for dbal.default" ), $this->logger->logs); - var_dump($this->conn->fetchAll("SELECT * FROM testdata")); + + $this->assertEquals(2, count($this->conn->fetchAll("SELECT * FROM testdata"))); } } @@ -129,9 +141,7 @@ public function firstAction() $conn = $this->container->get('doctrine.dbal.default_connection'); $conn->insert("testdata", array("val" => "foo")); - try { - $this->container->get('http_kernel')->forward('DBALTestController:secondAxtion'); - } catch(\Exception $e) {} + $this->container->get('http_kernel')->forward('DBALTestController:secondAction'); return new Response('data', 200); } @@ -141,7 +151,8 @@ public function secondAction() $conn = $this->container->get('doctrine.dbal.default_connection'); $conn->insert("testdata", array("val" => "foo")); - throw new \InvalidArgumentException("blablabla!"); + return new Response('data', 200); } } + diff --git a/Tests/Functional/TransactionalKernelTest.php b/Tests/Functional/TransactionalKernelTest.php index 6fb7fef..3969fc6 100644 --- a/Tests/Functional/TransactionalKernelTest.php +++ b/Tests/Functional/TransactionalKernelTest.php @@ -35,13 +35,9 @@ public function setUp() $resolver->expects($this->once())->method('getArguments')->will($this->returnValue(array())); $txStatus1 = $this->getMock('SimpleThings\TransactionalBundle\Transactions\TransactionStatus'); - $manager = $this->getMock( - 'SimpleThings\TransactionalBundle\Transactions\AbstractTransactionManager', - array('doBeginTransaction', 'doCommit', 'doRollBack'), - array($this->getMock('SimpleThings\TransactionalBundle\Transactions\ScopeHandler', array(), array(), '', false)) - ); - $manager->expects($this->at(0))->method('doBeginTransaction')->will($this->returnValue($txStatus1)); - $manager->expects($this->at(1))->method('doCommit'); + $manager = $this->getMock('SimpleThings\TransactionalBundle\Transactions\TransactionManagerInterface', array(), array(), '', false); + $manager->expects($this->at(0))->method('getTransaction')->will($this->returnValue($txStatus1)); + $manager->expects($this->at(1))->method('commit'); $this->logger = new StackLogger; $container = $this->getMock('Symfony\Component\DependencyInjection\ContainerInterface'); @@ -52,8 +48,6 @@ public function setUp() $matcher = new TransactionalMatcher(array(), array( 'conn' => 'dbal.default', 'methods' => array('POST'), - 'propagation' => TransactionDefinition::PROPAGATION_JOINED, - 'isolation' => TransactionDefinition::ISOLATION_DEFAULT, )); $txListener = new HttpTransactionsListener($registry, $matcher, $this->logger); $dispatcher = new EventDispatcher(); diff --git a/Tests/Transactions/AbstractTransactionManagerTest.php b/Tests/Transactions/AbstractTransactionManagerTest.php deleted file mode 100644 index 9a05077..0000000 --- a/Tests/Transactions/AbstractTransactionManagerTest.php +++ /dev/null @@ -1,214 +0,0 @@ -manager = $this->getMock( - 'SimpleThings\TransactionalBundle\Transactions\AbstractTransactionManager', - array('doBeginTransaction', 'doCommit', 'doRollBack'), - array($this->getMock('SimpleThings\TransactionalBundle\Transactions\ScopeHandler', array(), array(), '', false)) - ); - } - - private function getTxStatusMock() - { - return $this->getMock('SimpleThings\TransactionalBundle\Transactions\TransactionStatus'); - } - - public function testGetTransaction() - { - $txStatus = $this->getTxStatusMock(); - $this->manager->expects($this->once())->method('doBeginTransaction')->will($this->returnValue($txStatus)); - $def = $this->createDefinition(TransactionDefinition::PROPAGATION_JOINED, TransactionDefinition::ISOLATION_DEFAULT); - $actualStatus = $this->manager->getTransaction($def); - - $this->assertSame($txStatus, $actualStatus); - } - - public function testGetNeverTransaction() - { - $def = $this->createDefinition(TransactionDefinition::PROPAGATION_MANUAL, TransactionDefinition::ISOLATION_DEFAULT); - $actualStatus = $this->manager->getTransaction($def); - - $this->assertNull($actualStatus); - } - - public function testGetTransactionNeverButOpen() - { - $txStatus = $this->getTxStatusMock(); - $this->manager->expects($this->once())->method('doBeginTransaction')->will($this->returnValue($txStatus)); - - $def1 = $this->createDefinition(TransactionDefinition::PROPAGATION_JOINED, TransactionDefinition::ISOLATION_DEFAULT); - $def2 = $this->createDefinition(TransactionDefinition::PROPAGATION_MANUAL, TransactionDefinition::ISOLATION_DEFAULT); - - $actualStatus1 = $this->manager->getTransaction($def1); - $this->setExpectedException("SimpleThings\TransactionalBundle\TransactionException"); - $actualStatus2 = $this->manager->getTransaction($def2); - } - - public function testGetRequireTransactionTwice() - { - $txStatus = $this->getTxStatusMock(); - $this->manager->expects($this->once())->method('doBeginTransaction')->will($this->returnValue($txStatus)); - - $def = $this->createDefinition(TransactionDefinition::PROPAGATION_JOINED, TransactionDefinition::ISOLATION_DEFAULT); - $actualStatus1 = $this->manager->getTransaction($def); - $actualStatus2 = $this->manager->getTransaction($def); - - $this->assertSame($actualStatus1, $actualStatus2); - } - - public function testGetRequiredIsolationLevelMissmatch() - { - $txStatus = $this->getTxStatusMock(); - $this->manager->expects($this->once())->method('doBeginTransaction')->will($this->returnValue($txStatus)); - - $def1 = $this->createDefinition(TransactionDefinition::PROPAGATION_JOINED, TransactionDefinition::ISOLATION_DEFAULT); - $def2 = $this->createDefinition(TransactionDefinition::PROPAGATION_JOINED, TransactionDefinition::ISOLATION_REPEATABLE_READ); - - $actualStatus1 = $this->manager->getTransaction($def1); - - $this->setExpectedException("SimpleThings\TransactionalBundle\TransactionException"); - $actualStatus2 = $this->manager->getTransaction($def2); - } - - public function testGetReadOnlyMissMatch() - { - $txStatus = $this->getTxStatusMock(); - $txStatus->expects($this->once())->method('isReadOnly')->will($this->returnValue(true)); - $this->manager->expects($this->once())->method('doBeginTransaction')->will($this->returnValue($txStatus)); - - $def1 = $this->createDefinition(TransactionDefinition::PROPAGATION_JOINED, TransactionDefinition::ISOLATION_DEFAULT); - $def2 = $this->createDefinition(TransactionDefinition::PROPAGATION_JOINED, TransactionDefinition::ISOLATION_DEFAULT); - - $actualStatus1 = $this->manager->getTransaction($def1); - - $this->setExpectedException("SimpleThings\TransactionalBundle\TransactionException"); - $actualStatus2 = $this->manager->getTransaction($def2); - } - - public function testGetTransactionPropagationSupports() - { - $this->manager->expects($this->never())->method('doBeginTransaction'); - - $def = $this->createDefinition(TransactionDefinition::PROPAGATION_SUPPORTS, TransactionDefinition::ISOLATION_DEFAULT); - - $status = $this->manager->getTransaction($def); - $this->assertNull($status); - } - - public function testGetTransactionPropagationSupportsNestedInRequired() - { - $txStatus = $this->getTxStatusMock(); - $this->manager->expects($this->once())->method('doBeginTransaction')->will($this->returnValue($txStatus)); - - $def1 = $this->createDefinition(TransactionDefinition::PROPAGATION_JOINED, TransactionDefinition::ISOLATION_DEFAULT); - $def2 = $this->createDefinition(TransactionDefinition::PROPAGATION_SUPPORTS, TransactionDefinition::ISOLATION_DEFAULT); - - $actualStatus1 = $this->manager->getTransaction($def1); - $actualStatus2 = $this->manager->getTransaction($def2); - - $this->assertSame($actualStatus1, $actualStatus2); - } - - public function testGetTransactionRequiresNew() - { - $txStatus1 = $this->getTxStatusMock(); - $txStatus2 = $this->getTxStatusMock(); - $this->manager->expects($this->at(0))->method('doBeginTransaction')->will($this->returnValue($txStatus1)); - $this->manager->expects($this->at(1))->method('doBeginTransaction')->will($this->returnValue($txStatus2)); - - $def1 = $this->createDefinition(TransactionDefinition::PROPAGATION_JOINED, TransactionDefinition::ISOLATION_DEFAULT); - $def2 = $this->createDefinition(TransactionDefinition::PROPAGATION_ISOLATED, TransactionDefinition::ISOLATION_DEFAULT); - - $actualStatus1 = $this->manager->getTransaction($def1); - $actualStatus2 = $this->manager->getTransaction($def2); - - $this->assertNotSame($actualStatus1, $actualStatus2); - } - - public function testCommit() - { - $txStatus1 = $this->getTxStatusMock(); - $this->manager->expects($this->at(0))->method('doBeginTransaction')->will($this->returnValue($txStatus1)); - $this->manager->expects($this->at(1))->method('doCommit'); - $def1 = $this->createDefinition(TransactionDefinition::PROPAGATION_JOINED, TransactionDefinition::ISOLATION_DEFAULT); - - $actualStatus1 = $this->manager->getTransaction($def1); - - $this->manager->commit($actualStatus1); - } - - public function testCommitRecommitException() - { - $txStatus1 = $this->getTxStatusMock(); - $this->manager->expects($this->at(0))->method('doBeginTransaction')->will($this->returnValue($txStatus1)); - $this->manager->expects($this->at(1))->method('doCommit'); - $def1 = $this->createDefinition(TransactionDefinition::PROPAGATION_JOINED, TransactionDefinition::ISOLATION_DEFAULT); - - $actualStatus1 = $this->manager->getTransaction($def1); - - $this->manager->commit($actualStatus1); - - $this->setExpectedException("SimpleThings\TransactionalBundle\TransactionException"); - $this->manager->commit($actualStatus1); - } - - public function testCommitRollbackOnly() - { - $txStatus1 = $this->getTxStatusMock(); - $txStatus1->expects($this->once())->method('isRollBackOnly')->will($this->returnValue(true)); - - $this->manager->expects($this->at(0))->method('doBeginTransaction')->will($this->returnValue($txStatus1)); - $this->manager->expects($this->at(1))->method('doRollBack'); - $def1 = $this->createDefinition(TransactionDefinition::PROPAGATION_JOINED, TransactionDefinition::ISOLATION_DEFAULT); - - $actualStatus1 = $this->manager->getTransaction($def1); - $this->manager->commit($actualStatus1); - } - - public function testCommitRequiresNew() - { - $txStatus1 = $this->getTxStatusMock(); - $txStatus1->expects($this->once())->method('isRollBackOnly')->will($this->returnValue(true)); - $txStatus2 = $this->getTxStatusMock(); - - $this->manager->expects($this->at(0))->method('doBeginTransaction')->will($this->returnValue($txStatus1)); - $this->manager->expects($this->at(1))->method('doBeginTransaction')->will($this->returnValue($txStatus2)); - $this->manager->expects($this->at(2))->method('doCommit')->with($this->equalTo($txStatus2)); - $this->manager->expects($this->at(2))->method('doCommit')->with($this->equalTo($txStatus1)); - - $def1 = $this->createDefinition(TransactionDefinition::PROPAGATION_JOINED, TransactionDefinition::ISOLATION_DEFAULT); - $def2 = $this->createDefinition(TransactionDefinition::PROPAGATION_ISOLATED, TransactionDefinition::ISOLATION_DEFAULT); - - $actualStatus1 = $this->manager->getTransaction($def1); - - $actualStatus2 = $this->manager->getTransaction($def2); - $this->manager->commit($actualStatus2); - $this->manager->commit($actualStatus1); - } - - private function createDefinition($propagation, $isolation, $readOnly = false) - { - return new TransactionDefinition("test", $propagation, $isolation, $readOnly, array()); - } -} - diff --git a/Tests/Transactions/Http/TransactionalMatcherTest.php b/Tests/Transactions/Http/TransactionalMatcherTest.php index 39c1544..0a19667 100644 --- a/Tests/Transactions/Http/TransactionalMatcherTest.php +++ b/Tests/Transactions/Http/TransactionalMatcherTest.php @@ -20,8 +20,6 @@ public function testMatchPattern($pattern, $method, $matched, $readOnly) 'pattern' => $pattern, 'methods' => array('POST', 'PUT'), 'conn' => 'orm.default', - 'isolation' => TransactionDefinition::ISOLATION_DEFAULT, - 'propagation' => TransactionDefinition::PROPAGATION_JOINED, 'noRollbackFor' => array(), ); @@ -32,7 +30,7 @@ public function testMatchPattern($pattern, $method, $matched, $readOnly) if ($matched) { $this->assertInstanceOf('SimpleThings\TransactionalBundle\Transactions\TransactionDefinition', $definition); - $this->assertEquals('orm.default', $definition->getManagerName()); + $this->assertEquals('orm.default', $definition->getConnectionName()); $this->assertEquals($readOnly, $definition->getReadOnly()); } else { $this->assertFalse($definition); @@ -42,8 +40,6 @@ public function testMatchPattern($pattern, $method, $matched, $readOnly) public function testMatchClassAnnotation() { $defaults = array( - 'isolation' => TransactionDefinition::ISOLATION_DEFAULT, - 'propagation' => TransactionDefinition::PROPAGATION_JOINED, 'noRollbackFor' => array(), ); @@ -63,8 +59,6 @@ public function testMatchClassAnnotation() $expectedDefinition = new TransactionDefinition( 'orm.default', - TransactionDefinition::PROPAGATION_JOINED, - TransactionDefinition::ISOLATION_DEFAULT, false, array() ); @@ -74,8 +68,6 @@ public function testMatchClassAnnotation() public function testMatchMethodAnnotation() { $defaults = array( - 'isolation' => TransactionDefinition::ISOLATION_DEFAULT, - 'propagation' => TransactionDefinition::PROPAGATION_JOINED, 'noRollbackFor' => array(), ); @@ -97,8 +89,6 @@ public function testMatchMethodAnnotation() $expectedDefinition = new TransactionDefinition( 'orm.default', - TransactionDefinition::PROPAGATION_JOINED, - TransactionDefinition::ISOLATION_DEFAULT, false, array() ); diff --git a/Transactions/AbstractTransactionManager.php b/Transactions/AbstractTransactionManager.php deleted file mode 100644 index e9439ce..0000000 --- a/Transactions/AbstractTransactionManager.php +++ /dev/null @@ -1,147 +0,0 @@ -scope = $scope; - } - - abstract protected function doBeginTransaction(TransactionDefinition $def); - - abstract protected function doCommit(TransactionStatus $def); - - abstract protected function doRollBack(TransactionStatus $def); - - protected function beginTransaction(TransactionDefinition $def) - { - $status = $this->doBeginTransaction($def); - $this->transactions[] = $this->currentTransaction = $status; - return $status; - } - - public function getTransaction(TransactionDefinition $def) - { - switch ($def->getPropagation()) { - case TransactionDefinition::PROPAGATION_ISOLATED: - $this->scope->enterScope(); - $status = $this->beginTransaction($def); - break; - case TransactionDefinition::PROPAGATION_MANUAL: - if (count($this->transactions)) { - throw new TransactionException("Controller does not want to run inside any transaction, but there is one open."); - } - return null; - case TransactionDefinition::PROPAGATION_JOINED: - $this->scope->enterScope(); - $this->scope->increaseNestingLevel(); - if ($this->currentTransaction) { - if ($def->getIsolationLevel() != $this->currentTransaction->getIsolationLevel()) { - throw new TransactionException("Trying to re-use transaction that has different isolation level than the already active one."); - } - - if ($this->currentTransaction->isReadOnly() && ! $def->getReadOnly()) { - throw new TransactionException("Cannot reuse readonly transaction when requesting a read/write transaction."); - } - $status = $this->currentTransaction; - } else { - $status = $this->beginTransaction($def); - } - - break; - case TransactionDefinition::PROPAGATION_SUPPORTS: - default: - if ($this->currentTransaction) { - $this->scope->increaseNestingLevel(); - } - $status = $this->currentTransaction; - break; - } - return $status; - } - - public function commit(TransactionStatus $status) - { - if ($status->isRollBackOnly()) { - return $this->rollBack($status); - } - - if ($status->isCompleted()) { - throw new TransactionException("Cannot commit an already completed transaction."); - } else if ($this->currentTransaction !== $status) { - throw new TransactionException("Cannot commit transaction that is not the currently active. The order of your transaction was messed up."); - } - - $this->scope->decreaseNestingLevel(); - if ($this->scope->getNestingLevel() > 0) { - return; - } - - $this->cleanupAfterTransaction($status); - if ($status->isReadOnly()) { - return; - } - - $this->doCommit($status); - } - - public function rollBack(TransactionStatus $status) - { - if ($status->isCompleted()) { - throw new TransactionException("Cannot rollback an already completed transaction."); - } else if ($this->currentTransaction !== $status) { - throw new TransactionException("Cannot commit transaction that is not the currently active. The order of your transaction was messed up."); - } - - $this->scope->decreaseNestingLevel(); - if ($this->scope->getNestingLevel() > 0) { - return; - } - - $this->cleanupAfterTransaction($status); - if ($status->isReadOnly()) { - return; - } - - $this->doRollBack($status); - } - - private function cleanupAfterTransaction($status) - { - $this->scope->leaveScope(); - array_pop($this->transactions); - $this->currentTransaction = end($this->transactions); - } -} - - diff --git a/Transactions/Http/HttpTransactionsListener.php b/Transactions/Http/HttpTransactionsListener.php index 0de51cf..cf93213 100644 --- a/Transactions/Http/HttpTransactionsListener.php +++ b/Transactions/Http/HttpTransactionsListener.php @@ -53,12 +53,12 @@ public function onCoreController(FilterControllerEvent $event) return; } - $txManager = $this->registry->getTransaction($definition); - $request->attributes->set('_transaction', $txManager); + $status = $this->registry->getTransaction($definition); + $request->attributes->set('_transaction', $status); $request->attributes->set('_transaction_def', $definition); - if ($txManager && $this->logger) { - $this->logger->info("[TransactionBundle] Started transaction for " . $definition->getManagerName()); + if ($status && $this->logger) { + $this->logger->info("[TransactionBundle] Started transaction for " . $definition->getConnectionName()); } } @@ -103,7 +103,7 @@ private function commit($txStatus, $txDefinition) $this->registry->commit($txStatus); if ($this->logger) { - $this->logger->info("[TransactionBundle] Committed transaction for " . $txDefinition->getManagerName()); + $this->logger->info("[TransactionBundle] Committed transaction for " . $txDefinition->getConnectionName()); } } @@ -112,7 +112,7 @@ private function rollBack($txStatus, $txDefinition) $this->registry->rollBack($txStatus); if ($this->logger) { - $this->logger->info("[TransactionBundle] Aborted transaction for " . $txDefinition->getManagerName()); + $this->logger->info("[TransactionBundle] Aborted transaction for " . $txDefinition->getConnectionName()); } } } diff --git a/Transactions/Http/TransactionalMatcher.php b/Transactions/Http/TransactionalMatcher.php index 5eca661..d2e840c 100644 --- a/Transactions/Http/TransactionalMatcher.php +++ b/Transactions/Http/TransactionalMatcher.php @@ -90,8 +90,6 @@ public function match($method, $controllerCallback) return new TransactionDefinition( $definition['conn'], - $definition['propagation'], - $definition['isolation'], ! in_array($method, (array)$definition['methods']) ); } @@ -143,8 +141,6 @@ private function storeMatch($subject, $pattern) { $this->cache[$subject] = array( 'conn' => $pattern['conn'], - 'isolation' => $pattern['isolation'], - 'propagation' => $pattern['propagation'], 'noRollbackFor' => $pattern['noRollbackFor'], 'methods' => $pattern['methods'], ); diff --git a/Transactions/TransactionDefinition.php b/Transactions/TransactionDefinition.php index cbb8d68..b4b3fd7 100644 --- a/Transactions/TransactionDefinition.php +++ b/Transactions/TransactionDefinition.php @@ -21,91 +21,28 @@ */ class TransactionDefinition { - /** - * A transaction definition of this kind doesnt mind if its nested inside - * another transaction or not and does not start a transaction on its own. - * - * @var int - */ - const PROPAGATION_SUPPORTS = 1; - - /** - * Joins into an existing transaction or opens a new one. - * - * This is the default behavior. - * - * @var int - */ - const PROPAGATION_JOINED = 2; - - /** - * A NEW transaction is required. When the transaction is finished the old - * higher level transaction will be restored. This mode may open new - * database connection leading to additional resources being used by your - * script. - * - * @var int - */ - const PROPAGATION_ISOLATED = 3; - - /** - * Throws an exception if a transaction is open. Doesn't open a transaction - * itself. Leaves transaction management to the user. - * - * @var int - */ - const PROPAGATION_MANUAL = 4; - - const ISOLATION_DEFAULT = 0; - const ISOLATION_READ_UNCOMMITTED = 1; - const ISOLATION_READ_COMMITTED = 2; - const ISOLATION_REPEATABLE_READ = 3; - const ISOLATION_SERIALIZABLE = 4; - /** * @var string */ - private $managerName; - - /** - * @var int - */ - private $isolationLevel; + private $connectionName; /** * @var bool */ private $readOnly; - /** - * @var int - */ - private $propagation; - /** * @var array */ private $noRollbackFor = array(); - public function __construct($managerName, $propagation, $isolationLevel, $readOnly = false, $noRollbackFor = array()) + public function __construct($connectionName, $readOnly = false, $noRollbackFor = array()) { - $this->managerName = $managerName; - $this->propagation = $propagation; - $this->isolationLevel = $isolationLevel; + $this->connectionName = $connectionName; $this->readOnly = $readOnly; $this->noRollbackFor = $noRollbackFor; } - /** - * Get propagation. - * - * @return propagation. - */ - public function getPropagation() - { - return $this->propagation; - } - /** * Get readOnly. * @@ -117,25 +54,15 @@ public function getReadOnly() } /** - * Get isolationLevel. + * Get connectionName. * - * @return isolationLevel. + * @return string */ - public function getIsolationLevel() + public function getConnectionName() { - return $this->isolationLevel; + return $this->connectionName; } - /** - * Get managerName. - * - * @return managerName. - */ - public function getManagerName() - { - return $this->managerName; - } - /** * Get noRollbackFor. * diff --git a/Transactions/TransactionManagerInterface.php b/Transactions/TransactionManagerInterface.php index e7cd138..2a6fc50 100644 --- a/Transactions/TransactionManagerInterface.php +++ b/Transactions/TransactionManagerInterface.php @@ -23,10 +23,6 @@ interface TransactionManagerInterface /** * Get a transaction status object. * - * 1. Returns a new transaction if none was opened with this manager yet. - * 2. Returns a previous transaction if the propagation is REQUIRED. - * 3. Returns a new transaction if the propagation is REQUIRES_NEW. - * * @return TransactionDefinition */ function getTransaction(TransactionDefinition $def); diff --git a/Transactions/TransactionStatus.php b/Transactions/TransactionStatus.php index 12866b2..0d58dde 100644 --- a/Transactions/TransactionStatus.php +++ b/Transactions/TransactionStatus.php @@ -18,13 +18,6 @@ */ interface TransactionStatus { - /** - * Get the Isolation level of this transaction - * - * @return int - */ - function getIsolationLevel(); - /** * Checks if the transaction is read-only. * @@ -57,13 +50,6 @@ function setRollBackOnly(); */ function isCompleted(); - /** - * Check if this transaction has savepoints. - * - * @return bool - */ - function hasSavepoint(); - /** * Return the connection object that is wrapped in this status. * diff --git a/Transactions/TransactionsRegistry.php b/Transactions/TransactionsRegistry.php index 9a44183..041d9b9 100644 --- a/Transactions/TransactionsRegistry.php +++ b/Transactions/TransactionsRegistry.php @@ -37,22 +37,22 @@ public function __construct(ContainerInterface $container, $connectionServices = public function getTransaction(TransactionDefinition $definition) { - $managerName = $definition->getManagerName(); - $status = $this->getTransactionManager($managerName)->getTransaction($definition); - $this->transactions[spl_object_hash($status)] = $managerName; + $connectionName = $definition->getConnectionName(); + $status = $this->getTransactionManager($connectionName)->getTransaction($definition); + $this->transactions[spl_object_hash($status)] = $connectionName; return $status; } public function commit(TransactionStatus $status) { - $managerName = $this->transactions[spl_object_hash($status)]; - $this->getTransactionManager($managerName)->commit($status); + $connectionName = $this->transactions[spl_object_hash($status)]; + $this->getTransactionManager($connectionName)->commit($status); } public function rollBack(TransactionStatus $status) { - $managerName = $this->transactions[spl_object_hash($status)]; - $this->getTransactionManager($managerName)->rollBack($status); + $connectionName = $this->transactions[spl_object_hash($status)]; + $this->getTransactionManager($connectionName)->rollBack($status); } private function getTransactionManager($name) From eb6ee97ae3034bad3654e998f9dd56820ad8fdb5 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Thu, 26 Jan 2012 21:13:45 +0100 Subject: [PATCH 15/24] More renaming, get rid of managers, only have providers, registry manages lifecycle. --- Doctrine/DBALTransactionManager.php | 89 ------------------- Doctrine/DBALTransactionProvider.php | 53 +++++++++++ Doctrine/DBALTransactionStatus.php | 45 +++++++++- Tests/Functional/EndToEndTest.php | 6 +- Tests/Functional/TransactionalKernelTest.php | 10 +-- .../Http/TransactionalMatcherTest.php | 2 +- Transactions/Annotations/Transactional.php | 8 -- Transactions/ScopeHandler.php | 65 -------------- Transactions/TransactionDefinition.php | 4 +- Transactions/TransactionManagerInterface.php | 51 ----------- Transactions/TransactionProviderInterface.php | 34 +++++++ Transactions/TransactionStatus.php | 25 ++++++ Transactions/TransactionsRegistry.php | 30 ++++--- 13 files changed, 181 insertions(+), 241 deletions(-) delete mode 100644 Doctrine/DBALTransactionManager.php create mode 100644 Doctrine/DBALTransactionProvider.php delete mode 100644 Transactions/ScopeHandler.php delete mode 100644 Transactions/TransactionManagerInterface.php create mode 100644 Transactions/TransactionProviderInterface.php diff --git a/Doctrine/DBALTransactionManager.php b/Doctrine/DBALTransactionManager.php deleted file mode 100644 index 7af0167..0000000 --- a/Doctrine/DBALTransactionManager.php +++ /dev/null @@ -1,89 +0,0 @@ -container = $container; - } - - /** - * Get a transaction status object. - * - * @return TransactionDefinition - */ - public function getTransaction(TransactionDefinition $def) - { - $conn = $this->container->get('simple_things_transactional.connections.' . $def->getConnectionName()); - $conn->beginTransaction(); - return new DBALTransactionStatus($conn, $def); - } - - /** - * Commit the transaction inside the status object. - * - * Depending on the Transaction#isRollBackOnly status this method commits - * or rollbacks the transaction wrapped inside the status. If an error - * happens during commit the original exception of the underlying - * connection is thrown from this method. - * - * @throws Exception - * @param TransactionStatus $status - * @return void - */ - public function commit(TransactionStatus $status) - { - $conn = $status->getWrappedConnection(); - $conn->commit(); - if ($conn->getTransactionNestingLevel() == 0) { - $status->markCompleted(); - } - } - - /** - * Rollback the transaction inside the status object. - * - * @param TransactionStatus $status - * @return void - */ - public function rollBack(TransactionStatus $status) - { - $conn = $status->getWrappedConnection(); - $conn->rollback(); - if ($conn->getTransactionNestingLevel() == 0) { - $status->markCompleted(); - } - } -} - diff --git a/Doctrine/DBALTransactionProvider.php b/Doctrine/DBALTransactionProvider.php new file mode 100644 index 0000000..4fec60e --- /dev/null +++ b/Doctrine/DBALTransactionProvider.php @@ -0,0 +1,53 @@ +container = $container; + } + + /** + * Get a transaction status object. + * + * @param TransactionDefinition $def + * @return TransactionStatus + */ + public function createTransaction(TransactionDefinition $def) + { + $conn = $this->container->get('doctrine.' . $def->getConnectionName().'_connection'); + return new DBALTransactionStatus($conn, $def); + } +} + diff --git a/Doctrine/DBALTransactionStatus.php b/Doctrine/DBALTransactionStatus.php index 1151e2a..9099f1a 100644 --- a/Doctrine/DBALTransactionStatus.php +++ b/Doctrine/DBALTransactionStatus.php @@ -40,7 +40,7 @@ public function __construct(Connection $conn, TransactionDefinition $def) */ public function isReadOnly() { - return $this->def->getReadOnly(); + return $this->def->isReadOnly(); } /** @@ -78,9 +78,48 @@ public function getWrappedConnection() return $this->conn; } - public function markCompleted() + /** + * Begin the transaction + */ + public function beginTransaction() + { + $this->conn->beginTransaction(); + } + + /** + * Commit the transaction + * + * Depending on the Transaction#isRollBackOnly status this method commits + * or rollbacks the transaction wrapped inside the status. If an error + * happens during commit the original exception of the underlying + * connection is thrown from this method. + * + * @throws Exception + * @return void + */ + public function commit() + { + if ($this->isReadOnly()) { + return $this->rollBack(); + } + + $this->conn->commit(); + if (0 === $this->conn->getTransactionNestingLevel()) { + $this->completed = true; + } + } + + /** + * Rollback the transaction inside the status object. + * + * @return void + */ + public function rollBack() { - $this->completed = true; + $this->conn->rollBack(); + if (0 === $this->conn->getTransactionNestingLevel()) { + $this->completed = true; + } } } diff --git a/Tests/Functional/EndToEndTest.php b/Tests/Functional/EndToEndTest.php index 13a7fd1..b356c1a 100644 --- a/Tests/Functional/EndToEndTest.php +++ b/Tests/Functional/EndToEndTest.php @@ -25,8 +25,7 @@ use SimpleThings\TransactionalBundle\Transactions\TransactionsRegistry; use SimpleThings\TransactionalBundle\Transactions\TransactionDefinition; use SimpleThings\TransactionalBundle\Transactions\Http\TransactionalMatcher; -use SimpleThings\TransactionalBundle\Transactions\ScopeHandler; -use SimpleThings\TransactionalBundle\Doctrine\DBALTransactionManager; +use SimpleThings\TransactionalBundle\Doctrine\DBALTransactionProvider; use Doctrine\DBAL\DriverManager; class EndToEndTest extends \PHPUnit_Framework_TestCase @@ -61,9 +60,8 @@ public function setUp() // set does not work here, get has to work! $container->set('doctrine.dbal.default_connection', $conn); $container->set('simple_things_transactional.connections.dbal.default', $conn); - $scope = new ScopeHandler($container); - $txManager = new DBALTransactionManager($container, $scope); + $txManager = new DBALTransactionProvider($container); $container->set('simple_things_transactional.tx.dbal.default', $txManager); $resolver = $this->getMock('Symfony\Component\HttpKernel\Controller\ControllerResolverInterface'); diff --git a/Tests/Functional/TransactionalKernelTest.php b/Tests/Functional/TransactionalKernelTest.php index 3969fc6..cd85383 100644 --- a/Tests/Functional/TransactionalKernelTest.php +++ b/Tests/Functional/TransactionalKernelTest.php @@ -35,19 +35,19 @@ public function setUp() $resolver->expects($this->once())->method('getArguments')->will($this->returnValue(array())); $txStatus1 = $this->getMock('SimpleThings\TransactionalBundle\Transactions\TransactionStatus'); - $manager = $this->getMock('SimpleThings\TransactionalBundle\Transactions\TransactionManagerInterface', array(), array(), '', false); - $manager->expects($this->at(0))->method('getTransaction')->will($this->returnValue($txStatus1)); - $manager->expects($this->at(1))->method('commit'); + $txStatus1->expects($this->any())->method('isReadOnly')->will($this->returnValue(false)); + $provider = $this->getMock('SimpleThings\TransactionalBundle\Transactions\TransactionProviderInterface', array(), array(), '', false); + $provider->expects($this->at(0))->method('createTransaction')->will($this->returnValue($txStatus1)); $this->logger = new StackLogger; $container = $this->getMock('Symfony\Component\DependencyInjection\ContainerInterface'); $container->expects($this->any())->method('has')->with($this->equalTo('simple_things_transactional.tx.dbal.default'))->will($this->returnValue(true)); - $container->expects($this->any())->method('get')->with($this->equalTo('simple_things_transactional.tx.dbal.default'))->will($this->returnValue($manager)); + $container->expects($this->any())->method('get')->with($this->equalTo('simple_things_transactional.tx.dbal.default'))->will($this->returnValue($provider)); $registry = new TransactionsRegistry($container); $matcher = new TransactionalMatcher(array(), array( 'conn' => 'dbal.default', - 'methods' => array('POST'), + 'methods' => array('POST', 'GET'), )); $txListener = new HttpTransactionsListener($registry, $matcher, $this->logger); $dispatcher = new EventDispatcher(); diff --git a/Tests/Transactions/Http/TransactionalMatcherTest.php b/Tests/Transactions/Http/TransactionalMatcherTest.php index 0a19667..23b93af 100644 --- a/Tests/Transactions/Http/TransactionalMatcherTest.php +++ b/Tests/Transactions/Http/TransactionalMatcherTest.php @@ -31,7 +31,7 @@ public function testMatchPattern($pattern, $method, $matched, $readOnly) if ($matched) { $this->assertInstanceOf('SimpleThings\TransactionalBundle\Transactions\TransactionDefinition', $definition); $this->assertEquals('orm.default', $definition->getConnectionName()); - $this->assertEquals($readOnly, $definition->getReadOnly()); + $this->assertEquals($readOnly, $definition->isReadOnly()); } else { $this->assertFalse($definition); } diff --git a/Transactions/Annotations/Transactional.php b/Transactions/Annotations/Transactional.php index 872d2e0..cb07279 100644 --- a/Transactions/Annotations/Transactional.php +++ b/Transactions/Annotations/Transactional.php @@ -25,14 +25,6 @@ class Transactional extends Annotation * @var string */ public $conn = null; - /** - * @var int - */ - public $propagation = null; - /** - * @var int - */ - public $isolation = null; /** * @var array */ diff --git a/Transactions/ScopeHandler.php b/Transactions/ScopeHandler.php deleted file mode 100644 index a399558..0000000 --- a/Transactions/ScopeHandler.php +++ /dev/null @@ -1,65 +0,0 @@ -container = $container; - } - - public function enterScope() - { - if ($this->levels) { - $this->container->enterScope('transactional'); - } - $this->levels[] = $this->currentLevel; - $this->currentLevel = 0; - } - - public function leaveScope() - { - if ($this->currentLevel > 0) { - throw new TransactionException("Cannot leave transaction scope that still has levels."); - } - $this->currentLevel = array_pop($this->levels); - if ($this->levels) { - $this->container->leaveScope('transactional'); - } - } - - public function increaseNestingLevel() - { - $this->currentLevel++; - } - - public function decreaseNestingLevel() - { - $this->currentLevel--; - } - - public function getNestingLevel() - { - return $this->currentLevel; - } -} - diff --git a/Transactions/TransactionDefinition.php b/Transactions/TransactionDefinition.php index b4b3fd7..82e3875 100644 --- a/Transactions/TransactionDefinition.php +++ b/Transactions/TransactionDefinition.php @@ -39,7 +39,7 @@ class TransactionDefinition public function __construct($connectionName, $readOnly = false, $noRollbackFor = array()) { $this->connectionName = $connectionName; - $this->readOnly = $readOnly; + $this->readOnly = (bool)$readOnly; $this->noRollbackFor = $noRollbackFor; } @@ -48,7 +48,7 @@ public function __construct($connectionName, $readOnly = false, $noRollbackFor = * * @return readOnly. */ - public function getReadOnly() + public function isReadOnly() { return $this->readOnly; } diff --git a/Transactions/TransactionManagerInterface.php b/Transactions/TransactionManagerInterface.php deleted file mode 100644 index 2a6fc50..0000000 --- a/Transactions/TransactionManagerInterface.php +++ /dev/null @@ -1,51 +0,0 @@ - - */ -interface TransactionManagerInterface -{ - /** - * Get a transaction status object. - * - * @return TransactionDefinition - */ - function getTransaction(TransactionDefinition $def); - - /** - * Commit the transaction inside the status object. - * - * Depending on the Transaction#isRollBackOnly status this method commits - * or rollbacks the transaction wrapped inside the status. If an error - * happens during commit the original exception of the underlying - * connection is thrown from this method. - * - * @throws Exception - * @param TransactionStatus $status - * @return void - */ - function commit(TransactionStatus $status); - - /** - * Rollback the transaction inside the status object. - * - * @param TransactionStatus $status - * @return void - */ - function rollBack(TransactionStatus $status); -} diff --git a/Transactions/TransactionProviderInterface.php b/Transactions/TransactionProviderInterface.php new file mode 100644 index 0000000..10a3d5d --- /dev/null +++ b/Transactions/TransactionProviderInterface.php @@ -0,0 +1,34 @@ + + */ +interface TransactionProviderInterface +{ + /** + * Get a transaction status object. + * + * Re-use an existing transaction status if there is already one for the + * currently active transaction. Throw an exception if the read-only status + * is not equal for the previously defined transaction. + * + * @param TransactionDefinition $def + * @return TransactionStatus + */ + function createTransaction(TransactionDefinition $def); +} diff --git a/Transactions/TransactionStatus.php b/Transactions/TransactionStatus.php index 0d58dde..95f4259 100644 --- a/Transactions/TransactionStatus.php +++ b/Transactions/TransactionStatus.php @@ -56,5 +56,30 @@ function isCompleted(); * @return object */ function getWrappedConnection(); + + /** + * Begin the transaction + */ + function beginTransaction(); + + /** + * Commit the transaction + * + * Depending on the Transaction#isRollBackOnly status this method commits + * or rollbacks the transaction wrapped inside the status. If an error + * happens during commit the original exception of the underlying + * connection is thrown from this method. + * + * @throws Exception + * @return void + */ + function commit(); + + /** + * Rollback the transaction inside the status object. + * + * @return void + */ + function rollBack(); } diff --git a/Transactions/TransactionsRegistry.php b/Transactions/TransactionsRegistry.php index 041d9b9..11057da 100644 --- a/Transactions/TransactionsRegistry.php +++ b/Transactions/TransactionsRegistry.php @@ -23,44 +23,48 @@ * request, however many are supported by this facade. The Transactions passed * here should always be in the same request, not from different requests. */ -class TransactionsRegistry implements TransactionManagerInterface +class TransactionsRegistry { private $container; - private $connectionServices; private $transactions; - public function __construct(ContainerInterface $container, $connectionServices = array()) + public function __construct(ContainerInterface $container) { $this->container = $container; - $this->connectionServices = $connectionServices; } public function getTransaction(TransactionDefinition $definition) { $connectionName = $definition->getConnectionName(); - $status = $this->getTransactionManager($connectionName)->getTransaction($definition); - $this->transactions[spl_object_hash($status)] = $connectionName; - return $status; + if ( ! isset($this->transactions[$connectionName] )) { + $status = $this->getTransactionProvider($connectionName)->createTransaction($definition); + $this->transactions[$connectionName] = $status; + } + + if ($definition->isReadOnly() !== $this->transactions[$connectionName]->isReadOnly()) { + throw new \RuntimeException("Cannot switch from read-only to write/read-transaction or vice-versa."); + } + + $this->transactions[$connectionName]->beginTransaction(); + return $this->transactions[$connectionName]; } public function commit(TransactionStatus $status) { - $connectionName = $this->transactions[spl_object_hash($status)]; - $this->getTransactionManager($connectionName)->commit($status); + $status->commit(); } public function rollBack(TransactionStatus $status) { - $connectionName = $this->transactions[spl_object_hash($status)]; - $this->getTransactionManager($connectionName)->rollBack($status); + $status->rollBack(); } - private function getTransactionManager($name) + private function getTransactionProvider($name) { $id = "simple_things_transactional.tx.".$name; if (!$this->container->has($id)) { throw new \InvalidArgumentException( - "A transactional manager by name of '".$name."' was requested, but does not exist." + "A transactional connection by name of '".$name."' was requested, but does not exist." ); } return $this->container->get($id); From c03fba85fc2efdae1e0a279143644b40ee52b108 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Thu, 26 Jan 2012 21:23:18 +0100 Subject: [PATCH 16/24] Remove txdef from request --- Tests/Functional/EndToEndTest.php | 8 ++++---- Tests/Functional/TransactionalKernelTest.php | 4 ++-- .../Http/HttpTransactionsListener.php | 19 ++++++++----------- 3 files changed, 14 insertions(+), 17 deletions(-) diff --git a/Tests/Functional/EndToEndTest.php b/Tests/Functional/EndToEndTest.php index b356c1a..d613fc6 100644 --- a/Tests/Functional/EndToEndTest.php +++ b/Tests/Functional/EndToEndTest.php @@ -92,8 +92,8 @@ public function testGetRequest() $this->assertEquals(array( "[TransactionBundle] Started transaction for dbal.default", "[TransactionBundle] Started transaction for dbal.default", - "[TransactionBundle] Committed transaction for dbal.default", - "[TransactionBundle] Committed transaction for dbal.default" + "[TransactionBundle] Committed transaction.", + "[TransactionBundle] Committed transaction." ), $this->logger->logs); $this->assertEquals(0, count($this->conn->fetchAll("SELECT * FROM testdata"))); @@ -107,8 +107,8 @@ public function testPostRequest() $this->assertEquals(array( "[TransactionBundle] Started transaction for dbal.default", "[TransactionBundle] Started transaction for dbal.default", - "[TransactionBundle] Committed transaction for dbal.default", - "[TransactionBundle] Committed transaction for dbal.default" + "[TransactionBundle] Committed transaction.", + "[TransactionBundle] Committed transaction." ), $this->logger->logs); $this->assertEquals(2, count($this->conn->fetchAll("SELECT * FROM testdata"))); diff --git a/Tests/Functional/TransactionalKernelTest.php b/Tests/Functional/TransactionalKernelTest.php index cd85383..e79f141 100644 --- a/Tests/Functional/TransactionalKernelTest.php +++ b/Tests/Functional/TransactionalKernelTest.php @@ -64,7 +64,7 @@ public function testGetRequest() $this->assertEquals(array( "[TransactionBundle] Started transaction for dbal.default", - "[TransactionBundle] Committed transaction for dbal.default" + "[TransactionBundle] Committed transaction." ), $this->logger->logs); } @@ -75,7 +75,7 @@ public function testPostRequest() $this->assertEquals(array( "[TransactionBundle] Started transaction for dbal.default", - "[TransactionBundle] Committed transaction for dbal.default" + "[TransactionBundle] Committed transaction." ), $this->logger->logs); } } diff --git a/Transactions/Http/HttpTransactionsListener.php b/Transactions/Http/HttpTransactionsListener.php index cf93213..b1079b6 100644 --- a/Transactions/Http/HttpTransactionsListener.php +++ b/Transactions/Http/HttpTransactionsListener.php @@ -55,7 +55,6 @@ public function onCoreController(FilterControllerEvent $event) $status = $this->registry->getTransaction($definition); $request->attributes->set('_transaction', $status); - $request->attributes->set('_transaction_def', $definition); if ($status && $this->logger) { $this->logger->info("[TransactionBundle] Started transaction for " . $definition->getConnectionName()); @@ -71,12 +70,11 @@ public function onKernelResponse(FilterResponseEvent $event) if ($txStatus === null) { return; } - $txDef = $request->attributes->get('_transaction_def'); if ($response->getStatusCode() >= 400 && $response->getStatusCode() != 404) { - $this->rollBack($txStatus, $txDef); + $this->rollBack($txStatus); } else { - $this->commit($txStatus, $txDef); + $this->commit($txStatus); } } @@ -89,30 +87,29 @@ public function onKernelException(GetResponseForExceptionEvent $event) if ($txStatus === null) { return; } - $txDef = $request->attributes->get('_transaction_def'); if ($ex instanceof NotFoundHttpException) { - $this->registry->commit($txStatus); + $this->commit($txStatus); } else { - $this->registry->rollBack($txStatus); + $this->rollBack($txStatus); } } - private function commit($txStatus, $txDefinition) + private function commit($txStatus) { $this->registry->commit($txStatus); if ($this->logger) { - $this->logger->info("[TransactionBundle] Committed transaction for " . $txDefinition->getConnectionName()); + $this->logger->info("[TransactionBundle] Committed transaction."); } } - private function rollBack($txStatus, $txDefinition) + private function rollBack($txStatus) { $this->registry->rollBack($txStatus); if ($this->logger) { - $this->logger->info("[TransactionBundle] Aborted transaction for " . $txDefinition->getConnectionName()); + $this->logger->info("[TransactionBundle] Aborted transaction."); } } } From 51149fd289c10ca77f1c88750b25c160262a71b4 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Thu, 26 Jan 2012 23:07:46 +0100 Subject: [PATCH 17/24] Further refactoring, cleaned up DI extension, added tests for Container --- .../CompilerPass/DetectConnectionPass.php | 26 +--- .../CompilerPass/TransactionalScopePass.php | 59 -------- Doctrine/DBALTransactionStatus.php | 1 + Doctrine/OrmTransactionManager.php | 42 ------ Doctrine/OrmTransactionProvider.php | 26 ++++ Doctrine/OrmTransactionStatus.php | 132 ++++++++++++++++++ README.markdown | 54 ++++--- Resources/config/services.xml | 14 +- SimpleThingsTransactionalBundle.php | 14 -- Tests/ContainerTest.php | 84 +++++++++++ .../Http/HttpTransactionsListener.php | 2 + Transactions/TransactionsRegistry.php | 1 - 12 files changed, 282 insertions(+), 173 deletions(-) delete mode 100644 DependencyInjection/CompilerPass/TransactionalScopePass.php delete mode 100644 Doctrine/OrmTransactionManager.php create mode 100644 Doctrine/OrmTransactionProvider.php create mode 100644 Doctrine/OrmTransactionStatus.php create mode 100644 Tests/ContainerTest.php diff --git a/DependencyInjection/CompilerPass/DetectConnectionPass.php b/DependencyInjection/CompilerPass/DetectConnectionPass.php index 3223e1f..4c72e13 100644 --- a/DependencyInjection/CompilerPass/DetectConnectionPass.php +++ b/DependencyInjection/CompilerPass/DetectConnectionPass.php @@ -21,57 +21,45 @@ /** * Detects connections and registers the transaction manager services. */ -class DetectConnectionsPass implements CompilerPassInterface +class DetectConnectionPass implements CompilerPassInterface { public function process(ContainerBuilder $builder) { -/* if ($builder->hasParameter('doctrine.connections')) { + if ($builder->hasParameter('doctrine.connections')) { foreach ($builder->getParameter('doctrine.connections') AS $alias => $service) { $builder->setDefinition( 'simple_things_transactional.tx.dbal.'.$alias, - new DefinitionDecorator('simple_things_transactional.manager.dbal') + new DefinitionDecorator('simple_things_transactional.provider.dbal') )->setArguments(array(new Reference($service))); } } -*/ - $connectionServices = array(); if ($builder->hasParameter('doctrine.entity_managers')) { foreach ($builder->getParameter('doctrine.entity_managers') AS $alias => $service) { - $connectionServices[] = 'doctrine.orm.' . $alias . '_entity_manager'; - $builder->setAlias( - 'simple_things_transactional.connections.orm.' . $alias, - 'doctrine.orm.' . $alias . '_entity_manager' - ); $builder->setDefinition( 'simple_things_transactional.tx.orm.'.$alias, - new DefinitionDecorator('simple_things_transactional.manager.orm') + new DefinitionDecorator('simple_things_transactional.provider.orm') )->setArguments(array(new Reference('service_container'))); } } if ($builder->hasParameter('doctrine_couchdb.document_managers')) { foreach ($builder->getParameter('doctrine_couchdb.document_managers') AS $alias => $service) { - $connectionServices[] = 'doctrine_couchdb.odm.' . $alias . '_document_manager'; - $builder->setAlias('simple_things_transactional.connections.couchdb.' . $alias $builder->setDefinition( 'simple_things_transactional.tx.couchdb.'.$alias, - new DefinitionDecorator('simple_things_transactional.manager.object_manager') + new DefinitionDecorator('simple_things_transactional.provider.object_manager') )->setArguments(array(new Reference('service_container'))); } } if ($builder->hasParameter('doctrine_mongodb.document_managers')) { foreach ($builder->getParameter('doctrine_mongodb.document_managers') AS $alias => $service) { - $connectionServices[] = 'doctrine_mongodb.odm.' . $alias . '_document_manager'; - $builder->setAlias('simple_things_transactional.connections.mongodb.' . $alias $builder->setDefinition( 'simple_things_transactional.tx.mongodb.'.$alias, - new DefinitionDecorator('simple_things_transactional.manager.object_manager') + new DefinitionDecorator('simple_things_transactional.provider.object_manager') )->setArguments(array(new Reference('service_container'))); } } - // add tags as well for external resources (Propel, raw-PDO whatever) - $container->setParameter('simple_things_transactional.connection_services', $connectionServices); } } + diff --git a/DependencyInjection/CompilerPass/TransactionalScopePass.php b/DependencyInjection/CompilerPass/TransactionalScopePass.php deleted file mode 100644 index ee055e8..0000000 --- a/DependencyInjection/CompilerPass/TransactionalScopePass.php +++ /dev/null @@ -1,59 +0,0 @@ - - */ -class TransactionalScopePass implements CompilerPassInterface -{ - public function process(ContainerBuilder $container) - { - $connectionServices = $container->getParameter('simple_things_transactional.connection_serices'); - $graph = $container->getCompiler()->getServiceReferenceGraph(); - - $visited = array(); - foreach ($connectionServices as $connectionServiceId) { - $this->changeScopeTransactional($connectionServiceId, $visited); - } - } - - private function changeScopeTransactional($serviceId, $visited) - { - if (isset($visited[$serviceId])) { - return; - } - $visited[$serviceId] = true; - - $node = $graph->getNode($serviceId); - $def = $container->getDefinition($serviceId); - - if ($def->getScope() == ContainerInterface::SCOPE_CONTAINER) { - $def->setScope('transactional'); - } - - foreach ($node->getOutNodes() as $outNode) { - $this->changeScopeTransactional($outNode->getId(), $visited); - } - } -} - diff --git a/Doctrine/DBALTransactionStatus.php b/Doctrine/DBALTransactionStatus.php index 9099f1a..f1051b0 100644 --- a/Doctrine/DBALTransactionStatus.php +++ b/Doctrine/DBALTransactionStatus.php @@ -1,4 +1,5 @@ container->get('simple_things_transactional.connections.' . $def->getConnectionName()); - $manager->beginTransaction(); - return $this->createTxStatus($manager, $def); - } - - protected function doCommit(TransactionStatus $status) - { - $manager = $status->getWrappedConnection(); - $manager->flush(); - $manager->commit(); - - $status->markCompleted(); - } - - protected function doRollBack(TransactionStatus $status) - { - $manager = $status->getWrappedConnection(); - $manager->rollBack(); - - $status->markCompleted(); - } -} - diff --git a/Doctrine/OrmTransactionProvider.php b/Doctrine/OrmTransactionProvider.php new file mode 100644 index 0000000..77d4bfb --- /dev/null +++ b/Doctrine/OrmTransactionProvider.php @@ -0,0 +1,26 @@ +container->get('doctrine.' . $def->getConnectionName().'_entity_manager'); + return new OrmTransactionStatus($manager, $def); + } +} diff --git a/Doctrine/OrmTransactionStatus.php b/Doctrine/OrmTransactionStatus.php new file mode 100644 index 0000000..f8f57b2 --- /dev/null +++ b/Doctrine/OrmTransactionStatus.php @@ -0,0 +1,132 @@ +manager = $manager; + $this->def = $def; + } + + /** + * Checks if the transaction is read-only. + * + * A read-only transaction does not commit changes to the database when + * commit is called. It allows the underlying transaction manager to + * perform optimizations to this regard if possible. + * + * @return bool + */ + public function isReadOnly() + { + return $this->def->isReadOnly(); + } + + /** + * Check if transaction is broken and has to be rolled back at this point. + * + * @return bool + */ + public function isRollBackOnly() + { + return $this->manager->getConnection()->isRollBackOnly(); + } + + /** + * Mark the transaction as rollback-only + * + * @return void + */ + public function setRollBackOnly() + { + return $this->manager->getConnection()->setRollBackOnly(True); + } + + /** + * Check if this transaction was committed already. + * + * @return bool + */ + public function isCompleted() + { + return $this->isCompleted; + } + + /** + * Return the connection object that is wrapped in this status. + * + * @return object + */ + public function getWrappedConnection() + { + return $this->manager; + } + + /** + * Begin the transaction + */ + public function beginTransaction() + { + $this->manager->beginTransaction(); + } + + /** + * Commit the transaction + * + * Depending on the Transaction#isRollBackOnly status this method commits + * or rollbacks the transaction wrapped inside the status. If an error + * happens during commit the original exception of the underlying + * connection is thrown from this method. + * + * @throws Exception + * @return void + */ + public function commit() + { + if ($this->isReadOnly()) { + return $this->rollBack(); + } + + if ( ! $this->isRollBackOnly() && $this->manager->getConnection()->getTransactionNestingLevel() == 1) { + $this->manager->flush(); + } + $this->manager->commit(); + + if ($this->manager->getConnection()->getTransactionNestingLevel() == 0) { + $this->completed = true; + } + } + + /** + * Rollback the transaction inside the status object. + * + * @return void + */ + public function rollBack() + { + $this->manager->rollBack(); + if ($this->manager->getConnection()->getTransactionNestingLevel() == 0) { + $this->completed = true; + } + } +} + diff --git a/README.markdown b/README.markdown index 0fd3f18..36d7d36 100644 --- a/README.markdown +++ b/README.markdown @@ -11,46 +11,48 @@ See at the end of this document. ## Problem Symfony2 allows to nest controllers into each other in unlimited amounts. These controllers can all modify and save -data, probably with different transactional needs. The Doctrine persistence solutions (ORM, MongoDB, CouchDB) use a transactional write-behind +data. The Doctrine persistence solutions (ORM, MongoDB, CouchDB) use a transactional write-behind mechanism to flush changes in batches, best executed at the end of the master request. If each controller or model service handles transactions themselves then you probably overuse the flush operation, which can lead to inconsistencies and performance penalities. -These flushes should not be executed in the model/services but should be handled by the controller layer, because it knows when all operations are done. +Transaction management should by seperated from your domain model, handled by the framework in a HTTP context. ## How it works For every Doctrine DBAL connection, every EntityManager and every DocumentManager the Transactional Bundle -creates a service that implements a transactions manager interface: +creates a service that implements a transactions provider interface: - interface TransactionManagerInterface + interface TransactionProviderInterface { - function getTransaction(TransactionDefinition $def); - function commit(TransactionStatus $status); - function rollBack(TransactionStatus $status); + /** + * @return TransactionStatus + */ + function createTransaction(TransactionDefinition $def); } With the transactional bundle the following workflow is applied to an action that is marked as transactional (by default always if POST, PUT, DELETE, PATCH request is found). -0. Detect which Transaction Manager(s) should wrap the to-be-excecuted action. -1. A transaction is started before the controller is called. -2. The controller execution is wrapped in a try-catch block -3. On successful response generation (status code < 400) the transaction is committed. This includes a call to EntityManager::flush or DocumentManager::flush in case of an orm, mongodb or couchdb "transaction". -4. On status-code >= 400 the transaction is rolled back. -5. If an exception is thrown the transaction is rolled back. +1. Detect which Connection should wrap the to-be-excecuted action and if its read/write or read-only. +2. A transaction is started before the controller is called. +3. The action is called by Symfony +4. On successful response generation (status code < 400) the transaction is committed. This includes a call to EntityManager::flush or DocumentManager::flush in case of an orm, mongodb or couchdb "transaction". +5. On status-code >= 400 the transaction is rolled back. +6. If an exception is thrown the transaction is rolled back. -Each transaction manager is named like the manager it belongs to: +You can mark actions as transactional by means of configuration. There are three different ways to configure the transactional behavior: - doctrine.orm.default_entity_manager => simplethings_tx.orm.default - doctrine_mongodb.odm.default_document_manager => simplethings_tx.mongodb.default - doctrine_couchdb.odm.default_document_manager => simplethings_tx.couchdb.default +### Architectural Details -You can mark actions as transactional by means of configuration. There are three different ways to configure the transactional behavior: +1. There is only exactly one transaction per action. This bundle will not automatically handle transactions across multiple data-source as this is a very implementation specific problem. If you need multiple connections handle one transaction in your application then implement your own transaction provider that implements some kind of two-phase commit. +2. A form extension is provided that will mark a transaction as rollback only validation on the form fails. +3. Transactions are either read/write or read-only. A read-only transaction is rolled back at the end of the request no matter what. In the context of ObjectMAnagers this means the flush operation is NOT called. The read/write or read-only status is detected by matching against the HTTP Verbs. By default GET requests are read-only and PUT, POST, DELETE and PATCH are read/write transactions. +4. If the read-only/read-write status switches during a sub-request an exception is thrown. Modes cannot be mixed. -### Working with a default transaction manager +### Auto-Transactional Mode -If you have a small RESTful application and you only use one transactional manager, for example the Doctrine ORM then your configuration +If you have a RESTful application and you only use one transactional manager, for example the Doctrine ORM then your configuration is as simple as configuring the transactional managers name in the app/config/config.yml extension configuration: simple_things_transactional: @@ -79,14 +81,10 @@ If a transaction is started for a connection multiple times then an exception is fos_user: pattern: "FOS\(.*)Controller::(.*)Action" # not giving conn: uses the default - propagation: REQUIRES_NEW - noRollbackFor: ["NotFoundHttpException"] acme: pattern: "Acme(.*)" - conn: "orm.default" acme_logging: pattern: "Acme\DemoBundle\Controller\IndexController::logAction" - isolation: READ_UNCOMMITTED conn: "orm.other" methods: ["GET"] @@ -110,7 +108,7 @@ The previous `Acme\DemoBundle\Controller\IndexController` can then be configure { public function indexAction() { - + // orm.default transaction here } /** @@ -118,7 +116,7 @@ The previous `Acme\DemoBundle\Controller\IndexController` can then be configure */ public function demoAction() { - // both orm.default and orm.other are transactions here + // orm.other transaction here } } @@ -190,7 +188,3 @@ Here is the code: simple_things_transactional: ~ -## Todos - -* Implement Propagation -* Implement Isolation diff --git a/Resources/config/services.xml b/Resources/config/services.xml index eacc7e9..6482de7 100644 --- a/Resources/config/services.xml +++ b/Resources/config/services.xml @@ -5,19 +5,18 @@ xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd"> - SimpleThings\TransactionalBundle\Transactions\Doctrine\DBALTransactionManager - SimpleThings\TransactionalBundle\Transactions\Doctrine\EntityTransactionManager - SimpleThings\TransactionalBundle\Transactions\Doctrine\ObjectTransactionManager + SimpleThings\TransactionalBundle\Doctrine\DBALTransactionProvider + SimpleThings\TransactionalBundle\Doctrine\OrmTransactionProvider + SimpleThings\TransactionalBundle\Transactions\Doctrine\ObjectTransactionProvider SimpleThings\TransactionalBundle\Transactions\Http\HttpTransactionsListener SimpleThings\TransactionalBundle\Transactions\Http\TransactionalMatcher SimpleThings\TransactionalBundle\Transactions\TransactionsRegistry - - - - + + + @@ -30,7 +29,6 @@ - %simple_things_transactional.connection_services% diff --git a/SimpleThingsTransactionalBundle.php b/SimpleThingsTransactionalBundle.php index 722bd17..5ef2745 100644 --- a/SimpleThingsTransactionalBundle.php +++ b/SimpleThingsTransactionalBundle.php @@ -16,28 +16,14 @@ use Symfony\Component\HttpKernel\Bundle\Bundle; use Symfony\Component\DependencyInjection\ContainerBuilder; -use Symfony\Component\DependencyInjection\Scope; -use SimpleThings\TransactionalBundle\DependencyInjection\CompilerPass\TransactionalScopePass; use SimpleThings\TransactionalBundle\DependencyInjection\CompilerPass\DetectConnectionsPass; class SimpleThingsTransactionalBundle extends Bundle { - public function boot() - { - $this->getContainer()->enterScope('transactional'); - } - public function build(ContainerBuilder $container) { parent::build($container); - $container->addScope(new Scope('transactional')); - $container->addCompilerPass(new TransactionalScopePass()); $container->addCompilerPass(new DetectConnectionsPass()); } - - public function shutdown() - { - $this->getContainer()->leaveScope('transactional'); - } } diff --git a/Tests/ContainerTest.php b/Tests/ContainerTest.php new file mode 100644 index 0000000..93970d0 --- /dev/null +++ b/Tests/ContainerTest.php @@ -0,0 +1,84 @@ +createTestContainer(); + $this->assertInstanceOf('SimpleThings\TransactionalBundle\Transactions\Http\HttpTransactionsListener', $container->get('simple_things_transactional.http_transactions_listener')); + $this->assertInstanceOf('SimpleThings\TransactionalBundle\Transactions\TransactionsRegistry', $container->get('simple_things_transactional.registry')); + $this->assertInstanceOf('SimpleThings\TransactionalBundle\Doctrine\DBALTransactionProvider', $container->get('simple_things_transactional.tx.dbal.default')); + $this->assertInstanceOf('SimpleThings\TransactionalBundle\Doctrine\OrmTransactionProvider', $container->get('simple_things_transactional.tx.orm.default')); + } + + public function createTestContainer() + { + $container = new ContainerBuilder(new ParameterBag(array( + 'kernel.debug' => false, + 'kernel.bundles' => array(), + 'kernel.cache_dir' => sys_get_temp_dir(), + 'kernel.environment' => 'test', + 'kernel.root_dir' => __DIR__ + ))); + $container->set('annotation_reader', new AnnotationReader()); + $loader = new DoctrineExtension(); + $container->registerExtension($loader); + $loader->load(array(array( + 'dbal' => array( + 'connections' => array( + 'default' => array( + 'driver' => 'pdo_mysql', + 'charset' => 'UTF8', + 'platform-service' => 'my.platform', + ) + ), + 'default_connection' => 'default', + 'types' => array( + 'test' => 'Symfony\Bundle\DoctrineBundle\Tests\DependencyInjection\TestType', + ), + ), 'orm' => array( + 'default_entity_manager' => 'default', + 'entity_managers' => array ( + 'default' => array('auto_mapping' => true) + ) + )) + ), $container); + + $container->setDefinition('my.platform', new \Symfony\Component\DependencyInjection\Definition('Doctrine\DBAL\Platforms\MySqlPlatform')); + + $loader = new SimpleThingsTransactionalExtension(); + $container->registerExtension($loader); + $loader->load(array(array('auto_transactional' => true, 'defaults' => array('conn' => 'orm.default'))), $container); + + $container->getCompilerPassConfig()->setOptimizationPasses(array( + new DetectConnectionPass(), + new ResolveDefinitionTemplatesPass() + )); + $container->getCompilerPassConfig()->setRemovingPasses(array()); + $container->compile(); + + return $container; + } +} + diff --git a/Transactions/Http/HttpTransactionsListener.php b/Transactions/Http/HttpTransactionsListener.php index b1079b6..dc96edd 100644 --- a/Transactions/Http/HttpTransactionsListener.php +++ b/Transactions/Http/HttpTransactionsListener.php @@ -54,6 +54,8 @@ public function onCoreController(FilterControllerEvent $event) } $status = $this->registry->getTransaction($definition); + $status->beginTransaction(); + $request->attributes->set('_transaction', $status); if ($status && $this->logger) { diff --git a/Transactions/TransactionsRegistry.php b/Transactions/TransactionsRegistry.php index 11057da..50b988f 100644 --- a/Transactions/TransactionsRegistry.php +++ b/Transactions/TransactionsRegistry.php @@ -45,7 +45,6 @@ public function getTransaction(TransactionDefinition $definition) throw new \RuntimeException("Cannot switch from read-only to write/read-transaction or vice-versa."); } - $this->transactions[$connectionName]->beginTransaction(); return $this->transactions[$connectionName]; } From 94e9a6defc6afb3c9bac5924cd9668049c38cf89 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Thu, 26 Jan 2012 23:21:47 +0100 Subject: [PATCH 18/24] Fix Form Support --- Tests/ContainerTest.php | 2 ++ Transactions/Form/RollbackInvalidFormExtension.php | 5 +++-- Transactions/Form/RollbackInvalidFormValidator.php | 6 ++---- composer.json | 2 +- 4 files changed, 8 insertions(+), 7 deletions(-) diff --git a/Tests/ContainerTest.php b/Tests/ContainerTest.php index 93970d0..50df762 100644 --- a/Tests/ContainerTest.php +++ b/Tests/ContainerTest.php @@ -30,6 +30,8 @@ public function testContainer() $this->assertInstanceOf('SimpleThings\TransactionalBundle\Transactions\TransactionsRegistry', $container->get('simple_things_transactional.registry')); $this->assertInstanceOf('SimpleThings\TransactionalBundle\Doctrine\DBALTransactionProvider', $container->get('simple_things_transactional.tx.dbal.default')); $this->assertInstanceOf('SimpleThings\TransactionalBundle\Doctrine\OrmTransactionProvider', $container->get('simple_things_transactional.tx.orm.default')); + $this->assertInstanceOf('SimpleThings\TransactionalBundle\Transactions\Form\RollbackInvalidFormExtension', $container->get('simple_things_transactional.form_extension')); + $this->assertInstanceOf('SimpleThings\TransactionalBundle\Transactions\Http\TransactionalMatcher', $container->get('simple_things_transactional.transactional_matcher')); } public function createTestContainer() diff --git a/Transactions/Form/RollbackInvalidFormExtension.php b/Transactions/Form/RollbackInvalidFormExtension.php index 18c983d..9e374bd 100644 --- a/Transactions/Form/RollbackInvalidFormExtension.php +++ b/Transactions/Form/RollbackInvalidFormExtension.php @@ -11,12 +11,13 @@ * to kontakt@beberlei.de so I can send you a copy immediately. */ -namespace SimpleThingsTransactionalBundle\Transactions\Form; +namespace SimpleThings\TransactionalBundle\Transactions\Form; use Symfony\Component\Form\AbstractTypeExtension; use Symfony\Component\Form\FormBuilder; +use Symfony\Component\Form\AbstractExtension; -class RollbackInvalidFormExtension extends FormExtension +class RollbackInvalidFormExtension extends AbstractExtension { public function buildForm(FormBuilder $builder, array $options) { diff --git a/Transactions/Form/RollbackInvalidFormValidator.php b/Transactions/Form/RollbackInvalidFormValidator.php index 8a8e393..6cfce63 100644 --- a/Transactions/Form/RollbackInvalidFormValidator.php +++ b/Transactions/Form/RollbackInvalidFormValidator.php @@ -39,10 +39,8 @@ public function validate(FormInterface $form) } $request = $this->container->get('request'); - if (!$form->isValid()) { - foreach ($request->attributes->get('_transactions') as $tx) { - $tx->setRollbackOnly(); - } + if ( ! $form->isValid( && $request->attributes->has('_transaction') ) { + $request->attributes->get('_transaction')->setRollBackOnly(true); } } } diff --git a/composer.json b/composer.json index 922b942..d03fe61 100644 --- a/composer.json +++ b/composer.json @@ -3,7 +3,7 @@ "description": "This bundles provides missing transactional support for Symfony2", "keywords": ["symfony2", "transactions", "persistence"], "type": "symfony-bundle", - "license": "MIT", + "license": "New BSD", "authors": [{ "name" : "Benjamin Eberlei", "email" : "kontakt@beberlei.de" From ccfbc51484bf0b726018e547fbc9450f4a0f4163 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Thu, 26 Jan 2012 23:31:39 +0100 Subject: [PATCH 19/24] Update README --- README.markdown | 35 ++++++++++++++++++++--------------- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/README.markdown b/README.markdown index 36d7d36..c7d6e7c 100644 --- a/README.markdown +++ b/README.markdown @@ -1,27 +1,22 @@ # SimpleThings TransactionalBundle -Wraps calls to controllers into a transaction, be it Doctrine DBAL or Persistence Managers (ORM, MongoDB, CouchDB). -Configuration is done via routing parameters or through a list of controllers/actions configured in the -extension config. +Provides the missing transactions support for Symfony2 on the framework level. The bundle wraps calls to controllers into transactions for either database connections or object managers. The bundle is stricly meant to be run in an HTTP context, using the HTTP verbs for differentating between read/write and read-only transactions. ## Installation See at the end of this document. -## Problem +## Problems to solve -Symfony2 allows to nest controllers into each other in unlimited amounts. These controllers can all modify and save -data. The Doctrine persistence solutions (ORM, MongoDB, CouchDB) use a transactional write-behind -mechanism to flush changes in batches, best executed at the end of the master request. If each controller -or model service handles transactions themselves then you probably overuse the flush operation, which -can lead to inconsistencies and performance penalities. +Symfony2 allows to nest controllers into each other in unlimited amounts. These controllers can all modify and save data. The Doctrine persistence solutions (ORM, MongoDB, CouchDB) use a transactional write-behind mechanism to flush changes in batches, best executed at the end of the master request. If each controller or model service handles transactions themselves then you probably overuse the flush operation, which can lead to inconsistencies and performance penalities. -Transaction management should by seperated from your domain model, handled by the framework in a HTTP context. +Additionally your domain code should not be cluttered with transactional code when its not stricly needed. + +Therefore transaction management should by seperated from your domain model, handled by the framework in a HTTP context. ## How it works -For every Doctrine DBAL connection, every EntityManager and every DocumentManager the Transactional Bundle -creates a service that implements a transactions provider interface: +For every Doctrine DBAL connection, every EntityManager and every DocumentManager the Transactional Bundle creates a service that implements a transactions provider interface: interface TransactionProviderInterface { @@ -31,8 +26,7 @@ creates a service that implements a transactions provider interface: function createTransaction(TransactionDefinition $def); } -With the transactional bundle the following workflow is applied to an action that is marked -as transactional (by default always if POST, PUT, DELETE, PATCH request is found). +With the transactional bundle the following workflow is applied to an action that is marked as transactional 1. Detect which Connection should wrap the to-be-excecuted action and if its read/write or read-only. 2. A transaction is started before the controller is called. @@ -50,6 +44,8 @@ You can mark actions as transactional by means of configuration. There are three 3. Transactions are either read/write or read-only. A read-only transaction is rolled back at the end of the request no matter what. In the context of ObjectMAnagers this means the flush operation is NOT called. The read/write or read-only status is detected by matching against the HTTP Verbs. By default GET requests are read-only and PUT, POST, DELETE and PATCH are read/write transactions. 4. If the read-only/read-write status switches during a sub-request an exception is thrown. Modes cannot be mixed. +## Configuration + ### Auto-Transactional Mode If you have a RESTful application and you only use one transactional manager, for example the Doctrine ORM then your configuration @@ -64,7 +60,7 @@ With this configuration every POST, PUT, DELETE and PATCH request is wrapped ins There is no way to disable this behavior except by throwing an exception. GET requests that need to write a transaction have to do this explicitly. -### Working with explicit configuration +### Controller Pattern Matching If you have an application that is either not RESTful, uses multiple transactional managers or has advanced requirements with regard to transactions then you should configure the transactional behavior explicitly. @@ -161,6 +157,10 @@ Here is the code: ## Installation +On Composer as 'simplethings/transactional-bundle' package. + +Or oldschool: + 1. Add TransactionalBundle to deps: [SimpleThingsTransactionalBundle] @@ -188,3 +188,8 @@ Here is the code: simple_things_transactional: ~ +# Todos + +* Move configuration to Symfony\Component\Config +* Add modes 'autocommit', 'commit_on_success' (default) and 'manual' that determine how an action should be handled transactionally. + From bdaf28f9a24da167ca4b8940f3d0779dbae9d4d1 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Sun, 29 Jan 2012 14:50:24 +0100 Subject: [PATCH 20/24] Test Object/ORM Providers and fix some bugs. --- Doctrine/DBALTransactionProvider.php | 8 +-- Doctrine/ObjectTransactionManager.php | 59 --------------- Doctrine/ObjectTransactionProvider.php | 38 ++++++++++ Doctrine/ObjectTransactionStatus.php | 72 ++++++++++++++----- Doctrine/OrmTransactionProvider.php | 7 ++ Doctrine/OrmTransactionStatus.php | 6 +- .../ObjectTransactionProviderTest.php | 35 +++++++++ Tests/Doctrine/OrmTransactionProviderTest.php | 35 +++++++++ 8 files changed, 173 insertions(+), 87 deletions(-) delete mode 100644 Doctrine/ObjectTransactionManager.php create mode 100644 Doctrine/ObjectTransactionProvider.php create mode 100644 Tests/Doctrine/ObjectTransactionProviderTest.php create mode 100644 Tests/Doctrine/OrmTransactionProviderTest.php diff --git a/Doctrine/DBALTransactionProvider.php b/Doctrine/DBALTransactionProvider.php index 4fec60e..b3cf3e8 100644 --- a/Doctrine/DBALTransactionProvider.php +++ b/Doctrine/DBALTransactionProvider.php @@ -18,13 +18,7 @@ use SimpleThings\TransactionalBundle\Transactions\TransactionProviderInterface; /** - * Doctrine Object TransactionManager for any Doctrine ObjectManager. - * - * The commit operation of this manager directly translates to the flush - * operation of the Object Manager, synchronizing any pending changes to the - * database. Creating a new transaction when one already exists translates to - * resetting the service and reconstituting the "previous" manager when the - * transaction is committed or rolled back. + * Doctrine DBAL Transaction Provider */ class DBALTransactionProvider implements TransactionProviderInterface { diff --git a/Doctrine/ObjectTransactionManager.php b/Doctrine/ObjectTransactionManager.php deleted file mode 100644 index a92b180..0000000 --- a/Doctrine/ObjectTransactionManager.php +++ /dev/null @@ -1,59 +0,0 @@ -container = $container; - } - - protected function createTxStatus($manager, $def) - { - return new ObjectTransactionStatus($manager, $def); - } - - protected function doBeginTransaction(TransactionDefinition $def) - { - $manager = $this->container->get('simple_things_transactional.connections.' . $def->getConnectionName()); - return $this->createTxStatus($manager, $def); - } - - protected function doCommit(TransactionStatus $status) - { - $manager = $status->getWrappedConnection(); - $manager->flush(); - $status->markCompleted(); - } - - protected function doRollBack(TransactionStatus $status) - { - $status->markCompleted(); - } -} - diff --git a/Doctrine/ObjectTransactionProvider.php b/Doctrine/ObjectTransactionProvider.php new file mode 100644 index 0000000..296cf0e --- /dev/null +++ b/Doctrine/ObjectTransactionProvider.php @@ -0,0 +1,38 @@ +container = $container; + } + + public function createTransaction(TransactionDefinition $def) + { + $conn = $this->container->get('doctrine.' . $def->getConnectionName() . '_manager'); + return new ObjectTransactionStatus($conn, $def); + } +} + diff --git a/Doctrine/ObjectTransactionStatus.php b/Doctrine/ObjectTransactionStatus.php index 6bb3e13..0823e6c 100644 --- a/Doctrine/ObjectTransactionStatus.php +++ b/Doctrine/ObjectTransactionStatus.php @@ -11,7 +11,7 @@ * to kontakt@beberlei.de so I can send you a copy immediately. */ -namespace SimpleThingsTransactionalBundle\Doctrine; +namespace SimpleThings\TransactionalBundle\Doctrine; use SimpleThings\TransactionalBundle\Transactions\TransactionStatus; use SimpleThings\TransactionalBundle\Transactions\TransactionDefinition; @@ -21,8 +21,9 @@ class ObjectTransactionStatus implements TransactionStatus { private $def; private $manager; - private $rollbackOnly = false; + private $rollBackOnly = false; private $completed = false; + private $nestingLevel = 0; public function __construct(ObjectManager $manager, TransactionDefinition $def) { @@ -30,11 +31,6 @@ public function __construct(ObjectManager $manager, TransactionDefinition $def) $this->def = $def; } - public function getIsolationLevel() - { - return $this->def->getIsolationLevel(); - } - /** * Checks if the transaction is read-only. * @@ -46,7 +42,7 @@ public function getIsolationLevel() */ public function isReadOnly() { - return $this->def->getReadOnly(); + return $this->def->isReadOnly(); } /** @@ -56,7 +52,7 @@ public function isReadOnly() */ public function isRollBackOnly() { - return $this->rollbackOnly; + return $this->rollBackOnly; } /** @@ -66,7 +62,7 @@ public function isRollBackOnly() */ public function setRollBackOnly() { - $this->rollbackOnly = true; + $this->rollBackOnly = true; } /** @@ -76,27 +72,65 @@ public function setRollBackOnly() */ public function isCompleted() { - return $this->completed; + return $this->isCompleted; } /** - * Check if this transaction has savepoints. + * Return the connection object that is wrapped in this status. * - * @return bool + * @return object */ - public function hasSavepoint() + public function getWrappedConnection() { - return false; + return $this->manager; } - public function getWrappedConnection() + /** + * Begin the transaction + */ + public function beginTransaction() { - return $this->manager; + $this->nestingLevel++; + } + + /** + * Commit the transaction + * + * Depending on the Transaction#isRollBackOnly status this method commits + * or rollbacks the transaction wrapped inside the status. If an error + * happens during commit the original exception of the underlying + * connection is thrown from this method. + * + * @throws Exception + * @return void + */ + public function commit() + { + if ( ! $this->isReadOnly() && ! $this->isRollBackOnly() && $this->nestingLevel === 1) { + $this->manager->flush(); + } + $this->decreateNestingLevel(); } - public function markCompleted() + private function decreateNestingLevel() + { + $this->nestingLevel--; + + if ($this->nestingLevel === 0) { + $this->completed = true; + } + } + + /** + * Rollback the transaction inside the status object. + * + * @return void + */ + public function rollBack() { - $this->completed = true; + $this->manager->clear(); + $this->rollBackOnly = true; + $this->decreateNestingLevel(); } } diff --git a/Doctrine/OrmTransactionProvider.php b/Doctrine/OrmTransactionProvider.php index 77d4bfb..8a9ff4e 100644 --- a/Doctrine/OrmTransactionProvider.php +++ b/Doctrine/OrmTransactionProvider.php @@ -18,6 +18,13 @@ class OrmTransactionProvider implements TransactionProviderInterface { + protected $container; + + public function __construct($container) + { + $this->container = $container; + } + public function createTransaction(TransactionDefinition $def) { $manager = $this->container->get('doctrine.' . $def->getConnectionName().'_entity_manager'); diff --git a/Doctrine/OrmTransactionStatus.php b/Doctrine/OrmTransactionStatus.php index f8f57b2..f64f9b7 100644 --- a/Doctrine/OrmTransactionStatus.php +++ b/Doctrine/OrmTransactionStatus.php @@ -11,8 +11,10 @@ * to kontakt@beberlei.de so I can send you a copy immediately. */ -namespace SimpleThingsTransactionalBundle\Doctrine; +namespace SimpleThings\TransactionalBundle\Doctrine; +use SimpleThings\TransactionalBundle\Transactions\TransactionStatus; +use SimpleThings\TransactionalBundle\Transactions\TransactionDefinition; use Doctrine\ORM\EntityManager; class OrmTransactionStatus implements TransactionStatus @@ -21,7 +23,7 @@ class OrmTransactionStatus implements TransactionStatus private $manager; private $completed = false; - public function __construct(EntityManager $conn, TransactionDefinition $def) + public function __construct(EntityManager $manager, TransactionDefinition $def) { $this->manager = $manager; $this->def = $def; diff --git a/Tests/Doctrine/ObjectTransactionProviderTest.php b/Tests/Doctrine/ObjectTransactionProviderTest.php new file mode 100644 index 0000000..9a77b53 --- /dev/null +++ b/Tests/Doctrine/ObjectTransactionProviderTest.php @@ -0,0 +1,35 @@ +getMock('Symfony\Component\DependencyInjection\ContainerInterface'); + $container->expects($this->once()) + ->method('get') + ->with($this->equalTo('doctrine.orm.default_entity_manager')) + ->will($this->returnValue($this->getMock('Doctrine\Common\Persistence\ObjectManager'))); + + $provider = new ObjectTransactionProvider($container); + $status = $provider->createTransaction(new TransactionDefinition('orm.default_entity')); + + $this->assertInstanceOf('SimpleThings\TransactionalBundle\Doctrine\ObjectTransactionStatus', $status); + } +} + diff --git a/Tests/Doctrine/OrmTransactionProviderTest.php b/Tests/Doctrine/OrmTransactionProviderTest.php new file mode 100644 index 0000000..0c03391 --- /dev/null +++ b/Tests/Doctrine/OrmTransactionProviderTest.php @@ -0,0 +1,35 @@ +getMock('Symfony\Component\DependencyInjection\ContainerInterface'); + $container->expects($this->once()) + ->method('get') + ->with($this->equalTo('doctrine.orm.default_entity_manager')) + ->will($this->returnValue($this->getMock('Doctrine\ORM\EntityManager', array(), array(), '', false))); + + $provider = new OrmTransactionProvider($container); + $status = $provider->createTransaction(new TransactionDefinition('orm.default')); + + $this->assertInstanceOf('SimpleThings\TransactionalBundle\Doctrine\OrmTransactionStatus', $status); + } +} + From e5b8a293509d8a63f7b9a47b7138cc91404a6a28 Mon Sep 17 00:00:00 2001 From: Deni Date: Tue, 20 Mar 2012 11:19:22 +0400 Subject: [PATCH 21/24] Fix use statements --- DependencyInjection/SimpleThingsTransactionalExtension.php | 1 + SimpleThingsTransactionalBundle.php | 4 ++-- Transactions/Form/RollbackInvalidFormValidator.php | 6 +++--- Transactions/Http/HttpTransactionsListener.php | 2 +- 4 files changed, 7 insertions(+), 6 deletions(-) diff --git a/DependencyInjection/SimpleThingsTransactionalExtension.php b/DependencyInjection/SimpleThingsTransactionalExtension.php index 4a4d3f7..065707c 100644 --- a/DependencyInjection/SimpleThingsTransactionalExtension.php +++ b/DependencyInjection/SimpleThingsTransactionalExtension.php @@ -15,6 +15,7 @@ namespace SimpleThings\TransactionalBundle\DependencyInjection; use Symfony\Component\DependencyInjection\ContainerBuilder; +use Symfony\Component\DependencyInjection\Reference; use Symfony\Component\DependencyInjection\Loader\XmlFileLoader; use Symfony\Component\HttpKernel\DependencyInjection\Extension; use Symfony\Component\Config\FileLocator; diff --git a/SimpleThingsTransactionalBundle.php b/SimpleThingsTransactionalBundle.php index 5ef2745..6480d47 100644 --- a/SimpleThingsTransactionalBundle.php +++ b/SimpleThingsTransactionalBundle.php @@ -16,7 +16,7 @@ use Symfony\Component\HttpKernel\Bundle\Bundle; use Symfony\Component\DependencyInjection\ContainerBuilder; -use SimpleThings\TransactionalBundle\DependencyInjection\CompilerPass\DetectConnectionsPass; +use SimpleThings\TransactionalBundle\DependencyInjection\CompilerPass\DetectConnectionPass; class SimpleThingsTransactionalBundle extends Bundle { @@ -24,6 +24,6 @@ public function build(ContainerBuilder $container) { parent::build($container); - $container->addCompilerPass(new DetectConnectionsPass()); + $container->addCompilerPass(new DetectConnectionPass()); } } diff --git a/Transactions/Form/RollbackInvalidFormValidator.php b/Transactions/Form/RollbackInvalidFormValidator.php index 6cfce63..1cfda71 100644 --- a/Transactions/Form/RollbackInvalidFormValidator.php +++ b/Transactions/Form/RollbackInvalidFormValidator.php @@ -11,10 +11,10 @@ * to kontakt@beberlei.de so I can send you a copy immediately. */ -namespace SimpleThingsTransactionalBundle\Transactions\Form; +namespace SimpleThings\TransactionalBundle\Transactions\Form; use Symfony\Component\Form\FormValidatorInterface; -use Symfony\Component\Form\Form; +use Symfony\Component\Form\FormInterface; /** * "Missusing" the FormValidator to set transactions to rollback only when the validation failed. @@ -39,7 +39,7 @@ public function validate(FormInterface $form) } $request = $this->container->get('request'); - if ( ! $form->isValid( && $request->attributes->has('_transaction') ) { + if ( ! $form->isValid() && $request->attributes->has('_transaction') ) { $request->attributes->get('_transaction')->setRollBackOnly(true); } } diff --git a/Transactions/Http/HttpTransactionsListener.php b/Transactions/Http/HttpTransactionsListener.php index dc96edd..00b2ce2 100644 --- a/Transactions/Http/HttpTransactionsListener.php +++ b/Transactions/Http/HttpTransactionsListener.php @@ -19,7 +19,7 @@ use Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent; use Symfony\Component\HttpKernel\HttpKernelInterface; use Symfony\Component\HttpKernel\Log\LoggerInterface; -use Symfony\Component\HttpFoundation\Exceptions\NotFoundHttpException; +use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; use SimpleThings\TransactionalBundle\Transactions\TransactionsRegistry; /** From 4c036a3a6cc410cc2ce5e1ce095194201bf2d9c7 Mon Sep 17 00:00:00 2001 From: Deni Date: Tue, 20 Mar 2012 12:28:40 +0400 Subject: [PATCH 22/24] Extracted dependency of the validator, inject it through the constructor of the form extension --- Resources/config/services.xml | 5 +++++ Transactions/Form/RollbackInvalidFormExtension.php | 12 +++++++++--- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/Resources/config/services.xml b/Resources/config/services.xml index 6482de7..36f5f8d 100644 --- a/Resources/config/services.xml +++ b/Resources/config/services.xml @@ -35,7 +35,12 @@ + + + + + diff --git a/Transactions/Form/RollbackInvalidFormExtension.php b/Transactions/Form/RollbackInvalidFormExtension.php index 9e374bd..05a5317 100644 --- a/Transactions/Form/RollbackInvalidFormExtension.php +++ b/Transactions/Form/RollbackInvalidFormExtension.php @@ -15,13 +15,19 @@ use Symfony\Component\Form\AbstractTypeExtension; use Symfony\Component\Form\FormBuilder; -use Symfony\Component\Form\AbstractExtension; -class RollbackInvalidFormExtension extends AbstractExtension +class RollbackInvalidFormExtension extends AbstractTypeExtension { + private $validator; + + public function __construct(RollbackInvalidFormValidator $rollbackValidator) + { + $this->validator = $rollbackValidator; + } + public function buildForm(FormBuilder $builder, array $options) { - $builder->addValidator(new RollbackInvalidFormValidator()); + $builder->addValidator($this->validator); } public function getExtendedType() From ce97a4b40aa94eff51e2322544daafd0fbc3ba0b Mon Sep 17 00:00:00 2001 From: Deni Date: Tue, 20 Mar 2012 14:06:12 +0400 Subject: [PATCH 23/24] Fixes some typo --- Doctrine/ObjectTransactionStatus.php | 2 +- Doctrine/OrmTransactionStatus.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Doctrine/ObjectTransactionStatus.php b/Doctrine/ObjectTransactionStatus.php index 0823e6c..9af5c97 100644 --- a/Doctrine/ObjectTransactionStatus.php +++ b/Doctrine/ObjectTransactionStatus.php @@ -72,7 +72,7 @@ public function setRollBackOnly() */ public function isCompleted() { - return $this->isCompleted; + return $this->completed; } /** diff --git a/Doctrine/OrmTransactionStatus.php b/Doctrine/OrmTransactionStatus.php index f64f9b7..e2ec78f 100644 --- a/Doctrine/OrmTransactionStatus.php +++ b/Doctrine/OrmTransactionStatus.php @@ -70,7 +70,7 @@ public function setRollBackOnly() */ public function isCompleted() { - return $this->isCompleted; + return $this->completed; } /** From 2e27d913bf9d077ecbf137cc2d332414679d19dc Mon Sep 17 00:00:00 2001 From: Deni Date: Thu, 22 Mar 2012 12:08:37 +0400 Subject: [PATCH 24/24] Fixed commit ORM transaction --- Doctrine/OrmTransactionStatus.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Doctrine/OrmTransactionStatus.php b/Doctrine/OrmTransactionStatus.php index e2ec78f..d0e3fe1 100644 --- a/Doctrine/OrmTransactionStatus.php +++ b/Doctrine/OrmTransactionStatus.php @@ -110,8 +110,8 @@ public function commit() if ( ! $this->isRollBackOnly() && $this->manager->getConnection()->getTransactionNestingLevel() == 1) { $this->manager->flush(); + $this->manager->getConnection()->commit(); } - $this->manager->commit(); if ($this->manager->getConnection()->getTransactionNestingLevel() == 0) { $this->completed = true;