fix(SchemaChecker): silence reports from db:add-missing-indices

core/Command/Db/CheckSchema.php

@@ -57,7 +57,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
 	}
 
 	/**
-	 * @param array<string, list<array{table: string, type: string, name?: string, changes?: list<string>, app: ?string, enabled: bool}>> $byDisabledApp
+	 * @param array<string, list<array{table: string, type: string, name?: string, changes?: list<string>, app: ?string, enabled: bool, optionalIndex: bool}>> $byDisabledApp
 	 */
 	private function printDisabledAppFindings(array $byDisabledApp, OutputInterface $output): void {
 		if ($byDisabledApp === []) {

lib/private/DB/SchemaChecker.php

@@ -12,11 +12,16 @@
 use Doctrine\DBAL\Schema\Schema;
 use Doctrine\DBAL\Schema\SchemaDiff;
 use Doctrine\DBAL\Schema\TableDiff;
+use Doctrine\DBAL\Types\StringType;
+use Doctrine\DBAL\Types\Type;
 use Doctrine\DBAL\Types\Types;
 use OC\Migration\NullOutput;
 use OCP\App\AppPathNotFoundException;
 use OCP\App\IAppManager;
+use OCP\DB\Events\AddMissingIndicesEvent;
+use OCP\EventDispatcher\IEventDispatcher;
 use OCP\IAppConfig;
+use OCP\IDBConnection;
 
 /**
  * Compares the live database schema against the schema expected for the
@@ -28,11 +33,12 @@ public function __construct(
 		private readonly Connection $connection,
 		private readonly IAppConfig $appConfig,
 		private readonly IAppManager $appManager,
+		private readonly IEventDispatcher $eventDispatcher,
 	) {
 	}
 
 	/**
-	 * @return list<array{table: string, type: string, name?: string, changes?: list<string>, app: ?string, enabled: bool}>
+	 * @return list<array{table: string, type: string, name?: string, changes?: list<string>, app: ?string, enabled: bool, optionalIndex: bool}>
 	 */
 	public function getFindings(?string $onlyTable = null): array {
 		$expectedSchema = new Schema();
@@ -55,6 +61,7 @@ public function getFindings(?string $onlyTable = null): array {
 
 		$this->addMigrationsTable($expectedSchema);
 		$this->materializeUniqueConstraints($expectedSchema);
+		$this->normalizeLongStringColumns($expectedSchema);
 
 		$liveSchema = $this->connection->createSchema();
 
@@ -65,18 +72,21 @@ public function getFindings(?string $onlyTable = null): array {
 
 		$comparator = $this->connection->createSchemaManager()->createComparator();
 		$diff = $comparator->compareSchemas($liveSchema, $expectedSchema);
+		$optionalIndexNames = $this->getOptionalIndexNames();
 
-		return array_map(function (array $finding) use ($disabledAppTableOwners, $enabledApps): array {
+		return array_map(function (array $finding) use ($disabledAppTableOwners, $enabledApps, $optionalIndexNames): array {
 			$app = $disabledAppTableOwners[$finding['table']] ?? null;
 			$finding['app'] = $app;
 			// Only tables owned by a disabled app are non-blocking.
 			$finding['enabled'] = $app === null || $app === 'core' || isset($enabledApps[$app]);
+			$finding['optionalIndex'] = ($finding['type'] === 'missing_index' || $finding['type'] === 'unexpected_index')
+				&& isset($optionalIndexNames[$finding['table']][$finding['name']]);
 			return $finding;
 		}, $this->buildFindings($diff));
 	}
 
 	/**
-	 * @param array{table: string, type: string, name?: string, changes?: list<string>, app?: ?string, enabled?: bool} $finding
+	 * @param array{table: string, type: string, name?: string, changes?: list<string>, app?: ?string, enabled?: bool, optionalIndex?: bool} $finding
 	 */
 	public function formatFinding(array $finding): string {
 		return match ($finding['type']) {
@@ -92,23 +102,28 @@ public function formatFinding(array $finding): string {
 	}
 
 	/**
-	 * Splits findings into blocking ones (from core or an enabled app) and
-	 * non-blocking ones, grouped by the disabled app that owns them.
+	 * Splits findings into blocking ones (from core or an enabled app),
+	 * non-blocking ones grouped by the disabled app that owns them, and
+	 * non-blocking optional-index findings (only relevant if occ
+	 * db:add-missing-indices was never run for that table).
 	 *
-	 * @param list<array{table: string, type: string, name?: string, changes?: list<string>, app: ?string, enabled: bool}> $findings
-	 * @return array{blocking: list<array{table: string, type: string, name?: string, changes?: list<string>, app: ?string, enabled: bool}>, byDisabledApp: array<string, list<array{table: string, type: string, name?: string, changes?: list<string>, app: ?string, enabled: bool}>>}
+	 * @param list<array{table: string, type: string, name?: string, changes?: list<string>, app: ?string, enabled: bool, optionalIndex: bool}> $findings
+	 * @return array{blocking: list<array{table: string, type: string, name?: string, changes?: list<string>, app: ?string, enabled: bool, optionalIndex: bool}>, byDisabledApp: array<string, list<array{table: string, type: string, name?: string, changes?: list<string>, app: ?string, enabled: bool, optionalIndex: bool}>>, optionalIndices: list<array{table: string, type: string, name?: string, changes?: list<string>, app: ?string, enabled: bool, optionalIndex: bool}>}
 	 */
 	public function partitionFindings(array $findings): array {
 		$blocking = [];
 		$byDisabledApp = [];
+		$optionalIndices = [];
 		foreach ($findings as $finding) {
-			if ($finding['enabled']) {
+			if ($finding['optionalIndex']) {
+				$optionalIndices[] = $finding;
+			} elseif ($finding['enabled']) {
 				$blocking[] = $finding;
 			} else {
 				$byDisabledApp[$finding['app']][] = $finding;
 			}
 		}
-		return ['blocking' => $blocking, 'byDisabledApp' => $byDisabledApp];
+		return ['blocking' => $blocking, 'byDisabledApp' => $byDisabledApp, 'optionalIndices' => $optionalIndices];
 	}
 
 	private function applyMigrations(string $app, Schema $schema): void {
@@ -214,6 +229,56 @@ private function materializeUniqueConstraints(Schema $schema): void {
 		}
 	}
 
+	/**
+	 * Migrator::getDiff() (see lib/private/DB/Migrator.php) rewrites any
+	 * STRING column longer than 4000 characters to TEXT before it generates
+	 * DDL, for consistency between the supported databases. That rewrite
+	 * only happens when a migration is actually applied, never when it is
+	 * replayed here to build the expected schema - so without repeating it,
+	 * any such column would forever be reported as a type mismatch.
+	 */
+	private function normalizeLongStringColumns(Schema $schema): void {
+		foreach ($schema->getTables() as $table) {
+			foreach ($table->getColumns() as $column) {
+				if ($column->getType() instanceof StringType && $column->getLength() > 4000) {
+					$column->setType(Type::getType(Types::TEXT));
+					$column->setLength(null);
+				}
+			}
+		}
+	}
+
+	/**
+	 * Apps can register indices that are only ever created or renamed via
+	 * occ db:add-missing-indices (AddMissingIndicesEvent), not through a
+	 * versioned migration. Since running that command is optional, whether
+	 * such an index exists on the live schema depends on whether an admin
+	 * ever ran it - it is not itself a sign of drift in either direction.
+	 * Collect their names here so findings about them can be reported
+	 * separately instead of as blocking missing/unexpected index findings.
+	 *
+	 * @return array<string, array<string, true>> table name => set of index names
+	 */
+	private function getOptionalIndexNames(): array {
+		$event = new AddMissingIndicesEvent();
+		$this->eventDispatcher->dispatchTyped($event);
+
+		$names = [];
+		foreach ($event->getMissingIndices() as $missingIndex) {
+			$table = $this->connection->getPrefix() . $missingIndex['tableName'];
+			$names[$table][$missingIndex['indexName']] = true;
+		}
+		foreach ($event->getIndicesToReplace() as $toReplace) {
+			$table = $this->connection->getPrefix() . $toReplace['tableName'];
+			$names[$table][$toReplace['newIndexName']] = true;
+			foreach ($toReplace['oldIndexNames'] as $oldIndexName) {
+				$names[$table][$oldIndexName] = true;
+			}
+		}
+
+		return $names;
+	}
+
 	private function keepOnlyTable(Schema $schema, string $tableName): void {
 		foreach ($schema->getTables() as $table) {
 			if ($table->getName() !== $tableName) {
@@ -301,7 +366,7 @@ private function getChangedColumnProperties(ColumnDiff $columnDiff): array {
 		if ($columnDiff->hasNotNullChanged()) {
 			$changes[] = 'nullable';
 		}
-		if ($columnDiff->hasDefaultChanged()) {
+		if ($columnDiff->hasDefaultChanged() && !$this->isIgnorableTextDefaultDiff($columnDiff)) {
 			$changes[] = 'default';
 		}
 		if ($columnDiff->hasAutoIncrementChanged()) {
@@ -319,4 +384,20 @@ private function getChangedColumnProperties(ColumnDiff $columnDiff): array {
 
 		return $changes;
 	}
+
+	/**
+	 * MySQL and MariaDB silently ignore a literal DEFAULT clause on TEXT and
+	 * BLOB columns - only NULL is ever actually stored for them. A migration
+	 * that declares such a default therefore always disagrees with the live
+	 * schema on these platforms, even though nothing has actually drifted.
+	 */
+	private function isIgnorableTextDefaultDiff(ColumnDiff $columnDiff): bool {
+		if (!in_array($this->connection->getDatabaseProvider(), [IDBConnection::PLATFORM_MYSQL, IDBConnection::PLATFORM_MARIADB], true)) {
+			return false;
+		}
+
+		$typeName = Type::getTypeRegistry()->lookupName($columnDiff->getNewColumn()->getType());
+
+		return in_array($typeName, [Types::TEXT, Types::BLOB], true);
+	}
 }