Refactor URL helper to DI and move its test to unit [MAILPOET-2009]

lib/Config/Changelog.php

@@ -17,14 +17,19 @@ class Changelog {
   /** @var Helper */
   private $wooCommerceHelper;
 
+  /** @var Url */
+  private $url_helper;
+
   function __construct(
     SettingsController $settings,
     WPFunctions $wp,
-    Helper $wooCommerceHelper
+    Helper $wooCommerceHelper,
+    Url $url_helper
   ) {
     $this->wooCommerceHelper = $wooCommerceHelper;
     $this->settings = $settings;
     $this->wp = $wp;
+    $this->url_helper = $url_helper;
   }
 
   function init() {
@@ -101,13 +106,13 @@ class Changelog {
       && $this->wooCommerceHelper->getOrdersCount() >= 1
       && $this->wp->currentUserCan('administrator')
     ) {
-      Url::redirectTo($this->wp->adminUrl('admin.php?page=mailpoet-woocommerce-list-import'));
+      $this->url_helper->redirectTo($this->wp->adminUrl('admin.php?page=mailpoet-woocommerce-list-import'));
     }
   }
 
   private function terminateWithRedirect($redirect_url) {
     // save version number
     $this->settings->set('version', Env::$version);
-    Url::redirectWithReferer($redirect_url);
+    $this->url_helper->redirectWithReferer($redirect_url);
   }
 }

lib/Config/Hooks.php

@@ -6,6 +6,7 @@ use MailPoet\Settings\SettingsController;
 use MailPoet\Statistics\Track\WooCommercePurchases;
 use MailPoet\Subscription\Comment;
 use MailPoet\Subscription\Form;
+use MailPoet\Subscription\Manage;
 use MailPoet\Subscription\Registration;
 use MailPoet\Segments\WooCommerce as WooCommerceSegment;
 use MailPoet\WooCommerce\Subscription as WooCommerceSubscription;
