Skip to content

Commit

Permalink
Migrate getenv test to new SetupCheck API
Browse files Browse the repository at this point in the history
Signed-off-by: Côme Chilliet <[email protected]>
  • Loading branch information
come-nc committed Oct 26, 2023
1 parent c76554a commit 3db1039
Show file tree
Hide file tree
Showing 8 changed files with 81 additions and 58 deletions.
1 change: 1 addition & 0 deletions apps/settings/composer/composer/autoload_classmap.php
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
1 change: 1 addition & 0 deletions apps/settings/composer/composer/autoload_static.php
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
2 changes: 2 additions & 0 deletions apps/settings/lib/AppInfo/Application.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
45 changes: 22 additions & 23 deletions apps/settings/lib/Controller/CheckSetupController.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,

Check notice

Code scanning / Psalm

DeprecatedInterface Note

Interface OCP\IServerContainer is marked as deprecated
ISetupCheckManager $setupCheckManager,
) {
parent::__construct($AppName, $request);
$this->config = $config;
Expand Down Expand Up @@ -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(),
Expand Down
55 changes: 55 additions & 0 deletions apps/settings/lib/SetupChecks/PhpGetEnv.php
Original file line number Diff line number Diff line change
@@ -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'));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -500,7 +500,6 @@ public function testCheck() {

$expected = new DataResponse(
[
'isGetenvServerWorking' => true,
'isReadOnlyConfig' => false,
'wasEmailTestSuccessful' => false,
'hasValidTransactionIsolationLevel' => true,
Expand Down
9 changes: 0 additions & 9 deletions core/js/setupchecks.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.'),
Expand Down
25 changes: 0 additions & 25 deletions core/js/tests/specs/setupchecksSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,6 @@ describe('OC.SetupChecks tests', function() {
},
JSON.stringify({
hasFileinfoInstalled: true,
isGetenvServerWorking: true,
isReadOnlyConfig: false,
wasEmailTestSuccessful: true,
hasWorkingFileLocking: true,
Expand Down Expand Up @@ -295,7 +294,6 @@ describe('OC.SetupChecks tests', function() {
},
JSON.stringify({
hasFileinfoInstalled: true,
isGetenvServerWorking: true,
isReadOnlyConfig: false,
wasEmailTestSuccessful: true,
hasWorkingFileLocking: true,
Expand Down Expand Up @@ -366,7 +364,6 @@ describe('OC.SetupChecks tests', function() {
},
JSON.stringify({
hasFileinfoInstalled: true,
isGetenvServerWorking: true,
isReadOnlyConfig: false,
wasEmailTestSuccessful: true,
hasWorkingFileLocking: true,
Expand Down Expand Up @@ -433,7 +430,6 @@ describe('OC.SetupChecks tests', function() {
},
JSON.stringify({
hasFileinfoInstalled: true,
isGetenvServerWorking: true,
isReadOnlyConfig: false,
wasEmailTestSuccessful: true,
hasWorkingFileLocking: true,
Expand Down Expand Up @@ -499,7 +495,6 @@ describe('OC.SetupChecks tests', function() {
},
JSON.stringify({
hasFileinfoInstalled: true,
isGetenvServerWorking: true,
isReadOnlyConfig: false,
wasEmailTestSuccessful: true,
hasWorkingFileLocking: true,
Expand Down Expand Up @@ -565,7 +560,6 @@ describe('OC.SetupChecks tests', function() {
},
JSON.stringify({
hasFileinfoInstalled: true,
isGetenvServerWorking: true,
isReadOnlyConfig: false,
wasEmailTestSuccessful: false,
hasWorkingFileLocking: true,
Expand Down Expand Up @@ -631,7 +625,6 @@ describe('OC.SetupChecks tests', function() {
},
JSON.stringify({
hasFileinfoInstalled: true,
isGetenvServerWorking: true,
isReadOnlyConfig: false,
wasEmailTestSuccessful: true,
hasWorkingFileLocking: false,
Expand Down Expand Up @@ -697,7 +690,6 @@ describe('OC.SetupChecks tests', function() {
},
JSON.stringify({
hasFileinfoInstalled: true,
isGetenvServerWorking: true,
isReadOnlyConfig: false,
wasEmailTestSuccessful: true,
hasWorkingFileLocking: true,
Expand Down Expand Up @@ -763,7 +755,6 @@ describe('OC.SetupChecks tests', function() {
},
JSON.stringify({
hasFileinfoInstalled: true,
isGetenvServerWorking: true,
isReadOnlyConfig: false,
wasEmailTestSuccessful: true,
hasWorkingFileLocking: true,
Expand Down Expand Up @@ -831,7 +822,6 @@ describe('OC.SetupChecks tests', function() {
},
JSON.stringify({
hasFileinfoInstalled: true,
isGetenvServerWorking: true,
isReadOnlyConfig: false,
wasEmailTestSuccessful: true,
hasWorkingFileLocking: true,
Expand Down Expand Up @@ -897,7 +887,6 @@ describe('OC.SetupChecks tests', function() {
},
JSON.stringify({
hasFileinfoInstalled: true,
isGetenvServerWorking: true,
isReadOnlyConfig: false,
wasEmailTestSuccessful: true,
hasWorkingFileLocking: true,
Expand Down Expand Up @@ -965,7 +954,6 @@ describe('OC.SetupChecks tests', function() {
},
JSON.stringify({
hasFileinfoInstalled: true,
isGetenvServerWorking: true,
isReadOnlyConfig: false,
wasEmailTestSuccessful: true,
hasWorkingFileLocking: true,
Expand Down Expand Up @@ -1031,7 +1019,6 @@ describe('OC.SetupChecks tests', function() {
},
JSON.stringify({
hasFileinfoInstalled: true,
isGetenvServerWorking: true,
isReadOnlyConfig: false,
wasEmailTestSuccessful: true,
hasWorkingFileLocking: true,
Expand Down Expand Up @@ -1117,7 +1104,6 @@ describe('OC.SetupChecks tests', function() {
},
JSON.stringify({
hasFileinfoInstalled: true,
isGetenvServerWorking: true,
isReadOnlyConfig: false,
wasEmailTestSuccessful: true,
hasWorkingFileLocking: true,
Expand Down Expand Up @@ -1190,7 +1176,6 @@ describe('OC.SetupChecks tests', function() {
},
JSON.stringify({
hasFileinfoInstalled: true,
isGetenvServerWorking: true,
isReadOnlyConfig: false,
wasEmailTestSuccessful: true,
hasWorkingFileLocking: true,
Expand Down Expand Up @@ -1256,7 +1241,6 @@ describe('OC.SetupChecks tests', function() {
},
JSON.stringify({
hasFileinfoInstalled: true,
isGetenvServerWorking: true,
isReadOnlyConfig: false,
wasEmailTestSuccessful: true,
hasWorkingFileLocking: true,
Expand Down Expand Up @@ -1322,7 +1306,6 @@ describe('OC.SetupChecks tests', function() {
},
JSON.stringify({
hasFileinfoInstalled: true,
isGetenvServerWorking: true,
isReadOnlyConfig: false,
wasEmailTestSuccessful: true,
hasWorkingFileLocking: true,
Expand Down Expand Up @@ -1392,7 +1375,6 @@ describe('OC.SetupChecks tests', function() {
},
JSON.stringify({
hasFileinfoInstalled: true,
isGetenvServerWorking: true,
isReadOnlyConfig: false,
wasEmailTestSuccessful: true,
hasWorkingFileLocking: true,
Expand Down Expand Up @@ -1459,7 +1441,6 @@ describe('OC.SetupChecks tests', function() {
},
JSON.stringify({
hasFileinfoInstalled: true,
isGetenvServerWorking: true,
isReadOnlyConfig: false,
wasEmailTestSuccessful: true,
hasWorkingFileLocking: true,
Expand Down Expand Up @@ -1523,7 +1504,6 @@ describe('OC.SetupChecks tests', function() {
},
JSON.stringify({
hasFileinfoInstalled: true,
isGetenvServerWorking: true,
isReadOnlyConfig: false,
wasEmailTestSuccessful: true,
hasWorkingFileLocking: true,
Expand Down Expand Up @@ -1590,7 +1570,6 @@ describe('OC.SetupChecks tests', function() {
},
JSON.stringify({
hasFileinfoInstalled: true,
isGetenvServerWorking: true,
isReadOnlyConfig: false,
wasEmailTestSuccessful: true,
hasWorkingFileLocking: true,
Expand Down Expand Up @@ -1657,7 +1636,6 @@ describe('OC.SetupChecks tests', function() {
},
JSON.stringify({
hasFileinfoInstalled: true,
isGetenvServerWorking: true,
isReadOnlyConfig: false,
wasEmailTestSuccessful: true,
hasWorkingFileLocking: true,
Expand Down Expand Up @@ -1723,7 +1701,6 @@ describe('OC.SetupChecks tests', function() {
},
JSON.stringify({
hasFileinfoInstalled: true,
isGetenvServerWorking: true,
isReadOnlyConfig: false,
wasEmailTestSuccessful: true,
hasWorkingFileLocking: true,
Expand Down Expand Up @@ -1789,7 +1766,6 @@ describe('OC.SetupChecks tests', function() {
},
JSON.stringify({
hasFileinfoInstalled: true,
isGetenvServerWorking: true,
isReadOnlyConfig: false,
wasEmailTestSuccessful: true,
hasWorkingFileLocking: true,
Expand Down Expand Up @@ -1862,7 +1838,6 @@ describe('OC.SetupChecks tests', function() {
},
JSON.stringify({
hasFileinfoInstalled: true,
isGetenvServerWorking: true,
isReadOnlyConfig: false,
wasEmailTestSuccessful: true,
hasWorkingFileLocking: true,
Expand Down

0 comments on commit 3db1039

Please sign in to comment.