From ed7784d945d91a2ac6c86c08facbe420e65087b2 Mon Sep 17 00:00:00 2001 From: wxa Date: Wed, 30 Oct 2019 16:17:05 +0300 Subject: [PATCH] Refactor CronHelper to a service [MAILPOET-2459] --- lib/Cron/CronHelper.php | 121 ++++++++++------------ lib/DI/ContainerConfigurator.php | 3 +- tests/integration/Cron/CronHelperTest.php | 73 +++++++------ 3 files changed, 98 insertions(+), 99 deletions(-) diff --git a/lib/Cron/CronHelper.php b/lib/Cron/CronHelper.php index a579dc97ab..a1698494dc 100644 --- a/lib/Cron/CronHelper.php +++ b/lib/Cron/CronHelper.php @@ -6,7 +6,6 @@ use MailPoet\Router\Endpoints\CronDaemon as CronDaemonEndpoint; use MailPoet\Router\Router; use MailPoet\Settings\SettingsController; use MailPoet\Util\Security; -use MailPoet\WP\Functions; use MailPoet\WP\Functions as WPFunctions; class CronHelper { @@ -19,20 +18,29 @@ class CronHelper { // Error codes const DAEMON_EXECUTION_LIMIT_REACHED = 1001; - static function getDaemonExecutionLimit() { - $wp = Functions::get(); - $limit = $wp->applyFilters('mailpoet_cron_get_execution_limit', self::DAEMON_EXECUTION_LIMIT); + /** @var SettingsController */ + private $settings; + + /** @var WPFunctions */ + private $wp; + + public function __construct(SettingsController $settings, WPFunctions $wp) { + $this->settings = $settings; + $this->wp = $wp; + } + + function getDaemonExecutionLimit() { + $limit = $this->wp->applyFilters('mailpoet_cron_get_execution_limit', self::DAEMON_EXECUTION_LIMIT); return $limit; } - static function getDaemonExecutionTimeout() { - $limit = self::getDaemonExecutionLimit(); + function getDaemonExecutionTimeout() { + $limit = $this->getDaemonExecutionLimit(); $timeout = $limit * 1.75; - $wp = Functions::get(); - return $wp->applyFilters('mailpoet_cron_get_execution_timeout', $timeout); + return $this->wp->applyFilters('mailpoet_cron_get_execution_timeout', $timeout); } - static function createDaemon($token) { + function createDaemon($token) { $daemon = [ 'token' => $token, 'status' => self::DAEMON_STATUS_ACTIVE, @@ -42,104 +50,93 @@ class CronHelper { 'last_error' => null, 'last_error_date' => null, ]; - self::saveDaemon($daemon); + $this->saveDaemon($daemon); return $daemon; } - static function restartDaemon($token) { - return self::createDaemon($token); + function restartDaemon($token) { + return $this->createDaemon($token); } - static function getDaemon() { - $settings = new SettingsController(); - return $settings->fetch(self::DAEMON_SETTING); + function getDaemon() { + return $this->settings->fetch(self::DAEMON_SETTING); } - static function saveDaemonLastError($error) { - $daemon = self::getDaemon(); + function saveDaemonLastError($error) { + $daemon = $this->getDaemon(); if ($daemon) { $daemon['last_error'] = $error; $daemon['last_error_date'] = time(); - self::saveDaemon($daemon); + $this->saveDaemon($daemon); } } - static function saveDaemonRunCompleted($run_completed_at) { - $daemon = self::getDaemon(); + function saveDaemonRunCompleted($run_completed_at) { + $daemon = $this->getDaemon(); if ($daemon) { $daemon['run_completed_at'] = $run_completed_at; - self::saveDaemon($daemon); + $this->saveDaemon($daemon); } } - static function saveDaemon($daemon) { + function saveDaemon($daemon) { $daemon['updated_at'] = time(); - $settings = new SettingsController(); - $settings->set( + $this->settings->set( self::DAEMON_SETTING, $daemon ); } - static function deactivateDaemon($daemon) { + function deactivateDaemon($daemon) { $daemon['status'] = self::DAEMON_STATUS_INACTIVE; - $settings = new SettingsController(); - $settings->set( + $this->settings->set( self::DAEMON_SETTING, $daemon ); } - static function createToken() { + function createToken() { return Security::generateRandomString(); } - static function pingDaemon($wp = null) { - if (is_null($wp)) { - $wp = new WPFunctions(); - } - $url = self::getCronUrl( - CronDaemonEndpoint::ACTION_PING_RESPONSE, - false, - $wp + function pingDaemon() { + $url = $this->getCronUrl( + CronDaemonEndpoint::ACTION_PING_RESPONSE ); - $result = self::queryCronUrl($url); + $result = $this->queryCronUrl($url); if (is_wp_error($result)) return $result->get_error_message(); - $response = $wp->wpRemoteRetrieveBody($result); + $response = $this->wp->wpRemoteRetrieveBody($result); $response = substr(trim($response), -strlen(DaemonHttpRunner::PING_SUCCESS_RESPONSE)) === DaemonHttpRunner::PING_SUCCESS_RESPONSE ? DaemonHttpRunner::PING_SUCCESS_RESPONSE : $response; return $response; } - static function validatePingResponse($response) { + function validatePingResponse($response) { return $response === DaemonHttpRunner::PING_SUCCESS_RESPONSE; } - static function accessDaemon($token, $wp = null) { - if (is_null($wp)) { - $wp = new WPFunctions(); - } + function accessDaemon($token) { $data = ['token' => $token]; - $url = self::getCronUrl( + $url = $this->getCronUrl( CronDaemonEndpoint::ACTION_RUN, $data ); - $daemon = self::getDaemon(); + $daemon = $this->getDaemon(); if (!$daemon) { throw new \LogicException('Daemon does not exist.'); } $daemon['run_accessed_at'] = time(); - self::saveDaemon($daemon); - $result = self::queryCronUrl($url, $wp); - return $wp->wpRemoteRetrieveBody($result); + $this->saveDaemon($daemon); + $result = $this->queryCronUrl($url); + return $this->wp->wpRemoteRetrieveBody($result); } /** * @return boolean|null */ - static function isDaemonAccessible() { - $daemon = self::getDaemon(); + function isDaemonAccessible() { + $daemon = $this->getDaemon(); if (!$daemon || !isset($daemon['run_accessed_at']) || $daemon['run_accessed_at'] === null) { return null; } @@ -155,11 +152,8 @@ class CronHelper { return null; } - static function queryCronUrl($url, $wp = null) { - if (is_null($wp)) { - $wp = new WPFunctions(); - } - $args = $wp->applyFilters( + function queryCronUrl($url) { + $args = $this->wp->applyFilters( 'mailpoet_cron_request_args', [ 'blocking' => true, @@ -168,25 +162,22 @@ class CronHelper { 'user-agent' => 'MailPoet Cron', ] ); - return $wp->wpRemotePost($url, $args); + return $this->wp->wpRemotePost($url, $args); } - static function getCronUrl($action, $data = false, $wp = null) { - if (is_null($wp)) { - $wp = new WPFunctions(); - } + function getCronUrl($action, $data = false) { $url = Router::buildRequest( CronDaemonEndpoint::ENDPOINT, $action, $data ); - $custom_cron_url = $wp->applyFilters('mailpoet_cron_request_url', $url); + $custom_cron_url = $this->wp->applyFilters('mailpoet_cron_request_url', $url); return ($custom_cron_url === $url) ? - str_replace(home_url(), self::getSiteUrl(), $url) : + str_replace(home_url(), $this->getSiteUrl(), $url) : $custom_cron_url; } - static function getSiteUrl($site_url = false) { + function getSiteUrl($site_url = false) { // additional check for some sites running inside a virtual machine or behind // proxy where there could be different ports (e.g., host:8080 => guest:80) $site_url = ($site_url) ? $site_url : WPFunctions::get()->homeUrl(); @@ -209,9 +200,9 @@ class CronHelper { throw new \Exception(__('Site URL is unreachable.', 'mailpoet')); } - static function enforceExecutionLimit($timer) { + function enforceExecutionLimit($timer) { $elapsed_time = microtime(true) - $timer; - if ($elapsed_time >= self::getDaemonExecutionLimit()) { + if ($elapsed_time >= $this->getDaemonExecutionLimit()) { throw new \Exception(__('Maximum execution time has been reached.', 'mailpoet'), self::DAEMON_EXECUTION_LIMIT_REACHED); } } diff --git a/lib/DI/ContainerConfigurator.php b/lib/DI/ContainerConfigurator.php index d9371fb17a..d93cab5642 100644 --- a/lib/DI/ContainerConfigurator.php +++ b/lib/DI/ContainerConfigurator.php @@ -125,6 +125,8 @@ class ContainerConfigurator implements IContainerConfigurator { // Dynamic segments $container->autowire(\MailPoet\DynamicSegments\DynamicSegmentHooks::class); // Cron + $container->autowire(\MailPoet\Cron\CronHelper::class)->setPublic(true); + $container->autowire(\MailPoet\Cron\CronTrigger::class)->setPublic(true); $container->autowire(\MailPoet\Cron\Daemon::class)->setPublic(true); $container->autowire(\MailPoet\Cron\DaemonHttpRunner::class)->setPublic(true); $container->autowire(\MailPoet\Cron\Workers\WorkersFactory::class)->setPublic(true); @@ -132,7 +134,6 @@ class ContainerConfigurator implements IContainerConfigurator { $container->autowire(\MailPoet\Cron\Workers\StatsNotifications\Scheduler::class); $container->autowire(\MailPoet\Cron\Workers\StatsNotifications\StatsNotificationsRepository::class); $container->autowire(\MailPoet\Cron\Workers\StatsNotifications\NewsletterLinkRepository::class); - $container->autowire(\MailPoet\Cron\CronTrigger::class)->setPublic(true); // Custom field $container->autowire(\MailPoet\CustomFields\ApiDataSanitizer::class); // Features diff --git a/tests/integration/Cron/CronHelperTest.php b/tests/integration/Cron/CronHelperTest.php index f24c814976..da7070b7bc 100644 --- a/tests/integration/Cron/CronHelperTest.php +++ b/tests/integration/Cron/CronHelperTest.php @@ -14,6 +14,9 @@ class CronHelperTest extends \MailPoetTest { /** @var SettingsController */ private $settings; + /** @var CronHelper */ + private $cron_helper; + function _before() { parent::_before(); $this->settings = new SettingsController(); @@ -26,6 +29,7 @@ class CronHelperTest extends \MailPoetTest { 'name' => 'John Doe', 'address' => 'john.doe@example.com', ]); + $this->cron_helper = new CronHelper($this->settings, new WPFunctions); } function testItDefinesConstants() { @@ -37,7 +41,7 @@ class CronHelperTest extends \MailPoetTest { function testItCreatesDaemon() { $token = 'create_token'; $time = time(); - CronHelper::createDaemon($token); + $this->cron_helper->createDaemon($token); $daemon = $this->settings->get(CronHelper::DAEMON_SETTING); expect($daemon)->equals( [ @@ -56,7 +60,7 @@ class CronHelperTest extends \MailPoetTest { function testItRestartsDaemon() { $token = 'restart_token'; $time = time(); - CronHelper::restartDaemon($token); + $this->cron_helper->restartDaemon($token); $daemon = $this->settings->get(CronHelper::DAEMON_SETTING); expect($daemon)->equals( [ @@ -78,7 +82,7 @@ class CronHelperTest extends \MailPoetTest { CronHelper::DAEMON_SETTING, $daemon ); - expect(CronHelper::getDaemon())->equals($daemon); + expect($this->cron_helper->getDaemon())->equals($daemon); } function testItSavesDaemon() { @@ -89,9 +93,9 @@ class CronHelperTest extends \MailPoetTest { $daemon ); $time = time(); - CronHelper::saveDaemon($daemon); + $this->cron_helper->saveDaemon($daemon); $daemon['updated_at'] = $time; - expect(CronHelper::getDaemon())->equals($daemon); + expect($this->cron_helper->getDaemon())->equals($daemon); } function testItUpdatesDaemonAccessedAt() { @@ -104,15 +108,16 @@ class CronHelperTest extends \MailPoetTest { $wp = Stub::make(new WPFunctions, [ 'wpRemotePost' => [], ]); - CronHelper::accessDaemon('some_token', $wp); - $updated_daemon = CronHelper::getDaemon(); + $cron_helper = new CronHelper($this->settings, $wp); + $cron_helper->accessDaemon('some_token'); + $updated_daemon = $cron_helper->getDaemon(); expect($updated_daemon['run_accessed_at'])->greaterOrEquals($time); expect($updated_daemon['run_accessed_at'])->lessThan($time + 2); } function testItThrowsAnExceptionIfAccessingNonExistingDaemon() { try { - CronHelper::accessDaemon('some_token'); + $this->cron_helper->accessDaemon('some_token'); $this->fail('An exception should have been thrown.'); } catch (\LogicException $e) { expect($e->getMessage())->equals('Daemon does not exist.'); @@ -130,7 +135,7 @@ class CronHelperTest extends \MailPoetTest { CronHelper::DAEMON_SETTING, $daemon ); - expect(CronHelper::isDaemonAccessible())->false(); + expect($this->cron_helper->isDaemonAccessible())->false(); } } @@ -143,7 +148,7 @@ class CronHelperTest extends \MailPoetTest { CronHelper::DAEMON_SETTING, $daemon ); - expect(CronHelper::isDaemonAccessible())->true(); + expect($this->cron_helper->isDaemonAccessible())->true(); } function testItDetectsUnknownStateOfTheDaemon() { @@ -171,7 +176,7 @@ class CronHelperTest extends \MailPoetTest { CronHelper::DAEMON_SETTING, $daemon ); - expect(CronHelper::isDaemonAccessible())->null(); + expect($this->cron_helper->isDaemonAccessible())->null(); } } @@ -182,8 +187,8 @@ class CronHelperTest extends \MailPoetTest { $daemon ); - CronHelper::deactivateDaemon($daemon); - $daemon = CronHelper::getDaemon(); + $this->cron_helper->deactivateDaemon($daemon); + $daemon = $this->cron_helper->getDaemon(); expect($daemon['status'])->equals(CronHelper::DAEMON_STATUS_INACTIVE); } @@ -195,8 +200,8 @@ class CronHelperTest extends \MailPoetTest { ); $time = time(); - CronHelper::saveDaemonLastError('error'); - $daemon = CronHelper::getDaemon(); + $this->cron_helper->saveDaemonLastError('error'); + $daemon = $this->cron_helper->getDaemon(); expect($daemon['last_error'])->equals('error'); expect($daemon['last_error_date'])->greaterOrEquals($time); } @@ -209,15 +214,15 @@ class CronHelperTest extends \MailPoetTest { $daemon ); - CronHelper::saveDaemonRunCompleted(123); - $daemon = CronHelper::getDaemon(); + $this->cron_helper->saveDaemonRunCompleted(123); + $daemon = $this->cron_helper->getDaemon(); expect($daemon['run_completed_at'])->equals(123); } function testItCreatesRandomToken() { // random token is a string of 5 characters - $token1 = CronHelper::createToken(); - $token2 = CronHelper::createToken(); + $token1 = $this->cron_helper->createToken(); + $token2 = $this->cron_helper->createToken(); expect($token1)->notEquals($token2); expect(is_string($token1))->true(); expect(strlen($token1))->equals(5); @@ -226,23 +231,23 @@ class CronHelperTest extends \MailPoetTest { function testItGetsSiteUrl() { // 1. do nothing when the url does not contain port $site_url = 'http://example.com'; - expect(CronHelper::getSiteUrl($site_url))->equals($site_url); + expect($this->cron_helper->getSiteUrl($site_url))->equals($site_url); if (getenv('WP_TEST_ENABLE_NETWORK_TESTS') !== 'true') $this->markTestSkipped(); // 2. when url contains valid port, try connecting to it $site_url = 'http://example.com:80'; - expect(CronHelper::getSiteUrl($site_url))->equals($site_url); + expect($this->cron_helper->getSiteUrl($site_url))->equals($site_url); // 3. when url contains invalid port, try connecting to it. when connection fails, // another attempt will be made to connect to the standard port derived from URL schema $site_url = 'http://example.com:8080'; - expect(CronHelper::getSiteUrl($site_url))->equals('http://example.com'); + expect($this->cron_helper->getSiteUrl($site_url))->equals('http://example.com'); // 4. when connection can't be established, exception should be thrown $site_url = 'https://invalid:80'; try { - CronHelper::getSiteUrl($site_url); + $this->cron_helper->getSiteUrl($site_url); self::fail('Site URL is unreachable exception not thrown.'); } catch (\Exception $e) { expect($e->getMessage())->equals('Site URL is unreachable.'); @@ -251,15 +256,15 @@ class CronHelperTest extends \MailPoetTest { function testItGetsSubsiteUrlOnMultisiteEnvironment() { if ((boolean)getenv('MULTISITE') === true) { - expect(CronHelper::getSiteUrl())->contains(getenv('WP_TEST_MULTISITE_SLUG')); + expect($this->cron_helper->getSiteUrl())->contains(getenv('WP_TEST_MULTISITE_SLUG')); } } function testItEnforcesExecutionLimit() { $time = microtime(true); - expect(CronHelper::enforceExecutionLimit($time))->null(); + expect($this->cron_helper->enforceExecutionLimit($time))->null(); try { - CronHelper::enforceExecutionLimit($time - CronHelper::getDaemonExecutionLimit()); + $this->cron_helper->enforceExecutionLimit($time - $this->cron_helper->getDaemonExecutionLimit()); self::fail('Execution limit exception not thrown.'); } catch (\Exception $e) { expect($e->getMessage())->equals('Maximum execution time has been reached.'); @@ -272,7 +277,7 @@ class CronHelperTest extends \MailPoetTest { return 'http://custom_cron_url'; }; add_filter('mailpoet_cron_request_url', $filter); - expect(CronHelper::getCronUrl('sample_action'))->equals('http://custom_cron_url'); + expect($this->cron_helper->getCronUrl('sample_action'))->equals('http://custom_cron_url'); remove_filter('mailpoet_cron_request_url', $filter); } @@ -294,7 +299,8 @@ class CronHelperTest extends \MailPoetTest { }, ]); $wp->addFilter('mailpoet_cron_request_args', $filter); - CronHelper::queryCronUrl('test', $wp); + $cron_helper = new CronHelper($this->settings, $wp); + $cron_helper->queryCronUrl('test'); expect($wp_remote_get_args[1])->equals($request_args); $wp->removeFilter('mailpoet_cron_request_args', $filter); @@ -302,20 +308,21 @@ class CronHelperTest extends \MailPoetTest { function testItReturnsErrorMessageAsPingResponseWhenCronUrlCannotBeAccessed() { $wp = Stub::make(new WPFunctions, [ - 'applyFilters' => false, + 'applyFilters' => [], ]); - expect(CronHelper::pingDaemon($wp))->equals('A valid URL was not provided.'); + $cron_helper = new CronHelper($this->settings, $wp); + expect($cron_helper->pingDaemon())->equals('A valid URL was not provided.'); } function testItPingsDaemon() { if (getenv('WP_TEST_ENABLE_NETWORK_TESTS') !== 'true') $this->markTestSkipped(); // raw response is returned - expect(CronHelper::pingDaemon())->equals(DaemonHttpRunner::PING_SUCCESS_RESPONSE); + expect($this->cron_helper->pingDaemon())->equals(DaemonHttpRunner::PING_SUCCESS_RESPONSE); } function testItValidatesPingResponse() { - expect(CronHelper::validatePingResponse('pong'))->true(); - expect(CronHelper::validatePingResponse('something else'))->false(); + expect($this->cron_helper->validatePingResponse('pong'))->true(); + expect($this->cron_helper->validatePingResponse('something else'))->false(); } function _after() {