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

CRM-18231 Environment variables #8724

Merged
merged 3 commits into from
Sep 3, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CRM/Admin/Form/Setting.php
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ public function setDefaultValues() {
$this->_defaults['contact_reference_options'] = self::getAutocompleteContactReference();
$this->_defaults['enableSSL'] = Civi::settings()->get('enableSSL');
$this->_defaults['verifySSL'] = Civi::settings()->get('verifySSL');
$this->_defaults['environment'] = CRM_Core_Config::environment();
$this->_defaults['enableComponents'] = Civi::settings()->get('enable_components');
}

Expand Down
7 changes: 7 additions & 0 deletions CRM/Admin/Form/Setting/Debugging.php
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ class CRM_Admin_Form_Setting_Debugging extends CRM_Admin_Form_Setting {
'backtrace' => CRM_Core_BAO_Setting::DEVELOPER_PREFERENCES_NAME,
'fatalErrorHandler' => CRM_Core_BAO_Setting::DEVELOPER_PREFERENCES_NAME,
'assetCache' => CRM_Core_BAO_Setting::DEVELOPER_PREFERENCES_NAME,
'environment' => CRM_Core_BAO_Setting::DEVELOPER_PREFERENCES_NAME,
);

/**
Expand All @@ -53,6 +54,12 @@ public function buildQuickForm() {
}

parent::buildQuickForm();

if (defined('CIVICRM_ENVIRONMENT')) {
$element = $this->getElement('environment');
$element->freeze();
CRM_Core_Session::setStatus(ts('The environment settings have been disabled because it has been overridden in the settings file.'), ts('Environment settings'), 'info');
}
}

}
4 changes: 4 additions & 0 deletions CRM/Admin/Page/Job.php
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,10 @@ public function run() {
* @param null $action
*/
public function browse($action = NULL) {
// check if non-prod mode is enabled.
if (CRM_Core_Config::environment() != 'Production') {
CRM_Core_Session::setStatus(ts('Execution of scheduled jobs has been turned off by default since this is a non-production environment. You can override this for particular jobs by adding runInNonProductionEnvironment=TRUE as a parameter.'), ts("Non-production Environment"), "warning", array('expires' => 0));
}

// using Export action for Execute. Doh.
if ($this->_action & CRM_Core_Action::EXPORT) {
Expand Down
62 changes: 62 additions & 0 deletions CRM/Core/BAO/Setting.php
Original file line number Diff line number Diff line change
Expand Up @@ -490,4 +490,66 @@ public static function isUpgradeFromPreFourOneAlpha1() {
return FALSE;
}

/**
* Check if environment is explicitly set.
*
* @return bool
*/
public static function isEnvironmentSet($setting, $value = NULL) {
$environment = CRM_Core_Config::environment();
if ($setting == 'environment' && $environment) {
return TRUE;
}
return FALSE;
}

/**
* Check if job is able to be executed by API.
*
* @throws API_Exception
*/
public static function isAPIJobAllowedToRun($params) {
$environment = CRM_Core_Config::environment(NULL, TRUE);
if ($environment != 'Production') {
if (CRM_Utils_Array::value('runInNonProductionEnvironment', $params)) {
$mailing = Civi::settings()->get('mailing_backend_store');
if ($mailing) {
Civi::settings()->set('mailing_backend', $mailing);
}
}
else {
throw new Exception(ts("Job has not been executed as it is a %1 (non-production) environment.", array(1 => $environment)));
}
}
}

/**
* Setting Callback - On Change.
*
* Respond to changes in the "environment" setting.
*
* @param array $oldValue
* Value of old environment mode.
* @param array $newValue
* Value of new environment mode.
* @param array $metadata
* Specification of the setting (per *.settings.php).
*/
public static function onChangeEnvironmentSetting($oldValue, $newValue, $metadata) {
if ($newValue != 'Production') {
$mailing = Civi::settings()->get('mailing_backend');
if ($mailing['outBound_option'] != 2) {
Civi::settings()->set('mailing_backend_store', $mailing);
}
Civi::settings()->set('mailing_backend', array('outBound_option' => CRM_Mailing_Config::OUTBOUND_OPTION_DISABLED));
CRM_Core_Session::setStatus(ts('Outbound emails have been disabled. Scheduled jobs will not run unless runInNonProductionEnvironment=TRUE is added as a parameter for a specific job'), ts("Non-production environment set"), "success");
}
else {
$mailing = Civi::settings()->get('mailing_backend_store');
if ($mailing) {
Civi::settings()->set('mailing_backend', $mailing);
}
}
}

}
29 changes: 29 additions & 0 deletions CRM/Core/Config.php
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,35 @@ public static function domainID($domainID = NULL, $reset = FALSE) {
return $domain;
}

/**
* Function to get environment.
*
* @param string $env
* @param bool $reset
*
* @return string
*/
public static function environment($env = NULL, $reset = FALSE) {
static $environment;
if ($env) {
$environment = $env;
}
if ($reset || empty($environment)) {
if (defined('CIVICRM_ENVIRONMENT')) {
$environment = CIVICRM_ENVIRONMENT;
global $civicrm_setting;
$civicrm_setting[CRM_Core_BAO_Setting::DEVELOPER_PREFERENCES_NAME]['environment'] = $environment;
}
else {
$environment = Civi::settings()->get('environment');
}
}
if (!$environment) {
$environment = 'Production';
}
return $environment;
}

/**
* Do general cleanup of caches, temp directories and temp tables
* CRM-8739
Expand Down
11 changes: 11 additions & 0 deletions CRM/Core/JobManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,17 @@ public function executeJobById($id) {
*/
public function executeJob($job) {
$this->currentJob = $job;

// CRM-18231 check if non-production environment.
try {
CRM_Core_BAO_Setting::isAPIJobAllowedToRun($job->apiParams);
}
catch (Exception $e) {
$this->logEntry('Error while executing ' . $job->name . ': ' . $e->getMessage());
$this->currentJob = FALSE;
return FALSE;
}

$this->logEntry('Starting execution of ' . $job->name);
$job->saveLastRun();

Expand Down
11 changes: 11 additions & 0 deletions CRM/Upgrade/Incremental/sql/4.7.25.mysql.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,14 @@

--CRM-21061 Increase report_id size from 64 to 512 to match civicrm_option_value.value column
ALTER TABLE civicrm_report_instance CHANGE COLUMN report_id report_id varchar(512) COMMENT 'FK to civicrm_option_value for the report template';

-- CRM-18231 Environment variables support
INSERT INTO civicrm_option_group
(name, {localize field='title'}title{/localize}, is_reserved, is_active) VALUES ('environment', {localize}'{ts escape="sql"}Environment{/ts}'{/localize}, 0, 1);

SELECT @option_group_id_env := max(id) from civicrm_option_group where name = 'environment';
INSERT INTO civicrm_option_value (option_group_id, {localize field='label'}label{/localize}, value, name, grouping, filter, is_default, weight, {localize field='description'}description{/localize}, is_optgroup, is_reserved, is_active, component_id, visibility_id)
VALUES
(@option_group_id_env, {localize}'{ts escape="sql"}Production{/ts}'{/localize}, 'Production', 'Production', NULL, 0, 1, 1, {localize}'{ts escape="sql"}Production Environment{/ts}'{/localize}, 0, 0, 1, NULL, NULL),
(@option_group_id_env, {localize}'{ts escape="sql"}Staging{/ts}'{/localize}, 'Staging', 'Staging', NULL, 0, NULL, 2, {localize}'{ts escape="sql"}Staging Environment{/ts}'{/localize}, 0, 0, 1, NULL, NULL),
(@option_group_id_env, {localize}'{ts escape="sql"}Development{/ts}'{/localize}, 'Development', 'Development', NULL, 0, NULL, 3, {localize}'{ts escape="sql"}Development Environment{/ts}'{/localize}, 0, 0, 1, NULL, NULL);
20 changes: 20 additions & 0 deletions CRM/Utils/Check/Component/Env.php
Original file line number Diff line number Diff line change
Expand Up @@ -863,4 +863,24 @@ public function checkMbstring() {
return $messages;
}

/**
* Check if environment is Production.
* @return array
*/
public function checkEnvironment() {
$messages = array();

$environment = CRM_Core_Config::environment();
if ($environment != 'Production') {
$messages[] = new CRM_Utils_Check_Message(
__FUNCTION__,
ts('The environment of this CiviCRM instance is set to \'%1\'. Certain functionality like scheduled jobs has been disabled.', array(1 => $environment)),
ts('Non-Production Environment'),
\Psr\Log\LogLevel::ALERT,
'fa-bug'
);
}
return $messages;
}

}
4 changes: 2 additions & 2 deletions CRM/Utils/Mail.php
Original file line number Diff line number Diff line change
Expand Up @@ -100,12 +100,12 @@ public static function createMailer() {
}
elseif ($mailingInfo['outBound_option'] == CRM_Mailing_Config::OUTBOUND_OPTION_DISABLED) {
CRM_Core_Error::debug_log_message(ts('Outbound mail has been disabled. Click <a href=\'%1\'>Administer >> System Setting >> Outbound Email</a> to set the OutBound Email.', array(1 => CRM_Utils_System::url('civicrm/admin/setting/smtp', 'reset=1'))));
CRM_Core_Session::setStatus(ts('Outbound mail has been disabled. Click <a href=\'%1\'>Administer >> System Setting >> Outbound Email</a> to set the OutBound Email.', array(1 => CRM_Utils_System::url('civicrm/admin/setting/smtp', 'reset=1'))));
CRM_Core_Error::statusBounce(ts('Outbound mail has been disabled. Click <a href=\'%1\'>Administer >> System Setting >> Outbound Email</a> to set the OutBound Email.', array(1 => CRM_Utils_System::url('civicrm/admin/setting/smtp', 'reset=1'))));
}
else {
CRM_Core_Error::debug_log_message(ts('There is no valid SMTP server Setting Or SendMail path setting. Click <a href=\'%1\'>Administer >> System Setting >> Outbound Email</a> to set the OutBound Email.', array(1 => CRM_Utils_System::url('civicrm/admin/setting/smtp', 'reset=1'))));
CRM_Core_Session::setStatus(ts('There is no valid SMTP server Setting Or sendMail path setting. Click <a href=\'%1\'>Administer >> System Setting >> Outbound Email</a> to set the OutBound Email.', array(1 => CRM_Utils_System::url('civicrm/admin/setting/smtp', 'reset=1'))));
CRM_Core_Error::debug_var('mailing_info', $mailingInfo);
CRM_Core_Error::statusBounce(ts('There is no valid SMTP server Setting Or sendMail path setting. Click <a href=\'%1\'>Administer >> System Setting >> Outbound Email</a> to set the OutBound Email.', array(1 => CRM_Utils_System::url('civicrm/admin/setting/smtp', 'reset=1'))));
}
return $mailer;
}
Expand Down
4 changes: 3 additions & 1 deletion Civi/Core/SettingsBag.php
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,9 @@ protected function setDb($name, $value) {
}
$dao->find(TRUE);

