Migrate getenv test to new SetupCheck API

Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>

apps/settings/composer/composer/autoload_classmap.php

@@ -78,6 +78,7 @@ return array(
     '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\\PhpModules' => $baseDir . '/../lib/SetupChecks/PhpModules.php',
     'OCA\\Settings\\SetupChecks\\PhpOutdated' => $baseDir . '/../lib/SetupChecks/PhpOutdated.php',
     'OCA\\Settings\\SetupChecks\\PhpOutputBuffering' => $baseDir . '/../lib/SetupChecks/PhpOutputBuffering.php',

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\\PhpModules' => __DIR__ . '/..' . '/../lib/SetupChecks/PhpModules.php',
         'OCA\\Settings\\SetupChecks\\PhpOutdated' => __DIR__ . '/..' . '/../lib/SetupChecks/PhpOutdated.php',
         'OCA\\Settings\\SetupChecks\\PhpOutputBuffering' => __DIR__ . '/..' . '/../lib/SetupChecks/PhpOutputBuffering.php',

apps/settings/lib/AppInfo/Application.php

@@ -54,6 +54,7 @@ use OCA\Settings\SetupChecks\InternetConnectivity;
 use OCA\Settings\SetupChecks\LegacySSEKeyFormat;
 use OCA\Settings\SetupChecks\PhpDefaultCharset;
 use OCA\Settings\SetupChecks\PhpModules;
+use OCA\Settings\SetupChecks\PhpGetEnv;
 use OCA\Settings\SetupChecks\PhpOutdated;
 use OCA\Settings\SetupChecks\PhpOutputBuffering;
 use OCA\Settings\SetupChecks\ReadOnlyConfig;
@@ -153,6 +154,7 @@ class Application extends App implements IBootstrap {
 		$context->registerSetupCheck(LegacySSEKeyFormat::class);
 		$context->registerSetupCheck(PhpDefaultCharset::class);
 		$context->registerSetupCheck(PhpModules::class);
+		$context->registerSetupCheck(PhpGetEnv::class);
 		$context->registerSetupCheck(PhpOutdated::class);
 		$context->registerSetupCheck(PhpOutputBuffering::class);
 		$context->registerSetupCheck(ReadOnlyConfig::class);

apps/settings/lib/Controller/CheckSetupController.php

@@ -58,8 +58,8 @@ use OC\DB\MissingIndexInformation;
 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;
@@ -817,7 +817,6 @@ Raw output
 	public function check() {
 		return new DataResponse(
 			[
-				'isGetenvServerWorking' => !empty(getenv('PATH')),
 				'isReadOnlyConfig' => $this->isReadOnlyConfig(),
 				'hasValidTransactionIsolationLevel' => $this->hasValidTransactionIsolationLevel(),
 				'wasEmailTestSuccessful' => $this->wasEmailTestSuccessful(),

apps/settings/lib/SetupChecks/PhpGetEnv.php

@@ -0,0 +1,55 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * @copyright Copyright (c) 2020 Daniel Kesselberg <mail@danielkesselberg.de>
+ *
+ * @author Daniel Kesselberg <mail@danielkesselberg.de>
+ *
+ * @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 <http://www.gnu.org/licenses/>.
+ *
+ */
+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'));
+		}
+	}
+}

apps/settings/tests/Controller/CheckSetupControllerTest.php

@@ -494,7 +494,6 @@ class CheckSetupControllerTest extends TestCase {
 
 		$expected = new DataResponse(
 			[
-				'isGetenvServerWorking' => true,
 				'isReadOnlyConfig' => false,
 				'wasEmailTestSuccessful' => false,
 				'hasValidTransactionIsolationLevel' => true,

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}', '<a target="_blank" rel="noreferrer noopener" class="external" href="' + OC.theme.docPlaceholderUrl.replace('PLACEHOLDER', 'admin-php-fpm') + '">')
-									.replace('{linkend}', '</a>'),
-							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.'),

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,
@@ -294,7 +293,6 @@ describe('OC.SetupChecks tests', function() {
 				},
 				JSON.stringify({
 					hasFileinfoInstalled: true,
-					isGetenvServerWorking: true,
 					isReadOnlyConfig: false,
 					wasEmailTestSuccessful: true,
 					hasWorkingFileLocking: true,
@@ -364,7 +362,6 @@ describe('OC.SetupChecks tests', function() {
 				},
 				JSON.stringify({
 					hasFileinfoInstalled: true,
-					isGetenvServerWorking: true,
 					isReadOnlyConfig: false,
 					wasEmailTestSuccessful: true,
 					hasWorkingFileLocking: true,
@@ -430,7 +427,6 @@ describe('OC.SetupChecks tests', function() {
 				},
 				JSON.stringify({
 					hasFileinfoInstalled: true,
-					isGetenvServerWorking: true,
 					isReadOnlyConfig: false,
 					wasEmailTestSuccessful: true,
 					hasWorkingFileLocking: true,
@@ -495,7 +491,6 @@ describe('OC.SetupChecks tests', function() {
 				},
 				JSON.stringify({
 					hasFileinfoInstalled: true,
-					isGetenvServerWorking: true,
 					isReadOnlyConfig: false,
 					wasEmailTestSuccessful: true,
 					hasWorkingFileLocking: true,
@@ -560,7 +555,6 @@ describe('OC.SetupChecks tests', function() {
 				},
 				JSON.stringify({
 					hasFileinfoInstalled: true,
-					isGetenvServerWorking: true,
 					isReadOnlyConfig: false,
 					wasEmailTestSuccessful: false,
 					hasWorkingFileLocking: true,
@@ -625,7 +619,6 @@ describe('OC.SetupChecks tests', function() {
 				},
 				JSON.stringify({
 					hasFileinfoInstalled: true,
-					isGetenvServerWorking: true,
 					isReadOnlyConfig: false,
 					wasEmailTestSuccessful: true,
 					hasWorkingFileLocking: false,
@@ -690,7 +683,6 @@ describe('OC.SetupChecks tests', function() {
 				},
 				JSON.stringify({
 					hasFileinfoInstalled: true,
-					isGetenvServerWorking: true,
 					isReadOnlyConfig: false,
 					wasEmailTestSuccessful: true,
 					hasWorkingFileLocking: true,
@@ -755,7 +747,6 @@ describe('OC.SetupChecks tests', function() {
 				},
 				JSON.stringify({
 					hasFileinfoInstalled: true,
-					isGetenvServerWorking: true,
 					isReadOnlyConfig: false,
 					wasEmailTestSuccessful: true,
 					hasWorkingFileLocking: true,
@@ -822,7 +813,6 @@ describe('OC.SetupChecks tests', function() {
 				},
 				JSON.stringify({
 					hasFileinfoInstalled: true,
-					isGetenvServerWorking: true,
 					isReadOnlyConfig: false,
 					wasEmailTestSuccessful: true,
 					hasWorkingFileLocking: true,
@@ -887,7 +877,6 @@ describe('OC.SetupChecks tests', function() {
 				},
 				JSON.stringify({
 					hasFileinfoInstalled: true,
-					isGetenvServerWorking: true,
 					isReadOnlyConfig: false,
 					wasEmailTestSuccessful: true,
 					hasWorkingFileLocking: true,
@@ -954,7 +943,6 @@ describe('OC.SetupChecks tests', function() {
 				},
 				JSON.stringify({
 					hasFileinfoInstalled: true,
-					isGetenvServerWorking: true,
 					isReadOnlyConfig: false,
 					wasEmailTestSuccessful: true,
 					hasWorkingFileLocking: true,
@@ -1019,7 +1007,6 @@ describe('OC.SetupChecks tests', function() {
 				},
 				JSON.stringify({
 					hasFileinfoInstalled: true,
-					isGetenvServerWorking: true,
 					isReadOnlyConfig: false,
 					wasEmailTestSuccessful: true,
 					hasWorkingFileLocking: true,
@@ -1104,7 +1091,6 @@ describe('OC.SetupChecks tests', function() {
 				},
 				JSON.stringify({
 					hasFileinfoInstalled: true,
-					isGetenvServerWorking: true,
 					isReadOnlyConfig: false,
 					wasEmailTestSuccessful: true,
 					hasWorkingFileLocking: true,
@@ -1176,7 +1162,6 @@ describe('OC.SetupChecks tests', function() {
 				},
 				JSON.stringify({
 					hasFileinfoInstalled: true,
-					isGetenvServerWorking: true,
 					isReadOnlyConfig: false,
 					wasEmailTestSuccessful: true,
 					hasWorkingFileLocking: true,
@@ -1241,7 +1226,6 @@ describe('OC.SetupChecks tests', function() {
 				},
 				JSON.stringify({
 					hasFileinfoInstalled: true,
-					isGetenvServerWorking: true,
 					isReadOnlyConfig: false,
 					wasEmailTestSuccessful: true,
 					hasWorkingFileLocking: true,
@@ -1306,7 +1290,6 @@ describe('OC.SetupChecks tests', function() {
 				},
 				JSON.stringify({
 					hasFileinfoInstalled: true,
-					isGetenvServerWorking: true,
 					isReadOnlyConfig: false,
 					wasEmailTestSuccessful: true,
 					hasWorkingFileLocking: true,
@@ -1375,7 +1358,6 @@ describe('OC.SetupChecks tests', function() {
 				},
 				JSON.stringify({
 					hasFileinfoInstalled: true,
-					isGetenvServerWorking: true,
 					isReadOnlyConfig: false,
 					wasEmailTestSuccessful: true,
 					hasWorkingFileLocking: true,
@@ -1441,7 +1423,6 @@ describe('OC.SetupChecks tests', function() {
 				},
 				JSON.stringify({
 					hasFileinfoInstalled: true,
-					isGetenvServerWorking: true,
 					isReadOnlyConfig: false,
 					wasEmailTestSuccessful: true,
 					hasWorkingFileLocking: true,
@@ -1504,7 +1485,6 @@ describe('OC.SetupChecks tests', function() {
 				},
 				JSON.stringify({
 					hasFileinfoInstalled: true,
-					isGetenvServerWorking: true,
 					isReadOnlyConfig: false,
 					wasEmailTestSuccessful: true,
 					hasWorkingFileLocking: true,
@@ -1570,7 +1550,6 @@ describe('OC.SetupChecks tests', function() {
 				},
 				JSON.stringify({
 					hasFileinfoInstalled: true,
-					isGetenvServerWorking: true,
 					isReadOnlyConfig: false,
 					wasEmailTestSuccessful: true,
 					hasWorkingFileLocking: true,
@@ -1636,7 +1615,6 @@ describe('OC.SetupChecks tests', function() {
 				},
 				JSON.stringify({
 					hasFileinfoInstalled: true,
-					isGetenvServerWorking: true,
 					isReadOnlyConfig: false,
 					wasEmailTestSuccessful: true,
 					hasWorkingFileLocking: true,
@@ -1701,7 +1679,6 @@ describe('OC.SetupChecks tests', function() {
 				},
 				JSON.stringify({
 					hasFileinfoInstalled: true,
-					isGetenvServerWorking: true,
 					isReadOnlyConfig: false,
 					wasEmailTestSuccessful: true,
 					hasWorkingFileLocking: true,
@@ -1766,7 +1743,6 @@ describe('OC.SetupChecks tests', function() {
 				},
 				JSON.stringify({
 					hasFileinfoInstalled: true,
-					isGetenvServerWorking: true,
 					isReadOnlyConfig: false,
 					wasEmailTestSuccessful: true,
 					hasWorkingFileLocking: true,
@@ -1838,7 +1814,6 @@ describe('OC.SetupChecks tests', function() {
 				},
 				JSON.stringify({
 					hasFileinfoInstalled: true,
-					isGetenvServerWorking: true,
 					isReadOnlyConfig: false,
 					wasEmailTestSuccessful: true,
 					hasWorkingFileLocking: true,