Stop using a singleton for Config (#298)

classes/Config.php

@@ -20,12 +20,6 @@ use Jawira\CaseConverter\Convert;
  */
 class Config
 {
-    /**
-     * Singleton instance.
-     *
-     * @var Config|null
-     */
-    private static $instance;
 
     /**
      * youtube-dl binary path.
@@ -160,10 +154,11 @@ class Config
      * @param mixed[] $options Options
      * @throws ConfigException
      */
-    private function __construct(array $options = [])
+    public function __construct(array $options = [])
     {
         $this->applyOptions($options);
         $this->getEnv();
+        $this->validateOptions();
         $localeManager = LocaleManager::getInstance();
 
         if (empty($this->genericFormats)) {
@@ -271,34 +266,17 @@ class Config
         }
     }
 
-    /**
-     * Get Config singleton instance.
-     *
-     * @return Config
-     * @todo Stop using a singleton.
-     */
-    public static function getInstance()
-    {
-        if (!isset(self::$instance)) {
-            self::$instance = new self();
-        }
-
-        return self::$instance;
-    }
-
     /**
      * Set options from a YAML file.
      *
      * @param string $file Path to the YAML file
-     * @return void
+     * @return Config
      * @throws ConfigException
      */
-    public static function setFile(string $file)
+    public static function fromFile(string $file)
     {
         if (is_file($file)) {
-            $options = Yaml::parse(strval(file_get_contents($file)));
+            return new self(Yaml::parse(strval(file_get_contents($file))));
-            self::$instance = new self($options);
-            self::$instance->validateOptions();
         } else {
             throw new ConfigException("Can't find config file at " . $file);
         }
@@ -308,29 +286,13 @@ class Config
      * Manually set some options.
      *
      * @param mixed[] $options Options (see `config/config.example.yml` for available options)
-     * @param bool $update True to update an existing instance
      * @return void
      * @throws ConfigException
      */
-    public static function setOptions(array $options, $update = true)
+    public function setOptions(array $options)
     {
-        if ($update) {
+        $this->applyOptions($options);
-            $config = self::getInstance();
+        $this->validateOptions();
-            $config->applyOptions($options);
-            $config->validateOptions();
-        } else {
-            self::$instance = new self($options);
-        }
-    }
-
-    /**
-     * Destroy singleton instance.
-     *
-     * @return void
-     */
-    public static function destroyInstance()
-    {
-        self::$instance = null;
     }
 
     /**

classes/ConfigFactory.php

@@ -19,10 +19,10 @@ class ConfigFactory
     {
         $configPath = __DIR__ . '/../config/config.yml';
         if (is_file($configPath)) {
-            Config::setFile($configPath);
+            $config = Config::fromFile($configPath);
+        } else {
+            $config = new Config();
         }
-
-        $config = Config::getInstance();
         if ($config->uglyUrls) {
             $container['router'] = new UglyRouter();
         }

tests/BaseTest.php

@@ -27,22 +27,12 @@ abstract class BaseTest extends TestCase
 
     /**
      * Prepare tests.
-     * @throws ConfigException
      */
     protected function setUp(): void
     {
-        Config::setFile($this->getConfigFile());
         $this->checkRequirements();
     }
 
-    /**
-     * Destroy properties after test.
-     */
-    protected function tearDown(): void
-    {
-        Config::destroyInstance();
-    }
-
     /**
      * Check tests requirements.
      * @return void

tests/ConfigTest.php

@@ -14,23 +14,6 @@ use Alltube\Exception\ConfigException;
  */
 class ConfigTest extends BaseTest
 {
-    /**
-     * Config class instance.
-     *
-     * @var Config
-     */
-    private $config;
-
-    /**
-     * Prepare tests.
-     * @throws ConfigException
-     */
-    protected function setUp(): void
-    {
-        parent::setUp();
-
-        $this->config = Config::getInstance();
-    }
 
     /**
      * Test the getInstance function.
@@ -39,21 +22,7 @@ class ConfigTest extends BaseTest
      */
     public function testGetInstance()
     {
-        $config = Config::getInstance();
+        $config = new Config();
-        $this->assertEquals(false, $config->convert);
-        $this->assertConfig($config);
-    }
-
-    /**
-     * Test the getInstance function.
-     *
-     * @return void
-     */
-    public function testGetInstanceFromScratch()
-    {
-        Config::destroyInstance();
-
-        $config = Config::getInstance();
         $this->assertEquals(false, $config->convert);
         $this->assertConfig($config);
     }
@@ -88,8 +57,8 @@ class ConfigTest extends BaseTest
      */
     public function testSetFile()
     {
-        Config::setFile($this->getConfigFile());
+        $config = Config::fromFile($this->getConfigFile());
-        $this->assertConfig($this->config);
+        $this->assertConfig($config);
     }
 
     /**
@@ -100,7 +69,7 @@ class ConfigTest extends BaseTest
     public function testSetFileWithMissingFile()
     {
         $this->expectException(ConfigException::class);
-        Config::setFile('foo');
+        Config::fromFile('foo');
     }
 
     /**
@@ -111,21 +80,8 @@ class ConfigTest extends BaseTest
      */
     public function testSetOptions()
     {
-        Config::setOptions(['appName' => 'foo']);
+        $config = new Config();
-        $config = Config::getInstance();
+        $config->setOptions(['appName' => 'foo']);
-        $this->assertEquals('foo', $config->appName);
-    }
-
-    /**
-     * Test the setOptions function.
-     *
-     * @return void
-     * @throws ConfigException
-     */
-    public function testSetOptionsWithoutUpdate()
-    {
-        Config::setOptions(['appName' => 'foo'], false);
-        $config = Config::getInstance();
         $this->assertEquals('foo', $config->appName);
     }
 
@@ -137,7 +93,8 @@ class ConfigTest extends BaseTest
     public function testSetOptionsWithBadYoutubedl()
     {
         $this->expectException(ConfigException::class);
-        Config::setOptions(['youtubedl' => 'foo']);
+        $config = new Config();
+        $config->setOptions(['youtubedl' => 'foo']);
     }
 
     /**
@@ -148,7 +105,8 @@ class ConfigTest extends BaseTest
     public function testSetOptionsWithBadPython()
     {
         $this->expectException(ConfigException::class);
-        Config::setOptions(['python' => 'foo']);
+        $config = new Config();
+        $config->setOptions(['python' => 'foo']);
     }
 
     /**
@@ -159,10 +117,8 @@ class ConfigTest extends BaseTest
      */
     public function testGetInstanceWithEnv()
     {
-        Config::destroyInstance();
         putenv('CONVERT=1');
-        Config::setFile($this->getConfigFile());
+        $config = Config::fromFile($this->getConfigFile());
-        $config = Config::getInstance();
         $this->assertEquals(true, $config->convert);
         putenv('CONVERT');
     }

tests/ControllerTest.php

@@ -65,7 +65,7 @@ abstract class ControllerTest extends BaseTest
         $this->container = new Container();
         $this->request = Request::createFromEnvironment(Environment::mock());
         $this->response = new Response();
-        $this->container['config'] = Config::getInstance();
+        $this->container['config'] = Config::fromFile($this->getConfigFile());
         $this->container['locale'] = LocaleManagerFactory::create();
         $this->container['view'] = ViewFactory::create($this->container, $this->request);
         $this->container['logger'] = new NullLogger();

tests/ConvertedPlaylistArchiveStreamTest.php

@@ -6,7 +6,6 @@
 
 namespace Alltube\Test;
 
-use Alltube\Config;
 use Alltube\Exception\ConfigException;
 use Alltube\Stream\ConvertedPlaylistArchiveStream;
 
@@ -24,10 +23,10 @@ class ConvertedPlaylistArchiveStreamTest extends StreamTest
     {
         parent::setUp();
 
-        $config = Config::getInstance();
+        $video = $this->downloader->getVideo(
-        $downloader = $config->getDownloader();
+            'https://www.youtube.com/playlist?list=PL1j4Ff8cAqPu5iowaeUAY8lRgkfT4RybJ'
-        $video = $downloader->getVideo('https://www.youtube.com/playlist?list=PL1j4Ff8cAqPu5iowaeUAY8lRgkfT4RybJ');
+        );
 
-        $this->stream = new ConvertedPlaylistArchiveStream($downloader, $video);
+        $this->stream = new ConvertedPlaylistArchiveStream($this->downloader, $video);
     }
 }

tests/DownloadControllerTest.php

@@ -6,7 +6,6 @@
 
 namespace Alltube\Test;
 
-use Alltube\Config;
 use Alltube\Controller\DownloadController;
 use Alltube\Exception\ConfigException;
 use Alltube\Exception\DependencyException;
@@ -69,11 +68,11 @@ class DownloadControllerTest extends ControllerTest
      * Test the download() function with streams enabled.
      *
      * @return void
-     * @throws ConfigException
      */
     public function testDownloadWithStream()
     {
-        Config::setOptions(['stream' => true]);
+        $config = $this->container->get('config');
+        $config->setOptions(['stream' => true]);
 
         $this->assertRequestIsOk(
             'download',
@@ -85,11 +84,11 @@ class DownloadControllerTest extends ControllerTest
      * Test the download() function with an M3U stream.
      *
      * @return void
-     * @throws ConfigException
      */
     public function testDownloadWithM3uStream()
     {
-        Config::setOptions(['stream' => true]);
+        $config = $this->container->get('config');
+        $config->setOptions(['stream' => true]);
 
         $this->assertRequestIsOk(
             'download',
@@ -105,13 +104,13 @@ class DownloadControllerTest extends ControllerTest
      * Test the download() function with an RTMP stream.
      *
      * @return void
-     * @throws ConfigException
      */
     public function testDownloadWithRtmpStream()
     {
         $this->markTestIncomplete('We need to find another RTMP video.');
 
-        Config::setOptions(['stream' => true]);
+        $config = $this->container->get('config');
+        $config->setOptions(['stream' => true]);
 
         $this->assertRequestIsOk(
             'download',
@@ -123,11 +122,11 @@ class DownloadControllerTest extends ControllerTest
      * Test the download() function with a remuxed video.
      *
      * @return void
-     * @throws ConfigException
      */
     public function testDownloadWithRemux()
     {
-        Config::setOptions(['remux' => true]);
+        $config = $this->container->get('config');
+        $config->setOptions(['remux' => true]);
 
         $this->assertRequestIsOk(
             'download',
@@ -196,11 +195,11 @@ class DownloadControllerTest extends ControllerTest
      *
      * @return void
      * @requires OS Linux
-     * @throws ConfigException
      */
     public function testDownloadWithPlaylist()
     {
-        Config::setOptions(['stream' => true]);
+        $config = $this->container->get('config');
+        $config->setOptions(['stream' => true]);
 
         $this->assertRequestIsOk(
             'download',
@@ -212,11 +211,11 @@ class DownloadControllerTest extends ControllerTest
      * Test the download() function with an advanced conversion.
      *
      * @return void
-     * @throws ConfigException
      */
     public function testDownloadWithAdvancedConversion()
     {
-        Config::setOptions(['convertAdvanced' => true]);
+        $config = $this->container->get('config');
+        $config->setOptions(['convertAdvanced' => true]);
 
         $this->assertRequestIsOk(
             'download',

tests/FrontControllerTest.php

@@ -6,7 +6,6 @@
 
 namespace Alltube\Test;
 
-use Alltube\Config;
 use Alltube\Controller\FrontController;
 use Alltube\Exception\ConfigException;
 use Alltube\Exception\DependencyException;
@@ -52,11 +51,11 @@ class FrontControllerTest extends ControllerTest
      * Test the constructor with streams enabled.
      *
      * @return void
-     * @throws ConfigException
      */
     public function testConstructorWithStream()
     {
-        Config::setOptions(['stream' => true]);
+        $config = $this->container->get('config');
+        $config->setOptions(['stream' => true]);
         $this->assertInstanceOf(FrontController::class, new FrontController($this->container));
     }
 
@@ -132,11 +131,11 @@ class FrontControllerTest extends ControllerTest
      *
      * @return void
      * @requires download
-     * @throws ConfigException
      */
     public function testInfoWithAudio()
     {
-        Config::setOptions(['convert' => true]);
+        $config = $this->container->get('config');
+        $config->setOptions(['convert' => true]);
 
         $this->assertRequestIsRedirect(
             'info',
@@ -149,11 +148,11 @@ class FrontControllerTest extends ControllerTest
      *
      * @return void
      * @requires download
-     * @throws ConfigException
      */
     public function testInfoWithVimeoAudio()
     {
-        Config::setOptions(['convert' => true]);
+        $config = $this->container->get('config');
+        $config->setOptions(['convert' => true]);
 
         // So we can test the fallback to default format
         $this->assertRequestIsRedirect('info', ['url' => 'https://vimeo.com/251997032', 'audio' => true]);
@@ -164,11 +163,11 @@ class FrontControllerTest extends ControllerTest
      *
      * @return void
      * @requires download
-     * @throws ConfigException
      */
     public function testInfoWithUnconvertedAudio()
     {
-        Config::setOptions(['convert' => true]);
+        $config = $this->container->get('config');
+        $config->setOptions(['convert' => true]);
 
         $this->assertRequestIsRedirect(
             'info',
@@ -213,11 +212,11 @@ class FrontControllerTest extends ControllerTest
      *
      * @return void
      * @requires download
-     * @throws ConfigException
      */
     public function testInfoWithStream()
     {
-        Config::setOptions(['stream' => true]);
+        $config = $this->container->get('config');
+        $config->setOptions(['stream' => true]);
 
         $this->assertRequestIsOk('info', ['url' => 'https://www.youtube.com/watch?v=M7IpKCZ47pU']);
         $this->assertRequestIsOk(

tests/PlaylistArchiveStreamTest.php

@@ -6,7 +6,6 @@
 
 namespace Alltube\Test;
 
-use Alltube\Config;
 use Alltube\Exception\ConfigException;
 use Alltube\Stream\PlaylistArchiveStream;
 
@@ -24,10 +23,10 @@ class PlaylistArchiveStreamTest extends StreamTest
     {
         parent::setUp();
 
-        $config = Config::getInstance();
+        $video = $this->downloader->getVideo(
-        $downloader = $config->getDownloader();
+            'https://www.youtube.com/playlist?list=PL1j4Ff8cAqPu5iowaeUAY8lRgkfT4RybJ'
-        $video = $downloader->getVideo('https://www.youtube.com/playlist?list=PL1j4Ff8cAqPu5iowaeUAY8lRgkfT4RybJ');
+        );
 
-        $this->stream = new PlaylistArchiveStream($downloader, $video);
+        $this->stream = new PlaylistArchiveStream($this->downloader, $video);
     }
 }

tests/StreamTest.php

@@ -6,6 +6,9 @@
 
 namespace Alltube\Test;
 
+use Alltube\Config;
+use Alltube\Exception\ConfigException;
+use Alltube\Library\Downloader;
 use Psr\Http\Message\StreamInterface;
 use RuntimeException;
 
@@ -20,6 +23,25 @@ abstract class StreamTest extends BaseTest
      */
     protected $stream;
 
+    /**
+     * Downloader class instance.
+     * @var Downloader
+     */
+    protected $downloader;
+
+    /**
+     * Prepare tests.
+     * @throws ConfigException
+     */
+    protected function setUp(): void
+    {
+        parent::setUp();
+
+        // So ffmpeg does not spam the output with broken pipe errors.
+        $config = new Config(['ffmpegVerbosity' => 'fatal']);
+        $this->downloader = $config->getDownloader();
+    }
+
     /**
      * Clean variables used in tests.
      *

tests/VideoStubsTest.php

@@ -7,7 +7,6 @@
 namespace Alltube\Test;
 
 use Alltube\Config;
-use Alltube\Exception\ConfigException;
 use Alltube\Library\Downloader;
 use Alltube\Library\Exception\AlltubeLibraryException;
 use Alltube\Library\Exception\PopenStreamException;
@@ -39,7 +38,6 @@ class VideoStubsTest extends BaseTest
 
     /**
      * Initialize properties used by test.
-     * @throws ConfigException
      */
     protected function setUp(): void
     {
@@ -48,7 +46,7 @@ class VideoStubsTest extends BaseTest
         PHPMockery::mock('Alltube\Library', 'popen');
         PHPMockery::mock('Alltube\Library', 'fopen');
 
-        $config = Config::getInstance();
+        $config = new Config();
         $this->downloader = $config->getDownloader();
         $this->video = $this->downloader->getVideo('https://www.youtube.com/watch?v=XJC9_JkzugE');
     }

tests/VideoTest.php

@@ -48,7 +48,8 @@ class VideoTest extends BaseTest
     {
         parent::setUp();
 
-        $config = Config::getInstance();
+        // So ffmpeg does not spam the output with broken pipe errors.
+        $config = new Config(['ffmpegVerbosity' => 'fatal']);
         $this->downloader = $config->getDownloader();
         $this->format = 'best';
     }
@@ -352,8 +353,7 @@ class VideoTest extends BaseTest
     public function testGetAudioStreamFfmpegError(string $url, string $format)
     {
         $this->expectException(AvconvException::class);
-        Config::setOptions(['ffmpeg' => 'foobar']);
+        $config = new Config(['ffmpeg' => 'foobar']);
-        $config = Config::getInstance();
         $downloader = $config->getDownloader();
 
         $video = new Video($this->downloader, $url, $format, $this->format);
@@ -502,8 +502,7 @@ class VideoTest extends BaseTest
     public function testGetM3uStreamFfmpegError(string $url, string $format)
     {
         $this->expectException(AvconvException::class);
-        Config::setOptions(['ffmpeg' => 'foobar']);
+        $config = new Config(['ffmpeg' => 'foobar']);
-        $config = Config::getInstance();
         $downloader = $config->getDownloader();
 
         $video = new Video($downloader, $url, $format);

tests/YoutubeChunkStreamTest.php

@@ -6,7 +6,6 @@
 
 namespace Alltube\Test;
 
-use Alltube\Config;
 use Alltube\Exception\ConfigException;
 use Alltube\Library\Exception\AlltubeLibraryException;
 use Alltube\Stream\YoutubeChunkStream;
@@ -19,17 +18,15 @@ class YoutubeChunkStreamTest extends StreamTest
 {
     /**
      * Prepare tests.
-     * @throws ConfigException
      * @throws AlltubeLibraryException
+     * @throws ConfigException
      */
     protected function setUp(): void
     {
         parent::setUp();
 
-        $config = Config::getInstance();
+        $video = $this->downloader->getVideo('https://www.youtube.com/watch?v=dQw4w9WgXcQ');
-        $downloader = $config->getDownloader();
-        $video = $downloader->getVideo('https://www.youtube.com/watch?v=dQw4w9WgXcQ');
 
-        $this->stream = new YoutubeChunkStream($downloader->getHttpResponse($video));
+        $this->stream = new YoutubeChunkStream($this->downloader->getHttpResponse($video));
     }
 }

tests/YoutubeStreamTest.php

@@ -6,7 +6,6 @@
 
 namespace Alltube\Test;
 
-use Alltube\Config;
 use Alltube\Exception\ConfigException;
 use Alltube\Library\Exception\AlltubeLibraryException;
 use Alltube\Stream\YoutubeStream;
@@ -19,17 +18,16 @@ class YoutubeStreamTest extends StreamTest
 {
     /**
      * Prepare tests.
-     * @throws ConfigException|AlltubeLibraryException
+     * @throws AlltubeLibraryException
+     * @throws ConfigException
      */
     protected function setUp(): void
     {
         parent::setUp();
 
-        $config = Config::getInstance();
+        $video = $this->downloader->getVideo('https://www.youtube.com/watch?v=dQw4w9WgXcQ', '135');
-        $downloader = $config->getDownloader();
-        $video = $downloader->getVideo('https://www.youtube.com/watch?v=dQw4w9WgXcQ', '135');
 
-        $this->stream = new YoutubeStream($downloader, $video);
+        $this->stream = new YoutubeStream($this->downloader, $video);
     }
 
     /**