From 3db10395b55c2807785c11c1b5cc6fd822bb5325 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Thu, 26 Oct 2023 12:20:25 +0200 Subject: [PATCH] Migrate getenv test 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 | 45 ++++++++------- apps/settings/lib/SetupChecks/PhpGetEnv.php | 55 +++++++++++++++++++ .../Controller/CheckSetupControllerTest.php | 1 - core/js/setupchecks.js | 9 --- core/js/tests/specs/setupchecksSpec.js | 25 --------- 8 files changed, 81 insertions(+), 58 deletions(-) create mode 100644 apps/settings/lib/SetupChecks/PhpGetEnv.php diff --git a/apps/settings/composer/composer/autoload_classmap.php b/apps/settings/composer/composer/autoload_classmap.php index c6b768e8b333f..5d40db5064e5e 100644 --- a/apps/settings/composer/composer/autoload_classmap.php +++ b/apps/settings/composer/composer/autoload_classmap.php @@ -78,6 +78,7 @@ 'OCA\\Settings\\SetupChecks\\InternetConnectivity' => $baseDir . '/../lib/SetupChecks/InternetConnectivity.php', 'OCA\\Settings\\SetupChecks\\LegacySSEKeyFormat' => $baseDir . '/../lib/SetupChecks/LegacySSEKeyFormat.php', 'OCA\\Settings\\SetupChecks\\PhpDefaultCharset' => $baseDir . '/../lib/SetupChecks/PhpDefaultCharset.php', + 'OCA\\Settings\\SetupChecks\\PhpGetEnv' => $baseDir . '/../lib/SetupChecks/PhpGetEnv.php', 'OCA\\Settings\\SetupChecks\\PhpOutdated' => $baseDir . '/../lib/SetupChecks/PhpOutdated.php', 'OCA\\Settings\\SetupChecks\\PhpOutputBuffering' => $baseDir . '/../lib/SetupChecks/PhpOutputBuffering.php', 'OCA\\Settings\\SetupChecks\\ReadOnlyConfig' => $baseDir . '/../lib/SetupChecks/ReadOnlyConfig.php', diff --git a/apps/settings/composer/composer/autoload_static.php b/apps/settings/composer/composer/autoload_static.php index 0905343597036..e1806f05ba46f 100644 --- a/apps/settings/composer/composer/autoload_static.php +++ b/apps/settings/composer/composer/autoload_static.php @@ -93,6 +93,7 @@ class ComposerStaticInitSettings 'OCA\\Settings\\SetupChecks\\InternetConnectivity' => __DIR__ . '/..' . '/../lib/SetupChecks/InternetConnectivity.php', 'OCA\\Settings\\SetupChecks\\LegacySSEKeyFormat' => __DIR__ . '/..' . '/../lib/SetupChecks/LegacySSEKeyFormat.php', 'OCA\\Settings\\SetupChecks\\PhpDefaultCharset' => __DIR__ . '/..' . '/../lib/SetupChecks/PhpDefaultCharset.php', + 'OCA\\Settings\\SetupChecks\\PhpGetEnv' => __DIR__ . '/..' . '/../lib/SetupChecks/PhpGetEnv.php', 'OCA\\Settings\\SetupChecks\\PhpOutdated' => __DIR__ . '/..' . '/../lib/SetupChecks/PhpOutdated.php', 'OCA\\Settings\\SetupChecks\\PhpOutputBuffering' => __DIR__ . '/..' . '/../lib/SetupChecks/PhpOutputBuffering.php', 'OCA\\Settings\\SetupChecks\\ReadOnlyConfig' => __DIR__ . '/..' . '/../lib/SetupChecks/ReadOnlyConfig.php', diff --git a/apps/settings/lib/AppInfo/Application.php b/apps/settings/lib/AppInfo/Application.php index e6106cb9b1edd..c2bda22fc6147 100644 --- a/apps/settings/lib/AppInfo/Application.php +++ b/apps/settings/lib/AppInfo/Application.php @@ -53,6 +53,7 @@ use OCA\Settings\SetupChecks\InternetConnectivity; use OCA\Settings\SetupChecks\LegacySSEKeyFormat; use OCA\Settings\SetupChecks\PhpDefaultCharset; +use OCA\Settings\SetupChecks\PhpGetEnv; use OCA\Settings\SetupChecks\PhpOutdated; use OCA\Settings\SetupChecks\PhpOutputBuffering; use OCA\Settings\SetupChecks\ReadOnlyConfig; @@ -151,6 +152,7 @@ public function register(IRegistrationContext $context): void { $context->registerSetupCheck(InternetConnectivity::class); $context->registerSetupCheck(LegacySSEKeyFormat::class); $context->registerSetupCheck(PhpDefaultCharset::class); + $context->registerSetupCheck(PhpGetEnv::class); $context->registerSetupCheck(PhpOutdated::class); $context->registerSetupCheck(PhpOutputBuffering::class); $context->registerSetupCheck(ReadOnlyConfig::class); diff --git a/apps/settings/lib/Controller/CheckSetupController.php b/apps/settings/lib/Controller/CheckSetupController.php index b2da455bc1166..b0b98e3aca4c5 100644 --- a/apps/settings/lib/Controller/CheckSetupController.php +++ b/apps/settings/lib/Controller/CheckSetupController.php @@ -58,8 +58,8 @@ use OC\DB\MissingPrimaryKeyInformation; use OC\DB\SchemaWrapper; use OC\IntegrityCheck\Checker; -use OC\Lock\NoopLockingProvider; use OC\Lock\DBLockingProvider; +use OC\Lock\NoopLockingProvider; use OC\MemoryInfo; use OCP\App\IAppManager; use OCP\AppFramework\Controller; @@ -131,27 +131,27 @@ class CheckSetupController extends Controller { private ISetupCheckManager $setupCheckManager; public function __construct($AppName, - IRequest $request, - IConfig $config, - IClientService $clientService, - IURLGenerator $urlGenerator, - IL10N $l10n, - Checker $checker, - LoggerInterface $logger, - IEventDispatcher $dispatcher, - Connection $db, - ILockingProvider $lockingProvider, - IDateTimeFormatter $dateTimeFormatter, - MemoryInfo $memoryInfo, - ISecureRandom $secureRandom, - IniGetWrapper $iniGetWrapper, - IDBConnection $connection, - IThrottler $throttler, - ITempManager $tempManager, - IManager $manager, - IAppManager $appManager, - IServerContainer $serverContainer, - ISetupCheckManager $setupCheckManager, + IRequest $request, + IConfig $config, + IClientService $clientService, + IURLGenerator $urlGenerator, + IL10N $l10n, + Checker $checker, + LoggerInterface $logger, + IEventDispatcher $dispatcher, + Connection $db, + ILockingProvider $lockingProvider, + IDateTimeFormatter $dateTimeFormatter, + MemoryInfo $memoryInfo, + ISecureRandom $secureRandom, + IniGetWrapper $iniGetWrapper, + IDBConnection $connection, + IThrottler $throttler, + ITempManager $tempManager, + IManager $manager, + IAppManager $appManager, + IServerContainer $serverContainer, + ISetupCheckManager $setupCheckManager, ) { parent::__construct($AppName, $request); $this->config = $config; @@ -846,7 +846,6 @@ protected function imageMagickLacksSVGSupport(): bool { public function check() { return new DataResponse( [ - 'isGetenvServerWorking' => !empty(getenv('PATH')), 'isReadOnlyConfig' => $this->isReadOnlyConfig(), 'hasValidTransactionIsolationLevel' => $this->hasValidTransactionIsolationLevel(), 'wasEmailTestSuccessful' => $this->wasEmailTestSuccessful(), diff --git a/apps/settings/lib/SetupChecks/PhpGetEnv.php b/apps/settings/lib/SetupChecks/PhpGetEnv.php new file mode 100644 index 0000000000000..50f1554098983 --- /dev/null +++ b/apps/settings/lib/SetupChecks/PhpGetEnv.php @@ -0,0 +1,55 @@ + + * + * @author Daniel Kesselberg + * + * @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\IURLGenerator; +use OCP\SetupCheck\ISetupCheck; +use OCP\SetupCheck\SetupResult; + +class PhpGetEnv implements ISetupCheck { + public function __construct( + private IL10N $l10n, + private IURLGenerator $urlGenerator, + ) { + } + + public function getName(): string { + return $this->l10n->t('PHP getenv'); + } + + public function getCategory(): string { + return 'php'; + } + + public function run(): SetupResult { + if (!empty(getenv('PATH'))) { + return SetupResult::success(); + } else { + return SetupResult::warning($this->l10n->t('PHP does not seem to be setup properly to query system environment variables. The test with getenv("PATH") only returns an empty response.'), $this->urlGenerator->linkToDocs('admin-php-fpm')); + } + } +} diff --git a/apps/settings/tests/Controller/CheckSetupControllerTest.php b/apps/settings/tests/Controller/CheckSetupControllerTest.php index 449f181e8c18b..94f92d05cf350 100644 --- a/apps/settings/tests/Controller/CheckSetupControllerTest.php +++ b/apps/settings/tests/Controller/CheckSetupControllerTest.php @@ -500,7 +500,6 @@ public function testCheck() { $expected = new DataResponse( [ - 'isGetenvServerWorking' => true, 'isReadOnlyConfig' => false, 'wasEmailTestSuccessful' => false, 'hasValidTransactionIsolationLevel' => true, diff --git a/core/js/setupchecks.js b/core/js/setupchecks.js index a2bddefbe698d..54f33bba9c2a2 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.isGetenvServerWorking) { - messages.push({ - msg: t('core', 'PHP does not seem to be setup properly to query system environment variables. The test with getenv("PATH") only returns an empty response.') + ' ' + - t('core', 'Please check the {linkstart}installation documentation ↗{linkend} for PHP configuration notes and the PHP configuration of your server, especially when using php-fpm.') - .replace('{linkstart}', '') - .replace('{linkend}', ''), - type: OC.SetupChecks.MESSAGE_TYPE_WARNING - }); - } if (data.isReadOnlyConfig) { messages.push({ msg: t('core', 'The read-only config has been enabled. This prevents setting some configurations via the web-interface. Furthermore, the file needs to be made writable manually for every update.'), diff --git a/core/js/tests/specs/setupchecksSpec.js b/core/js/tests/specs/setupchecksSpec.js index 676111c80a4b4..8732e2328268a 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({ hasFileinfoInstalled: true, - isGetenvServerWorking: true, isReadOnlyConfig: false, wasEmailTestSuccessful: true, hasWorkingFileLocking: true, @@ -295,7 +294,6 @@ describe('OC.SetupChecks tests', function() { }, JSON.stringify({ hasFileinfoInstalled: true, - isGetenvServerWorking: true, isReadOnlyConfig: false, wasEmailTestSuccessful: true, hasWorkingFileLocking: true, @@ -366,7 +364,6 @@ describe('OC.SetupChecks tests', function() { }, JSON.stringify({ hasFileinfoInstalled: true, - isGetenvServerWorking: true, isReadOnlyConfig: false, wasEmailTestSuccessful: true, hasWorkingFileLocking: true, @@ -433,7 +430,6 @@ describe('OC.SetupChecks tests', function() { }, JSON.stringify({ hasFileinfoInstalled: true, - isGetenvServerWorking: true, isReadOnlyConfig: false, wasEmailTestSuccessful: true, hasWorkingFileLocking: true, @@ -499,7 +495,6 @@ describe('OC.SetupChecks tests', function() { }, JSON.stringify({ hasFileinfoInstalled: true, - isGetenvServerWorking: true, isReadOnlyConfig: false, wasEmailTestSuccessful: true, hasWorkingFileLocking: true, @@ -565,7 +560,6 @@ describe('OC.SetupChecks tests', function() { }, JSON.stringify({ hasFileinfoInstalled: true, - isGetenvServerWorking: true, isReadOnlyConfig: false, wasEmailTestSuccessful: false, hasWorkingFileLocking: true, @@ -631,7 +625,6 @@ describe('OC.SetupChecks tests', function() { }, JSON.stringify({ hasFileinfoInstalled: true, - isGetenvServerWorking: true, isReadOnlyConfig: false, wasEmailTestSuccessful: true, hasWorkingFileLocking: false, @@ -697,7 +690,6 @@ describe('OC.SetupChecks tests', function() { }, JSON.stringify({ hasFileinfoInstalled: true, - isGetenvServerWorking: true, isReadOnlyConfig: false, wasEmailTestSuccessful: true, hasWorkingFileLocking: true, @@ -763,7 +755,6 @@ describe('OC.SetupChecks tests', function() { }, JSON.stringify({ hasFileinfoInstalled: true, - isGetenvServerWorking: true, isReadOnlyConfig: false, wasEmailTestSuccessful: true, hasWorkingFileLocking: true, @@ -831,7 +822,6 @@ describe('OC.SetupChecks tests', function() { }, JSON.stringify({ hasFileinfoInstalled: true, - isGetenvServerWorking: true, isReadOnlyConfig: false, wasEmailTestSuccessful: true, hasWorkingFileLocking: true, @@ -897,7 +887,6 @@ describe('OC.SetupChecks tests', function() { }, JSON.stringify({ hasFileinfoInstalled: true, - isGetenvServerWorking: true, isReadOnlyConfig: false, wasEmailTestSuccessful: true, hasWorkingFileLocking: true, @@ -965,7 +954,6 @@ describe('OC.SetupChecks tests', function() { }, JSON.stringify({ hasFileinfoInstalled: true, - isGetenvServerWorking: true, isReadOnlyConfig: false, wasEmailTestSuccessful: true, hasWorkingFileLocking: true, @@ -1031,7 +1019,6 @@ describe('OC.SetupChecks tests', function() { }, JSON.stringify({ hasFileinfoInstalled: true, - isGetenvServerWorking: true, isReadOnlyConfig: false, wasEmailTestSuccessful: true, hasWorkingFileLocking: true, @@ -1117,7 +1104,6 @@ describe('OC.SetupChecks tests', function() { }, JSON.stringify({ hasFileinfoInstalled: true, - isGetenvServerWorking: true, isReadOnlyConfig: false, wasEmailTestSuccessful: true, hasWorkingFileLocking: true, @@ -1190,7 +1176,6 @@ describe('OC.SetupChecks tests', function() { }, JSON.stringify({ hasFileinfoInstalled: true, - isGetenvServerWorking: true, isReadOnlyConfig: false, wasEmailTestSuccessful: true, hasWorkingFileLocking: true, @@ -1256,7 +1241,6 @@ describe('OC.SetupChecks tests', function() { }, JSON.stringify({ hasFileinfoInstalled: true, - isGetenvServerWorking: true, isReadOnlyConfig: false, wasEmailTestSuccessful: true, hasWorkingFileLocking: true, @@ -1322,7 +1306,6 @@ describe('OC.SetupChecks tests', function() { }, JSON.stringify({ hasFileinfoInstalled: true, - isGetenvServerWorking: true, isReadOnlyConfig: false, wasEmailTestSuccessful: true, hasWorkingFileLocking: true, @@ -1392,7 +1375,6 @@ describe('OC.SetupChecks tests', function() { }, JSON.stringify({ hasFileinfoInstalled: true, - isGetenvServerWorking: true, isReadOnlyConfig: false, wasEmailTestSuccessful: true, hasWorkingFileLocking: true, @@ -1459,7 +1441,6 @@ describe('OC.SetupChecks tests', function() { }, JSON.stringify({ hasFileinfoInstalled: true, - isGetenvServerWorking: true, isReadOnlyConfig: false, wasEmailTestSuccessful: true, hasWorkingFileLocking: true, @@ -1523,7 +1504,6 @@ describe('OC.SetupChecks tests', function() { }, JSON.stringify({ hasFileinfoInstalled: true, - isGetenvServerWorking: true, isReadOnlyConfig: false, wasEmailTestSuccessful: true, hasWorkingFileLocking: true, @@ -1590,7 +1570,6 @@ describe('OC.SetupChecks tests', function() { }, JSON.stringify({ hasFileinfoInstalled: true, - isGetenvServerWorking: true, isReadOnlyConfig: false, wasEmailTestSuccessful: true, hasWorkingFileLocking: true, @@ -1657,7 +1636,6 @@ describe('OC.SetupChecks tests', function() { }, JSON.stringify({ hasFileinfoInstalled: true, - isGetenvServerWorking: true, isReadOnlyConfig: false, wasEmailTestSuccessful: true, hasWorkingFileLocking: true, @@ -1723,7 +1701,6 @@ describe('OC.SetupChecks tests', function() { }, JSON.stringify({ hasFileinfoInstalled: true, - isGetenvServerWorking: true, isReadOnlyConfig: false, wasEmailTestSuccessful: true, hasWorkingFileLocking: true, @@ -1789,7 +1766,6 @@ describe('OC.SetupChecks tests', function() { }, JSON.stringify({ hasFileinfoInstalled: true, - isGetenvServerWorking: true, isReadOnlyConfig: false, wasEmailTestSuccessful: true, hasWorkingFileLocking: true, @@ -1862,7 +1838,6 @@ describe('OC.SetupChecks tests', function() { }, JSON.stringify({ hasFileinfoInstalled: true, - isGetenvServerWorking: true, isReadOnlyConfig: false, wasEmailTestSuccessful: true, hasWorkingFileLocking: true,