From 23654ce566c18c86de30f3263752be49cb0541de Mon Sep 17 00:00:00 2001 From: Philipp Holzer Date: Sun, 24 Mar 2019 22:51:30 +0100 Subject: [PATCH] Added Update checks - Logging - Console - Admin overview --- mod/admin.php | 7 +- src/App.php | 17 +--- src/Core/Update.php | 87 ++++++++++++++++++- src/Factory/DependencyFactory.php | 2 +- src/Util/Config/ConfigFileLoader.php | 2 +- src/Util/Config/ConfigFileSaver.php | 57 ++++++++++-- src/Worker/DBUpdate.php | 6 +- tests/include/ApiTest.php | 2 +- tests/src/Database/DBATest.php | 2 +- tests/src/Database/DBStructureTest.php | 2 +- tests/src/Util/Config/ConfigFileSaverTest.php | 54 +++++++++++- update.php | 37 ++------ 12 files changed, 215 insertions(+), 60 deletions(-) diff --git a/mod/admin.php b/mod/admin.php index 851dd0b093..848ed2521f 100644 --- a/mod/admin.php +++ b/mod/admin.php @@ -923,6 +923,10 @@ function admin_page_summary(App $a) $showwarning = true; $warningtext[] = L10n::t('The database update failed. Please run "php bin/console.php dbstructure update" from the command line and have a look at the errors that might appear.'); } + if (Config::get('system', 'update') == Update::FAILED) { + $showwarning = true; + $warningtext[] = L10n::t('The last update failed. Please run "php bin/console.php dbstructure update" from the command line and have a look at the errors that might appear. (Some of the errors are possibly inside the logfile.)'); + } $last_worker_call = Config::get('system', 'last_worker_execution', false); if (!$last_worker_call) { @@ -1088,9 +1092,8 @@ function admin_page_site_post(App $a) // update config $configFileSaver = new \Friendica\Util\Config\ConfigFileSaver($a->getBasePath()); - $configFileSaver->addConfigValue('system', 'hostname', parse_url($new_url, PHP_URL_HOST)); + $configFileSaver->addConfigValue('config', 'hostname', parse_url($new_url, PHP_URL_HOST)); $configFileSaver->saveToConfigFile(); - Config::set('system', 'hostname', parse_url($new_url, PHP_URL_HOST)); Config::set('system', 'url', $new_url); $a->setBaseURL($new_url); diff --git a/src/App.php b/src/App.php index 3a96182b1e..1ed6390b43 100644 --- a/src/App.php +++ b/src/App.php @@ -75,11 +75,6 @@ class App */ private $mode; - /** - * @var string The App base path - */ - private $basePath; - /** * @var string The App URL path */ @@ -142,7 +137,7 @@ class App */ public function getBasePath() { - return $this->basePath; + return $this->config->get('system', 'basepath'); } /** @@ -216,7 +211,6 @@ class App /** * @brief App constructor. * - * @param string $basePath The basedir of the app * @param Configuration $config The Configuration * @param App\Mode $mode The mode of this Friendica app * @param LoggerInterface $logger The current app logger @@ -225,7 +219,7 @@ class App * * @throws Exception if the Basepath is not usable */ - public function __construct($basePath, Configuration $config, App\Mode $mode, LoggerInterface $logger, Profiler $profiler, $isBackend = true) + public function __construct(Configuration $config, App\Mode $mode, LoggerInterface $logger, Profiler $profiler, $isBackend = true) { BaseObject::setApp($this); @@ -233,13 +227,6 @@ class App $this->config = $config; $this->profiler = $profiler; $this->mode = $mode; - $cfgBasePath = $this->config->get('system', 'basepath'); - $this->basePath = !empty($cfgBasePath) ? $cfgBasePath : $basePath; - - if (!Core\System::isDirectoryUsable($this->getBasePath(), false)) { - throw new Exception('Basepath \'' . $this->getBasePath() . '\' isn\'t usable.'); - } - $this->basePath = rtrim($this->getBasePath(), DIRECTORY_SEPARATOR); $this->checkBackend($isBackend); $this->checkFriendicaApp(); diff --git a/src/Core/Update.php b/src/Core/Update.php index bb2513d388..7cb3212679 100644 --- a/src/Core/Update.php +++ b/src/Core/Update.php @@ -2,8 +2,12 @@ namespace Friendica\Core; +use Friendica\App; +use Friendica\Core\Config\Cache\IConfigCache; use Friendica\Database\DBA; use Friendica\Database\DBStructure; +use Friendica\Util\Config\ConfigFileLoader; +use Friendica\Util\Config\ConfigFileSaver; use Friendica\Util\Strings; class Update @@ -24,6 +28,11 @@ class Update return; } + // Don't check the status if the last update was failed + if (Config::get('system', 'update', Update::SUCCESS, true) == Update::FAILED) { + return; + } + $build = Config::get('system', 'build'); if (empty($build)) { @@ -101,7 +110,9 @@ class Update for ($x = $stored + 1; $x <= $current; $x++) { $r = self::runUpdateFunction($x, 'pre_update'); if (!$r) { - break; + Config::set('system', 'update', Update::FAILED); + Lock::release('dbupdate'); + return $r; } } @@ -115,6 +126,7 @@ class Update ); } Logger::error('Update ERROR.', ['from' => $stored, 'to' => $current, 'retval' => $retval]); + Config::set('system', 'update', Update::FAILED); Lock::release('dbupdate'); return $retval; } else { @@ -127,7 +139,9 @@ class Update for ($x = $stored + 1; $x <= $current; $x++) { $r = self::runUpdateFunction($x, 'update'); if (!$r) { - break; + Config::set('system', 'update', Update::FAILED); + Lock::release('dbupdate'); + return $r; } } @@ -136,6 +150,7 @@ class Update self::updateSuccessfull($stored, $current); } + Config::set('system', 'update', Update::SUCCESS); Lock::release('dbupdate'); } } @@ -208,6 +223,74 @@ class Update } } + /** + * Checks the config settings and saves given config values into the config file + * + * @param string $basePath The basepath of Friendica + * @param App\Mode $mode The Application mode + * + * @return bool True, if something has been saved + */ + public static function saveConfigToFile($basePath, App\Mode $mode) + { + $configFileLoader = new ConfigFileLoader($basePath, $mode); + $configCache = new Config\Cache\ConfigCache(); + $configFileLoader->setupCache($configCache); + $configFileSaver = new ConfigFileSaver($basePath); + + $updated = false; + + if (self::updateConfigEntry($configCache, $configFileSaver,'config', 'hostname')) { + $updated = true; + }; + if (self::updateConfigEntry($configCache, $configFileSaver,'system', 'basepath')) { + $updated = true; + } + + if (!$configFileSaver->saveToConfigFile()) { + Logger::alert('Config entry update failed - maybe wrong permission?'); + return false; + } + + DBA::delete('config', ['cat' => 'config', 'k' => 'hostname']); + DBA::delete('config', ['cat' => 'system', 'k' => 'basepath']); + + return $updated; + } + + /** + * Adds a value to the ConfigFileSave in case it isn't already updated + * + * @param IConfigCache $configCache The cached config file + * @param ConfigFileSaver $configFileSaver The config file saver + * @param string $cat The config category + * @param string $key The config key + * + * @return boolean True, if a value was updated + * + * @throws \Exception if DBA or Logger doesn't work + */ + private static function updateConfigEntry(IConfigCache $configCache, ConfigFileSaver $configFileSaver, $cat, $key) + { + // check if the config file differs from the whole configuration (= The db contains other values) + $fileConfig = $configCache->get($cat, $key); + + $savedConfig = DBA::selectFirst('config', ['v'], ['cat' => $cat, 'k' => $key]); + + if (!DBA::isResult($savedConfig)) { + return false; + } + + if ($fileConfig !== $savedConfig['v']) { + Logger::info('Difference in config found', ['cat' => $cat, 'key' => $key, 'file' => $fileConfig, 'saved' => $savedConfig['v']]); + $configFileSaver->addConfigValue($cat, $key, $savedConfig['v']); + } else { + Logger::info('No Difference in config found', ['cat' => $cat, 'key' => $key, 'value' => $fileConfig, 'saved' => $savedConfig['v']]); + } + + return true; + } + /** * send the email and do what is needed to do on update fails * diff --git a/src/Factory/DependencyFactory.php b/src/Factory/DependencyFactory.php index 65bdf37140..63defd95f5 100644 --- a/src/Factory/DependencyFactory.php +++ b/src/Factory/DependencyFactory.php @@ -34,6 +34,6 @@ class DependencyFactory $logger = Factory\LoggerFactory::create($channel, $config, $profiler); Factory\LoggerFactory::createDev($channel, $config, $profiler); - return new App($basePath, $config, $mode, $logger, $profiler, $isBackend); + return new App($config, $mode, $logger, $profiler, $isBackend); } } diff --git a/src/Util/Config/ConfigFileLoader.php b/src/Util/Config/ConfigFileLoader.php index 67a44a0274..d677d4bfe0 100644 --- a/src/Util/Config/ConfigFileLoader.php +++ b/src/Util/Config/ConfigFileLoader.php @@ -33,7 +33,7 @@ class ConfigFileLoader extends ConfigFileManager * First loads the default value for all the configuration keys, then the legacy configuration files, then the * expected local.config.php * - * @param IConfigCache The config cache to load to + * @param IConfigCache $config The config cache to load to * * @throws \Exception */ diff --git a/src/Util/Config/ConfigFileSaver.php b/src/Util/Config/ConfigFileSaver.php index 2c127188f1..e32e11a4d5 100644 --- a/src/Util/Config/ConfigFileSaver.php +++ b/src/Util/Config/ConfigFileSaver.php @@ -34,6 +34,17 @@ class ConfigFileSaver extends ConfigFileManager */ public function addConfigValue($cat, $key, $value) { + $settingsCount = count(array_keys($this->settings)); + + for ($i = 0; $i < $settingsCount; $i++) { + // if already set, overwrite the value + if ($this->settings[$i]['cat'] === $cat && + $this->settings[$i]['key'] === $key) { + $this->settings[$i] = ['cat' => $cat, 'key' => $key, 'value' => $value]; + return; + } + } + $this->settings[] = ['cat' => $cat, 'key' => $key, 'value' => $value]; } @@ -51,10 +62,15 @@ class ConfigFileSaver extends ConfigFileManager * * @param string $name The name of the configuration file (default is empty, which means the default name each type) * - * @return bool true, if at least one configuration file was successfully updated + * @return bool true, if at least one configuration file was successfully updated or nothing to do */ public function saveToConfigFile($name = '') { + // If no settings et, return true + if (count(array_keys($this->settings)) === 0) { + return true; + } + $saved = false; // Check for the *.config.php file inside the /config/ path @@ -108,13 +124,22 @@ class ConfigFileSaver extends ConfigFileManager return [null, null]; } - $reading = fopen($fullName, 'r'); + try { + $reading = fopen($fullName, 'r'); + } catch (\Exception $exception) { + return [null, null]; + } if (!$reading) { return [null, null]; } - $writing = fopen($fullName . '.tmp', 'w'); + try { + $writing = fopen($fullName . '.tmp', 'w'); + } catch (\Exception $exception) { + fclose($reading); + return [null, null]; + } if (!$writing) { fclose($reading); @@ -138,11 +163,25 @@ class ConfigFileSaver extends ConfigFileManager fclose($reading); fclose($writing); - if (!rename($fullName, $fullName . '.old')) { + try { + $renamed = rename($fullName, $fullName . '.old'); + } catch (\Exception $exception) { return false; } - if (!rename($fullName . '.tmp', $fullName)) { + if (!$renamed) { + return false; + } + + try { + $renamed = rename($fullName . '.tmp', $fullName); + } catch (\Exception $exception) { + // revert the move of the current config file to have at least the old config + rename($fullName . '.old', $fullName); + return false; + } + + if (!$renamed) { // revert the move of the current config file to have at least the old config rename($fullName . '.old', $fullName); return false; @@ -234,11 +273,16 @@ class ConfigFileSaver extends ConfigFileManager // check for each added setting if we have to replace a config line for ($i = 0; $i < $settingsCount; $i++) { + // find the category of the current setting if (!$categoryFound[$i] && stristr($line, sprintf('[%s]', $this->settings[$i]['cat']))) { $categoryFound[$i] = true; + + // check the current value } elseif ($categoryFound[$i] && preg_match_all('/^' . $this->settings[$i]['key'] . '\s*=\s*(.*?)$/', $line, $matches, PREG_SET_ORDER)) { $line = $this->settings[$i]['key'] . ' = ' . $this->settings[$i]['value'] . PHP_EOL; $categoryFound[$i] = false; + + // If end of INI file, add the line before the INI end } elseif ($categoryFound[$i] && (preg_match_all('/^\[.*?\]$/', $line) || preg_match_all('/^INI;.*$/', $line))) { $categoryFound[$i] = false; $newLine = $this->settings[$i]['key'] . ' = ' . $this->settings[$i]['value'] . PHP_EOL; @@ -267,9 +311,12 @@ class ConfigFileSaver extends ConfigFileManager // check for each added setting if we have to replace a config line for ($i = 0; $i < $settingsCount; $i++) { + // check for a non plain config setting (use category too) if ($this->settings[$i]['cat'] !== 'config' && preg_match_all('/^\$a\-\>config\[\'' . $this->settings[$i]['cat'] . '\'\]\[\'' . $this->settings[$i]['key'] . '\'\]\s*=\s\'*(.*?)\';$/', $line, $matches, PREG_SET_ORDER)) { $line = '$a->config[\'' . $this->settings[$i]['cat'] . '\'][\'' . $this->settings[$i]['key'] . '\'] = \'' . $this->settings[$i]['value'] . '\';' . PHP_EOL; $found[$i] = true; + + // check for a plain config setting (don't use a category) } elseif ($this->settings[$i]['cat'] === 'config' && preg_match_all('/^\$a\-\>config\[\'' . $this->settings[$i]['key'] . '\'\]\s*=\s\'*(.*?)\';$/', $line, $matches, PREG_SET_ORDER)) { $line = '$a->config[\'' . $this->settings[$i]['key'] . '\'] = \'' . $this->settings[$i]['value'] . '\';' . PHP_EOL; $found[$i] = true; diff --git a/src/Worker/DBUpdate.php b/src/Worker/DBUpdate.php index 05bace1467..001df25a81 100644 --- a/src/Worker/DBUpdate.php +++ b/src/Worker/DBUpdate.php @@ -6,12 +6,16 @@ namespace Friendica\Worker; use Friendica\BaseObject; +use Friendica\Core\Config; use Friendica\Core\Update; class DBUpdate extends BaseObject { public static function execute() { - Update::run(self::getApp()->getBasePath()); + // Just in case the last update wasn't failed + if (Config::get('system', 'update', Update::SUCCESS, true) != Update::FAILED) { + Update::run(self::getApp()->getBasePath()); + } } } diff --git a/tests/include/ApiTest.php b/tests/include/ApiTest.php index 5c2e1657af..1eb7ce78d0 100644 --- a/tests/include/ApiTest.php +++ b/tests/include/ApiTest.php @@ -45,7 +45,7 @@ class ApiTest extends DatabaseTest $config = Factory\ConfigFactory::createConfig($configCache); Factory\ConfigFactory::createPConfig($configCache); $logger = Factory\LoggerFactory::create('test', $config, $profiler); - $this->app = new App($basePath, $config, $mode, $logger, $profiler, false); + $this->app = new App($config, $mode, $logger, $profiler, false); parent::setUp(); diff --git a/tests/src/Database/DBATest.php b/tests/src/Database/DBATest.php index bc8743da5b..c941377219 100644 --- a/tests/src/Database/DBATest.php +++ b/tests/src/Database/DBATest.php @@ -22,7 +22,7 @@ class DBATest extends DatabaseTest $config = Factory\ConfigFactory::createConfig($configCache); Factory\ConfigFactory::createPConfig($configCache); $logger = Factory\LoggerFactory::create('test', $config, $profiler); - $this->app = new App($basePath, $config, $mode, $logger, $profiler, false); + $this->app = new App($config, $mode, $logger, $profiler, false); parent::setUp(); diff --git a/tests/src/Database/DBStructureTest.php b/tests/src/Database/DBStructureTest.php index 0c1da172f5..152014c114 100644 --- a/tests/src/Database/DBStructureTest.php +++ b/tests/src/Database/DBStructureTest.php @@ -22,7 +22,7 @@ class DBStructureTest extends DatabaseTest $config = Factory\ConfigFactory::createConfig($configCache); Factory\ConfigFactory::createPConfig($configCache); $logger = Factory\LoggerFactory::create('test', $config, $profiler); - $this->app = new App($basePath, $config, $mode, $logger, $profiler, false); + $this->app = new App($config, $mode, $logger, $profiler, false); parent::setUp(); } diff --git a/tests/src/Util/Config/ConfigFileSaverTest.php b/tests/src/Util/Config/ConfigFileSaverTest.php index 5b472df49f..04adf6014a 100644 --- a/tests/src/Util/Config/ConfigFileSaverTest.php +++ b/tests/src/Util/Config/ConfigFileSaverTest.php @@ -89,14 +89,22 @@ class ConfigFileSaverTest extends MockedTest $configFileLoader->setupCache($configCache); $this->assertEquals('admin@test.it', $configCache->get('config', 'admin_email')); + $this->assertEquals('frio', $configCache->get('system', 'theme')); $this->assertNull($configCache->get('config', 'test_val')); $this->assertNull($configCache->get('system', 'test_val2')); - $configFileSaver->addConfigValue('system', 'theme', 'frio'); + // update values (system and config value) $configFileSaver->addConfigValue('config', 'admin_email', 'new@mail.it'); - $configFileSaver->addConfigValue('config', 'test_val', 'Testingwith@all.we can'); $configFileSaver->addConfigValue('system', 'theme', 'vier'); + + // insert values (system and config value) + $configFileSaver->addConfigValue('config', 'test_val', 'Testingwith@all.we can'); + $configFileSaver->addConfigValue('system', 'test_val2', 'TestIt First'); + + // overwrite value $configFileSaver->addConfigValue('system', 'test_val2', 'TestIt Now'); + + // save it $this->assertTrue($configFileSaver->saveToConfigFile()); $newConfigCache = new ConfigCache(); @@ -135,5 +143,47 @@ class ConfigFileSaverTest extends MockedTest vfsStream::newFile($fileName) ->at($root) ->setContent(file_get_contents($filePath . DIRECTORY_SEPARATOR . $fileName)); + + $configFileSaver = new ConfigFileSaver($this->root->url()); + + $configFileSaver->addConfigValue('system', 'test_val2', 'TestIt Now'); + + // wrong mod, so return false if nothing to write + $this->assertFalse($configFileSaver->saveToConfigFile()); + } + + /** + * Test the saveToConfigFile() method with nothing to do + * @dataProvider dataConfigFiles + */ + public function testNothingToDo($fileName, $filePath, $relativePath) + { + $this->delConfigFile('local.config.php'); + + if (empty($relativePath)) { + $root = $this->root; + $relativeFullName = $fileName; + } else { + $root = $this->root->getChild($relativePath); + $relativeFullName = $relativePath . DIRECTORY_SEPARATOR . $fileName; + } + + vfsStream::newFile($fileName) + ->at($root) + ->setContent(file_get_contents($filePath . DIRECTORY_SEPARATOR . $fileName)); + + $configFileSaver = new ConfigFileSaver($this->root->url()); + $configFileLoader = new ConfigFileLoader($this->root->url(), $this->mode); + $configCache = new ConfigCache(); + $configFileLoader->setupCache($configCache); + + // save nothing + $this->assertTrue($configFileSaver->saveToConfigFile()); + + $this->assertTrue($this->root->hasChild($relativeFullName)); + $this->assertFalse($this->root->hasChild($relativeFullName . '.old')); + $this->assertFalse($this->root->hasChild($relativeFullName . '.tmp')); + + $this->assertEquals(file_get_contents($filePath . DIRECTORY_SEPARATOR . $fileName), file_get_contents($this->root->getChild($relativeFullName)->url())); } } diff --git a/update.php b/update.php index 4ee85739d3..8626ed2991 100644 --- a/update.php +++ b/update.php @@ -1,5 +1,6 @@ getConfigCache(); - $configFileSaver = new ConfigFileSaver($app->getBasePath()); - - $updateConfigEntry = function($cat, $key) use ($configCache, $configFileSaver) { - // check if the config file differs from the whole configuration (= The db contains other values) - $fileConfig = $configCache->get($cat, $key); - if ($fileConfig === '!!') { - $fileConfig = null; - } - $savedConfig = Config::get($cat, $key, null, true); - if ($fileConfig !== $savedConfig) { - Logger::info('Difference in config found', ['cat' => $cat, 'key' => $key, 'file' => $fileConfig, 'saved' => $savedConfig]); - $configFileSaver->addConfigValue($cat, $key, $savedConfig); - } else { - Logger::info('No Difference in config found', ['cat' => $cat, 'key' => $key, 'value' => $fileConfig, 'saved' => $savedConfig]); - } - }; - - $configFileSaver->saveToConfigFile(); - - $updateConfigEntry('config', 'hostname'); - $updateConfigEntry('system', 'basepath'); - return Update::SUCCESS; + $app = BaseObject::getApp(); + if (Update::saveConfigToFile($app->getBasePath(), $app->getMode())) { + return Update::SUCCESS; + } else { + return Update::FAILED; + } }