Unverified Commit 20429eb9 authored by Rich Lott's avatar Rich Lott Committed by GitHub

Scheduled jobs belonging to extensions cannot be disabled (#16430)

Authored-by: Rich's avatarRich Lott / Artful Robot <forums@artfulrobot.uk>
parent 0f4c9fab
......@@ -251,7 +251,7 @@ class CRM_Core_ManagedEntities {
}
/**
* Update an entity which (a) is believed to exist and which (b) ought to be active.
* Update an entity which is believed to exist.
*
* @param CRM_Core_DAO_Managed $dao
* @param array $todo
......@@ -262,12 +262,26 @@ class CRM_Core_ManagedEntities {
$doUpdate = ($policy == 'always');
if ($doUpdate) {
$defaults = [
'id' => $dao->entity_id,
// FIXME: test whether is_active is valid
'is_active' => 1,
];
$defaults = ['id' => $dao->entity_id, 'is_active' => 1];
$params = array_merge($defaults, $todo['params']);
$manager = CRM_Extension_System::singleton()->getManager();
if ($dao->entity_type === 'Job' && !$manager->extensionIsBeingInstalledOrEnabled($dao->module)) {
// Special treatment for scheduled jobs:
//
// If we're being called as part of enabling/installing a module then
// we want the default behaviour of setting is_active = 1.
//
// However, if we're just being called by a normal cache flush then we
// should not re-enable a job that an administrator has decided to disable.
//
// Without this logic there was a problem: site admin might disable
// a job, but then when there was a flush op, the job was re-enabled
// which can cause significant embarrassment, depending on the job
// ("Don't worry, sending mailings is disabled right now...").
unset($params['is_active']);
}
$result = civicrm_api($dao->entity_type, 'create', $params);
if ($result['is_error']) {
$this->onApiError($dao->entity_type, 'create', $params, $result);
......
......@@ -13,6 +13,10 @@
* The extension manager handles installing, disabling enabling, and
* uninstalling extensions.
*
* You should obtain a singleton of this class via
*
* $manager = CRM_Extension_Manager::singleton()->getManager();
*
* @package CRM
* @copyright CiviCRM LLC https://civicrm.org/licensing
*/
......@@ -94,6 +98,34 @@ class CRM_Extension_Manager {
*/
public $statuses;
/**
* Live process(es) per extension.
*
* @var array
*
* Format is: {
* extensionKey => [
* ['operation' => 'install|enable|uninstall|disable', 'phase' => 'queued|live|completed'
* ...
* ],
* ...
* }
*
* The inner array is a stack, so the most recent current operation is the
* last entry. As this manager handles multiple extensions at once, here's
* the flow for an install operation.
*
* $manager->install(['ext1', 'ext2']);
*
* 0. {}
* 1. { ext1: ['install'], ext2: ['install'] }
* 2. { ext1: ['install', 'installing'], ext2: ['install'] }
* 3. { ext1: ['install'], ext2: ['install', 'installing'] }
* 4. { ext1: ['install'], ext2: ['install'] }
* 5. {}
*/
protected $processes = [];
/**
* Class constructor.
*
......@@ -201,9 +233,10 @@ class CRM_Extension_Manager {
*
* @param string|array $keys
* One or more extension keys.
* @param string $mode install|enable
* @throws CRM_Extension_Exception
*/
public function install($keys) {
public function install($keys, $mode = 'install') {
$keys = (array) $keys;
$origStatuses = $this->getStatuses();
......@@ -221,6 +254,9 @@ class CRM_Extension_Manager {
throw new CRM_Extension_Exception('Cannot install incompatible extension: ' . implode(', ', $incompatible));
}
// Keep state for these operations.
$this->addProcess($keys, $mode);
foreach ($keys as $key) {
/** @var CRM_Extension_Info $info */
/** @var CRM_Extension_Manager_Base $typeManager */
......@@ -228,11 +264,17 @@ class CRM_Extension_Manager {
switch ($origStatuses[$key]) {
case self::STATUS_INSTALLED:
// ok, nothing to do
// ok, nothing to do. As such the status of this process is no longer
// 'install' install was the intent, which might have resulted in
// changes but these changes will not be happening, so processes that
// are sensitive to installs (like the managed entities reconcile
// operation) should not assume that these changes have happened.
$this->popProcess([$key]);
break;
case self::STATUS_DISABLED:
// re-enable it
$this->addProcess([$key], 'enabling');
$typeManager->onPreEnable($info);
$this->_setExtensionActive($info, 1);
$typeManager->onPostEnable($info);
......@@ -241,10 +283,13 @@ class CRM_Extension_Manager {
// later extensions to access classes from earlier extensions.
$this->statuses = NULL;
$this->mapper->refresh();
$this->popProcess([$key]);
break;
case self::STATUS_UNINSTALLED:
// install anew
$this->addProcess([$key], 'installing');
$typeManager->onPreInstall($info);
$this->_createExtensionEntry($info);
$typeManager->onPostInstall($info);
......@@ -253,6 +298,8 @@ class CRM_Extension_Manager {
// later extensions to access classes from earlier extensions.
$this->statuses = NULL;
$this->mapper->refresh();
$this->popProcess([$key]);
break;
case self::STATUS_UNKNOWN:
......@@ -264,6 +311,7 @@ class CRM_Extension_Manager {
$this->statuses = NULL;
$this->mapper->refresh();
CRM_Core_Invoke::rebuildMenuAndCaches(TRUE);
$schema = new CRM_Logging_Schema();
$schema->fixSchemaDifferences();
......@@ -282,7 +330,9 @@ class CRM_Extension_Manager {
case self::STATUS_UNINSTALLED:
// install anew
$this->addProcess([$key], 'installing');
$typeManager->onPostPostInstall($info);
$this->popProcess([$key]);
break;
case self::STATUS_UNKNOWN:
......@@ -291,6 +341,8 @@ class CRM_Extension_Manager {
}
}
// All processes for these keys
$this->popProcess($keys);
}
/**
......@@ -301,7 +353,7 @@ class CRM_Extension_Manager {
* @throws CRM_Extension_Exception
*/
public function enable($keys) {
$this->install($keys);
$this->install($keys, 'enable');
}
/**
......@@ -326,14 +378,18 @@ class CRM_Extension_Manager {
throw new CRM_Extension_Exception_DependencyException("Cannot disable extension due to dependencies. Consider disabling all these: " . implode(',', $disableRequirements));
}
$this->addProcess($keys, 'disable');
foreach ($keys as $key) {
switch ($origStatuses[$key]) {
case self::STATUS_INSTALLED:
$this->addProcess([$key], 'disabling');
// throws Exception
list ($info, $typeManager) = $this->_getInfoTypeHandler($key);
$typeManager->onPreDisable($info);
$this->_setExtensionActive($info, 0);
$typeManager->onPostDisable($info);
$this->popProcess([$key]);
break;
case self::STATUS_INSTALLED_MISSING:
......@@ -348,6 +404,8 @@ class CRM_Extension_Manager {
case self::STATUS_DISABLED_MISSING:
case self::STATUS_UNINSTALLED:
// ok, nothing to do
// Remove the 'disable' process as we're not doing that.
$this->popProcess([$key]);
break;
case self::STATUS_UNKNOWN:
......@@ -359,6 +417,8 @@ class CRM_Extension_Manager {
$this->statuses = NULL;
$this->mapper->refresh();
CRM_Core_Invoke::rebuildMenuAndCaches(TRUE);
$this->popProcess($keys);
}
/**
......@@ -375,6 +435,8 @@ class CRM_Extension_Manager {
// TODO: to mitigate the risk of crashing during installation, scan
// keys/statuses/types before doing anything
$this->addProcess($keys, 'uninstall');
foreach ($keys as $key) {
switch ($origStatuses[$key]) {
case self::STATUS_INSTALLED:
......@@ -382,6 +444,7 @@ class CRM_Extension_Manager {
throw new CRM_Extension_Exception("Cannot uninstall extension; disable it first: $key");
case self::STATUS_DISABLED:
$this->addProcess([$key], 'uninstalling');
// throws Exception
list ($info, $typeManager) = $this->_getInfoTypeHandler($key);
$typeManager->onPreUninstall($info);
......@@ -399,6 +462,8 @@ class CRM_Extension_Manager {
case self::STATUS_UNINSTALLED:
// ok, nothing to do
// remove the 'uninstall' process since we're not doing that.
$this->popProcess([$key]);
break;
case self::STATUS_UNKNOWN:
......@@ -410,6 +475,7 @@ class CRM_Extension_Manager {
$this->statuses = NULL;
$this->mapper->refresh();
CRM_Core_Invoke::rebuildMenuAndCaches(TRUE);
$this->popProcess($keys);
}
/**
......@@ -491,6 +557,34 @@ class CRM_Extension_Manager {
$this->mapper->refresh();
}
/**
* Return current processes for given extension.
*
* @param String $key extension key
*
* @return array
*/
public function getActiveProcesses(string $key) :Array {
return $this->processes[$key] ?? [];
}
/**
* Determine if the extension specified is currently involved in an install
* or enable process. Just sugar code to make things more readable.
*
* @param String $key extension key
*
* @return bool
*/
public function extensionIsBeingInstalledOrEnabled($key) :bool {
foreach ($this->getActiveProcesses($key) as $process) {
if (in_array($process, ['install', 'installing', 'enable', 'enabling'])) {
return TRUE;
}
}
return FALSE;
}
// ----------------------
/**
......@@ -711,6 +805,15 @@ class CRM_Extension_Manager {
return $sorter->sort();
}
/**
* Provides way to set processes property for phpunit tests - not for general use.
*
* @param $processes
*/
public function setProcessesForTesting(array $processes) {
$this->processes = $processes;
}
/**
* @param $infos
* @param $filterStatuses
......@@ -726,4 +829,29 @@ class CRM_Extension_Manager {
return $matches;
}
/**
* Add a process to the stacks for the extensions.
*
* @param array $keys extensionKey
* @param string $process one of: install|uninstall|enable|disable|installing|uninstalling|enabling|disabling
*/
protected function addProcess(array $keys, string $process) {
foreach ($keys as $key) {
$this->processes[$key][] = $process;
}
}
/**
* Pop the top op from the stacks for the extensions.
*
* @param array $keys extensionKey
*/
protected function popProcess(array $keys) {
foreach ($keys as $key) {
if (!empty($this->process[$key])) {
array_pop($this->process[$key]);
}
}
}
}
......@@ -78,6 +78,20 @@ class CRM_Core_ManagedEntitiesTest extends CiviUnitTestCase {
],
];
$this->fixtures['com.example.one-Job'] = [
'module' => 'com.example.one',
'name' => 'Job',
'entity' => 'Job',
'params' => [
'version' => 3,
'name' => 'test_job',
'run_frequency' => 'Daily',
'api_entity' => 'Job',
'api_action' => 'Get',
'parameters' => '',
],
];
$this->apiKernel = \Civi::service('civi_api_kernel');
$this->adhocProvider = new \Civi\API\Provider\AdhocProvider(3, 'CustomSearch');
$this->apiKernel->registerApiProvider($this->adhocProvider);
......@@ -361,9 +375,13 @@ class CRM_Core_ManagedEntitiesTest extends CiviUnitTestCase {
* module
*/
public function testDeactivateReactivateModule() {
$manager = CRM_Extension_System::singleton()->getManager();
// create first managed entity ('foo')
$decls = [];
$decls[] = $this->fixtures['com.example.one-foo'];
// Mock the contextual process info that would be added by CRM_Extension_Manager::install
$manager->setProcessesForTesting(['com.example.one' => ['install']]);
$me = new CRM_Core_ManagedEntities($this->modules, $decls);
$me->reconcile();
$foo = $me->get('com.example.one', 'foo');
......@@ -373,6 +391,8 @@ class CRM_Core_ManagedEntitiesTest extends CiviUnitTestCase {
// now deactivate module, which has empty decls and which cascades to managed object
$this->modules['one']->is_active = FALSE;
// Mock the contextual process info that would be added by CRM_Extension_Manager::disable
$manager->setProcessesForTesting(['com.example.one' => ['disable']]);
$me = new CRM_Core_ManagedEntities($this->modules, []);
$me->reconcile();
$foo = $me->get('com.example.one', 'foo');
......@@ -382,12 +402,86 @@ class CRM_Core_ManagedEntitiesTest extends CiviUnitTestCase {
// and reactivate module, which again provides decls and which cascades to managed object
$this->modules['one']->is_active = TRUE;
// Mock the contextual process info that would be added by CRM_Extension_Manager::enable
$manager->setProcessesForTesting(['com.example.one' => ['enable']]);
$me = new CRM_Core_ManagedEntities($this->modules, $decls);
$me->reconcile();
$foo = $me->get('com.example.one', 'foo');
$this->assertEquals(1, $foo['is_active']);
$this->assertEquals('CRM_Example_One_Foo', $foo['name']);
$this->assertDBQuery(1, 'SELECT is_active FROM civicrm_option_value WHERE name = "CRM_Example_One_Foo"');
// Special case: Job entities.
//
// First we repeat the above steps, but adding the context that
// CRM_Extension_Manager adds when installing/enabling extensions.
//
// The behaviour should be as above.
$decls = [$this->fixtures['com.example.one-Job']];
// Mock the contextual process info that would be added by CRM_Extension_Manager::install
$manager->setProcessesForTesting(['com.example.one' => ['install']]);
$me = new CRM_Core_ManagedEntities($this->modules, $decls);
$me->reconcile();
$job = $me->get('com.example.one', 'Job');
$this->assertEquals(1, $job['is_active']);
$this->assertEquals('test_job', $job['name']);
$this->assertDBQuery(1, 'SELECT is_active FROM civicrm_job WHERE name = "test_job"');
// Reset context.
$manager->setProcessesForTesting([]);
// now deactivate module, which has empty decls and which cascades to managed object
$this->modules['one']->is_active = FALSE;
// Mock the contextual process info that would be added by CRM_Extension_Manager::disable
$manager->setProcessesForTesting(['com.example.one' => ['disable']]);
$me = new CRM_Core_ManagedEntities($this->modules, []);
$me->reconcile();
$job = $me->get('com.example.one', 'Job');
$this->assertEquals(0, $job['is_active']);
$this->assertEquals('test_job', $job['name']);
$this->assertDBQuery(0, 'SELECT is_active FROM civicrm_job WHERE name = "test_job"');
// and reactivate module, which again provides decls and which cascades to managed object
$this->modules['one']->is_active = TRUE;
$me = new CRM_Core_ManagedEntities($this->modules, $decls);
// Mock the contextual process info that would be added by CRM_Extension_Manager::enable
$manager->setProcessesForTesting(['com.example.one' => ['enable']]);
$me->reconcile();
$job = $me->get('com.example.one', 'Job');
$this->assertEquals(1, $job['is_active']);
$this->assertEquals('test_job', $job['name']);
$this->assertDBQuery(1, 'SELECT is_active FROM civicrm_job WHERE name = "test_job"');
// Currently: module enabled, job enabled.
// Test that if we now manually disable the job, calling reconcile in a
// normal flush situation does NOT re-enable it.
// ... manually disable job.
$this->callAPISuccess('Job', 'create', ['id' => $job['id'], 'is_active' => 0]);
// ... now call reconcile in the context of a normal flush operation.
// Mock the contextual process info - there would not be any
$manager->setProcessesForTesting([]);
$me = new CRM_Core_ManagedEntities($this->modules, $decls);
$me->reconcile();
$job = $me->get('com.example.one', 'Job');
$this->assertEquals(0, $job['is_active'], "Job that was manually set inactive should not have been set active again, but it was.");
$this->assertDBQuery(0, 'SELECT is_active FROM civicrm_job WHERE name = "test_job"');
// Now call reconcile again, but in the context of the job's extension being installed/enabled. This should re-enable the job.
foreach (['enable', 'install'] as $process) {
// Manually disable the job
$this->callAPISuccess('Job', 'create', ['id' => $job['id'], 'is_active' => 0]);
// Mock the contextual process info that would be added by CRM_Extension_Manager::enable
$manager->setProcessesForTesting(['com.example.one' => [$process]]);
$me = new CRM_Core_ManagedEntities($this->modules, $decls);
$me->reconcile();
$job = $me->get('com.example.one', 'Job');
$this->assertEquals(1, $job['is_active']);
$this->assertEquals('test_job', $job['name']);
$this->assertDBQuery(1, 'SELECT is_active FROM civicrm_job WHERE name = "test_job"');
}
// Reset context.
$manager->setProcessesForTesting([]);
}
/**
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment