Use Cookies service within Session and add unit test

[MAILPOET-2248]

lib/Config/Session.php

@@ -2,6 +2,7 @@
 
 namespace MailPoet\Config;
 
+use MailPoet\Util\Cookies;
 use MailPoet\Util\Security;
 
 class Session
@@ -10,30 +11,41 @@ class Session
   const KEY_LENGTH = 32;
   const COOKIE_EXPIRATION = 84600; // day
 
+  /** @var Cookies */
+  private $cookies;
+
+  function __construct(Cookies $cookies) {
+    $this->cookies = $cookies;
+  }
+
   function getId() {
-    return isset($_COOKIE[self::COOKIE_NAME]) ? $_COOKIE[self::COOKIE_NAME] : null;
+    return $this->cookies->get(self::COOKIE_NAME) ?: null;
   }
 
   function init() {
-    $id = $this->getId() ?: Security::generateRandomString(self::KEY_LENGTH);
+    if (headers_sent()) {
-    if (!headers_sent()) {
+      return false;
-      $this->setCookie($id);
     }
+    $id = $this->getId() ?: Security::generateRandomString(self::KEY_LENGTH);
+    $this->setCookie($id);
+    return true;
   }
 
   function destroy() {
     if ($this->getId() === null) {
       return;
     }
-    unset($_COOKIE[self::COOKIE_NAME]);
+    $this->cookies->delete(self::COOKIE_NAME);
   }
 
   private function setCookie($id) {
-    setcookie(
+    $this->cookies->set(
       self::COOKIE_NAME,
       $id,
-      time() + self::COOKIE_EXPIRATION,
+      [
-      "/"
+        'expires' => time() + self::COOKIE_EXPIRATION,
+        'path' => '/',
+      ]
     );
   }
 }

lib/Subscription/Captcha.php

@@ -5,6 +5,7 @@ namespace MailPoet\Subscription;
 use MailPoet\Config\Session;
 use MailPoet\Models\Subscriber;
 use MailPoet\Models\SubscriberIP;
+use MailPoet\Util\Cookies;
 use MailPoet\Util\Helpers;
 use MailPoet\WP\Functions as WPFunctions;
 use MailPoetVendor\Gregwar\Captcha\CaptchaBuilder;
@@ -25,7 +26,7 @@ class Captcha {
       $wp = new WPFunctions;
     }
     if ($captcha_session === null) {
-      $captcha_session = new CaptchaSession($wp, new Session());
+      $captcha_session = new CaptchaSession($wp, new Session(new Cookies()));
     }
     $this->wp = $wp;
     $this->captcha_session = $captcha_session;

lib/Util/Cookies.php

@@ -44,4 +44,8 @@ class Cookies {
     }
     return $value;
   }
+
+  function delete($name) {
+    unset($_COOKIE[$name]);
+  }
 }

tests/integration/API/JSON/v1/SubscribersTest.php

@@ -8,6 +8,8 @@ use MailPoet\API\JSON\Response as APIResponse;
 use MailPoet\Config\Session;
 use MailPoet\DI\ContainerWrapper;
 use MailPoet\Form\Util\FieldNameObfuscator;
+use MailPoet\Listing\BulkActionController;
+use MailPoet\Listing\Handler;
 use MailPoet\Models\CustomField;
 use MailPoet\Models\Form;
 use MailPoet\Models\Newsletter;
@@ -19,10 +21,14 @@ use MailPoet\Models\SubscriberIP;
 use MailPoet\Models\Segment;
 use MailPoet\Models\Setting;
 use MailPoet\Models\SubscriberSegment;
+use MailPoet\Segments\SubscribersListings;
 use MailPoet\Settings\SettingsController;
+use MailPoet\Subscribers\RequiredCustomFieldValidator;
 use MailPoet\Subscribers\Source;
+use MailPoet\Subscribers\SubscriberActions;
 use MailPoet\Subscription\Captcha;
 use MailPoet\Subscription\CaptchaSession;
