Refactor lib/Router to use with container for endpoints

[MAILPOET-1605]
This commit is contained in:
Rostislav Wolny
2018-10-17 17:39:32 +02:00
parent 71ad9f50cb
commit fff8176a49
14 changed files with 132 additions and 111 deletions

View File

@ -94,6 +94,11 @@ class Initializer {
));
}
function compileContainer() {
$this->container = ContainerFactory::getContainer();
$this->container->compile();
}
function checkRequirements() {
$requirements = new RequirementsChecker();
return $requirements->checkAllRequirements();
@ -258,7 +263,7 @@ class Initializer {
}
function setupRouter() {
$router = new Router\Router($this->access_control);
$router = new Router\Router($this->access_control, $this->container);
$router->init();
}

View File

@ -6,7 +6,6 @@ require_once(ABSPATH . 'wp-includes/pluggable.php');
class DaemonHttpRunner {
public $settings_daemon_data;
public $request_data;
public $timer;
public $token;
@ -15,15 +14,11 @@ class DaemonHttpRunner {
const PING_SUCCESS_RESPONSE = 'pong';
function __construct($request_data = false, $daemon = null) {
$this->request_data = $request_data;
function __construct(Daemon $daemon = null) {
$this->settings_daemon_data = CronHelper::getDaemon();
$this->token = CronHelper::createToken();
$this->timer = microtime(true);
$this->daemon = $daemon;
if(!$daemon) {
$this->daemon = new Daemon($this->settings_daemon_data);
}
}
function ping() {
@ -31,20 +26,20 @@ class DaemonHttpRunner {
$this->terminateRequest(self::PING_SUCCESS_RESPONSE);
}
function run() {
function run($request_data) {
ignore_user_abort(true);
if(strpos(@ini_get('disable_functions'), 'set_time_limit') === false) {
set_time_limit(0);
}
$this->addCacheHeaders();
if(!$this->request_data) {
if(!$request_data) {
$error = __('Invalid or missing request data.', 'mailpoet');
} else {
if(!$this->settings_daemon_data) {
$error = __('Daemon does not exist.', 'mailpoet');
} else {
if(!isset($this->request_data['token']) ||
$this->request_data['token'] !== $this->settings_daemon_data['token']
if(!isset($request_data['token']) ||
$request_data['token'] !== $this->settings_daemon_data['token']
) {
$error = 'Invalid or missing token.';
}

View File

@ -18,9 +18,10 @@ class ContainerFactory {
return self::$container;
}
private static function createContainer() {
static function createContainer() {
self::$container = new ContainerBuilder();
$loader = new YamlFileLoader(self::$container, new FileLocator(__DIR__));
$loader->load('services.yml');
return self::$container;
}
}

View File

@ -3,3 +3,24 @@ parameters:
services:
_defaults:
autowire: true
MailPoet\Config\AccessControl:
class: MailPoet\Config\AccessControl
MailPoet\Cron\Daemon:
class: MailPoet\Cron\Daemon
MailPoet\Cron\DaemonHttpRunner:
class: MailPoet\Cron\DaemonHttpRunner
MailPoet\Router\Endpoints\CronDaemon:
class: MailPoet\Router\Endpoints\CronDaemon
MailPoet\Router\Endpoints\Subscription:
class: MailPoet\Router\Endpoints\Subscription
MailPoet\Router\Endpoints\Track:
class: MailPoet\Router\Endpoints\Track
MailPoet\Router\Endpoints\ViewInBrowser:
class: MailPoet\Router\Endpoints\ViewInBrowser

View File

@ -23,13 +23,15 @@ class CronDaemon {
'global' => AccessControl::NO_ACCESS_RESTRICTION
);
function __construct($data) {
$this->data = $data;
/** @var DaemonHttpRunner */
private $daemon_runner;
function __construct(DaemonHttpRunner $daemon_runner) {
$this->daemon_runner = $daemon_runner;
}
function run() {
$queue = new DaemonHttpRunner($this->data);
$queue->run();
function run($data) {
$this->daemon_runner->run($data);
}
function ping() {
@ -37,7 +39,6 @@ class CronDaemon {
}
function pingResponse() {
$queue = new DaemonHttpRunner();
$queue->ping();
$this->daemon_runner->ping();
}
}

View File

@ -17,30 +17,25 @@ class Subscription {
self::ACTION_MANAGE,
self::ACTION_UNSUBSCRIBE
);
public $data;
public $permissions = array(
'global' => AccessControl::NO_ACCESS_RESTRICTION
);
function __construct($data) {
$this->data = $data;
}
function confirm() {
$subscription = $this->initSubscriptionPage(UserSubscription\Pages::ACTION_CONFIRM);
function confirm($data) {
$subscription = $this->initSubscriptionPage(UserSubscription\Pages::ACTION_CONFIRM, $data);
$subscription->confirm();
}
function manage() {
$subscription = $this->initSubscriptionPage(UserSubscription\Pages::ACTION_MANAGE);
function manage($data) {
$subscription = $this->initSubscriptionPage(UserSubscription\Pages::ACTION_MANAGE, $data);
}
function unsubscribe() {
$subscription = $this->initSubscriptionPage(UserSubscription\Pages::ACTION_UNSUBSCRIBE);
function unsubscribe($data) {
$subscription = $this->initSubscriptionPage(UserSubscription\Pages::ACTION_UNSUBSCRIBE, $data);
$subscription->unsubscribe();
}
private function initSubscriptionPage($action) {
return new UserSubscription\Pages($action, $this->data, true, true);
private function initSubscriptionPage($action, $data) {
return new UserSubscription\Pages($action, $data, true, true);
}
}

View File

@ -22,23 +22,18 @@ class Track {
self::ACTION_CLICK,
self::ACTION_OPEN
);
public $data;
public $permissions = array(
'global' => AccessControl::NO_ACCESS_RESTRICTION
);
function __construct($data) {
$this->data = $this->_processTrackData($data);
}
function click() {
function click($data) {
$click_event = new Clicks();
return $click_event->track($this->data);
return $click_event->track($this->_processTrackData($data));
}
function open() {
function open($data) {
$open_event = new Opens();
return $open_event->track($this->data);
return $open_event->track($this->_processTrackData($data));
}
function _processTrackData($data) {
@ -82,7 +77,7 @@ class Track {
false;
}
private function terminate($code) {
function terminate($code) {
status_header($code);
get_template_part((string)$code);
exit;

View File

@ -17,19 +17,18 @@ class ViewInBrowser {
const ENDPOINT = 'view_in_browser';
const ACTION_VIEW = 'view';
public $allowed_actions = array(self::ACTION_VIEW);
public $data;
public $permissions = array(
'global' => AccessControl::NO_ACCESS_RESTRICTION
);
function __construct($data, AccessControl $access_control) {
function __construct(AccessControl $access_control) {
$this->access_control = $access_control;
$this->data = $this->_processBrowserPreviewData($data);
}
function view() {
function view($data) {
$data = $this->_processBrowserPreviewData($data);
$view_in_browser = new NewsletterViewInBrowser();
return $this->_displayNewsletter($view_in_browser->view($this->data));
return $this->_displayNewsletter($view_in_browser->view($data));
}
function _processBrowserPreviewData($data) {

View File

@ -3,6 +3,7 @@
namespace MailPoet\Router;
use MailPoet\Config\AccessControl;
use MailPoet\Dependencies\Symfony\Component\DependencyInjection\Container;
use MailPoet\Util\Helpers;
if(!defined('ABSPATH')) exit;
@ -12,11 +13,13 @@ class Router {
public $endpoint;
public $action;
public $data;
/** @var Container */
private $container;
const NAME = 'mailpoet_router';
const RESPONSE_ERROR = 404;
const RESPONE_FORBIDDEN = 403;
function __construct(AccessControl $access_control, $api_data = false) {
function __construct(AccessControl $access_control, Container $container, $api_data = false) {
$api_data = ($api_data) ? $api_data : $_GET;
$this->api_request = isset($api_data[self::NAME]);
$this->endpoint = isset($api_data['endpoint']) ?
@ -29,15 +32,19 @@ class Router {
self::decodeRequestData($api_data['data']) :
array();
$this->access_control = $access_control;
$this->container = $container;
}
function init() {
$endpoint_class = __NAMESPACE__ . "\\Endpoints\\" . ucfirst($this->endpoint);
if(!$this->api_request) return;
$endpoint_class = __NAMESPACE__ . "\\Endpoints\\" . ucfirst($this->endpoint);
if(!$this->endpoint || !class_exists($endpoint_class)) {
return $this->terminateRequest(self::RESPONSE_ERROR, __('Invalid router endpoint', 'mailpoet'));
}
$endpoint = new $endpoint_class($this->data, $this->access_control);
$endpoint = $this->container->get($endpoint_class);
if(!method_exists($endpoint, $this->endpoint_action) || !in_array($this->endpoint_action, $endpoint->allowed_actions)) {
return $this->terminateRequest(self::RESPONSE_ERROR, __('Invalid router endpoint action', 'mailpoet'));
}
@ -46,10 +53,11 @@ class Router {
}
do_action('mailpoet_conflict_resolver_router_url_query_parameters');
return call_user_func(
array(
[
$endpoint,
$this->endpoint_action
)
$this->endpoint_action,
],
$this->data
);
}

View File

@ -14,15 +14,14 @@ class DaemonHttpRunnerTest extends \MailPoetTest {
CronHelper::DAEMON_SETTING,
[]
);
$daemon = new DaemonHttpRunner($request_data = 'request data');
expect($daemon->request_data)->equals('request data');
$daemon = new DaemonHttpRunner();
expect(strlen($daemon->timer))->greaterOrEquals(5);
expect(strlen($daemon->token))->greaterOrEquals(5);
}
function testItDoesNotRunWithoutRequestData() {
$daemon = Stub::construct(
new DaemonHttpRunner(),
new DaemonHttpRunner(new Daemon()),
array(),
array(
'abortWithError' => function($message) {
@ -30,13 +29,12 @@ class DaemonHttpRunnerTest extends \MailPoetTest {
}
)
);
$daemon->request_data = false;
expect($daemon->run())->equals('Invalid or missing request data.');
expect($daemon->run(false))->equals('Invalid or missing request data.');
}
function testItDoesNotRunWhenThereIsInvalidOrMissingToken() {
$daemon = Stub::construct(
new DaemonHttpRunner(),
new DaemonHttpRunner(new Daemon()),
array(),
array(
'abortWithError' => function($message) {
@ -47,8 +45,7 @@ class DaemonHttpRunnerTest extends \MailPoetTest {
$daemon->settings_daemon_data = array(
'token' => 123
);
$daemon->request_data = array('token' => 456);
expect($daemon->run())->equals('Invalid or missing token.');
expect($daemon->run(['token' => 456]))->equals('Invalid or missing token.');
}
function testItStoresErrorMessageAndContinuesExecutionWhenWorkersThrowException() {
@ -63,13 +60,13 @@ class DaemonHttpRunnerTest extends \MailPoetTest {
throw new \Exception();
},
), $this);
$daemon_http_runner = Stub::make(new DaemonHttpRunner($data, $daemon), array(
$daemon_http_runner = Stub::make(new DaemonHttpRunner($daemon), array(
'pauseExecution' => null,
'callSelf' => null
), $this);
Setting::setValue(CronHelper::DAEMON_SETTING, $data);
$daemon_http_runner->__construct($data, $daemon);
$daemon_http_runner->run();
$daemon_http_runner->__construct($daemon);
$daemon_http_runner->run($data);
$updated_daemon = Setting::getValue(CronHelper::DAEMON_SETTING);
expect($updated_daemon['last_error'])->greaterOrEquals('Message');
}
@ -79,7 +76,7 @@ class DaemonHttpRunnerTest extends \MailPoetTest {
'executeScheduleWorker' => null,
'executeQueueWorker' => null,
), $this);
$daemon_http_runner = Stub::make(new DaemonHttpRunner(true, $daemon), array(
$daemon_http_runner = Stub::make(new DaemonHttpRunner($daemon), array(
'pauseExecution' => Expected::exactly(1, function($pause_delay) {
expect($pause_delay)->lessThan(CronHelper::DAEMON_EXECUTION_LIMIT);
expect($pause_delay)->greaterThan(CronHelper::DAEMON_EXECUTION_LIMIT - 1);
@ -90,13 +87,13 @@ class DaemonHttpRunnerTest extends \MailPoetTest {
'token' => 123
);
Setting::setValue(CronHelper::DAEMON_SETTING, $data);
$daemon_http_runner->__construct($data, $daemon);
$daemon_http_runner->run();
$daemon_http_runner->__construct($daemon);
$daemon_http_runner->run($data);
}
function testItTerminatesExecutionWhenDaemonIsDeleted() {
$daemon = Stub::make(new DaemonHttpRunner(true), array(
$daemon = Stub::make(new DaemonHttpRunner(new Daemon()), array(
'executeScheduleWorker' => function() {
Setting::deleteValue(CronHelper::DAEMON_SETTING);
},
@ -108,12 +105,12 @@ class DaemonHttpRunnerTest extends \MailPoetTest {
'token' => 123
);
Setting::setValue(CronHelper::DAEMON_SETTING, $data);
$daemon->__construct($data);
$daemon->run();
$daemon->__construct(new Daemon());
$daemon->run($data);
}
function testItTerminatesExecutionWhenDaemonTokenChangesAndKeepsChangedToken() {
$daemon = Stub::make(new DaemonHttpRunner(true), array(
$daemon = Stub::make(new DaemonHttpRunner(new Daemon()), array(
'executeScheduleWorker' => function() {
Setting::setValue(
CronHelper::DAEMON_SETTING,
@ -128,14 +125,14 @@ class DaemonHttpRunnerTest extends \MailPoetTest {
'token' => 123
);
Setting::setValue(CronHelper::DAEMON_SETTING, $data);
$daemon->__construct($data);
$daemon->run();
$daemon->__construct(new Daemon());
$daemon->run($data);
$data_after_run = Setting::getValue(CronHelper::DAEMON_SETTING);
expect($data_after_run['token'], 567);
}
function testItTerminatesExecutionWhenDaemonIsDeactivated() {
$daemon = Stub::make(new DaemonHttpRunner(true), [
$daemon = Stub::make(new DaemonHttpRunner(new Daemon()), [
'executeScheduleWorker' => null,
'executeQueueWorker' => null,
'pauseExecution' => null,
@ -146,12 +143,12 @@ class DaemonHttpRunnerTest extends \MailPoetTest {
'status' => CronHelper::DAEMON_STATUS_INACTIVE,
];
Setting::setValue(CronHelper::DAEMON_SETTING, $data);
$daemon->__construct($data);
$daemon->run();
$daemon->__construct(new Daemon());
$daemon->run($data);
}
function testItUpdatesDaemonTokenDuringExecution() {
$daemon_http_runner = Stub::make(new DaemonHttpRunner(true), array(
$daemon_http_runner = Stub::make(new DaemonHttpRunner(new Daemon()), array(
'executeScheduleWorker' => null,
'executeQueueWorker' => null,
'pauseExecution' => null,
@ -161,8 +158,8 @@ class DaemonHttpRunnerTest extends \MailPoetTest {
'token' => 123
);
Setting::setValue(CronHelper::DAEMON_SETTING, $data);
$daemon_http_runner->__construct($data);
$daemon_http_runner->run();
$daemon_http_runner->__construct(new Daemon());
$daemon_http_runner->run($data);
$updated_daemon = Setting::getValue(CronHelper::DAEMON_SETTING);
expect($updated_daemon['token'])->equals($daemon_http_runner->token);
}
@ -174,7 +171,7 @@ class DaemonHttpRunnerTest extends \MailPoetTest {
},
'executeQueueWorker' => null,
), $this);
$daemon_http_runner = Stub::make(new DaemonHttpRunner(true, $daemon), array(
$daemon_http_runner = Stub::make(new DaemonHttpRunner($daemon), array(
'pauseExecution' => null,
'callSelf' => null
), $this);
@ -183,8 +180,8 @@ class DaemonHttpRunnerTest extends \MailPoetTest {
);
$now = time();
Setting::setValue(CronHelper::DAEMON_SETTING, $data);
$daemon_http_runner->__construct($data, $daemon);
$daemon_http_runner->run();
$daemon_http_runner->__construct($daemon);
$daemon_http_runner->run($data);
$updated_daemon = Setting::getValue(CronHelper::DAEMON_SETTING);
expect($updated_daemon['run_started_at'])->greaterOrEquals($now);
expect($updated_daemon['run_started_at'])->lessThan($now + 2);
@ -195,7 +192,7 @@ class DaemonHttpRunnerTest extends \MailPoetTest {
function testItCanRun() {
ignore_user_abort(0);
expect(ignore_user_abort())->equals(0);
$daemon = Stub::make(new DaemonHttpRunner(true), array(
$daemon = Stub::make(new DaemonHttpRunner(new Daemon()), array(
'pauseExecution' => null,
// workers should be executed
'executeScheduleWorker' => Expected::exactly(1),
@ -207,13 +204,13 @@ class DaemonHttpRunnerTest extends \MailPoetTest {
'token' => 123
);
Setting::setValue(CronHelper::DAEMON_SETTING, $data);
$daemon->__construct($data);
$daemon->run();
$daemon->__construct(new Daemon());
$daemon->run($data);
expect(ignore_user_abort())->equals(1);
}
function testItRespondsToPingRequest() {
$daemon = Stub::make(new DaemonHttpRunner(true), array(
$daemon = Stub::make(new DaemonHttpRunner(new Daemon()), array(
'terminateRequest' => Expected::exactly(1, function($message) {
expect($message)->equals('pong');
})

View File

@ -1,7 +1,7 @@
<?php
namespace MailPoet\Test\Router\Endpoints;
use AspectMock\Test as Mock;
use Codeception\Stub;
use MailPoet\Models\Newsletter;
use MailPoet\Models\NewsletterLink;
use MailPoet\Models\ScheduledTask;
@ -74,9 +74,10 @@ class TrackTest extends \MailPoetTest {
)
);
$data->subscriber->email = 'random@email.com';
$track = Mock::double($this->track, array('terminate' => null));
$track = Stub::make(new Track(), ['terminate' => function($code) {
expect($code)->equals(403);
}]);
$track->_validateTrackData($data);
$track->verifyInvokedOnce('terminate', array(403));
}
function testItFailsWhenSubscriberIsNotOnProcessedList() {

View File

@ -39,7 +39,7 @@ class ViewInBrowserTest extends \MailPoetTest {
'preview' => false
);
// instantiate class
$this->view_in_browser = new ViewInBrowser($this->browser_preview_data, new AccessControl());
$this->view_in_browser = new ViewInBrowser(new AccessControl());
}
function testItAbortsWhenBrowserPreviewDataIsMissing() {
@ -203,8 +203,7 @@ class ViewInBrowserTest extends \MailPoetTest {
$view_in_browser = Stub::make($this->view_in_browser, array(
'_displayNewsletter' => Expected::exactly(1)
), $this);
$view_in_browser->data = $view_in_browser->_processBrowserPreviewData($this->browser_preview_data);
$view_in_browser->view();
$view_in_browser->view($this->browser_preview_data);
}
function _after() {

View File

@ -5,6 +5,9 @@ namespace MailPoet\Test\Router;
use Codeception\Stub;
use Codeception\Stub\Expected;
use MailPoet\Config\AccessControl;
use MailPoet\Dependencies\Symfony\Component\DependencyInjection\Container;
use MailPoet\DI\ContainerFactory;
use MailPoet\Router\Endpoints\RouterTestMockEndpoint;
use MailPoet\Router\Router;
require_once('RouterTestMockEndpoint.php');
@ -12,6 +15,8 @@ require_once('RouterTestMockEndpoint.php');
class RouterTest extends \MailPoetTest {
public $access_control;
public $router_data;
/** @var Container */
private $container;
function _before() {
$this->router_data = array(
@ -21,7 +26,10 @@ class RouterTest extends \MailPoetTest {
'data' => base64_encode(json_encode(array('data' => 'dummy data')))
);
$this->access_control = new AccessControl();
$this->router = new Router($this->access_control, $this->router_data);
$this->container = ContainerFactory::createContainer();
$this->container->register(RouterTestMockEndpoint::class)->setPublic(true);
$this->container->compile();
$this->router = new Router($this->access_control, $this->container, $this->router_data);
}
function testItCanGetAPIDataFromGetRequest() {
@ -29,7 +37,7 @@ class RouterTest extends \MailPoetTest {
$url = 'http://example.com/?' . Router::NAME . '&endpoint=view_in_browser&action=view&data='
. base64_encode(json_encode($data));
parse_str(parse_url($url, PHP_URL_QUERY), $_GET);
$router = new Router($this->access_control);
$router = new Router($this->access_control, $this->container);
expect($router->api_request)->equals(true);
expect($router->endpoint)->equals('viewInBrowser');
expect($router->endpoint_action)->equals('view');
@ -41,7 +49,7 @@ class RouterTest extends \MailPoetTest {
unset($router_data[Router::NAME]);
$router = Stub::construct(
'\MailPoet\Router\Router',
array($this->access_control, $router_data)
array($this->access_control, $this->container, $router_data)
);
$result = $router->init();
expect($result)->null();
@ -52,7 +60,7 @@ class RouterTest extends \MailPoetTest {
$router_data['endpoint'] = 'invalid_endpoint';
$router = Stub::construct(
'\MailPoet\Router\Router',
array($this->access_control, $router_data),
array($this->access_control, $this->container, $router_data),
array(
'terminateRequest' => function($code, $error) {
return array(
@ -76,7 +84,7 @@ class RouterTest extends \MailPoetTest {
$router_data['action'] = 'invalid_action';
$router = Stub::construct(
'\MailPoet\Router\Router',
array($this->access_control, $router_data),
array($this->access_control, $this->container, $router_data),
array(
'terminateRequest' => function($code, $error) {
return array(
@ -164,7 +172,7 @@ class RouterTest extends \MailPoetTest {
function testItValidatesPermissionBeforeProcessingEndpointAction() {
$router = Stub::construct(
'\MailPoet\Router\Router',
array($this->access_control, $this->router_data),
array($this->access_control, $this->container, $this->router_data),
array(
'validatePermissions' => function($action, $permissions) {
expect($action)->equals($this->router_data['action']);
@ -186,7 +194,7 @@ class RouterTest extends \MailPoetTest {
function testItReturnsForbiddenResponseWhenPermissionFailsValidation() {
$router = Stub::construct(
'\MailPoet\Router\Router',
array($this->access_control, $this->router_data),
array($this->access_control, $this->container, $this->router_data),
array(
'validatePermissions' => false,
'terminateRequest' => function($code, $error) {

View File

@ -14,11 +14,7 @@ class RouterTestMockEndpoint {
'global' => AccessControl::NO_ACCESS_RESTRICTION
);
function __construct($data) {
$this->data = $data;
}
function test() {
return $this->data;
function test($data) {
return $data;
}
}