Move ModelValidator::validateNonRoleEmail() to a new Validator class

We want to remove/refactor the whole ModelValidator class as part of the
Doctrine refactor.

This commit moves the method ModelValidator::validateNonRoleEmail() to a
new Validator class as the method is not used by the validator system of
the Paris models. ModelValidator::validateEmail() was also moved as it
is called by ModelValidator::validateNonRoleEmail().

[MAILPOET-4343]

mailpoet/lib/API/JSON/v1/ImportExport.php

@@ -15,6 +15,7 @@ use MailPoet\Newsletter\Options\NewsletterOptionsRepository;
 use MailPoet\Segments\SegmentSaveController;
 use MailPoet\Segments\SegmentsRepository;
 use MailPoet\Segments\WP;
+use MailPoet\Services\Validator;
 use MailPoet\Subscribers\ImportExport\Export\Export;
 use MailPoet\Subscribers\ImportExport\Import\Import;
 use MailPoet\Subscribers\ImportExport\Import\MailChimp;
@@ -51,6 +52,9 @@ class ImportExport extends APIEndpoint {
   /** @var TagRepository */
   private $tagRepository;
 
+  /** @var Validator */
+  private $validator;
+
   /** @var CronWorkerScheduler */
   private $cronWorkerScheduler;
 
@@ -68,7 +72,8 @@ class ImportExport extends APIEndpoint {
     SegmentsResponseBuilder $segmentsResponseBuilder,
     CronWorkerScheduler $cronWorkerScheduler,
     SubscribersRepository $subscribersRepository,
-    TagRepository $tagRepository
+    TagRepository $tagRepository,
+    Validator $validator
   ) {
     $this->wpSegment = $wpSegment;
     $this->customFieldsRepository = $customFieldsRepository;
@@ -80,6 +85,7 @@ class ImportExport extends APIEndpoint {
     $this->cronWorkerScheduler = $cronWorkerScheduler;
     $this->segmentsResponseBuilder = $segmentsResponseBuilder;
     $this->tagRepository = $tagRepository;
+    $this->validator = $validator;
   }
 
   public function getMailChimpLists($data) {
@@ -133,6 +139,7 @@ class ImportExport extends APIEndpoint {
         $this->newsletterOptionsRepository,
         $this->subscriberRepository,
         $this->tagRepository,
+        $this->validator,
         json_decode($data, true)
       );
       $process = $import->process();

mailpoet/lib/AdminPages/Pages/SubscribersImport.php

@@ -4,7 +4,7 @@ namespace MailPoet\AdminPages\Pages;
 
 use MailPoet\AdminPages\PageRenderer;
 use MailPoet\Form\Block;
-use MailPoet\Models\ModelValidator;
+use MailPoet\Services\Validator;
 use MailPoet\Subscribers\ImportExport\ImportExportFactory;
 
 class SubscribersImport {
@@ -30,7 +30,7 @@ class SubscribersImport {
       'date_formats' => $this->dateBlock->getDateFormats(),
       'month_names' => $this->dateBlock->getMonthNames(),
       'sub_menu' => 'mailpoet-subscribers',
-      'role_based_emails' => json_encode(ModelValidator::ROLE_EMAILS),
+      'role_based_emails' => json_encode(Validator::ROLE_EMAILS),
     ]);
     $this->pageRenderer->displayPage('subscribers/importExport/import.html', $data);
   }

mailpoet/lib/DI/ContainerConfigurator.php

@@ -437,6 +437,7 @@ class ContainerConfigurator implements IContainerConfigurator {
     $container->autowire(\MailPoet\Services\CongratulatoryMssEmailController::class)->setPublic(true);
     $container->autowire(\MailPoet\Services\AuthorizedSenderDomainController::class)->setPublic(true);
     $container->autowire(\MailPoet\Services\SubscribersCountReporter::class)->setPublic(true);
+    $container->autowire(\MailPoet\Services\Validator::class)->setPublic(true);
     // Settings
     $container->autowire(\MailPoet\Settings\SettingsController::class)->setPublic(true);
     $container->autowire(\MailPoet\Settings\SettingsChangeHandler::class)->setPublic(true);

mailpoet/lib/Form/Block/BlockRendererHelper.php

@@ -3,7 +3,7 @@
 namespace MailPoet\Form\Block;
 
 use MailPoet\Form\Util\FieldNameObfuscator;
-use MailPoet\Models\ModelValidator;
+use MailPoet\Services\Validator;
 use MailPoet\WP\Functions as WPFunctions;
 
 /**
@@ -32,8 +32,8 @@ class BlockRendererHelper {
 
     if ($blockId === 'email') {
       $rules['required'] = true;
-      $rules['minlength'] = ModelValidator::EMAIL_MIN_LENGTH;
+      $rules['minlength'] = Validator::EMAIL_MIN_LENGTH;
-      $rules['maxlength'] = ModelValidator::EMAIL_MAX_LENGTH;
+      $rules['maxlength'] = Validator::EMAIL_MAX_LENGTH;
       $rules['type-message'] = __('This value should be a valid email.', 'mailpoet');
     }
 

mailpoet/lib/Models/ModelValidator.php

@@ -2,50 +2,13 @@
 
 namespace MailPoet\Models;
 
+use MailPoet\DI\ContainerWrapper;
+use MailPoet\Services\Validator;
 use MailPoet\Util\Helpers;
-use MailPoet\WP\Functions as WPFunctions;
 
 class ModelValidator extends \MailPoetVendor\Sudzy\Engine {
   public $validators;
 
-  const EMAIL_MIN_LENGTH = 6;
-  const EMAIL_MAX_LENGTH = 150;
-
-  const ROLE_EMAILS = [
-    'abuse',
-    'compliance',
-    'devnull',
-    'dns',
-    'ftp',
-    'hostmaster',
-    'inoc',
-    'ispfeedback',
-    'ispsupport',
-    'list-request',
-    'list',
-    'maildaemon',
-    'noc',
-    'no-reply',
-    'noreply',
-    'nospam',
-    'null',
-    'phish',
-    'phishing',
-    'postmaster',
-    'privacy',
-    'registrar',
-    'root',
-    'security',
-    'spam',
-    'sysadmin',
-    'undisclosed-recipients',
-    'unsubscribe',
-    'usenet',
-    'uucp',
-    'webmaster',
-    'www',
-  ];
-
   public function __construct() {
     parent::__construct();
     $this->validators = [
@@ -68,15 +31,8 @@ class ModelValidator extends \MailPoetVendor\Sudzy\Engine {
   }
 
   public function validateEmail($email) {
-    $permittedLength = (strlen($email) >= self::EMAIL_MIN_LENGTH && strlen($email) <= self::EMAIL_MAX_LENGTH);
+    $validator = ContainerWrapper::getInstance()->get(Validator::class);
-    $validEmail = WPFunctions::get()->isEmail($email) !== false && parent::_isEmail($email, null);
+    return $validator->validateEmail($email);
-    return ($permittedLength && $validEmail);
-  }
-
-  public function validateNonRoleEmail($email) {
-    if (!$this->validateEmail($email)) return false;
-    $firstPart = strtolower(substr($email, 0, (int)strpos($email, '@')));
-    return array_search($firstPart, self::ROLE_EMAILS) === false;
   }
 
   public function validateRenderedNewsletterBody($newsletterBody) {

mailpoet/lib/Services/Validator.php

@@ -0,0 +1,60 @@
+<?php declare(strict_types = 1);
+
+namespace MailPoet\Services;
+
+use MailPoet\WP\Functions as WPFunctions;
+
+/**
+ * This class contains validation methods that were extracted from the \MailPoet\Models\ModelValidator class.
+ * It is used only in a few places and there is a chance in the future we can remove it.
+ */
+class Validator {
+  const EMAIL_MIN_LENGTH = 6;
+  const EMAIL_MAX_LENGTH = 150;
+  const ROLE_EMAILS = [
+    'abuse',
+    'compliance',
+    'devnull',
+    'dns',
+    'ftp',
+    'hostmaster',
+    'inoc',
+    'ispfeedback',
+    'ispsupport',
+    'list-request',
+    'list',
+    'maildaemon',
+    'noc',
+    'no-reply',
+    'noreply',
+    'nospam',
+    'null',
+    'phish',
+    'phishing',
+    'postmaster',
+    'privacy',
+    'registrar',
+    'root',
+    'security',
+    'spam',
+    'sysadmin',
+    'undisclosed-recipients',
+    'unsubscribe',
+    'usenet',
+    'uucp',
+    'webmaster',
+    'www',
+  ];
+
+  public function validateEmail($email) {
+    $permittedLength = (strlen($email) >= self::EMAIL_MIN_LENGTH && strlen($email) <= self::EMAIL_MAX_LENGTH);
+    $validEmail = WPFunctions::get()->isEmail($email) !== false && filter_var($email, FILTER_VALIDATE_EMAIL) !== false;
+    return ($permittedLength && $validEmail);
+  }
+
+  public function validateNonRoleEmail($email) {
+    if (!$this->validateEmail($email)) return false;
+    $firstPart = strtolower(substr($email, 0, (int)strpos($email, '@')));
+    return array_search($firstPart, self::ROLE_EMAILS) === false;
+  }
+}

mailpoet/lib/Subscribers/ImportExport/Import/Import.php

@@ -8,9 +8,9 @@ use MailPoet\Entities\SubscriberCustomFieldEntity;
 use MailPoet\Entities\SubscriberEntity;
 use MailPoet\Entities\SubscriberSegmentEntity;
 use MailPoet\Entities\SubscriberTagEntity;
-use MailPoet\Models\ModelValidator;
 use MailPoet\Newsletter\Options\NewsletterOptionsRepository;
 use MailPoet\Segments\WP;
+use MailPoet\Services\Validator;
 use MailPoet\Subscribers\ImportExport\ImportExportFactory;
 use MailPoet\Subscribers\ImportExport\ImportExportRepository;
 use MailPoet\Subscribers\Source;
@@ -71,6 +71,9 @@ class Import {
   /** @var TagRepository */
   private $tagRepository;
 
+  /** @var Validator */
+  private $validator;
+
   public function __construct(
     WP $wpSegment,
     CustomFieldsRepository $customFieldsRepository,
@@ -78,6 +81,7 @@ class Import {
     NewsletterOptionsRepository $newsletterOptionsRepository,
     SubscribersRepository $subscriberRepository,
     TagRepository $tagRepository,
+    Validator $validator,
     array $data
   ) {
     $this->wpSegment = $wpSegment;
@@ -86,6 +90,7 @@ class Import {
     $this->newsletterOptionsRepository = $newsletterOptionsRepository;
     $this->subscriberRepository = $subscriberRepository;
     $this->tagRepository = $tagRepository;
+    $this->validator = $validator;
     $this->validateImportData($data);
     $this->subscribersData = $this->transformSubscribersData(
       $data['subscribers'],
@@ -219,12 +224,11 @@ class Import {
    */
   public function validateSubscribersData(array $subscribersData) {
     $invalidRecords = [];
-    $validator = new ModelValidator();
     foreach ($subscribersData as $column => &$data) {
       if ($column === 'email') {
         $data = array_map(
-          function($index, $email) use(&$invalidRecords, $validator) {
+          function($index, $email) use(&$invalidRecords) {
-            if (!$validator->validateNonRoleEmail($email)) {
+            if (!$this->validator->validateNonRoleEmail($email)) {
               $invalidRecords[] = $index;
             }
             return strtolower($email);

mailpoet/tasks/phpstan/phpstan-7-baseline.neon

@@ -56,7 +56,7 @@ parameters:
 			path: ../../lib/API/JSON/v1/ImportExport.php
 
 		-
-			message: "#^Parameter \\#7 \\$data of class MailPoet\\\\Subscribers\\\\ImportExport\\\\Import\\\\Import constructor expects array, mixed given\\.$#"
+			message: "#^Parameter \\#8 \\$data of class MailPoet\\\\Subscribers\\\\ImportExport\\\\Import\\\\Import constructor expects array, mixed given\\.$#"
 			count: 1
 			path: ../../lib/API/JSON/v1/ImportExport.php
 

mailpoet/tasks/phpstan/phpstan-8-baseline.neon

@@ -56,7 +56,7 @@ parameters:
 			path: ../../lib/API/JSON/v1/ImportExport.php
 
 		-
-			message: "#^Parameter \\#7 \\$data of class MailPoet\\\\Subscribers\\\\ImportExport\\\\Import\\\\Import constructor expects array, mixed given\\.$#"
+			message: "#^Parameter \\#8 \\$data of class MailPoet\\\\Subscribers\\\\ImportExport\\\\Import\\\\Import constructor expects array, mixed given\\.$#"
 			count: 1
 			path: ../../lib/API/JSON/v1/ImportExport.php
 

mailpoet/tasks/phpstan/phpstan-8.1-baseline.neon

@@ -55,7 +55,7 @@ parameters:
 			path: ../../lib/API/JSON/v1/ImportExport.php
 
 		-
-			message: "#^Parameter \\#7 \\$data of class MailPoet\\\\Subscribers\\\\ImportExport\\\\Import\\\\Import constructor expects array, mixed given\\.$#"
+			message: "#^Parameter \\#8 \\$data of class MailPoet\\\\Subscribers\\\\ImportExport\\\\Import\\\\Import constructor expects array, mixed given\\.$#"
 			count: 1
 			path: ../../lib/API/JSON/v1/ImportExport.php
 

mailpoet/tests/integration/Models/ModelValidatorTest.php

@@ -27,12 +27,6 @@ class ModelValidatorTest extends \MailPoetTest {
     expect($this->validator->validateEmail('a@b.c'))->false();
   }
 
-  public function testItValidatesNonRoleEmail() {
-    expect($this->validator->validateNonRoleEmail('test'))->false();
-    expect($this->validator->validateNonRoleEmail('webmaster@example.com'))->false();
-    expect($this->validator->validateNonRoleEmail('test@example.com'))->true();
-  }
-
   public function testItValidatesRenderedNewsletterBody() {
     expect($this->validator->validateRenderedNewsletterBody('test'))->false();
     expect($this->validator->validateRenderedNewsletterBody(serialize('test')))->false();

mailpoet/tests/integration/Services/ValidatorTest.php

@@ -0,0 +1,28 @@
+<?php declare(strict_types = 1);
+
+namespace MailPoet\Test\Services;
+
+use MailPoet\Services\Validator;
+
+class ValidatorTest extends \MailPoetTest {
+  public $validator;
+
+  public function _before() {
+    parent::_before();
+    $this->validator = $this->diContainer->get(Validator::class);
+  }
+
+  public function testItValidatesEmail() {
+    expect($this->validator->validateEmail('test'))->false();
+    expect($this->validator->validateEmail('tést@éxample.com'))->false();
+    expect($this->validator->validateEmail('test@example.com'))->true();
+    expect($this->validator->validateEmail('loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong_email@example.com'))->false();
+    expect($this->validator->validateEmail('a@b.c'))->false();
+  }
+
+  public function testItValidatesNonRoleEmail() {
+    expect($this->validator->validateNonRoleEmail('test'))->false();
+    expect($this->validator->validateNonRoleEmail('webmaster@example.com'))->false();
+    expect($this->validator->validateNonRoleEmail('test@example.com'))->true();
+  }
+}

mailpoet/tests/integration/Subscribers/ImportExport/Import/ImportTest.php

@@ -13,6 +13,7 @@ use MailPoet\Entities\TagEntity;
 use MailPoet\Newsletter\Options\NewsletterOptionsRepository;
 use MailPoet\Segments\SegmentsRepository;
 use MailPoet\Segments\WP;
+use MailPoet\Services\Validator;
 use MailPoet\Subscribers\ImportExport\ImportExportRepository;
 use MailPoet\Subscribers\SubscriberCustomFieldRepository;
 use MailPoet\Subscribers\SubscriberSegmentRepository;
@@ -71,6 +72,9 @@ class ImportTest extends \MailPoetTest {
   /** @var SubscriberTagRepository */
   private $subscribersTagRepository;
 
+  /** @var Validator */
+  private $validator;
+
   public function _before(): void {
     $this->wpSegment = $this->diContainer->get(WP::class);
     $this->customFieldsRepository = $this->diContainer->get(CustomFieldsRepository::class);
@@ -81,6 +85,7 @@ class ImportTest extends \MailPoetTest {
     $this->subscriberRepository = $this->diContainer->get(SubscribersRepository::class);
     $this->subscriberSegmentRepository = $this->diContainer->get(SubscriberSegmentRepository::class);
     $this->tagRepository = $this->diContainer->get(TagRepository::class);
+    $this->validator = $this->diContainer->get(Validator::class);
     $this->subscribersTagRepository = $this->diContainer->get(SubscriberTagRepository::class);
     $customField = $this->customFieldsRepository->createOrUpdate([
       'name' => 'country',
@@ -832,6 +837,7 @@ class ImportTest extends \MailPoetTest {
       $this->newsletterOptionsRepository,
       $this->subscriberRepository,
       $this->tagRepository,
+      $this->validator,
       $data
     );
   }