From 95ea46c99c6618ead70cda69d2cf81ce05507aa8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Thu, 16 Nov 2023 11:48:04 +0100 Subject: [PATCH] Merge SQlite warning to existing SupportedDatabase setup check 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 | 6 ---- .../lib/SetupChecks/SupportedDatabase.php | 7 +++- .../Controller/CheckSetupControllerTest.php | 6 ---- .../SetupChecks/SupportedDatabaseTest.php | 33 +++++++++++++++++-- core/js/setupchecks.js | 9 ----- 5 files changed, 36 insertions(+), 25 deletions(-) diff --git a/apps/settings/lib/Controller/CheckSetupController.php b/apps/settings/lib/Controller/CheckSetupController.php index 07ab12426bb0d..14f7068ce59a1 100644 --- a/apps/settings/lib/Controller/CheckSetupController.php +++ b/apps/settings/lib/Controller/CheckSetupController.php @@ -401,10 +401,6 @@ protected function getOpcacheSetupRecommendations(): array { return $recommendations; } - protected function isSqliteUsed() { - return str_contains($this->config->getSystemValue('dbtype'), 'sqlite'); - } - protected function getSuggestedOverwriteCliURL(): string { $currentOverwriteCliUrl = $this->config->getSystemValue('overwrite.cli.url', ''); $suggestedOverwriteCliUrl = $this->request->getServerProtocol() . '://' . $this->request->getInsecureServerHost() . \OC::$WEBROOT; @@ -574,8 +570,6 @@ public function check() { 'codeIntegrityCheckerDocumentation' => $this->urlGenerator->linkToDocs('admin-code-integrity'), 'OpcacheSetupRecommendations' => $this->getOpcacheSetupRecommendations(), 'isSettimelimitAvailable' => $this->isSettimelimitAvailable(), - 'isSqliteUsed' => $this->isSqliteUsed(), - 'databaseConversionDocumentation' => $this->urlGenerator->linkToDocs('admin-db-conversion'), 'appDirsWithDifferentOwner' => $this->getAppDirsWithDifferentOwner(), 'isImagickEnabled' => $this->isImagickEnabled(), 'areWebauthnExtensionsEnabled' => $this->areWebauthnExtensionsEnabled(), diff --git a/apps/settings/lib/SetupChecks/SupportedDatabase.php b/apps/settings/lib/SetupChecks/SupportedDatabase.php index 7cb4820952fc0..5739bc17958f5 100644 --- a/apps/settings/lib/SetupChecks/SupportedDatabase.php +++ b/apps/settings/lib/SetupChecks/SupportedDatabase.php @@ -33,12 +33,14 @@ use Doctrine\DBAL\Platforms\SqlitePlatform; use OCP\IDBConnection; use OCP\IL10N; +use OCP\IURLGenerator; use OCP\SetupCheck\ISetupCheck; use OCP\SetupCheck\SetupResult; class SupportedDatabase implements ISetupCheck { public function __construct( private IL10N $l10n, + private IURLGenerator $urlGenerator, private IDBConnection $connection, ) { } @@ -81,7 +83,10 @@ public function run(): SetupResult { } elseif ($databasePlatform instanceof OraclePlatform) { $version = 'Oracle'; } elseif ($databasePlatform instanceof SqlitePlatform) { - $version = 'Sqlite'; + return SetupResult::warning( + $this->l10n->t('SQLite is currently being used as the backend database. For larger installations we recommend that you switch to a different database backend. This is particularly recommended when using the desktop client for file synchronisation. To migrate to another database use the command line tool: "occ db:convert-type".'), + $this->urlGenerator->linkToDocs('admin-db-conversion') + ); } else { return SetupResult::error($this->l10n->t('Unknown database platform')); } diff --git a/apps/settings/tests/Controller/CheckSetupControllerTest.php b/apps/settings/tests/Controller/CheckSetupControllerTest.php index bd3ed270cdd68..a4cd6bb0a961e 100644 --- a/apps/settings/tests/Controller/CheckSetupControllerTest.php +++ b/apps/settings/tests/Controller/CheckSetupControllerTest.php @@ -165,7 +165,6 @@ protected function setUp(): void { 'getCurlVersion', 'isPhpOutdated', 'getOpcacheSetupRecommendations', - 'isSqliteUsed', 'isPHPMailerUsed', 'getAppDirsWithDifferentOwner', 'isImagickEnabled', @@ -213,9 +212,6 @@ public function testCheck() { ->expects($this->once()) ->method('getOpcacheSetupRecommendations') ->willReturn(['recommendation1', 'recommendation2']); - $this->checkSetupController - ->method('isSqliteUsed') - ->willReturn(false); $this->checkSetupController ->expects($this->once()) ->method('getSuggestedOverwriteCliURL') @@ -305,8 +301,6 @@ public function testCheck() { 'codeIntegrityCheckerDocumentation' => 'http://docs.example.org/server/go.php?to=admin-code-integrity', 'OpcacheSetupRecommendations' => ['recommendation1', 'recommendation2'], 'isSettimelimitAvailable' => true, - 'isSqliteUsed' => false, - 'databaseConversionDocumentation' => 'http://docs.example.org/server/go.php?to=admin-db-conversion', 'appDirsWithDifferentOwner' => [], 'isImagickEnabled' => false, 'areWebauthnExtensionsEnabled' => false, diff --git a/apps/settings/tests/SetupChecks/SupportedDatabaseTest.php b/apps/settings/tests/SetupChecks/SupportedDatabaseTest.php index 5521bec34b56a..aede25475c5f9 100644 --- a/apps/settings/tests/SetupChecks/SupportedDatabaseTest.php +++ b/apps/settings/tests/SetupChecks/SupportedDatabaseTest.php @@ -25,8 +25,11 @@ */ namespace OCA\Settings\Tests; +use Doctrine\DBAL\Platforms\SqlitePlatform; use OCA\Settings\SetupChecks\SupportedDatabase; +use OCP\IDBConnection; use OCP\IL10N; +use OCP\IUrlGenerator; use OCP\SetupCheck\SetupResult; use Test\TestCase; @@ -34,9 +37,33 @@ * @group DB */ class SupportedDatabaseTest extends TestCase { + private IL10N $l10n; + private IUrlGenerator $urlGenerator; + private IDBConnection $connection; + + private SupportedDatabase $check; + + protected function setUp(): void { + parent::setUp(); + + $this->l10n = $this->getMockBuilder(IL10N::class)->getMock(); + $this->urlGenerator = $this->getMockBuilder(IUrlGenerator::class)->getMock(); + $this->connection = \OCP\Server::get(IDBConnection::class); + + $this->check = new SupportedDatabase( + $this->l10n, + $this->urlGenerator, + \OCP\Server::get(IDBConnection::class) + ); + } + public function testPass(): void { - $l10n = $this->getMockBuilder(IL10N::class)->getMock(); - $check = new SupportedDatabase($l10n, \OC::$server->getDatabaseConnection()); - $this->assertEquals(SetupResult::SUCCESS, $check->run()->getSeverity()); + $platform = $this->connection->getDatabasePlatform(); + if ($platform instanceof SqlitePlatform) { + /** SQlite always gets a warning */ + $this->assertEquals(SetupResult::WARNING, $this->check->run()->getSeverity()); + } else { + $this->assertEquals(SetupResult::SUCCESS, $this->check->run()->getSeverity()); + } } } diff --git a/core/js/setupchecks.js b/core/js/setupchecks.js index c947f3c30e742..e7ad920760eda 100644 --- a/core/js/setupchecks.js +++ b/core/js/setupchecks.js @@ -282,15 +282,6 @@ type: OC.SetupChecks.MESSAGE_TYPE_INFO }) } - if (data.isSqliteUsed) { - messages.push({ - msg: t('core', 'SQLite is currently being used as the backend database. For larger installations we recommend that you switch to a different database backend.') + ' ' + t('core', 'This is particularly recommended when using the desktop client for file synchronisation.') + ' ' + - t('core', 'To migrate to another database use the command line tool: "occ db:convert-type", or see the {linkstart}documentation ↗{linkend}.') - .replace('{linkstart}', '') - .replace('{linkend}', ''), - type: OC.SetupChecks.MESSAGE_TYPE_WARNING - }) - } if(data.appDirsWithDifferentOwner && data.appDirsWithDifferentOwner.length > 0) { var appDirsWithDifferentOwner = data.appDirsWithDifferentOwner.reduce(