Added Update checks

- Logging
- Console
- Admin overview

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);
 

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();

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
 	 *

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);
 	}
 }

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
 	 */

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;

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());
+		}
 	}
 }

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();
 

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();
 

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();
 	}

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()));
 	}
 }

update.php

@@ -1,5 +1,6 @@
 <?php
 
+use Friendica\BaseObject;
 use Friendica\Core\Addon;
 use Friendica\Core\Config;
 use Friendica\Core\L10n;
@@ -12,7 +13,6 @@ use Friendica\Model\Contact;
 use Friendica\Model\GContact;
 use Friendica\Model\Item;
 use Friendica\Model\User;
-use Friendica\Util\Config\ConfigFileSaver;
 use Friendica\Util\DateTimeFormat;
 
 /**
@@ -349,34 +349,15 @@ function update_1298()
 }
 
 /**
- * @see https://github.com/friendica/friendica/pull/6815
+ * @see https://github.com/friendica/friendica/pull/6920
- *
  * @return int Success
  */
-function update_1303()
+function update_1305()
 {
-	$app = \Friendica\BaseObject::getApp();
+	$app = BaseObject::getApp();
-	$configCache = $app->getConfigCache();
+	if (Update::saveConfigToFile($app->getBasePath(), $app->getMode())) {
-	$configFileSaver = new ConfigFileSaver($app->getBasePath());
+		return Update::SUCCESS;
-
+	} else {
-	$updateConfigEntry = function($cat, $key) use ($configCache, $configFileSaver) {
+		return Update::FAILED;
-		// check if the config file differs from the whole configuration (= The db contains other values)
+	}
-		$fileConfig = $configCache->get($cat, $key);
-		if ($fileConfig === '!<unset>!') {
-			$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;
 }