From 388ced8b53e17a6a3faa20ce8c8319b0cdca9a1b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jan=20Jakes=CC=8C?= <jan@jakes.pro>
Date: Mon, 28 Oct 2019 10:39:48 +0100
Subject: [PATCH] Remove static behavior from SettingsController

[MAILPOET-2436]
---
 lib/Settings/SettingsController.php           | 33 ++++++++-----------
 tests/DataFactories/Newsletter.php            |  1 -
 tests/DataFactories/Settings.php              |  2 +-
 .../integration/API/JSON/v1/SettingsTest.php  |  2 +-
 tests/integration/_bootstrap.php              |  2 +-
 5 files changed, 17 insertions(+), 23 deletions(-)

diff --git a/lib/Settings/SettingsController.php b/lib/Settings/SettingsController.php
index c5df6ca540..852e62870f 100644
--- a/lib/Settings/SettingsController.php
+++ b/lib/Settings/SettingsController.php
@@ -16,16 +16,16 @@ class SettingsController {
   const DEFAULT_SENDING_FREQUENCY_INTERVAL = 5; // in minutes
   const DEFAULT_DEACTIVATE_SUBSCRIBER_AFTER_INACTIVE_DAYS = 180;
 
-  private static $loaded = false;
+  private $loaded = false;
 
-  private static $settings = [];
+  private $settings = [];
 
   private $defaults = null;
 
   function get($key, $default = null) {
     $this->ensureLoaded();
     $key_parts = explode('.', $key);
-    $setting = self::$settings;
+    $setting = $this->settings;
     if ($default === null) {
       $default = $this->getDefaultValue($key_parts);
     }
@@ -81,13 +81,13 @@ class SettingsController {
   function fetch($key, $default = null) {
     $keys = explode('.', $key);
     $main_key = $keys[0];
-    self::$settings[$main_key] = $this->fetchValue($main_key);
+    $this->settings[$main_key] = $this->fetchValue($main_key);
     return $this->get($key, $default);
   }
 
   function getAll() {
     $this->ensureLoaded();
-    return array_replace_recursive($this->getAllDefaults(), self::$settings);
+    return array_replace_recursive($this->getAllDefaults(), $this->settings);
   }
 
   function set($key, $value) {
@@ -95,7 +95,7 @@ class SettingsController {
     $key_parts = explode('.', $key);
     $main_key = $key_parts[0];
     $last_key = array_pop($key_parts);
-    $setting =& self::$settings;
+    $setting =& $this->settings;
     foreach ($key_parts as $key_part) {
       $setting =& $setting[$key_part];
       if (!is_array($setting)) {
@@ -103,20 +103,20 @@ class SettingsController {
       }
     }
     $setting[$last_key] = $value;
-    $this->saveValue($main_key, self::$settings[$main_key]);
+    $this->saveValue($main_key, $this->settings[$main_key]);
   }
 
   function delete($key) {
     Setting::deleteValue($key);
-    unset(self::$settings[$key]);
+    unset($this->settings[$key]);
   }
 
   private function ensureLoaded() {
-    if (self::$loaded) {
+    if ($this->loaded) {
       return;
     }
-    self::$settings = Setting::getAll() ?: [];
-    self::$loaded = true;
+    $this->settings = Setting::getAll() ?: [];
+    $this->loaded = true;
   }
 
   private function getDefaultValue($keys) {
@@ -156,14 +156,9 @@ class SettingsController {
     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)
-   * Once this is achieved we can make properties not static and remove this method
-   */
-  static function resetCache() {
-    self::$settings = [];
-    self::$loaded = false;
+  function resetCache() {
+    $this->settings = [];
+    $this->loaded = false;
   }
 
   /** @return SettingsController */
diff --git a/tests/DataFactories/Newsletter.php b/tests/DataFactories/Newsletter.php
index 5dec056413..760a258809 100644
--- a/tests/DataFactories/Newsletter.php
+++ b/tests/DataFactories/Newsletter.php
@@ -39,7 +39,6 @@ class Newsletter {
     $this->queue_options = [];
     $this->task_subscribers = [];
     $this->loadBodyFrom('newsletterWithALC.json');
-    SettingsController::resetCache(); // Newsletter model reads settings so we need to ensure it use fresh data
   }
 
   /**
diff --git a/tests/DataFactories/Settings.php b/tests/DataFactories/Settings.php
index c547b8f512..73a0508956 100644
--- a/tests/DataFactories/Settings.php
+++ b/tests/DataFactories/Settings.php
@@ -10,8 +10,8 @@ class Settings {
   private $settings;
 
   public function __construct() {
-    SettingsController::resetCache();
     $this->settings = SettingsController::getInstance();
+    $this->settings->resetCache();
   }
 
   function withDefaultSettings() {
diff --git a/tests/integration/API/JSON/v1/SettingsTest.php b/tests/integration/API/JSON/v1/SettingsTest.php
index c70a818d86..bf476a4e0c 100644
--- a/tests/integration/API/JSON/v1/SettingsTest.php
+++ b/tests/integration/API/JSON/v1/SettingsTest.php
@@ -42,7 +42,7 @@ class SettingsTest extends \MailPoetTest {
     expect($response->data['some']['setting']['key'])->true();
 
     Setting::deleteMany();
-    SettingsController::resetCache();
+    $this->settings->resetCache();
     $response = $this->endpoint->get();
     expect($response->status)->equals(APIResponse::STATUS_OK);
     expect($response->data)->equals($this->settings->getAllDefaults());
diff --git a/tests/integration/_bootstrap.php b/tests/integration/_bootstrap.php
index 494db6951d..61d109bacd 100644
--- a/tests/integration/_bootstrap.php
+++ b/tests/integration/_bootstrap.php
@@ -149,7 +149,7 @@ abstract class MailPoetTest extends \Codeception\TestCase\Test {
     $this->di_container = ContainerWrapper::getInstance(WP_DEBUG);
     $this->connection = $this->di_container->get(Connection::class);
     $this->entity_manager = $this->di_container->get(EntityManager::class);
-    \MailPoet\Settings\SettingsController::resetCache();
+    $this->di_container->get(SettingsController::class)->resetCache();
     parent::setUp();
   }