From ad88c04f2de08e2adda5dc2031699d892466fd94 Mon Sep 17 00:00:00 2001 From: =?utf8?q?C=C3=B4me=20Chilliet?= Date: Tue, 14 Nov 2023 15:51:26 +0100 Subject: [PATCH] Migrate missing index database check to new API MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- .../composer/composer/autoload_classmap.php | 1 + .../composer/composer/autoload_static.php | 1 + apps/settings/lib/AppInfo/Application.php | 2 + .../lib/Controller/CheckSetupController.php | 26 ------ .../SetupChecks/DatabaseHasMissingIndices.php | 89 +++++++++++++++++++ .../Controller/CheckSetupControllerTest.php | 4 - core/js/setupchecks.js | 12 --- core/js/tests/specs/setupchecksSpec.js | 17 ---- lib/private/DB/MissingIndexInformation.php | 10 +-- 9 files changed, 98 insertions(+), 64 deletions(-) create mode 100644 apps/settings/lib/SetupChecks/DatabaseHasMissingIndices.php diff --git a/apps/settings/composer/composer/autoload_classmap.php b/apps/settings/composer/composer/autoload_classmap.php index c3ce8650443..7c1c1534a74 100644 --- a/apps/settings/composer/composer/autoload_classmap.php +++ b/apps/settings/composer/composer/autoload_classmap.php @@ -76,6 +76,7 @@ return array( 'OCA\\Settings\\SetupChecks\\BruteForceThrottler' => $baseDir . '/../lib/SetupChecks/BruteForceThrottler.php', '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\\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 fe123d74d81..010b19f7243 100644 --- a/apps/settings/composer/composer/autoload_static.php +++ b/apps/settings/composer/composer/autoload_static.php @@ -91,6 +91,7 @@ class ComposerStaticInitSettings 'OCA\\Settings\\SetupChecks\\BruteForceThrottler' => __DIR__ . '/..' . '/../lib/SetupChecks/BruteForceThrottler.php', '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\\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 d73961968f8..65ef8ac74a4 100644 --- a/apps/settings/lib/AppInfo/Application.php +++ b/apps/settings/lib/AppInfo/Application.php @@ -51,6 +51,7 @@ use OCA\Settings\Search\UserSearch; use OCA\Settings\SetupChecks\BruteForceThrottler; use OCA\Settings\SetupChecks\CheckUserCertificates; use OCA\Settings\SetupChecks\DatabaseHasMissingColumns; +use OCA\Settings\SetupChecks\DatabaseHasMissingIndices; use OCA\Settings\SetupChecks\DefaultPhoneRegionSet; use OCA\Settings\SetupChecks\EmailTestSuccessful; use OCA\Settings\SetupChecks\FileLocking; @@ -162,6 +163,7 @@ class Application extends App implements IBootstrap { $context->registerSetupCheck(BruteForceThrottler::class); $context->registerSetupCheck(CheckUserCertificates::class); $context->registerSetupCheck(DatabaseHasMissingColumns::class); + $context->registerSetupCheck(DatabaseHasMissingIndices::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 1fa6a658878..27ee0df4971 100644 --- 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\MissingIndexInformation; use OC\DB\MissingPrimaryKeyInformation; use OC\DB\SchemaWrapper; use OC\IntegrityCheck\Checker; @@ -61,7 +60,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\AddMissingIndicesEvent; use OCP\DB\Events\AddMissingPrimaryKeyEvent; use OCP\DB\Types; use OCP\EventDispatcher\IEventDispatcher; @@ -417,29 +415,6 @@ Raw output return $recommendations; } - protected function hasMissingIndexes(): array { - $indexInfo = new MissingIndexInformation(); - - // Dispatch event so apps can also hint for pending index updates if needed - $event = new AddMissingIndicesEvent(); - $this->dispatcher->dispatchTyped($event); - $missingIndices = $event->getMissingIndices(); - - if ($missingIndices !== []) { - $schema = new SchemaWrapper(\OCP\Server::get(Connection::class)); - foreach ($missingIndices as $missingIndex) { - if ($schema->hasTable($missingIndex['tableName'])) { - $table = $schema->getTable($missingIndex['tableName']); - if (!$table->hasIndex($missingIndex['indexName'])) { - $indexInfo->addHintForMissingSubject($missingIndex['tableName'], $missingIndex['indexName']); - } - } - } - } - - return $indexInfo->getListOfMissingIndexes(); - } - protected function hasMissingPrimaryKeys(): array { $info = new MissingPrimaryKeyInformation(); // Dispatch event so apps can also hint for pending key updates if needed @@ -679,7 +654,6 @@ Raw output 'OpcacheSetupRecommendations' => $this->getOpcacheSetupRecommendations(), 'isSettimelimitAvailable' => $this->isSettimelimitAvailable(), 'missingPrimaryKeys' => $this->hasMissingPrimaryKeys(), - 'missingIndexes' => $this->hasMissingIndexes(), 'isSqliteUsed' => $this->isSqliteUsed(), 'databaseConversionDocumentation' => $this->urlGenerator->linkToDocs('admin-db-conversion'), 'appDirsWithDifferentOwner' => $this->getAppDirsWithDifferentOwner(), diff --git a/apps/settings/lib/SetupChecks/DatabaseHasMissingIndices.php b/apps/settings/lib/SetupChecks/DatabaseHasMissingIndices.php new file mode 100644 index 00000000000..b19c3abb690 --- /dev/null +++ b/apps/settings/lib/SetupChecks/DatabaseHasMissingIndices.php @@ -0,0 +1,89 @@ + + * + * @author Côme Chilliet + * + * @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 . + * + */ +namespace OCA\Settings\SetupChecks; + +use OC\DB\Connection; +use OC\DB\MissingIndexInformation; +use OC\DB\SchemaWrapper; +use OCP\DB\Events\AddMissingIndicesEvent; +use OCP\EventDispatcher\IEventDispatcher; +use OCP\IL10N; +use OCP\SetupCheck\ISetupCheck; +use OCP\SetupCheck\SetupResult; + +class DatabaseHasMissingIndices 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 indices'); + } + + private function getMissingIndices(): array { + $indexInfo = new MissingIndexInformation(); + // Dispatch event so apps can also hint for pending index updates if needed + $event = new AddMissingIndicesEvent(); + $this->dispatcher->dispatchTyped($event); + $missingIndices = $event->getMissingIndices(); + + if (!empty($missingIndices)) { + $schema = new SchemaWrapper($this->connection); + foreach ($missingIndices as $missingIndex) { + if ($schema->hasTable($missingIndex['tableName'])) { + $table = $schema->getTable($missingIndex['tableName']); + if (!$table->hasIndex($missingIndex['indexName'])) { + $indexInfo->addHintForMissingIndex($missingIndex['tableName'], $missingIndex['indexName']); + } + } + } + } + + return $indexInfo->getListOfMissingIndices(); + } + + public function run(): SetupResult { + $missingIndices = $this->getMissingIndices(); + if (empty($missingIndices)) { + return SetupResult::success('None'); + } else { + $list = ''; + foreach ($missingIndices as $missingIndex) { + $list .= "\n".$this->l10n->t('Missing optional index "%s" in table "%s".', [$missingIndex['indexName'], $missingIndex['tableName']]); + } + return SetupResult::info( + $this->l10n->t('The database is missing some indexes. Due to the fact that adding indexes on big tables could take some time they were not added automatically. By running "occ db:add-missing-indices" those missing indexes could be added manually while the instance keeps running. Once the indexes are added queries to those tables are usually much faster.').$list + ); + } + } +} diff --git a/apps/settings/tests/Controller/CheckSetupControllerTest.php b/apps/settings/tests/Controller/CheckSetupControllerTest.php index e5db2d52e3c..a493722a3ce 100644 --- 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', - 'hasMissingIndexes', 'hasMissingPrimaryKeys', 'isSqliteUsed', 'isPHPMailerUsed', @@ -229,9 +228,6 @@ class CheckSetupControllerTest extends TestCase { ->expects($this->once()) ->method('getOpcacheSetupRecommendations') ->willReturn(['recommendation1', 'recommendation2']); - $this->checkSetupController - ->method('hasMissingIndexes') - ->willReturn([]); $this->checkSetupController ->method('hasMissingPrimaryKeys') ->willReturn([]); diff --git a/core/js/setupchecks.js b/core/js/setupchecks.js index 5cd2cadaa2d..f926d1534a5 100644 --- a/core/js/setupchecks.js +++ b/core/js/setupchecks.js @@ -258,18 +258,6 @@ type: OC.SetupChecks.MESSAGE_TYPE_WARNING }); } - if (data.missingIndexes.length > 0) { - var listOfMissingIndexes = ""; - data.missingIndexes.forEach(function(element){ - listOfMissingIndexes += '
  • '; - listOfMissingIndexes += t('core', 'Missing index "{indexName}" in table "{tableName}".', element); - listOfMissingIndexes += '
    • '; - }); - messages.push({ - msg: t('core', 'The database is missing some indexes. Due to the fact that adding indexes on big tables could take some time they were not added automatically. By running "occ db:add-missing-indices" those missing indexes could be added manually while the instance keeps running. Once the indexes are added queries to those tables are usually much faster.') + '
      ' + listOfMissingIndexes + '
    ', - type: OC.SetupChecks.MESSAGE_TYPE_INFO - }) - } if (data.missingPrimaryKeys.length > 0) { var listOfMissingPrimaryKeys = ""; data.missingPrimaryKeys.forEach(function(element){ diff --git a/core/js/tests/specs/setupchecksSpec.js b/core/js/tests/specs/setupchecksSpec.js index 9cba013bb87..d48f92d5dc3 100644 --- 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, - missingIndexes: [], missingPrimaryKeys: [], cronErrors: [], cronInfo: { @@ -281,7 +280,6 @@ describe('OC.SetupChecks tests', function() { hasPassedCodeIntegrityCheck: true, OpcacheSetupRecommendations: [], isSettimelimitAvailable: true, - missingIndexes: [], missingPrimaryKeys: [], cronErrors: [], cronInfo: { @@ -333,7 +331,6 @@ describe('OC.SetupChecks tests', function() { hasPassedCodeIntegrityCheck: true, OpcacheSetupRecommendations: [], isSettimelimitAvailable: true, - missingIndexes: [], missingPrimaryKeys: [], cronErrors: [], cronInfo: { @@ -385,7 +382,6 @@ describe('OC.SetupChecks tests', function() { hasPassedCodeIntegrityCheck: true, OpcacheSetupRecommendations: [], isSettimelimitAvailable: true, - missingIndexes: [], missingPrimaryKeys: [], cronErrors: [], cronInfo: { @@ -435,7 +431,6 @@ describe('OC.SetupChecks tests', function() { hasPassedCodeIntegrityCheck: true, OpcacheSetupRecommendations: [], isSettimelimitAvailable: true, - missingIndexes: [], missingPrimaryKeys: [], cronErrors: [], cronInfo: { @@ -488,7 +483,6 @@ describe('OC.SetupChecks tests', function() { hasPassedCodeIntegrityCheck: true, OpcacheSetupRecommendations: [], isSettimelimitAvailable: false, - missingIndexes: [], missingPrimaryKeys: [], cronErrors: [], cronInfo: { @@ -539,7 +533,6 @@ describe('OC.SetupChecks tests', function() { hasPassedCodeIntegrityCheck: true, OpcacheSetupRecommendations: [], isSettimelimitAvailable: true, - missingIndexes: [], missingPrimaryKeys: [], cronErrors: [], cronInfo: { @@ -621,7 +614,6 @@ describe('OC.SetupChecks tests', function() { hasPassedCodeIntegrityCheck: true, OpcacheSetupRecommendations: [], isSettimelimitAvailable: true, - missingIndexes: [], missingPrimaryKeys: [], cronErrors: [], cronInfo: { @@ -678,7 +670,6 @@ describe('OC.SetupChecks tests', function() { hasPassedCodeIntegrityCheck: true, OpcacheSetupRecommendations: ['recommendation1', 'recommendation2'], isSettimelimitAvailable: true, - missingIndexes: [], missingPrimaryKeys: [], cronErrors: [], cronInfo: { @@ -728,7 +719,6 @@ describe('OC.SetupChecks tests', function() { hasPassedCodeIntegrityCheck: true, OpcacheSetupRecommendations: [], isSettimelimitAvailable: true, - missingIndexes: [], missingPrimaryKeys: [], cronErrors: [], cronInfo: { @@ -782,7 +772,6 @@ describe('OC.SetupChecks tests', function() { hasPassedCodeIntegrityCheck: true, OpcacheSetupRecommendations: [], isSettimelimitAvailable: true, - missingIndexes: [], missingPrimaryKeys: [], cronErrors: [], cronInfo: { @@ -833,7 +822,6 @@ describe('OC.SetupChecks tests', function() { hasPassedCodeIntegrityCheck: true, OpcacheSetupRecommendations: [], isSettimelimitAvailable: true, - missingIndexes: [], missingPrimaryKeys: [], cronErrors: [], cronInfo: { @@ -881,7 +869,6 @@ describe('OC.SetupChecks tests', function() { hasPassedCodeIntegrityCheck: true, OpcacheSetupRecommendations: [], isSettimelimitAvailable: true, - missingIndexes: [], missingPrimaryKeys: [], cronErrors: [], cronInfo: { @@ -932,7 +919,6 @@ describe('OC.SetupChecks tests', function() { hasPassedCodeIntegrityCheck: true, OpcacheSetupRecommendations: [], isSettimelimitAvailable: true, - missingIndexes: [], missingPrimaryKeys: [], cronErrors: [], cronInfo: { @@ -983,7 +969,6 @@ describe('OC.SetupChecks tests', function() { hasPassedCodeIntegrityCheck: true, OpcacheSetupRecommendations: [], isSettimelimitAvailable: true, - missingIndexes: [], missingPrimaryKeys: [], cronErrors: [], cronInfo: { @@ -1033,7 +1018,6 @@ describe('OC.SetupChecks tests', function() { hasPassedCodeIntegrityCheck: true, OpcacheSetupRecommendations: [], isSettimelimitAvailable: true, - missingIndexes: [], missingPrimaryKeys: [], cronErrors: [], cronInfo: { @@ -1090,7 +1074,6 @@ describe('OC.SetupChecks tests', function() { hasPassedCodeIntegrityCheck: true, OpcacheSetupRecommendations: [], isSettimelimitAvailable: true, - missingIndexes: [], missingPrimaryKeys: [], cronErrors: [], cronInfo: { diff --git a/lib/private/DB/MissingIndexInformation.php b/lib/private/DB/MissingIndexInformation.php index 74498668349..4fc3a52d3a4 100644 --- a/lib/private/DB/MissingIndexInformation.php +++ b/lib/private/DB/MissingIndexInformation.php @@ -27,16 +27,16 @@ declare(strict_types=1); namespace OC\DB; class MissingIndexInformation { - private $listOfMissingIndexes = []; + private array $listOfMissingIndices = []; - public function addHintForMissingSubject(string $tableName, string $indexName) { - $this->listOfMissingIndexes[] = [ + public function addHintForMissingIndex(string $tableName, string $indexName): void { + $this->listOfMissingIndices[] = [ 'tableName' => $tableName, 'indexName' => $indexName ]; } - public function getListOfMissingIndexes(): array { - return $this->listOfMissingIndexes; + public function getListOfMissingIndices(): array { + return $this->listOfMissingIndices; } } -- 2.39.5