if (isset($metadata['on_change']) && !($value == 0 && ($dao->value === NULL || unserialize($dao->value) == 0))) {
// string comparison with 0 always return true, so to be ensure the type use ===
// ref - https://stackoverflow.com/questions/8671942/php-string-comparasion-to-0-integer-returns-true
if (isset($metadata['on_change']) && !($value === 0 && ($dao->value === NULL || unserialize($dao->value) == 0))) {
foreach ($metadata['on_change'] as $callback) {
call_user_func(
\Civi\Core\Resolver::singleton()->get($callback),
Expand Down
8 changes: 8 additions & 0 deletions api/v3/Job.php
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,13 @@ function civicrm_api3_job_process_pledge($params) {
function civicrm_api3_job_process_mailing($params) {
$mailsProcessedOrig = CRM_Mailing_BAO_MailingJob::$mailsProcessed;

try {
CRM_Core_BAO_Setting::isAPIJobAllowedToRun($params);
}
catch (Exception $e) {
return civicrm_api3_create_error($e->getMessage());
}

if (!CRM_Mailing_BAO_Mailing::processQueue()) {
return civicrm_api3_create_error('Process Queue failed');
}
Expand Down Expand Up @@ -494,6 +501,7 @@ function civicrm_api3_job_process_batch_merge($params) {
'options' => array('limit' => 1),
));
}
$rgid = CRM_Utils_Array::value('rgid', $params);
$gid = CRM_Utils_Array::value('gid', $params);
$mode = CRM_Utils_Array::value('mode', $params, 'safe');

Expand Down
6 changes: 6 additions & 0 deletions api/v3/Setting.php
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,12 @@ function civicrm_api3_setting_getoptions($params) {
$values = Civi\Core\Resolver::singleton()->call($pseudoconstant['callback'], array());
return civicrm_api3_create_success($values, $params, 'Setting', 'getoptions');
}
elseif (!empty($pseudoconstant['optionGroupName'])) {
return civicrm_api3_create_success(
CRM_Core_OptionGroup::values($pseudoconstant['optionGroupName'], FALSE, FALSE, TRUE),
$params, 'Setting', 'getoptions'
);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@eileenmcnaughton reason to add this code is we cannot call CRM_Core_Pseudoconstant::get(..) OR buildOptions here as it needs DAO/BAO name to be passed either as parameter or class respectively as stated above at line 156.


throw new API_Exception("The field '" . $params['field'] . "' uses an unsupported option list.");
}
Expand Down
20 changes: 20 additions & 0 deletions settings/Developer.setting.php
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,26 @@
'is_contact' => 0,
'description' => "Set this value to Yes if you want to display a backtrace listing when a fatal error is encountered. This feature should NOT be enabled for production sites",
),
'environment' => array(
'group_name' => 'Developer Preferences',
'group' => 'developer',
'name' => 'environment',
'type' => 'String',
'html_type' => 'Select',
'quick_form_type' => 'Select',
'default' => 'Production',
'pseudoconstant' => array(
'optionGroupName' => 'environment',
),
'add' => '4.7',
'title' => 'Environment',
'is_domain' => 1,
'is_contact' => 0,
'description' => "Setting to define the environment in which this CiviCRM instance is running.",
'on_change' => array(
'CRM_Core_BAO_Setting::onChangeEnvironmentSetting',
),
),
'fatalErrorHandler' => array(
'group_name' => 'Developer Preferences',
'group' => 'developer',
Expand Down
68 changes: 34 additions & 34 deletions sql/civicrm_generated.mysql

Large diffs are not rendered by default.

5 changes: 5 additions & 0 deletions templates/CRM/Admin/Form/Setting/Debugging.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,11 @@
<td>{$form.backtrace.html}<br />
<span class="description">{ts}<strong>This feature should NOT be enabled for production sites.</strong><br />Set this value to <strong>Yes</strong> if you want to display a backtrace listing when a fatal error is encountered.{/ts}</span></td>
</tr>
<tr class="crm-debugging-form-block-environment">
<td class="label">{$form.environment.label}</td>
<td>{$form.environment.html}<br />
<span class="description">{ts}Set this value to <strong>Staging/Development</strong> to prevent cron jobs & mailings from being executed.{/ts}</span></td>
</tr>
<tr class="crm-debugging-form-block-fatalErrorHandler">
<td class="label">{$form.fatalErrorHandler.label}</td>
<td>{$form.fatalErrorHandler.html}<br />
Expand Down
11 changes: 11 additions & 0 deletions templates/CRM/common/civicrm.settings.php.template
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,17 @@ if (!defined('CIVICRM_DOMAIN_ID')) {
define( 'CIVICRM_DOMAIN_ID', 1);
}

/**
* Setting to define the environment in which this CiviCRM instance is running.
* Note the setting here must be value from the option group 'Environment',
* (see Administration > System Settings > Option Groups, Options beside Environment)
* which by default has three option values: 'Production', 'Staging', 'Development'.
* NB: defining a value from CIVICRM_ENVIRONMENT here prevents it from being set
* via the browser.
*/

// define( 'CIVICRM_ENVIRONMENT', 'Production' );

/**
* Settings to enable external caching using a cache server. This is an
* advanced feature, and you should read and understand the documentation
Expand Down
14 changes: 14 additions & 0 deletions tests/phpunit/CRM/Core/BAO/SettingTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -180,4 +180,18 @@ public static function _testOnChange_onChangeExample($oldValue, $newValue, $meta
$_testOnChange_hookCalls['metadata'] = $metadata;
}

/**
* Test to set isProductionEnvironment
*
*/
public function testSetCivicrmEnvironment() {
CRM_Core_BAO_Setting::setItem('Staging', CRM_Core_BAO_Setting::DEVELOPER_PREFERENCES_NAME, 'environment');
$values = CRM_Core_BAO_Setting::getItem(CRM_Core_BAO_Setting::DEVELOPER_PREFERENCES_NAME, 'environment');
$this->assertEquals('Staging', $values);

define('CIVICRM_ENVIRONMENT', 'Development');
Copy link
Member

@totten totten Sep 3, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(a) It's not safe for tests to call define() (at least, given the standard headless test environment). Trivially, suppose this test runs twice in a row. The second time, it will fail. Less trivially, it makes the test environment harder to predict. Compare:

  • When you run CRM_AllTests, any earlier tests (like CRM/Activity/**Test.php) run in an open-ended environment. (The default appears to be Production.)
  • When you run CRM_AllTests, any later tests (like CRM/Utils/**Test.php) are unalterably pegged to the Development environment.
  • When you run any test by itself (like CRM/Utils/**Test.php), you again have an open-ended environment (where the default is Production).
  • Consequently, tests will some of the time run in Production and some of the time in Development.
  • (Edit: Also, if the dev machine has its own define(), then this test will crash on trying to modify it.)

(b) Does it actually help to provide special handling for this setting (define(), CRM_Core_Config::environment()) rather than vanilla settings ($civicrm_setitings, Civi::settings()->get('environment'))?

  • For example, if the reason for the define() is that you want to be able to hard-code a value in civicrm.settings.php... then that's already handled by setting $civicrm_settings['domain']['environment']. (Note: Recall that in 4.7 you don't need to index by the DEVELOPER_PREFERENCES_NAME or whatever -- they're all just aliases for domain.)
  • On the other hand, if the setting is boot critical (meaning that it must be available as part of the early initialization of the settings/extensions/container system), then maybe it is needed. (But I'd really curious to know why it's boot-critical -- nothing pops while reading.)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My immediate thinking here is that what the test here is trying to prove is that if there is a value set in civicrm.settings.php that it overrides everything else. so you have a method set using essentially Civi::settings()->set to set the value to staging but then someone is then overriding that by setting Development in civicrm.settings.php However i'm thinking that the style at https://github.com/civicrm/civicrm-core/pull/8724/files#diff-d392832fe610e31e06039270b3a100e6R546 seems to be more robust way of testing civicrm.settings.php

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@seamuslee001 check Tim's comment about getMandatory - it makes me happy to see that

$environment = CRM_Core_Config::environment();
$this->assertEquals('Development', $environment);
}

}
2 changes: 1 addition & 1 deletion tests/phpunit/CRM/Mailing/BaseMailingSystemTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -395,7 +395,7 @@ protected function runMailingSuccess($params) {
$mailingParams = array_merge($this->defaultParams, $params);
$this->callAPISuccess('mailing', 'create', $mailingParams);
$this->_mut->assertRecipients(array());
$this->callAPISuccess('job', 'process_mailing', array());
$this->callAPISuccess('job', 'process_mailing', array('runInNonProductionEnvironment' => TRUE));

$allMessages = $this->_mut->getAllMessages('ezc');
// There are exactly two contacts produced by setUp().
Expand Down
Loading