Skip to content

Commit

Permalink
(POC) Add hook_civicrm_postCommit, a variant of hook_civicrm_post
Browse files Browse the repository at this point in the history
Overview
--------

So here's a pattern that's occurred a few times: one wants to provide an
extra notification or log or correlated-record after something is chanaged
or created.  So you implement `hook_civicrm_post`.  It sounds simple, but it
doesn't work quite as expected - because running within the transaction can
have some special implications:

1. Performing subsequent SQL queries within the same transaction be wonky
2. Errors in your notification bubble-up and break the transaction.

You can resolve this by deferring your work until the transaction completes. The
technique has been discussed in various media over the years; e.g. here's a mention in the dev docs:

https://docs.civicrm.org/dev/en/latest/hooks/hook_civicrm_post/#example-with-transaction-callback

However, I think the problem may be that the default is basically backwards: there are more use-cases
for 'post' hook in which you prefer to run *after the transaction commits*, but doing that case requires
the special incantation.

This patch is a proof-of-concept in which the system provides two hooks:

* `hook_civicrm_post`: Runs immediately after the change is *sent* to the DB.
  If there's a SQL transaction, then it runs *within* the transaction.
* `hook_civicrm_postCommit`: Runs after the change is *committed* to the DB.
  Runs outside of any SQL transactions.

My theory is that more developers would run their logic at the correct time
if they had `postCommit` available as a hook (and, of course, the downstream
code would look tidier).  This isn't really a pain-point for me, so I'm not
super-motivated to push it, and I haven't looked very hard for systemic
side-effects of buffering more posts.  However, I think it could provide
better DX (making it easier for more folks to get the right timing), so I
wanted to share the POC.

Before
------

```php
/**
 * Hook fired after the INSERT/UPDATE is sent to the DB
 */
function hook_civicrm_post($op, $objectName, $objectId, &$objectRef)
```

After
------

```php
/**
 * Hook fired after the INSERT/UPDATE is sent to the DB
 */
function hook_civicrm_post($op, $objectName, $objectId, &$objectRef);

/**
 * Hook fired after the record is committed to the DB.
 * This may be immediate (for non-transactional work) or it may be
 * delayed a few milliseconds (for transactional work).
 */
function hook_civicrm_postCommit($op, $objectName, $objectId, $objectRef);
```
  • Loading branch information
totten committed Sep 20, 2019
1 parent a1c8e96 commit 74effac
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 0 deletions.
20 changes: 20 additions & 0 deletions CRM/Core/Transaction.php
Original file line number Diff line number Diff line change
Expand Up @@ -252,4 +252,24 @@ public static function addCallback($phase, $callback, $params = NULL, $id = NULL
$frame->addCallback($phase, $callback, $params, $id);
}

/**
* Whenever hook_civicrm_post fires, schedule an equivalent
* call to hook_civicrm_postCommit.
*
* @param \Civi\Core\Event\PostEvent $e
* @see CRM_Utils_Hook::post
*/
public static function addPostCommit($e) {
// Do we want to dedupe post-commit hooks for the same txn? Setting an ID
// would allow this.
// $id = $e->entity . chr(0) . $e->action . chr(0) . $e->id;
$frame = \Civi\Core\Transaction\Manager::singleton()->getBaseFrame();
if ($frame) {
$frame->addCallback(self::PHASE_POST_COMMIT, ['CRM_Utils_Hook', 'postCommit'], [$e->action, $e->entity, $e->id, $e->object]);
}
else {
\CRM_Utils_Hook::postCommit($e->action, $e->entity, $e->id, $e->object);
}
}

}
30 changes: 30 additions & 0 deletions CRM/Utils/Hook.php
Original file line number Diff line number Diff line change
Expand Up @@ -393,6 +393,36 @@ public static function post($op, $objectName, $objectId, &$objectRef = NULL) {
return $event->getReturnValues();
}

/**
* This hook is equivalent to post(), except that it is guaranteed to run
* outside of any SQL transaction. The objectRef is not modifiable.
*
* This hook is defined for two cases:
*
* 1. If the original action runs within a transaction, then the hook fires
* after the transaction commits.
* 2. If the original action runs outside a transaction, then the data was
* committed immediately, and we can run the hook immediately.
*
* @param string $op
* The type of operation being performed.
* @param string $objectName
* The name of the object.
* @param int $objectId
* The unique identifier for the object.
* @param object $objectRef
* The reference to the object if available.
*
* @return mixed
* based on op. pre-hooks return a boolean or
* an error message which aborts the operation
*/
public static function postCommit($op, $objectName, $objectId, $objectRef = NULL) {
$event = new \Civi\Core\Event\PostEvent($op, $objectName, $objectId, $objectRef);
\Civi::dispatcher()->dispatch('hook_civicrm_postCommit', $event);
return $event->getReturnValues();
}

/**
* This hook retrieves links from other modules and injects it into.
* the view contact tabs
Expand Down
1 change: 1 addition & 0 deletions Civi/Core/Container.php
Original file line number Diff line number Diff line change
Expand Up @@ -335,6 +335,7 @@ public function createEventDispatcher($container) {
$dispatcher->addListener(SystemInstallEvent::EVENT_NAME, ['\Civi\Core\InstallationCanary', 'check']);
$dispatcher->addListener(SystemInstallEvent::EVENT_NAME, ['\Civi\Core\DatabaseInitializer', 'initialize']);
$dispatcher->addListener(SystemInstallEvent::EVENT_NAME, ['\Civi\Core\LocalizationInitializer', 'initialize']);
$dispatcher->addListener('hook_civicrm_post', ['\CRM_Core_Transaction', 'addPostCommit'], -1000);
$dispatcher->addListener('hook_civicrm_pre', ['\Civi\Core\Event\PreEvent', 'dispatchSubevent'], 100);
$dispatcher->addListener('hook_civicrm_post', ['\Civi\Core\Event\PostEvent', 'dispatchSubevent'], 100);
$dispatcher->addListener('hook_civicrm_post::Activity', ['\Civi\CCase\Events', 'fireCaseChange']);
Expand Down

0 comments on commit 74effac

Please sign in to comment.