Skip to content

Migrate checks to new SetupCheck API #41134

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
susnux merged 12 commits into from
Nov 7, 2023
Merged

Migrate checks to new SetupCheck API #41134

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
dec52f6
Migrate getenv test to new SetupCheck API
come-nc Oct 26, 2023
506f396
Remove duplicated ReadOnlyConfig setup check
come-nc Oct 26, 2023
5957a2b
Migrate email test to new SetupCheck API
come-nc Oct 26, 2023
3c75075
Migrate database transaction level check to SetupCheck API
come-nc Oct 26, 2023
f9770b8
Remove duplicate fileinfo PHP module check
come-nc Oct 26, 2023
6911dc3
Migrated both file locking checks to SetupCheck API
come-nc Oct 26, 2023
ddd13a9
Migrate memcache check to SetupCheck API
come-nc Oct 26, 2023
147b29f
Migrate Randomness secure check to SetupCheck API
come-nc Oct 26, 2023
086d3ce
Migrate code integrity to SetupCheck API
come-nc Oct 26, 2023
ac3da13
Migrate Freetype check to SetupCheck API
come-nc Oct 26, 2023
0fd5ed9
Migrate 32bit check to SetupCheck API
come-nc Oct 26, 2023
00d3856
Revert "Migrate code integrity to SetupCheck API"
come-nc Nov 6, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Migrate database transaction level check to SetupCheck API 
Signed-off-by: Côme Chilliet <[email protected]>
  • Loading branch information
@come-nc
come-nc committed Nov 7, 2023
commit 3c75075eba3414da7b3c4db9aefe22da074cdcd9
1 change: 1 addition & 0 deletions apps/settings/composer/composer/autoload_classmap.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@
'OCA\\Settings\\SetupChecks\\PhpOutputBuffering' => $baseDir . '/../lib/SetupChecks/PhpOutputBuffering.php',
'OCA\\Settings\\SetupChecks\\ReadOnlyConfig' => $baseDir . '/../lib/SetupChecks/ReadOnlyConfig.php',
'OCA\\Settings\\SetupChecks\\SupportedDatabase' => $baseDir . '/../lib/SetupChecks/SupportedDatabase.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',
'OCA\\Settings\\WellKnown\\ChangePasswordHandler' => $baseDir . '/../lib/WellKnown/ChangePasswordHandler.php',
Expand Down
1 change: 1 addition & 0 deletions apps/settings/composer/composer/autoload_static.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ class ComposerStaticInitSettings
'OCA\\Settings\\SetupChecks\\PhpOutputBuffering' => __DIR__ . '/..' . '/../lib/SetupChecks/PhpOutputBuffering.php',
'OCA\\Settings\\SetupChecks\\ReadOnlyConfig' => __DIR__ . '/..' . '/../lib/SetupChecks/ReadOnlyConfig.php',
'OCA\\Settings\\SetupChecks\\SupportedDatabase' => __DIR__ . '/..' . '/../lib/SetupChecks/SupportedDatabase.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',
'OCA\\Settings\\WellKnown\\ChangePasswordHandler' => __DIR__ . '/..' . '/../lib/WellKnown/ChangePasswordHandler.php',
Expand Down
2 changes: 2 additions & 0 deletions apps/settings/lib/AppInfo/Application.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@
use OCA\Settings\SetupChecks\PhpOutputBuffering;
use OCA\Settings\SetupChecks\ReadOnlyConfig;
use OCA\Settings\SetupChecks\SupportedDatabase;
use OCA\Settings\SetupChecks\TransactionIsolation;
use OCA\Settings\UserMigration\AccountMigrator;
use OCA\Settings\WellKnown\ChangePasswordHandler;
use OCA\Settings\WellKnown\SecurityTxtHandler;
Expand Down Expand Up @@ -161,6 +162,7 @@ public function register(IRegistrationContext $context): void {
$context->registerSetupCheck(PhpOutputBuffering::class);
$context->registerSetupCheck(ReadOnlyConfig::class);
$context->registerSetupCheck(SupportedDatabase::class);
$context->registerSetupCheck(TransactionIsolation::class);

$context->registerUserMigrator(AccountMigrator::class);
}
Expand Down
17 changes: 0 additions & 17 deletions apps/settings/lib/Controller/CheckSetupController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,6 @@

use bantu\IniGetWrapper\IniGetWrapper;
use DirectoryIterator;
use Doctrine\DBAL\Exception;
use Doctrine\DBAL\TransactionIsolationLevel;
use GuzzleHttp\Exception\ClientException;
use OC;
use OC\AppFramework\Http;
Expand Down Expand Up @@ -564,20 +562,6 @@ protected function isSqliteUsed() {
return str_contains($this->config->getSystemValue('dbtype'), 'sqlite');
}

