Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dev/core#1460 - CiviEventDispatcher - Add policy options #17127

Merged
merged 1 commit into from
Apr 28, 2020

Conversation

totten
Copy link
Member

@totten totten commented Apr 21, 2020

Overview

This updates the CiviEventDispatcher with a method setDispatchPolicy() that can be used to dynamically toggle support for specific events/hooks.

This is intended for use in upgrades and debugging - not for general runtime use. It aims to support https://lab.civicrm.org/dev/core/issues/1460 and is an excerpt from #17126.

Before

Any call to Civi::dispatcher()->dispatch('x'...) will unconditionally dispatch the event x.

After

Under normal conditions, the dispatch-policy is NULL. Any call to Civi::dispatcher()->dispatch('x'...) will dispatch the event x.

Under special conditions (upgrading or debugging), you may set a dispatch-policy to specify which events will be supported.

Technical Details

The setDispatchPolicy() function takes an array ([string $eventNameRegex => string $action]) To see how this array works, let's consider an example policy:

[
      /*1*/ 'hook_civicrm_config' => 'warn',
      /*2*/ '/^hook_civicrm_/'    => 'drop',
      /*3*/ '/^civi\./'           => 'run',
]

Now what happens as different events are run?

Civi::dispatcher()->dispatch('civi.dao.preDelete', ...);
// ^^ Matches #3. This event will run normally.

Civi::dispatcher()->dispatch('hook_civicrm_config', ...);
// ^^ Matches #1. This event will run, but it will log a warning.

Civi::dispatcher()->dispatch('hook_civicrm_alterMailer', ...);
// ^^ Matches #2. This event will be dropped/ignored. No listeners are actually called.

@civibot
Copy link

civibot bot commented Apr 21, 2020

(Standard links)

@totten
Copy link
Member Author

totten commented Apr 22, 2020

Note: there isn't a whole lot to r-run in isolation (since this really just lays groundwork for #17126), but there is unit-test coverage, and you can experiment with it a bit using a throw-away script - eg cv scr /tmp/dispatch.php with a file:

<?php
// FILE: /tmp/dispatch.php
Civi::dispatcher()->addListener('hook_civicrm_pre', function($e){
  print_r(['rx', $e->action, $e->entity]);
});

Civi::dispatcher()->setDispatchPolicy([
  '/^hook_civicrm_pre/' => 'run',
//  '/^hook_civicrm_pre/' => 'warn',
//  '/^hook_civicrm_pre/' => 'fail',
//  '/^hook_civicrm_pre/' => 'drop',
  '/^civi\./' => 'run',
]);

$obj = new stdClass();
\CRM_Utils_Hook::pre('edit', 'Foo', 123, $obj);

@eileenmcnaughton
Copy link
Contributor

I'm going to merge this - it seems to 'do what it says on the packet' and has a test.

I do have minor concerns around style - and more specifically we are introducing new functions without 'summary lines' or without a line after the summary line - which I understand as being part of the standard (and I think code sniffer would if we let it do full drupal standards) https://www.drupal.org/docs/develop/standards/api-documentation-and-comment-standards

and I understand that the full standard is comment blocks for all functions - I know that is a bit debated & it's only missing for tests but as far as i understand it the standard is just 'a comment block for all functions' and by the same note it can be hard to figure out what testes ARE testing.

@totten
Copy link
Member Author

totten commented May 1, 2020

@eileenmcnaughton based on your comment about docblocks, I included some updates in #17216.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants