From 06952c4ef237930f7ceb8016270c7151e094f4ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Mon, 15 Jan 2024 16:45:19 +0100 Subject: [PATCH 01/13] Migrate memcached PHP module setup check to new API MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Merged it with the other existing memcache setup check as it fits Signed-off-by: Côme Chilliet --- .../lib/Controller/CheckSetupController.php | 19 ------- .../lib/SetupChecks/MemcacheConfigured.php | 22 ++++++-- .../Controller/CheckSetupControllerTest.php | 1 - core/js/setupchecks.js | 8 --- core/js/tests/specs/setupchecksSpec.js | 51 ------------------- 5 files changed, 19 insertions(+), 82 deletions(-) diff --git a/apps/settings/lib/Controller/CheckSetupController.php b/apps/settings/lib/Controller/CheckSetupController.php index 7c304d43be9..8ef0fdda600 100644 --- a/apps/settings/lib/Controller/CheckSetupController.php +++ b/apps/settings/lib/Controller/CheckSetupController.php @@ -123,24 +123,6 @@ class CheckSetupController extends Controller { return $this->manager->isFairUseOfFreePushService(); } - /** - * Checks if the correct memcache module for PHP is installed. Only - * fails if memcached is configured and the working module is not installed. - * - * @return bool - */ - private function isCorrectMemcachedPHPModuleInstalled() { - $memcacheDistributedClass = $this->config->getSystemValue('memcache.distributed', null); - if ($memcacheDistributedClass === null || ltrim($memcacheDistributedClass, '\\') !== \OC\Memcache\Memcached::class) { - return true; - } - - // there are two different memcache modules for PHP - // we only support memcached and not memcache - // https://code.google.com/p/memcached/wiki/PHPClientComparison - return !(!extension_loaded('memcached') && extension_loaded('memcache')); - } - /** * Checks if set_time_limit is not disabled. * @@ -293,7 +275,6 @@ Raw output [ 'isFairUseOfFreePushService' => $this->isFairUseOfFreePushService(), 'reverseProxyDocs' => $this->urlGenerator->linkToDocs('admin-reverse-proxy'), - 'isCorrectMemcachedPHPModuleInstalled' => $this->isCorrectMemcachedPHPModuleInstalled(), 'isSettimelimitAvailable' => $this->isSettimelimitAvailable(), 'areWebauthnExtensionsEnabled' => $this->areWebauthnExtensionsEnabled(), 'isMysqlUsedWithoutUTF8MB4' => $this->isMysqlUsedWithoutUTF8MB4(), diff --git a/apps/settings/lib/SetupChecks/MemcacheConfigured.php b/apps/settings/lib/SetupChecks/MemcacheConfigured.php index 2cde18a25df..1afe9cb879c 100644 --- a/apps/settings/lib/SetupChecks/MemcacheConfigured.php +++ b/apps/settings/lib/SetupChecks/MemcacheConfigured.php @@ -48,13 +48,29 @@ class MemcacheConfigured implements ISetupCheck { } public function run(): SetupResult { - if ($this->config->getSystemValue('memcache.local', null) !== null) { - return SetupResult::success($this->l10n->t('Configured')); - } else { + $memcacheDistributedClass = $this->config->getSystemValue('memcache.distributed', null); + $memcacheLockingClass = $this->config->getSystemValue('memcache.locking', null); + $memcacheLocalClass = $this->config->getSystemValue('memcache.local', null); + $caches = array_filter([$memcacheDistributedClass,$memcacheLockingClass,$memcacheLocalClass]); + if (in_array(\OC\Memcache\Memcached::class, array_map(fn (string $class) => ltrim($class, '\\'), $caches))) { + if (extension_loaded('memcache')) { + return SetupResult::warning( + $this->l10n->t('Memcached is configured as distributed cache, but the wrong PHP module "memcache" is installed. \\OC\\Memcache\\Memcached only supports "memcached" and not "memcache".'), + 'https://code.google.com/p/memcached/wiki/PHPClientComparison' + ); + } + if (!extension_loaded('memcached')) { + return SetupResult::warning( + $this->l10n->t('Memcached is configured as distributed cache, but the PHP module "memcached" is not installed.') + ); + } + } + if ($memcacheLocalClass === null) { return SetupResult::info( $this->l10n->t('No memory cache has been configured. To enhance performance, please configure a memcache, if available.'), $this->urlGenerator->linkToDocs('admin-performance') ); } + return SetupResult::success($this->l10n->t('Configured')); } } diff --git a/apps/settings/tests/Controller/CheckSetupControllerTest.php b/apps/settings/tests/Controller/CheckSetupControllerTest.php index 3bec435bd6a..deec0e6aedd 100644 --- a/apps/settings/tests/Controller/CheckSetupControllerTest.php +++ b/apps/settings/tests/Controller/CheckSetupControllerTest.php @@ -192,7 +192,6 @@ class CheckSetupControllerTest extends TestCase { $expected = new DataResponse( [ 'reverseProxyDocs' => 'reverse-proxy-doc-link', - 'isCorrectMemcachedPHPModuleInstalled' => true, 'isSettimelimitAvailable' => true, 'areWebauthnExtensionsEnabled' => false, 'isMysqlUsedWithoutUTF8MB4' => false, diff --git a/core/js/setupchecks.js b/core/js/setupchecks.js index 67ebabe1ae6..cd7bdeea17f 100644 --- a/core/js/setupchecks.js +++ b/core/js/setupchecks.js @@ -188,14 +188,6 @@ type: OC.SetupChecks.MESSAGE_TYPE_ERROR }); } - if(!data.isCorrectMemcachedPHPModuleInstalled) { - messages.push({ - msg: t('core', 'Memcached is configured as distributed cache, but the wrong PHP module "memcache" is installed. \\OC\\Memcache\\Memcached only supports "memcached" and not "memcache". See the {linkstart}memcached wiki about both modules ↗{linkend}.') - .replace('{linkstart}', '') - .replace('{linkend}', ''), - type: OC.SetupChecks.MESSAGE_TYPE_WARNING - }); - } if(!data.isSettimelimitAvailable) { messages.push({ msg: t('core', 'The PHP function "set_time_limit" is not available. This could result in scripts being halted mid-execution, breaking your installation. Enabling this function is strongly recommended.'), diff --git a/core/js/tests/specs/setupchecksSpec.js b/core/js/tests/specs/setupchecksSpec.js index 0214e589fe7..2aa95e0fd41 100644 --- a/core/js/tests/specs/setupchecksSpec.js +++ b/core/js/tests/specs/setupchecksSpec.js @@ -224,7 +224,6 @@ describe('OC.SetupChecks tests', function() { }, JSON.stringify({ isFairUseOfFreePushService: true, - isCorrectMemcachedPHPModuleInstalled: true, isSettimelimitAvailable: true, areWebauthnExtensionsEnabled: true, isMysqlUsedWithoutUTF8MB4: false, @@ -264,7 +263,6 @@ describe('OC.SetupChecks tests', function() { }, JSON.stringify({ isFairUseOfFreePushService: true, - isCorrectMemcachedPHPModuleInstalled: true, isSettimelimitAvailable: true, areWebauthnExtensionsEnabled: true, isMysqlUsedWithoutUTF8MB4: false, @@ -304,7 +302,6 @@ describe('OC.SetupChecks tests', function() { }, JSON.stringify({ isFairUseOfFreePushService: true, - isCorrectMemcachedPHPModuleInstalled: true, isSettimelimitAvailable: true, areWebauthnExtensionsEnabled: true, isMysqlUsedWithoutUTF8MB4: false, @@ -334,44 +331,6 @@ describe('OC.SetupChecks tests', function() { }); }); - it('should return an error if the wrong memcache PHP module is installed', function(done) { - var async = OC.SetupChecks.checkSetup(); - - suite.server.requests[0].respond( - 200, - { - 'Content-Type': 'application/json', - }, - JSON.stringify({ - isFairUseOfFreePushService: true, - isCorrectMemcachedPHPModuleInstalled: false, - isSettimelimitAvailable: true, - areWebauthnExtensionsEnabled: true, - isMysqlUsedWithoutUTF8MB4: false, - isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true, - reverseProxyGeneratedURL: 'https://server', - temporaryDirectoryWritable: true, - generic: { - network: { - "Internet connectivity": { - severity: "success", - description: null, - linkToDoc: null - } - }, - }, - }) - ); - - async.done(function( data, s, x ){ - expect(data).toEqual([{ - msg: 'Memcached is configured as distributed cache, but the wrong PHP module "memcache" is installed. \\OC\\Memcache\\Memcached only supports "memcached" and not "memcache". See the memcached wiki about both modules ↗.', - type: OC.SetupChecks.MESSAGE_TYPE_WARNING - }]); - done(); - }); - }); - it('should return an error if set_time_limit is unavailable', function(done) { var async = OC.SetupChecks.checkSetup(); @@ -383,7 +342,6 @@ describe('OC.SetupChecks tests', function() { JSON.stringify({ isFairUseOfFreePushService: true, reverseProxyDocs: 'https://docs.nextcloud.com/foo/bar.html', - isCorrectMemcachedPHPModuleInstalled: true, isSettimelimitAvailable: false, areWebauthnExtensionsEnabled: true, isMysqlUsedWithoutUTF8MB4: false, @@ -422,7 +380,6 @@ describe('OC.SetupChecks tests', function() { JSON.stringify({ isFairUseOfFreePushService: true, reverseProxyDocs: 'https://docs.nextcloud.com/foo/bar.html', - isCorrectMemcachedPHPModuleInstalled: true, isSettimelimitAvailable: true, areWebauthnExtensionsEnabled: true, isMysqlUsedWithoutUTF8MB4: false, @@ -492,7 +449,6 @@ describe('OC.SetupChecks tests', function() { }, JSON.stringify({ isFairUseOfFreePushService: true, - isCorrectMemcachedPHPModuleInstalled: true, isSettimelimitAvailable: true, areWebauthnExtensionsEnabled: true, isMysqlUsedWithoutUTF8MB4: false, @@ -537,7 +493,6 @@ describe('OC.SetupChecks tests', function() { }, JSON.stringify({ isFairUseOfFreePushService: true, - isCorrectMemcachedPHPModuleInstalled: true, isSettimelimitAvailable: true, areWebauthnExtensionsEnabled: true, isMysqlUsedWithoutUTF8MB4: true, @@ -579,7 +534,6 @@ describe('OC.SetupChecks tests', function() { }, JSON.stringify({ isFairUseOfFreePushService: true, - isCorrectMemcachedPHPModuleInstalled: true, isSettimelimitAvailable: true, areWebauthnExtensionsEnabled: true, isMysqlUsedWithoutUTF8MB4: false, @@ -618,7 +572,6 @@ describe('OC.SetupChecks tests', function() { }, JSON.stringify({ isFairUseOfFreePushService: true, - isCorrectMemcachedPHPModuleInstalled: true, isSettimelimitAvailable: true, areWebauthnExtensionsEnabled: true, isMysqlUsedWithoutUTF8MB4: false, @@ -654,7 +607,6 @@ describe('OC.SetupChecks tests', function() { }, JSON.stringify({ isFairUseOfFreePushService: true, - isCorrectMemcachedPHPModuleInstalled: true, isSettimelimitAvailable: true, areWebauthnExtensionsEnabled: true, isMysqlUsedWithoutUTF8MB4: false, @@ -692,7 +644,6 @@ describe('OC.SetupChecks tests', function() { }, JSON.stringify({ isFairUseOfFreePushService: true, - isCorrectMemcachedPHPModuleInstalled: true, isSettimelimitAvailable: true, areWebauthnExtensionsEnabled: false, isMysqlUsedWithoutUTF8MB4: false, @@ -730,7 +681,6 @@ describe('OC.SetupChecks tests', function() { }, JSON.stringify({ isFairUseOfFreePushService: true, - isCorrectMemcachedPHPModuleInstalled: true, isSettimelimitAvailable: true, areWebauthnExtensionsEnabled: true, isMysqlUsedWithoutUTF8MB4: false, @@ -775,7 +725,6 @@ describe('OC.SetupChecks tests', function() { }, JSON.stringify({ isFairUseOfFreePushService: true, - isCorrectMemcachedPHPModuleInstalled: true, isSettimelimitAvailable: true, areWebauthnExtensionsEnabled: true, isMysqlUsedWithoutUTF8MB4: false, From b31d51639d752d4a41184568b6fde50711c442d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Mon, 15 Jan 2024 17:08:14 +0100 Subject: [PATCH 02/13] Migrate set_time_limit setup check to new API MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- .../composer/composer/autoload_classmap.php | 1 + .../composer/composer/autoload_static.php | 1 + apps/settings/lib/AppInfo/Application.php | 2 + .../lib/Controller/CheckSetupController.php | 15 ----- .../lib/SetupChecks/PhpDisabledFunctions.php | 56 +++++++++++++++++++ apps/settings/lib/SetupChecks/PhpModules.php | 2 +- .../Controller/CheckSetupControllerTest.php | 1 - core/js/setupchecks.js | 6 -- core/js/tests/specs/setupchecksSpec.js | 50 ----------------- 9 files changed, 61 insertions(+), 73 deletions(-) create mode 100644 apps/settings/lib/SetupChecks/PhpDisabledFunctions.php diff --git a/apps/settings/composer/composer/autoload_classmap.php b/apps/settings/composer/composer/autoload_classmap.php index f00a2aad146..effa9c3b426 100644 --- a/apps/settings/composer/composer/autoload_classmap.php +++ b/apps/settings/composer/composer/autoload_classmap.php @@ -97,6 +97,7 @@ return array( 'OCA\\Settings\\SetupChecks\\MemcacheConfigured' => $baseDir . '/../lib/SetupChecks/MemcacheConfigured.php', 'OCA\\Settings\\SetupChecks\\OverwriteCliUrl' => $baseDir . '/../lib/SetupChecks/OverwriteCliUrl.php', 'OCA\\Settings\\SetupChecks\\PhpDefaultCharset' => $baseDir . '/../lib/SetupChecks/PhpDefaultCharset.php', + 'OCA\\Settings\\SetupChecks\\PhpDisabledFunctions' => $baseDir . '/../lib/SetupChecks/PhpDisabledFunctions.php', 'OCA\\Settings\\SetupChecks\\PhpFreetypeSupport' => $baseDir . '/../lib/SetupChecks/PhpFreetypeSupport.php', 'OCA\\Settings\\SetupChecks\\PhpGetEnv' => $baseDir . '/../lib/SetupChecks/PhpGetEnv.php', 'OCA\\Settings\\SetupChecks\\PhpMemoryLimit' => $baseDir . '/../lib/SetupChecks/PhpMemoryLimit.php', diff --git a/apps/settings/composer/composer/autoload_static.php b/apps/settings/composer/composer/autoload_static.php index d1aa4e3ea96..20834644eb5 100644 --- a/apps/settings/composer/composer/autoload_static.php +++ b/apps/settings/composer/composer/autoload_static.php @@ -112,6 +112,7 @@ class ComposerStaticInitSettings 'OCA\\Settings\\SetupChecks\\MemcacheConfigured' => __DIR__ . '/..' . '/../lib/SetupChecks/MemcacheConfigured.php', 'OCA\\Settings\\SetupChecks\\OverwriteCliUrl' => __DIR__ . '/..' . '/../lib/SetupChecks/OverwriteCliUrl.php', 'OCA\\Settings\\SetupChecks\\PhpDefaultCharset' => __DIR__ . '/..' . '/../lib/SetupChecks/PhpDefaultCharset.php', + 'OCA\\Settings\\SetupChecks\\PhpDisabledFunctions' => __DIR__ . '/..' . '/../lib/SetupChecks/PhpDisabledFunctions.php', 'OCA\\Settings\\SetupChecks\\PhpFreetypeSupport' => __DIR__ . '/..' . '/../lib/SetupChecks/PhpFreetypeSupport.php', 'OCA\\Settings\\SetupChecks\\PhpGetEnv' => __DIR__ . '/..' . '/../lib/SetupChecks/PhpGetEnv.php', 'OCA\\Settings\\SetupChecks\\PhpMemoryLimit' => __DIR__ . '/..' . '/../lib/SetupChecks/PhpMemoryLimit.php', diff --git a/apps/settings/lib/AppInfo/Application.php b/apps/settings/lib/AppInfo/Application.php index 0e89d0f989f..926703ecc1a 100644 --- a/apps/settings/lib/AppInfo/Application.php +++ b/apps/settings/lib/AppInfo/Application.php @@ -69,6 +69,7 @@ use OCA\Settings\SetupChecks\MaintenanceWindowStart; use OCA\Settings\SetupChecks\MemcacheConfigured; use OCA\Settings\SetupChecks\OverwriteCliUrl; use OCA\Settings\SetupChecks\PhpDefaultCharset; +use OCA\Settings\SetupChecks\PhpDisabledFunctions; use OCA\Settings\SetupChecks\PhpFreetypeSupport; use OCA\Settings\SetupChecks\PhpGetEnv; use OCA\Settings\SetupChecks\PhpMemoryLimit; @@ -187,6 +188,7 @@ class Application extends App implements IBootstrap { $context->registerSetupCheck(MemcacheConfigured::class); $context->registerSetupCheck(OverwriteCliUrl::class); $context->registerSetupCheck(PhpDefaultCharset::class); + $context->registerSetupCheck(PhpDisabledFunctions::class); $context->registerSetupCheck(PhpFreetypeSupport::class); $context->registerSetupCheck(PhpGetEnv::class); $context->registerSetupCheck(PhpMemoryLimit::class); diff --git a/apps/settings/lib/Controller/CheckSetupController.php b/apps/settings/lib/Controller/CheckSetupController.php index 8ef0fdda600..074625e02c0 100644 --- a/apps/settings/lib/Controller/CheckSetupController.php +++ b/apps/settings/lib/Controller/CheckSetupController.php @@ -123,20 +123,6 @@ class CheckSetupController extends Controller { return $this->manager->isFairUseOfFreePushService(); } - /** - * Checks if set_time_limit is not disabled. - * - * @return bool - */ - private function isSettimelimitAvailable() { - if (function_exists('set_time_limit') - && !str_contains(ini_get('disable_functions'), 'set_time_limit')) { - return true; - } - - return false; - } - /** * @NoCSRFRequired * @return RedirectResponse @@ -275,7 +261,6 @@ Raw output [ 'isFairUseOfFreePushService' => $this->isFairUseOfFreePushService(), 'reverseProxyDocs' => $this->urlGenerator->linkToDocs('admin-reverse-proxy'), - 'isSettimelimitAvailable' => $this->isSettimelimitAvailable(), 'areWebauthnExtensionsEnabled' => $this->areWebauthnExtensionsEnabled(), 'isMysqlUsedWithoutUTF8MB4' => $this->isMysqlUsedWithoutUTF8MB4(), 'isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed' => $this->isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed(), diff --git a/apps/settings/lib/SetupChecks/PhpDisabledFunctions.php b/apps/settings/lib/SetupChecks/PhpDisabledFunctions.php new file mode 100644 index 00000000000..b080f9bcecc --- /dev/null +++ b/apps/settings/lib/SetupChecks/PhpDisabledFunctions.php @@ -0,0 +1,56 @@ + + * + * @author Côme Chilliet + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ +namespace OCA\Settings\SetupChecks; + +use OCP\IL10N; +use OCP\SetupCheck\ISetupCheck; +use OCP\SetupCheck\SetupResult; + +class PhpDisabledFunctions implements ISetupCheck { + + public function __construct( + private IL10N $l10n, + ) { + } + + public function getName(): string { + return $this->l10n->t('PHP set_time_limit'); + } + + public function getCategory(): string { + return 'php'; + } + + public function run(): SetupResult { + if (function_exists('set_time_limit') && !str_contains(ini_get('disable_functions'), 'set_time_limit')) { + return SetupResult::success($this->l10n->t('The function is available.')); + } else { + return SetupResult::warning( + $this->l10n->t('The PHP function "set_time_limit" is not available. This could result in scripts being halted mid-execution, breaking your installation. Enabling this function is strongly recommended.'), + ); + } + } +} diff --git a/apps/settings/lib/SetupChecks/PhpModules.php b/apps/settings/lib/SetupChecks/PhpModules.php index 22c40b92841..bacf70eb9c9 100644 --- a/apps/settings/lib/SetupChecks/PhpModules.php +++ b/apps/settings/lib/SetupChecks/PhpModules.php @@ -66,7 +66,7 @@ class PhpModules implements ISetupCheck { } public function getCategory(): string { - return 'system'; + return 'php'; } public function run(): SetupResult { diff --git a/apps/settings/tests/Controller/CheckSetupControllerTest.php b/apps/settings/tests/Controller/CheckSetupControllerTest.php index deec0e6aedd..f135e075279 100644 --- a/apps/settings/tests/Controller/CheckSetupControllerTest.php +++ b/apps/settings/tests/Controller/CheckSetupControllerTest.php @@ -192,7 +192,6 @@ class CheckSetupControllerTest extends TestCase { $expected = new DataResponse( [ 'reverseProxyDocs' => 'reverse-proxy-doc-link', - 'isSettimelimitAvailable' => true, 'areWebauthnExtensionsEnabled' => false, 'isMysqlUsedWithoutUTF8MB4' => false, 'isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed' => true, diff --git a/core/js/setupchecks.js b/core/js/setupchecks.js index cd7bdeea17f..678cb02bfa2 100644 --- a/core/js/setupchecks.js +++ b/core/js/setupchecks.js @@ -188,12 +188,6 @@ type: OC.SetupChecks.MESSAGE_TYPE_ERROR }); } - if(!data.isSettimelimitAvailable) { - messages.push({ - msg: t('core', 'The PHP function "set_time_limit" is not available. This could result in scripts being halted mid-execution, breaking your installation. Enabling this function is strongly recommended.'), - type: OC.SetupChecks.MESSAGE_TYPE_WARNING - }); - } if (!data.areWebauthnExtensionsEnabled) { messages.push({ msg: t( diff --git a/core/js/tests/specs/setupchecksSpec.js b/core/js/tests/specs/setupchecksSpec.js index 2aa95e0fd41..857156f1437 100644 --- a/core/js/tests/specs/setupchecksSpec.js +++ b/core/js/tests/specs/setupchecksSpec.js @@ -224,7 +224,6 @@ describe('OC.SetupChecks tests', function() { }, JSON.stringify({ isFairUseOfFreePushService: true, - isSettimelimitAvailable: true, areWebauthnExtensionsEnabled: true, isMysqlUsedWithoutUTF8MB4: false, isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true, @@ -263,7 +262,6 @@ describe('OC.SetupChecks tests', function() { }, JSON.stringify({ isFairUseOfFreePushService: true, - isSettimelimitAvailable: true, areWebauthnExtensionsEnabled: true, isMysqlUsedWithoutUTF8MB4: false, isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true, @@ -302,7 +300,6 @@ describe('OC.SetupChecks tests', function() { }, JSON.stringify({ isFairUseOfFreePushService: true, - isSettimelimitAvailable: true, areWebauthnExtensionsEnabled: true, isMysqlUsedWithoutUTF8MB4: false, isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true, @@ -331,44 +328,6 @@ describe('OC.SetupChecks tests', function() { }); }); - it('should return an error if set_time_limit is unavailable', function(done) { - var async = OC.SetupChecks.checkSetup(); - - suite.server.requests[0].respond( - 200, - { - 'Content-Type': 'application/json', - }, - JSON.stringify({ - isFairUseOfFreePushService: true, - reverseProxyDocs: 'https://docs.nextcloud.com/foo/bar.html', - isSettimelimitAvailable: false, - areWebauthnExtensionsEnabled: true, - isMysqlUsedWithoutUTF8MB4: false, - isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true, - reverseProxyGeneratedURL: 'https://server', - temporaryDirectoryWritable: true, - generic: { - network: { - "Internet connectivity": { - severity: "success", - description: null, - linkToDoc: null - } - }, - }, - }) - ); - - async.done(function( data, s, x ){ - expect(data).toEqual([{ - msg: 'The PHP function "set_time_limit" is not available. This could result in scripts being halted mid-execution, breaking your installation. Enabling this function is strongly recommended.', - type: OC.SetupChecks.MESSAGE_TYPE_WARNING - }]); - done(); - }); - }); - it('should return a warning if the memory limit is below the recommended value', function(done) { var async = OC.SetupChecks.checkSetup(); @@ -380,7 +339,6 @@ describe('OC.SetupChecks tests', function() { JSON.stringify({ isFairUseOfFreePushService: true, reverseProxyDocs: 'https://docs.nextcloud.com/foo/bar.html', - isSettimelimitAvailable: true, areWebauthnExtensionsEnabled: true, isMysqlUsedWithoutUTF8MB4: false, isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true, @@ -449,7 +407,6 @@ describe('OC.SetupChecks tests', function() { }, JSON.stringify({ isFairUseOfFreePushService: true, - isSettimelimitAvailable: true, areWebauthnExtensionsEnabled: true, isMysqlUsedWithoutUTF8MB4: false, isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true, @@ -493,7 +450,6 @@ describe('OC.SetupChecks tests', function() { }, JSON.stringify({ isFairUseOfFreePushService: true, - isSettimelimitAvailable: true, areWebauthnExtensionsEnabled: true, isMysqlUsedWithoutUTF8MB4: true, isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true, @@ -534,7 +490,6 @@ describe('OC.SetupChecks tests', function() { }, JSON.stringify({ isFairUseOfFreePushService: true, - isSettimelimitAvailable: true, areWebauthnExtensionsEnabled: true, isMysqlUsedWithoutUTF8MB4: false, isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true, @@ -572,7 +527,6 @@ describe('OC.SetupChecks tests', function() { }, JSON.stringify({ isFairUseOfFreePushService: true, - isSettimelimitAvailable: true, areWebauthnExtensionsEnabled: true, isMysqlUsedWithoutUTF8MB4: false, isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true, @@ -607,7 +561,6 @@ describe('OC.SetupChecks tests', function() { }, JSON.stringify({ isFairUseOfFreePushService: true, - isSettimelimitAvailable: true, areWebauthnExtensionsEnabled: true, isMysqlUsedWithoutUTF8MB4: false, isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: false, @@ -644,7 +597,6 @@ describe('OC.SetupChecks tests', function() { }, JSON.stringify({ isFairUseOfFreePushService: true, - isSettimelimitAvailable: true, areWebauthnExtensionsEnabled: false, isMysqlUsedWithoutUTF8MB4: false, isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true, @@ -681,7 +633,6 @@ describe('OC.SetupChecks tests', function() { }, JSON.stringify({ isFairUseOfFreePushService: true, - isSettimelimitAvailable: true, areWebauthnExtensionsEnabled: true, isMysqlUsedWithoutUTF8MB4: false, isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true, @@ -725,7 +676,6 @@ describe('OC.SetupChecks tests', function() { }, JSON.stringify({ isFairUseOfFreePushService: true, - isSettimelimitAvailable: true, areWebauthnExtensionsEnabled: true, isMysqlUsedWithoutUTF8MB4: false, isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true, From b2cc51cf32b775867d53a5d477f319c62e06a6e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Tue, 16 Jan 2024 10:03:22 +0100 Subject: [PATCH 03/13] Merge gmp and bcmath module checks with the existing PHP modules setup check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Also add description for why each module is recommended Signed-off-by: Côme Chilliet --- .../lib/Controller/CheckSetupController.php | 11 ----- apps/settings/lib/SetupChecks/PhpModules.php | 21 ++++++++- .../Controller/CheckSetupControllerTest.php | 7 --- core/js/setupchecks.js | 9 ---- core/js/tests/specs/setupchecksSpec.js | 47 ------------------- 5 files changed, 20 insertions(+), 75 deletions(-) diff --git a/apps/settings/lib/Controller/CheckSetupController.php b/apps/settings/lib/Controller/CheckSetupController.php index 074625e02c0..21161be7cf2 100644 --- a/apps/settings/lib/Controller/CheckSetupController.php +++ b/apps/settings/lib/Controller/CheckSetupController.php @@ -202,16 +202,6 @@ Raw output return false; } - protected function areWebauthnExtensionsEnabled(): bool { - if (!extension_loaded('bcmath')) { - return false; - } - if (!extension_loaded('gmp')) { - return false; - } - return true; - } - protected function isMysqlUsedWithoutUTF8MB4(): bool { return ($this->config->getSystemValue('dbtype', 'sqlite') === 'mysql') && ($this->config->getSystemValue('mysql.utf8mb4', false) === false); } @@ -261,7 +251,6 @@ Raw output [ 'isFairUseOfFreePushService' => $this->isFairUseOfFreePushService(), 'reverseProxyDocs' => $this->urlGenerator->linkToDocs('admin-reverse-proxy'), - 'areWebauthnExtensionsEnabled' => $this->areWebauthnExtensionsEnabled(), 'isMysqlUsedWithoutUTF8MB4' => $this->isMysqlUsedWithoutUTF8MB4(), 'isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed' => $this->isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed(), 'reverseProxyGeneratedURL' => $this->urlGenerator->getAbsoluteURL('index.php'), diff --git a/apps/settings/lib/SetupChecks/PhpModules.php b/apps/settings/lib/SetupChecks/PhpModules.php index bacf70eb9c9..e1084d68d5a 100644 --- a/apps/settings/lib/SetupChecks/PhpModules.php +++ b/apps/settings/lib/SetupChecks/PhpModules.php @@ -69,6 +69,18 @@ class PhpModules implements ISetupCheck { return 'php'; } + protected function getRecommendedModuleDescription(string $module): string { + return match($module) { + 'bz2' => $this->l10n->t('required for extraction of apps compressed as bz2'), + 'intl' => $this->l10n->t('increases language translation performance and fixes sorting of non-ASCII characters'), + 'sodium' => $this->l10n->t('for Argon2 for password hashing'), + 'bcmath' => $this->l10n->t('for WebAuthn passwordless login'), + 'gmp' => $this->l10n->t('for WebAuthn passwordless login, and SFTP storage'), + 'exif' => $this->l10n->t('for image rotation in pictures app'), + default => '', + }; + } + public function run(): SetupResult { $missingRecommendedModules = $this->getMissingModules(self::RECOMMENDED_MODULES); $missingRequiredModules = $this->getMissingModules(self::REQUIRED_MODULES); @@ -78,8 +90,15 @@ class PhpModules implements ISetupCheck { $this->urlGenerator->linkToDocs('admin-php-modules') ); } elseif (!empty($missingRecommendedModules)) { + $moduleList = implode( + "\n", + array_map( + fn (string $module) => '- '.$module.' '.$this->getRecommendedModuleDescription($module), + $missingRecommendedModules + ) + ); return SetupResult::info( - $this->l10n->t('This instance is missing some recommended PHP modules. For improved performance and better compatibility it is highly recommended to install them: %s.', implode(', ', $missingRecommendedModules)), + $this->l10n->t("This instance is missing some recommended PHP modules. For improved performance and better compatibility it is highly recommended to install them:\n%s", $moduleList), $this->urlGenerator->linkToDocs('admin-php-modules') ); } else { diff --git a/apps/settings/tests/Controller/CheckSetupControllerTest.php b/apps/settings/tests/Controller/CheckSetupControllerTest.php index f135e075279..bad15a72191 100644 --- a/apps/settings/tests/Controller/CheckSetupControllerTest.php +++ b/apps/settings/tests/Controller/CheckSetupControllerTest.php @@ -118,7 +118,6 @@ class CheckSetupControllerTest extends TestCase { 'getCurlVersion', 'isPhpOutdated', 'isPHPMailerUsed', - 'areWebauthnExtensionsEnabled', 'isMysqlUsedWithoutUTF8MB4', 'isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed', ])->getMock(); @@ -143,11 +142,6 @@ class CheckSetupControllerTest extends TestCase { $this->request->expects($this->never()) ->method('getHeader'); - $this->checkSetupController - ->expects($this->once()) - ->method('areWebauthnExtensionsEnabled') - ->willReturn(false); - $this->checkSetupController ->expects($this->once()) ->method('isMysqlUsedWithoutUTF8MB4') @@ -192,7 +186,6 @@ class CheckSetupControllerTest extends TestCase { $expected = new DataResponse( [ 'reverseProxyDocs' => 'reverse-proxy-doc-link', - 'areWebauthnExtensionsEnabled' => false, 'isMysqlUsedWithoutUTF8MB4' => false, 'isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed' => true, 'reverseProxyGeneratedURL' => 'https://server/index.php', diff --git a/core/js/setupchecks.js b/core/js/setupchecks.js index 678cb02bfa2..0be5ceafd36 100644 --- a/core/js/setupchecks.js +++ b/core/js/setupchecks.js @@ -188,15 +188,6 @@ type: OC.SetupChecks.MESSAGE_TYPE_ERROR }); } - if (!data.areWebauthnExtensionsEnabled) { - messages.push({ - msg: t( - 'core', - 'The PHP modules "gmp" and/or "bcmath" are not enabled. If you use WebAuthn passwordless authentication, these modules are required.' - ), - type: OC.SetupChecks.MESSAGE_TYPE_INFO - }) - } if (data.isMysqlUsedWithoutUTF8MB4) { messages.push({ diff --git a/core/js/tests/specs/setupchecksSpec.js b/core/js/tests/specs/setupchecksSpec.js index 857156f1437..68325123efb 100644 --- a/core/js/tests/specs/setupchecksSpec.js +++ b/core/js/tests/specs/setupchecksSpec.js @@ -224,7 +224,6 @@ describe('OC.SetupChecks tests', function() { }, JSON.stringify({ isFairUseOfFreePushService: true, - areWebauthnExtensionsEnabled: true, isMysqlUsedWithoutUTF8MB4: false, isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true, reverseProxyGeneratedURL: 'https://server', @@ -262,7 +261,6 @@ describe('OC.SetupChecks tests', function() { }, JSON.stringify({ isFairUseOfFreePushService: true, - areWebauthnExtensionsEnabled: true, isMysqlUsedWithoutUTF8MB4: false, isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true, reverseProxyGeneratedURL: 'https://server', @@ -300,7 +298,6 @@ describe('OC.SetupChecks tests', function() { }, JSON.stringify({ isFairUseOfFreePushService: true, - areWebauthnExtensionsEnabled: true, isMysqlUsedWithoutUTF8MB4: false, isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true, reverseProxyGeneratedURL: 'https://server', @@ -339,7 +336,6 @@ describe('OC.SetupChecks tests', function() { JSON.stringify({ isFairUseOfFreePushService: true, reverseProxyDocs: 'https://docs.nextcloud.com/foo/bar.html', - areWebauthnExtensionsEnabled: true, isMysqlUsedWithoutUTF8MB4: false, isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true, reverseProxyGeneratedURL: 'https://server', @@ -407,7 +403,6 @@ describe('OC.SetupChecks tests', function() { }, JSON.stringify({ isFairUseOfFreePushService: true, - areWebauthnExtensionsEnabled: true, isMysqlUsedWithoutUTF8MB4: false, isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true, reverseProxyGeneratedURL: 'https://server', @@ -450,7 +445,6 @@ describe('OC.SetupChecks tests', function() { }, JSON.stringify({ isFairUseOfFreePushService: true, - areWebauthnExtensionsEnabled: true, isMysqlUsedWithoutUTF8MB4: true, isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true, reverseProxyGeneratedURL: 'https://server', @@ -490,7 +484,6 @@ describe('OC.SetupChecks tests', function() { }, JSON.stringify({ isFairUseOfFreePushService: true, - areWebauthnExtensionsEnabled: true, isMysqlUsedWithoutUTF8MB4: false, isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true, reverseProxyDocs: 'https://docs.nextcloud.com/foo/bar.html', @@ -527,7 +520,6 @@ describe('OC.SetupChecks tests', function() { }, JSON.stringify({ isFairUseOfFreePushService: true, - areWebauthnExtensionsEnabled: true, isMysqlUsedWithoutUTF8MB4: false, isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true, reverseProxyDocs: 'https://docs.nextcloud.com/foo/bar.html', @@ -561,7 +553,6 @@ describe('OC.SetupChecks tests', function() { }, JSON.stringify({ isFairUseOfFreePushService: true, - areWebauthnExtensionsEnabled: true, isMysqlUsedWithoutUTF8MB4: false, isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: false, reverseProxyGeneratedURL: 'https://server', @@ -587,42 +578,6 @@ describe('OC.SetupChecks tests', function() { }); }); - it('should return an error if gmp or bcmath are not enabled', function(done) { - var async = OC.SetupChecks.checkSetup(); - - suite.server.requests[0].respond( - 200, - { - 'Content-Type': 'application/json', - }, - JSON.stringify({ - isFairUseOfFreePushService: true, - areWebauthnExtensionsEnabled: false, - isMysqlUsedWithoutUTF8MB4: false, - isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true, - reverseProxyGeneratedURL: 'https://server', - temporaryDirectoryWritable: true, - generic: { - network: { - "Internet connectivity": { - severity: "success", - description: null, - linkToDoc: null - } - }, - }, - }) - ); - - async.done(function( data, s, x ){ - expect(data).toEqual([{ - msg: 'The PHP modules "gmp" and/or "bcmath" are not enabled. If you use WebAuthn passwordless authentication, these modules are required.', - type: OC.SetupChecks.MESSAGE_TYPE_INFO - }]); - done(); - }); - }); - it('should return an info if there is no default phone region', function(done) { var async = OC.SetupChecks.checkSetup(); @@ -633,7 +588,6 @@ describe('OC.SetupChecks tests', function() { }, JSON.stringify({ isFairUseOfFreePushService: true, - areWebauthnExtensionsEnabled: true, isMysqlUsedWithoutUTF8MB4: false, isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true, reverseProxyGeneratedURL: 'https://server', @@ -676,7 +630,6 @@ describe('OC.SetupChecks tests', function() { }, JSON.stringify({ isFairUseOfFreePushService: true, - areWebauthnExtensionsEnabled: true, isMysqlUsedWithoutUTF8MB4: false, isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true, reverseProxyGeneratedURL: 'https://server', From ec0a9acef48ffb79874ced2063078bef29340d4f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Thu, 18 Jan 2024 14:57:13 +0100 Subject: [PATCH 04/13] Fix description for exif PHP module MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- apps/settings/lib/SetupChecks/PhpModules.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/settings/lib/SetupChecks/PhpModules.php b/apps/settings/lib/SetupChecks/PhpModules.php index e1084d68d5a..d6bd8bc68e9 100644 --- a/apps/settings/lib/SetupChecks/PhpModules.php +++ b/apps/settings/lib/SetupChecks/PhpModules.php @@ -76,7 +76,7 @@ class PhpModules implements ISetupCheck { 'sodium' => $this->l10n->t('for Argon2 for password hashing'), 'bcmath' => $this->l10n->t('for WebAuthn passwordless login'), 'gmp' => $this->l10n->t('for WebAuthn passwordless login, and SFTP storage'), - 'exif' => $this->l10n->t('for image rotation in pictures app'), + 'exif' => $this->l10n->t('for picture rotation in server and metadata extraction in the Photos app'), default => '', }; } From 873912c3b995e36cb38687769a506c5174538e29 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Mon, 22 Jan 2024 15:08:59 +0100 Subject: [PATCH 05/13] Add missing recommended modules gmp and bcmath MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- apps/settings/lib/SetupChecks/PhpModules.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/apps/settings/lib/SetupChecks/PhpModules.php b/apps/settings/lib/SetupChecks/PhpModules.php index d6bd8bc68e9..1f22c3a7996 100644 --- a/apps/settings/lib/SetupChecks/PhpModules.php +++ b/apps/settings/lib/SetupChecks/PhpModules.php @@ -49,7 +49,9 @@ class PhpModules implements ISetupCheck { 'zlib', ]; protected const RECOMMENDED_MODULES = [ + 'bcmath', 'exif', + 'gmp', 'intl', 'sodium', 'sysvsem', From bfb451c883745eb3c620c5c66d0169585b7d5ed2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Mon, 22 Jan 2024 15:56:40 +0100 Subject: [PATCH 06/13] Remove bz2 leftover description MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- apps/settings/lib/SetupChecks/PhpModules.php | 1 - 1 file changed, 1 deletion(-) diff --git a/apps/settings/lib/SetupChecks/PhpModules.php b/apps/settings/lib/SetupChecks/PhpModules.php index 1f22c3a7996..9a6ddadbb1b 100644 --- a/apps/settings/lib/SetupChecks/PhpModules.php +++ b/apps/settings/lib/SetupChecks/PhpModules.php @@ -73,7 +73,6 @@ class PhpModules implements ISetupCheck { protected function getRecommendedModuleDescription(string $module): string { return match($module) { - 'bz2' => $this->l10n->t('required for extraction of apps compressed as bz2'), 'intl' => $this->l10n->t('increases language translation performance and fixes sorting of non-ASCII characters'), 'sodium' => $this->l10n->t('for Argon2 for password hashing'), 'bcmath' => $this->l10n->t('for WebAuthn passwordless login'), From 28dafd9413613d3d71b3d6f2de3227bb244f68e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Tue, 16 Jan 2024 11:30:08 +0100 Subject: [PATCH 07/13] Migrate MySQL utf8mb4 check to new SetupCheck API MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- .../composer/composer/autoload_classmap.php | 1 + .../composer/composer/autoload_static.php | 1 + apps/settings/lib/AppInfo/Application.php | 2 + .../lib/Controller/CheckSetupController.php | 5 -- .../lib/SetupChecks/MysqlUnicodeSupport.php | 63 +++++++++++++++++++ .../Controller/CheckSetupControllerTest.php | 51 --------------- core/js/setupchecks.js | 8 --- core/js/tests/specs/setupchecksSpec.js | 45 ------------- 8 files changed, 67 insertions(+), 109 deletions(-) create mode 100644 apps/settings/lib/SetupChecks/MysqlUnicodeSupport.php diff --git a/apps/settings/composer/composer/autoload_classmap.php b/apps/settings/composer/composer/autoload_classmap.php index effa9c3b426..80953034208 100644 --- a/apps/settings/composer/composer/autoload_classmap.php +++ b/apps/settings/composer/composer/autoload_classmap.php @@ -95,6 +95,7 @@ return array( 'OCA\\Settings\\SetupChecks\\LegacySSEKeyFormat' => $baseDir . '/../lib/SetupChecks/LegacySSEKeyFormat.php', 'OCA\\Settings\\SetupChecks\\MaintenanceWindowStart' => $baseDir . '/../lib/SetupChecks/MaintenanceWindowStart.php', 'OCA\\Settings\\SetupChecks\\MemcacheConfigured' => $baseDir . '/../lib/SetupChecks/MemcacheConfigured.php', + 'OCA\\Settings\\SetupChecks\\MysqlUnicodeSupport' => $baseDir . '/../lib/SetupChecks/MysqlUnicodeSupport.php', 'OCA\\Settings\\SetupChecks\\OverwriteCliUrl' => $baseDir . '/../lib/SetupChecks/OverwriteCliUrl.php', 'OCA\\Settings\\SetupChecks\\PhpDefaultCharset' => $baseDir . '/../lib/SetupChecks/PhpDefaultCharset.php', 'OCA\\Settings\\SetupChecks\\PhpDisabledFunctions' => $baseDir . '/../lib/SetupChecks/PhpDisabledFunctions.php', diff --git a/apps/settings/composer/composer/autoload_static.php b/apps/settings/composer/composer/autoload_static.php index 20834644eb5..9e23bbb47ae 100644 --- a/apps/settings/composer/composer/autoload_static.php +++ b/apps/settings/composer/composer/autoload_static.php @@ -110,6 +110,7 @@ class ComposerStaticInitSettings 'OCA\\Settings\\SetupChecks\\LegacySSEKeyFormat' => __DIR__ . '/..' . '/../lib/SetupChecks/LegacySSEKeyFormat.php', 'OCA\\Settings\\SetupChecks\\MaintenanceWindowStart' => __DIR__ . '/..' . '/../lib/SetupChecks/MaintenanceWindowStart.php', 'OCA\\Settings\\SetupChecks\\MemcacheConfigured' => __DIR__ . '/..' . '/../lib/SetupChecks/MemcacheConfigured.php', + 'OCA\\Settings\\SetupChecks\\MysqlUnicodeSupport' => __DIR__ . '/..' . '/../lib/SetupChecks/MysqlUnicodeSupport.php', 'OCA\\Settings\\SetupChecks\\OverwriteCliUrl' => __DIR__ . '/..' . '/../lib/SetupChecks/OverwriteCliUrl.php', 'OCA\\Settings\\SetupChecks\\PhpDefaultCharset' => __DIR__ . '/..' . '/../lib/SetupChecks/PhpDefaultCharset.php', 'OCA\\Settings\\SetupChecks\\PhpDisabledFunctions' => __DIR__ . '/..' . '/../lib/SetupChecks/PhpDisabledFunctions.php', diff --git a/apps/settings/lib/AppInfo/Application.php b/apps/settings/lib/AppInfo/Application.php index 926703ecc1a..2acbc35160b 100644 --- a/apps/settings/lib/AppInfo/Application.php +++ b/apps/settings/lib/AppInfo/Application.php @@ -67,6 +67,7 @@ use OCA\Settings\SetupChecks\JavaScriptModules; use OCA\Settings\SetupChecks\LegacySSEKeyFormat; use OCA\Settings\SetupChecks\MaintenanceWindowStart; use OCA\Settings\SetupChecks\MemcacheConfigured; +use OCA\Settings\SetupChecks\MysqlUnicodeSupport; use OCA\Settings\SetupChecks\OverwriteCliUrl; use OCA\Settings\SetupChecks\PhpDefaultCharset; use OCA\Settings\SetupChecks\PhpDisabledFunctions; @@ -186,6 +187,7 @@ class Application extends App implements IBootstrap { $context->registerSetupCheck(LegacySSEKeyFormat::class); $context->registerSetupCheck(MaintenanceWindowStart::class); $context->registerSetupCheck(MemcacheConfigured::class); + $context->registerSetupCheck(MysqlUnicodeSupport::class); $context->registerSetupCheck(OverwriteCliUrl::class); $context->registerSetupCheck(PhpDefaultCharset::class); $context->registerSetupCheck(PhpDisabledFunctions::class); diff --git a/apps/settings/lib/Controller/CheckSetupController.php b/apps/settings/lib/Controller/CheckSetupController.php index 21161be7cf2..bc898060928 100644 --- a/apps/settings/lib/Controller/CheckSetupController.php +++ b/apps/settings/lib/Controller/CheckSetupController.php @@ -202,10 +202,6 @@ Raw output return false; } - protected function isMysqlUsedWithoutUTF8MB4(): bool { - return ($this->config->getSystemValue('dbtype', 'sqlite') === 'mysql') && ($this->config->getSystemValue('mysql.utf8mb4', false) === false); - } - protected function isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed(): bool { $objectStore = $this->config->getSystemValue('objectstore', null); $objectStoreMultibucket = $this->config->getSystemValue('objectstore_multibucket', null); @@ -251,7 +247,6 @@ Raw output [ 'isFairUseOfFreePushService' => $this->isFairUseOfFreePushService(), 'reverseProxyDocs' => $this->urlGenerator->linkToDocs('admin-reverse-proxy'), - 'isMysqlUsedWithoutUTF8MB4' => $this->isMysqlUsedWithoutUTF8MB4(), 'isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed' => $this->isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed(), 'reverseProxyGeneratedURL' => $this->urlGenerator->getAbsoluteURL('index.php'), 'temporaryDirectoryWritable' => $this->isTemporaryDirectoryWritable(), diff --git a/apps/settings/lib/SetupChecks/MysqlUnicodeSupport.php b/apps/settings/lib/SetupChecks/MysqlUnicodeSupport.php new file mode 100644 index 00000000000..c90081c7398 --- /dev/null +++ b/apps/settings/lib/SetupChecks/MysqlUnicodeSupport.php @@ -0,0 +1,63 @@ + + * + * @author Côme Chilliet + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ +namespace OCA\Settings\SetupChecks; + +use OCP\IConfig; +use OCP\IL10N; +use OCP\IURLGenerator; +use OCP\SetupCheck\ISetupCheck; +use OCP\SetupCheck\SetupResult; + +class MysqlUnicodeSupport implements ISetupCheck { + public function __construct( + private IL10N $l10n, + private IConfig $config, + private IURLGenerator $urlGenerator, + ) { + } + + public function getName(): string { + return $this->l10n->t('MySQL unicode support'); + } + + public function getCategory(): string { + return 'database'; + } + + public function run(): SetupResult { + if ($this->config->getSystemValueString('dbtype') !== 'mysql') { + return SetupResult::success($this->l10n->t('You are not using MySQL')); + } + if ($this->config->getSystemValueBool('mysql.utf8mb4', false)) { + return SetupResult::success($this->l10n->t('MySQL is used as database and does support 4-byte characters')); + } else { + return SetupResult::warning( + $this->l10n->t('MySQL is used as database but does not support 4-byte characters. To be able to handle 4-byte characters (like emojis) without issues in filenames or comments for example it is recommended to enable the 4-byte support in MySQL.'), + $this->urlGenerator->linkToDocs('admin-mysql-utf8mb4'), + ); + } + } +} diff --git a/apps/settings/tests/Controller/CheckSetupControllerTest.php b/apps/settings/tests/Controller/CheckSetupControllerTest.php index bad15a72191..b888e0ce7c8 100644 --- a/apps/settings/tests/Controller/CheckSetupControllerTest.php +++ b/apps/settings/tests/Controller/CheckSetupControllerTest.php @@ -118,7 +118,6 @@ class CheckSetupControllerTest extends TestCase { 'getCurlVersion', 'isPhpOutdated', 'isPHPMailerUsed', - 'isMysqlUsedWithoutUTF8MB4', 'isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed', ])->getMock(); } @@ -142,11 +141,6 @@ class CheckSetupControllerTest extends TestCase { $this->request->expects($this->never()) ->method('getHeader'); - $this->checkSetupController - ->expects($this->once()) - ->method('isMysqlUsedWithoutUTF8MB4') - ->willReturn(false); - $this->checkSetupController ->expects($this->once()) ->method('isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed') @@ -186,7 +180,6 @@ class CheckSetupControllerTest extends TestCase { $expected = new DataResponse( [ 'reverseProxyDocs' => 'reverse-proxy-doc-link', - 'isMysqlUsedWithoutUTF8MB4' => false, 'isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed' => true, 'reverseProxyGeneratedURL' => 'https://server/index.php', 'isFairUseOfFreePushService' => false, @@ -653,50 +646,6 @@ Array $this->assertEquals($expected, $this->checkSetupController->getFailedIntegrityCheckFiles()); } - public function dataForIsMysqlUsedWithoutUTF8MB4() { - return [ - ['sqlite', false, false], - ['sqlite', true, false], - ['postgres', false, false], - ['postgres', true, false], - ['oci', false, false], - ['oci', true, false], - ['mysql', false, true], - ['mysql', true, false], - ]; - } - - /** - * @dataProvider dataForIsMysqlUsedWithoutUTF8MB4 - */ - public function testIsMysqlUsedWithoutUTF8MB4(string $db, bool $useUTF8MB4, bool $expected) { - $this->config->method('getSystemValue') - ->willReturnCallback(function ($key, $default) use ($db, $useUTF8MB4) { - if ($key === 'dbtype') { - return $db; - } - if ($key === 'mysql.utf8mb4') { - return $useUTF8MB4; - } - return $default; - }); - - $checkSetupController = new CheckSetupController( - 'settings', - $this->request, - $this->config, - $this->urlGenerator, - $this->l10n, - $this->checker, - $this->logger, - $this->tempManager, - $this->notificationManager, - $this->setupCheckManager, - ); - - $this->assertSame($expected, $this->invokePrivate($checkSetupController, 'isMysqlUsedWithoutUTF8MB4')); - } - public function dataForIsEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed() { return [ ['singlebucket', 'OC\\Files\\ObjectStore\\Swift', true], diff --git a/core/js/setupchecks.js b/core/js/setupchecks.js index 0be5ceafd36..761fca14ed9 100644 --- a/core/js/setupchecks.js +++ b/core/js/setupchecks.js @@ -189,14 +189,6 @@ }); } - if (data.isMysqlUsedWithoutUTF8MB4) { - messages.push({ - msg: t('core', 'MySQL is used as database but does not support 4-byte characters. To be able to handle 4-byte characters (like emojis) without issues in filenames or comments for example it is recommended to enable the 4-byte support in MySQL. For further details read {linkstart}the documentation page about this ↗{linkend}.') - .replace('{linkstart}', '') - .replace('{linkend}', ''), - type: OC.SetupChecks.MESSAGE_TYPE_WARNING - }) - } if (!data.isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed) { messages.push({ msg: t('core', 'This instance uses an S3 based object store as primary storage. The uploaded files are stored temporarily on the server and thus it is recommended to have 50 GB of free space available in the temp directory of PHP. Check the logs for full details about the path and the available space. To improve this please change the temporary directory in the php.ini or make more space available in that path.'), diff --git a/core/js/tests/specs/setupchecksSpec.js b/core/js/tests/specs/setupchecksSpec.js index 68325123efb..ef5ed8fbf4d 100644 --- a/core/js/tests/specs/setupchecksSpec.js +++ b/core/js/tests/specs/setupchecksSpec.js @@ -224,7 +224,6 @@ describe('OC.SetupChecks tests', function() { }, JSON.stringify({ isFairUseOfFreePushService: true, - isMysqlUsedWithoutUTF8MB4: false, isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true, reverseProxyGeneratedURL: 'https://server', temporaryDirectoryWritable: true, @@ -261,7 +260,6 @@ describe('OC.SetupChecks tests', function() { }, JSON.stringify({ isFairUseOfFreePushService: true, - isMysqlUsedWithoutUTF8MB4: false, isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true, reverseProxyGeneratedURL: 'https://server', temporaryDirectoryWritable: true, @@ -298,7 +296,6 @@ describe('OC.SetupChecks tests', function() { }, JSON.stringify({ isFairUseOfFreePushService: true, - isMysqlUsedWithoutUTF8MB4: false, isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true, reverseProxyGeneratedURL: 'https://server', temporaryDirectoryWritable: true, @@ -336,7 +333,6 @@ describe('OC.SetupChecks tests', function() { JSON.stringify({ isFairUseOfFreePushService: true, reverseProxyDocs: 'https://docs.nextcloud.com/foo/bar.html', - isMysqlUsedWithoutUTF8MB4: false, isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true, reverseProxyGeneratedURL: 'https://server', temporaryDirectoryWritable: true, @@ -403,7 +399,6 @@ describe('OC.SetupChecks tests', function() { }, JSON.stringify({ isFairUseOfFreePushService: true, - isMysqlUsedWithoutUTF8MB4: false, isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true, reverseProxyGeneratedURL: 'https://server', temporaryDirectoryWritable: true, @@ -435,41 +430,6 @@ describe('OC.SetupChecks tests', function() { }); }); - it('should return an error if the php version is no longer supported', function(done) { - var async = OC.SetupChecks.checkSetup(); - - suite.server.requests[0].respond( - 200, - { - 'Content-Type': 'application/json', - }, - JSON.stringify({ - isFairUseOfFreePushService: true, - isMysqlUsedWithoutUTF8MB4: true, - isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true, - reverseProxyGeneratedURL: 'https://server', - temporaryDirectoryWritable: true, - generic: { - network: { - "Internet connectivity": { - severity: "success", - description: null, - linkToDoc: null - } - }, - }, - }) - ); - - async.done(function( data, s, x ){ - expect(data).toEqual([{ - msg: 'MySQL is used as database but does not support 4-byte characters. To be able to handle 4-byte characters (like emojis) without issues in filenames or comments for example it is recommended to enable the 4-byte support in MySQL. For further details read the documentation page about this ↗.', - type: OC.SetupChecks.MESSAGE_TYPE_WARNING - }]); - done(); - }); - }); - // THe following test is invalid as the code in core/js/setupchecks.js is calling // window.location.protocol which always return http during tests // if there is a way to trick window.location.protocol during test, then we could re-activate it @@ -484,7 +444,6 @@ describe('OC.SetupChecks tests', function() { }, JSON.stringify({ isFairUseOfFreePushService: true, - isMysqlUsedWithoutUTF8MB4: false, isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true, reverseProxyDocs: 'https://docs.nextcloud.com/foo/bar.html', reverseProxyGeneratedURL: 'http://server', @@ -520,7 +479,6 @@ describe('OC.SetupChecks tests', function() { }, JSON.stringify({ isFairUseOfFreePushService: true, - isMysqlUsedWithoutUTF8MB4: false, isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true, reverseProxyDocs: 'https://docs.nextcloud.com/foo/bar.html', reverseProxyGeneratedURL: 'http://server', @@ -553,7 +511,6 @@ describe('OC.SetupChecks tests', function() { }, JSON.stringify({ isFairUseOfFreePushService: true, - isMysqlUsedWithoutUTF8MB4: false, isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: false, reverseProxyGeneratedURL: 'https://server', temporaryDirectoryWritable: true, @@ -588,7 +545,6 @@ describe('OC.SetupChecks tests', function() { }, JSON.stringify({ isFairUseOfFreePushService: true, - isMysqlUsedWithoutUTF8MB4: false, isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true, reverseProxyGeneratedURL: 'https://server', temporaryDirectoryWritable: true, @@ -630,7 +586,6 @@ describe('OC.SetupChecks tests', function() { }, JSON.stringify({ isFairUseOfFreePushService: true, - isMysqlUsedWithoutUTF8MB4: false, isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true, reverseProxyGeneratedURL: 'https://server', temporaryDirectoryWritable: false, From 9dcf13fa1295ecbb582fd793aa301dff47f0bd77 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Tue, 16 Jan 2024 12:11:56 +0100 Subject: [PATCH 08/13] Migrate available temp space check to new SetupCheck API MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- .../composer/composer/autoload_classmap.php | 1 + .../composer/composer/autoload_static.php | 1 + apps/settings/lib/AppInfo/Application.php | 2 + .../TempSpaceAvailableIfS3PrimaryStorage.php | 93 +++++++++++++++++++ 4 files changed, 97 insertions(+) create mode 100644 apps/settings/lib/SetupChecks/TempSpaceAvailableIfS3PrimaryStorage.php diff --git a/apps/settings/composer/composer/autoload_classmap.php b/apps/settings/composer/composer/autoload_classmap.php index 80953034208..7166ba2aab0 100644 --- a/apps/settings/composer/composer/autoload_classmap.php +++ b/apps/settings/composer/composer/autoload_classmap.php @@ -110,6 +110,7 @@ return array( 'OCA\\Settings\\SetupChecks\\ReadOnlyConfig' => $baseDir . '/../lib/SetupChecks/ReadOnlyConfig.php', 'OCA\\Settings\\SetupChecks\\SupportedDatabase' => $baseDir . '/../lib/SetupChecks/SupportedDatabase.php', 'OCA\\Settings\\SetupChecks\\SystemIs64bit' => $baseDir . '/../lib/SetupChecks/SystemIs64bit.php', + 'OCA\\Settings\\SetupChecks\\TempSpaceAvailableIfS3PrimaryStorage' => $baseDir . '/../lib/SetupChecks/TempSpaceAvailableIfS3PrimaryStorage.php', 'OCA\\Settings\\SetupChecks\\TransactionIsolation' => $baseDir . '/../lib/SetupChecks/TransactionIsolation.php', 'OCA\\Settings\\UserMigration\\AccountMigrator' => $baseDir . '/../lib/UserMigration/AccountMigrator.php', 'OCA\\Settings\\UserMigration\\AccountMigratorException' => $baseDir . '/../lib/UserMigration/AccountMigratorException.php', diff --git a/apps/settings/composer/composer/autoload_static.php b/apps/settings/composer/composer/autoload_static.php index 9e23bbb47ae..89517edf9f3 100644 --- a/apps/settings/composer/composer/autoload_static.php +++ b/apps/settings/composer/composer/autoload_static.php @@ -125,6 +125,7 @@ class ComposerStaticInitSettings 'OCA\\Settings\\SetupChecks\\ReadOnlyConfig' => __DIR__ . '/..' . '/../lib/SetupChecks/ReadOnlyConfig.php', 'OCA\\Settings\\SetupChecks\\SupportedDatabase' => __DIR__ . '/..' . '/../lib/SetupChecks/SupportedDatabase.php', 'OCA\\Settings\\SetupChecks\\SystemIs64bit' => __DIR__ . '/..' . '/../lib/SetupChecks/SystemIs64bit.php', + 'OCA\\Settings\\SetupChecks\\TempSpaceAvailableIfS3PrimaryStorage' => __DIR__ . '/..' . '/../lib/SetupChecks/TempSpaceAvailableIfS3PrimaryStorage.php', 'OCA\\Settings\\SetupChecks\\TransactionIsolation' => __DIR__ . '/..' . '/../lib/SetupChecks/TransactionIsolation.php', 'OCA\\Settings\\UserMigration\\AccountMigrator' => __DIR__ . '/..' . '/../lib/UserMigration/AccountMigrator.php', 'OCA\\Settings\\UserMigration\\AccountMigratorException' => __DIR__ . '/..' . '/../lib/UserMigration/AccountMigratorException.php', diff --git a/apps/settings/lib/AppInfo/Application.php b/apps/settings/lib/AppInfo/Application.php index 2acbc35160b..f029ebf8120 100644 --- a/apps/settings/lib/AppInfo/Application.php +++ b/apps/settings/lib/AppInfo/Application.php @@ -82,6 +82,7 @@ use OCA\Settings\SetupChecks\RandomnessSecure; use OCA\Settings\SetupChecks\ReadOnlyConfig; use OCA\Settings\SetupChecks\SupportedDatabase; use OCA\Settings\SetupChecks\SystemIs64bit; +use OCA\Settings\SetupChecks\TempSpaceAvailableIfS3PrimaryStorage; use OCA\Settings\SetupChecks\TransactionIsolation; use OCA\Settings\UserMigration\AccountMigrator; use OCA\Settings\WellKnown\ChangePasswordHandler; @@ -202,6 +203,7 @@ class Application extends App implements IBootstrap { $context->registerSetupCheck(ReadOnlyConfig::class); $context->registerSetupCheck(SupportedDatabase::class); $context->registerSetupCheck(SystemIs64bit::class); + $context->registerSetupCheck(TempSpaceAvailableIfS3PrimaryStorage::class); $context->registerSetupCheck(TransactionIsolation::class); $context->registerUserMigrator(AccountMigrator::class); diff --git a/apps/settings/lib/SetupChecks/TempSpaceAvailableIfS3PrimaryStorage.php b/apps/settings/lib/SetupChecks/TempSpaceAvailableIfS3PrimaryStorage.php new file mode 100644 index 00000000000..b8d7cf7d3e1 --- /dev/null +++ b/apps/settings/lib/SetupChecks/TempSpaceAvailableIfS3PrimaryStorage.php @@ -0,0 +1,93 @@ + + * + * @author Côme Chilliet + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ +namespace OCA\Settings\SetupChecks; + +use OCP\IConfig; +use OCP\IL10N; +use OCP\IURLGenerator; +use OCP\SetupCheck\ISetupCheck; +use OCP\SetupCheck\SetupResult; + +class TempSpaceAvailableIfS3PrimaryStorage implements ISetupCheck { + public function __construct( + private IL10N $l10n, + private IConfig $config, + private IURLGenerator $urlGenerator, + ) { + } + + public function getName(): string { + return $this->l10n->t('Temporary space available'); + } + + public function getCategory(): string { + return 'system'; + } + + public function run(): SetupResult { + $objectStore = $this->config->getSystemValue('objectstore', null); + $objectStoreMultibucket = $this->config->getSystemValue('objectstore_multibucket', null); + + // TODO we should check and display temp space available even if not s3 + if (!isset($objectStoreMultibucket) && !isset($objectStore)) { + return SetupResult::success($this->l10n->t('This instance does not use an S3 based object store as primary storage')); + } + + if (isset($objectStoreMultibucket['class']) && $objectStoreMultibucket['class'] !== 'OC\\Files\\ObjectStore\\S3') { + return SetupResult::success($this->l10n->t('This instance does not use an S3 based object store as primary storage')); + } + + if (isset($objectStore['class']) && $objectStore['class'] !== 'OC\\Files\\ObjectStore\\S3') { + return SetupResult::success($this->l10n->t('This instance does not use an S3 based object store as primary storage')); + } + + $tempPath = sys_get_temp_dir(); + if (!is_dir($tempPath)) { + return SetupResult::error($this->l10n->t('Error while checking the temporary PHP path - it was not properly set to a directory. Returned value: %s', [$tempPath])); + } + $freeSpaceInTemp = function_exists('disk_free_space') ? disk_free_space($tempPath) : false; + if ($freeSpaceInTemp === false) { + return SetupResult::error($this->l10n->t('Error while checking the available disk space of temporary PHP path or no free disk space returned. Temporary path: %s', [$tempPath])); + } + + $freeSpaceInTempInGB = $freeSpaceInTemp / 1024 / 1024 / 1024; + if ($freeSpaceInTempInGB > 50) { + return SetupResult::success( + $this->l10n->t( + "This instance uses an S3 based object store as primary storage, and has enough space in the temporary directory.\nAvailable: %.1f GiB\nPath: %s", + [round($freeSpaceInTempInGB, 1),$tempPath] + ) + ); + } + + return SetupResult::warning( + $this->l10n->t( + "This instance uses an S3 based object store as primary storage. The uploaded files are stored temporarily on the server and thus it is recommended to have 50 GiB of free space available in the temp directory of PHP. To improve this please change the temporary directory in the php.ini or make more space available in that path. \nChecking the available space in the temporary path resulted in %.1f GiB instead of the recommended 50 GiB. Path: %s", + [round($freeSpaceInTempInGB, 1),$tempPath] + ) + ); + } +} From 11cde3096a17157ad3896a9f641828e60af04bfd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Tue, 16 Jan 2024 13:50:44 +0100 Subject: [PATCH 09/13] Remove old version of temporary space setup check and fix tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- .../lib/Controller/CheckSetupController.php | 37 -------------- .../Controller/CheckSetupControllerTest.php | 51 ------------------- core/js/setupchecks.js | 6 --- core/js/tests/specs/setupchecksSpec.js | 43 ---------------- 4 files changed, 137 deletions(-) diff --git a/apps/settings/lib/Controller/CheckSetupController.php b/apps/settings/lib/Controller/CheckSetupController.php index bc898060928..ba39579e445 100644 --- a/apps/settings/lib/Controller/CheckSetupController.php +++ b/apps/settings/lib/Controller/CheckSetupController.php @@ -202,42 +202,6 @@ Raw output return false; } - protected function isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed(): bool { - $objectStore = $this->config->getSystemValue('objectstore', null); - $objectStoreMultibucket = $this->config->getSystemValue('objectstore_multibucket', null); - - if (!isset($objectStoreMultibucket) && !isset($objectStore)) { - return true; - } - - if (isset($objectStoreMultibucket['class']) && $objectStoreMultibucket['class'] !== 'OC\\Files\\ObjectStore\\S3') { - return true; - } - - if (isset($objectStore['class']) && $objectStore['class'] !== 'OC\\Files\\ObjectStore\\S3') { - return true; - } - - $tempPath = sys_get_temp_dir(); - if (!is_dir($tempPath)) { - $this->logger->error('Error while checking the temporary PHP path - it was not properly set to a directory. Returned value: ' . $tempPath); - return false; - } - $freeSpaceInTemp = function_exists('disk_free_space') ? disk_free_space($tempPath) : false; - if ($freeSpaceInTemp === false) { - $this->logger->error('Error while checking the available disk space of temporary PHP path or no free disk space returned. Temporary path: ' . $tempPath); - return false; - } - - $freeSpaceInTempInGB = $freeSpaceInTemp / 1024 / 1024 / 1024; - if ($freeSpaceInTempInGB > 50) { - return true; - } - - $this->logger->warning('Checking the available space in the temporary path resulted in ' . round($freeSpaceInTempInGB, 1) . ' GB instead of the recommended 50GB. Path: ' . $tempPath); - return false; - } - /** * @return DataResponse * @AuthorizedAdminSetting(settings=OCA\Settings\Settings\Admin\Overview) @@ -247,7 +211,6 @@ Raw output [ 'isFairUseOfFreePushService' => $this->isFairUseOfFreePushService(), 'reverseProxyDocs' => $this->urlGenerator->linkToDocs('admin-reverse-proxy'), - 'isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed' => $this->isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed(), 'reverseProxyGeneratedURL' => $this->urlGenerator->getAbsoluteURL('index.php'), 'temporaryDirectoryWritable' => $this->isTemporaryDirectoryWritable(), 'generic' => $this->setupCheckManager->runAll(), diff --git a/apps/settings/tests/Controller/CheckSetupControllerTest.php b/apps/settings/tests/Controller/CheckSetupControllerTest.php index b888e0ce7c8..f8c9b346e7d 100644 --- a/apps/settings/tests/Controller/CheckSetupControllerTest.php +++ b/apps/settings/tests/Controller/CheckSetupControllerTest.php @@ -118,7 +118,6 @@ class CheckSetupControllerTest extends TestCase { 'getCurlVersion', 'isPhpOutdated', 'isPHPMailerUsed', - 'isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed', ])->getMock(); } @@ -141,11 +140,6 @@ class CheckSetupControllerTest extends TestCase { $this->request->expects($this->never()) ->method('getHeader'); - $this->checkSetupController - ->expects($this->once()) - ->method('isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed') - ->willReturn(true); - $this->urlGenerator->method('linkToDocs') ->willReturnCallback(function (string $key): string { if ($key === 'admin-performance') { @@ -180,7 +174,6 @@ class CheckSetupControllerTest extends TestCase { $expected = new DataResponse( [ 'reverseProxyDocs' => 'reverse-proxy-doc-link', - 'isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed' => true, 'reverseProxyGeneratedURL' => 'https://server/index.php', 'isFairUseOfFreePushService' => false, 'temporaryDirectoryWritable' => false, @@ -645,48 +638,4 @@ Array ); $this->assertEquals($expected, $this->checkSetupController->getFailedIntegrityCheckFiles()); } - - public function dataForIsEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed() { - return [ - ['singlebucket', 'OC\\Files\\ObjectStore\\Swift', true], - ['multibucket', 'OC\\Files\\ObjectStore\\Swift', true], - ['singlebucket', 'OC\\Files\\ObjectStore\\Custom', true], - ['multibucket', 'OC\Files\\ObjectStore\\Custom', true], - ['singlebucket', 'OC\Files\ObjectStore\Swift', true], - ['multibucket', 'OC\Files\ObjectStore\Swift', true], - ['singlebucket', 'OC\Files\ObjectStore\Custom', true], - ['multibucket', 'OC\Files\ObjectStore\Custom', true], - ]; - } - - /** - * @dataProvider dataForIsEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed - */ - public function testIsEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed(string $mode, string $className, bool $expected) { - $this->config->method('getSystemValue') - ->willReturnCallback(function ($key, $default) use ($mode, $className) { - if ($key === 'objectstore' && $mode === 'singlebucket') { - return ['class' => $className]; - } - if ($key === 'objectstore_multibucket' && $mode === 'multibucket') { - return ['class' => $className]; - } - return $default; - }); - - $checkSetupController = new CheckSetupController( - 'settings', - $this->request, - $this->config, - $this->urlGenerator, - $this->l10n, - $this->checker, - $this->logger, - $this->tempManager, - $this->notificationManager, - $this->setupCheckManager, - ); - - $this->assertSame($expected, $this->invokePrivate($checkSetupController, 'isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed')); - } } diff --git a/core/js/setupchecks.js b/core/js/setupchecks.js index 761fca14ed9..7e068ffd1d0 100644 --- a/core/js/setupchecks.js +++ b/core/js/setupchecks.js @@ -189,12 +189,6 @@ }); } - if (!data.isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed) { - messages.push({ - msg: t('core', 'This instance uses an S3 based object store as primary storage. The uploaded files are stored temporarily on the server and thus it is recommended to have 50 GB of free space available in the temp directory of PHP. Check the logs for full details about the path and the available space. To improve this please change the temporary directory in the php.ini or make more space available in that path.'), - type: OC.SetupChecks.MESSAGE_TYPE_WARNING - }) - } if (!data.temporaryDirectoryWritable) { messages.push({ msg: t('core', 'The temporary directory of this instance points to an either non-existing or non-writable directory.'), diff --git a/core/js/tests/specs/setupchecksSpec.js b/core/js/tests/specs/setupchecksSpec.js index ef5ed8fbf4d..6a4f68b977d 100644 --- a/core/js/tests/specs/setupchecksSpec.js +++ b/core/js/tests/specs/setupchecksSpec.js @@ -224,7 +224,6 @@ describe('OC.SetupChecks tests', function() { }, JSON.stringify({ isFairUseOfFreePushService: true, - isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true, reverseProxyGeneratedURL: 'https://server', temporaryDirectoryWritable: true, generic: { @@ -260,7 +259,6 @@ describe('OC.SetupChecks tests', function() { }, JSON.stringify({ isFairUseOfFreePushService: true, - isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true, reverseProxyGeneratedURL: 'https://server', temporaryDirectoryWritable: true, generic: { @@ -296,7 +294,6 @@ describe('OC.SetupChecks tests', function() { }, JSON.stringify({ isFairUseOfFreePushService: true, - isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true, reverseProxyGeneratedURL: 'https://server', temporaryDirectoryWritable: true, generic: { @@ -333,7 +330,6 @@ describe('OC.SetupChecks tests', function() { JSON.stringify({ isFairUseOfFreePushService: true, reverseProxyDocs: 'https://docs.nextcloud.com/foo/bar.html', - isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true, reverseProxyGeneratedURL: 'https://server', temporaryDirectoryWritable: true, generic: { @@ -399,7 +395,6 @@ describe('OC.SetupChecks tests', function() { }, JSON.stringify({ isFairUseOfFreePushService: true, - isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true, reverseProxyGeneratedURL: 'https://server', temporaryDirectoryWritable: true, generic: { @@ -444,7 +439,6 @@ describe('OC.SetupChecks tests', function() { }, JSON.stringify({ isFairUseOfFreePushService: true, - isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true, reverseProxyDocs: 'https://docs.nextcloud.com/foo/bar.html', reverseProxyGeneratedURL: 'http://server', temporaryDirectoryWritable: true, @@ -479,7 +473,6 @@ describe('OC.SetupChecks tests', function() { }, JSON.stringify({ isFairUseOfFreePushService: true, - isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true, reverseProxyDocs: 'https://docs.nextcloud.com/foo/bar.html', reverseProxyGeneratedURL: 'http://server', temporaryDirectoryWritable: true, @@ -501,40 +494,6 @@ describe('OC.SetupChecks tests', function() { }); }); - it('should return an error if there is not enough free space in the temp directory', function(done) { - var async = OC.SetupChecks.checkSetup(); - - suite.server.requests[0].respond( - 200, - { - 'Content-Type': 'application/json', - }, - JSON.stringify({ - isFairUseOfFreePushService: true, - isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: false, - reverseProxyGeneratedURL: 'https://server', - temporaryDirectoryWritable: true, - generic: { - network: { - "Internet connectivity": { - severity: "success", - description: null, - linkToDoc: null - } - }, - }, - }) - ); - - async.done(function( data, s, x ){ - expect(data).toEqual([{ - msg: 'This instance uses an S3 based object store as primary storage. The uploaded files are stored temporarily on the server and thus it is recommended to have 50 GB of free space available in the temp directory of PHP. Check the logs for full details about the path and the available space. To improve this please change the temporary directory in the php.ini or make more space available in that path.', - type: OC.SetupChecks.MESSAGE_TYPE_WARNING - }]); - done(); - }); - }); - it('should return an info if there is no default phone region', function(done) { var async = OC.SetupChecks.checkSetup(); @@ -545,7 +504,6 @@ describe('OC.SetupChecks tests', function() { }, JSON.stringify({ isFairUseOfFreePushService: true, - isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true, reverseProxyGeneratedURL: 'https://server', temporaryDirectoryWritable: true, generic: { @@ -586,7 +544,6 @@ describe('OC.SetupChecks tests', function() { }, JSON.stringify({ isFairUseOfFreePushService: true, - isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true, reverseProxyGeneratedURL: 'https://server', temporaryDirectoryWritable: false, generic: { From b92024efa09eb362a9626e1d4a57171948545713 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Thu, 18 Jan 2024 15:32:24 +0100 Subject: [PATCH 10/13] Merge writable temporary space check with the s3 one, and improve MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It will now show available space and path of both PHP and Nextcloud temporary directories if they differ. Signed-off-by: Côme Chilliet --- .../composer/composer/autoload_classmap.php | 2 +- .../composer/composer/autoload_static.php | 2 +- apps/settings/lib/AppInfo/Application.php | 4 +- .../lib/Controller/CheckSetupController.php | 16 ----- ...maryStorage.php => TempSpaceAvailable.php} | 61 ++++++++++++++----- .../Controller/CheckSetupControllerTest.php | 6 -- core/js/setupchecks.js | 6 -- core/js/tests/specs/setupchecksSpec.js | 41 ------------- 8 files changed, 51 insertions(+), 87 deletions(-) rename apps/settings/lib/SetupChecks/{TempSpaceAvailableIfS3PrimaryStorage.php => TempSpaceAvailable.php} (60%) diff --git a/apps/settings/composer/composer/autoload_classmap.php b/apps/settings/composer/composer/autoload_classmap.php index 7166ba2aab0..6e641fab4c6 100644 --- a/apps/settings/composer/composer/autoload_classmap.php +++ b/apps/settings/composer/composer/autoload_classmap.php @@ -110,7 +110,7 @@ return array( 'OCA\\Settings\\SetupChecks\\ReadOnlyConfig' => $baseDir . '/../lib/SetupChecks/ReadOnlyConfig.php', 'OCA\\Settings\\SetupChecks\\SupportedDatabase' => $baseDir . '/../lib/SetupChecks/SupportedDatabase.php', 'OCA\\Settings\\SetupChecks\\SystemIs64bit' => $baseDir . '/../lib/SetupChecks/SystemIs64bit.php', - 'OCA\\Settings\\SetupChecks\\TempSpaceAvailableIfS3PrimaryStorage' => $baseDir . '/../lib/SetupChecks/TempSpaceAvailableIfS3PrimaryStorage.php', + 'OCA\\Settings\\SetupChecks\\TempSpaceAvailable' => $baseDir . '/../lib/SetupChecks/TempSpaceAvailable.php', 'OCA\\Settings\\SetupChecks\\TransactionIsolation' => $baseDir . '/../lib/SetupChecks/TransactionIsolation.php', 'OCA\\Settings\\UserMigration\\AccountMigrator' => $baseDir . '/../lib/UserMigration/AccountMigrator.php', 'OCA\\Settings\\UserMigration\\AccountMigratorException' => $baseDir . '/../lib/UserMigration/AccountMigratorException.php', diff --git a/apps/settings/composer/composer/autoload_static.php b/apps/settings/composer/composer/autoload_static.php index 89517edf9f3..9b775f4358b 100644 --- a/apps/settings/composer/composer/autoload_static.php +++ b/apps/settings/composer/composer/autoload_static.php @@ -125,7 +125,7 @@ class ComposerStaticInitSettings 'OCA\\Settings\\SetupChecks\\ReadOnlyConfig' => __DIR__ . '/..' . '/../lib/SetupChecks/ReadOnlyConfig.php', 'OCA\\Settings\\SetupChecks\\SupportedDatabase' => __DIR__ . '/..' . '/../lib/SetupChecks/SupportedDatabase.php', 'OCA\\Settings\\SetupChecks\\SystemIs64bit' => __DIR__ . '/..' . '/../lib/SetupChecks/SystemIs64bit.php', - 'OCA\\Settings\\SetupChecks\\TempSpaceAvailableIfS3PrimaryStorage' => __DIR__ . '/..' . '/../lib/SetupChecks/TempSpaceAvailableIfS3PrimaryStorage.php', + 'OCA\\Settings\\SetupChecks\\TempSpaceAvailable' => __DIR__ . '/..' . '/../lib/SetupChecks/TempSpaceAvailable.php', 'OCA\\Settings\\SetupChecks\\TransactionIsolation' => __DIR__ . '/..' . '/../lib/SetupChecks/TransactionIsolation.php', 'OCA\\Settings\\UserMigration\\AccountMigrator' => __DIR__ . '/..' . '/../lib/UserMigration/AccountMigrator.php', 'OCA\\Settings\\UserMigration\\AccountMigratorException' => __DIR__ . '/..' . '/../lib/UserMigration/AccountMigratorException.php', diff --git a/apps/settings/lib/AppInfo/Application.php b/apps/settings/lib/AppInfo/Application.php index f029ebf8120..42905cf4cd9 100644 --- a/apps/settings/lib/AppInfo/Application.php +++ b/apps/settings/lib/AppInfo/Application.php @@ -82,7 +82,7 @@ use OCA\Settings\SetupChecks\RandomnessSecure; use OCA\Settings\SetupChecks\ReadOnlyConfig; use OCA\Settings\SetupChecks\SupportedDatabase; use OCA\Settings\SetupChecks\SystemIs64bit; -use OCA\Settings\SetupChecks\TempSpaceAvailableIfS3PrimaryStorage; +use OCA\Settings\SetupChecks\TempSpaceAvailable; use OCA\Settings\SetupChecks\TransactionIsolation; use OCA\Settings\UserMigration\AccountMigrator; use OCA\Settings\WellKnown\ChangePasswordHandler; @@ -203,7 +203,7 @@ class Application extends App implements IBootstrap { $context->registerSetupCheck(ReadOnlyConfig::class); $context->registerSetupCheck(SupportedDatabase::class); $context->registerSetupCheck(SystemIs64bit::class); - $context->registerSetupCheck(TempSpaceAvailableIfS3PrimaryStorage::class); + $context->registerSetupCheck(TempSpaceAvailable::class); $context->registerSetupCheck(TransactionIsolation::class); $context->registerUserMigrator(AccountMigrator::class); diff --git a/apps/settings/lib/Controller/CheckSetupController.php b/apps/settings/lib/Controller/CheckSetupController.php index ba39579e445..323c673b0d0 100644 --- a/apps/settings/lib/Controller/CheckSetupController.php +++ b/apps/settings/lib/Controller/CheckSetupController.php @@ -55,7 +55,6 @@ use OCP\AppFramework\Http\RedirectResponse; use OCP\IConfig; use OCP\IL10N; use OCP\IRequest; -use OCP\ITempManager; use OCP\IURLGenerator; use OCP\Notification\IManager; use OCP\SetupCheck\ISetupCheckManager; @@ -73,8 +72,6 @@ class CheckSetupController extends Controller { private $checker; /** @var LoggerInterface */ private $logger; - /** @var ITempManager */ - private $tempManager; /** @var IManager */ private $manager; private ISetupCheckManager $setupCheckManager; @@ -86,7 +83,6 @@ class CheckSetupController extends Controller { IL10N $l10n, Checker $checker, LoggerInterface $logger, - ITempManager $tempManager, IManager $manager, ISetupCheckManager $setupCheckManager, ) { @@ -96,7 +92,6 @@ class CheckSetupController extends Controller { $this->l10n = $l10n; $this->checker = $checker; $this->logger = $logger; - $this->tempManager = $tempManager; $this->manager = $manager; $this->setupCheckManager = $setupCheckManager; } @@ -192,16 +187,6 @@ Raw output ); } - private function isTemporaryDirectoryWritable(): bool { - try { - if (!empty($this->tempManager->getTempBaseDir())) { - return true; - } - } catch (\Exception $e) { - } - return false; - } - /** * @return DataResponse * @AuthorizedAdminSetting(settings=OCA\Settings\Settings\Admin\Overview) @@ -212,7 +197,6 @@ Raw output 'isFairUseOfFreePushService' => $this->isFairUseOfFreePushService(), 'reverseProxyDocs' => $this->urlGenerator->linkToDocs('admin-reverse-proxy'), 'reverseProxyGeneratedURL' => $this->urlGenerator->getAbsoluteURL('index.php'), - 'temporaryDirectoryWritable' => $this->isTemporaryDirectoryWritable(), 'generic' => $this->setupCheckManager->runAll(), ] ); diff --git a/apps/settings/lib/SetupChecks/TempSpaceAvailableIfS3PrimaryStorage.php b/apps/settings/lib/SetupChecks/TempSpaceAvailable.php similarity index 60% rename from apps/settings/lib/SetupChecks/TempSpaceAvailableIfS3PrimaryStorage.php rename to apps/settings/lib/SetupChecks/TempSpaceAvailable.php index b8d7cf7d3e1..6a8aa1eab51 100644 --- a/apps/settings/lib/SetupChecks/TempSpaceAvailableIfS3PrimaryStorage.php +++ b/apps/settings/lib/SetupChecks/TempSpaceAvailable.php @@ -27,15 +27,17 @@ namespace OCA\Settings\SetupChecks; use OCP\IConfig; use OCP\IL10N; +use OCP\ITempManager; use OCP\IURLGenerator; use OCP\SetupCheck\ISetupCheck; use OCP\SetupCheck\SetupResult; -class TempSpaceAvailableIfS3PrimaryStorage implements ISetupCheck { +class TempSpaceAvailable implements ISetupCheck { public function __construct( private IL10N $l10n, private IConfig $config, private IURLGenerator $urlGenerator, + private ITempManager $tempManager, ) { } @@ -47,38 +49,69 @@ class TempSpaceAvailableIfS3PrimaryStorage implements ISetupCheck { return 'system'; } - public function run(): SetupResult { + private function isPrimaryStorageS3(): bool { $objectStore = $this->config->getSystemValue('objectstore', null); $objectStoreMultibucket = $this->config->getSystemValue('objectstore_multibucket', null); - // TODO we should check and display temp space available even if not s3 if (!isset($objectStoreMultibucket) && !isset($objectStore)) { - return SetupResult::success($this->l10n->t('This instance does not use an S3 based object store as primary storage')); + return false; } if (isset($objectStoreMultibucket['class']) && $objectStoreMultibucket['class'] !== 'OC\\Files\\ObjectStore\\S3') { - return SetupResult::success($this->l10n->t('This instance does not use an S3 based object store as primary storage')); + return false; } if (isset($objectStore['class']) && $objectStore['class'] !== 'OC\\Files\\ObjectStore\\S3') { - return SetupResult::success($this->l10n->t('This instance does not use an S3 based object store as primary storage')); + return false; } - $tempPath = sys_get_temp_dir(); - if (!is_dir($tempPath)) { - return SetupResult::error($this->l10n->t('Error while checking the temporary PHP path - it was not properly set to a directory. Returned value: %s', [$tempPath])); + return true; + } + + public function run(): SetupResult { + $phpTempPath = sys_get_temp_dir(); + $nextcloudTempPath = ''; + try { + $nextcloudTempPath = $this->tempManager->getTempBaseDir(); + } catch (\Exception $e) { } - $freeSpaceInTemp = function_exists('disk_free_space') ? disk_free_space($tempPath) : false; + + if (empty($nextcloudTempPath)) { + return SetupResult::error('The temporary directory of this instance points to an either non-existing or non-writable directory.'); + } + + if (!is_dir($phpTempPath)) { + return SetupResult::error($this->l10n->t('Error while checking the temporary PHP path - it was not properly set to a directory. Returned value: %s', [$phpTempPath])); + } + + $freeSpaceInTemp = function_exists('disk_free_space') ? disk_free_space($phpTempPath) : false; if ($freeSpaceInTemp === false) { - return SetupResult::error($this->l10n->t('Error while checking the available disk space of temporary PHP path or no free disk space returned. Temporary path: %s', [$tempPath])); + return SetupResult::error($this->l10n->t('Error while checking the available disk space of temporary PHP path or no free disk space returned. Temporary path: %s', [$phpTempPath])); } + /** Build details data about temporary directory, either one or two of them */ $freeSpaceInTempInGB = $freeSpaceInTemp / 1024 / 1024 / 1024; + $spaceDetail = $this->l10n->t('- %.1f GiB available in %s (PHP temporary directory)', [round($freeSpaceInTempInGB, 1),$phpTempPath]); + if ($nextcloudTempPath !== $phpTempPath) { + $freeSpaceInNextcloudTemp = function_exists('disk_free_space') ? disk_free_space($nextcloudTempPath) : false; + if ($freeSpaceInNextcloudTemp === false) { + return SetupResult::error($this->l10n->t('Error while checking the available disk space of temporary PHP path or no free disk space returned. Temporary path: %s', [$nextcloudTempPath])); + } + $freeSpaceInNextcloudTempInGB = $freeSpaceInNextcloudTemp / 1024 / 1024 / 1024; + $spaceDetail .= "\n".$this->l10n->t('- %.1f GiB available in %s (Nextcloud temporary directory)', [round($freeSpaceInNextcloudTempInGB, 1),$nextcloudTempPath]); + } + + if (!$this->isPrimaryStorageS3()) { + return SetupResult::success( + $this->l10n->t("Temporary directory is correctly configured:\n%s", [$spaceDetail]) + ); + } + if ($freeSpaceInTempInGB > 50) { return SetupResult::success( $this->l10n->t( - "This instance uses an S3 based object store as primary storage, and has enough space in the temporary directory.\nAvailable: %.1f GiB\nPath: %s", - [round($freeSpaceInTempInGB, 1),$tempPath] + "This instance uses an S3 based object store as primary storage, and has enough space in the temporary directory.\n%s", + [$spaceDetail] ) ); } @@ -86,7 +119,7 @@ class TempSpaceAvailableIfS3PrimaryStorage implements ISetupCheck { return SetupResult::warning( $this->l10n->t( "This instance uses an S3 based object store as primary storage. The uploaded files are stored temporarily on the server and thus it is recommended to have 50 GiB of free space available in the temp directory of PHP. To improve this please change the temporary directory in the php.ini or make more space available in that path. \nChecking the available space in the temporary path resulted in %.1f GiB instead of the recommended 50 GiB. Path: %s", - [round($freeSpaceInTempInGB, 1),$tempPath] + [round($freeSpaceInTempInGB, 1),$phpTempPath] ) ); } diff --git a/apps/settings/tests/Controller/CheckSetupControllerTest.php b/apps/settings/tests/Controller/CheckSetupControllerTest.php index f8c9b346e7d..c83273b467d 100644 --- a/apps/settings/tests/Controller/CheckSetupControllerTest.php +++ b/apps/settings/tests/Controller/CheckSetupControllerTest.php @@ -43,7 +43,6 @@ use OCP\AppFramework\Http\RedirectResponse; use OCP\IConfig; use OCP\IL10N; use OCP\IRequest; -use OCP\ITempManager; use OCP\IURLGenerator; use OCP\Notification\IManager; use OCP\SetupCheck\ISetupCheckManager; @@ -72,8 +71,6 @@ class CheckSetupControllerTest extends TestCase { private $logger; /** @var Checker|\PHPUnit\Framework\MockObject\MockObject */ private $checker; - /** @var ITempManager|\PHPUnit\Framework\MockObject\MockObject */ - private $tempManager; /** @var IManager|\PHPUnit\Framework\MockObject\MockObject */ private $notificationManager; /** @var ISetupCheckManager|MockObject */ @@ -98,7 +95,6 @@ class CheckSetupControllerTest extends TestCase { $this->checker = $this->getMockBuilder('\OC\IntegrityCheck\Checker') ->disableOriginalConstructor()->getMock(); $this->logger = $this->getMockBuilder(LoggerInterface::class)->getMock(); - $this->tempManager = $this->getMockBuilder(ITempManager::class)->getMock(); $this->notificationManager = $this->getMockBuilder(IManager::class)->getMock(); $this->setupCheckManager = $this->createMock(ISetupCheckManager::class); $this->checkSetupController = $this->getMockBuilder(CheckSetupController::class) @@ -110,7 +106,6 @@ class CheckSetupControllerTest extends TestCase { $this->l10n, $this->checker, $this->logger, - $this->tempManager, $this->notificationManager, $this->setupCheckManager, ]) @@ -176,7 +171,6 @@ class CheckSetupControllerTest extends TestCase { 'reverseProxyDocs' => 'reverse-proxy-doc-link', 'reverseProxyGeneratedURL' => 'https://server/index.php', 'isFairUseOfFreePushService' => false, - 'temporaryDirectoryWritable' => false, 'generic' => [], ] ); diff --git a/core/js/setupchecks.js b/core/js/setupchecks.js index 7e068ffd1d0..7864169dd2b 100644 --- a/core/js/setupchecks.js +++ b/core/js/setupchecks.js @@ -189,12 +189,6 @@ }); } - if (!data.temporaryDirectoryWritable) { - messages.push({ - msg: t('core', 'The temporary directory of this instance points to an either non-existing or non-writable directory.'), - type: OC.SetupChecks.MESSAGE_TYPE_WARNING - }) - } if (window.location.protocol === 'https:' && data.reverseProxyGeneratedURL.split('/')[0] !== 'https:') { messages.push({ msg: t('core', 'You are accessing your instance over a secure connection, however your instance is generating insecure URLs. This most likely means that you are behind a reverse proxy and the overwrite config variables are not set correctly. Please read {linkstart}the documentation page about this ↗{linkend}.') diff --git a/core/js/tests/specs/setupchecksSpec.js b/core/js/tests/specs/setupchecksSpec.js index 6a4f68b977d..b2e1baa618b 100644 --- a/core/js/tests/specs/setupchecksSpec.js +++ b/core/js/tests/specs/setupchecksSpec.js @@ -225,7 +225,6 @@ describe('OC.SetupChecks tests', function() { JSON.stringify({ isFairUseOfFreePushService: true, reverseProxyGeneratedURL: 'https://server', - temporaryDirectoryWritable: true, generic: { network: { "Internet connectivity": { @@ -260,7 +259,6 @@ describe('OC.SetupChecks tests', function() { JSON.stringify({ isFairUseOfFreePushService: true, reverseProxyGeneratedURL: 'https://server', - temporaryDirectoryWritable: true, generic: { network: { "Internet connectivity": { @@ -295,7 +293,6 @@ describe('OC.SetupChecks tests', function() { JSON.stringify({ isFairUseOfFreePushService: true, reverseProxyGeneratedURL: 'https://server', - temporaryDirectoryWritable: true, generic: { network: { "Internet connectivity": { @@ -331,7 +328,6 @@ describe('OC.SetupChecks tests', function() { isFairUseOfFreePushService: true, reverseProxyDocs: 'https://docs.nextcloud.com/foo/bar.html', reverseProxyGeneratedURL: 'https://server', - temporaryDirectoryWritable: true, generic: { network: { "Internet connectivity": { @@ -396,7 +392,6 @@ describe('OC.SetupChecks tests', function() { JSON.stringify({ isFairUseOfFreePushService: true, reverseProxyGeneratedURL: 'https://server', - temporaryDirectoryWritable: true, generic: { network: { "Internet connectivity": { @@ -441,7 +436,6 @@ describe('OC.SetupChecks tests', function() { isFairUseOfFreePushService: true, reverseProxyDocs: 'https://docs.nextcloud.com/foo/bar.html', reverseProxyGeneratedURL: 'http://server', - temporaryDirectoryWritable: true, generic: { network: { "Internet connectivity": { @@ -475,7 +469,6 @@ describe('OC.SetupChecks tests', function() { isFairUseOfFreePushService: true, reverseProxyDocs: 'https://docs.nextcloud.com/foo/bar.html', reverseProxyGeneratedURL: 'http://server', - temporaryDirectoryWritable: true, generic: { network: { "Internet connectivity": { @@ -505,7 +498,6 @@ describe('OC.SetupChecks tests', function() { JSON.stringify({ isFairUseOfFreePushService: true, reverseProxyGeneratedURL: 'https://server', - temporaryDirectoryWritable: true, generic: { network: { "Internet connectivity": { @@ -533,39 +525,6 @@ describe('OC.SetupChecks tests', function() { done(); }); }); - - it('should return an info if the temporary directory is either non-existent or non-writable', function(done) { - var async = OC.SetupChecks.checkSetup(); - - suite.server.requests[0].respond( - 200, - { - 'Content-Type': 'application/json', - }, - JSON.stringify({ - isFairUseOfFreePushService: true, - reverseProxyGeneratedURL: 'https://server', - temporaryDirectoryWritable: false, - generic: { - network: { - "Internet connectivity": { - severity: "success", - description: null, - linkToDoc: null - } - }, - }, - }) - ); - - async.done(function( data, s, x ){ - expect(data).toEqual([{ - msg: 'The temporary directory of this instance points to an either non-existing or non-writable directory.', - type: OC.SetupChecks.MESSAGE_TYPE_WARNING - }]); - done(); - }); - }); }); describe('checkGeneric', function() { From 44c0b7d9b4fefa96cab08d506556a71440680e42 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Thu, 18 Jan 2024 15:53:03 +0100 Subject: [PATCH 11/13] Migrate fair use of free push service check to new SetupCheck API MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- .../composer/composer/autoload_classmap.php | 1 + .../composer/composer/autoload_static.php | 1 + apps/settings/lib/AppInfo/Application.php | 4 + .../lib/Controller/CheckSetupController.php | 19 ----- .../SetupChecks/FairUseOfFreePushService.php | 79 +++++++++++++++++++ .../Controller/CheckSetupControllerTest.php | 6 -- core/js/setupchecks.js | 9 --- core/js/tests/specs/setupchecksSpec.js | 8 -- 8 files changed, 85 insertions(+), 42 deletions(-) create mode 100644 apps/settings/lib/SetupChecks/FairUseOfFreePushService.php diff --git a/apps/settings/composer/composer/autoload_classmap.php b/apps/settings/composer/composer/autoload_classmap.php index 6e641fab4c6..866c27f0f25 100644 --- a/apps/settings/composer/composer/autoload_classmap.php +++ b/apps/settings/composer/composer/autoload_classmap.php @@ -88,6 +88,7 @@ return array( 'OCA\\Settings\\SetupChecks\\DatabasePendingBigIntConversions' => $baseDir . '/../lib/SetupChecks/DatabasePendingBigIntConversions.php', 'OCA\\Settings\\SetupChecks\\DefaultPhoneRegionSet' => $baseDir . '/../lib/SetupChecks/DefaultPhoneRegionSet.php', 'OCA\\Settings\\SetupChecks\\EmailTestSuccessful' => $baseDir . '/../lib/SetupChecks/EmailTestSuccessful.php', + 'OCA\\Settings\\SetupChecks\\FairUseOfFreePushService' => $baseDir . '/../lib/SetupChecks/FairUseOfFreePushService.php', 'OCA\\Settings\\SetupChecks\\FileLocking' => $baseDir . '/../lib/SetupChecks/FileLocking.php', 'OCA\\Settings\\SetupChecks\\ForwardedForHeaders' => $baseDir . '/../lib/SetupChecks/ForwardedForHeaders.php', 'OCA\\Settings\\SetupChecks\\InternetConnectivity' => $baseDir . '/../lib/SetupChecks/InternetConnectivity.php', diff --git a/apps/settings/composer/composer/autoload_static.php b/apps/settings/composer/composer/autoload_static.php index 9b775f4358b..99dd68250c1 100644 --- a/apps/settings/composer/composer/autoload_static.php +++ b/apps/settings/composer/composer/autoload_static.php @@ -103,6 +103,7 @@ class ComposerStaticInitSettings 'OCA\\Settings\\SetupChecks\\DatabasePendingBigIntConversions' => __DIR__ . '/..' . '/../lib/SetupChecks/DatabasePendingBigIntConversions.php', 'OCA\\Settings\\SetupChecks\\DefaultPhoneRegionSet' => __DIR__ . '/..' . '/../lib/SetupChecks/DefaultPhoneRegionSet.php', 'OCA\\Settings\\SetupChecks\\EmailTestSuccessful' => __DIR__ . '/..' . '/../lib/SetupChecks/EmailTestSuccessful.php', + 'OCA\\Settings\\SetupChecks\\FairUseOfFreePushService' => __DIR__ . '/..' . '/../lib/SetupChecks/FairUseOfFreePushService.php', 'OCA\\Settings\\SetupChecks\\FileLocking' => __DIR__ . '/..' . '/../lib/SetupChecks/FileLocking.php', 'OCA\\Settings\\SetupChecks\\ForwardedForHeaders' => __DIR__ . '/..' . '/../lib/SetupChecks/ForwardedForHeaders.php', 'OCA\\Settings\\SetupChecks\\InternetConnectivity' => __DIR__ . '/..' . '/../lib/SetupChecks/InternetConnectivity.php', diff --git a/apps/settings/lib/AppInfo/Application.php b/apps/settings/lib/AppInfo/Application.php index 42905cf4cd9..11c7b9a0374 100644 --- a/apps/settings/lib/AppInfo/Application.php +++ b/apps/settings/lib/AppInfo/Application.php @@ -60,6 +60,7 @@ use OCA\Settings\SetupChecks\DatabaseHasMissingPrimaryKeys; use OCA\Settings\SetupChecks\DatabasePendingBigIntConversions; use OCA\Settings\SetupChecks\DefaultPhoneRegionSet; use OCA\Settings\SetupChecks\EmailTestSuccessful; +use OCA\Settings\SetupChecks\FairUseOfFreePushService; use OCA\Settings\SetupChecks\FileLocking; use OCA\Settings\SetupChecks\ForwardedForHeaders; use OCA\Settings\SetupChecks\InternetConnectivity; @@ -205,6 +206,9 @@ class Application extends App implements IBootstrap { $context->registerSetupCheck(SystemIs64bit::class); $context->registerSetupCheck(TempSpaceAvailable::class); $context->registerSetupCheck(TransactionIsolation::class); + if (!\OCP\Server::get(\OCP\Support\Subscription\IRegistry::class)->delegateHasValidSubscription()) { + $context->registerSetupCheck(FairUseOfFreePushService::class); + } $context->registerUserMigrator(AccountMigrator::class); } diff --git a/apps/settings/lib/Controller/CheckSetupController.php b/apps/settings/lib/Controller/CheckSetupController.php index 323c673b0d0..80083518385 100644 --- a/apps/settings/lib/Controller/CheckSetupController.php +++ b/apps/settings/lib/Controller/CheckSetupController.php @@ -56,7 +56,6 @@ use OCP\IConfig; use OCP\IL10N; use OCP\IRequest; use OCP\IURLGenerator; -use OCP\Notification\IManager; use OCP\SetupCheck\ISetupCheckManager; use Psr\Log\LoggerInterface; @@ -72,8 +71,6 @@ class CheckSetupController extends Controller { private $checker; /** @var LoggerInterface */ private $logger; - /** @var IManager */ - private $manager; private ISetupCheckManager $setupCheckManager; public function __construct($AppName, @@ -83,7 +80,6 @@ class CheckSetupController extends Controller { IL10N $l10n, Checker $checker, LoggerInterface $logger, - IManager $manager, ISetupCheckManager $setupCheckManager, ) { parent::__construct($AppName, $request); @@ -92,7 +88,6 @@ class CheckSetupController extends Controller { $this->l10n = $l10n; $this->checker = $checker; $this->logger = $logger; - $this->manager = $manager; $this->setupCheckManager = $setupCheckManager; } @@ -105,19 +100,6 @@ class CheckSetupController extends Controller { return new DataResponse($this->setupCheckManager->runAll()); } - /** - * Check if is fair use of free push service - * @return bool - */ - private function isFairUseOfFreePushService(): bool { - $rateLimitReached = (int) $this->config->getAppValue('notifications', 'rate_limit_reached', '0'); - if ($rateLimitReached >= (time() - 7 * 24 * 3600)) { - // Notifications app is showing a message already - return true; - } - return $this->manager->isFairUseOfFreePushService(); - } - /** * @NoCSRFRequired * @return RedirectResponse @@ -194,7 +176,6 @@ Raw output public function check() { return new DataResponse( [ - 'isFairUseOfFreePushService' => $this->isFairUseOfFreePushService(), 'reverseProxyDocs' => $this->urlGenerator->linkToDocs('admin-reverse-proxy'), 'reverseProxyGeneratedURL' => $this->urlGenerator->getAbsoluteURL('index.php'), 'generic' => $this->setupCheckManager->runAll(), diff --git a/apps/settings/lib/SetupChecks/FairUseOfFreePushService.php b/apps/settings/lib/SetupChecks/FairUseOfFreePushService.php new file mode 100644 index 00000000000..431d1be4d6a --- /dev/null +++ b/apps/settings/lib/SetupChecks/FairUseOfFreePushService.php @@ -0,0 +1,79 @@ + + * + * @author Côme Chilliet + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ +namespace OCA\Settings\SetupChecks; + +use OCP\IConfig; +use OCP\IL10N; +use OCP\Notification\IManager; +use OCP\SetupCheck\ISetupCheck; +use OCP\SetupCheck\SetupResult; + +class FairUseOfFreePushService implements ISetupCheck { + public function __construct( + private IL10N $l10n, + private IConfig $config, + private IManager $notificationsManager, + ) { + } + + public function getName(): string { + return $this->l10n->t('Free push service'); + } + + public function getCategory(): string { + return 'system'; + } + + /** + * Check if is fair use of free push service + */ + private function isFairUseOfFreePushService(): bool { + $rateLimitReached = (int) $this->config->getAppValue('notifications', 'rate_limit_reached', '0'); + if ($rateLimitReached >= (time() - 7 * 24 * 3600)) { + // Notifications app is showing a message already + return true; + } + return $this->notificationsManager->isFairUseOfFreePushService(); + } + + public function run(): SetupResult { + if ($this->isFairUseOfFreePushService()) { + return SetupResult::success(); + } + + return SetupResult::error( + $this->l10n->t('This is the unsupported community build of Nextcloud. Given the size of this instance, performance, reliability and scalability cannot be guaranteed. Push notifications are limited to avoid overloading our free service. Learn more about the benefits of Nextcloud Enterprise at {link}.'), + descriptionParameters:[ + 'link' => [ + 'type' => 'highlight', + 'id' => 'link', + 'name' => 'https://nextcloud.com/enterprise', + 'link' => 'https://nextcloud.com/enterprise', + ], + ], + ); + } +} diff --git a/apps/settings/tests/Controller/CheckSetupControllerTest.php b/apps/settings/tests/Controller/CheckSetupControllerTest.php index c83273b467d..924a76fa4f5 100644 --- a/apps/settings/tests/Controller/CheckSetupControllerTest.php +++ b/apps/settings/tests/Controller/CheckSetupControllerTest.php @@ -44,7 +44,6 @@ use OCP\IConfig; use OCP\IL10N; use OCP\IRequest; use OCP\IURLGenerator; -use OCP\Notification\IManager; use OCP\SetupCheck\ISetupCheckManager; use PHPUnit\Framework\MockObject\MockObject; use Psr\Log\LoggerInterface; @@ -71,8 +70,6 @@ class CheckSetupControllerTest extends TestCase { private $logger; /** @var Checker|\PHPUnit\Framework\MockObject\MockObject */ private $checker; - /** @var IManager|\PHPUnit\Framework\MockObject\MockObject */ - private $notificationManager; /** @var ISetupCheckManager|MockObject */ private $setupCheckManager; @@ -95,7 +92,6 @@ class CheckSetupControllerTest extends TestCase { $this->checker = $this->getMockBuilder('\OC\IntegrityCheck\Checker') ->disableOriginalConstructor()->getMock(); $this->logger = $this->getMockBuilder(LoggerInterface::class)->getMock(); - $this->notificationManager = $this->getMockBuilder(IManager::class)->getMock(); $this->setupCheckManager = $this->createMock(ISetupCheckManager::class); $this->checkSetupController = $this->getMockBuilder(CheckSetupController::class) ->setConstructorArgs([ @@ -106,7 +102,6 @@ class CheckSetupControllerTest extends TestCase { $this->l10n, $this->checker, $this->logger, - $this->notificationManager, $this->setupCheckManager, ]) ->setMethods([ @@ -170,7 +165,6 @@ class CheckSetupControllerTest extends TestCase { [ 'reverseProxyDocs' => 'reverse-proxy-doc-link', 'reverseProxyGeneratedURL' => 'https://server/index.php', - 'isFairUseOfFreePushService' => false, 'generic' => [], ] ); diff --git a/core/js/setupchecks.js b/core/js/setupchecks.js index 7864169dd2b..7400d02d30d 100644 --- a/core/js/setupchecks.js +++ b/core/js/setupchecks.js @@ -180,15 +180,6 @@ var afterCall = function(data, statusText, xhr) { var messages = []; if (xhr.status === 200 && data) { - if (!data.isFairUseOfFreePushService) { - messages.push({ - msg: t('core', 'This is the unsupported community build of Nextcloud. Given the size of this instance, performance, reliability and scalability cannot be guaranteed. Push notifications are limited to avoid overloading our free service. Learn more about the benefits of Nextcloud Enterprise at {linkstart}https://nextcloud.com/enterprise{linkend}.') - .replace('{linkstart}', '') - .replace('{linkend}', ''), - type: OC.SetupChecks.MESSAGE_TYPE_ERROR - }); - } - if (window.location.protocol === 'https:' && data.reverseProxyGeneratedURL.split('/')[0] !== 'https:') { messages.push({ msg: t('core', 'You are accessing your instance over a secure connection, however your instance is generating insecure URLs. This most likely means that you are behind a reverse proxy and the overwrite config variables are not set correctly. Please read {linkstart}the documentation page about this ↗{linkend}.') diff --git a/core/js/tests/specs/setupchecksSpec.js b/core/js/tests/specs/setupchecksSpec.js index b2e1baa618b..9ac31ce8acb 100644 --- a/core/js/tests/specs/setupchecksSpec.js +++ b/core/js/tests/specs/setupchecksSpec.js @@ -223,7 +223,6 @@ describe('OC.SetupChecks tests', function() { 'Content-Type': 'application/json' }, JSON.stringify({ - isFairUseOfFreePushService: true, reverseProxyGeneratedURL: 'https://server', generic: { network: { @@ -257,7 +256,6 @@ describe('OC.SetupChecks tests', function() { 'Content-Type': 'application/json' }, JSON.stringify({ - isFairUseOfFreePushService: true, reverseProxyGeneratedURL: 'https://server', generic: { network: { @@ -291,7 +289,6 @@ describe('OC.SetupChecks tests', function() { 'Content-Type': 'application/json', }, JSON.stringify({ - isFairUseOfFreePushService: true, reverseProxyGeneratedURL: 'https://server', generic: { network: { @@ -325,7 +322,6 @@ describe('OC.SetupChecks tests', function() { 'Content-Type': 'application/json', }, JSON.stringify({ - isFairUseOfFreePushService: true, reverseProxyDocs: 'https://docs.nextcloud.com/foo/bar.html', reverseProxyGeneratedURL: 'https://server', generic: { @@ -390,7 +386,6 @@ describe('OC.SetupChecks tests', function() { 'Content-Type': 'application/json', }, JSON.stringify({ - isFairUseOfFreePushService: true, reverseProxyGeneratedURL: 'https://server', generic: { network: { @@ -433,7 +428,6 @@ describe('OC.SetupChecks tests', function() { 'Content-Type': 'application/json', }, JSON.stringify({ - isFairUseOfFreePushService: true, reverseProxyDocs: 'https://docs.nextcloud.com/foo/bar.html', reverseProxyGeneratedURL: 'http://server', generic: { @@ -466,7 +460,6 @@ describe('OC.SetupChecks tests', function() { 'Content-Type': 'application/json', }, JSON.stringify({ - isFairUseOfFreePushService: true, reverseProxyDocs: 'https://docs.nextcloud.com/foo/bar.html', reverseProxyGeneratedURL: 'http://server', generic: { @@ -496,7 +489,6 @@ describe('OC.SetupChecks tests', function() { 'Content-Type': 'application/json', }, JSON.stringify({ - isFairUseOfFreePushService: true, reverseProxyGeneratedURL: 'https://server', generic: { network: { From 1ad788d5f0eb2edbbf468f63aa4cf61f2b46f1e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Thu, 25 Jan 2024 15:23:54 +0100 Subject: [PATCH 12/13] Rename to "Push service" and enable on all instances to avoid trouble MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- .../composer/composer/autoload_classmap.php | 2 +- .../composer/composer/autoload_static.php | 2 +- apps/settings/lib/AppInfo/Application.php | 6 ++---- ...rUseOfFreePushService.php => PushService.php} | 16 ++++++++++++---- 4 files changed, 16 insertions(+), 10 deletions(-) rename apps/settings/lib/SetupChecks/{FairUseOfFreePushService.php => PushService.php} (80%) diff --git a/apps/settings/composer/composer/autoload_classmap.php b/apps/settings/composer/composer/autoload_classmap.php index 866c27f0f25..0d8e177c1fd 100644 --- a/apps/settings/composer/composer/autoload_classmap.php +++ b/apps/settings/composer/composer/autoload_classmap.php @@ -88,7 +88,6 @@ return array( 'OCA\\Settings\\SetupChecks\\DatabasePendingBigIntConversions' => $baseDir . '/../lib/SetupChecks/DatabasePendingBigIntConversions.php', 'OCA\\Settings\\SetupChecks\\DefaultPhoneRegionSet' => $baseDir . '/../lib/SetupChecks/DefaultPhoneRegionSet.php', 'OCA\\Settings\\SetupChecks\\EmailTestSuccessful' => $baseDir . '/../lib/SetupChecks/EmailTestSuccessful.php', - 'OCA\\Settings\\SetupChecks\\FairUseOfFreePushService' => $baseDir . '/../lib/SetupChecks/FairUseOfFreePushService.php', 'OCA\\Settings\\SetupChecks\\FileLocking' => $baseDir . '/../lib/SetupChecks/FileLocking.php', 'OCA\\Settings\\SetupChecks\\ForwardedForHeaders' => $baseDir . '/../lib/SetupChecks/ForwardedForHeaders.php', 'OCA\\Settings\\SetupChecks\\InternetConnectivity' => $baseDir . '/../lib/SetupChecks/InternetConnectivity.php', @@ -107,6 +106,7 @@ return array( 'OCA\\Settings\\SetupChecks\\PhpOpcacheSetup' => $baseDir . '/../lib/SetupChecks/PhpOpcacheSetup.php', 'OCA\\Settings\\SetupChecks\\PhpOutdated' => $baseDir . '/../lib/SetupChecks/PhpOutdated.php', 'OCA\\Settings\\SetupChecks\\PhpOutputBuffering' => $baseDir . '/../lib/SetupChecks/PhpOutputBuffering.php', + 'OCA\\Settings\\SetupChecks\\PushService' => $baseDir . '/../lib/SetupChecks/PushService.php', 'OCA\\Settings\\SetupChecks\\RandomnessSecure' => $baseDir . '/../lib/SetupChecks/RandomnessSecure.php', 'OCA\\Settings\\SetupChecks\\ReadOnlyConfig' => $baseDir . '/../lib/SetupChecks/ReadOnlyConfig.php', 'OCA\\Settings\\SetupChecks\\SupportedDatabase' => $baseDir . '/../lib/SetupChecks/SupportedDatabase.php', diff --git a/apps/settings/composer/composer/autoload_static.php b/apps/settings/composer/composer/autoload_static.php index 99dd68250c1..1b95d35a906 100644 --- a/apps/settings/composer/composer/autoload_static.php +++ b/apps/settings/composer/composer/autoload_static.php @@ -103,7 +103,6 @@ class ComposerStaticInitSettings 'OCA\\Settings\\SetupChecks\\DatabasePendingBigIntConversions' => __DIR__ . '/..' . '/../lib/SetupChecks/DatabasePendingBigIntConversions.php', 'OCA\\Settings\\SetupChecks\\DefaultPhoneRegionSet' => __DIR__ . '/..' . '/../lib/SetupChecks/DefaultPhoneRegionSet.php', 'OCA\\Settings\\SetupChecks\\EmailTestSuccessful' => __DIR__ . '/..' . '/../lib/SetupChecks/EmailTestSuccessful.php', - 'OCA\\Settings\\SetupChecks\\FairUseOfFreePushService' => __DIR__ . '/..' . '/../lib/SetupChecks/FairUseOfFreePushService.php', 'OCA\\Settings\\SetupChecks\\FileLocking' => __DIR__ . '/..' . '/../lib/SetupChecks/FileLocking.php', 'OCA\\Settings\\SetupChecks\\ForwardedForHeaders' => __DIR__ . '/..' . '/../lib/SetupChecks/ForwardedForHeaders.php', 'OCA\\Settings\\SetupChecks\\InternetConnectivity' => __DIR__ . '/..' . '/../lib/SetupChecks/InternetConnectivity.php', @@ -122,6 +121,7 @@ class ComposerStaticInitSettings 'OCA\\Settings\\SetupChecks\\PhpOpcacheSetup' => __DIR__ . '/..' . '/../lib/SetupChecks/PhpOpcacheSetup.php', 'OCA\\Settings\\SetupChecks\\PhpOutdated' => __DIR__ . '/..' . '/../lib/SetupChecks/PhpOutdated.php', 'OCA\\Settings\\SetupChecks\\PhpOutputBuffering' => __DIR__ . '/..' . '/../lib/SetupChecks/PhpOutputBuffering.php', + 'OCA\\Settings\\SetupChecks\\PushService' => __DIR__ . '/..' . '/../lib/SetupChecks/PushService.php', 'OCA\\Settings\\SetupChecks\\RandomnessSecure' => __DIR__ . '/..' . '/../lib/SetupChecks/RandomnessSecure.php', 'OCA\\Settings\\SetupChecks\\ReadOnlyConfig' => __DIR__ . '/..' . '/../lib/SetupChecks/ReadOnlyConfig.php', 'OCA\\Settings\\SetupChecks\\SupportedDatabase' => __DIR__ . '/..' . '/../lib/SetupChecks/SupportedDatabase.php', diff --git a/apps/settings/lib/AppInfo/Application.php b/apps/settings/lib/AppInfo/Application.php index 11c7b9a0374..4581af12e1c 100644 --- a/apps/settings/lib/AppInfo/Application.php +++ b/apps/settings/lib/AppInfo/Application.php @@ -60,7 +60,6 @@ use OCA\Settings\SetupChecks\DatabaseHasMissingPrimaryKeys; use OCA\Settings\SetupChecks\DatabasePendingBigIntConversions; use OCA\Settings\SetupChecks\DefaultPhoneRegionSet; use OCA\Settings\SetupChecks\EmailTestSuccessful; -use OCA\Settings\SetupChecks\FairUseOfFreePushService; use OCA\Settings\SetupChecks\FileLocking; use OCA\Settings\SetupChecks\ForwardedForHeaders; use OCA\Settings\SetupChecks\InternetConnectivity; @@ -79,6 +78,7 @@ use OCA\Settings\SetupChecks\PhpModules; use OCA\Settings\SetupChecks\PhpOpcacheSetup; use OCA\Settings\SetupChecks\PhpOutdated; use OCA\Settings\SetupChecks\PhpOutputBuffering; +use OCA\Settings\SetupChecks\PushService; use OCA\Settings\SetupChecks\RandomnessSecure; use OCA\Settings\SetupChecks\ReadOnlyConfig; use OCA\Settings\SetupChecks\SupportedDatabase; @@ -206,9 +206,7 @@ class Application extends App implements IBootstrap { $context->registerSetupCheck(SystemIs64bit::class); $context->registerSetupCheck(TempSpaceAvailable::class); $context->registerSetupCheck(TransactionIsolation::class); - if (!\OCP\Server::get(\OCP\Support\Subscription\IRegistry::class)->delegateHasValidSubscription()) { - $context->registerSetupCheck(FairUseOfFreePushService::class); - } + $context->registerSetupCheck(PushService::class); $context->registerUserMigrator(AccountMigrator::class); } diff --git a/apps/settings/lib/SetupChecks/FairUseOfFreePushService.php b/apps/settings/lib/SetupChecks/PushService.php similarity index 80% rename from apps/settings/lib/SetupChecks/FairUseOfFreePushService.php rename to apps/settings/lib/SetupChecks/PushService.php index 431d1be4d6a..3f9f99380e8 100644 --- a/apps/settings/lib/SetupChecks/FairUseOfFreePushService.php +++ b/apps/settings/lib/SetupChecks/PushService.php @@ -25,22 +25,26 @@ declare(strict_types=1); */ namespace OCA\Settings\SetupChecks; +use OCP\AppFramework\Utility\ITimeFactory; use OCP\IConfig; use OCP\IL10N; use OCP\Notification\IManager; use OCP\SetupCheck\ISetupCheck; use OCP\SetupCheck\SetupResult; +use OCP\Support\Subscription\IRegistry; -class FairUseOfFreePushService implements ISetupCheck { +class PushService implements ISetupCheck { public function __construct( private IL10N $l10n, private IConfig $config, private IManager $notificationsManager, + private IRegistry $subscriptionRegistry, + private ITimeFactory $timeFactory, ) { } public function getName(): string { - return $this->l10n->t('Free push service'); + return $this->l10n->t('Push service'); } public function getCategory(): string { @@ -52,7 +56,7 @@ class FairUseOfFreePushService implements ISetupCheck { */ private function isFairUseOfFreePushService(): bool { $rateLimitReached = (int) $this->config->getAppValue('notifications', 'rate_limit_reached', '0'); - if ($rateLimitReached >= (time() - 7 * 24 * 3600)) { + if ($rateLimitReached >= ($this->timeFactory->now()->getTimestamp() - 7 * 24 * 3600)) { // Notifications app is showing a message already return true; } @@ -60,8 +64,12 @@ class FairUseOfFreePushService implements ISetupCheck { } public function run(): SetupResult { + if ($this->subscriptionRegistry->delegateHasValidSubscription()) { + return SetupResult::success($this->l10n->t('Valid enterprise license')); + } + if ($this->isFairUseOfFreePushService()) { - return SetupResult::success(); + return SetupResult::success($this->l10n->t('Free push service')); } return SetupResult::error( From 71e051079b9909e5184270121e2d8e1174e8f8ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Thu, 18 Jan 2024 16:09:11 +0100 Subject: [PATCH 13/13] Migrate debug mode check to new SetupCheck API MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- .../composer/composer/autoload_classmap.php | 1 + .../composer/composer/autoload_static.php | 1 + apps/settings/lib/AppInfo/Application.php | 2 + apps/settings/lib/SetupChecks/DebugMode.php | 55 +++++++++++++++++++ core/js/setupchecks.js | 6 -- 5 files changed, 59 insertions(+), 6 deletions(-) create mode 100644 apps/settings/lib/SetupChecks/DebugMode.php diff --git a/apps/settings/composer/composer/autoload_classmap.php b/apps/settings/composer/composer/autoload_classmap.php index 0d8e177c1fd..ef81604ca0e 100644 --- a/apps/settings/composer/composer/autoload_classmap.php +++ b/apps/settings/composer/composer/autoload_classmap.php @@ -86,6 +86,7 @@ return array( 'OCA\\Settings\\SetupChecks\\DatabaseHasMissingIndices' => $baseDir . '/../lib/SetupChecks/DatabaseHasMissingIndices.php', 'OCA\\Settings\\SetupChecks\\DatabaseHasMissingPrimaryKeys' => $baseDir . '/../lib/SetupChecks/DatabaseHasMissingPrimaryKeys.php', 'OCA\\Settings\\SetupChecks\\DatabasePendingBigIntConversions' => $baseDir . '/../lib/SetupChecks/DatabasePendingBigIntConversions.php', + 'OCA\\Settings\\SetupChecks\\DebugMode' => $baseDir . '/../lib/SetupChecks/DebugMode.php', 'OCA\\Settings\\SetupChecks\\DefaultPhoneRegionSet' => $baseDir . '/../lib/SetupChecks/DefaultPhoneRegionSet.php', 'OCA\\Settings\\SetupChecks\\EmailTestSuccessful' => $baseDir . '/../lib/SetupChecks/EmailTestSuccessful.php', 'OCA\\Settings\\SetupChecks\\FileLocking' => $baseDir . '/../lib/SetupChecks/FileLocking.php', diff --git a/apps/settings/composer/composer/autoload_static.php b/apps/settings/composer/composer/autoload_static.php index 1b95d35a906..fe23224d214 100644 --- a/apps/settings/composer/composer/autoload_static.php +++ b/apps/settings/composer/composer/autoload_static.php @@ -101,6 +101,7 @@ class ComposerStaticInitSettings 'OCA\\Settings\\SetupChecks\\DatabaseHasMissingIndices' => __DIR__ . '/..' . '/../lib/SetupChecks/DatabaseHasMissingIndices.php', 'OCA\\Settings\\SetupChecks\\DatabaseHasMissingPrimaryKeys' => __DIR__ . '/..' . '/../lib/SetupChecks/DatabaseHasMissingPrimaryKeys.php', 'OCA\\Settings\\SetupChecks\\DatabasePendingBigIntConversions' => __DIR__ . '/..' . '/../lib/SetupChecks/DatabasePendingBigIntConversions.php', + 'OCA\\Settings\\SetupChecks\\DebugMode' => __DIR__ . '/..' . '/../lib/SetupChecks/DebugMode.php', 'OCA\\Settings\\SetupChecks\\DefaultPhoneRegionSet' => __DIR__ . '/..' . '/../lib/SetupChecks/DefaultPhoneRegionSet.php', 'OCA\\Settings\\SetupChecks\\EmailTestSuccessful' => __DIR__ . '/..' . '/../lib/SetupChecks/EmailTestSuccessful.php', 'OCA\\Settings\\SetupChecks\\FileLocking' => __DIR__ . '/..' . '/../lib/SetupChecks/FileLocking.php', diff --git a/apps/settings/lib/AppInfo/Application.php b/apps/settings/lib/AppInfo/Application.php index 4581af12e1c..3e04250bca7 100644 --- a/apps/settings/lib/AppInfo/Application.php +++ b/apps/settings/lib/AppInfo/Application.php @@ -58,6 +58,7 @@ use OCA\Settings\SetupChecks\DatabaseHasMissingColumns; use OCA\Settings\SetupChecks\DatabaseHasMissingIndices; use OCA\Settings\SetupChecks\DatabaseHasMissingPrimaryKeys; use OCA\Settings\SetupChecks\DatabasePendingBigIntConversions; +use OCA\Settings\SetupChecks\DebugMode; use OCA\Settings\SetupChecks\DefaultPhoneRegionSet; use OCA\Settings\SetupChecks\EmailTestSuccessful; use OCA\Settings\SetupChecks\FileLocking; @@ -180,6 +181,7 @@ class Application extends App implements IBootstrap { $context->registerSetupCheck(DatabaseHasMissingIndices::class); $context->registerSetupCheck(DatabaseHasMissingPrimaryKeys::class); $context->registerSetupCheck(DatabasePendingBigIntConversions::class); + $context->registerSetupCheck(DebugMode::class); $context->registerSetupCheck(DefaultPhoneRegionSet::class); $context->registerSetupCheck(EmailTestSuccessful::class); $context->registerSetupCheck(FileLocking::class); diff --git a/apps/settings/lib/SetupChecks/DebugMode.php b/apps/settings/lib/SetupChecks/DebugMode.php new file mode 100644 index 00000000000..83b52d6b7c2 --- /dev/null +++ b/apps/settings/lib/SetupChecks/DebugMode.php @@ -0,0 +1,55 @@ + + * + * @author Côme Chilliet + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ +namespace OCA\Settings\SetupChecks; + +use OCP\IConfig; +use OCP\IL10N; +use OCP\SetupCheck\ISetupCheck; +use OCP\SetupCheck\SetupResult; + +class DebugMode implements ISetupCheck { + public function __construct( + private IL10N $l10n, + private IConfig $config, + ) { + } + + public function getName(): string { + return $this->l10n->t('Debug mode'); + } + + public function getCategory(): string { + return 'system'; + } + + public function run(): SetupResult { + if ($this->config->getSystemValueBool('debug', false)) { + return SetupResult::warning($this->l10n->t('This instance is running in debug mode. Only enable this for local development and not in production environments.')); + } else { + return SetupResult::success($this->l10n->t('Debug mode is disabled.')); + } + } +} diff --git a/core/js/setupchecks.js b/core/js/setupchecks.js index 7400d02d30d..fe6ab1105a2 100644 --- a/core/js/setupchecks.js +++ b/core/js/setupchecks.js @@ -188,12 +188,6 @@ type: OC.SetupChecks.MESSAGE_TYPE_WARNING }) } - if (window.oc_debug) { - messages.push({ - msg: t('core', 'This instance is running in debug mode. Only enable this for local development and not in production environments.'), - type: OC.SetupChecks.MESSAGE_TYPE_WARNING - }) - } if (Object.keys(data.generic).length > 0) { Object.keys(data.generic).forEach(function(key){ Object.keys(data.generic[key]).forEach(function(title){