[2011-09-07] DB transaction support for common CMS actions

[silverstripe-issues]

created by: @chillu (ischommer) created at: 2011-09-07 original ticket: http://open.silverstripe.org/ticket/6714

---

We should wrap database operations which logically span more than one write call into a transaction. Note that even a single DataObject->write() call usually causes more than one DB INSERT (e.g. through Versioned extension, writing relationships etc.). This applies to most CMS actions, for example:

• LeftAndMain->save(), ajaxupdateparent(), ajaxupdatesort()
• AssetAdmin->savefile(), deletemarked()
• SecurityAdmin->memberimport()

These methods should catch common errors/exceptions, initiate a transaction rollback and respond gracefully to the user. Transaction support can be enabled even if database/table doesn't have support for it (ignored silently).

More care has to be taken for actions affecting the filesystem, e.g. Filesystem::sync() or AssetAdmin::doUpload(). Try to roll back data at the right level of granularity (e.g. only delete related DB row if a file can't be written to filesystem).

[chillu]

Confirmed this is still the case in 4.0. Related to https://github.com/silverstripe/silverstripe-graphql/issues/75

[th3hamm0r]

As we are evaluating an upgrade to 4.x, I wanted to know if there are any near plans for this issue?

Atm we are using a thin wrapper around GridFieldDetailForm_ItemRequest's doSave/doDelete which covers most of the CMS-related DB operations in 3.x. We've also added additional callbacks (onAfterWriteCommit and onAfterDeleteCommit), since external services/processes won't see those changes, if called synchronously in onAfterWrite/onAfterDelete. For now I think this should work for us in 4.x too, but the coverage will be lower due to the fewer usages of the GridField (eg. assets admin).

[chillu]

It's priority is slightly higher now that we have campaigns in 4.x, which rely on more writes to be performed consistently together. Good point about the separate callbacks for the commit phase. I think this makes introduction of transactions an API change: You can't rely on onAfterWrite to actually have the desired effect. For example, if you were refreshing a collection cache containing this object as part of a query, you'd still use the old version. How do you keep track of all the objects which take part in a transaction on an ORM level so you can call onAfterWriteCommit on them? Care to share some code?

[th3hamm0r]

Actually our solution is more a quick-fix and far away from a solid solution to be integrated into the SilverStripe Framework/CMS ;) We just wrapped the write operations in doSave/doDelete in GridFieldDetailForm_ItemRequest with an additional try-catch-block. In the doSave-method this looks like this:

$this->record->invokeWithExtensions('onTransactionStart');
DB::get_conn()->transactionStart();
try {
    $form->saveInto($this->record);
    // This avoids two writes for DataObjects in a HasManyList (eg. ContentParts), since add() also calls write() for those lists.
    if (!($list instanceof HasManyList)) {
        $this->record->write();
    }
    $extraData = $this->getExtraSavedData($this->record, $list);
    $list->add($this->record, $extraData);

    DB::get_conn()->transactionEnd();
    $this->record->invokeWithExtensions('onTransactionEnd');
} catch (Exception $e) {
    DB::get_conn()->transactionRollback();
    $this->record->invokeWithExtensions('onTransactionRollback');
    throw $e;
}

The following DataExtension is added to DataObject:

class TransactionalGridFieldExtension extends DataExtension {

    private $_inTransaction = false;

    public function onTransactionStart() {
        $this->_inTransaction = true;
    }

    public function onAfterWrite() {
        $this->maybeInvokeOnAfterWriteCommit();
    }

    public function onAfterDelete() {
        $this->maybeInvokeOnAfterDeleteCommit();
    }

    public function onTransactionEnd() {
        $this->_inTransaction = false;
        $this->maybeInvokeOnAfterWriteCommit();
    }

    public function onTransactionRollback() {
        $this->_inTransaction = false;
    }

    /**
     * Only invokes onAfterWriteCommit() if write() is called outside a transaction, or the transaction has been committed.
     */
    private function maybeInvokeOnAfterWriteCommit() {
        if (!$this->_inTransaction) {
            $this->owner->invokeWithExtensions('onAfterWriteCommit');
        }
    }

    /**
     * Only invokes onAfterDeleteCommit() if delete() is called outside a transaction, or the transaction has been committed.
     */
    private function maybeInvokeOnAfterDeleteCommit() {
        if (!$this->_inTransaction) {
            $this->owner->invokeWithExtensions('onAfterDeleteCommit');
        }
    }
}

So this is very simple and only covers the current record handled by the GridField, and therefore only triggers our custom events there.

A solid solution for the framework is of course not that easy. I even think that the framework can only provide some additional helpers to manage transactions (like it has been implemented in django, https://docs.djangoproject.com/en/2.0/topics/db/transactions/) and the CMS should use them in its save/write actions.

[tractorcow]

In 4.0 we do this for campaign publishing.

DB::get_conn()->withTransaction(function () {
    foreach ($this->Changes() as $change) {
        /** @var ChangeSetItem $change */
        $change->publish();
    }
    // ...
}

All we need to do is shift this pattern into the various CMS save handler tada.