protected function hasValidTransactionIsolationLevel(): bool {
try {
if ($this->connection->getDatabaseProvider() === IDBConnection::PLATFORM_SQLITE) {
return true;
}

return $this->db->getTransactionIsolation() === TransactionIsolationLevel::READ_COMMITTED;
} catch (Exception $e) {
// ignore
}

return true;
}

protected function hasFileinfoInstalled(): bool {
return \OC_Util::fileInfoLoaded();
}
Expand Down Expand Up @@ -799,7 +783,6 @@ protected function imageMagickLacksSVGSupport(): bool {
public function check() {
return new DataResponse(
[
'hasValidTransactionIsolationLevel' => $this->hasValidTransactionIsolationLevel(),
'hasFileinfoInstalled' => $this->hasFileinfoInstalled(),
'hasWorkingFileLocking' => $this->hasWorkingFileLocking(),
'hasDBFileLocking' => $this->hasDBFileLocking(),
Expand Down
75 changes: 75 additions & 0 deletions apps/settings/lib/SetupChecks/TransactionIsolation.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
<?php

declare(strict_types=1);

/**
* @copyright Copyright (c) 2023 Côme Chilliet <[email protected]>
*
* @author Côme Chilliet <[email protected]>
*
* @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 Doctrine\DBAL\Exception;
use Doctrine\DBAL\TransactionIsolationLevel;
use OC\DB\Connection;
use OCP\IDBConnection;
use OCP\IL10N;
use OCP\IURLGenerator;
use OCP\SetupCheck\ISetupCheck;
use OCP\SetupCheck\SetupResult;

class TransactionIsolation implements ISetupCheck {
public function __construct(
private IL10N $l10n,
private IURLGenerator $urlGenerator,
private IDBConnection $connection,
private Connection $db,
) {
}

public function getName(): string {
return $this->l10n->t('Database transaction isolation level');
}

public function getCategory(): string {
return 'database';
}

public function run(): SetupResult {
try {
if ($this->connection->getDatabaseProvider() === IDBConnection::PLATFORM_SQLITE) {
return SetupResult::success();
}

if ($this->db->getTransactionIsolation() === TransactionIsolationLevel::READ_COMMITTED) {
return SetupResult::success('Read committed');
} else {
return SetupResult::error(
$this->l10n->t('Your database does not run with "READ COMMITTED" transaction isolation level. This can cause problems when multiple actions are executed in parallel.'),
$this->urlGenerator->linkToDocs('admin-db-transaction')
);
}
} catch (Exception $e) {
return SetupResult::warning(
$this->l10n->t('Was not able to get transaction isolation level: %s', $e->getMessage())
);
}
}
}
6 changes: 0 additions & 6 deletions apps/settings/tests/Controller/CheckSetupControllerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,6 @@ protected function setUp(): void {
$this->setupCheckManager,
])
->setMethods([
'hasValidTransactionIsolationLevel',
'hasFileinfoInstalled',
'hasWorkingFileLocking',
'hasDBFileLocking',
Expand Down Expand Up @@ -377,10 +376,6 @@ public function testCheck() {
$this->checkSetupController
->method('isSqliteUsed')
->willReturn(false);
$this->checkSetupController
->expects($this->once())
->method('hasValidTransactionIsolationLevel')
->willReturn(true);
$this->checkSetupController
->expects($this->once())
->method('hasFileinfoInstalled')
Expand Down Expand Up @@ -484,7 +479,6 @@ public function testCheck() {

$expected = new DataResponse(
[
'hasValidTransactionIsolationLevel' => true,
'hasFileinfoInstalled' => true,
'hasWorkingFileLocking' => true,
'hasDBFileLocking' => true,
Expand Down
24 changes: 0 additions & 24 deletions core/js/tests/specs/setupchecksSpec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,6 @@ describe('OC.SetupChecks tests', function() {
hasFileinfoInstalled: true,
hasWorkingFileLocking: true,
hasDBFileLocking: false,
hasValidTransactionIsolationLevel: true,
suggestedOverwriteCliURL: '',
isRandomnessSecure: true,
isFairUseOfFreePushService: true,
Expand Down Expand Up @@ -293,7 +292,6 @@ describe('OC.SetupChecks tests', function() {
hasFileinfoInstalled: true,
hasWorkingFileLocking: true,
hasDBFileLocking: false,
hasValidTransactionIsolationLevel: true,
suggestedOverwriteCliURL: '',
isRandomnessSecure: true,
isFairUseOfFreePushService: true,
Expand Down Expand Up @@ -360,7 +358,6 @@ describe('OC.SetupChecks tests', function() {
hasFileinfoInstalled: true,
hasWorkingFileLocking: true,
hasDBFileLocking: false,
hasValidTransactionIsolationLevel: true,
suggestedOverwriteCliURL: '',
isRandomnessSecure: true,
isFairUseOfFreePushService: true,
Expand Down Expand Up @@ -423,7 +420,6 @@ describe('OC.SetupChecks tests', function() {
hasFileinfoInstalled: true,
hasWorkingFileLocking: true,
hasDBFileLocking: false,
hasValidTransactionIsolationLevel: true,
suggestedOverwriteCliURL: '',
isRandomnessSecure: false,
securityDocs: 'https://docs.nextcloud.com/myDocs.html',
Expand Down Expand Up @@ -485,7 +481,6 @@ describe('OC.SetupChecks tests', function() {
hasFileinfoInstalled: true,
hasWorkingFileLocking: true,
hasDBFileLocking: false,
hasValidTransactionIsolationLevel: true,
suggestedOverwriteCliURL: '',
isRandomnessSecure: true,
securityDocs: 'https://docs.nextcloud.com/myDocs.html',
Expand Down Expand Up @@ -547,7 +542,6 @@ describe('OC.SetupChecks tests', function() {
hasFileinfoInstalled: true,
hasWorkingFileLocking: false,
hasDBFileLocking: false,
hasValidTransactionIsolationLevel: true,
suggestedOverwriteCliURL: '',
isRandomnessSecure: true,
securityDocs: 'https://docs.nextcloud.com/myDocs.html',
Expand Down Expand Up @@ -609,7 +603,6 @@ describe('OC.SetupChecks tests', function() {
hasFileinfoInstalled: true,
hasWorkingFileLocking: true,
hasDBFileLocking: true,
hasValidTransactionIsolationLevel: true,
suggestedOverwriteCliURL: '',
isRandomnessSecure: true,
securityDocs: 'https://docs.nextcloud.com/myDocs.html',
Expand Down Expand Up @@ -671,7 +664,6 @@ describe('OC.SetupChecks tests', function() {
hasFileinfoInstalled: true,
hasWorkingFileLocking: true,
hasDBFileLocking: false,
hasValidTransactionIsolationLevel: true,
suggestedOverwriteCliURL: '',
isRandomnessSecure: true,
securityDocs: 'https://docs.nextcloud.com/myDocs.html',
Expand Down Expand Up @@ -735,7 +727,6 @@ describe('OC.SetupChecks tests', function() {
hasFileinfoInstalled: true,
hasWorkingFileLocking: true,
hasDBFileLocking: false,
hasValidTransactionIsolationLevel: true,
suggestedOverwriteCliURL: '',
isRandomnessSecure: true,
isFairUseOfFreePushService: true,
Expand Down Expand Up @@ -797,7 +788,6 @@ describe('OC.SetupChecks tests', function() {
hasFileinfoInstalled: true,
hasWorkingFileLocking: true,
hasDBFileLocking: false,
hasValidTransactionIsolationLevel: true,
suggestedOverwriteCliURL: '',
isRandomnessSecure: true,
isFairUseOfFreePushService: true,
Expand Down Expand Up @@ -861,7 +851,6 @@ describe('OC.SetupChecks tests', function() {
hasFileinfoInstalled: true,
hasWorkingFileLocking: true,
hasDBFileLocking: false,
hasValidTransactionIsolationLevel: true,
suggestedOverwriteCliURL: '',
isRandomnessSecure: true,
isFairUseOfFreePushService: true,
Expand Down Expand Up @@ -923,7 +912,6 @@ describe('OC.SetupChecks tests', function() {
hasFileinfoInstalled: true,
hasWorkingFileLocking: true,
hasDBFileLocking: false,
hasValidTransactionIsolationLevel: true,
suggestedOverwriteCliURL: '',
isRandomnessSecure: true,
isFairUseOfFreePushService: true,
Expand Down Expand Up @@ -1005,7 +993,6 @@ describe('OC.SetupChecks tests', function() {
hasFileinfoInstalled: true,
hasWorkingFileLocking: true,
hasDBFileLocking: false,
hasValidTransactionIsolationLevel: true,
suggestedOverwriteCliURL: '',
isRandomnessSecure: true,
securityDocs: 'https://docs.nextcloud.com/myDocs.html',
Expand Down Expand Up @@ -1074,7 +1061,6 @@ describe('OC.SetupChecks tests', function() {
hasFileinfoInstalled: true,
hasWorkingFileLocking: true,
hasDBFileLocking: false,
hasValidTransactionIsolationLevel: true,
suggestedOverwriteCliURL: '',
isRandomnessSecure: true,
securityDocs: 'https://docs.nextcloud.com/myDocs.html',
Expand Down Expand Up @@ -1136,7 +1122,6 @@ describe('OC.SetupChecks tests', function() {
hasFileinfoInstalled: true,
hasWorkingFileLocking: true,
hasDBFileLocking: false,
hasValidTransactionIsolationLevel: true,
suggestedOverwriteCliURL: '',
isRandomnessSecure: true,
securityDocs: 'https://docs.nextcloud.com/myDocs.html',
Expand Down Expand Up @@ -1198,7 +1183,6 @@ describe('OC.SetupChecks tests', function() {
hasFileinfoInstalled: true,
hasWorkingFileLocking: true,
hasDBFileLocking: false,
hasValidTransactionIsolationLevel: true,
suggestedOverwriteCliURL: '',
isRandomnessSecure: true,
securityDocs: 'https://docs.nextcloud.com/myDocs.html',
Expand Down Expand Up @@ -1264,7 +1248,6 @@ describe('OC.SetupChecks tests', function() {
hasFileinfoInstalled: true,
hasWorkingFileLocking: true,
hasDBFileLocking: false,
hasValidTransactionIsolationLevel: true,
suggestedOverwriteCliURL: '',
isRandomnessSecure: true,
securityDocs: 'https://docs.nextcloud.com/myDocs.html',
Expand Down Expand Up @@ -1327,7 +1310,6 @@ describe('OC.SetupChecks tests', function() {
hasFileinfoInstalled: true,
hasWorkingFileLocking: true,
hasDBFileLocking: false,
hasValidTransactionIsolationLevel: true,
suggestedOverwriteCliURL: '',
isRandomnessSecure: true,
securityDocs: 'https://docs.nextcloud.com/myDocs.html',
Expand Down Expand Up @@ -1387,7 +1369,6 @@ describe('OC.SetupChecks tests', function() {
hasFileinfoInstalled: true,
hasWorkingFileLocking: true,
hasDBFileLocking: false,
hasValidTransactionIsolationLevel: true,
suggestedOverwriteCliURL: '',
isRandomnessSecure: true,
securityDocs: 'https://docs.nextcloud.com/myDocs.html',
Expand Down Expand Up @@ -1450,7 +1431,6 @@ describe('OC.SetupChecks tests', function() {
hasFileinfoInstalled: true,
hasWorkingFileLocking: true,
hasDBFileLocking: false,
hasValidTransactionIsolationLevel: true,
suggestedOverwriteCliURL: '',
isRandomnessSecure: true,
securityDocs: 'https://docs.nextcloud.com/myDocs.html',
Expand Down Expand Up @@ -1513,7 +1493,6 @@ describe('OC.SetupChecks tests', function() {
hasFileinfoInstalled: true,
hasWorkingFileLocking: true,
hasDBFileLocking: false,
hasValidTransactionIsolationLevel: true,
suggestedOverwriteCliURL: '',
isRandomnessSecure: true,
securityDocs: 'https://docs.nextcloud.com/myDocs.html',
Expand Down Expand Up @@ -1575,7 +1554,6 @@ describe('OC.SetupChecks tests', function() {
hasFileinfoInstalled: true,
hasWorkingFileLocking: true,
hasDBFileLocking: false,
hasValidTransactionIsolationLevel: true,
suggestedOverwriteCliURL: '',
isRandomnessSecure: true,
securityDocs: 'https://docs.nextcloud.com/myDocs.html',
Expand Down Expand Up @@ -1637,7 +1615,6 @@ describe('OC.SetupChecks tests', function() {
hasFileinfoInstalled: true,
hasWorkingFileLocking: true,
hasDBFileLocking: false,
hasValidTransactionIsolationLevel: true,
suggestedOverwriteCliURL: '',
isRandomnessSecure: true,
securityDocs: 'https://docs.nextcloud.com/myDocs.html',
Expand Down Expand Up @@ -1706,7 +1683,6 @@ describe('OC.SetupChecks tests', function() {
hasFileinfoInstalled: true,
hasWorkingFileLocking: true,
hasDBFileLocking: false,
hasValidTransactionIsolationLevel: true,
suggestedOverwriteCliURL: '',
isRandomnessSecure: true,
securityDocs: 'https://docs.nextcloud.com/myDocs.html',
Expand Down