Do not use F3 for configuration management

Fatfree is badly designed and should not have lived past 2005. Most notably, it relies on a single huge singleton object, which makes testing difficult. What is worse, the unholy things it does to error handling makes PHPStan and PHPUnit crash. For these reasons and other pains, selfoss will be gradually removing its use. Configuration values are read throughout the codebase so I decided to tackle those first. Ideally, we would construct a configuration container using something like <https://gitlab.com/Khartir/typed-config> but that requires more recent PHP version than we support. Other configuration libraries either suffer from the same problem, are untyped like f3 was (domains of the options are known so we should not have to cast the string loaded from the ini file at the point of use), or are completely undocumented. Custom solution had to be implemented. This patch adds `helpers\Configuration` class, which will load data from `config.ini` and environment variables and store them in properties. The class is injected by Dice into whichever other class that needs access to configuration. In addition to reducing the percentage of code paths tainted by F3, it will also bring better developer experience since editors and other tools can make use of the property annotations. Since the default values of configuration option are now stored as default values of the class properties, `defaults.ini` is no longer used. It will still be included in the distribution zipball for convenience (generated from the code. In the future, we can move the option descriptions into the class and have the options documentation generated as well.
fossar · Apr 27, 2021 · 1f194f8 · 1f194f8
1 parent 6e74ffa
commit 1f194f8
Show file tree

Hide file tree

Showing 31 changed files with 601 additions and 330 deletions.
diff --git a/.gitignore b/.gitignore
@@ -10,7 +10,7 @@
 docs/public
 user.css
 user.js
-config.ini
+*.ini
 node_modules
 .env
 vendor/

