Refactor MailPoet\API to use new settings

[MAILPOET-1757]

lib/API/JSON/API.php

@@ -2,8 +2,8 @@
 namespace MailPoet\API\JSON;
 
 use MailPoet\Config\AccessControl;
+use MailPoet\Settings\SettingsController;
 use MailPoetVendor\Psr\Container\ContainerInterface;
-use MailPoet\Models\Setting;
 use MailPoet\Util\Helpers;
 use MailPoet\Util\Security;
 use MailPoet\WP\Functions as WPFunctions;
@@ -30,12 +30,22 @@ class API {
   /** @var WPFunctions */
   private $wp;
 
+  /** @var SettingsController */
+  private $settings;
+
   const CURRENT_VERSION = 'v1';
 
-  function __construct(ContainerInterface $container, AccessControl $access_control, WPFunctions $wp) {
+
-    $this->wp = $wp;
+  function __construct(
+    ContainerInterface $container,
+    AccessControl $access_control,
+    SettingsController $settings,
+    WPFunctions $wp
+  ) {
     $this->container = $container;
     $this->access_control = $access_control;
+    $this->settings = $settings;
+    $this->wp = $wp;
     foreach($this->_available_api_versions as $available_api_version) {
       $this->addEndpointNamespace(
         sprintf('%s\%s', __NAMESPACE__, $available_api_version),
@@ -69,7 +79,7 @@ class API {
     $this->setRequestData($_POST);
 
     $ignoreToken = (
-      Setting::getValue('re_captcha.enabled') &&
+      $this->settings->get('re_captcha.enabled') &&
       $this->_request_endpoint === 'subscribers' &&
       $this->_request_method === 'subscribe'
     );

lib/API/JSON/v1/Newsletters.php

@@ -20,6 +20,7 @@ use MailPoet\Models\Subscriber;
 use MailPoet\Newsletter\Renderer\Renderer;
 use MailPoet\Newsletter\Scheduler\Scheduler;
 use MailPoet\Newsletter\Url as NewsletterUrl;
+use MailPoet\Settings\SettingsController;
 use MailPoet\WP\Functions as WPFunctions;
 
 if(!defined('ABSPATH')) exit;
@@ -35,6 +36,9 @@ class Newsletters extends APIEndpoint {
   /** @var WPFunctions */
   private $wp;
 
+  /** @var SettingsController */
+  private $settings;
+
   public $permissions = array(
     'global' => AccessControl::PERMISSION_MANAGE_EMAILS
   );
@@ -42,11 +46,13 @@ class Newsletters extends APIEndpoint {
   function __construct(
     Listing\BulkActionController $bulk_action,
     Listing\Handler $listing_handler,
-    WPFunctions $wp
+    WPFunctions $wp,
+    SettingsController $settings
   ) {
     $this->bulk_action = $bulk_action;
     $this->listing_handler = $listing_handler;
     $this->wp = $wp;
+    $this->settings = $settings;
   }
 
   function get($data = array()) {
@@ -444,8 +450,8 @@ class Newsletters extends APIEndpoint {
       'count' => $listing_data['count'],
       'filters' => $listing_data['filters'],
       'groups' => $listing_data['groups'],
-      'mta_log' => Setting::getValue('mta_log'),
+      'mta_log' => $this->settings->get('mta_log'),
-      'mta_method' => Setting::getValue('mta.method'),
+      'mta_method' => $this->settings->get('mta.method'),
       'cron_accessible' => CronHelper::isDaemonAccessible(),
       'current_time' => $this->wp->currentTime('mysql')
     ));

lib/API/JSON/v1/Settings.php

@@ -7,16 +7,25 @@ use MailPoet\API\JSON\Error as APIError;
 use MailPoet\Config\AccessControl;
 use MailPoet\Models\Setting;
 use MailPoet\Services\Bridge;
+use MailPoet\Settings\SettingsController;
 
 if(!defined('ABSPATH')) exit;
 
 class Settings extends APIEndpoint {
+
+  /** @var SettingsController */
+  private $settings;
+
   public $permissions = array(
     'global' => AccessControl::PERMISSION_MANAGE_SETTINGS
   );
 
+  function __construct(SettingsController $settings) {
+    $this->settings = $settings;
+  }
+
   function get() {
-    return $this->successResponse(Setting::getAll());
+    return $this->successResponse($this->settings->getAll());
   }
 
   function set($settings = array()) {
@@ -28,11 +37,11 @@ class Settings extends APIEndpoint {
         ));
     } else {
       foreach($settings as $name => $value) {
-        Setting::setValue($name, $value);
+        $this->settings->set($name, $value);
       }
       $bridge = new Bridge();
       $bridge->onSettingsSave($settings);
-      return $this->successResponse(Setting::getAll());
+      return $this->successResponse($this->settings->getAll());
     }
   }
 }

lib/API/JSON/v1/Setup.php

@@ -6,6 +6,7 @@ use MailPoet\API\JSON\Endpoint as APIEndpoint;
 use MailPoet\WP\Functions as WPFunctions;
 use MailPoet\Config\AccessControl;
 use MailPoet\Config\Activator;
+use MailPoet\Settings\SettingsController;
 
 if(!defined('ABSPATH')) exit;
 
@@ -21,7 +22,7 @@ class Setup extends APIEndpoint {
 
   function reset() {
     try {
-      $activator = new Activator();
+      $activator = new Activator(new SettingsController());
       $activator->deactivate();
       $activator->activate();
       $this->wp->doAction('mailpoet_setup_reset');

lib/API/JSON/v1/Subscribers.php

@@ -8,12 +8,12 @@ use MailPoet\Config\AccessControl;
 use MailPoet\Form\Util\FieldNameObfuscator;
 use MailPoet\Listing;
 use MailPoet\Models\Form;
-use MailPoet\Models\Setting;
 use MailPoet\Models\StatisticsForms;
 use MailPoet\Models\Subscriber;
 use MailPoet\Newsletter\Scheduler\Scheduler;
 use MailPoet\Segments\BulkAction;
 use MailPoet\Segments\SubscribersListings;
+use MailPoet\Settings\SettingsController;
 use MailPoet\Subscribers\RequiredCustomFieldValidator;
 use MailPoet\Subscribers\Source;
 use MailPoet\Subscription\Throttling as SubscriptionThrottling;
@@ -41,20 +41,26 @@ class Subscribers extends APIEndpoint {
   /** @var Listing\Handler */
   private $listing_handler;
 
+  /** @var WPFunctions */
   private $wp;
 
+  /** @var SettingsController */
+  private $settings;
+
   public function __construct(
     Listing\BulkActionController $bulk_action_controller,
     SubscribersListings $subscribers_listings,
     RequiredCustomFieldValidator $required_custom_field_validator,
     Listing\Handler $listing_handler,
-    WPFunctions $wp
+    WPFunctions $wp,
+    SettingsController $settings
   ) {
     $this->bulk_action_controller = $bulk_action_controller;
     $this->subscribers_listings = $subscribers_listings;
     $this->required_custom_field_validator = $required_custom_field_validator;
     $this->listing_handler = $listing_handler;
     $this->wp = $wp;
+    $this->settings = $settings;
   }
 
   function get($data = array()) {
@@ -105,7 +111,7 @@ class Subscribers extends APIEndpoint {
     $form = Form::findOne($form_id);
     unset($data['form_id']);
 
-    $recaptcha = Setting::getValue('re_captcha');
+    $recaptcha = $this->settings->get('re_captcha');
 
     if(!$form) {
       return $this->badRequest(array(

tests/integration/API/JSON/APITest.php

@@ -13,6 +13,7 @@ use MailPoet\API\JSON\v1\APITestNamespacedEndpointStubV1;
 use MailPoet\API\JSON\v2\APITestNamespacedEndpointStubV2;
 use MailPoet\Config\AccessControl;
 use MailPoet\DI\ContainerConfigurator;
+use MailPoet\Settings\SettingsController;
 use MailPoetVendor\Symfony\Component\DependencyInjection\Container;
 use MailPoet\DI\ContainerFactory;
 use MailPoet\WP\Functions as WPFunctions;
@@ -26,6 +27,9 @@ class APITest extends \MailPoetTest {
   /** @var Container */
   private $container;
 
+  /** @var SettingsController */
+  private $settings;
+
   function _before() {
     parent::_before();
     // create WP user
@@ -42,7 +46,13 @@ class APITest extends \MailPoetTest {
     $this->container->autowire(APITestNamespacedEndpointStubV1::class)->setPublic(true);
     $this->container->autowire(APITestNamespacedEndpointStubV2::class)->setPublic(true);
     $this->container->compile();
-    $this->api = new \MailPoet\API\JSON\API($this->container, $this->container->get(AccessControl::class), new WPFunctions);
+    $this->settings = $this->container->get(SettingsController::class);
+    $this->api = new \MailPoet\API\JSON\API(
+      $this->container,
+      $this->container->get(AccessControl::class),
+      $this->settings,
+      new WPFunctions
+    );
   }
 
   function testItCallsAPISetupAction() {
@@ -59,7 +69,8 @@ class APITest extends \MailPoetTest {
       'setupAjax',
       array(
         'wp' => new WPFunctions,
-        'processRoute' => Stub::makeEmpty(new SuccessResponse)
+        'processRoute' => Stub::makeEmpty(new SuccessResponse),
+        'settings' => $this->container->get(SettingsController::class)
       )
     );
     $api->setupAjax();
@@ -197,7 +208,8 @@ class APITest extends \MailPoetTest {
       new AccessControl(),
       array('validatePermission' => false)
     );
-    $api = new JSONAPI($this->container, $access_control, new WPFunctions);
+
+    $api = new JSONAPI($this->container, $access_control, $this->settings, new WPFunctions);
     $api->addEndpointNamespace($namespace['name'], $namespace['version']);
     $api->setRequestData($data);
     $response = $api->processRoute();
@@ -218,7 +230,8 @@ class APITest extends \MailPoetTest {
         })
       )
     );
-    $api = new JSONAPI($this->container, $access_control, new WPFunctions);
+
+    $api = new JSONAPI($this->container, $access_control, $this->settings, new WPFunctions);
     expect($api->validatePermissions(null, $permissions))->false();
 
     $access_control = Stub::make(
@@ -230,7 +243,7 @@ class APITest extends \MailPoetTest {
         })
       )
     );
-    $api = new JSONAPI($this->container, $access_control, new WPFunctions);
+    $api = new JSONAPI($this->container, $access_control, $this->settings, new WPFunctions);
     expect($api->validatePermissions(null, $permissions))->true();
   }
 
@@ -251,7 +264,8 @@ class APITest extends \MailPoetTest {
         })
       )
     );
-    $api = new JSONAPI($this->container, $access_control, new WPFunctions);
+
+    $api = new JSONAPI($this->container, $access_control, $this->settings, new WPFunctions);
     expect($api->validatePermissions('test', $permissions))->false();
 
     $access_control = Stub::make(
@@ -263,7 +277,8 @@ class APITest extends \MailPoetTest {
         })
       )
     );
-    $api = new JSONAPI($this->container, $access_control, new WPFunctions);
+
+    $api = new JSONAPI($this->container, $access_control, $this->settings, new WPFunctions);
     expect($api->validatePermissions('test', $permissions))->true();
   }
 

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

@@ -22,6 +22,7 @@ use MailPoet\Models\SendingQueue;
 use MailPoet\Newsletter\Scheduler\Scheduler;
 use MailPoet\Newsletter\Url;
 use MailPoet\Router\Router;
+use MailPoet\Settings\SettingsController;
 use MailPoet\Subscription\Url as SubscriptionUrl;
 use MailPoet\Tasks\Sending as SendingTask;
 use MailPoet\WP\Functions as WPFunctions;
@@ -108,7 +109,8 @@ class NewslettersTest extends \MailPoetTest {
     $this->endpoint = new Newsletters(
       ContainerWrapper::getInstance()->get(BulkActionController::class),
       ContainerWrapper::getInstance()->get(Handler::class),
-      $wp
+      $wp,
+      new SettingsController()
     );
     $response = $this->endpoint->get(array('id' => $this->newsletter->id));
     expect($response->status)->equals(APIResponse::STATUS_OK);
@@ -145,7 +147,8 @@ class NewslettersTest extends \MailPoetTest {
     $this->endpoint = new Newsletters(
       ContainerWrapper::getInstance()->get(BulkActionController::class),
       ContainerWrapper::getInstance()->get(Handler::class),
-      $wp
+      $wp,
+      new SettingsController()
     );
 
     $response = $this->endpoint->save($valid_data);
@@ -509,7 +512,8 @@ class NewslettersTest extends \MailPoetTest {
     $this->endpoint = new Newsletters(
       ContainerWrapper::getInstance()->get(BulkActionController::class),
       ContainerWrapper::getInstance()->get(Handler::class),
-      $wp
+      $wp,
+      new SettingsController()
     );
 
     $response = $this->endpoint->duplicate(array('id' => $this->newsletter->id));

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

@@ -10,6 +10,7 @@ use MailPoet\Models\NewsletterOptionField;
 use MailPoet\Models\ScheduledTask;
 use MailPoet\Models\SendingQueue as SendingQueueModel;
 use MailPoet\Models\Setting;
+use MailPoet\Settings\SettingsController;
 use MailPoet\Tasks\Sending;
 
 class SendingQueueTest extends \MailPoetTest {
@@ -22,7 +23,8 @@ class SendingQueueTest extends \MailPoetTest {
         'type' => Newsletter::TYPE_STANDARD
       )
     );
-    Setting::setValue('sender', array(
+    $settings = new SettingsController();
+    $settings->set('sender', array(
       'name' => 'John Doe',
       'address' => 'john.doe@example.com'
     ));

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

@@ -6,14 +6,18 @@ use Codeception\Stub\Expected;
 use MailPoet\API\JSON\v1\Services;
 use MailPoet\API\JSON\Response as APIResponse;
 use MailPoet\Config\Installer;
-use MailPoet\Models\Setting;
 use MailPoet\Services\Bridge;
+use MailPoet\Settings\SettingsController;
 
 class ServicesTest extends \MailPoetTest {
+  /** @var SettingsController */
+  private $settings;
+
   function _before() {
     parent::_before();
     $this->services_endpoint = new Services();
     $this->data = array('key' => '1234567890abcdef');
+    $this->settings = new SettingsController();
   }
 
   function testItRespondsWithErrorIfNoMSSKeyIsGiven() {
@@ -268,8 +272,8 @@ class ServicesTest extends \MailPoetTest {
 
   function testItRespondsWithPublicIdForMSS() {
     $fake_public_id = 'a-fake-public_id';
-    Setting::deleteValue('public_id');
+    $this->settings->delete('public_id');
-    Setting::deleteValue('new_public_id');
+    $this->settings->delete('new_public_id');
 
     $this->services_endpoint->bridge = Stub::make(
       new Bridge(),
@@ -284,13 +288,13 @@ class ServicesTest extends \MailPoetTest {
     );
     $response = $this->services_endpoint->checkMSSKey($this->data);
 
-    expect(Setting::getValue('public_id'))->equals($fake_public_id);
+    expect($this->settings->get('public_id'))->equals($fake_public_id);
-    expect(Setting::getValue('new_public_id'))->equals('true');
+    expect($this->settings->get('new_public_id'))->equals('true');
   }
 
   function testItRespondsWithoutPublicIdForMSS() {
-    Setting::deleteValue('public_id');
+    $this->settings->delete('public_id');
-    Setting::deleteValue('new_public_id');
+    $this->settings->delete('new_public_id');
 
     $this->services_endpoint->bridge = Stub::make(
       new Bridge(),
@@ -302,14 +306,14 @@ class ServicesTest extends \MailPoetTest {
     );
     $response = $this->services_endpoint->checkMSSKey($this->data);
 
-    expect(Setting::getValue('public_id', null))->null();
+    expect($this->settings->get('public_id', null))->null();
-    expect(Setting::getValue('new_public_id', null))->null();
+    expect($this->settings->get('new_public_id', null))->null();
   }
 
   function testItRespondsWithPublicIdForPremium() {
     $fake_public_id = 'another-fake-public_id';
-    Setting::deleteValue('public_id');
+    $this->settings->delete('public_id');
-    Setting::deleteValue('new_public_id');
+    $this->settings->delete('new_public_id');
 
     $this->services_endpoint->bridge = Stub::make(
       new Bridge(),
@@ -324,13 +328,13 @@ class ServicesTest extends \MailPoetTest {
     );
     $response = $this->services_endpoint->checkPremiumKey($this->data);
 
-    expect(Setting::getValue('public_id'))->equals($fake_public_id);
+    expect($this->settings->get('public_id'))->equals($fake_public_id);
-    expect(Setting::getValue('new_public_id'))->equals('true');
+    expect($this->settings->get('new_public_id'))->equals('true');
   }
 
   function testItRespondsWithoutPublicIdForPremium() {
-    Setting::deleteValue('public_id');
+    $this->settings->delete('public_id');
-    Setting::deleteValue('new_public_id');
+    $this->settings->delete('new_public_id');
 
     $this->services_endpoint->bridge = Stub::make(
       new Bridge(),
@@ -342,7 +346,7 @@ class ServicesTest extends \MailPoetTest {
     );
     $response = $this->services_endpoint->checkPremiumKey($this->data);
 
-    expect(Setting::getValue('public_id', null))->null();
+    expect($this->settings->get('public_id', null))->null();
-    expect(Setting::getValue('new_public_id', null))->null();
+    expect($this->settings->get('new_public_id', null))->null();
   }
 }

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

@@ -5,24 +5,30 @@ use MailPoet\API\JSON\Response as APIResponse;
 use MailPoet\API\JSON\Error as APIError;
 use MailPoet\API\JSON\v1\Settings;
 use MailPoet\Models\Setting;
+use MailPoet\Settings\SettingsController;
 
 class SettingsTest extends \MailPoetTest {
+
+  /** @var Settings */
+  private $endpoint;
+
   function _before() {
     parent::_before();
-    Setting::setValue('some.setting.key', true);
+    $settings = new SettingsController();
+    $settings->set('some.setting.key', true);
+    $this->endpoint = new Settings($settings);
   }
 
   function testItCanGetSettings() {
-    $router = new Settings();
+    $response = $this->endpoint->get();
-
-    $response = $router->get();
     expect($response->status)->equals(APIResponse::STATUS_OK);
 
     expect($response->data)->notEmpty();
     expect($response->data['some']['setting']['key'])->true();
 
     Setting::deleteMany();
-    $response = $router->get();
+    SettingsController::resetCache();
+    $response = $this->endpoint->get();
     expect($response->status)->equals(APIResponse::STATUS_OK);
     expect($response->data)->equals(Setting::getDefaults());
   }
@@ -37,16 +43,14 @@ class SettingsTest extends \MailPoetTest {
       )
     );
 
-    $router = new Settings();
+    $response = $this->endpoint->set(/* missing data */);
-
-    $response = $router->set(/* missing data */);
     expect($response->errors[0]['error'])->equals(APIError::BAD_REQUEST);
     expect($response->status)->equals(APIResponse::STATUS_BAD_REQUEST);
 
-    $response = $router->set($new_settings);
+    $response = $this->endpoint->set($new_settings);
     expect($response->status)->equals(APIResponse::STATUS_OK);
 
-    $response = $router->get();
+    $response = $this->endpoint->get();
     expect($response->status)->equals(APIResponse::STATUS_OK);
     expect($response->data['some']['setting'])->hasntKey('key');
     expect($response->data['some']['setting']['new_key'])->true();

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

@@ -7,11 +7,13 @@ use MailPoet\API\JSON\v1\Setup;
 use MailPoet\WP\Functions as WPFunctions;
 use Helper\WordPressHooks as WPHooksHelper;
 use MailPoet\API\JSON\Response as APIResponse;
+use MailPoet\Settings\SettingsController;
 
 class SetupTest extends \MailPoetTest {
   function _before() {
     parent::_before();
-    Setting::setValue('signup_confirmation.enabled', false);
+    $settings = new SettingsController();
+    $settings->set('signup_confirmation.enabled', false);
   }
 
   function testItCanReinstall() {
@@ -23,7 +25,8 @@ class SetupTest extends \MailPoetTest {
     $response = $router->reset();
     expect($response->status)->equals(APIResponse::STATUS_OK);
 
-    $signup_confirmation = Setting::getValue('signup_confirmation.enabled');
+    $settings = new SettingsController();
+    $signup_confirmation = $settings->fetch('signup_confirmation.enabled');
     expect($signup_confirmation)->true();
 
     $hook_name = 'mailpoet_setup_reset';

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

@@ -18,6 +18,7 @@ use MailPoet\Models\SubscriberIP;
 use MailPoet\Models\Segment;
 use MailPoet\Models\Setting;
 use MailPoet\Models\SubscriberSegment;
+use MailPoet\Settings\SettingsController;
 use MailPoet\Subscribers\Source;
 
 class SubscribersTest extends \MailPoetTest {
@@ -25,6 +26,9 @@ class SubscribersTest extends \MailPoetTest {
   /** @var Subscribers */
   private $endpoint;
 
+  /** @var SettingsController */
+  private $settings;
+
   function _before() {
     parent::_before();
     $this->cleanup();
@@ -66,8 +70,9 @@ class SubscribersTest extends \MailPoetTest {
       )
     ));
 
+    $this->settings = new SettingsController();
     // setup mailer
-    Setting::setValue('sender', array(
+    $this->settings->set('sender', array(
       'address' => 'sender@mailpoet.com',
       'name' => 'Sender'
     ));
@@ -451,7 +456,7 @@ class SubscribersTest extends \MailPoetTest {
   }
 
   function testItCannotSubscribeWithoutCaptchaWhenEnabled() {
-    Setting::setValue('re_captcha', array('enabled' => true));
+    $this->settings->set('re_captcha', array('enabled' => true));
     $response = $this->endpoint->subscribe(array(
       $this->obfuscatedEmail => 'toto@mailpoet.com',
       'form_id' => $this->form->id,
@@ -459,7 +464,7 @@ class SubscribersTest extends \MailPoetTest {
     ));
     expect($response->status)->equals(APIResponse::STATUS_BAD_REQUEST);
     expect($response->errors[0]['message'])->equals('Please check the CAPTCHA.');
-    Setting::setValue('re_captcha', array());
+    $this->settings->set('re_captcha', array());
   }
 
   function testItCannotSubscribeWithoutMandatoryCustomField() {
@@ -639,5 +644,6 @@ class SubscribersTest extends \MailPoetTest {
     \ORM::raw_execute('TRUNCATE ' . SubscriberSegment::$_table);
     \ORM::raw_execute('TRUNCATE ' . SubscriberIP::$_table);
     \ORM::raw_execute('TRUNCATE ' . CustomField::$_table);
+    \ORM::raw_execute('TRUNCATE ' . Setting::$_table);
   }
 }