From 01a9e53394f1c12e7d2d1cb2e5497a3fb98f4c25 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Sat, 21 Jul 2018 15:50:58 +0100 Subject: [PATCH] Config: Clean up error handling, throw exceptions instead of returning false (#2314) This also has the happy side effect of removing a cyclic dependency between Config and Server. There's only the dependency on MainLogger left to get rid of now. --- src/pocketmine/utils/Config.php | 163 +++++++++++++------------------- 1 file changed, 65 insertions(+), 98 deletions(-) diff --git a/src/pocketmine/utils/Config.php b/src/pocketmine/utils/Config.php index de8b65eb9..98e9aa2a4 100644 --- a/src/pocketmine/utils/Config.php +++ b/src/pocketmine/utils/Config.php @@ -23,9 +23,6 @@ declare(strict_types=1); namespace pocketmine\utils; -use pocketmine\Server; - - /** * Config Class for simple config manipulation of multiple formats. */ @@ -76,14 +73,12 @@ class Config{ ]; /** - * @param string $file Path of the file to be loaded - * @param int $type Config type to load, -1 by default (detect) - * @param array $default Array with the default values that will be written to the file if it did not exist - * @param null &$correct Sets correct to true if everything has been loaded correctly + * @param string $file Path of the file to be loaded + * @param int $type Config type to load, -1 by default (detect) + * @param array $default Array with the default values that will be written to the file if it did not exist */ - public function __construct(string $file, int $type = Config::DETECT, array $default = [], &$correct = null){ + public function __construct(string $file, int $type = Config::DETECT, array $default = []){ $this->load($file, $type, $default); - $correct = $this->correct; } /** @@ -92,7 +87,6 @@ class Config{ public function reload(){ $this->config = []; $this->nestedCache = []; - $this->correct = false; $this->load($this->file, $this->type); } @@ -114,14 +108,14 @@ class Config{ } /** - * @param $file - * @param int $type - * @param array $default + * @param string $file + * @param int $type + * @param array $default * - * @return bool + * @throws \InvalidArgumentException if config type could not be auto-detected + * @throws \InvalidStateException if config type is invalid */ - public function load(string $file, int $type = Config::DETECT, array $default = []) : bool{ - $this->correct = true; + public function load(string $file, int $type = Config::DETECT, array $default = []) : void{ $this->file = $file; $this->type = $type; @@ -131,7 +125,7 @@ class Config{ if(isset(Config::$formats[$extension])){ $this->type = Config::$formats[$extension]; }else{ - $this->correct = false; + throw new \InvalidArgumentException("Cannot detect config type of " . $this->file); } } @@ -139,93 +133,66 @@ class Config{ $this->config = $default; $this->save(); }else{ - if($this->correct){ - $content = file_get_contents($this->file); - switch($this->type){ - case Config::PROPERTIES: - $this->parseProperties($content); - break; - case Config::JSON: - $this->config = json_decode($content, true); - break; - case Config::YAML: - $content = self::fixYAMLIndexes($content); - $this->config = yaml_parse($content); - break; - case Config::SERIALIZED: - $this->config = unserialize($content); - break; - case Config::ENUM: - $this->parseList($content); - break; - default: - $this->correct = false; - - return false; - } - if(!is_array($this->config)){ - $this->config = $default; - } - if($this->fillDefaults($default, $this->config) > 0){ - $this->save(); - } - }else{ - return false; + $content = file_get_contents($this->file); + switch($this->type){ + case Config::PROPERTIES: + $this->parseProperties($content); + break; + case Config::JSON: + $this->config = json_decode($content, true); + break; + case Config::YAML: + $content = self::fixYAMLIndexes($content); + $this->config = yaml_parse($content); + break; + case Config::SERIALIZED: + $this->config = unserialize($content); + break; + case Config::ENUM: + $this->parseList($content); + break; + default: + throw new \InvalidStateException("Config type is unknown"); + } + if(!is_array($this->config)){ + $this->config = $default; + } + if($this->fillDefaults($default, $this->config) > 0){ + $this->save(); } } - - return true; } /** - * @return bool + * Flushes the config to disk in the appropriate format. + * + * @throws \InvalidStateException if config type is not valid */ - public function check() : bool{ - return $this->correct; - } - - /** - * @return bool - */ - public function save() : bool{ - if($this->correct){ - try{ - $content = null; - switch($this->type){ - case Config::PROPERTIES: - $content = $this->writeProperties(); - break; - case Config::JSON: - $content = json_encode($this->config, $this->jsonOptions); - break; - case Config::YAML: - $content = yaml_emit($this->config, YAML_UTF8_ENCODING); - break; - case Config::SERIALIZED: - $content = serialize($this->config); - break; - case Config::ENUM: - $content = implode("\r\n", array_keys($this->config)); - break; - default: - throw new \InvalidStateException("Config type is unknown, has not been set or not detected"); - } - - file_put_contents($this->file, $content); - }catch(\Throwable $e){ - $logger = Server::getInstance()->getLogger(); - $logger->critical("Could not save Config " . $this->file . ": " . $e->getMessage()); - if(\pocketmine\DEBUG > 1){ - $logger->logException($e); - } - } - - $this->changed = false; - - return true; - }else{ - return false; + public function save() : void{ + $content = null; + switch($this->type){ + case Config::PROPERTIES: + $content = $this->writeProperties(); + break; + case Config::JSON: + $content = json_encode($this->config, $this->jsonOptions); + break; + case Config::YAML: + $content = yaml_emit($this->config, YAML_UTF8_ENCODING); + break; + case Config::SERIALIZED: + $content = serialize($this->config); + break; + case Config::ENUM: + $content = implode("\r\n", array_keys($this->config)); + break; + default: + throw new \InvalidStateException("Config type is unknown, has not been set or not detected"); } + + file_put_contents($this->file, $content); + + $this->changed = false; } /** @@ -415,7 +382,7 @@ class Config{ * @return bool|mixed */ public function get($k, $default = false){ - return ($this->correct and isset($this->config[$k])) ? $this->config[$k] : $default; + return $this->config[$k] ?? $default; } /**