Transaction API helper #368

Closed
staabm opened this Issue May 5, 2013 · 16 comments

Projects

None yet

6 participants

@staabm
Member
staabm commented May 5, 2013

as proposed in propelorm/Propel#673 (comment), I think we should introduce a helper method to the Connection/Transaction API to simpify transaction+exception handling.

QUOTE:

I tried to add a Helper-Method to the PropelPDO which encapsulates the exception+transaction handling, but this will only be usefull in case anonymous function are available (which is not in propel1). This technique is inspired by Doctrines EntityManager::transactional(), https://github.com/doctrine/doctrine2/blob/master/lib/Doctrine/ORM/EntityManager.php#L218.

    /**
     * Executes the given callable within a transaction.
     * This helper method takes care to commit or rollback the transaction.
     * This <code>PropelPDO</code> instance is passed as a first parameter to the callable.
     *
     * @param $callable A callable to be wrapped in a transaction
     * @return bool|mixed Returns the result of the callable on success, or <code>true</code> when the callable doesn't return anything.
     * @throws Exception Re-throws a possible <code>Exception</code> triggered by the callable.
     */
    public function transaction($callable) {
        if (!is_callable($callable)) {
            throw new PropelException('Expects parameter to be a callable');
        }

        $this->beginTransaction();

        try {
            $result = call_user_func($callable, $this);

            $this->commit();

            if ($result) {
                return $result;
            }
            return true;
        } catch (Exception $e) {
            $this->rollBack();

            throw $e;
        }
    }

this would transform the client-code from e.g.

    $con->beginTransaction();
    try {
        // archive all results one by one
        foreach ($criteria->find($con) as $object) {
            $object->archive($con);
            $totalArchivedObjects++;
        }
        $con->commit();
    } catch (Exception $e) {
        $con->rollBack();
        throw $e;
    }

to

    $con->transaction(function() use ($criteria, $totalArchivedObjects){
        // archive all results one by one
        foreach ($criteria->find($con) as $object) {
            $object->archive($con);
            $totalArchivedObjects++;
        }
    });
@willdurand
Member

I am 👍 on this

@hhamon
Member
hhamon commented May 11, 2013

👍

@staabm
Member
staabm commented May 11, 2013

Please assign it to me, I will do it.

@staabm staabm referenced this issue May 11, 2013
Closed

Transaction leak #365

@staabm staabm was assigned May 12, 2013
@mhitza
Contributor
mhitza commented May 13, 2013

Maybe instead of using Exceptions for control flow, have the provided anonymous function return a boolean value if the transaction should be committed or not?

Also you should be able to use the callable typehint that is available with PHP 5.4

@staabm
Member
staabm commented May 13, 2013

Having a real return value allows chaining the result of the transactions and handling commit/rollback using exceptions makes sure that that we don't leak things and thats what this helper is all about.

I will use the typehint though.

@staabm staabm added a commit to staabm/Propel2 that referenced this issue May 16, 2013
@staabm staabm added new transaction API on ConnectionInterface and its implementors…
…. refs #368.
6fc9b6f
@staabm staabm referenced this issue May 16, 2013
Closed

Transactionapi #396

@staabm staabm added a commit to staabm/Propel2 that referenced this issue May 16, 2013
@staabm staabm added new transaction API on ConnectionInterface and its implementors…
…. refs #368.
0b16987
@staabm staabm added a commit to staabm/Propel2 that referenced this issue May 17, 2013
@staabm staabm added new transaction API on ConnectionInterface and its implementors…
…. refs #368.
96852d1
@willdurand
Member

so what is the status of this issue?

@staabm
Member
staabm commented May 18, 2013

@willdurand in case you like to pick the commits pushed in #396 (commits) it would work like in the other PRs which you already merged.

Sorry for the mixup in these PRs.. I will create a new fork after all PRs are merged, so I can provide clean PRs again.

@staabm staabm added a commit to staabm/Propel2 that referenced this issue May 18, 2013
@staabm @staabm staabm + staabm added new transaction API on ConnectionInterface and its implementors…
…. refs #368.
a58d3e5
@staabm staabm referenced this issue May 18, 2013
Merged

Transaction API #401

@staabm
Member
staabm commented May 18, 2013

i will continue my work in #401

@staabm staabm added a commit to staabm/Propel2 that referenced this issue May 18, 2013
@staabm @staabm staabm + staabm added new transaction API on ConnectionInterface and its implementors…
…. refs #368.

added initial tests.
faf93f1
@staabm staabm added a commit to staabm/Propel2 that referenced this issue May 18, 2013
@staabm @staabm staabm + staabm added new transaction API on ConnectionInterface and its implementors…
…. refs #368.

added initial tests.
3382141
@staabm staabm added a commit to staabm/Propel2 that referenced this issue May 18, 2013
@staabm @staabm staabm + staabm added new transaction API on ConnectionInterface and its implementors…
…. refs #368.

added initial tests.
5269e3b
@staabm staabm added a commit to staabm/Propel2 that referenced this issue May 18, 2013
@staabm @staabm staabm + staabm added new transaction API on ConnectionInterface and its implementors…
…. refs #368.

added initial tests.
afe08e4
@staabm staabm added a commit to staabm/Propel2 that referenced this issue May 18, 2013
@staabm @staabm staabm + staabm added new transaction API on ConnectionInterface and its implementors…
…. refs #368.

added initial tests.
adc8f07
@staabm staabm added a commit to staabm/Propel2 that referenced this issue May 18, 2013
@staabm @staabm staabm + staabm added new transaction API on ConnectionInterface and its implementors…
…. refs #368.

added initial tests.
d4a6022
@staabm staabm added a commit to staabm/Propel2 that referenced this issue May 29, 2013
@staabm staabm added new transaction API on ConnectionInterface and its implementors…
…. refs #368.

added initial tests.
7fa6544
@staabm staabm added a commit to staabm/Propel2 that referenced this issue May 29, 2013
@staabm staabm added new transaction API on ConnectionInterface and its implementors…
…. refs #368.

added initial tests.
fcca505
@staabm staabm added a commit to staabm/Propel2 that referenced this issue May 29, 2013
@staabm staabm added new transaction API on ConnectionInterface and its implementors…
…. refs #368.

added initial tests.
72b9505
@staabm staabm added a commit to staabm/Propel2 that referenced this issue May 29, 2013
@staabm staabm added new transaction API on ConnectionInterface and its implementors…
…. refs #368.

added initial tests.
3e8e53e
@willdurand
Member

Scheduled for alpha-2

@mhitza
Contributor
mhitza commented Jun 5, 2013

@staabm you've added the Closure typehint to the transaction method, whereas I've suggested the callable typehint.

With Closure you can't pass an array/string to be called. E.g

<?php

function X(callable $a) {
  $a();
}

function Y(Closure $a) {
  $a();
}

function toCall() {
  echo "called\n";
}

X("toCall");
Y("toCall");
@willdurand
Member

Re-scheduled for alpha3. What do we need here?

@willdurand
Member

@staabm do you think that you can ship this feature for alpha3?

@staabm
Member
staabm commented Oct 28, 2013

atm my time is rather limited, therefore not sure if I can get it ready...

@marcj
Member
marcj commented Jan 27, 2014

@staabm any news here?

@gharlan
Contributor
gharlan commented Mar 15, 2014

I'm working on the internal usage..

@staabm
Member
staabm commented Mar 15, 2014

@gharlan you might just continue my work from https://github.com/staabm/Propel2/commits/transactionapi

There should be everything already done, after the recently discussed issue in #401 has been fixed

@marcj marcj closed this in #578 Mar 17, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment