Moved Config object to the DI container and override w/ TestConfig th…

…rough DI config. Replaced the singleton GlobalSettingsProvider hack w/ the concept of an EnvironmentManipulator (internal to Piwik, not to be used anywhere but TestingEnvironment.php).
matomo-org · May 24, 2015 · 38a51b0 · 38a51b0
1 parent 0afc478
commit 38a51b0
Show file tree

Hide file tree

Showing 12 changed files with 127 additions and 103 deletions.
diff --git a/config/environment/test.php b/config/environment/test.php
@@ -14,4 +14,5 @@
     'Piwik\Translation\Translator' => DI\object()
         ->constructorParameter('directories', array()),
 
+    'Piwik\Config' => DI\object('Piwik\Tests\Framework\Mock\TestConfig'),
 );
diff --git a/core/Application/Environment.php b/core/Application/Environment.php
@@ -39,6 +39,12 @@
  */
 class Environment
 {
+    /**
+     * @internal
+     * @var EnvironmentManipulator[]
+     */
+    private static $globalEnvironmentManipulators = array();
+
     /**
      * @var string
      */
@@ -115,7 +121,8 @@ private function createContainer()
     protected function getGlobalSettingsCached()
     {
         if ($this->globalSettingsProvider === null) {
-            $this->globalSettingsProvider = $this->getGlobalSettings();
+            $globalSettingsProvider = $this->getGlobalSettingsProviderOverride();
+            $this->globalSettingsProvider = $globalSettingsProvider ?: $this->getGlobalSettings();
         }
         return $this->globalSettingsProvider;
     }
@@ -136,9 +143,7 @@ protected function getPluginListCached()
      */
     protected function getGlobalSettings()
     {
-        // TODO: need to be able to set path global/local/etc. which is in DI... for now works because TestingEnvironment creates
-        //       singleton instance before this method.
-        return GlobalSettingsProvider::getSingletonInstance();
+        return new GlobalSettingsProvider();
     }
 
     /**
@@ -159,4 +164,25 @@ private function validateEnvironment()
         $validator = $this->container->get('Piwik\Application\Kernel\EnvironmentValidator');
         $validator->validate();
     }
+
+    /**
+     * @param EnvironmentManipulator $manipulator
+     * @internal
+     */
+    public static function addEnvironmentManipulator(EnvironmentManipulator $manipulator)
+    {
+        self::$globalEnvironmentManipulators[] = $manipulator;
+    }
+
+    private function getGlobalSettingsProviderOverride()
+    {
+        foreach (self::$globalEnvironmentManipulators as $manipulator) {
+            $result = $manipulator->makeGlobalSettingsProvider();
+            if (!empty($result)) {
+                return $result;
+            }
+        }
+
+        return null;
+    }
 }
diff --git a/core/Application/EnvironmentManipulator.php b/core/Application/EnvironmentManipulator.php
@@ -0,0 +1,26 @@
+<?php
+/**
+ * Piwik - free/libre analytics platform
+ *
+ * @link http://piwik.org
+ * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later
+ */
+
+namespace Piwik\Application;
+
+use Piwik\Application\Kernel\GlobalSettingsProvider;
+
+/**
+ * Used to manipulate Environment instances before the container is created.
+ * Only used by the testing environment setup code, shouldn't be used anywhere
+ * else.
+ */
+interface EnvironmentManipulator
+{
+    /**
+     * Create a custom GlobalSettingsProvider kernel object, overriding the default behavior.
+     *
+     * @return GlobalSettingsProvider
+     */
+    public function makeGlobalSettingsProvider();
+}
diff --git a/core/Application/Kernel/GlobalSettingsProvider.php b/core/Application/Kernel/GlobalSettingsProvider.php
@@ -22,8 +22,6 @@
  */
 class GlobalSettingsProvider
 {
-    private static $instance = null;
-
     /**
      * @var IniFileChain
      */
@@ -110,30 +108,4 @@ public function getPathCommon()
     {
         return $this->pathCommon;
     }
-
-    public static function getSingletonInstance($pathGlobal = null, $pathLocal = null, $pathCommon = null)
-    {
-        if (self::$instance === null) {
-            self::$instance = new GlobalSettingsProvider($pathGlobal, $pathLocal, $pathCommon);
-        } else {
-            // sanity check. the parameters should only be non-null when creating the GlobalSettingsProvider the first time.
-            // if it's done after, it may point to a problem in the tests. (tests are the only place where these arguments
-            // should be specified)
-            if ($pathGlobal !== null
-                || $pathLocal !== null
-                || $pathCommon !== null
-            ) {
-                $message = "Unexpected state in GlobalSettingsProvider::getSingletonInstance: singleton already created but paths supplied:\n";
-                $message .= "global = '$pathGlobal', local = '$pathLocal', common = '$pathCommon'\n";
-                throw new \Exception($message);
-            }
-        }
-
-        return self::$instance;
-    }
-
-    public static function unsetSingletonInstance()
-    {
-        self::$instance = null;
-    }
 }