@@ -19,6 +20,9 @@ class Hooks {
   /** @var Comment */
   private $subscription_comment;
 
+  /** @var Manage */
+  private $subscription_manage;
+
   /** @var Registration */
   private $subscription_registration;
 
@@ -40,6 +44,7 @@ class Hooks {
   function __construct(
     Form $subscription_form,
     Comment $subscription_comment,
+    Manage $subscription_manage,
     Registration $subscription_registration,
     SettingsController $settings,
     WPFunctions $wp,
@@ -49,6 +54,7 @@ class Hooks {
   ) {
     $this->subscription_form = $subscription_form;
     $this->subscription_comment = $subscription_comment;
+    $this->subscription_manage = $subscription_manage;
     $this->subscription_registration = $subscription_registration;
     $this->settings = $settings;
     $this->wp = $wp;
@@ -138,11 +144,11 @@ class Hooks {
     // Manage subscription
     $this->wp->addAction(
       'admin_post_mailpoet_subscription_update',
-      '\MailPoet\Subscription\Manage::onSave'
+      [$this->subscription_manage, 'onSave']
     );
     $this->wp->addAction(
       'admin_post_nopriv_mailpoet_subscription_update',
-      '\MailPoet\Subscription\Manage::onSave'
+      [$this->subscription_manage, 'onSave']
     );
 
     // Subscription form

lib/DI/ContainerConfigurator.php

@@ -71,6 +71,8 @@ class ContainerConfigurator implements IContainerConfigurator {
     $container->autowire(\MailPoet\Cron\Workers\SendingQueue\SendingErrorHandler::class)->setPublic(true);
     $container->autowire(\MailPoet\Cron\Workers\StatsNotifications\Scheduler::class);
     $container->autowire(\MailPoet\Cron\CronTrigger::class)->setPublic(true);
+    // Form
+    $container->autowire(\MailPoet\Form\Util\FieldNameObfuscator::class)->setPublic(true);
     // Listing
     $container->autowire(\MailPoet\Listing\BulkActionController::class)->setPublic(true);
     $container->autowire(\MailPoet\Listing\BulkActionFactory::class)->setPublic(true);
@@ -103,9 +105,12 @@ class ContainerConfigurator implements IContainerConfigurator {
     // Subscription
     $container->autowire(\MailPoet\Subscription\Comment::class)->setPublic(true);
     $container->autowire(\MailPoet\Subscription\Form::class)->setPublic(true);
+    $container->autowire(\MailPoet\Subscription\Manage::class)->setPublic(true);
     $container->autowire(\MailPoet\Subscription\Registration::class)->setPublic(true);
     // Newsletter
     $container->autowire(\MailPoet\Newsletter\AutomatedLatestContent::class)->setPublic(true);
+    // Util
+    $container->autowire(\MailPoet\Util\Url::class)->setPublic(true);
     // WooCommerce
     $container->autowire(\MailPoet\WooCommerce\Helper::class)->setPublic(true);
     $container->autowire(\MailPoet\WooCommerce\Subscription::class)->setPublic(true);

lib/Subscription/Form.php

@@ -11,8 +11,12 @@ class Form {
   /** @var API */
   private $api;
 
-  function __construct(API $api) {
+  /** @var UrlHelper */
+  private $url_helper;
+
+  function __construct(API $api, UrlHelper $url_helper) {
     $this->api = $api;
+    $this->url_helper = $url_helper;
   }
 
   function onSubmit($request_data = false) {
@@ -21,7 +25,7 @@ class Form {
     $form_id = (!empty($request_data['data']['form_id'])) ? (int)$request_data['data']['form_id'] : false;
     $response = $this->api->processRoute();
     if ($response->status !== APIResponse::STATUS_OK) {
-      return UrlHelper::redirectBack(
+      return $this->url_helper->redirectBack(
         array(
           'mailpoet_error' => ($form_id) ? $form_id : true,
           'mailpoet_success' => null
@@ -29,8 +33,8 @@ class Form {
       );
     } else {
       return (isset($response->meta['redirect_url'])) ?
-        UrlHelper::redirectTo($response->meta['redirect_url']) :
+        $this->url_helper->redirectTo($response->meta['redirect_url']) :
-        UrlHelper::redirectBack(
+        $this->url_helper->redirectBack(
           array(
             'mailpoet_success' => $form_id,
             'mailpoet_error' => null

lib/Subscription/Manage.php

@@ -5,33 +5,43 @@ namespace MailPoet\Subscription;
 use MailPoet\Form\Util\FieldNameObfuscator;
 use MailPoet\Models\CustomField;
 use MailPoet\Models\Subscriber;
-use MailPoet\Util\Url;
+use MailPoet\Util\Url as UrlHelper;
 
 class Manage {
 
-  static function onSave() {
+  /** @var UrlHelper */
+  private $url_helper;
+
+  /** @var FieldNameObfuscator */
+  private $field_name_obfuscator;
+
+  function __construct(UrlHelper $url_helper, FieldNameObfuscator $field_name_obfuscator) {
+    $this->url_helper = $url_helper;
+    $this->field_name_obfuscator = $field_name_obfuscator;
+  }
+
+  function onSave() {
     $action = (isset($_POST['action']) ? $_POST['action'] : null);
     $token = (isset($_POST['token']) ? $_POST['token'] : null);
 
     if ($action !== 'mailpoet_subscription_update' || empty($_POST['data'])) {
-      Url::redirectBack();
+      $this->url_helper->redirectBack();
     }
     $subscriber_data = $_POST['data'];
-    $obfuscator = new FieldNameObfuscator();
+    $subscriber_data = $this->field_name_obfuscator->deobfuscateFormPayload($subscriber_data);
-    $subscriber_data = $obfuscator->deobfuscateFormPayload($subscriber_data);
 
     if (!empty($subscriber_data['email']) && Subscriber::verifyToken($subscriber_data['email'], $token)) {
       if ($subscriber_data['email'] !== Pages::DEMO_EMAIL) {
-        $subscriber = Subscriber::createOrUpdate(static::filterOutEmptyMandatoryFields($subscriber_data));
+        $subscriber = Subscriber::createOrUpdate($this->filterOutEmptyMandatoryFields($subscriber_data));
         $subscriber->getErrors();
       }
     }
 
-    Url::redirectBack();
+    $this->url_helper->redirectBack();
   }
 
-  private static function filterOutEmptyMandatoryFields(array $subscriber_data) {
+  private function filterOutEmptyMandatoryFields(array $subscriber_data) {
-    $mandatory = self::getMandatory();
+    $mandatory = $this->getMandatory();
     foreach ($mandatory as $name) {
       if (strlen(trim($subscriber_data[$name])) === 0) {
         unset($subscriber_data[$name]);
@@ -40,7 +50,7 @@ class Manage {
     return $subscriber_data;
   }
 
-  private static function getMandatory() {
+  private function getMandatory() {
     $mandatory = [];
     $required_custom_fields = CustomField::findMany();
     foreach ($required_custom_fields as $custom_field) {

lib/Subscription/Pages.php

@@ -29,6 +29,9 @@ class Pages {
   /** @var SettingsController */
   private $settings;
 
+  /** @var UrlHelper */
+  private $url_helper;
+
   function __construct($action = false, $data = array(), $init_shortcodes = false, $init_page_filters = false, $new_subscriber_notification_sender = null) {
     $this->action = $action;
     $this->data = $data;
@@ -41,6 +44,7 @@ class Pages {
       $this->new_subscriber_notification_sender = new NewSubscriberNotificationMailer();
     }
     $this->settings = new SettingsController();
+    $this->url_helper = new UrlHelper(new WPFunctions());
   }
 
   private function isPreview() {
@@ -395,7 +399,7 @@ class Pages {
       ' value="mailpoet_subscription_update" />';
     $form_html .= '<input type="hidden" name="data[segments]" value="" />';
     $form_html .= '<input type="hidden" name="mailpoet_redirect" '.
-      'value="' . htmlspecialchars(UrlHelper::getCurrentUrl(), ENT_QUOTES) . '" />';
+      'value="' . htmlspecialchars($this->url_helper->getCurrentUrl(), ENT_QUOTES) . '" />';
     $form_html .= '<input type="hidden" name="data[email]" value="'.
       $subscriber->email.
     '" />';

lib/Util/Url.php

@@ -4,47 +4,54 @@ namespace MailPoet\Util;
 use MailPoet\WP\Functions as WPFunctions;
 
 class Url {
-  static function getCurrentUrl() {
+  /** @var WPFunctions */
-    $home_url = parse_url(home_url());
+  private $wp;
-    $query_args = WPFunctions::get()->addQueryArg(null, null);
 
-    // Remove WPFunctions::get()->homeUrl() path from add_query_arg
+  function __construct(WPFunctions $wp) {
+    $this->wp = $wp;
+  }
+
+  function getCurrentUrl() {
+    $home_url = parse_url($this->wp->homeUrl());
+    $query_args = $this->wp->addQueryArg(null, null);
+
+    // Remove $this->wp->homeUrl() path from add_query_arg
     if (isset($home_url['path'])) {
       $query_args = str_replace($home_url['path'], '', $query_args);
     }
 
-    return WPFunctions::get()->homeUrl($query_args);
+    return $this->wp->homeUrl($query_args);
   }
 
-  static function redirectTo($url = null) {
+  function redirectTo($url = null) {
-    WPFunctions::get()->wpSafeRedirect($url);
+    $this->wp->wpSafeRedirect($url);
     exit();
   }
 
-  static function redirectBack($params = array()) {
+  function redirectBack($params = array()) {
     // check mailpoet_redirect parameter
     $referer = (isset($_POST['mailpoet_redirect'])
       ? $_POST['mailpoet_redirect']
-      : WPFunctions::get()->wpGetReferer()
+      : $this->wp->wpGetReferer()
     );
 
     // fallback: home_url
     if (!$referer) {
-      $referer = WPFunctions::get()->homeUrl();
+      $referer = $this->wp->homeUrl();
     }
 
     // append extra params to url
     if (!empty($params)) {
-      $referer = WPFunctions::get()->addQueryArg($params, $referer);
+      $referer = $this->wp->addQueryArg($params, $referer);
     }
 
-    self::redirectTo($referer);
+    $this->redirectTo($referer);
     exit();
   }
 
-  static function redirectWithReferer($url = null) {
+  function redirectWithReferer($url = null) {
-    $current_url = self::getCurrentUrl();
+    $current_url = $this->getCurrentUrl();
-    $url = WPFunctions::get()->addQueryArg(
+    $url = $this->wp->addQueryArg(
       array(
         'mailpoet_redirect' => urlencode($current_url)
       ),
@@ -52,7 +59,7 @@ class Url {
     );
 
     if ($url !== $current_url) {
-      self::redirectTo($url);
+      $this->redirectTo($url);
     }
     exit();
   }

tests/integration/Subscribers/ConfirmationEmailMailerTest.php

@@ -116,6 +116,7 @@ class ConfirmationEmailMailerTest extends \MailPoetTest {
   }
 
   function _after() {
+    Mock::clean();
     \ORM::raw_execute('TRUNCATE ' . Subscriber::$_table);
     \ORM::raw_execute('TRUNCATE ' . Segment::$_table);
     \ORM::raw_execute('TRUNCATE ' . SubscriberSegment::$_table);

tests/integration/Subscription/FormTest.php

@@ -1,7 +1,8 @@
 <?php
 namespace MailPoet\Test\Subscription;
 
-use AspectMock\Test as Mock;
+use Codeception\Stub;
+use MailPoet\API\JSON\API;
 use MailPoet\DI\ContainerWrapper;
 use MailPoet\Form\Util\FieldNameObfuscator;
 use MailPoet\Models\Form as FormModel;
@@ -11,6 +12,7 @@ use MailPoet\Models\Subscriber as SubscriberModel;
 use MailPoet\Settings\SettingsController;
 use MailPoet\Subscription\Form;
 use MailPoet\Util\Security;
+use MailPoet\Util\Url as UrlHelper;
 
 class FormTest extends \MailPoetTest {
 
@@ -68,18 +70,17 @@ class FormTest extends \MailPoetTest {
       )
     );
     $this->settings->set('signup_confirmation.enabled', false);
-    $this->form_controller = ContainerWrapper::getInstance()->get(Form::class);
   }
 
   function testItSubscribesAndRedirectsBackWithSuccessResponse() {
-    $mock = Mock::double('MailPoet\Util\Url', [
+    $url_helper = Stub::make(UrlHelper::class, [
       'redirectBack' => function($params) {
         return $params;
       }
-    ]);
+    ], $this);
-    $result = $this->form_controller->onSubmit($this->request_data);
+    $form_controller = new Form(ContainerWrapper::getInstance()->get(API::class), $url_helper);
+    $result = $form_controller->onSubmit($this->request_data);
     expect(SubscriberModel::findOne($this->testEmail))->notEmpty();
-    $mock->verifyInvoked('redirectBack');
     expect($result['mailpoet_success'])->equals($this->form->id);
     expect($result['mailpoet_error'])->null();
   }
@@ -92,17 +93,17 @@ class FormTest extends \MailPoetTest {
     $form_settings['success_page'] = $this->post;
     $form->settings = serialize($form_settings);
     $form->save();
-    $mock = Mock::double('MailPoet\Util\Url', [
+    $url_helper = Stub::make(UrlHelper::class, [
       'redirectTo' => function($params) {
         return $params;
       },
       'redirectBack' => function($params) {
         return $params;
       }
-    ]);
+    ], $this);
-    $result = $this->form_controller->onSubmit($this->request_data);
+    $form_controller = new Form(ContainerWrapper::getInstance()->get(API::class), $url_helper);
+    $result = $form_controller->onSubmit($this->request_data);
     expect(SubscriberModel::findOne($this->testEmail))->notEmpty();
-    $mock->verifyInvoked('redirectTo');
     expect($result)->regExp('/http.*?sample-post/i');
   }
 
@@ -110,20 +111,19 @@ class FormTest extends \MailPoetTest {
     // clear subscriber email so that subscription fails
     $request_data = $this->request_data;
     $request_data['data']['email'] = false;
-    $mock = Mock::double('MailPoet\Util\Url', [
+    $url_helper = Stub::make(UrlHelper::class, [
       'redirectBack' => function($params) {
         return $params;
       }
-    ]);
+    ], $this);
-    $result = $this->form_controller->onSubmit($request_data);
+    $form_controller = new Form(ContainerWrapper::getInstance()->get(API::class), $url_helper);
+    $result = $form_controller->onSubmit($request_data);
     expect(SubscriberModel::findMany())->isEmpty();
-    $mock->verifyInvoked('redirectBack');
     expect($result['mailpoet_error'])->equals($this->form->id);
     expect($result['mailpoet_success'])->null();
   }
 
   function _after() {
-    Mock::clean();
     wp_delete_post($this->post);
     \ORM::raw_execute('TRUNCATE ' . SegmentModel::$_table);
     \ORM::raw_execute('TRUNCATE ' . FormModel::$_table);

tests/integration/Util/UrlTest.php

@@ -1,12 +0,0 @@
-<?php
-namespace MailPoet\Test\Util;
-
-use MailPoet\Util\Url;
-
-class UrlTest extends \MailPoetTest {
-  function testCurrentUrlReturnsHomeUrlOnHome() {
-    $current_url = Url::getCurrentUrl();
-    $home_url = home_url();
-    expect($current_url)->equals($home_url);
-  }
-}

tests/unit/Util/UrlTest.php

@@ -0,0 +1,18 @@
+<?php
+namespace MailPoet\Test\Util;
+
+use Codeception\Stub;
+use MailPoet\Util\Url;
+use MailPoet\WP\Functions as WPFunctions;
+
+class UrlTest extends \MailPoetUnitTest {
+  function testCurrentUrlReturnsHomeUrlOnHome() {
+    $home_url = 'http://example.com';
+    $url_helper = new Url(Stub::make(new WPFunctions(), [
+      'homeUrl' => $home_url,
+      'addQueryArg' => '',
+    ]));
+    $current_url = $url_helper->getCurrentUrl();
+    expect($current_url)->equals($home_url);
+  }
+}