Move Setting getValue and setValue to controller as a private methods

[MAILPOET-1757]

lib/Models/Setting.php

@@ -1,9 +1,7 @@
 <?php
 namespace MailPoet\Models;
 
-use MailPoet\Cron\CronTrigger;
 use MailPoet\Settings\SettingsController;
-use MailPoet\Util\Helpers;
 
 if(!defined('ABSPATH')) exit;
 
@@ -23,31 +21,6 @@ class Setting extends Model {
     ));
   }
 
-  public static function getValue($key) {
-    $setting = Setting::where('name', $key)->findOne();
-    if($setting === false) {
-        return null;
-    }
-    if(is_serialized($setting->value)) {
-      return unserialize($setting->value);
-    } else {
-      return $setting->value;
-    }
-  }
-
-  public static function setValue($key, $value) {
-    $value = Helpers::recursiveTrim($value);
-    if(is_array($value)) {
-      $value = serialize($value);
-    }
-
-    $setting = Setting::createOrUpdate(array(
-      'name' => $key,
-      'value' => $value
-    ));
-    return ($setting->id() > 0 && $setting->getErrors() === false);
-  }
-
   public static function getAll() {
     $settingsCollection = self::findMany();
     $settings = array();

lib/Settings/SettingsController.php

@@ -3,6 +3,7 @@ namespace MailPoet\Settings;
 
 use MailPoet\Cron\CronTrigger;
 use MailPoet\Models\Setting;
+use MailPoet\Util\Helpers;
 
 class SettingsController {
 
@@ -78,7 +79,7 @@ class SettingsController {
   function fetch($key, $default = null) {
     $keys = explode('.', $key);
     $main_key = $keys[0];
-    self::$settings[$main_key] = Setting::getValue($main_key);
+    self::$settings[$main_key] = $this->fetchValue($main_key);
     return $this->get($key, $default);
   }
 
@@ -100,7 +101,7 @@ class SettingsController {
       }
     }
     $setting[$last_key] = $value;
-    Setting::setValue($main_key, self::$settings[$main_key]);
+    $this->saveValue($main_key, self::$settings[$main_key]);
   }
 
   function delete($key) {
@@ -128,6 +129,31 @@ class SettingsController {
     return $default;
   }
 
+  private function fetchValue($key) {
+    $setting = Setting::where('name', $key)->findOne();
+    if($setting === false) {
+      return null;
+    }
+    if(is_serialized($setting->value)) {
+      return unserialize($setting->value);
+    } else {
+      return $setting->value;
+    }
+  }
+
+  private function saveValue($key, $value) {
+    $value = Helpers::recursiveTrim($value);
+    if(is_array($value)) {
+      $value = serialize($value);
+    }
+
+    $setting = Setting::createOrUpdate([
+      'name' => $key,
+      'value' => $value,
+    ]);
+    return ($setting->id() > 0 && $setting->getErrors() === false);
+  }
+
   /**
    * Temporary function for tests use only.
    * It is needed until this is only instantiated in one place (DI Container)

tests/integration/Cron/CronHelperTest.php

@@ -23,10 +23,10 @@ class CronHelperTest extends \MailPoetTest {
     $this->settings->set('cron_trigger', array(
       'method' => 'none'
     ));
-    Setting::setValue('sender', array(
+    $this->settings->set('sender', [
       'name' => 'John Doe',
       'address' => 'john.doe@example.com'
-    ));
+    ]);
   }
 
   function testItDefinesConstants() {

tests/integration/Models/SettingTest.php

@@ -23,12 +23,12 @@ class SettingTest extends \MailPoetTest {
   }
 
   function testItCanGetAllSettings() {
-    Setting::setValue('key_1', 'value_1');
+    Setting::createOrUpdate(['name' => 'key_1', 'value' => 'value_1']);
-    Setting::setValue('key_2', 'value_2');
+    Setting::createOrUpdate(['name' => 'key_2', 'value' => 'value_2']);
-    Setting::setValue('key_3', array(
+    Setting::createOrUpdate(['name' => 'key_3', 'value' => serialize([
       'subkey_1' => 'subvalue_1',
       'subkey_2' => 'subvalue_2'
-    ));
+    ])]);
 
     $settings = Setting::getAll();
     expect($settings['key_1'])->equals('value_1');
@@ -39,20 +39,6 @@ class SettingTest extends \MailPoetTest {
     ));
   }
 
-  function testItCanSetAndGetValues() {
-    // try to get an "unknown" key
-    $setting = Setting::getValue('unknown_key');
-    expect($setting)->equals(null);
-
-    // setting a "known" key
-    $setting = Setting::setValue('known_key', '  actual_value  ');
-    expect($setting)->equals(true);
-
-    // try to get a "known" key
-    $setting = Setting::getValue('known_key', 'default_value');
-    expect($setting)->equals('actual_value');
-  }
-
   function testItCanCreateOrUpdate() {
     $data = array(
       'name'  => 'new',
@@ -75,19 +61,6 @@ class SettingTest extends \MailPoetTest {
     expect($setting->value)->equals('new data');
   }
 
-  function testItCanGetAndSetValue() {
-    expect(Setting::setValue('test', '  123  '))->true();
-    expect(Setting::getValue('test'))->equals('123');
-  }
-
-  function testItCanSetValueToNull() {
-    expect(Setting::setValue('test_key', true))->true();
-    expect(Setting::getValue('test_key'))->equals(true);
-
-    expect(Setting::setValue('test_key', null))->true();
-    expect(Setting::getValue('test_key'))->null();
-  }
-
   function testSaveDefaultSenderIfNeededNotSaveEmptyValue() {
     $settings_controller = new SettingsController();
     Setting::saveDefaultSenderIfNeeded('', null);

tests/integration/Settings/SettingsControllerTest.php

@@ -16,26 +16,26 @@ class SettingsControllerTest extends \MailPoetTest {
   }
 
   function testItReturnsStoredValue() {
-    Setting::setValue('test_key', 1);
+    Setting::createOrUpdate(['name' => 'test_key', 'value' => 1]);
     $this->assertEquals(1, $this->controller->get('test_key'));
   }
 
   function testItReturnsStoredNestedValue() {
-    Setting::setValue('test_key', ['sub_key' => 'value']);
+    Setting::createOrUpdate(['name' => 'test_key', 'value' => serialize(['sub_key' => 'value'])]);
     $this->assertEquals('value', $this->controller->get('test_key.sub_key'));
   }
 
   function testItReturnsNullForUnknownSetting() {
     $this->assertEquals(null, $this->controller->get('test_key'));
     $this->assertEquals(null, $this->controller->get('test_key.sub_key'));
-    Setting::setValue('test_key', ['sub_key' => 'value']);
+    Setting::createOrUpdate(['name' => 'test_key', 'value' => serialize(['sub_key' => 'value'])]);
     $this->assertEquals(null, $this->controller->get('test_key.wrong_subkey'));
   }
 
   function testItReturnsDefaultValueForUnknownSetting() {
     $this->assertEquals('default', $this->controller->get('test_key', 'default'));
     $this->assertEquals('default', $this->controller->get('test_key.sub_key', 'default'));
-    Setting::setValue('test_key', ['sub_key' => 'value']);
+    Setting::createOrUpdate(['name' => 'test_key', 'value' => serialize(['sub_key' => 'value'])]);
     $this->assertEquals('default', $this->controller->get('test_key.wrong_subkey', 'default'));
   }
 
@@ -52,8 +52,8 @@ class SettingsControllerTest extends \MailPoetTest {
   }
 
   function testItCanReturnAllSettings() {
-    Setting::setValue('test_key1', 1);
+    Setting::createOrUpdate(['name' => 'test_key1', 'value' => 1]);
-    Setting::setValue('test_key2', 2);
+    Setting::createOrUpdate(['name' => 'test_key2', 'value' => 2]);
     $all = $this->controller->getAll();
     $this->assertEquals(1, $all['test_key1']);
     $this->assertEquals(2, $all['test_key2']);
@@ -62,14 +62,14 @@ class SettingsControllerTest extends \MailPoetTest {
   function testItCanSetAtTopLevel() {
     $this->controller->set('test_key', 1);
     $this->assertEquals(1, $this->controller->get('test_key'));
-    $db_value = Setting::getValue('test_key');
+    $db_value = Setting::where('name', 'test_key')->findOne();
-    $this->assertEquals(1, $db_value);
+    $this->assertEquals(1, $db_value->value);
   }
 
   function testItCanSetAtNestedLevel() {
     $this->controller->set('test_key.key1.key2', 1);
     $this->assertEquals(1, $this->controller->get('test_key.key1.key2'));
-    $db_value = Setting::getValue('test_key');
+    $db_value = unserialize(Setting::where('name', 'test_key')->findOne()->value);
     $this->assertEquals(1, $db_value['key1']['key2']);
   }
 
@@ -78,7 +78,7 @@ class SettingsControllerTest extends \MailPoetTest {
     $this->assertEquals(1, $this->controller->get('test_key.key1.key2'));
     $this->controller->set('test_key.key1.key2', null);
     $this->assertNull(null, $this->controller->get('test_key.key1.key2'));
-    $db_value = Setting::getValue('test_key');
+    $db_value = unserialize(Setting::where('name', 'test_key')->findOne()->value);
     $this->assertNull($db_value['key1']['key2']);
   }
 
@@ -86,14 +86,14 @@ class SettingsControllerTest extends \MailPoetTest {
     $this->controller->set('test_key.key1', 1);
     $this->controller->set('test_key.key1.key2', 1);
     $this->assertEquals(1, $this->controller->get('test_key.key1.key2'));
-    $db_value = Setting::getValue('test_key');
+    $db_value = unserialize(Setting::where('name', 'test_key')->findOne()->value);
     $this->assertEquals(1, $db_value['key1']['key2']);
   }
 
   function testItLoadsFromDbOnlyOnce() {
-    Setting::setValue('test_key', 1);
+    Setting::createOrUpdate(['name' => 'test_key', 'value' => 1]);
     $this->assertEquals(1, $this->controller->get('test_key'));
-    Setting::setValue('test_key', 2);
+    Setting::createOrUpdate(['name' => 'test_key', 'value' => 2]);
     $this->assertEquals(1, $this->controller->get('test_key'));
     $this->assertEquals(true, true);
   }

tests/integration/Subscription/FormTest.php

@@ -6,12 +6,10 @@ use MailPoet\DI\ContainerWrapper;
 use MailPoet\Form\Util\FieldNameObfuscator;
 use MailPoet\Models\Form as FormModel;
 use MailPoet\Models\Segment as SegmentModel;
-use MailPoet\Models\Setting as SettingModel;
 use MailPoet\Models\Setting;
 use MailPoet\Models\Subscriber as SubscriberModel;
 use MailPoet\Settings\SettingsController;
 use MailPoet\Subscription\Form;
-use MailPoet\Test\Cron\Workers\SendingErrorHandlerTest;
 use MailPoet\Util\Security;
 
 class FormTest extends \MailPoetTest {
@@ -19,10 +17,13 @@ class FormTest extends \MailPoetTest {
   /** @var Form */
   private $form_controller;
 
+  /** @var SettingsController */
+  private $settings;
+
   function _before() {
     parent::_before();
-    $settings = new SettingsController();
+    $this->settings = new SettingsController();
-    $settings->set('sender', array(
+    $this->settings->set('sender', array(
       'name' => 'John Doe',
       'address' => 'john.doe@example.com'
     ));
@@ -66,7 +67,7 @@ class FormTest extends \MailPoetTest {
         'post_status' => 'publish',
       )
     );
-    SettingModel::setValue('signup_confirmation.enabled', false);
+    $this->settings->set('signup_confirmation.enabled', false);
     $this->form_controller = ContainerWrapper::getInstance()->get(Form::class);
   }
 
@@ -127,6 +128,6 @@ class FormTest extends \MailPoetTest {
     \ORM::raw_execute('TRUNCATE ' . SegmentModel::$_table);
     \ORM::raw_execute('TRUNCATE ' . FormModel::$_table);
     \ORM::raw_execute('TRUNCATE ' . SubscriberModel::$_table);
-    \ORM::raw_execute('TRUNCATE ' . SettingModel::$_table);
+    \ORM::raw_execute('TRUNCATE ' . Setting::$_table);
   }
 }