diff --git a/core/Config.php b/core/Config.php
@@ -11,6 +11,7 @@
 
 use Exception;
 use Piwik\Application\Kernel\GlobalSettingsProvider;
+use Piwik\Container\StaticContainer;
 
 /**
  * Singleton that provides read & write access to Piwik's INI configuration.
@@ -36,8 +37,6 @@
  *
  *     Config::getInstance()->MySection = array('myoption' => 1);
  *     Config::getInstance()->forceSave();
- *
- * @method static Config getInstance()
  */
 class Config extends Singleton
 {
@@ -55,9 +54,17 @@ class Config extends Singleton
      */
     protected $settings;
 
-    public function __construct($pathGlobal = null, $pathLocal = null, $pathCommon = null)
+    /**
+     * @return Config
+     */
+    public static function getInstance()
+    {
+        return StaticContainer::get('Piwik\Config');
+    }
+
+    public function __construct(GlobalSettingsProvider $settings)
     {
-        $this->settings = GlobalSettingsProvider::getSingletonInstance($pathGlobal, $pathLocal, $pathCommon);
+        $this->settings = $settings;
     }
 
     /**

diff --git a/tests/PHPUnit/Framework/Fixture.php b/tests/PHPUnit/Framework/Fixture.php
@@ -9,7 +9,6 @@
 
 use Piwik\Access;
 use Piwik\Application\Environment;
-use Piwik\Application\Kernel\GlobalSettingsProvider;
 use Piwik\Archive;
 use Piwik\Cache\Backend\File;
 use Piwik\Cache as PiwikCache;
@@ -20,7 +19,6 @@
 use Piwik\Date;
 use Piwik\Db;
 use Piwik\DbHelper;
-use Piwik\EventDispatcher;
 use Piwik\Ini\IniReader;
 use Piwik\Log;
 use Piwik\Option;
@@ -42,7 +40,6 @@
 use Piwik\SettingsServer;
 use Piwik\Site;
 use Piwik\Tests\Framework\Mock\FakeAccess;
-use Piwik\Tests\Framework\Mock\TestConfig;
 use Piwik\Tests\Framework\TestCase\SystemTestCase;
 use Piwik\Tracker;
 use Piwik\Tracker\Cache;
@@ -53,7 +50,6 @@
 use PiwikTracker;
 use Piwik_LocalTracker;
 use Piwik\Updater;
-use Piwik\Plugins\CoreUpdater\CoreUpdater;
 use Exception;
 use ReflectionClass;
 
@@ -83,7 +79,12 @@ class Fixture extends \PHPUnit_Framework_Assert
     const ADMIN_USER_PASSWORD = 'superUserPass';
 
     public $dbName = false;
+
+    /**
+     * @deprecated has no effect now.
+     */
     public $createConfig = true;
+
     public $dropDatabaseInSetUp = true;
     public $dropDatabaseInTearDown = true;
     public $loadTranslations = true;
@@ -158,16 +159,8 @@ public function getDbName()
 
     public function performSetUp($setupEnvironmentOnly = false)
     {
-        if ($this->createConfig) {
-            GlobalSettingsProvider::unsetSingletonInstance();
-        }
-
         $this->createEnvironmentInstance();
 
-        if ($this->createConfig) {
-            Config::setSingletonInstance(new TestConfig());
-        }
-
         try {
             $this->dbName = $this->getDbName();
 
@@ -336,9 +329,6 @@ public function clearInMemoryCaches()
         $_GET = $_REQUEST = array();
         Translate::reset();
 
-        GlobalSettingsProvider::unsetSingletonInstance();
-        Config::setSingletonInstance(new TestConfig());
-
         Config::getInstance()->Plugins; // make sure Plugins exists in a config object for next tests that use Plugin\Manager
         // since Plugin\Manager uses getFromGlobalConfig which doesn't init the config object
     }

diff --git a/tests/PHPUnit/Framework/Mock/TestConfig.php b/tests/PHPUnit/Framework/Mock/TestConfig.php
@@ -8,23 +8,22 @@
 
 namespace Piwik\Tests\Framework\Mock;
 
+use Piwik\Application\Kernel\GlobalSettingsProvider;
 use Piwik\Config;
 
 class TestConfig extends Config
 {
     private $allowSave = false;
     private $doSetTestEnvironment = false;
 
-    public function __construct($pathGlobal = null, $pathLocal = null, $pathCommon = null, $allowSave = false, $doSetTestEnvironment = true)
+    public function __construct(GlobalSettingsProvider $provider, $allowSave = false, $doSetTestEnvironment = true)
     {
-        \Piwik\Application\Kernel\GlobalSettingsProvider::unsetSingletonInstance();
-
-        parent::__construct($pathGlobal, $pathLocal, $pathCommon);
+        parent::__construct($provider);
 
         $this->allowSave = $allowSave;
         $this->doSetTestEnvironment = $doSetTestEnvironment;
 
-        $this->reload($pathGlobal, $pathLocal, $pathCommon);
+        $this->reload();
 
         $testingEnvironment = new \Piwik_TestingEnvironment();
         $this->setFromTestEnvironment($testingEnvironment);

diff --git a/tests/PHPUnit/Framework/TestCase/UnitTestCase.php b/tests/PHPUnit/Framework/TestCase/UnitTestCase.php
@@ -9,8 +9,6 @@
 namespace Piwik\Tests\Framework\TestCase;
 
 use Piwik\Application\Environment;
-use Piwik\Application\Kernel\GlobalSettingsProvider;
-use Piwik\Container\StaticContainer;
 use Piwik\EventDispatcher;
 use Piwik\Tests\Framework\Mock\File;
 
@@ -30,8 +28,6 @@ public function setUp()
     {
         parent::setUp();
 
-        GlobalSettingsProvider::unsetSingletonInstance();
-
         $this->initEnvironment();
 
         File::reset();
@@ -41,8 +37,6 @@ public function tearDown()
     {
         File::reset();
 
-        GlobalSettingsProvider::unsetSingletonInstance();
-
         // make sure the global container exists for the next test case that is executed (since logging can be done
         // before a test sets up an environment)
         $nextTestEnviornment = new Environment('test', array(), $postBootstrappedEvent = false);

diff --git a/tests/PHPUnit/Framework/TestingEnvironment/MakeGlobalSettingsWithFile.php b/tests/PHPUnit/Framework/TestingEnvironment/MakeGlobalSettingsWithFile.php
@@ -0,0 +1,31 @@
+<?php
+/**
+ * Piwik - free/libre analytics platform
+ *
+ * @link http://piwik.org
+ * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later
+ */
+namespace Piwik\Tests\Framework\TestingEnvironment;
+
+use Piwik\Application\EnvironmentManipulator;
+use Piwik\Application\Kernel\GlobalSettingsProvider;
+use Piwik\Tests\Framework\TestingEnvironment;
+
+class MakeGlobalSettingsWithFile implements EnvironmentManipulator
+{
+    private $configFileGlobal;
+    private $configFileLocal;
+    private $configFileCommon;
+
+    public function __construct(\Piwik_TestingEnvironment $testingEnvironment)
+    {
+        $this->configFileGlobal = $testingEnvironment->configFileGlobal;
+        $this->configFileLocal = $testingEnvironment->configFileLocal;
+        $this->configFileCommon = $testingEnvironment->configFileCommon;
+    }
+
+    public function makeGlobalSettingsProvider()
+    {
+        return new GlobalSettingsProvider($this->configFileGlobal, $this->configFileLocal, $this->configFileCommon);
+    }
+}
diff --git a/tests/PHPUnit/Integration/TrackerTest.php b/tests/PHPUnit/Integration/TrackerTest.php
@@ -59,7 +59,6 @@ public function setUp()
     {
         parent::setUp();
 
-        GlobalSettingsProvider::unsetSingletonInstance();
         Config::unsetInstance();
 
         Fixture::createWebsite('2014-01-01 00:00:00');