Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

provide transactional interface for EntityManager and Connection #171

Closed
wants to merge 4 commits into from

5 participants

@dfreudenberger

No description provided.

Daniel Freud... added some commits
@travisbot

This pull request fails (merged 6da6b7f into 00ac50c).

Daniel Freudenberger fixed missing , 5ef5a37
@travisbot

This pull request fails (merged 6ae5658 into 00ac50c).

@travisbot

This pull request fails (merged 5ef5a37 into 00ac50c).

lib/Doctrine/DBAL/Transactional.php
((19 lines not shown))
+ * <http://www.doctrine-project.org>.
+*/
+
+namespace Doctrine\DBAL;
+
+/**
+ * Interface for transactional aware classes
+ *
+ * @license http://www.opensource.org/licenses/lgpl-license.php LGPL
+ * @link www.doctrine-project.com
+ * @version $Revision$
+ * @author Daniel Freudenberger <d.freudenberger@rebuy.de>
+ */
+interface Transactional
+{
+ public function transactional(\Closure $func);
@stof
stof added a note

If we add an interface, I don't think it should typehint \Closure but allow any callable instead (without typehinting them as it is only possible on 5.4). I don't see why other callables should be forbidden.
Removing teh typehint is possible in the Connection class, but it should be done before adding an interface as changing the typehint in an interface is a BC break.

@beberlei thoughts ?

@beberlei Owner

I am +1 on removing \Closure and make a docblock hint for "callable"

i totally agree with you. i'm just wondering why you typehint \Closure in EntityManager and Connection? Is there a good reason why a concrete class should be more specific than an interface?

@stof
stof added a note

@dfreudenberger because of a crappy decision at the beginning (we have the same issue in collections). It can be changed in the EntityManager and the Connection (and will be needed if the interface enforces no typehint) but cannot be changed for collections without a BC break unfortunately.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
lib/Doctrine/DBAL/Transactional.php
((14 lines not shown))
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ *
+ * This software consists of voluntary contributions made by many individuals
+ * and is licensed under the MIT license. For more information, see
+ * <http://www.doctrine-project.org>.
+*/
+
+namespace Doctrine\DBAL;
+
+/**
+ * Interface for transactional aware classes
+ *
+ * @license http://www.opensource.org/licenses/lgpl-license.php LGPL
+ * @link www.doctrine-project.com
+ * @version $Revision$
@stof
stof added a note

this should be removed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
lib/Doctrine/DBAL/Transactional.php
((12 lines not shown))
+ * 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.
+ *
+ * This software consists of voluntary contributions made by many individuals
+ * and is licensed under the MIT license. For more information, see
+ * <http://www.doctrine-project.org>.
+*/
+
+namespace Doctrine\DBAL;
+
+/**
+ * Interface for transactional aware classes
+ *
+ * @license http://www.opensource.org/licenses/lgpl-license.php LGPL
@stof
stof added a note

this is wrong. The code is MIT

actually i copied the license and the $id$ part from Doctrine\DBAL\LockMode - but sure i'll fix it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
lib/Doctrine/DBAL/Transactional.php
@@ -0,0 +1,35 @@
+<?php
+/*
+ * $Id$
@stof
stof added a note

this should be removed. It was used in the SVN area

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Daniel Freudenberger droped typehint from signature
Doctrine\DBAL\Connection::transactional() throws exception for invalid argument
added unit test
345ba96
@travisbot

This pull request fails (merged 345ba96 into 00ac50c).

@dfreudenberger

do you need anything else from me? not sure what the current status is

@beberlei beberlei commented on the diff
lib/Doctrine/DBAL/Transactional.php
((19 lines not shown))
+
+namespace Doctrine\DBAL;
+
+/**
+ * Interface for transactional aware classes
+ *
+ * @link www.doctrine-project.com
+ * @author Daniel Freudenberger <d.freudenberger@rebuy.de>
+ */
+interface Transactional
+{
+ /**
+ * wraps the provided callable in a transaction.
+ *
+ * @param Callable $func
+ * @return void
@beberlei Owner

The original methods didnt return void or? At least the EntityManager has a return value here. This should be replicated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@beberlei
Owner

You need to rebase the PR onto master again, its not mergable anymore.

@ramonornela

This interface should be to Common.

@beberlei
Owner

I don't think this adds value over not having an interface. There are very seldom situations where you need this API independent of the actual underyling connection.

@beberlei beberlei closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jul 12, 2012
  1. added new interface Transactional (should be implemented by EntityMan…

    Daniel Freudenberger authored
    …ager and Connection)
  2. Connection should implement the new interface

    Daniel Freudenberger authored
  3. fixed missing ,

    Daniel Freudenberger authored
  4. droped typehint from signature

    Daniel Freudenberger authored
    Doctrine\DBAL\Connection::transactional() throws exception for invalid argument
    added unit test
This page is out of date. Refresh to see the latest.
View
5 UPGRADE
@@ -1,5 +1,10 @@
# Upgrade to 2.3
+## Doctrine\DBAL\Connection#transactional() changed signature
+
+To match the signature provided by the new Doctrine\DBAL\Transactional interface, the \Closure typehint was dropped.
+In case you have classes overriding Doctrine\DBAL\Connection#transactional() you have to drop the typehint as well.
+
## Doctrine\DBAL\Connection#setCharsetSQL() removed
This method only worked on MySQL and it is considered unsafe on MySQL to use SET NAMES UTF-8 instead
View
15 lib/Doctrine/DBAL/Connection.php
@@ -27,14 +27,15 @@
Doctrine\DBAL\Cache\ResultCacheStatement,
Doctrine\DBAL\Cache\QueryCacheProfile,
Doctrine\DBAL\Cache\ArrayStatement,
- Doctrine\DBAL\Cache\CacheException;
+ Doctrine\DBAL\Cache\CacheException,
+ Doctrine\DBAL\Transactional;
/**
* A wrapper around a Doctrine\DBAL\Driver\Connection that adds features like
* events, transaction isolation levels, configuration, emulated transaction nesting,
* lazy connecting and more.
*
- *
+ *
* @link www.doctrine-project.org
* @since 2.0
* @author Guilherme Blanco <guilhermeblanco@hotmail.com>
@@ -44,7 +45,7 @@
* @author Lukas Smith <smith@pooteeweet.org> (MDB2 library)
* @author Benjamin Eberlei <kontakt@beberlei.de>
*/
-class Connection implements DriverConnection
+class Connection implements DriverConnection, Transactional
{
/**
* Constant for transaction isolation level READ UNCOMMITTED.
@@ -863,10 +864,14 @@ public function lastInsertId($seqName = null)
* If an exception occurs during execution of the function or transaction commit,
* the transaction is rolled back and the exception re-thrown.
*
- * @param Closure $func The function to execute transactionally.
+ * @param Callable $func The function to execute transactionally.
*/
- public function transactional(Closure $func)
+ public function transactional($func)
{
+ if (!is_callable($func)) {
+ throw ConnectionException::invalidCallback('func');
+ }
+
$this->beginTransaction();
try {
$func($this);
View
10 lib/Doctrine/DBAL/DBALException.php
@@ -92,4 +92,14 @@ public static function typeNotFound($name)
{
return new self('Type to be overwritten '.$name.' does not exist.');
}
+
+ /**
+ * @param string $name
+ * @return DBALException
+ */
+ public static function invalidCallback($name)
+ {
+ $message = sprintf('method expects parameter %s to be callable.', $name);
+ return new self($message);
+ }
}
View
37 lib/Doctrine/DBAL/Transactional.php
@@ -0,0 +1,37 @@
+<?php
+/*
+ * 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
+ * OWNER 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.
+ *
+ * This software consists of voluntary contributions made by many individuals
+ * and is licensed under the MIT license. For more information, see
+ * <http://www.doctrine-project.org>.
+*/
+
+namespace Doctrine\DBAL;
+
+/**
+ * Interface for transactional aware classes
+ *
+ * @link www.doctrine-project.com
+ * @author Daniel Freudenberger <d.freudenberger@rebuy.de>
+ */
+interface Transactional
+{
+ /**
+ * wraps the provided callable in a transaction.
+ *
+ * @param Callable $func
+ * @return void
@beberlei Owner

The original methods didnt return void or? At least the EntityManager has a return value here. This should be replicated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ */
+ public function transactional($func);
+}
View
14 tests/Doctrine/Tests/DBAL/Functional/ConnectionTest.php
@@ -4,6 +4,7 @@
use Doctrine\DBAL\ConnectionException;
use Doctrine\DBAL\Types\Type;
+use Doctrine\DBAL\DBALException;
require_once __DIR__ . '/../../TestInit.php';
@@ -209,6 +210,19 @@ public function testTransactional()
});
}
+ public function testTransactionalThrowsExceptionForInvalidCallback()
+ {
+ $message = null;
+ try {
+ $this->_conn->transactional('invalid callback');
+ } catch (DBALException $ex) {
+ $message = $ex->getMessage();
+ }
+
+ $expectedMessage = DBALException::invalidCallback('func')->getMessage();
+ $this->assertSame($expectedMessage, $message);
+ }
+
/**
* Tests that the quote function accepts DBAL and PDO types.
*/
Something went wrong with that request. Please try again.