1

I am implementing a ReviewableBehavior to implement a four eyes principle. The behavior implements beforeDelete(), beforeSave() and afterSave() and uses a reviews table to store CUD requests.

  1. for added records, a $review record is created and saved in afterSave() (because only the we have the id of the newly added record, which we need to store in the $review)
  2. for edited records, in beforeSave() the values that have been changed are saved in a $review record, in the edited record these field values are set back to their original values (so basically no changes are saved)
  3. for deleted records, in beforeDelete() a $review is saved to store the delete request, and false is returned in order to cancel the delete

Number 3. is the challenge, because although $review always had a correctly set primary key value as if the save was actually successful, and save($review) returned true as if everything went well, it was not actually saved in the database.

The reason, as far as I understand: Deletions are by default done within a transaction. The transaction is started in delete() of the table, then beforeDelete() of the behavior is triggered. With in the event handler, I call ReviewTable->save($review). As a transaction has been started, this save() happens within the transaction. Then I return false, because I want the deletion to be stopped. This rolls back the transaction and with it the ReviewTable->save($review).

Solution attempts:

  • If I do not return false, the $review is saved in the database, but the "main" record is also deleted. Disadvantage: Not a feasible approach as record is deleted which we do not want.
  • If I call delete($entity, ['atomic' => false]); then there is no transaction started, hence ReviewTable->save($review) is executed. Disadvantage: For any model that uses this behavior, we would need to amend every call to delete() to switch the atomic of. Also this switches the use of transactions off, which does not appear a good approach to me.
  • "Overwrite" delete method in ReviewableBehavior, so for any table using this behavior, when delete() is called, actually the delete() of the _ReviewableBehavior_is performed. Disadvantage: technically not possible to overwrite table methods with a behavior.
    • Create a new table class, overwrite delete() method in it, and derive any table using the ReviewableBehavior from the table class. Disadvantage: ugly approach having to use both the behavior and a new table class.
    • Create a new method deleteRequest() in the ReviewableBehavior, and instead of calling Table->delete() we call Table->deleteRequest(). In it, we can save the delete request in a $review record, deletion is not done anyway as we did not actually call delete(). Disdavantage: For any model that uses this behavior, we would need to change every call to delete() into deleteRequest()

Currently I go with the last approach, but I would really like to hear some opinions about this, and also whether there is any better method to somehow keep the transaction, but saving something "in between".

ndm
  • 59,784
  • 9
  • 71
  • 110
Christian Kirchhoff
  • 285
  • 1
  • 5
  • 15

1 Answers1

2

Using a separate method is a sensible approach. Another option might be to circumvent the deletion process by stopping the Model.beforeDelete event, when doing so you can return true to indicate a successful delete operation, ie no rollback will happen.

It should be noted that stopping the event will cause possible other listeners in the queue to not be notified! Also halting the regular deletion process will prevent cascading deletes (ie deleting associated records), and the Model.afterDelete event, so if you need cascading deletes, then you'd need to trigger them manually, and the afterDelete event should usually be triggered for successful deletes in any case.

Here's a quick and dirty example, see also \Cake\ORM\Table::_processDelete() for insight on how the regular deletion process works:

public function beforeDelete(
    \Cake\Event\Event $event,
    \Cake\Datasource\EntityInterface $entity,
    \ArrayObject $options
) {
    // this will prevent the regular deletion process
    $event->stopPropagation();

    $table = $event->getSubject();

    // this deletes associated records
    $table->associations()->cascadeDelete(
        $entity,
        ['_primary' => false] + $options->getArrayCopy()
    );

    $result = /* create review record */;
    if (!$result) {
        // this will cause a rollback
        return false;
    }

    $table->dispatchEvent('Model.afterDelete', [
        'entity' => $entity,
        'options' => $options,
    ]);

    return true;
}

See also

ndm
  • 59,784
  • 9
  • 71
  • 110