Remove ConfigurationSetter (#5251)

This class has not been maintained for a while. Only a subset of our configuration properties are there, and some code is not relevant anymore. Furthermore, it is relying exclusively on dynamically invoked functions, making it challenging to maintain, in particular to find out what is used and what is not, what is handled and what is not.
It is not well suited for changes in data formats, which have been handled in the Context class instead.
It is also not able to handle configuration properties that are missing.
It is the class with most errors for PHPStan level 6 (179 errors). It is also making intense use of is_callable and call_user_func_array, which are performance killers.
Should the need arrise again to perform validation of our internal configuration files, I suggest an implementation with the opposite approach, namely driven by our code instead of driven by the data.
In summary, at the moment, this class is costly, while not offering many guarantees.

5 files changed, 25 insertions, 456 deletions

diff --git a/app/Models/ConfigurationSetter.php b/app/Models/ConfigurationSetter.php
deleted file mode 100644
index 2f33ab925..000000000
--- a/app/Models/ConfigurationSetter.php
+++ /dev/null
@@ -1,429 +0,0 @@
-<?php
-
-class FreshRSS_ConfigurationSetter {
-	/**
-	 * Return if the given key is supported by this setter.
-	 * @param string $key the key to test.
-	 * @return boolean true if the key is supported, false otherwise.
-	 */
-	public function support($key) {
-		$name_setter = '_' . $key;
-		return is_callable(array($this, $name_setter));
-	}
-
-	/**
-	 * Set the given key in data with the current value.
-	 * @param array $data an array containing the list of all configuration data.
-	 * @param string $key the key to update.
-	 * @param mixed $value the value to set.
-	 */
-	public function handle(&$data, $key, $value) {
-		$name_setter = '_' . $key;
-		call_user_func_array(array($this, $name_setter), array(&$data, $value));
-	}
-
-	/**
-	 * A helper to set boolean values.
-	 *
-	 * @param mixed $value the tested value.
-	 * @return boolean true if value is true and different from no, false else.
-	 */
-	private function handleBool($value) {
-		return ((bool)$value) && $value !== 'no';
-	}
-
-	/**
-	 * The (long) list of setters for user configuration.
-	 */
-	private function _apiPasswordHash(&$data, $value) {
-		$data['apiPasswordHash'] = ctype_graph($value) && (strlen($value) >= 60) ? $value : '';
-	}
-
-	private function _content_width(&$data, $value) {
-		$value = strtolower($value);
-		if (!in_array($value, array('thin', 'medium', 'large', 'no_limit'))) {
-			$value = 'thin';
-		}
-
-		$data['content_width'] = $value;
-	}
-
-	private function _default_state(&$data, $value) {
-		$data['default_state'] = (int)$value;
-	}
-
-	private function _default_view(&$data, $value) {
-		switch ($value) {
-		case 'all':
-			$data['default_view'] = $value;
-			$data['default_state'] = (FreshRSS_Entry::STATE_READ + FreshRSS_Entry::STATE_NOT_READ);
-			break;
-		case 'adaptive':
-		case 'unread':
-		default:
-			$data['default_view'] = $value;
-			$data['default_state'] = FreshRSS_Entry::STATE_NOT_READ;
-		}
-	}
-
-	// It works for system config too!
-	private function _extensions_enabled(&$data, $value) {
-		if (!is_array($value)) {
-			$value = array($value);
-		}
-		$data['extensions_enabled'] = $value;
-	}
-
-	private function _html5_notif_timeout(&$data, $value) {
-		$value = intval($value);
-		$data['html5_notif_timeout'] = $value >= 0 ? $value : 0;
-	}
-
-	// It works for system config too!
-	private function _language(&$data, $value) {
-		$value = strtolower($value);
-		$languages = Minz_Translate::availableLanguages();
-		if (!in_array($value, $languages)) {
-			$value = 'en';
-		}
-		$data['language'] = $value;
-	}
-
-	private function _passwordHash(&$data, $value) {
-		$data['passwordHash'] = ctype_graph($value) && (strlen($value) >= 60) ? $value : '';
-	}
-
-	private function _posts_per_page(&$data, $value) {
-		$value = intval($value);
-		$data['posts_per_page'] = $value > 0 ? $value : 10;
-	}
-
-	private function _queries(&$data, $values) {
-		$data['queries'] = array();
-		foreach ($values as $value) {
-			if ($value instanceof FreshRSS_UserQuery) {
-				$data['queries'][] = $value->toArray();
-			} elseif (is_array($value)) {
-				$data['queries'][] = $value;
-			}
-		}
-	}
-
-	private function _sharing(&$data, $values) {
-		$data['sharing'] = array();
-		foreach ($values as $value) {
-			if (!is_array($value)) {
-				continue;
-			}
-
-			// Verify URL and add default value when needed
-			if (isset($value['url'])) {
-				$is_url = checkUrl($value['url']);
-				if (!$is_url) {
-					continue;
-				}
-			} else {
-				$value['url'] = null;
-			}
-
-			$data['sharing'][] = $value;
-		}
-	}
-
-	private function _shortcuts(&$data, $values) {
-		if (!is_array($values)) {
-			return;
-		}
-
-		$data['shortcuts'] = $values;
-	}
-
-	private function _sort_order(&$data, $value) {
-		$data['sort_order'] = $value === 'ASC' ? 'ASC' : 'DESC';
-	}
-
-	private function _ttl_default(&$data, $value) {
-		$value = intval($value);
-		$data['ttl_default'] = $value > FreshRSS_Feed::TTL_DEFAULT ? $value : 3600;
-	}
-
-	private function _view_mode(&$data, $value) {
-		$value = strtolower($value);
-		if (!in_array($value, array('global', 'normal', 'reader'))) {
-			$value = 'normal';
-		}
-		$data['view_mode'] = $value;
-	}
-
-	/**
-	 * A list of boolean setters.
-	 */
-	private function _anon_access(&$data, $value) {
-		$data['anon_access'] = $this->handleBool($value);
-	}
-
-	private function _auto_load_more(&$data, $value) {
-		$data['auto_load_more'] = $this->handleBool($value);
-	}
-
-	private function _auto_remove_article(&$data, $value) {
-		$data['auto_remove_article'] = $this->handleBool($value);
-	}
-
-	private function _mark_updated_article_unread(&$data, $value) {
-		$data['mark_updated_article_unread'] = $this->handleBool($value);
-	}
-
-	private function _show_nav_buttons(&$data, $value) {
-		$data['show_nav_buttons'] = $this->handleBool($value);
-	}
-
-	private function _show_fav_unread(&$data, $value) {
-		$data['show_fav_unread'] = $this->handleBool($value);
-	}
-
-	private function _display_categories(&$data, $value) {
-		if (!in_array($value, [ 'active', 'remember', 'all', 'none' ], true)) {
-			$value = $value === true ? 'all' : 'active';
-		}
-		$data['display_categories'] = $value;
-	}
-
-	private function show_tags(&$data, $value) {
-		$data['show_tags'] = $value;
-	}
-
-	private function show_tags_max(&$data, $value) {
-		$value = intval($value);
-		$data['show_tags_max'] = $value >= 0 ? $value : 0;
-	}
-
-	private function _show_author_date(&$data, $value) {
-		$data['show_author_date'] = $value;
-	}
-
-	private function _show_feed_name(&$data, $value) {
-		$data['show_feed_name'] = $value;
-	}
-
-	private function _display_posts(&$data, $value) {
-		$data['display_posts'] = $this->handleBool($value);
-	}
-
-	private function _hide_read_feeds(&$data, $value) {
-		$data['hide_read_feeds'] = $this->handleBool($value);
-	}
-
-	private function _sides_close_article(&$data, $value) {
-		$data['sides_close_article'] = $this->handleBool($value);
-	}
-
-	private function _lazyload(&$data, $value) {
-		$data['lazyload'] = $this->handleBool($value);
-	}
-
-	private function _onread_jump_next(&$data, $value) {
-		$data['onread_jump_next'] = $this->handleBool($value);
-	}
-
-	private function _reading_confirm(&$data, $value) {
-		$data['reading_confirm'] = $this->handleBool($value);
-	}
-
-	private function _sticky_post(&$data, $value) {
-		$data['sticky_post'] = $this->handleBool($value);
-	}
-
-	private function _darkMode(&$data, $value) {
-		if (!in_array($value, [ 'no', 'auto'], true)) {
-			$value = 'no';
-		}
-		$data['darkMode'] = $value;
-	}
-
-	private function _bottomline_date(&$data, $value) {
-		$data['bottomline_date'] = $this->handleBool($value);
-	}
-	private function _bottomline_favorite(&$data, $value) {
-		$data['bottomline_favorite'] = $this->handleBool($value);
-	}
-	private function _bottomline_link(&$data, $value) {
-		$data['bottomline_link'] = $this->handleBool($value);
-	}
-	private function _bottomline_read(&$data, $value) {
-		$data['bottomline_read'] = $this->handleBool($value);
-	}
-	private function _bottomline_sharing(&$data, $value) {
-		$data['bottomline_sharing'] = $this->handleBool($value);
-	}
-	private function _bottomline_tags(&$data, $value) {
-		$data['bottomline_tags'] = $this->handleBool($value);
-	}
-
-	private function _topline_date(&$data, $value) {
-		$data['topline_date'] = $this->handleBool($value);
-	}
-	private function _topline_favorite(&$data, $value) {
-		$data['topline_favorite'] = $this->handleBool($value);
-	}
-	private function _topline_link(&$data, $value) {
-		$data['topline_link'] = $this->handleBool($value);
-	}
-	private function _topline_read(&$data, $value) {
-		$data['topline_read'] = $this->handleBool($value);
-	}
-	private function _topline_website(&$data, $value) {
-		$value = strtolower($value);
-		if (!in_array($value, array('none', 'icon', 'name', 'full'))) {
-			$value = 'full';
-		}
-		$data['topline_website'] = $value;
-	}
-	private function _topline_thumbnail(&$data, $value) {
-		$value = strtolower($value);
-		if (!in_array($value, array('none', 'portrait', 'square', 'landscape'))) {
-			$value = 'none';
-		}
-		$data['topline_thumbnail'] = $value;
-	}
-	private function _topline_summary(&$data, $value) {
-		$data['topline_summary'] = $this->handleBool($value);
-	}
-	private function _topline_display_authors(&$data, $value) {
-		$data['topline_display_authors'] = $this->handleBool($value);
-	}
-
-	/**
-	 * The (not so long) list of setters for system configuration.
-	 */
-	private function _allow_anonymous(&$data, $value) {
-		$data['allow_anonymous'] = $this->handleBool($value) && FreshRSS_Auth::accessNeedsAction();
-	}
-
-	private function _allow_anonymous_refresh(&$data, $value) {
-		$data['allow_anonymous_refresh'] = $this->handleBool($value) && $data['allow_anonymous'];
-	}
-
-	private function _api_enabled(&$data, $value) {
-		$data['api_enabled'] = $this->handleBool($value);
-	}
-
-	private function _auth_type(&$data, $value) {
-		$value = strtolower($value);
-		if (!in_array($value, array('form', 'http_auth', 'none'))) {
-			$value = 'none';
-		}
-		$data['auth_type'] = $value;
-		$this->_allow_anonymous($data, $data['allow_anonymous']);
-	}
-
-	private function _db(&$data, $value) {
-		if (!isset($value['type'])) {
-			return;
-		}
-
-		switch ($value['type']) {
-		case 'mysql':
-		case 'pgsql':
-			if (empty($value['host']) ||
-					empty($value['user']) ||
-					empty($value['base']) ||
-					!isset($value['password'])) {
-				return;
-			}
-
-			$data['db']['type'] = $value['type'];
-			$data['db']['host'] = $value['host'];
-			$data['db']['user'] = $value['user'];
-			$data['db']['base'] = $value['base'];
-			$data['db']['password'] = $value['password'];
-			$data['db']['prefix'] = isset($value['prefix']) ? $value['prefix'] : '';
-			break;
-		case 'sqlite':
-			$data['db']['type'] = $value['type'];
-			$data['db']['host'] = '';
-			$data['db']['user'] = '';
-			$data['db']['base'] = '';
-			$data['db']['password'] = '';
-			$data['db']['prefix'] = '';
-			break;
-		default:
-			return;
-		}
-	}
-
-	private function _default_user(&$data, $value) {
-		$user_list = listUsers();
-		if (in_array($value, $user_list)) {
-			$data['default_user'] = $value;
-		}
-	}
-
-	private function _environment(&$data, $value) {
-		$value = strtolower($value);
-		if (!in_array($value, array('silent', 'development', 'production'))) {
-			$value = 'production';
-		}
-		$data['environment'] = $value;
-	}
-
-	private function _limits(&$data, $values) {
-		$max_small_int = 16384;
-		$limits_keys = array(
-			'cookie_duration' => array(
-				'min' => 0,
-			),
-			'cache_duration' => array(
-				'min' => 0,
-			),
-			'timeout' => array(
-				'min' => 0,
-			),
-			'max_inactivity' => array(
-				'min' => 0,
-			),
-			'max_feeds' => array(
-				'min' => 0,
-				'max' => $max_small_int,
-			),
-			'max_categories' => array(
-				'min' => 0,
-				'max' => $max_small_int,
-			),
-			'max_registrations' => array(
-				'min' => 0,
-			),
-		);
-
-		foreach ($values as $key => $value) {
-			if (!isset($limits_keys[$key])) {
-				continue;
-			}
-
-			$value = intval($value);
-			$limits = $limits_keys[$key];
-			// @phpstan-ignore-next-line
-			if ((!isset($limits['min']) || $value >= $limits['min']) &&
-				(!isset($limits['max']) || $value <= $limits['max'])
-			) {
-				$data['limits'][$key] = $value;
-			}
-		}
-	}
-
-	private function _unsafe_autologin_enabled(&$data, $value) {
-		$data['unsafe_autologin_enabled'] = $this->handleBool($value);
-	}
-
-	private function _auto_update_url(&$data, $value) {
-		if (!$value) {
-			return;
-		}
-
-		$data['auto_update_url'] = $value;
-	}
-
-	private function _force_email_validation(&$data, $value) {
-		$data['force_email_validation'] = $this->handleBool($value);
-	}
-}
diff --git a/app/Models/Context.php b/app/Models/Context.php
index 7c7af2791..b2631291a 100644
--- a/app/Models/Context.php
+++ b/app/Models/Context.php
@@ -104,9 +104,6 @@ final class FreshRSS_Context {
 		if ($reload || FreshRSS_Context::$system_conf == null) {
 			//TODO: Keep in session what we need instead of always reloading from disk
 			FreshRSS_Context::$system_conf = FreshRSS_SystemConfiguration::init(DATA_PATH . '/config.php', FRESHRSS_PATH . '/config.default.php');
-			// Register the configuration setter for the system configuration
-			$configurationSetter = new FreshRSS_ConfigurationSetter();
-			FreshRSS_Context::$system_conf->_configurationSetter($configurationSetter);
 		}
 		return FreshRSS_Context::$system_conf;
 	}
@@ -132,8 +129,7 @@ final class FreshRSS_Context {
 				//TODO: Keep in session what we need instead of always reloading from disk
 				FreshRSS_Context::$user_conf = FreshRSS_UserConfiguration::init(
 					USERS_PATH . '/' . $username . '/config.php',
-					FRESHRSS_PATH . '/config-user.default.php',
-					FreshRSS_Context::$system_conf->configurationSetter());
+					FRESHRSS_PATH . '/config-user.default.php');
 
 				Minz_User::change($username);
 			} catch (Exception $ex) {
diff --git a/app/Models/UserConfiguration.php b/app/Models/UserConfiguration.php
index 9a77c4761..df19d95e7 100644
--- a/app/Models/UserConfiguration.php
+++ b/app/Models/UserConfiguration.php
@@ -71,9 +71,8 @@
  */
 final class FreshRSS_UserConfiguration extends Minz_Configuration {
 
-	public static function init(string $config_filename, ?string $default_filename = null,
-		?FreshRSS_ConfigurationSetter $configuration_setter = null): FreshRSS_UserConfiguration {
-		parent::register('user', $config_filename, $default_filename, $configuration_setter);
+	public static function init(string $config_filename, ?string $default_filename = null): FreshRSS_UserConfiguration {
+		parent::register('user', $config_filename, $default_filename);
 		return parent::get('user');
 	}
 }
diff --git a/lib/Minz/Configuration.php b/lib/Minz/Configuration.php
index f286138e2..4259c4052 100644
--- a/lib/Minz/Configuration.php
+++ b/lib/Minz/Configuration.php
@@ -14,6 +14,7 @@
 class Minz_Configuration {
 	/**
 	 * The list of configurations.
+	 * @var array<string,static>
 	 */
 	private static $config_list = array();
 
@@ -25,7 +26,7 @@ class Minz_Configuration {
 	 * @param string $default_filename a filename containing default values for the configuration
 	 * @param object $configuration_setter an optional helper to set values in configuration
 	 */
-	public static function register($namespace, $config_filename, $default_filename = null, $configuration_setter = null) {
+	public static function register(string $namespace, string $config_filename, string $default_filename = null, object $configuration_setter = null): void {
 		self::$config_list[$namespace] = new static(
 			$namespace, $config_filename, $default_filename, $configuration_setter
 		);
@@ -35,10 +36,10 @@ class Minz_Configuration {
 	 * Parse a file and return its data.
 	 *
 	 * @param string $filename the name of the file to parse.
-	 * @return array of values
+	 * @return array<string,mixed> of values
 	 * @throws Minz_FileNotExistException if the file does not exist or is invalid.
 	 */
-	public static function load($filename) {
+	public static function load(string $filename): array {
 		$data = @include($filename);
 		if (is_array($data)) {
 			return $data;
@@ -54,7 +55,7 @@ class Minz_Configuration {
 	 * @return static object
 	 * @throws Minz_ConfigurationNamespaceException if the namespace does not exist.
 	 */
-	public static function get($namespace) {
+	public static function get(string $namespace) {
 		if (!isset(self::$config_list[$namespace])) {
 			throw new Minz_ConfigurationNamespaceException(
 				$namespace . ' namespace does not exist'
@@ -73,21 +74,25 @@ class Minz_Configuration {
 
 	/**
 	 * The filename for the current configuration.
+	 * @var string
 	 */
 	private $config_filename = '';
 
 	/**
 	 * The filename for the current default values, null by default.
+	 * @var string|null
 	 */
 	private $default_filename = null;
 
 	/**
 	 * The configuration values, an empty array by default.
+	 * @var array<string,mixed>
 	 */
 	private $data = array();
 
 	/**
 	 * An object which help to set good values in configuration.
+	 * @var object|null
 	 */
 	private $configuration_setter = null;
 
@@ -99,7 +104,7 @@ class Minz_Configuration {
 	 * @param string $default_filename the file containing default values, null by default.
 	 * @param object $configuration_setter an optional helper to set values in configuration
 	 */
-	private final function __construct($namespace, $config_filename, $default_filename = null, $configuration_setter = null) {
+	private final function __construct(string $namespace, string $config_filename, string $default_filename = null, object $configuration_setter = null) {
 		$this->namespace = $namespace;
 		$this->config_filename = $config_filename;
 		$this->default_filename = $default_filename;
@@ -122,25 +127,23 @@ class Minz_Configuration {
 
 	/**
 	 * Set a configuration setter for the current configuration.
-	 * @param object $configuration_setter the setter to call when modifying data. It
+	 * @param object|null $configuration_setter the setter to call when modifying data. It
 	 *        must implement an handle($key, $value) method.
 	 */
-	public function _configurationSetter($configuration_setter) {
+	public function _configurationSetter(?object $configuration_setter): void {
 		if (is_callable(array($configuration_setter, 'handle'))) {
 			$this->configuration_setter = $configuration_setter;
 		}
 	}
 
-	public function configurationSetter() {
+	public function configurationSetter(): object {
 		return $this->configuration_setter;
 	}
 
 	/**
 	 * Check if a parameter is defined in the configuration
-	 *
-	 * @return bool
 	 */
-	public function hasParam(string $key) {
+	public function hasParam(string $key): bool {
 		return isset($this->data[$key]);
 	}
 
@@ -149,10 +152,10 @@ class Minz_Configuration {
 	 *
 	 * @param string $key the name of the param.
 	 * @param mixed $default default value to return if key does not exist.
-	 * @return mixed value corresponding to the key.
+	 * @return array|mixed value corresponding to the key.
 	 * @throws Minz_ConfigurationParamException if the param does not exist
 	 */
-	public function param($key, $default = null) {
+	public function param(string $key, $default = null) {
 		if (isset($this->data[$key])) {
 			return $this->data[$key];
 		} elseif (!is_null($default)) {
@@ -165,8 +168,9 @@ class Minz_Configuration {
 
 	/**
 	 * A wrapper for param().
+	 * @return array|mixed
 	 */
-	public function __get($key) {
+	public function __get(string $key) {
 		return $this->param($key);
 	}
 
@@ -176,7 +180,7 @@ class Minz_Configuration {
 	 * @param string $key the param name to set.
 	 * @param mixed $value the value to set. If null, the key is removed from the configuration.
 	 */
-	public function _param($key, $value = null) {
+	public function _param(string $key, $value = null): void {
 		if (!is_null($this->configuration_setter) && $this->configuration_setter->support($key)) {
 			$this->configuration_setter->handle($this->data, $key, $value);
 		} elseif (isset($this->data[$key]) && is_null($value)) {
@@ -188,15 +192,16 @@ class Minz_Configuration {
 
 	/**
 	 * A wrapper for _param().
+	 * @param mixed $value
 	 */
-	public function __set($key, $value) {
+	public function __set(string $key, $value): void {
 		$this->_param($key, $value);
 	}
 
 	/**
 	 * Save the current configuration in the configuration file.
 	 */
-	public function save() {
+	public function save(): bool {
 		$back_filename = $this->config_filename . '.bak.php';
 		@rename($this->config_filename, $back_filename);
 
diff --git a/tests/phpstan-next.txt b/tests/phpstan-next.txt
index 262ca6562..6dc506469 100644
--- a/tests/phpstan-next.txt
+++ b/tests/phpstan-next.txt
@@ -9,7 +9,6 @@
 ./app/install.php
 ./app/Models/Category.php
 ./app/Models/CategoryDAO.php
-./app/Models/ConfigurationSetter.php
 ./app/Models/Entry.php
 ./app/Models/Feed.php
 ./app/Models/FeedDAO.php
@@ -24,7 +23,6 @@
 ./cli/i18n/I18nFile.php
 ./cli/i18n/I18nValue.php
 ./lib/http-conditional.php
-./lib/Minz/Configuration.php
 ./lib/Minz/Dispatcher.php
 ./lib/Minz/Log.php
 ./lib/Minz/Migrator.php