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 <hey@morrisjobke.de>

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');
+					}
+				}
+			}
+		);
 	}
 }

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 += "<li>";
+							listOfMissingIndexes += t('core', 'Missing index "{indexName}" in table "{tableName}".', element);
+							listOfMissingIndexes += "</li>";
+						});
+						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.'
+							) + "<ul>" + listOfMissingIndexes + "</ul>",
+							type: OC.SetupChecks.MESSAGE_TYPE_INFO
+						})
+					}
 				} else {
 					messages.push({
 						msg: t('core', 'Error occurred while checking server setup'),

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',

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',

lib/private/DB/MissingIndexInformation.php

@@ -0,0 +1,39 @@
+<?php
+/**
+ * @copyright Copyright (c) 2018 Morris Jobke <hey@morrisjobke.de>
+ *
+ * @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 OC\DB;
+
+
+class MissingIndexInformation {
+
+	private $listOfMissingIndexes = [];
+
+	public function addHintForMissingSubject($tableName, $indexName) {
+		$this->listOfMissingIndexes[] = [
+			'tableName' => $tableName,
+			'indexName' => $indexName
+		];
+	}
+
+	public function getListOfMissingIndexes() {
+		return $this->listOfMissingIndexes;
+	}
+}

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.

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(),
 			]
 		);
 	}

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 {

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();