Use Doctrine entity in calling LinkTokens::verifyToken

[MAILPOET-3269]

lib/Newsletter/ViewInBrowser/ViewInBrowserController.php

@@ -4,7 +4,6 @@ namespace MailPoet\Newsletter\ViewInBrowser;
 
 use MailPoet\Entities\SendingQueueEntity;
 use MailPoet\Models\Newsletter;
-use MailPoet\Models\Subscriber;
 use MailPoet\Newsletter\Sending\SendingQueuesRepository;
 use MailPoet\Newsletter\Url as NewsletterUrl;
 use MailPoet\Subscribers\LinkTokens;
@@ -91,11 +90,7 @@ class ViewInBrowserController {
       throw new \InvalidArgumentException("Missing 'subscriber_token'");
     }
 
-    $subscriberModel = Subscriber::findOne($subscriber->getId());
+    if (!$this->linkTokens->verifyToken($subscriber, $data['subscriber_token'])) {
-    if (!$subscriberModel) {
-      return null;
-    }
-    if (!$this->linkTokens->verifyToken($subscriberModel, $data['subscriber_token'])) {
       throw new \InvalidArgumentException("Invalid 'subscriber_token'");
     }
     return $subscriber;

lib/Router/Endpoints/Track.php

@@ -5,7 +5,6 @@ namespace MailPoet\Router\Endpoints;
 use MailPoet\Config\AccessControl;
 use MailPoet\Cron\Workers\StatsNotifications\NewsletterLinkRepository;
 use MailPoet\Entities\SendingQueueEntity;
-use MailPoet\Models\Subscriber;
 use MailPoet\Newsletter\Links\Links;
 use MailPoet\Newsletter\NewslettersRepository;
 use MailPoet\Newsletter\Sending\SendingQueuesRepository;
@@ -99,13 +98,12 @@ class Track {
 
   public function _validateTrackData($data) {
     if (!$data->subscriber || !$data->queue || !$data->newsletter) return false;
-    $subscriberModel = Subscriber::findOne($data->subscriber->getId());
+    $subscriberTokenMatch = $this->linkTokens->verifyToken($data->subscriber, $data->subscriber_token); // phpcs:ignore Squiz.NamingConventions.ValidVariableName.NotCamelCaps
-    $subscriberTokenMatch = $this->linkTokens->verifyToken($subscriberModel, $data->subscriber_token); // phpcs:ignore Squiz.NamingConventions.ValidVariableName.NotCamelCaps
     if (!$subscriberTokenMatch) {
       $this->terminate(403);
     }
     // return if this is a WP user previewing the newsletter
-    if ($subscriberModel->isWPUser() && $data->preview) {
+    if ($data->subscriber->isWPUser() && $data->preview) {
       return $data;
     }
     // check if the newsletter was sent to the subscriber

lib/Subscription/Manage.php

@@ -6,6 +6,7 @@ use MailPoet\Entities\StatisticsUnsubscribeEntity;
 use MailPoet\Entities\SubscriberEntity;
 use MailPoet\Entities\SubscriberSegmentEntity;
 use MailPoet\Form\Util\FieldNameObfuscator;
+use MailPoet\InvalidStateException;
 use MailPoet\Models\CustomField;
 use MailPoet\Models\Segment;
 use MailPoet\Models\Subscriber;
@@ -16,6 +17,7 @@ use MailPoet\Statistics\Track\Unsubscribes;
 use MailPoet\Subscribers\LinkTokens;
 use MailPoet\Subscribers\NewSubscriberNotificationMailer;
 use MailPoet\Subscribers\SubscriberSegmentRepository;
+use MailPoet\Subscribers\SubscribersRepository;
 use MailPoet\Util\Url as UrlHelper;
 
 class Manage {
@@ -41,6 +43,9 @@ class Manage {
   /** @var WelcomeScheduler */
   private $welcomeScheduler;
 
+  /** @var SubscribersRepository */
+  private $subscribersRepository;
+
   /** @var SubscriberSegmentRepository */
   private $subscriberSegmentRepository;
 
@@ -52,6 +57,7 @@ class Manage {
     SettingsController $settings,
     NewSubscriberNotificationMailer $newSubscriberNotificationMailer,
     WelcomeScheduler $welcomeScheduler,
+    SubscribersRepository $subscribersRepository,
     SubscriberSegmentRepository $subscriberSegmentRepository
   ) {
     $this->urlHelper = $urlHelper;
@@ -61,6 +67,7 @@ class Manage {
     $this->settings = $settings;
     $this->newSubscriberNotificationMailer = $newSubscriberNotificationMailer;
     $this->welcomeScheduler = $welcomeScheduler;
+    $this->subscribersRepository = $subscribersRepository;
     $this->subscriberSegmentRepository = $subscriberSegmentRepository;
   }
 
@@ -76,25 +83,29 @@ class Manage {
 
     $result = [];
     if (!empty($subscriberData['email'])) {
-      $subscriber = Subscriber::where('email', $subscriberData['email'])->findOne();
+      $subscriber = $this->subscribersRepository->findOneBy(['email' => $subscriberData['email']]);
 
       if (
         ($subscriberData['status'] === SubscriberEntity::STATUS_UNSUBSCRIBED)
-        && ($subscriber instanceof Subscriber)
+        && ($subscriber instanceof SubscriberEntity)
-        && ($subscriber->status === SubscriberEntity::STATUS_SUBSCRIBED)
+        && ($subscriber->getStatus() === SubscriberEntity::STATUS_SUBSCRIBED)
       ) {
         $this->unsubscribesTracker->track(
-          (int)$subscriber->id,
+          (int)$subscriber->getId(),
           StatisticsUnsubscribeEntity::SOURCE_MANAGE
         );
       }
 
       if ($subscriber && $this->linkTokens->verifyToken($subscriber, $token)) {
+        $subscriberModel = Subscriber::findOne($subscriber->getId());
+        if (!$subscriberModel instanceof Subscriber) {
+          throw new InvalidStateException();
+        }
         if ($subscriberData['email'] !== Pages::DEMO_EMAIL) {
-          $this->updateSubscriptions($subscriber, $subscriberData);
+          $this->updateSubscriptions($subscriberModel, $subscriberData);
           unset($subscriberData['segments']);
-          $subscriber = Subscriber::createOrUpdate($this->filterOutEmptyMandatoryFields($subscriberData));
+          $subscriberModel = Subscriber::createOrUpdate($this->filterOutEmptyMandatoryFields($subscriberData));
-          $subscriber->getErrors();
+          $subscriberModel->getErrors();
         }
       }
       $result = ['success' => true];

lib/Subscription/Pages.php

@@ -12,6 +12,7 @@ use MailPoet\Settings\SettingsController;
 use MailPoet\Statistics\Track\Unsubscribes;
 use MailPoet\Subscribers\LinkTokens;
 use MailPoet\Subscribers\NewSubscriberNotificationMailer;
+use MailPoet\Subscribers\SubscribersRepository;
 use MailPoet\Util\Helpers;
 use MailPoet\WP\Functions as WPFunctions;
 
@@ -60,6 +61,9 @@ class Pages {
   /** @var ManageSubscriptionFormRenderer */
   private $manageSubscriptionFormRenderer;
 
+  /** @var SubscribersRepository */
+  private $subscribersRepository;
+
   public function __construct(
     NewSubscriberNotificationMailer $newSubscriberNotificationSender,
     WPFunctions $wp,
@@ -71,7 +75,8 @@ class Pages {
     AssetsController $assetsController,
     TemplateRenderer $templateRenderer,
     Unsubscribes $unsubscribesTracker,
-    ManageSubscriptionFormRenderer $manageSubscriptionFormRenderer
+    ManageSubscriptionFormRenderer $manageSubscriptionFormRenderer,
+    SubscribersRepository $subscribersRepository
   ) {
     $this->wp = $wp;
     $this->newSubscriberNotificationSender = $newSubscriberNotificationSender;
@@ -84,6 +89,7 @@ class Pages {
     $this->templateRenderer = $templateRenderer;
     $this->unsubscribesTracker = $unsubscribesTracker;
     $this->manageSubscriptionFormRenderer = $manageSubscriptionFormRenderer;
+    $this->subscribersRepository = $subscribersRepository;
   }
 
   public function init($action = false, $data = [], $initShortcodes = false, $initPageFilters = false) {
@@ -135,7 +141,8 @@ class Pages {
     }
 
     $subscriber = Subscriber::where('email', $email)->findOne();
-    return ($subscriber && $this->linkTokens->verifyToken($subscriber, $token)) ? $subscriber : null;
+    $subscriberEntity = $subscriber ? $this->subscribersRepository->findOneById($subscriber->id) : null;
+    return ($subscriber && $subscriberEntity && $this->linkTokens->verifyToken($subscriberEntity, $token)) ? $subscriber : null;
   }
 
   public function confirm() {

tests/integration/Newsletter/ViewInBrowser/ViewInBrowserControllerTest.php

@@ -66,16 +66,14 @@ class ViewInBrowserControllerTest extends \MailPoetTest {
     $sendingTask->setSubscribers([$subscriber->getId()]);
     $sendingTask->updateProcessedSubscribers([$subscriber->getId()]);
     $this->sendingTask = $sendingTask->save();
-    $linkTokens = new LinkTokens;
 
     // build browser preview data
-    $subscriberModel = Subscriber::findOne($subscriber->getId());
     $this->browserPreviewData = [
       'queue_id' => $sendingTask->queue()->id,
       'subscriber_id' => $subscriber->getId(),
       'newsletter_id' => $newsletter->id,
       'newsletter_hash' => $newsletter->hash,
-      'subscriber_token' => $linkTokens->getToken($subscriberModel),
+      'subscriber_token' => $this->linkTokens->getToken($subscriber),
       'preview' => false,
     ];
   }

tests/integration/Router/Endpoints/TrackTest.php

@@ -12,7 +12,6 @@ use MailPoet\Entities\SubscriberEntity;
 use MailPoet\Models\Newsletter;
 use MailPoet\Models\NewsletterLink;
 use MailPoet\Models\SendingQueue;
-use MailPoet\Models\Subscriber;
 use MailPoet\Newsletter\Sending\SendingQueuesRepository;
 use MailPoet\Router\Endpoints\Track;
 use MailPoet\Subscribers\LinkTokens;
@@ -26,6 +25,9 @@ class TrackTest extends \MailPoetTest {
   public $subscriber;
   public $newsletter;
 
+  /** @var LinkTokens */
+  private $linkTokens;
+
   public function _before() {
     parent::_before();
     $this->cleanup();
@@ -60,21 +62,20 @@ class TrackTest extends \MailPoetTest {
     $scheduledTaskSubscriber = new ScheduledTaskSubscriberEntity($task, $subscriber, 1);
     $this->entityManager->persist($scheduledTaskSubscriber);
     $this->entityManager->flush();
-    $subscriberModel = Subscriber::findOne($subscriber->getId());
+    $this->linkTokens = $this->diContainer->get(LinkTokens::class);
-    $linkTokens = new LinkTokens;
     // build track data
     $this->trackData = [
       'queue_id' => $queue->getId(),
       'subscriber_id' => $subscriber->getId(),
       'newsletter_id' => $newsletter->getId(),
-      'subscriber_token' => $linkTokens->getToken($subscriberModel),
+      'subscriber_token' => $this->linkTokens->getToken($subscriber),
       'link_hash' => $link->getHash(),
       'preview' => false,
     ];
     $queue = SendingQueue::findOne($queue->getId());
     assert($queue instanceof SendingQueue);
     $queue = SendingTask::createFromQueue($queue);
-    $queue->updateProcessedSubscribers([$subscriberModel->id]);
+    $queue->updateProcessedSubscribers([$subscriber->getId()]);
     // instantiate class
     $this->track = $this->diContainer->get(Track::class);
   }
@@ -106,8 +107,8 @@ class TrackTest extends \MailPoetTest {
     $data->subscriber->setEmail('random@email.com');
     $this->entityManager->flush();
     $track = Stub::make(Track::class, [
-      'linkTokens' => new LinkTokens,
       'sendingQueuesRepository' => $this->diContainer->get(SendingQueuesRepository::class),
+      'linkTokens' => $this->linkTokens,
       'terminate' => function($code) {
         expect($code)->equals(403);
       },

tests/integration/Subscription/ManageTest.php

@@ -13,6 +13,7 @@ use MailPoet\Statistics\Track\Unsubscribes;
 use MailPoet\Subscribers\LinkTokens;
 use MailPoet\Subscribers\NewSubscriberNotificationMailer;
 use MailPoet\Subscribers\SubscriberSegmentRepository;
+use MailPoet\Subscribers\SubscribersRepository;
 use MailPoet\Subscription\Manage;
 use MailPoet\Util\Url as UrlHelper;
 use MailPoetVendor\Idiorm\ORM;
@@ -62,6 +63,7 @@ class ManageTest extends \MailPoetTest {
       $this->settings,
       $this->diContainer->get(NewSubscriberNotificationMailer::class),
       $this->diContainer->get(WelcomeScheduler::class),
+      $this->diContainer->get(SubscribersRepository::class),
       $this->diContainer->get(SubscriberSegmentRepository::class)
     );
     $_POST['action'] = 'mailpoet_subscription_update';

tests/integration/Subscription/PagesTest.php

@@ -3,9 +3,9 @@
 namespace MailPoet\Test\Subscription;
 
 use Codeception\Stub;
-use Codeception\Util\Fixtures;
 use MailPoet\Config\Renderer;
 use MailPoet\DI\ContainerWrapper;
+use MailPoet\Entities\SubscriberEntity;
 use MailPoet\Form\AssetsController;
 use MailPoet\Models\Newsletter;
 use MailPoet\Models\NewsletterOption;
@@ -21,6 +21,7 @@ use MailPoet\Settings\SettingsController;
 use MailPoet\Statistics\Track\Unsubscribes;
 use MailPoet\Subscribers\LinkTokens;
 use MailPoet\Subscribers\NewSubscriberNotificationMailer;
+use MailPoet\Subscribers\SubscribersRepository;
 use MailPoet\Subscription\CaptchaRenderer;
 use MailPoet\Subscription\ManageSubscriptionFormRenderer;
 use MailPoet\Subscription\Pages;
@@ -32,18 +33,24 @@ use MailPoetVendor\Idiorm\ORM;
 class PagesTest extends \MailPoetTest {
   private $testData = [];
 
-  /** @var Subscriber */
+  /** @var SubscriberEntity */
   private $subscriber;
 
+  /** @var SubscribersRepository */
+  private $subscribersRepository;
+
   public function _before() {
     parent::_before();
-    $this->subscriber = Subscriber::create();
+    $this->subscribersRepository = $this->diContainer->get(SubscribersRepository::class);
-    $this->subscriber->hydrate(Fixtures::get('subscriber_template'));
+    $this->subscriber = new SubscriberEntity();
-    $this->subscriber->status = Subscriber::STATUS_UNCONFIRMED;
+    $this->subscriber->setFirstName('John');
-    $this->subscriber->save();
+    $this->subscriber->setLastName('John');
-    $linkTokens = new LinkTokens;
+    $this->subscriber->setEmail('john.doe@example.com');
-    expect($this->subscriber->getErrors())->false();
+    $this->subscriber->setStatus(Subscriber::STATUS_UNCONFIRMED);
-    $this->testData['email'] = $this->subscriber->email;
+    $this->subscribersRepository->persist($this->subscriber);
+    $this->subscribersRepository->flush();
+    $linkTokens = $this->diContainer->get(LinkTokens::class);
+    $this->testData['email'] = $this->subscriber->getEmail();
     $this->testData['token'] = $linkTokens->getToken($this->subscriber);
   }
 
@@ -52,7 +59,7 @@ class PagesTest extends \MailPoetTest {
     $pages = $this->getPages($newSubscriberNotificationSender);
     $subscription = $pages->init($action = false, $this->testData, false, false);
     $subscription->confirm();
-    $confirmedSubscriber = Subscriber::findOne($this->subscriber->id);
+    $confirmedSubscriber = Subscriber::findOne($this->subscriber->getId());
     expect($confirmedSubscriber->status)->equals(Subscriber::STATUS_SUBSCRIBED);
     expect($confirmedSubscriber->lastSubscribedAt)->greaterOrEquals(Carbon::createFromTimestamp((int)current_time('timestamp'))->subSecond());
     expect($confirmedSubscriber->lastSubscribedAt)->lessOrEquals(Carbon::createFromTimestamp((int)current_time('timestamp'))->addSecond());
@@ -62,21 +69,23 @@ class PagesTest extends \MailPoetTest {
     $newSubscriberNotificationSender = $this->makeEmpty(NewSubscriberNotificationMailer::class, ['send' => Stub\Expected::never()]);
     $pages = $this->getPages($newSubscriberNotificationSender);
     $subscriber = $this->subscriber;
-    $subscriber->status = Subscriber::STATUS_SUBSCRIBED;
+    $subscriber->setStatus(Subscriber::STATUS_SUBSCRIBED);
-    $subscriber->firstName = 'First name';
+    $subscriber->setFirstName('First name');
-    $subscriber->unconfirmedData = '{"first_name" : "Updated first name", "email" : "' . $this->subscriber->email . '"}';
+    $subscriber->setUnconfirmedData('{"first_name" : "Updated first name", "email" : "' . $this->subscriber->getEmail() . '"}');
-    $subscriber->lastSubscribedAt = Carbon::now()->subDays(10);
+    $subscriber->setLastSubscribedAt(Carbon::now()->subDays(10));
-    $subscriber->confirmedAt = Carbon::now()->subDays(10);
+    $subscriber->setConfirmedIp(Carbon::now()->subDays(10));
-    $subscriber->save();
+    $this->entityManager->flush();
     $subscription = $pages->init($action = false, $this->testData, false, false);
     $subscription->confirm();
-    $confirmedSubscriber = Subscriber::findOne($this->subscriber->id);
+    $this->entityManager->clear();
-    expect($confirmedSubscriber->status)->equals(Subscriber::STATUS_SUBSCRIBED);
+    $confirmedSubscriber = $this->subscribersRepository->findOneById($subscriber->getId());
-    expect($confirmedSubscriber->confirmedAt)->greaterOrEquals(Carbon::createFromTimestamp((int)current_time('timestamp'))->subSecond());
+    assert($confirmedSubscriber instanceof SubscriberEntity);
-    expect($confirmedSubscriber->confirmedAt)->lessOrEquals(Carbon::createFromTimestamp((int)current_time('timestamp'))->addSecond());
+    expect($confirmedSubscriber->getStatus())->equals(Subscriber::STATUS_SUBSCRIBED);
-    expect($confirmedSubscriber->lastSubscribedAt)->greaterOrEquals(Carbon::createFromTimestamp((int)current_time('timestamp'))->subSecond());
+    expect($confirmedSubscriber->getConfirmedAt())->greaterOrEquals(Carbon::createFromTimestamp((int)current_time('timestamp'))->subSecond());
-    expect($confirmedSubscriber->lastSubscribedAt)->lessOrEquals(Carbon::createFromTimestamp((int)current_time('timestamp'))->addSecond());
+    expect($confirmedSubscriber->getConfirmedAt())->lessOrEquals(Carbon::createFromTimestamp((int)current_time('timestamp'))->addSecond());
-    expect($confirmedSubscriber->firstName)->equals('Updated first name');
+    expect($confirmedSubscriber->getLastSubscribedAt())->greaterOrEquals(Carbon::createFromTimestamp((int)current_time('timestamp'))->subSecond());
+    expect($confirmedSubscriber->getLastSubscribedAt())->lessOrEquals(Carbon::createFromTimestamp((int)current_time('timestamp'))->addSecond());
+    expect($confirmedSubscriber->getFirstName())->equals('Updated first name');
   }
 
   public function testItSendsWelcomeNotificationUponConfirmingSubscription() {
@@ -92,7 +101,7 @@ class PagesTest extends \MailPoetTest {
     $subscriberSegment = SubscriberSegment::create();
     $subscriberSegment->hydrate(
       [
-        'subscriber_id' => $this->subscriber->id,
+        'subscriber_id' => $this->subscriber->getId(),
         'segment_id' => $segment->id,
       ]
     );
@@ -143,7 +152,7 @@ class PagesTest extends \MailPoetTest {
   public function testItUnsubscribes() {
     $pages = $this->getPages()->init($action = 'unsubscribe', $this->testData);
     $pages->unsubscribe();
-    $updatedSubscriber = Subscriber::findOne($this->subscriber->id);
+    $updatedSubscriber = Subscriber::findOne($this->subscriber->getId());
     expect($updatedSubscriber->status)->equals(Subscriber::STATUS_UNSUBSCRIBED);
   }
 
@@ -167,7 +176,7 @@ class PagesTest extends \MailPoetTest {
     $this->testData['preview'] = 1;
     $pages = $this->getPages()->init($action = 'unsubscribe', $this->testData);
     $pages->unsubscribe();
-    $updatedSubscriber = Subscriber::findOne($this->subscriber->id);
+    $updatedSubscriber = Subscriber::findOne($this->subscriber->getId());
     expect($updatedSubscriber->status)->notEquals(Subscriber::STATUS_UNSUBSCRIBED);
   }
 
@@ -199,7 +208,8 @@ class PagesTest extends \MailPoetTest {
       $container->get(AssetsController::class),
       $container->get(Renderer::class),
       $unsubscribesMock ?? $container->get(Unsubscribes::class),
-      $container->get(ManageSubscriptionFormRenderer::class)
+      $container->get(ManageSubscriptionFormRenderer::class),
+      $this->subscribersRepository
     );
   }
 }