Migrate missing primary key database check to new API

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

diff --git a/apps/settings/composer/composer/autoload_classmap.php b/apps/settings/composer/composer/autoload_classmap.php
index 7c1c1534a747839507ebf89f7a462ee6bed990ca..6dbc2518219c83360029ff73b272eefe90174162 100644 (file)
--- a/apps/settings/composer/composer/autoload_classmap.php
+++ b/apps/settings/composer/composer/autoload_classmap.php
@@ -77,6 +77,7 @@ return array(
     'OCA\\Settings\\SetupChecks\\CheckUserCertificates' => $baseDir . '/../lib/SetupChecks/CheckUserCertificates.php',
     'OCA\\Settings\\SetupChecks\\DatabaseHasMissingColumns' => $baseDir . '/../lib/SetupChecks/DatabaseHasMissingColumns.php',
     'OCA\\Settings\\SetupChecks\\DatabaseHasMissingIndices' => $baseDir . '/../lib/SetupChecks/DatabaseHasMissingIndices.php',
+    'OCA\\Settings\\SetupChecks\\DatabaseHasMissingPrimaryKeys' => $baseDir . '/../lib/SetupChecks/DatabaseHasMissingPrimaryKeys.php',
     'OCA\\Settings\\SetupChecks\\DefaultPhoneRegionSet' => $baseDir . '/../lib/SetupChecks/DefaultPhoneRegionSet.php',
     'OCA\\Settings\\SetupChecks\\EmailTestSuccessful' => $baseDir . '/../lib/SetupChecks/EmailTestSuccessful.php',
     'OCA\\Settings\\SetupChecks\\FileLocking' => $baseDir . '/../lib/SetupChecks/FileLocking.php',

diff --git a/apps/settings/composer/composer/autoload_static.php b/apps/settings/composer/composer/autoload_static.php
index 010b19f7243b54dcb7c9568da3885c7e75fd767d..c05f58ac45917411bc860a2233eefef64e17760b 100644 (file)
--- a/apps/settings/composer/composer/autoload_static.php
+++ b/apps/settings/composer/composer/autoload_static.php
@@ -92,6 +92,7 @@ class ComposerStaticInitSettings
         'OCA\\Settings\\SetupChecks\\CheckUserCertificates' => __DIR__ . '/..' . '/../lib/SetupChecks/CheckUserCertificates.php',
         'OCA\\Settings\\SetupChecks\\DatabaseHasMissingColumns' => __DIR__ . '/..' . '/../lib/SetupChecks/DatabaseHasMissingColumns.php',
         'OCA\\Settings\\SetupChecks\\DatabaseHasMissingIndices' => __DIR__ . '/..' . '/../lib/SetupChecks/DatabaseHasMissingIndices.php',
+        'OCA\\Settings\\SetupChecks\\DatabaseHasMissingPrimaryKeys' => __DIR__ . '/..' . '/../lib/SetupChecks/DatabaseHasMissingPrimaryKeys.php',
         'OCA\\Settings\\SetupChecks\\DefaultPhoneRegionSet' => __DIR__ . '/..' . '/../lib/SetupChecks/DefaultPhoneRegionSet.php',
         'OCA\\Settings\\SetupChecks\\EmailTestSuccessful' => __DIR__ . '/..' . '/../lib/SetupChecks/EmailTestSuccessful.php',
         'OCA\\Settings\\SetupChecks\\FileLocking' => __DIR__ . '/..' . '/../lib/SetupChecks/FileLocking.php',

diff --git a/apps/settings/lib/AppInfo/Application.php b/apps/settings/lib/AppInfo/Application.php
index 65ef8ac74a4c03c7634f3d965999868a630b46e8..d96715a9ecaa3f7d417ba1c56699eeb0a9835487 100644 (file)
--- a/apps/settings/lib/AppInfo/Application.php
+++ b/apps/settings/lib/AppInfo/Application.php
@@ -52,6 +52,7 @@ use OCA\Settings\SetupChecks\BruteForceThrottler;
 use OCA\Settings\SetupChecks\CheckUserCertificates;
 use OCA\Settings\SetupChecks\DatabaseHasMissingColumns;
 use OCA\Settings\SetupChecks\DatabaseHasMissingIndices;
+use OCA\Settings\SetupChecks\DatabaseHasMissingPrimaryKeys;
 use OCA\Settings\SetupChecks\DefaultPhoneRegionSet;
 use OCA\Settings\SetupChecks\EmailTestSuccessful;
 use OCA\Settings\SetupChecks\FileLocking;
@@ -164,6 +165,7 @@ class Application extends App implements IBootstrap {
                $context->registerSetupCheck(CheckUserCertificates::class);
                $context->registerSetupCheck(DatabaseHasMissingColumns::class);
                $context->registerSetupCheck(DatabaseHasMissingIndices::class);
+               $context->registerSetupCheck(DatabaseHasMissingPrimaryKeys::class);
                $context->registerSetupCheck(DefaultPhoneRegionSet::class);
                $context->registerSetupCheck(EmailTestSuccessful::class);
                $context->registerSetupCheck(FileLocking::class);

diff --git a/apps/settings/lib/Controller/CheckSetupController.php b/apps/settings/lib/Controller/CheckSetupController.php
index 27ee0df49716fbb39dd5a7bd402e37e4bb9ac7d0..7bb2c8319c2e3e45d264124f0800723952935788 100644 (file)
--- a/apps/settings/lib/Controller/CheckSetupController.php
+++ b/apps/settings/lib/Controller/CheckSetupController.php
@@ -51,7 +51,6 @@ use GuzzleHttp\Exception\ClientException;
 use OC;
 use OC\AppFramework\Http;
 use OC\DB\Connection;
-use OC\DB\MissingPrimaryKeyInformation;
 use OC\DB\SchemaWrapper;
 use OC\IntegrityCheck\Checker;
 use OCP\App\IAppManager;
@@ -60,7 +59,6 @@ use OCP\AppFramework\Http\Attribute\IgnoreOpenAPI;
 use OCP\AppFramework\Http\DataDisplayResponse;
 use OCP\AppFramework\Http\DataResponse;
 use OCP\AppFramework\Http\RedirectResponse;
-use OCP\DB\Events\AddMissingPrimaryKeyEvent;
 use OCP\DB\Types;
 use OCP\EventDispatcher\IEventDispatcher;
 use OCP\Http\Client\IClientService;
@@ -415,28 +413,6 @@ Raw output
                return $recommendations;
        }
 
-       protected function hasMissingPrimaryKeys(): array {
-               $info = new MissingPrimaryKeyInformation();
-               // Dispatch event so apps can also hint for pending key updates if needed
-               $event = new AddMissingPrimaryKeyEvent();
-               $this->dispatcher->dispatchTyped($event);
-               $missingKeys = $event->getMissingPrimaryKeys();
-
-               if (!empty($missingKeys)) {
-                       $schema = new SchemaWrapper(\OCP\Server::get(Connection::class));
-                       foreach ($missingKeys as $missingKey) {
-                               if ($schema->hasTable($missingKey['tableName'])) {
-                                       $table = $schema->getTable($missingKey['tableName']);
-                                       if (!$table->hasPrimaryKey()) {
-                                               $info->addHintForMissingSubject($missingKey['tableName']);
-                                       }
-                               }
-                       }
-               }
-
-               return $info->getListOfMissingPrimaryKeys();
-       }
-
        protected function isSqliteUsed() {
                return str_contains($this->config->getSystemValue('dbtype'), 'sqlite');
        }
@@ -653,7 +629,6 @@ Raw output
                                'codeIntegrityCheckerDocumentation' => $this->urlGenerator->linkToDocs('admin-code-integrity'),
                                'OpcacheSetupRecommendations' => $this->getOpcacheSetupRecommendations(),
                                'isSettimelimitAvailable' => $this->isSettimelimitAvailable(),
-                               'missingPrimaryKeys' => $this->hasMissingPrimaryKeys(),
                                'isSqliteUsed' => $this->isSqliteUsed(),
                                'databaseConversionDocumentation' => $this->urlGenerator->linkToDocs('admin-db-conversion'),
                                'appDirsWithDifferentOwner' => $this->getAppDirsWithDifferentOwner(),

diff --git a/apps/settings/lib/SetupChecks/DatabaseHasMissingPrimaryKeys.php b/apps/settings/lib/SetupChecks/DatabaseHasMissingPrimaryKeys.php
new file mode 100644 (file)
index 0000000..ca872fa
--- /dev/null
+++ b/apps/settings/lib/SetupChecks/DatabaseHasMissingPrimaryKeys.php
@@ -0,0 +1,89 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * @copyright Copyright (c) 2023 Côme Chilliet <come.chilliet@nextcloud.com>
+ *
+ * @author Côme Chilliet <come.chilliet@nextcloud.com>
+ *
+ * @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 OC\DB\Connection;
+use OC\DB\MissingPrimaryKeyInformation;
+use OC\DB\SchemaWrapper;
+use OCP\DB\Events\AddMissingPrimaryKeyEvent;
+use OCP\EventDispatcher\IEventDispatcher;
+use OCP\IL10N;
+use OCP\SetupCheck\ISetupCheck;
+use OCP\SetupCheck\SetupResult;
+
+class DatabaseHasMissingPrimaryKeys implements ISetupCheck {
+       public function __construct(
+               private IL10N $l10n,
+               private Connection $connection,
+               private IEventDispatcher $dispatcher,
+       ) {
+       }
+
+       public function getCategory(): string {
+               return 'database';
+       }
+
+       public function getName(): string {
+               return $this->l10n->t('Database missing primary keys');
+       }
+
+       private function getMissingPrimaryKeys(): array {
+               $primaryKeyInfo = new MissingPrimaryKeyInformation();
+               // Dispatch event so apps can also hint for pending primary key updates if needed
+               $event = new AddMissingPrimaryKeyEvent();
+               $this->dispatcher->dispatchTyped($event);
+               $missingPrimaryKeys = $event->getMissingPrimaryKeys();
+
+               if (!empty($missingPrimaryKeys)) {
+                       $schema = new SchemaWrapper($this->connection);
+                       foreach ($missingPrimaryKeys as $missingPrimaryKey) {
+                               if ($schema->hasTable($missingPrimaryKey['tableName'])) {
+                                       $table = $schema->getTable($missingPrimaryKey['tableName']);
+                                       if (!$table->hasPrimaryKey()) {
+                                               $primaryKeyInfo->addHintForMissingPrimaryKey($missingPrimaryKey['tableName']);
+                                       }
+                               }
+                       }
+               }
+
+               return $primaryKeyInfo->getListOfMissingPrimaryKeys();
+       }
+
+       public function run(): SetupResult {
+               $missingPrimaryKeys = $this->getMissingPrimaryKeys();
+               if (empty($missingPrimaryKeys)) {
+                       return SetupResult::success('None');
+               } else {
+                       $list = '';
+                       foreach ($missingPrimaryKeys as $missingPrimaryKey) {
+                               $list .= "\n".$this->l10n->t('Missing primary key on table "%s".', [$missingPrimaryKey['tableName']]);
+                       }
+                       return SetupResult::info(
+                               $this->l10n->t('The database is missing some primary keys. Due to the fact that adding primary keys on big tables could take some time they were not added automatically. By running "occ db:add-missing-primary-keys" those missing primary keys could be added manually while the instance keeps running.').$list
+                       );
+               }
+       }
+}

diff --git a/apps/settings/tests/Controller/CheckSetupControllerTest.php b/apps/settings/tests/Controller/CheckSetupControllerTest.php
index a493722a3ce18b7afb0b315f495f87c2d4cefa26..fe299b6e7999ecf741cdaaee1e0b0fe4e9e9dc43 100644 (file)
--- a/apps/settings/tests/Controller/CheckSetupControllerTest.php
+++ b/apps/settings/tests/Controller/CheckSetupControllerTest.php
@@ -178,7 +178,6 @@ class CheckSetupControllerTest extends TestCase {
                                'getCurlVersion',
                                'isPhpOutdated',
                                'getOpcacheSetupRecommendations',
-                               'hasMissingPrimaryKeys',
                                'isSqliteUsed',
                                'isPHPMailerUsed',
                                'getAppDirsWithDifferentOwner',
@@ -228,9 +227,6 @@ class CheckSetupControllerTest extends TestCase {
                        ->expects($this->once())
                        ->method('getOpcacheSetupRecommendations')
                        ->willReturn(['recommendation1', 'recommendation2']);
-               $this->checkSetupController
-                       ->method('hasMissingPrimaryKeys')
-                       ->willReturn([]);
                $this->checkSetupController
                        ->method('isSqliteUsed')
                        ->willReturn(false);

diff --git a/core/js/setupchecks.js b/core/js/setupchecks.js
index f926d1534a56cc9c6c113ce80f45d1918bbc11fa..24fec85aeb3e0fede861bc7e13afb17b48b8efa1 100644 (file)
--- a/core/js/setupchecks.js
+++ b/core/js/setupchecks.js
@@ -258,18 +258,6 @@
                                                        type: OC.SetupChecks.MESSAGE_TYPE_WARNING
                                                });
                                        }
