diff --git a/assets/js/src/common/print_boolean.jsx b/assets/js/src/common/print_boolean.jsx new file mode 100644 index 0000000000..fd4956b77a --- /dev/null +++ b/assets/js/src/common/print_boolean.jsx @@ -0,0 +1,26 @@ +import React from 'react'; +import MailPoet from 'mailpoet'; + +const PrintBoolean = props => ( + + {(props.children === true && props.truthy) || + (props.children === false && props.falsy) || + (props.unknown)} + +); + +PrintBoolean.propTypes = { + truthy: React.PropTypes.string, + falsy: React.PropTypes.string, + unknown: React.PropTypes.string, + children: React.PropTypes.bool, +}; + +PrintBoolean.defaultProps = { + truthy: MailPoet.I18n.t('yes'), + falsy: MailPoet.I18n.t('no'), + unknown: MailPoet.I18n.t('unknown'), + children: null, +}; + +module.exports = PrintBoolean; diff --git a/assets/js/src/help/cron_status.jsx b/assets/js/src/help/cron_status.jsx new file mode 100644 index 0000000000..80b268ab74 --- /dev/null +++ b/assets/js/src/help/cron_status.jsx @@ -0,0 +1,71 @@ +import MailPoet from 'mailpoet'; +import React from 'react'; +import PrintBoolean from 'common/print_boolean.jsx'; + +function renderStatusTableRow(title, value) { + return ( + + { title }{ value } + + ); +} + +const CronStatus = (props) => { + const status = props.status_data; + const activeStatusMapping = { + active: MailPoet.I18n.t('cronRunning'), + inactive: MailPoet.I18n.t('cronWaiting'), + }; + return ( +
+

{MailPoet.I18n.t('systemStatusCronStatusTitle')}

+ + + {renderStatusTableRow( + MailPoet.I18n.t('accessible'), + {status.accessible}) + } + {renderStatusTableRow( + MailPoet.I18n.t('status'), + activeStatusMapping[status.status] ? activeStatusMapping[status.status] : MailPoet.I18n.t('unknown')) + } + {renderStatusTableRow( + MailPoet.I18n.t('lastUpdated'), + status.updated_at ? MailPoet.Date.full(status.updated_at * 1000) : MailPoet.I18n.t('unknown')) + } + {renderStatusTableRow( + MailPoet.I18n.t('lastRunStarted'), + status.run_accessed_at ? MailPoet.Date.full(status.run_started_at * 1000) : MailPoet.I18n.t('unknown')) + } + {renderStatusTableRow( + MailPoet.I18n.t('lastRunCompleted'), + status.run_completed_at ? MailPoet.Date.full(status.run_completed_at * 1000) : MailPoet.I18n.t('unknown')) + } + {renderStatusTableRow(MailPoet.I18n.t('lastSeenError'), status.last_error || '-')} + +
+
+ ); +}; + +CronStatus.propTypes = { + status_data: React.PropTypes.shape({ + accessible: React.PropTypes.bool, + status: React.PropTypes.string, + updated_at: React.PropTypes.number, + run_accessed_at: React.PropTypes.number, + run_completed_at: React.PropTypes.number, + }).isRequired, +}; + +CronStatus.defaultProps = { + status_data: { + accessible: null, + status: null, + updated_at: null, + run_accessed_at: null, + run_completed_at: null, + }, +}; + +module.exports = CronStatus; diff --git a/assets/js/src/help/help.jsx b/assets/js/src/help/help.jsx index b4a16411a1..9dba90fc45 100644 --- a/assets/js/src/help/help.jsx +++ b/assets/js/src/help/help.jsx @@ -21,11 +21,11 @@ if (container) { ReactDOM.render(( - + {/* Pages */} + - ), container); diff --git a/assets/js/src/help/system_status.jsx b/assets/js/src/help/system_status.jsx index f4750aba56..7eb7f7b8cc 100644 --- a/assets/js/src/help/system_status.jsx +++ b/assets/js/src/help/system_status.jsx @@ -1,6 +1,7 @@ import MailPoet from 'mailpoet'; import React from 'react'; import ReactStringReplace from 'react-string-replace'; +import CronStatus from './cron_status.jsx'; import Tabs from './tabs.jsx'; function renderStatusMessage(status, error, link) { @@ -65,6 +66,7 @@ function SystemStatus() { {renderCronSection(systemStatusData)} {renderMSSSection(systemStatusData)} + ); } diff --git a/assets/js/src/help/tabs.jsx b/assets/js/src/help/tabs.jsx index 2db98f386e..6d258b6817 100644 --- a/assets/js/src/help/tabs.jsx +++ b/assets/js/src/help/tabs.jsx @@ -4,6 +4,11 @@ import classNames from 'classnames'; import MailPoet from 'mailpoet'; const tabs = [ + { + name: 'knowledgeBase', + label: MailPoet.I18n.t('tabKnowledgeBaseTitle'), + link: '/knowledgeBase', + }, { name: 'systemStatus', label: MailPoet.I18n.t('tabSystemStatusTitle'), @@ -14,11 +19,6 @@ const tabs = [ label: MailPoet.I18n.t('tabSystemInfoTitle'), link: '/systemInfo', }, - { - name: 'knowledgeBase', - label: MailPoet.I18n.t('tabKnowledgeBaseTitle'), - link: '/knowledgeBase', - }, ]; function Tabs(props) { @@ -45,6 +45,6 @@ function Tabs(props) { } Tabs.propTypes = { tab: React.PropTypes.string }; -Tabs.defaultProps = { tab: 'systemStatus' }; +Tabs.defaultProps = { tab: 'knowledgeBase' }; module.exports = Tabs; diff --git a/lib/Config/Menu.php b/lib/Config/Menu.php index fd2afa4b25..97f168d3f7 100644 --- a/lib/Config/Menu.php +++ b/lib/Config/Menu.php @@ -448,17 +448,19 @@ class Menu { function help() { $system_info_data = Beacon::getData(); - $system_status_data = array( - 'cron' => array( + $system_status_data = [ + 'cron' => [ 'url' => CronHelper::getCronUrl(CronDaemon::ACTION_PING), 'isReachable' => CronHelper::pingDaemon(true) - ), - 'mss' => array( + ], + 'mss' => [ 'enabled' => (Bridge::isMPSendingServiceEnabled()) ? - array('isReachable' => Bridge::pingBridge()) : + ['isReachable' => Bridge::pingBridge()] : false - ) - ); + ], + 'cronStatus' => CronHelper::getDaemon(), + ]; + $system_status_data['cronStatus']['accessible'] = CronHelper::isDaemonAccessible(); $this->displayPage( 'help.html', array( diff --git a/lib/Cron/CronHelper.php b/lib/Cron/CronHelper.php index c215e42297..820798eedd 100644 --- a/lib/Cron/CronHelper.php +++ b/lib/Cron/CronHelper.php @@ -16,15 +16,18 @@ class CronHelper { const DAEMON_EXECUTION_TIMEOUT = 35; // seconds const DAEMON_REQUEST_TIMEOUT = 5; // seconds const DAEMON_SETTING = 'cron_daemon'; + const DAEMON_STATUS_ACTIVE = 'active'; + const DAEMON_STATUS_INACTIVE = 'inactive'; static function createDaemon($token) { - $daemon = array( + $daemon = [ 'token' => $token, + 'status' => self::DAEMON_STATUS_ACTIVE, 'run_accessed_at' => null, 'run_started_at' => null, 'run_completed_at' => null, 'last_error' => null, - ); + ]; self::saveDaemon($daemon); return $daemon; } @@ -45,8 +48,12 @@ class CronHelper { ); } - static function deleteDaemon() { - return Setting::deleteValue(self::DAEMON_SETTING); + static function deactivateDaemon($daemon) { + $daemon['status'] = self::DAEMON_STATUS_INACTIVE; + return Setting::setValue( + self::DAEMON_SETTING, + $daemon + ); } static function createToken() { @@ -89,7 +96,7 @@ class CronHelper { */ static function isDaemonAccessible() { $daemon = self::getDaemon(); - if(!$daemon || $daemon['run_accessed_at'] === null) { + if(!$daemon || !isset($daemon['run_accessed_at']) || $daemon['run_accessed_at'] === null) { return null; } if($daemon['run_accessed_at'] <= (int)$daemon['run_started_at']) { diff --git a/lib/Cron/Daemon.php b/lib/Cron/Daemon.php index 2e230bdc36..0b848c6345 100644 --- a/lib/Cron/Daemon.php +++ b/lib/Cron/Daemon.php @@ -75,7 +75,7 @@ class Daemon { } // after each execution, re-read daemon data in case it changed $daemon = CronHelper::getDaemon(); - if(!$daemon || $daemon['token'] !== $this->token) { + if($this->shouldTerminateExecution($daemon)) { return $this->terminateRequest(); } return $this->callSelf(); @@ -128,4 +128,13 @@ class Daemon { function terminateRequest($message = false) { die($message); } -} + + /** + * @return boolean + */ + private function shouldTerminateExecution(array $daemon = null) { + return !$daemon || + $daemon['token'] !== $this->token || + (isset($daemon['status']) && $daemon['status'] !== CronHelper::DAEMON_STATUS_ACTIVE); + } +} \ No newline at end of file diff --git a/lib/Cron/Supervisor.php b/lib/Cron/Supervisor.php index 4cb29edda0..dfbda9d04a 100644 --- a/lib/Cron/Supervisor.php +++ b/lib/Cron/Supervisor.php @@ -16,7 +16,9 @@ class Supervisor { $daemon = $this->daemon; $execution_timeout_exceeded = (time() - (int)$daemon['updated_at']) >= CronHelper::DAEMON_EXECUTION_TIMEOUT; - if($execution_timeout_exceeded) { + $daemon_is_inactive = + isset($daemon['status']) && $daemon['status'] === CronHelper::DAEMON_STATUS_INACTIVE; + if($execution_timeout_exceeded || $daemon_is_inactive) { CronHelper::restartDaemon($this->token); return $this->runDaemon(); } diff --git a/lib/Cron/Triggers/WordPress.php b/lib/Cron/Triggers/WordPress.php index 117919c23c..9bfc417db8 100644 --- a/lib/Cron/Triggers/WordPress.php +++ b/lib/Cron/Triggers/WordPress.php @@ -18,7 +18,7 @@ class WordPress { static function run() { return (self::checkExecutionRequirements()) ? MailPoet::run() : - self::cleanup(); + self::stop(); } static function checkExecutionRequirements() { @@ -60,10 +60,10 @@ class WordPress { ); } - static function cleanup() { + static function stop() { $cron_daemon = CronHelper::getDaemon(); if($cron_daemon) { - CronHelper::deleteDaemon(); + CronHelper::deactivateDaemon($cron_daemon); } } } diff --git a/tests/unit/Cron/CronHelperTest.php b/tests/unit/Cron/CronHelperTest.php index b19825c1d6..1b2c7c3e11 100644 --- a/tests/unit/Cron/CronHelperTest.php +++ b/tests/unit/Cron/CronHelperTest.php @@ -33,6 +33,7 @@ class CronHelperTest extends \MailPoetTest { expect($daemon)->equals( array( 'token' => $token, + 'status' => CronHelper::DAEMON_STATUS_ACTIVE, 'updated_at' => $time, 'run_accessed_at' => null, 'run_started_at' => null, @@ -50,6 +51,7 @@ class CronHelperTest extends \MailPoetTest { expect($daemon)->equals( array( 'token' => $token, + 'status' => CronHelper::DAEMON_STATUS_ACTIVE, 'updated_at' => $time, 'run_accessed_at' => null, 'run_started_at' => null, @@ -62,6 +64,7 @@ class CronHelperTest extends \MailPoetTest { function testItLoadsDaemon() { $daemon = array( 'token' => 'some_token', + 'status' => CronHelper::DAEMON_STATUS_ACTIVE, 'updated_at' => '12345678', 'run_accessed_at' => null, 'run_started_at' => null, @@ -79,6 +82,7 @@ class CronHelperTest extends \MailPoetTest { // when saving daemon, 'updated_at' value should change $daemon = array( 'token' => 'some_token', + 'status' => CronHelper::DAEMON_STATUS_ACTIVE, 'updated_at' => '12345678', 'run_accessed_at' => null, 'run_started_at' => null, @@ -98,6 +102,7 @@ class CronHelperTest extends \MailPoetTest { function testItUpdatesDaemonAccessedAt() { $daemon = [ 'token' => 'some_token', + 'status' => CronHelper::DAEMON_STATUS_ACTIVE, 'updated_at' => 12345678, 'run_accessed_at' => null, 'run_started_at' => null, @@ -131,6 +136,7 @@ class CronHelperTest extends \MailPoetTest { foreach($run_start_values as $run_start) { $daemon = [ 'token' => 'some_token', + 'status' => CronHelper::DAEMON_STATUS_ACTIVE, 'updated_at' => 12345678, 'run_accessed_at' => $time - 10, 'run_started_at' => $run_start, @@ -149,6 +155,7 @@ class CronHelperTest extends \MailPoetTest { $time = time(); $daemon = [ 'token' => 'some_token', + 'status' => CronHelper::DAEMON_STATUS_ACTIVE, 'updated_at' => 12345678, 'run_accessed_at' => $time - 5, 'run_started_at' => $time - 4, @@ -182,6 +189,7 @@ class CronHelperTest extends \MailPoetTest { foreach($test_inputs as $test_input) { $daemon = [ 'token' => 'some_token', + 'status' => CronHelper::DAEMON_STATUS_ACTIVE, 'updated_at' => 12345678, 'run_accessed_at' => $test_input['run_access'], 'run_started_at' => $test_input['run_start'], @@ -196,6 +204,26 @@ class CronHelperTest extends \MailPoetTest { } } + function testItDeactivatesDaemon() { + $daemon = [ + 'token' => 'some_token', + 'status' => CronHelper::DAEMON_STATUS_ACTIVE, + 'updated_at' => '12345678', + 'run_accessed_at' => null, + 'run_started_at' => null, + 'run_completed_at' => null, + 'last_error' => null, + ]; + Setting::setValue( + CronHelper::DAEMON_SETTING, + $daemon + ); + + CronHelper::deactivateDaemon($daemon); + $daemon = CronHelper::getDaemon(); + expect($daemon['status'])->equals(CronHelper::DAEMON_STATUS_INACTIVE); + } + function testItCreatesRandomToken() { // random token is a string of 5 characters $token1 = CronHelper::createToken(); diff --git a/tests/unit/Cron/DaemonTest.php b/tests/unit/Cron/DaemonTest.php index e48aa4c439..e64f43f004 100644 --- a/tests/unit/Cron/DaemonTest.php +++ b/tests/unit/Cron/DaemonTest.php @@ -157,6 +157,22 @@ class DaemonTest extends \MailPoetTest { $daemon->run(); } + function testItTerminatesExecutionWhenDaemonIsDeactivated() { + $daemon = Stub::make(new Daemon(true), [ + 'executeScheduleWorker' => null, + 'executeQueueWorker' => null, + 'pauseExecution' => null, + 'terminateRequest' => Expected::exactly(1) + ], $this); + $data = [ + 'token' => 123, + 'status' => CronHelper::DAEMON_STATUS_INACTIVE, + ]; + Setting::setValue(CronHelper::DAEMON_SETTING, $data); + $daemon->__construct($data); + $daemon->run(); + } + function testItUpdatesDaemonTokenDuringExecution() { $daemon = Stub::make(new Daemon(true), array( 'executeScheduleWorker' => null, diff --git a/tests/unit/Cron/SupervisorTest.php b/tests/unit/Cron/SupervisorTest.php index 47c74f5999..23bd8f6e5d 100644 --- a/tests/unit/Cron/SupervisorTest.php +++ b/tests/unit/Cron/SupervisorTest.php @@ -50,6 +50,16 @@ class SupervisorTest extends \MailPoetTest { $daemon = $supervisor->checkDaemon(); expect(is_int($daemon['updated_at']))->true(); expect($daemon['updated_at'])->notEquals($supervisor->daemon['updated_at']); + expect($daemon['status'])->equals(CronHelper::DAEMON_STATUS_ACTIVE); + } + + function testRestartsDaemonWhenItIsInactive() { + if(getenv('WP_TEST_ENABLE_NETWORK_TESTS') !== 'true') return; + $supervisor = new Supervisor(); + $supervisor->daemon['updated_at'] = time(); + $supervisor->daemon['status'] = CronHelper::DAEMON_STATUS_INACTIVE; + $daemon = $supervisor->checkDaemon(); + expect($daemon['status'])->equals(CronHelper::DAEMON_STATUS_ACTIVE); } function _after() { diff --git a/tests/unit/Cron/Triggers/WordPressTest.php b/tests/unit/Cron/Triggers/WordPressTest.php index d6330ad8c7..cdab7f486b 100644 --- a/tests/unit/Cron/Triggers/WordPressTest.php +++ b/tests/unit/Cron/Triggers/WordPressTest.php @@ -77,11 +77,11 @@ class WordPressTest extends \MailPoetTest { expect(WordPress::checkExecutionRequirements())->false(); } - function testItCanDeleteRunningDaemon() { - Setting::setValue(CronHelper::DAEMON_SETTING, true); - expect(Setting::getValue(CronHelper::DAEMON_SETTING))->notNull(); - WordPress::cleanup(); - expect(Setting::getValue(CronHelper::DAEMON_SETTING))->null(); + function testItCanDeactivateRunningDaemon() { + Setting::setValue(CronHelper::DAEMON_SETTING, ['status' => CronHelper::DAEMON_STATUS_ACTIVE]); + expect(Setting::getValue(CronHelper::DAEMON_SETTING)['status'])->equals(CronHelper::DAEMON_STATUS_ACTIVE); + WordPress::stop(); + expect(Setting::getValue(CronHelper::DAEMON_SETTING)['status'])->equals(CronHelper::DAEMON_STATUS_INACTIVE); } function testItRunsWhenExecutionRequirementsAreMet() { @@ -93,11 +93,11 @@ class WordPressTest extends \MailPoetTest { expect(Setting::getValue(CronHelper::DAEMON_SETTING))->notNull(); } - function testItDeletesCronDaemonWhenExecutionRequirementsAreNotMet() { - Setting::setValue(CronHelper::DAEMON_SETTING, true); - expect(Setting::getValue(CronHelper::DAEMON_SETTING))->notNull(); + function testItDeactivatesCronDaemonWhenExecutionRequirementsAreNotMet() { + Setting::setValue(CronHelper::DAEMON_SETTING, ['status' => CronHelper::DAEMON_STATUS_ACTIVE]); + expect(Setting::getValue(CronHelper::DAEMON_SETTING)['status'])->equals(CronHelper::DAEMON_STATUS_ACTIVE); WordPress::run(); - expect(Setting::getValue(CronHelper::DAEMON_SETTING))->null(); + expect(Setting::getValue(CronHelper::DAEMON_SETTING)['status'])->equals(CronHelper::DAEMON_STATUS_INACTIVE); } function _addMTAConfigAndLog($sent, $status = null) { diff --git a/views/help.html b/views/help.html index 9f509a3e23..1d9c7b9f2f 100644 --- a/views/help.html +++ b/views/help.html @@ -33,6 +33,18 @@ 'knowledgeBaseButton': __('Visit our Knowledge Base for more articles'), 'systemInfoIntro': __('The information below is useful when you need to get in touch with our support. Just copy all the text below and paste it into a message to us.'), 'systemInfoDataError': __('Sorry, there was an error, please try again later.'), + 'systemStatusCronStatusTitle': __('Cron'), + 'lastUpdated': __('Last updated'), + 'lastRunStarted': __('Last run started'), + 'lastRunCompleted': __('Last run completed'), + 'lastSeenError': __('Last seen error'), + 'unknown': __('unknown'), + 'accessible': __('Accessible'), + 'status': __('Status'), + 'yes': __('yes'), + 'no': __('no'), + 'cronRunning': __('running'), + 'cronWaiting': __('waiting for the next run'), }) %> <% endblock %>