Make feature defaults array private

[MAILPOET-2008]

lib/API/JSON/v1/FeatureFlags.php

@@ -12,14 +12,18 @@ if (!defined('ABSPATH')) exit;
 
 class FeatureFlags extends APIEndpoint {
 
-  /** @var FeatureFlagsController */
-  private $feature_flags_controller;
-
   public $permissions = [
     'global' => AccessControl::PERMISSION_MANAGE_FEATURES,
   ];
 
-  function __construct(FeatureFlagsController $feature_flags) {
+  /** @var FeaturesController */
+  private $features_controller;
+
+  /** @var FeatureFlagsController */
+  private $feature_flags_controller;
+
+  function __construct(FeaturesController $features_controller, FeatureFlagsController $feature_flags) {
+    $this->features_controller = $features_controller;
     $this->feature_flags_controller = $feature_flags;
   }
 
@@ -30,7 +34,7 @@ class FeatureFlags extends APIEndpoint {
 
   function set(array $flags) {
     foreach ($flags as $name => $value) {
-      if (!isset(FeaturesController::$defaults[$name])) {
+      if (!$this->features_controller->exists($name)) {
         return $this->badRequest([
           APIError::BAD_REQUEST => "Feature '$name' does not exist'",
         ]);

lib/Features/FeatureFlagsController.php

@@ -6,8 +6,15 @@ use MailPoet\Models\FeatureFlag;
 use function MailPoet\Util\array_column;
 
 class FeatureFlagsController {
+  /** @var FeaturesController */
+  private $features_controller;
+
+  function __construct(FeaturesController $features_controller) {
+    $this->features_controller = $features_controller;
+  }
+
   function set($name, $value) {
-    if (!isset(FeaturesController::$defaults[$name])) {
+    if (!$this->features_controller->exists($name)) {
       throw new \RuntimeException("Feature '$name' does not exist'");
     }
 
@@ -26,7 +33,7 @@ class FeatureFlagsController {
     $flagsMap = array_combine(array_column($flags, 'name'), $flags);
 
     $output = [];
-    foreach (FeaturesController::$defaults as $name => $default) {
+    foreach ($this->features_controller->getDefaults() as $name => $default) {
       $output[] = [
         'name' => $name,
         'value' => isset($flagsMap[$name]) ? (bool)$flagsMap[$name]['value'] : $default,

lib/Features/FeaturesController.php

@@ -12,7 +12,7 @@ class FeaturesController {
 
   // Define feature defaults in the array below in the following form:
   //   self::FEATURE_NAME_OF_FEATURE => true,
-  public static $defaults = [
+  private $defaults = [
     self::FEATURE_DISPLAY_WOOCOMMERCE_REVENUES => false,
   ];
 
@@ -21,13 +21,23 @@ class FeaturesController {
 
   /** @return bool */
   function isSupported($feature) {
-    $this->ensureFlagsLoaded();
+    if (!$this->exists($feature)) {
-    if (!array_key_exists($feature, $this->flags)) {
       throw new \RuntimeException("Unknown feature '$feature'");
     }
+    $this->ensureFlagsLoaded();
     return $this->flags[$feature];
   }
 
+  /** @return bool */
+  function exists($feature) {
+    return array_key_exists($feature, $this->defaults);
+  }
+
+  /** @return array */
+  function getDefaults() {
+    return $this->defaults;
+  }
+
   /** @return array */
   function getAllFlags() {
     $this->ensureFlagsLoaded();
@@ -41,7 +51,7 @@ class FeaturesController {
 
     $this->flags = [];
     $flagsMap = $this->getValueMap();
-    foreach (self::$defaults as $name => $default) {
+    foreach ($this->defaults as $name => $default) {
       $this->flags[$name] = isset($flagsMap[$name]) ? $flagsMap[$name] : $default;
     }
   }

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

@@ -9,23 +9,18 @@ use MailPoet\Features\FeaturesController;
 use MailPoet\Models\FeatureFlag;
 
 class FeatureFlagsTest extends \MailPoetTest {
-  /** @var array */
-  private $defaults_backup;
 
   function _before() {
     parent::_before();
     FeatureFlag::deleteMany();
-    $this->defaults_backup = FeaturesController::$defaults;
   }
 
   function testItReturnsDefaults() {
-    FeaturesController::$defaults = [
+    $endpoint = $this->createEndpointWithFeatureDefaults([
       'feature-a' => true,
       'feature-b' => false,
-    ];
+    ]);
 
-    $controller = new FeatureFlagsController();
-    $endpoint = new FeatureFlags($controller);
     expect($endpoint->getAll()->data)->equals([
       [
         'name' => 'feature-a',
@@ -41,17 +36,14 @@ class FeatureFlagsTest extends \MailPoetTest {
   }
 
   function testItReturnsDatabaseValue() {
-    FeaturesController::$defaults = [
-      'feature-a' => true,
-    ];
-
     FeatureFlag::createOrUpdate([
       'name' => 'feature-a',
       'value' => false,
     ]);
 
-    $controller = new FeatureFlagsController();
+    $endpoint = $this->createEndpointWithFeatureDefaults([
-    $endpoint = new FeatureFlags($controller);
+      'feature-a' => true,
+    ]);
 
     expect($endpoint->getAll()->data)->equals([
       [
@@ -63,12 +55,10 @@ class FeatureFlagsTest extends \MailPoetTest {
   }
 
   function testItSetsDatabaseValue() {
-    FeaturesController::$defaults = [
+    $endpoint = $this->createEndpointWithFeatureDefaults([
       'feature-a' => true,
-    ];
+    ]);
 
-    $controller = new FeatureFlagsController();
-    $endpoint = new FeatureFlags($controller);
     $endpoint->set([
       'feature-a' => false,
     ]);
@@ -81,17 +71,15 @@ class FeatureFlagsTest extends \MailPoetTest {
 
 
   function testItUpdatesDatabaseValue() {
-    FeaturesController::$defaults = [
-      'feature-a' => true,
-    ];
-
     FeatureFlag::createOrUpdate([
       'name' => 'feature-a',
       'value' => false,
     ]);
 
-    $controller = new FeatureFlagsController();
+    $endpoint = $this->createEndpointWithFeatureDefaults([
-    $endpoint = new FeatureFlags($controller);
+      'feature-a' => true,
+    ]);
+
     $endpoint->set([
       'feature-a' => true,
     ]);
@@ -103,24 +91,17 @@ class FeatureFlagsTest extends \MailPoetTest {
   }
 
   function testItDoesNotReturnUnknownFlag() {
-    FeaturesController::$defaults = [];
-
     FeatureFlag::createOrUpdate([
       'name' => 'feature-unknown',
       'value' => true,
     ]);
 
-    $controller = new FeatureFlagsController();
+    $endpoint = $this->createEndpointWithFeatureDefaults([]);
-    $endpoint = new FeatureFlags($controller);
-
     expect($endpoint->getAll()->data)->isEmpty();
   }
 
   function testItDoesNotSaveUnknownFlag() {
-    FeaturesController::$defaults = [];
+    $endpoint = $this->createEndpointWithFeatureDefaults([]);
-
-    $controller = new FeatureFlagsController();
-    $endpoint = new FeatureFlags($controller);
     $response = $endpoint->set([
       'feature-unknown' => false,
     ]);
@@ -132,9 +113,16 @@ class FeatureFlagsTest extends \MailPoetTest {
     expect(count($features))->equals(0);
   }
 
+  private function createEndpointWithFeatureDefaults(array $defaults) {
+    $features_controller = $this->make(FeaturesController::class, [
+      'defaults' => $defaults,
+    ]);
+    $controller = new FeatureFlagsController($features_controller);
+    return new FeatureFlags($features_controller, $controller);
+  }
+
   function _after() {
     parent::_before();
     FeatureFlag::deleteMany();
-    FeaturesController::$defaults = $this->defaults_backup;
   }
 }

tests/integration/Features/FeaturesControllerTest.php

@@ -5,22 +5,20 @@ use MailPoet\Features\FeaturesController;
 use MailPoet\Models\FeatureFlag;
 
 class FeaturesControllerTest extends \MailPoetTest {
-  /** @var array */
-  private $defaults_backup;
 
   function _before() {
     parent::_before();
     FeatureFlag::deleteMany();
-    $this->defaults_backup = FeaturesController::$defaults;
   }
 
-  function testItReturnsDefaults() {
+  function testItWorksWithDefaults() {
-    FeaturesController::$defaults = [
+    $controller = $this->make(FeaturesController::class, [
-      'feature-a' => true,
+      'defaults' => [
-      'feature-b' => false,
+        'feature-a' => true,
-    ];
+        'feature-b' => false,
+      ],
+    ]);
 
-    $controller = new FeaturesController();
     expect($controller->isSupported('feature-a'))->equals(true);
     expect($controller->isSupported('feature-b'))->equals(false);
     expect($controller->getAllFlags())->equals([
@@ -29,17 +27,18 @@ class FeaturesControllerTest extends \MailPoetTest {
     ]);
   }
 
-  function testItReturnsDatabaseValue() {
+  function testItWorksWithDatabaseValues() {
-    FeaturesController::$defaults = [
-      'feature-a' => true,
-    ];
-
     FeatureFlag::createOrUpdate([
       'name' => 'feature-a',
       'value' => false,
     ]);
 
-    $controller = new FeaturesController();
+    $controller = $this->make(FeaturesController::class, [
+      'defaults' => [
+        'feature-a' => true,
+      ],
+    ]);
+
     expect($controller->isSupported('feature-a'))->equals(false);
     expect($controller->getAllFlags())->equals([
       'feature-a' => false,
@@ -47,14 +46,15 @@ class FeaturesControllerTest extends \MailPoetTest {
   }
 
   function testItDoesNotReturnUnknownFlag() {
-    FeaturesController::$defaults = [];
-
     FeatureFlag::createOrUpdate([
       'name' => 'feature-unknown',
       'value' => true,
     ]);
 
-    $controller = new FeaturesController();
+    $controller = $this->make(FeaturesController::class, [
+      'defaults' => [],
+    ]);
+
     try {
       $controller->isSupported('feature-unknown');
     } catch (\RuntimeException $e) {
@@ -66,6 +66,5 @@ class FeaturesControllerTest extends \MailPoetTest {
   function _after() {
     parent::_before();
     FeatureFlag::deleteMany();
-    FeaturesController::$defaults = $this->defaults_backup;
   }
 }