-                                       if (data.missingPrimaryKeys.length > 0) {
-                                               var listOfMissingPrimaryKeys = "";
-                                               data.missingPrimaryKeys.forEach(function(element){
-                                                       listOfMissingPrimaryKeys += '<li>';
-                                                       listOfMissingPrimaryKeys += t('core', 'Missing primary key on table "{tableName}".', element);
-                                                       listOfMissingPrimaryKeys += '</li>';
-                                               });
-                                               messages.push({
-                                                       msg: t('core', 'The database is missing some primary keys. Due to the fact that adding primary keys on big tables could take some time they were not added automatically. By running "occ db:add-missing-primary-keys" those missing primary keys could be added manually while the instance keeps running.') + '<ul>' + listOfMissingPrimaryKeys + '</ul>',
-                                                       type: OC.SetupChecks.MESSAGE_TYPE_INFO
-                                               })
-                                       }
                                        if (!data.isImagickEnabled) {
                                                messages.push({
                                                        msg: t(

diff --git a/core/js/tests/specs/setupchecksSpec.js b/core/js/tests/specs/setupchecksSpec.js
index d48f92d5dc328dc0e461efeef0552e402f4ba383..9020a895af557aa14066fe264154624e148b5144 100644 (file)
--- a/core/js/tests/specs/setupchecksSpec.js
+++ b/core/js/tests/specs/setupchecksSpec.js
@@ -229,7 +229,6 @@ describe('OC.SetupChecks tests', function() {
                                        hasPassedCodeIntegrityCheck: true,
                                        OpcacheSetupRecommendations: [],
                                        isSettimelimitAvailable: true,
-                                       missingPrimaryKeys: [],
                                        cronErrors: [],
                                        cronInfo: {
                                                diffInSeconds: 0
@@ -280,7 +279,6 @@ describe('OC.SetupChecks tests', function() {
                                        hasPassedCodeIntegrityCheck: true,
                                        OpcacheSetupRecommendations: [],
                                        isSettimelimitAvailable: true,
-                                       missingPrimaryKeys: [],
                                        cronErrors: [],
                                        cronInfo: {
                                                diffInSeconds: 0
@@ -331,7 +329,6 @@ describe('OC.SetupChecks tests', function() {
                                        hasPassedCodeIntegrityCheck: true,
                                        OpcacheSetupRecommendations: [],
                                        isSettimelimitAvailable: true,
-                                       missingPrimaryKeys: [],
                                        cronErrors: [],
                                        cronInfo: {
                                                diffInSeconds: 0
@@ -382,7 +379,6 @@ describe('OC.SetupChecks tests', function() {
                                        hasPassedCodeIntegrityCheck: true,
                                        OpcacheSetupRecommendations: [],
                                        isSettimelimitAvailable: true,
-                                       missingPrimaryKeys: [],
                                        cronErrors: [],
                                        cronInfo: {
                                                diffInSeconds: 0
@@ -431,7 +427,6 @@ describe('OC.SetupChecks tests', function() {
                                        hasPassedCodeIntegrityCheck: true,
                                        OpcacheSetupRecommendations: [],
                                        isSettimelimitAvailable: true,
-                                       missingPrimaryKeys: [],
                                        cronErrors: [],
                                        cronInfo: {
                                                diffInSeconds: 0
@@ -483,7 +478,6 @@ describe('OC.SetupChecks tests', function() {
                                        hasPassedCodeIntegrityCheck: true,
                                        OpcacheSetupRecommendations: [],
                                        isSettimelimitAvailable: false,
-                                       missingPrimaryKeys: [],
                                        cronErrors: [],
                                        cronInfo: {
                                                diffInSeconds: 0
@@ -533,7 +527,6 @@ describe('OC.SetupChecks tests', function() {
                                        hasPassedCodeIntegrityCheck: true,
                                        OpcacheSetupRecommendations: [],
                                        isSettimelimitAvailable: true,
-                                       missingPrimaryKeys: [],
                                        cronErrors: [],
                                        cronInfo: {
                                                diffInSeconds: 0
@@ -614,7 +607,6 @@ describe('OC.SetupChecks tests', function() {
                                        hasPassedCodeIntegrityCheck: true,
                                        OpcacheSetupRecommendations: [],
                                        isSettimelimitAvailable: true,
-                                       missingPrimaryKeys: [],
                                        cronErrors: [],
                                        cronInfo: {
                                                diffInSeconds: 0
@@ -670,7 +662,6 @@ describe('OC.SetupChecks tests', function() {
                                        hasPassedCodeIntegrityCheck: true,
                                        OpcacheSetupRecommendations: ['recommendation1', 'recommendation2'],
                                        isSettimelimitAvailable: true,
-                                       missingPrimaryKeys: [],
                                        cronErrors: [],
                                        cronInfo: {
                                                diffInSeconds: 0
@@ -719,7 +710,6 @@ describe('OC.SetupChecks tests', function() {
                                        hasPassedCodeIntegrityCheck: true,
                                        OpcacheSetupRecommendations: [],
                                        isSettimelimitAvailable: true,
-                                       missingPrimaryKeys: [],
                                        cronErrors: [],
                                        cronInfo: {
                                                diffInSeconds: 0
@@ -772,7 +762,6 @@ describe('OC.SetupChecks tests', function() {
                                        hasPassedCodeIntegrityCheck: true,
                                        OpcacheSetupRecommendations: [],
                                        isSettimelimitAvailable: true,
-                                       missingPrimaryKeys: [],
                                        cronErrors: [],
                                        cronInfo: {
                                                diffInSeconds: 0
@@ -822,7 +811,6 @@ describe('OC.SetupChecks tests', function() {
                                        hasPassedCodeIntegrityCheck: true,
                                        OpcacheSetupRecommendations: [],
                                        isSettimelimitAvailable: true,
-                                       missingPrimaryKeys: [],
                                        cronErrors: [],
                                        cronInfo: {
                                                diffInSeconds: 0
@@ -869,7 +857,6 @@ describe('OC.SetupChecks tests', function() {
                                        hasPassedCodeIntegrityCheck: true,
                                        OpcacheSetupRecommendations: [],
                                        isSettimelimitAvailable: true,
-                                       missingPrimaryKeys: [],
                                        cronErrors: [],
                                        cronInfo: {
                                                diffInSeconds: 0
@@ -919,7 +906,6 @@ describe('OC.SetupChecks tests', function() {
                                        hasPassedCodeIntegrityCheck: true,
                                        OpcacheSetupRecommendations: [],
                                        isSettimelimitAvailable: true,
-                                       missingPrimaryKeys: [],
                                        cronErrors: [],
                                        cronInfo: {
                                                diffInSeconds: 0
@@ -969,7 +955,6 @@ describe('OC.SetupChecks tests', function() {
                                        hasPassedCodeIntegrityCheck: true,
                                        OpcacheSetupRecommendations: [],
                                        isSettimelimitAvailable: true,
-                                       missingPrimaryKeys: [],
                                        cronErrors: [],
                                        cronInfo: {
                                                diffInSeconds: 0
@@ -1018,7 +1003,6 @@ describe('OC.SetupChecks tests', function() {
                                        hasPassedCodeIntegrityCheck: true,
                                        OpcacheSetupRecommendations: [],
                                        isSettimelimitAvailable: true,
-                                       missingPrimaryKeys: [],
                                        cronErrors: [],
                                        cronInfo: {
                                                diffInSeconds: 0
@@ -1074,7 +1058,6 @@ describe('OC.SetupChecks tests', function() {
                                        hasPassedCodeIntegrityCheck: true,
                                        OpcacheSetupRecommendations: [],
                                        isSettimelimitAvailable: true,
-                                       missingPrimaryKeys: [],
                                        cronErrors: [],
                                        cronInfo: {
                                                diffInSeconds: 0

diff --git a/lib/private/DB/MissingPrimaryKeyInformation.php b/lib/private/DB/MissingPrimaryKeyInformation.php
index f28c8cfb35215f86e621df070177671c1bfe7d0e..42e5584291c58d3a0f48ff6b53c55e9115e43398 100644 (file)
--- a/lib/private/DB/MissingPrimaryKeyInformation.php
+++ b/lib/private/DB/MissingPrimaryKeyInformation.php
@@ -26,9 +26,9 @@ declare(strict_types=1);
 namespace OC\DB;
 
 class MissingPrimaryKeyInformation {
-       private $listOfMissingPrimaryKeys = [];
+       private array $listOfMissingPrimaryKeys = [];
 
-       public function addHintForMissingSubject(string $tableName) {
+       public function addHintForMissingPrimaryKey(string $tableName): void {
                $this->listOfMissingPrimaryKeys[] = [
                        'tableName' => $tableName,
                ];