(POC) Add `hook_civicrm_postCommit`, a variant of `hook_civicrm_post`
authorTim Otten <totten@civicrm.org>
Fri, 20 Sep 2019 21:46:13 +0000 (14:46 -0700)
committerTim Otten <totten@civicrm.org>
Fri, 20 Sep 2019 22:04:01 +0000 (15:04 -0700)
commit74effac4f391e5daa8d0d43eecbe9f1350b75dfc
tree996bf44933c9a99c6b1e8280c13a3d0ab323a5c9
parenta1c8e9601f64f65bd3f2a56e47bb49f1d35d90ef
(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);
```
CRM/Core/Transaction.php
CRM/Utils/Hook.php
Civi/Core/Container.php