+use MailPoet\Util\Cookies;
 use MailPoet\WP\Functions;
 
 class SubscribersTest extends \MailPoetTest {
@@ -33,10 +39,29 @@ class SubscribersTest extends \MailPoetTest {
   /** @var SettingsController */
   private $settings;
 
+  /** @var CaptchaSession */
+  private $captcha_session;
+
   function _before() {
     parent::_before();
     $this->cleanup();
-    $this->endpoint = ContainerWrapper::getInstance()->get(Subscribers::class);
+    $cookies_mock = $this->createMock(Cookies::class);
+    $cookies_mock->method('get')
+      ->willReturn('abcd');
+    $session = new Session($cookies_mock);
+    $container = ContainerWrapper::getInstance();
+    $this->captcha_session = new CaptchaSession($container->get(Functions::class), $session);
+    $this->endpoint = new Subscribers(
+      $container->get(BulkActionController::class),
+      $container->get(SubscribersListings::class),
+      $container->get(SubscriberActions::class),
+      $container->get(RequiredCustomFieldValidator::class),
+      $container->get(Handler::class),
+      $container->get(Captcha::class),
+      $container->get(Functions::class),
+      $container->get(SettingsController::class),
+      $this->captcha_session
+    );
     $obfuscator = new FieldNameObfuscator();
     $this->obfuscatedEmail = $obfuscator->obfuscate('email');
     $this->obfuscatedSegments = $obfuscator->obfuscate('segments');
@@ -80,9 +105,6 @@ class SubscribersTest extends \MailPoetTest {
       'address' => 'sender@mailpoet.com',
       'name' => 'Sender',
     ]);
-
-    // MAILPOET SESSION
-    $_COOKIE[Session::COOKIE_NAME] = 'abcd';
   }
 
   function testItCanGetASubscriber() {
@@ -523,8 +545,7 @@ class SubscribersTest extends \MailPoetTest {
     $subscriber->count_confirmations = 1;
     $subscriber->save();
     $captcha_value = 'ihg5w';
-    $captcha_session = new CaptchaSession(new Functions(), new Session());
+    $this->captcha_session->setCaptchaHash('ihg5w');
-    $captcha_session->setCaptchaHash($captcha_value);
     $response = $this->endpoint->subscribe([
       $this->obfuscatedEmail => $email,
       'form_id' => $this->form->id,

tests/integration/Subscription/CaptchaTest.php

@@ -8,13 +8,25 @@ use MailPoet\Models\Subscriber;
 use MailPoet\Models\SubscriberIP;
 use MailPoet\Subscription\Captcha;
 use MailPoet\Subscription\CaptchaSession;
+use MailPoet\Util\Cookies;
 use MailPoet\WP\Functions as WPFunctions;
 use MailPoet\WP\Functions;
 
 class CaptchaTest extends \MailPoetTest {
+
+  /** @var Captcha */
+  private $captcha;
+
+  /** @var CaptchaSession */
+  private $captcha_session;
+
   function _before() {
-    $this->captcha = new Captcha(new WPFunctions);
+    $cookies_mock = $this->createMock(Cookies::class);
+    $cookies_mock->method('get')->willReturn('abcd');
+    $this->captcha_session = new CaptchaSession(new Functions(), new Session($cookies_mock));
+    $this->captcha = new Captcha(new WPFunctions, $this->captcha_session);
     $_SERVER['REMOTE_ADDR'] = '127.0.0.1';
+    $this->captcha_session->reset();
   }
 
   function testItDoesNotRequireCaptchaForTheFirstSubscription() {
@@ -42,13 +54,11 @@ class CaptchaTest extends \MailPoetTest {
     expect($result)->equals(true);
   }
 
-  function testItRendersImage() {
+  function testItRendersImageAndStoresHashToSession() {
-    $_COOKIE[Session::COOKIE_NAME] = 'abcd';
+    expect($this->captcha_session->getCaptchaHash())->false();
-    $captcha_session = new CaptchaSession(new Functions(), new Session());
-    expect($captcha_session->getCaptchaHash())->false();
     $image = $this->captcha->renderImage(null, null, true);
     expect($image)->notEmpty();
-    expect($captcha_session->getCaptchaHash())->notEmpty();
+    expect($this->captcha_session->getCaptchaHash())->notEmpty();
   }
 
   function _after() {

tests/unit/Config/SessionTest.php

@@ -0,0 +1,79 @@
+<?php
+namespace MailPoet\Config;
+
+use MailPoet\Util\Cookies;
+use PHPUnit_Framework_MockObject_MockObject as MockObject;
+
+class SessionTest extends \MailPoetUnitTest {
+
+  /** @var Session */
+  private $session;
+
+  /** @var MockObject */
+  private $cookies_mock;
+
+  function _before() {
+    parent::_before();
+    unset($_COOKIE[Session::COOKIE_NAME]);
+    $this->cookies_mock = $this->createMock(Cookies::class);
+    $this->session = new Session($this->cookies_mock);
+  }
+
+  function testItInitializesNewSessionCorrectly() {
+    $this->cookies_mock
+      ->expects($this->once())
+      ->method('get')
+      ->willReturn(null);
+    $this->cookies_mock
+      ->expects($this->once())
+      ->method('set')
+      ->with(
+        $this->equalTo(Session::COOKIE_NAME),
+        $this->isType('string'),
+        $this->callback(function ($options) {
+          if (!isset($options['expires']) || $options['expires'] < time()) {
+            return false;
+          }
+          if ($options['path'] !== '/') {
+            return false;
+          }
+          return true;
+        })
+      );
+    $this->session->init();
+  }
+
+  function testItPrologsCurrentSessionDuringInitialization() {
+    $session_id = 'abcd';
+    $this->cookies_mock
+      ->expects($this->once())
+      ->method('get')
+      ->willReturn($session_id);
+    $this->cookies_mock
+      ->expects($this->once())
+      ->method('set')
+      ->with(
+        $this->equalTo(Session::COOKIE_NAME),
+        $this->equalTo($session_id),
+        $this->callback(function ($options) {
+          if (!isset($options['expires']) || $options['expires'] < time()) {
+            return false;
+          }
+          if ($options['path'] !== '/') {
+            return false;
+          }
+          return true;
+        })
+      );
+    $this->session->init();
+  }
+
+  function testItReturnsSessionIdCorrectly() {
+    $session_id = 'abcd';
+    $this->cookies_mock
+      ->expects($this->once())
+      ->method('get')
+      ->willReturn($session_id);
+    expect($this->session->getid())->equals($session_id);
+  }
+}