(POC) Add `hook_civicrm_postCommit`, a variant of `hook_civicrm_post`

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);
```

diff --git a/CRM/Core/Transaction.php b/CRM/Core/Transaction.php
index e23e9f4315eed0dcad62b44a4a4a6f4bcc45d04a..d42197f9de76adf4012fab4e3007b91d2d1ee52c 100644 (file)
--- a/CRM/Core/Transaction.php
+++ b/CRM/Core/Transaction.php
@@ -252,4 +252,24 @@ class CRM_Core_Transaction {
     $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);
+    }
+  }
+
 }

diff --git a/CRM/Utils/Hook.php b/CRM/Utils/Hook.php
index 76887f2bc3cba8bb4ab9908f63b7c4cf6c739db6..e0e7a9eec13bca8aa5781202243264b81d194d90 100644 (file)
--- a/CRM/Utils/Hook.php
+++ b/CRM/Utils/Hook.php
@@ -393,6 +393,36 @@ abstract class CRM_Utils_Hook {
     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

diff --git a/Civi/Core/Container.php b/Civi/Core/Container.php
index 0dfebb73ed9564e5496eacb80361c903e08a4475..c9232136f0a474d7bb6c37ec84a77435718d2fa2 100644 (file)
--- a/Civi/Core/Container.php
+++ b/Civi/Core/Container.php
@@ -335,6 +335,7 @@ class 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']);