From 393d9aae74862e14e80223e44880f4f706c88d6f Mon Sep 17 00:00:00 2001 From: Morris Jobke Date: Mon, 4 Jun 2018 16:20:01 +0200 Subject: [PATCH] Add a hint that some indexes are not added yet * gives the admin a chance to discover the missing indexes and improve the performance of the instance without digging through the manual * nicely integrated in the setup checks where this kind of hints belong to * also adds an option to integrate this from an app based on events * fix style of setting warnings Signed-off-by: Morris Jobke --- core/Application.php | 27 +++++++++++++ core/js/setupchecks.js | 15 +++++++ lib/composer/composer/autoload_classmap.php | 1 + lib/composer/composer/autoload_static.php | 1 + lib/private/DB/MissingIndexInformation.php | 39 +++++++++++++++++++ lib/public/IDBConnection.php | 1 + settings/Controller/CheckSetupController.php | 35 +++++++++++------ settings/css/settings.scss | 22 ++++++++--- .../Controller/CheckSetupControllerTest.php | 17 ++++++-- 9 files changed, 137 insertions(+), 21 deletions(-) create mode 100644 lib/private/DB/MissingIndexInformation.php diff --git a/core/Application.php b/core/Application.php index 9a29b4bcdf..400d86f599 100644 --- a/core/Application.php +++ b/core/Application.php @@ -28,8 +28,12 @@ namespace OC\Core; +use OC\DB\MissingIndexInformation; +use OC\DB\SchemaWrapper; use OCP\AppFramework\App; +use OCP\IDBConnection; use OCP\Util; +use Symfony\Component\EventDispatcher\GenericEvent; /** * Class Application @@ -46,5 +50,28 @@ class Application extends App { $container->registerService('defaultMailAddress', function () { return Util::getDefaultEmailAddress('lostpassword-noreply'); }); + + $server = $container->getServer(); + $eventDispatcher = $server->getEventDispatcher(); + + $eventDispatcher->addListener(IDBConnection::CHECK_MISSING_INDEXES_EVENT, + function(GenericEvent $event) use ($container) { + /** @var MissingIndexInformation $subject */ + $subject = $event->getSubject(); + + $schema = new SchemaWrapper($container->query(IDBConnection::class)); + + if ($schema->hasTable('share')) { + $table = $schema->getTable('share'); + + if (!$table->hasIndex('share_with_index')) { + $subject->addHintForMissingSubject($table->getName(), 'share_with_index'); + } + if (!$table->hasIndex('parent_index')) { + $subject->addHintForMissingSubject($table->getName(), 'parent_index'); + } + } + } + ); } } diff --git a/core/js/setupchecks.js b/core/js/setupchecks.js index a2a7508693..a3155287ac 100644 --- a/core/js/setupchecks.js +++ b/core/js/setupchecks.js @@ -183,6 +183,21 @@ type: OC.SetupChecks.MESSAGE_TYPE_INFO }) } + if (data.hasMissingIndexes) { + var listOfMissingIndexes = ""; + data.hasMissingIndexes.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.' + ) + "", + type: OC.SetupChecks.MESSAGE_TYPE_INFO + }) + } } else { messages.push({ msg: t('core', 'Error occurred while checking server setup'), diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index b5b8e2b169..701f723468 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -579,6 +579,7 @@ return array( 'OC\\DB\\MigrationException' => $baseDir . '/lib/private/DB/MigrationException.php', 'OC\\DB\\MigrationService' => $baseDir . '/lib/private/DB/MigrationService.php', 'OC\\DB\\Migrator' => $baseDir . '/lib/private/DB/Migrator.php', + 'OC\\DB\\MissingIndexInformation' => $baseDir . '/lib/private/DB/MissingIndexInformation.php', 'OC\\DB\\MySQLMigrator' => $baseDir . '/lib/private/DB/MySQLMigrator.php', 'OC\\DB\\MySqlTools' => $baseDir . '/lib/private/DB/MySqlTools.php', 'OC\\DB\\OCSqlitePlatform' => $baseDir . '/lib/private/DB/OCSqlitePlatform.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index 3e0bff2bd6..da4556a718 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -609,6 +609,7 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c 'OC\\DB\\MigrationException' => __DIR__ . '/../../..' . '/lib/private/DB/MigrationException.php', 'OC\\DB\\MigrationService' => __DIR__ . '/../../..' . '/lib/private/DB/MigrationService.php', 'OC\\DB\\Migrator' => __DIR__ . '/../../..' . '/lib/private/DB/Migrator.php', + 'OC\\DB\\MissingIndexInformation' => __DIR__ . '/../../..' . '/lib/private/DB/MissingIndexInformation.php', 'OC\\DB\\MySQLMigrator' => __DIR__ . '/../../..' . '/lib/private/DB/MySQLMigrator.php', 'OC\\DB\\MySqlTools' => __DIR__ . '/../../..' . '/lib/private/DB/MySqlTools.php', 'OC\\DB\\OCSqlitePlatform' => __DIR__ . '/../../..' . '/lib/private/DB/OCSqlitePlatform.php', diff --git a/lib/private/DB/MissingIndexInformation.php b/lib/private/DB/MissingIndexInformation.php new file mode 100644 index 0000000000..d6e40e0b09 --- /dev/null +++ b/lib/private/DB/MissingIndexInformation.php @@ -0,0 +1,39 @@ + + * + * @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 OC\DB; + + +class MissingIndexInformation { + + private $listOfMissingIndexes = []; + + public function addHintForMissingSubject($tableName, $indexName) { + $this->listOfMissingIndexes[] = [ + 'tableName' => $tableName, + 'indexName' => $indexName + ]; + } + + public function getListOfMissingIndexes() { + return $this->listOfMissingIndexes; + } +} \ No newline at end of file diff --git a/lib/public/IDBConnection.php b/lib/public/IDBConnection.php index 024e759de6..4e450eae73 100644 --- a/lib/public/IDBConnection.php +++ b/lib/public/IDBConnection.php @@ -47,6 +47,7 @@ use OCP\DB\QueryBuilder\IQueryBuilder; interface IDBConnection { const ADD_MISSING_INDEXES_EVENT = self::class . '::ADD_MISSING_INDEXES'; + const CHECK_MISSING_INDEXES_EVENT = self::class . '::CHECK_MISSING_INDEXES'; /** * Gets the QueryBuilder for the connection. diff --git a/settings/Controller/CheckSetupController.php b/settings/Controller/CheckSetupController.php index 4debc52fc5..e1073be469 100644 --- a/settings/Controller/CheckSetupController.php +++ b/settings/Controller/CheckSetupController.php @@ -33,6 +33,7 @@ namespace OC\Settings\Controller; use bantu\IniGetWrapper\IniGetWrapper; use GuzzleHttp\Exception\ClientException; use OC\AppFramework\Http; +use OC\DB\MissingIndexInformation; use OC\IntegrityCheck\Checker; use OCP\AppFramework\Controller; use OCP\AppFramework\Http\DataDisplayResponse; @@ -40,10 +41,13 @@ use OCP\AppFramework\Http\DataResponse; use OCP\AppFramework\Http\RedirectResponse; use OCP\Http\Client\IClientService; use OCP\IConfig; +use OCP\IDBConnection; use OCP\IL10N; use OCP\ILogger; use OCP\IRequest; use OCP\IURLGenerator; +use Symfony\Component\EventDispatcher\EventDispatcherInterface; +use Symfony\Component\EventDispatcher\GenericEvent; /** * @package OC\Settings\Controller @@ -63,18 +67,9 @@ class CheckSetupController extends Controller { private $checker; /** @var ILogger */ private $logger; + /** @var EventDispatcherInterface */ + private $dispatcher; - /** - * @param string $AppName - * @param IRequest $request - * @param IConfig $config - * @param IClientService $clientService - * @param IURLGenerator $urlGenerator - * @param \OC_Util $util - * @param IL10N $l10n - * @param Checker $checker - * @param ILogger $logger - */ public function __construct($AppName, IRequest $request, IConfig $config, @@ -83,7 +78,8 @@ class CheckSetupController extends Controller { \OC_Util $util, IL10N $l10n, Checker $checker, - ILogger $logger) { + ILogger $logger, + EventDispatcherInterface $dispatcher) { parent::__construct($AppName, $request); $this->config = $config; $this->clientService = $clientService; @@ -92,6 +88,7 @@ class CheckSetupController extends Controller { $this->l10n = $l10n; $this->checker = $checker; $this->logger = $logger; + $this->dispatcher = $dispatcher; } /** @@ -418,6 +415,19 @@ Raw output return function_exists('imagettfbbox') && function_exists('imagettftext'); } + /** + * Check if the required FreeType functions are present + * @return bool + */ + protected function hasMissingIndexes() { + $indexInfo = new MissingIndexInformation(); + // Dispatch event so apps can also hint for pending index updates if needed + $event = new GenericEvent($indexInfo); + $this->dispatcher->dispatch(IDBConnection::CHECK_MISSING_INDEXES_EVENT, $event); + + return $indexInfo->getListOfMissingIndexes(); + } + /** * @return DataResponse */ @@ -440,6 +450,7 @@ Raw output 'phpOpcacheDocumentation' => $this->urlGenerator->linkToDocs('admin-php-opcache'), 'isSettimelimitAvailable' => $this->isSettimelimitAvailable(), 'hasFreeTypeSupport' => $this->hasFreeTypeSupport(), + 'hasMissingIndexes' => $this->hasMissingIndexes(), ] ); } diff --git a/settings/css/settings.scss b/settings/css/settings.scss index b200a4932c..6adac7704c 100644 --- a/settings/css/settings.scss +++ b/settings/css/settings.scss @@ -1038,12 +1038,6 @@ table.grid td.date { margin-top: 20px; } -#security-warning li { - list-style: initial; - margin: 10px 0; - list-style-position: inside; -} - #security-warning-state span { padding-left: 25px; background-position: 5px center; @@ -1198,6 +1192,18 @@ doesnotexist:-o-prefocus, .strengthify-wrapper { } #postsetupchecks { + ul { + margin-left: 44px; + list-style: disc; + + li { + margin: 10px 0; + } + + ul { + list-style: circle; + } + } .loading { height: 50px; background-position: left center; @@ -1208,6 +1214,10 @@ doesnotexist:-o-prefocus, .strengthify-wrapper { .warnings, .warnings a { color: $color-warning; } + + .hint { + margin: 20px 0; + } } #security-warning > ul { diff --git a/tests/Settings/Controller/CheckSetupControllerTest.php b/tests/Settings/Controller/CheckSetupControllerTest.php index 9faf4a4b71..7760be1649 100644 --- a/tests/Settings/Controller/CheckSetupControllerTest.php +++ b/tests/Settings/Controller/CheckSetupControllerTest.php @@ -34,6 +34,7 @@ use OCP\IRequest; use OCP\IURLGenerator; use OC_Util; use Psr\Http\Message\ResponseInterface; +use Symfony\Component\EventDispatcher\EventDispatcher; use Test\TestCase; use OC\IntegrityCheck\Checker; @@ -61,6 +62,8 @@ class CheckSetupControllerTest extends TestCase { private $logger; /** @var Checker | \PHPUnit_Framework_MockObject_MockObject */ private $checker; + /** @var EventDispatcher|\PHPUnit_Framework_MockObject_MockObject */ + private $dispatcher; public function setUp() { parent::setUp(); @@ -82,6 +85,8 @@ class CheckSetupControllerTest extends TestCase { ->will($this->returnCallback(function($message, array $replace) { return vsprintf($message, $replace); })); + $this->dispatcher = $this->getMockBuilder(EventDispatcher::class) + ->disableOriginalConstructor()->getMock(); $this->checker = $this->getMockBuilder('\OC\IntegrityCheck\Checker') ->disableOriginalConstructor()->getMock(); $this->logger = $this->getMockBuilder(ILogger::class)->getMock(); @@ -95,9 +100,10 @@ class CheckSetupControllerTest extends TestCase { $this->util, $this->l10n, $this->checker, - $this->logger + $this->logger, + $this->dispatcher, ]) - ->setMethods(['getCurlVersion', 'isPhpOutdated', 'isOpcacheProperlySetup', 'hasFreeTypeSupport'])->getMock(); + ->setMethods(['getCurlVersion', 'isPhpOutdated', 'isOpcacheProperlySetup', 'hasFreeTypeSupport', 'hasMissingIndexes'])->getMock(); } public function testIsInternetConnectionWorkingDisabledViaConfig() { @@ -329,6 +335,9 @@ class CheckSetupControllerTest extends TestCase { $this->checkSetupController ->method('hasFreeTypeSupport') ->willReturn(false); + $this->checkSetupController + ->method('hasMissingIndexes') + ->willReturn([]); $expected = new DataResponse( [ @@ -351,6 +360,7 @@ class CheckSetupControllerTest extends TestCase { 'phpOpcacheDocumentation' => 'http://docs.example.org/server/go.php?to=admin-php-opcache', 'isSettimelimitAvailable' => true, 'hasFreeTypeSupport' => false, + 'hasMissingIndexes' => [], ] ); $this->assertEquals($expected, $this->checkSetupController->check()); @@ -367,7 +377,8 @@ class CheckSetupControllerTest extends TestCase { $this->util, $this->l10n, $this->checker, - $this->logger + $this->logger, + $this->dispatcher, ]) ->setMethods(null)->getMock();