From a11ef5134cb4200399e74811bbf9d1100923efcd Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 6 Feb 2019 17:07:20 +0100 Subject: [PATCH 1/3] Add methods to get casted system values Signed-off-by: Joas Schilling --- lib/private/AllConfig.php | 36 ++++++++++++++++++++++++++++++++++++ lib/public/IConfig.php | 30 ++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+) diff --git a/lib/private/AllConfig.php b/lib/private/AllConfig.php index 09520aae2a..05ac1bad64 100644 --- a/lib/private/AllConfig.php +++ b/lib/private/AllConfig.php @@ -125,6 +125,42 @@ class AllConfig implements \OCP\IConfig { return $this->systemConfig->getValue($key, $default); } + /** + * Looks up a boolean system wide defined value + * + * @param string $key the key of the value, under which it was saved + * @param mixed $default the default value to be returned if the value isn't set + * @return mixed the value or $default + * @since 16.0.0 + */ + public function getSystemValueBool(string $key, bool $default = false): bool { + return (bool) $this->getSystemValue($key, $default); + } + + /** + * Looks up an integer system wide defined value + * + * @param string $key the key of the value, under which it was saved + * @param mixed $default the default value to be returned if the value isn't set + * @return mixed the value or $default + * @since 16.0.0 + */ + public function getSystemValueInt(string $key, int $default = 0): int { + return (int) $this->getSystemValue($key, $default); + } + + /** + * Looks up a string system wide defined value + * + * @param string $key the key of the value, under which it was saved + * @param mixed $default the default value to be returned if the value isn't set + * @return mixed the value or $default + * @since 16.0.0 + */ + public function getSystemValueString(string $key, string $default = ''): string { + return (string) $this->getSystemValue($key, $default); + } + /** * Looks up a system wide defined value and filters out sensitive data * diff --git a/lib/public/IConfig.php b/lib/public/IConfig.php index e4cd158f87..878c0acf0c 100644 --- a/lib/public/IConfig.php +++ b/lib/public/IConfig.php @@ -74,6 +74,36 @@ interface IConfig { */ public function getSystemValue($key, $default = ''); + /** + * Looks up a boolean system wide defined value + * + * @param string $key the key of the value, under which it was saved + * @param bool $default the default value to be returned if the value isn't set + * @return bool the value or $default + * @since 16.0.0 + */ + public function getSystemValueBool(string $key, bool $default = false): bool; + + /** + * Looks up an integer system wide defined value + * + * @param string $key the key of the value, under which it was saved + * @param int $default the default value to be returned if the value isn't set + * @return int the value or $default + * @since 16.0.0 + */ + public function getSystemValueInt(string $key, int $default = 0): int; + + /** + * Looks up a string system wide defined value + * + * @param string $key the key of the value, under which it was saved + * @param string $default the default value to be returned if the value isn't set + * @return string the value or $default + * @since 16.0.0 + */ + public function getSystemValueString(string $key, string $default = ''): string; + /** * Looks up a system wide defined value and filters out sensitive data * From 0c77cd21f94dd9922e0e04ec4c7e99eff9376e07 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 6 Feb 2019 17:08:41 +0100 Subject: [PATCH 2/3] Make sure maintenance mode is always casted to bool Signed-off-by: Joas Schilling --- apps/dav/lib/Connector/Sabre/MaintenancePlugin.php | 2 +- apps/encryption/lib/AppInfo/Application.php | 2 +- core/Command/Encryption/DecryptAll.php | 2 +- core/Command/Encryption/EncryptAll.php | 2 +- core/Command/Maintenance/Mode.php | 2 +- core/Command/Maintenance/Repair.php | 2 +- core/Command/Upgrade.php | 4 ++-- cron.php | 2 +- lib/base.php | 8 ++++---- lib/private/Console/Application.php | 10 +++++----- lib/private/Route/Router.php | 2 +- lib/private/Updater.php | 3 +-- lib/private/legacy/app.php | 2 +- ocs/v1.php | 3 +-- 14 files changed, 22 insertions(+), 24 deletions(-) diff --git a/apps/dav/lib/Connector/Sabre/MaintenancePlugin.php b/apps/dav/lib/Connector/Sabre/MaintenancePlugin.php index 36110c81db..9c5946481b 100644 --- a/apps/dav/lib/Connector/Sabre/MaintenancePlugin.php +++ b/apps/dav/lib/Connector/Sabre/MaintenancePlugin.php @@ -78,7 +78,7 @@ class MaintenancePlugin extends ServerPlugin { * @return bool */ public function checkMaintenanceMode() { - if ($this->config->getSystemValue('maintenance', false)) { + if ($this->config->getSystemValueBool('maintenance')) { throw new ServiceUnavailable('System in maintenance mode.'); } if (Util::needUpgrade()) { diff --git a/apps/encryption/lib/AppInfo/Application.php b/apps/encryption/lib/AppInfo/Application.php index 55839e097a..5846a01db1 100644 --- a/apps/encryption/lib/AppInfo/Application.php +++ b/apps/encryption/lib/AppInfo/Application.php @@ -75,7 +75,7 @@ class Application extends \OCP\AppFramework\App { * register hooks */ public function registerHooks() { - if (!$this->config->getSystemValue('maintenance', false)) { + if (!$this->config->getSystemValueBool('maintenance')) { $container = $this->getContainer(); $server = $container->getServer(); diff --git a/core/Command/Encryption/DecryptAll.php b/core/Command/Encryption/DecryptAll.php index 253223f952..6ae9019696 100644 --- a/core/Command/Encryption/DecryptAll.php +++ b/core/Command/Encryption/DecryptAll.php @@ -89,7 +89,7 @@ class DecryptAll extends Command { */ protected function forceMaintenanceAndTrashbin() { $this->wasTrashbinEnabled = $this->appManager->isEnabledForUser('files_trashbin'); - $this->wasMaintenanceModeEnabled = $this->config->getSystemValue('maintenance', false); + $this->wasMaintenanceModeEnabled = $this->config->getSystemValueBool('maintenance'); $this->config->setSystemValue('maintenance', true); $this->appManager->disableApp('files_trashbin'); } diff --git a/core/Command/Encryption/EncryptAll.php b/core/Command/Encryption/EncryptAll.php index b16fa0af2c..7a257aac20 100644 --- a/core/Command/Encryption/EncryptAll.php +++ b/core/Command/Encryption/EncryptAll.php @@ -78,7 +78,7 @@ class EncryptAll extends Command { */ protected function forceMaintenanceAndTrashbin() { $this->wasTrashbinEnabled = $this->appManager->isEnabledForUser('files_trashbin'); - $this->wasMaintenanceModeEnabled = $this->config->getSystemValue('maintenance', false); + $this->wasMaintenanceModeEnabled = $this->config->getSystemValueBool('maintenance'); $this->config->setSystemValue('maintenance', true); $this->appManager->disableApp('files_trashbin'); } diff --git a/core/Command/Maintenance/Mode.php b/core/Command/Maintenance/Mode.php index db4c9dc8c0..1692eb08d8 100644 --- a/core/Command/Maintenance/Mode.php +++ b/core/Command/Maintenance/Mode.php @@ -59,7 +59,7 @@ class Mode extends Command { } protected function execute(InputInterface $input, OutputInterface $output) { - $maintenanceMode = $this->config->getSystemValue('maintenance', false); + $maintenanceMode = $this->config->getSystemValueBool('maintenance'); if ($input->getOption('on')) { if ($maintenanceMode === false) { $this->config->setSystemValue('maintenance', true); diff --git a/core/Command/Maintenance/Repair.php b/core/Command/Maintenance/Repair.php index e9595a2228..460bc6880c 100644 --- a/core/Command/Maintenance/Repair.php +++ b/core/Command/Maintenance/Repair.php @@ -106,7 +106,7 @@ class Repair extends Command { } } - $maintenanceMode = $this->config->getSystemValue('maintenance', false); + $maintenanceMode = $this->config->getSystemValueBool('maintenance'); $this->config->setSystemValue('maintenance', true); $this->progress = new ProgressBar($output); diff --git a/core/Command/Upgrade.php b/core/Command/Upgrade.php index 5a2deea0b6..2d7ec4f688 100644 --- a/core/Command/Upgrade.php +++ b/core/Command/Upgrade.php @@ -180,7 +180,7 @@ class Upgrade extends Command { $dispatcher->addListener('\OC\Repair::info', $repairListener); $dispatcher->addListener('\OC\Repair::warning', $repairListener); $dispatcher->addListener('\OC\Repair::error', $repairListener); - + $updater->listen('\OC\Updater', 'maintenanceEnabled', function () use($output) { $output->writeln('Turned on maintenance mode'); @@ -264,7 +264,7 @@ class Upgrade extends Command { } return self::ERROR_SUCCESS; - } else if($this->config->getSystemValue('maintenance', false)) { + } else if($this->config->getSystemValueBool('maintenance')) { //Possible scenario: Nextcloud core is updated but an app failed $output->writeln('Nextcloud is in maintenance mode'); $output->write('Maybe an upgrade is already in process. Please check the ' diff --git a/cron.php b/cron.php index b79c40d2a7..cdfa0fb367 100644 --- a/cron.php +++ b/cron.php @@ -44,7 +44,7 @@ try { \OC::$server->getLogger()->debug('Update required, skipping cron', ['app' => 'cron']); exit; } - if (\OC::$server->getSystemConfig()->getValue('maintenance', false)) { + if ((bool) \OC::$server->getSystemConfig()->getValue('maintenance', false)) { \OC::$server->getLogger()->debug('We are in maintenance mode, skipping cron', ['app' => 'cron']); exit; } diff --git a/lib/base.php b/lib/base.php index 90fa549620..d5149fc044 100644 --- a/lib/base.php +++ b/lib/base.php @@ -285,7 +285,7 @@ class OC { public static function checkMaintenanceMode() { // Allow ajax update script to execute without being stopped - if (\OC::$server->getSystemConfig()->getValue('maintenance', false) && OC::$SUBURI != '/core/ajax/update.php') { + if (((bool) \OC::$server->getSystemConfig()->getValue('maintenance', false)) && OC::$SUBURI != '/core/ajax/update.php') { // send http status 503 http_response_code(503); header('Retry-After: 120'); @@ -938,7 +938,7 @@ class OC { if (function_exists('opcache_reset')) { opcache_reset(); } - if (!$systemConfig->getValue('maintenance', false)) { + if (!((bool) $systemConfig->getValue('maintenance', false))) { self::printUpgradePage($systemConfig); exit(); } @@ -966,7 +966,7 @@ class OC { // Load minimum set of apps if (!\OCP\Util::needUpgrade() - && !$systemConfig->getValue('maintenance', false)) { + && !((bool) $systemConfig->getValue('maintenance', false))) { // For logged-in users: Load everything if(\OC::$server->getUserSession()->isLoggedIn()) { OC_App::loadApps(); @@ -979,7 +979,7 @@ class OC { if (!self::$CLI) { try { - if (!$systemConfig->getValue('maintenance', false) && !\OCP\Util::needUpgrade()) { + if (!((bool) $systemConfig->getValue('maintenance', false)) && !\OCP\Util::needUpgrade()) { OC_App::loadApps(array('filesystem', 'logging')); OC_App::loadApps(); } diff --git a/lib/private/Console/Application.php b/lib/private/Console/Application.php index 0e30fa02b9..d7c047527f 100644 --- a/lib/private/Console/Application.php +++ b/lib/private/Console/Application.php @@ -91,10 +91,10 @@ class Application { $inputDefinition = $application->getDefinition(); $inputDefinition->addOption( new InputOption( - 'no-warnings', - null, - InputOption::VALUE_NONE, - 'Skip global warnings, show command output only', + 'no-warnings', + null, + InputOption::VALUE_NONE, + 'Skip global warnings, show command output only', null ) ); @@ -119,7 +119,7 @@ class Application { if ($this->config->getSystemValue('installed', false)) { if (\OCP\Util::needUpgrade()) { throw new NeedsUpdateException(); - } elseif ($this->config->getSystemValue('maintenance', false)) { + } elseif ($this->config->getSystemValueBool('maintenance')) { $this->writeMaintenanceModeInfo($input, $output); } else { OC_App::loadApps(); diff --git a/lib/private/Route/Router.php b/lib/private/Route/Router.php index b44b3f7c2c..1839b35642 100644 --- a/lib/private/Route/Router.php +++ b/lib/private/Route/Router.php @@ -260,7 +260,7 @@ class Router implements IRouter { $this->loadRoutes($app); } else if (substr($url, 0, 6) === '/core/' or substr($url, 0, 10) === '/settings/') { \OC::$REQUESTEDAPP = $url; - if (!\OC::$server->getConfig()->getSystemValue('maintenance', false) && !Util::needUpgrade()) { + if (!\OC::$server->getConfig()->getSystemValueBool('maintenance') && !Util::needUpgrade()) { \OC_App::loadApps(); } $this->loadRoutes('core'); diff --git a/lib/private/Updater.php b/lib/private/Updater.php index 4b4723be94..313cfc82ff 100644 --- a/lib/private/Updater.php +++ b/lib/private/Updater.php @@ -104,7 +104,7 @@ class Updater extends BasicEmitter { $this->emit('\OC\Updater', 'setDebugLogLevel', [ $logLevel, $this->logLevelNames[$logLevel] ]); $this->config->setSystemValue('loglevel', ILogger::DEBUG); - $wasMaintenanceModeEnabled = $this->config->getSystemValue('maintenance', false); + $wasMaintenanceModeEnabled = $this->config->getSystemValueBool('maintenance'); if(!$wasMaintenanceModeEnabled) { $this->config->setSystemValue('maintenance', true); @@ -614,4 +614,3 @@ class Updater extends BasicEmitter { } } - diff --git a/lib/private/legacy/app.php b/lib/private/legacy/app.php index 4cab92eba6..9b4a83de34 100644 --- a/lib/private/legacy/app.php +++ b/lib/private/legacy/app.php @@ -106,7 +106,7 @@ class OC_App { * if $types is set to non-empty array, only apps of those types will be loaded */ public static function loadApps(array $types = []): bool { - if (\OC::$server->getSystemConfig()->getValue('maintenance', false)) { + if ((bool) \OC::$server->getSystemConfig()->getValue('maintenance', false)) { return false; } // Load the enabled apps here diff --git a/ocs/v1.php b/ocs/v1.php index 36749f44c1..4e50392e75 100644 --- a/ocs/v1.php +++ b/ocs/v1.php @@ -33,7 +33,7 @@ require_once __DIR__ . '/../lib/versioncheck.php'; require_once __DIR__ . '/../lib/base.php'; if (\OCP\Util::needUpgrade() - || \OC::$server->getSystemConfig()->getValue('maintenance', false)) { + || \OC::$server->getConfig()->getSystemValueBool('maintenance')) { // since the behavior of apps or remotes are unpredictable during // an upgrade, return a 503 directly http_response_code(503); @@ -103,4 +103,3 @@ try { .' http://www.freedesktop.org/wiki/Specifications/open-collaboration-services.'."\n"; OC_API::respond(new \OC\OCS\Result(null, \OCP\API::RESPOND_NOT_FOUND, $txt), $format); } - From b4902369fbbbdddd80ef5043bfb3e9dbd9bf1732 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Fri, 22 Feb 2019 08:27:22 +0100 Subject: [PATCH 3/3] Fix unit tests Signed-off-by: Joas Schilling --- apps/dav/tests/unit/Connector/Sabre/MaintenancePluginTest.php | 3 ++- tests/Core/Command/Encryption/EncryptAllTest.php | 2 +- tests/Core/Command/Maintenance/ModeTest.php | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/apps/dav/tests/unit/Connector/Sabre/MaintenancePluginTest.php b/apps/dav/tests/unit/Connector/Sabre/MaintenancePluginTest.php index d4cc00f158..5a1a4619ea 100644 --- a/apps/dav/tests/unit/Connector/Sabre/MaintenancePluginTest.php +++ b/apps/dav/tests/unit/Connector/Sabre/MaintenancePluginTest.php @@ -53,7 +53,8 @@ class MaintenancePluginTest extends TestCase { public function testMaintenanceMode() { $this->config ->expects($this->exactly(1)) - ->method('getSystemValue') + ->method('getSystemValueBool') + ->with('maintenance') ->will($this->returnValue(true)); $this->maintenancePlugin->checkMaintenanceMode(); diff --git a/tests/Core/Command/Encryption/EncryptAllTest.php b/tests/Core/Command/Encryption/EncryptAllTest.php index ca7b264c98..c6af7e3872 100644 --- a/tests/Core/Command/Encryption/EncryptAllTest.php +++ b/tests/Core/Command/Encryption/EncryptAllTest.php @@ -91,7 +91,7 @@ class EncryptAllTest extends TestCase { $this->appManager->expects($this->once())->method('disableApp')->with('files_trashbin'); // enable single user mode to avoid that other user login during encryption // destructor should disable the single user mode again - $this->config->expects($this->once())->method('getSystemValue')->with('maintenance', false)->willReturn(false); + $this->config->expects($this->once())->method('getSystemValueBool')->with('maintenance', false)->willReturn(false); $this->config->expects($this->at(1))->method('setSystemValue')->with('maintenance', true); $this->config->expects($this->at(2))->method('setSystemValue')->with('maintenance', false); diff --git a/tests/Core/Command/Maintenance/ModeTest.php b/tests/Core/Command/Maintenance/ModeTest.php index da5e95998e..895a06bbec 100644 --- a/tests/Core/Command/Maintenance/ModeTest.php +++ b/tests/Core/Command/Maintenance/ModeTest.php @@ -123,7 +123,7 @@ class ModeTest extends TestCase { string $expectedOutput ) { $this->config->expects($this->any()) - ->method('getSystemValue') + ->method('getSystemValueBool') ->willReturn($currentMaintenanceState); if ($expectedMaintenanceState !== null) {