diff --git a/NEWS.md b/NEWS.md
@@ -94,6 +94,7 @@
 - Placeholders are now used for images before they are loaded to avoid content jumping around ([#1204](https://github.com/fossar/selfoss/pull/1204))
 - Search button is now always on the screen, avoiding the need to scroll to top to be able to use it. ([#1231](https://github.com/fossar/selfoss/issues/1231))
 - Button for opening articles, tags, sources and filters in the sidebar, as well as the source and tag links in articles are now real links, allowing to open them in a new tab by middle-clicking them. ([#1216](https://github.com/fossar/selfoss/issues/1216), [#695](https://github.com/fossar/selfoss/issues/695))
+- Configuration is no longer managed by F3 framework. ([#1261](https://github.com/fossar/selfoss/pull/1261))
 
 
 ## 2.18 – 2018-03-05

diff --git a/defaults.ini b/defaults.ini
diff --git a/docs/content/_index.md b/docs/content/_index.md
@@ -35,7 +35,7 @@ For further questions or any problems, use our [support forum](forum). For a mor
 ## Configuring selfoss {#configuration}
 <div class="documentation-entry">
 
-All [configuration options](@/docs/administration/options.md) are optional. Any settings in `config.ini` will override the settings in `defaults.ini`. To customize settings follow these instructions:
+All [configuration options](@/docs/administration/options.md) are optional. Any settings in `config.ini` will override the settings in `src/helpers/Configuration.php`. For convenience, the archive includes `defaults.ini` file containing the default configuration exported in INI format. To customize settings follow these instructions:
 
 1. Copy `defaults.ini` to `config.ini`.
 2. Edit `config.ini` and delete any lines you do not wish to override.

diff --git a/src/common.php b/src/common.php
@@ -1,6 +1,7 @@
 <?php
 
 use Dice\Dice;
+use helpers\Configuration;
 use Monolog\Formatter\LineFormatter;
 use Monolog\Handler\ErrorLogHandler;
 use Monolog\Handler\NullHandler;
@@ -30,62 +31,42 @@
 // but we have not set an error handler yet because it needs a Logger instantiated by Dice.
 error_reporting(E_ALL & ~E_DEPRECATED);
 
-$f3->set('DEBUG', 0);
-$f3->set('version', '2.19-SNAPSHOT');
+const SELFOSS_VERSION = '2.19-SNAPSHOT';
 
 // independent of selfoss version
 // needs to be bumped each time public API is changed (follows semver)
 // keep in sync with docs/api-description.json
-$f3->set('apiversion', '4.0.0');
+const SELFOSS_API_VERSION = '4.0.0';
 
 $f3->set('AUTOLOAD', false);
 $f3->set('BASEDIR', BASEDIR);
 $f3->set('LOCALES', BASEDIR . '/assets/locale/');
 
-// internal but overridable values
-$f3->set('datadir', BASEDIR . '/data');
-$f3->set('cache', '%datadir%/cache');
-$f3->set('ftrss_custom_data_dir', '%datadir%/fulltextrss');
+$configuration = new Configuration(__DIR__ . '/../config.ini', $_ENV);
 
-// read defaults
-$f3->config('defaults.ini');
-
-// read config, if it exists
-if (file_exists('config.ini')) {
-    $f3->config('config.ini');
-}
-
-// overwrite config with ENV variables
-$env_prefix = $f3->get('env_prefix');
-foreach ($f3->get('ENV') as $key => $value) {
-    if (strncasecmp($key, $env_prefix, strlen($env_prefix)) === 0) {
-        $f3->set(strtolower(substr($key, strlen($env_prefix))), $value);
-    }
-}
-
-// interpolate variables in the config values
-$interpolatedKeys = [
-    'db_file',
-    'logger_destination',
-    'cache',
-    'ftrss_custom_data_dir',
-];
-$datadir = $f3->get('datadir');
-foreach ($interpolatedKeys as $key) {
-    $value = $f3->get($key);
-    $f3->set($key, str_replace('%datadir%', $datadir, $value));
-}
+$f3->set('DEBUG', $configuration->debug);
+$f3->set('cache', $configuration->cache);
+$f3->set('language', $configuration->language);
 
 $dice = new Dice();
 
 // DI rules
-// Choose database implementation based on config
 $substitutions = [
     'substitutions' => [
-        daos\DatabaseInterface::class => ['instance' => 'daos\\' . $f3->get('db_type') . '\\Database'],
-        daos\ItemsInterface::class => ['instance' => 'daos\\' . $f3->get('db_type') . '\\Items'],
-        daos\SourcesInterface::class => ['instance' => 'daos\\' . $f3->get('db_type') . '\\Sources'],
-        daos\TagsInterface::class => ['instance' => 'daos\\' . $f3->get('db_type') . '\\Tags'],
+        // Instantiate configuration container.
+        Configuration::class => [
+            'instance' => function() use ($configuration) {
+                return $configuration;
+            },
+            'shared' => true,
+        ],
+
+        // Choose database implementation based on config
+        daos\DatabaseInterface::class => ['instance' => 'daos\\' . $configuration->dbType . '\\Database'],
+        daos\ItemsInterface::class => ['instance' => 'daos\\' . $configuration->dbType . '\\Items'],
+        daos\SourcesInterface::class => ['instance' => 'daos\\' . $configuration->dbType . '\\Sources'],
+        daos\TagsInterface::class => ['instance' => 'daos\\' . $configuration->dbType . '\\Tags'],
+
         Dice::class => ['instance' => function() use ($dice) {
             return $dice;
         }],
@@ -111,8 +92,8 @@
 $dice->addRule(daos\TagsInterface::class, $shared);
 
 // Database connection
-if ($f3->get('db_type') === 'sqlite') {
-    $db_file = $f3->get('db_file');
+if ($configuration->dbType === 'sqlite') {
+    $db_file = $configuration->dbFile;
 
     // create empty database file if it does not exist
     if (!is_file($db_file)) {
@@ -123,38 +104,38 @@
     $dbParams = [
         $dsn
     ];
-} elseif ($f3->get('db_type') === 'mysql') {
-    $host = $f3->get('db_host');
-    $port = $f3->get('db_port');
-    $database = $f3->get('db_database');
+} elseif ($configuration->dbType === 'mysql') {
+    $host = $configuration->dbHost;
+    $port = $configuration->dbPort;
+    $database = $configuration->dbDatabase;
 
-    if ($port) {
+    if ($port !== null) {
         $dsn = "mysql:host=$host; port=$port; dbname=$database";
     } else {
         $dsn = "mysql:host=$host; dbname=$database";
     }
 
     $dbParams = [
         $dsn,
-        $f3->get('db_username'),
-        $f3->get('db_password'),
+        $configuration->dbUsername,
+        $configuration->dbPassword,
         [PDO::MYSQL_ATTR_INIT_COMMAND => 'SET NAMES utf8mb4;']
     ];
-} elseif ($f3->get('db_type') === 'pgsql') {
-    $host = $f3->get('db_host');
-    $port = $f3->get('db_port');
-    $database = $f3->get('db_database');
+} elseif ($configuration->dbType === 'pgsql') {
+    $host = $configuration->dbHost;
+    $port = $configuration->dbPort;
+    $database = $configuration->dbDatabase;
 
-    if ($port) {
+    if ($port !== null) {
         $dsn = "pgsql:host=$host; port=$port; dbname=$database";
     } else {
         $dsn = "pgsql:host=$host; dbname=$database";
     }
 
     $dbParams = [
         $dsn,
-        $f3->get('db_username'),
-        $f3->get('db_password')
+        $configuration->dbUsername,
+        $configuration->dbPassword
     ];
 }
 
@@ -163,7 +144,7 @@
 ]);
 
 // Define regexp function for SQLite
-if ($f3->get('db_type') === 'sqlite') {
+if ($configuration->dbType === 'sqlite') {
     $sqlParams = array_merge($sqlParams, [
         'call' => [
             [
@@ -194,7 +175,7 @@ function($pattern, $text) {
 $dice->addRule('$iconStorageBackend', [
     'instanceOf' => helpers\Storage\FileStorage::class,
     'constructParams' => [
-        \F3::get('datadir') . '/favicons'
+        $configuration->datadir . '/favicons'
     ],
 ]);
 
@@ -207,7 +188,7 @@ function($pattern, $text) {
 $dice->addRule('$thumbnailStorageBackend', [
     'instanceOf' => helpers\Storage\FileStorage::class,
     'constructParams' => [
-        \F3::get('datadir') . '/thumbnails'
+        $configuration->datadir . '/thumbnails'
     ],
 ]);
 
@@ -231,22 +212,22 @@ function($pattern, $text) {
 
 $dice->addRule(helpers\FeedReader::class, [
     'constructParams' => [
-        \F3::get('cache'),
+        $configuration->cache,
     ],
 ]);
 
 // init logger
 $log = $dice->create(Logger::class);
 
-if ($f3->get('logger_level') === 'NONE') {
+if ($configuration->loggerLevel === 'NONE') {
     $handler = new NullHandler();
 } else {
-    $logger_destination = $f3->get('logger_destination');
+    $logger_destination = $configuration->loggerDestination;
 
     if (strpos($logger_destination, 'file:') === 0) {
-        $handler = new StreamHandler(substr($logger_destination, 5), $f3->get('logger_level'));
+        $handler = new StreamHandler(substr($logger_destination, 5), $configuration->loggerLevel);
     } elseif ($logger_destination === 'error_log') {
-        $handler = new ErrorLogHandler(ErrorLogHandler::OPERATING_SYSTEM, $f3->get('logger_level'));
+        $handler = new ErrorLogHandler(ErrorLogHandler::OPERATING_SYSTEM, $configuration->loggerLevel);
     } else {
         echo 'The `logger_destination` option needs to be either `error_log` or a file path prefixed by `file:`.';
         exit;
@@ -264,7 +245,7 @@ function($pattern, $text) {
 
 // init error handling
 $f3->set('ONERROR',
-    function(Base $f3) use ($log, $handler) {
+    function(Base $f3) use ($configuration, $log, $handler) {
         $exception = $f3->get('EXCEPTION');
 
         try {
@@ -274,7 +255,7 @@ function(Base $f3) use ($log, $handler) {
                 $log->error($f3->get('ERROR.text'));
             }
 
-            if ($f3->get('DEBUG') != 0) {
+            if ($configuration->debug !== 0) {
                 echo $f3->get('lang_error') . ': ';
                 echo $f3->get('ERROR.text') . "\n";
                 echo $f3->get('ERROR.trace');
@@ -294,6 +275,6 @@ function(Base $f3) use ($log, $handler) {
     }
 );
 
-if ($f3->get('DEBUG') != 0) {
+if ($configuration->debug !== 0) {
     ini_set('display_errors', '0');
 }
diff --git a/src/controllers/About.php b/src/controllers/About.php
@@ -2,8 +2,8 @@
 
 namespace controllers;
 
-use Base;
 use helpers\Authentication;
+use helpers\Configuration;
 use helpers\View;
 
 /**
@@ -13,11 +13,15 @@ class About {
     /** @var Authentication authentication helper */
     private $authentication;
 
+    /** @var Configuration configuration */
+    private $configuration;
+
     /** @var View view helper */
     private $view;
 
-    public function __construct(Authentication $authentication, View $view) {
+    public function __construct(Authentication $authentication, Configuration $configuration, View $view) {
         $this->authentication = $authentication;
+        $this->configuration = $configuration;
         $this->view = $view;
     }
 
@@ -27,37 +31,36 @@ public function __construct(Authentication $authentication, View $view) {
      *
      * @return void
      */
-    public function about(Base $f3) {
-        $anonymizer = \helpers\Anonymizer::getAnonymizer();
-        $wallabag = !empty($f3->get('wallabag')) ? [
-            'url' => $f3->get('wallabag'), // string
-            'version' => $f3->get('wallabag_version'), // int
+    public function about() {
+        $wallabag = !empty($this->configuration->wallabag) ? [
+            'url' => $this->configuration->wallabag, // string
+            'version' => $this->configuration->wallabagVersion, // int
         ] : null;
 
         $configuration = [
-            'version' => $f3->get('version'),
-            'apiversion' => $f3->get('apiversion'),
+            'version' => SELFOSS_VERSION,
+            'apiversion' => SELFOSS_API_VERSION,
             'configuration' => [
-                'homepage' => $f3->get('homepage') ? $f3->get('homepage') : 'newest', // string
-                'anonymizer' => $anonymizer === '' ? null : $anonymizer, // ?string
-                'share' => (string) $f3->get('share'), // string
+                'homepage' => $this->configuration->homepage ? $this->configuration->homepage : 'newest', // string
+                'anonymizer' => $this->configuration->anonymizer, // ?string
+                'share' => $this->configuration->share, // string
                 'wallabag' => $wallabag, // ?array
-                'wordpress' => $f3->get('wordpress'), // ?string
-                'autoMarkAsRead' => $f3->get('auto_mark_as_read') == 1, // bool
-                'autoCollapse' => $f3->get('auto_collapse') == 1, // bool
-                'autoStreamMore' => $f3->get('auto_stream_more') == 1, // bool
-                'loadImagesOnMobile' => $f3->get('load_images_on_mobile') == 1, // bool
-                'itemsPerPage' => $f3->get('items_perpage'), // int
-                'unreadOrder' => $f3->get('unread_order'), // string
-                'autoHideReadOnMobile' => $f3->get('auto_hide_read_on_mobile') == 1, // bool
-                'scrollToArticleHeader' => $f3->get('scroll_to_article_header') == 1, // bool
-                'showThumbnails' => $f3->get('show_thumbnails') == 1, // bool
-                'htmlTitle' => trim($f3->get('html_title')), // string
-                'allowPublicUpdate' => $f3->get('allow_public_update_access') == 1, // bool
-                'publicMode' => $f3->get('public') == 1, // bool
+                'wordpress' => $this->configuration->wordpress, // ?string
+                'autoMarkAsRead' => $this->configuration->autoMarkAsRead, // bool
+                'autoCollapse' => $this->configuration->autoCollapse, // bool
+                'autoStreamMore' => $this->configuration->autoStreamMore, // bool
+                'loadImagesOnMobile' => $this->configuration->loadImagesOnMobile, // bool
+                'itemsPerPage' => $this->configuration->itemsPerpage, // int
+                'unreadOrder' => $this->configuration->unreadOrder, // string
+                'autoHideReadOnMobile' => $this->configuration->autoHideReadOnMobile, // bool
+                'scrollToArticleHeader' => $this->configuration->scrollToArticleHeader, // bool
+                'showThumbnails' => $this->configuration->showThumbnails, // bool
+                'htmlTitle' => trim($this->configuration->htmlTitle), // string
+                'allowPublicUpdate' => $this->configuration->allowPublicUpdateAccess, // bool
+                'publicMode' => $this->configuration->public, // bool
                 'authEnabled' => $this->authentication->enabled() === true, // bool
-                'readingSpeed' => $f3->get('reading_speed_wpm') > 0 ? (int) $f3->get('reading_speed_wpm') : null, // ?int
-                'language' => $f3->get('language') === 0 ? null : $f3->get('language'), // ?string
+                'readingSpeed' => $this->configuration->readingSpeedWpm > 0 ? $this->configuration->readingSpeedWpm : null, // ?int
+                'language' => $this->configuration->language === '0' ? null : $this->configuration->language, // ?string
                 'userCss' => file_exists(BASEDIR . '/user.css') ? filemtime(BASEDIR . '/user.css') : null, // ?int
                 'userJs' => file_exists(BASEDIR . '/user.js') ? filemtime(BASEDIR . '/user.js') : null, // ?int
             ],