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

core/Command/Db/CheckSchema.php

@@ -39,15 +39,15 @@ protected function execute(InputInterface $input, OutputInterface $output): int
 		['blocking' => $blocking, 'byDisabledApp' => $byDisabledApp] = $this->schemaChecker->partitionFindings($findings);
 
 		if ($input->getOption('output') === self::OUTPUT_FORMAT_PLAIN) {
-			if ($findings === []) {
+			if ($blocking === []) {
 				$output->writeln('<info>The live database schema matches the expected schema.</info>');
 			} else {
 				foreach ($blocking as $finding) {
 					$output->writeln('<comment>' . $this->schemaChecker->formatFinding($finding) . '</comment>');
 				}
-				if ($output->isVerbose()) {
-					$this->printDisabledAppFindings($byDisabledApp, $output);
-				}
+			}
+			if ($output->isVerbose()) {
+				$this->printDisabledAppFindings($byDisabledApp, $output);
 			}
 		} else {
 			$this->writeArrayInOutputFormat($input, $output, $findings);

lib/private/DB/SchemaChecker.php

@@ -12,11 +12,17 @@
 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;
+use Psr\Log\LoggerInterface;
 
 /**
  * Compares the live database schema against the schema expected for the
@@ -28,6 +34,8 @@ public function __construct(
 		private readonly Connection $connection,
 		private readonly IAppConfig $appConfig,
 		private readonly IAppManager $appManager,
+		private readonly IEventDispatcher $eventDispatcher,
+		private readonly LoggerInterface $logger,
 	) {
 	}
 
@@ -60,6 +68,7 @@ public function getFindings(?string $onlyTable = null): array {
 
 		$this->addMigrationsTable($expectedSchema);
 		$this->materializeUniqueConstraints($expectedSchema);
+		$this->normalizeLongStringColumns($expectedSchema);
 
 		$liveSchema = $this->connection->createSchema();
 
@@ -70,6 +79,8 @@ public function getFindings(?string $onlyTable = null): array {
 
 		$comparator = $this->connection->createSchemaManager()->createComparator();
 		$diff = $comparator->compareSchemas($liveSchema, $expectedSchema);
+		$optionalIndexNames = $this->getOptionalIndexNames();
+		$findings = array_filter($this->buildFindings($diff), fn (array $finding): bool => !$this->isOptionalIndexFinding($finding, $optionalIndexNames));
 
 		return array_map(function (array $finding) use ($disabledAppTableOwners, $enabledApps): array {
 			$app = $disabledAppTableOwners[$finding['table']] ?? null;
@@ -84,7 +95,7 @@ public function getFindings(?string $onlyTable = null): array {
 				$finding['enabled'] = $app === null || $app === 'core' || isset($enabledApps[$app]);
 			}
 			return $finding;
-		}, $this->buildFindings($diff));
+		}, array_values($findings));
 	}
 
 	/**
@@ -170,13 +181,18 @@ private function applyDisabledMigrations(string $app, Schema $schema, array &$di
 			}
 
 			$this->applyMigrations($app, $schema);
-		} catch (\Throwable) {
-			return;
-		}
-
-		foreach ($schema->getTables() as $table) {
-			if (!isset($existingTables[$table->getName()])) {
-				$disabledAppTableOwners[$table->getName()] = $app;
+		} catch (\Throwable $e) {
+			$this->logger->warning('Could not replay migrations for disabled app {app}', [
+				'app' => $app,
+				'exception' => $e,
+			]);
+		} finally {
+			// Attribute whatever was applied before a failure too, so it
+			// isn't misreported as blocking drift owned by no app.
+			foreach ($schema->getTables() as $table) {
+				if (!isset($existingTables[$table->getName()])) {
+					$disabledAppTableOwners[$table->getName()] = $app;
+				}
 			}
 		}
 	}
@@ -237,6 +253,65 @@ 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 filtered out
+	 * entirely, rather than reported as 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;
+	}
+
+	/**
+	 * @param array{table: string, type: string, name?: string, changes?: list<string>} $finding
+	 * @param array<string, array<string, true>> $optionalIndexNames table name => set of index names, as returned by getOptionalIndexNames()
+	 */
+	private function isOptionalIndexFinding(array $finding, array $optionalIndexNames): bool {
+		return ($finding['type'] === 'missing_index' || $finding['type'] === 'unexpected_index')
+			&& isset($optionalIndexNames[$finding['table']][$finding['name']]);
+	}
+
 	private function keepOnlyTable(Schema $schema, string $tableName): void {
 		foreach ($schema->getTables() as $table) {
 			if ($table->getName() !== $tableName) {
@@ -324,7 +399,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()) {
@@ -342,4 +417,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);
+	}
 }

tests/lib/DB/SchemaCheckerTest.php

@@ -0,0 +1,164 @@
+<?php
+
+declare(strict_types=1);
+/**
+ * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+namespace Test\DB;
+
+use Doctrine\DBAL\Schema\Column;
+use Doctrine\DBAL\Schema\ColumnDiff;
+use Doctrine\DBAL\Schema\Schema;
+use Doctrine\DBAL\Types\TextType;
+use Doctrine\DBAL\Types\Type;
+use Doctrine\DBAL\Types\Types;
+use OC\DB\Connection;
+use OC\DB\SchemaChecker;
+use OCP\App\IAppManager;
+use OCP\DB\Events\AddMissingIndicesEvent;
+use OCP\EventDispatcher\IEventDispatcher;
+use OCP\IAppConfig;
+use OCP\IDBConnection;
+use PHPUnit\Framework\Attributes\DataProvider;
+use PHPUnit\Framework\MockObject\MockObject;
+use Psr\Log\LoggerInterface;
+
+class SchemaCheckerTest extends \Test\TestCase {
+	private Connection&MockObject $connection;
+	private IAppConfig&MockObject $appConfig;
+	private IAppManager&MockObject $appManager;
+	private IEventDispatcher&MockObject $eventDispatcher;
+	private LoggerInterface&MockObject $logger;
+	private SchemaChecker $schemaChecker;
+
+	#[\Override]
+	protected function setUp(): void {
+		parent::setUp();
+
+		$this->connection = $this->createMock(Connection::class);
+		$this->appConfig = $this->createMock(IAppConfig::class);
+		$this->appManager = $this->createMock(IAppManager::class);
+		$this->eventDispatcher = $this->createMock(IEventDispatcher::class);
+		$this->logger = $this->createMock(LoggerInterface::class);
+
+		$this->schemaChecker = new SchemaChecker(
+			$this->connection,
+			$this->appConfig,
+			$this->appManager,
+			$this->eventDispatcher,
+			$this->logger,
+		);
+	}
+
+	public static function dataFormatFinding(): array {
+		return [
+			'missing_table' => [['table' => 'oc_foo', 'type' => 'missing_table'], "missing table 'oc_foo'"],
+			'unexpected_table' => [['table' => 'oc_foo', 'type' => 'unexpected_table'], "unexpected table 'oc_foo'"],
+			'missing_column' => [['table' => 'oc_foo', 'type' => 'missing_column', 'name' => 'bar'], "oc_foo: missing column 'bar'"],
+			'unexpected_column' => [['table' => 'oc_foo', 'type' => 'unexpected_column', 'name' => 'bar'], "oc_foo: unexpected column 'bar'"],
+			'modified_column' => [['table' => 'oc_foo', 'type' => 'modified_column', 'name' => 'bar', 'changes' => ['type', 'default']], "oc_foo: column 'bar' differs in: type, default"],
+			'missing_index' => [['table' => 'oc_foo', 'type' => 'missing_index', 'name' => 'bar_idx'], "oc_foo: missing index 'bar_idx'"],
+			'unexpected_index' => [['table' => 'oc_foo', 'type' => 'unexpected_index', 'name' => 'bar_idx'], "oc_foo: unexpected index 'bar_idx'"],
+			'unknown' => [['table' => 'oc_foo', 'type' => 'something_else'], "oc_foo: unknown finding 'something_else'"],
+		];
+	}
+
+	#[DataProvider('dataFormatFinding')]
+	public function testFormatFinding(array $finding, string $expected): void {
+		$this->assertSame($expected, $this->schemaChecker->formatFinding($finding));
+	}
+
+	public function testPartitionFindingsSplitsBlockingAndDisabled(): void {
+		$blockingFinding = ['table' => 'oc_foo', 'type' => 'missing_column', 'name' => 'a', 'app' => 'core', 'enabled' => true];
+		$disabledAppFinding = ['table' => 'oc_bar', 'type' => 'missing_column', 'name' => 'b', 'app' => 'files', 'enabled' => false];
+		$unattributedFinding = ['table' => 'oc_baz', 'type' => 'unexpected_table', 'app' => null, 'enabled' => false];
+
+		$result = $this->schemaChecker->partitionFindings([
+			$blockingFinding,
+			$disabledAppFinding,
+			$unattributedFinding,
+		]);
+
+		$this->assertSame([$blockingFinding], $result['blocking']);
+		$this->assertSame(['files' => [$disabledAppFinding]], array_intersect_key($result['byDisabledApp'], ['files' => true]));
+		$this->assertSame(['(unknown app)' => [$unattributedFinding]], array_intersect_key($result['byDisabledApp'], ['(unknown app)' => true]));
+	}
+
+	public function testNormalizeLongStringColumnsRewritesOnlyColumnsOverTheLimit(): void {
+		$schema = new Schema();
+		$table = $schema->createTable('oc_test');
+		$table->addColumn('short_col', Types::STRING, ['length' => 255]);
+		$table->addColumn('long_col', Types::STRING, ['length' => 4001]);
+
+		self::invokePrivate($this->schemaChecker, 'normalizeLongStringColumns', [$schema]);
+
+		$this->assertSame(Types::STRING, Type::getTypeRegistry()->lookupName($table->getColumn('short_col')->getType()));
+		$this->assertSame(255, $table->getColumn('short_col')->getLength());
+
+		$this->assertInstanceOf(TextType::class, $table->getColumn('long_col')->getType());
+		$this->assertNull($table->getColumn('long_col')->getLength());
+	}
+
+	public static function dataIsIgnorableTextDefaultDiff(): array {
+		return [
+			'mysql text' => [IDBConnection::PLATFORM_MYSQL, Types::TEXT, true],
+			'mariadb blob' => [IDBConnection::PLATFORM_MARIADB, Types::BLOB, true],
+			'mysql string is not ignorable' => [IDBConnection::PLATFORM_MYSQL, Types::STRING, false],
+			'sqlite text is not ignorable' => [IDBConnection::PLATFORM_SQLITE, Types::TEXT, false],
+		];
+	}
+
+	#[DataProvider('dataIsIgnorableTextDefaultDiff')]
+	public function testIsIgnorableTextDefaultDiff(string $provider, string $typeName, bool $expected): void {
+		$this->connection->method('getDatabaseProvider')->willReturn($provider);
+
+		$column = new Column('some_col', Type::getType($typeName));
+		$columnDiff = new ColumnDiff('some_col', $column, ['default'], $column);
+
+		$result = self::invokePrivate($this->schemaChecker, 'isIgnorableTextDefaultDiff', [$columnDiff]);
+
+		$this->assertSame($expected, $result);
+	}
+
+	public function testGetOptionalIndexNamesCollectsMissingAndReplacedIndices(): void {
+		$this->connection->method('getPrefix')->willReturn('oc_');
+		$this->eventDispatcher->method('dispatchTyped')
+			->willReturnCallback(function (AddMissingIndicesEvent $event): void {
+				$event->addMissingIndex('foo', 'foo_idx', ['col']);
+				$event->replaceIndex('bar', ['old_idx'], 'new_idx', ['col'], false);
+			});
+
+		$names = self::invokePrivate($this->schemaChecker, 'getOptionalIndexNames');
+
+		$this->assertSame([
+			'oc_foo' => ['foo_idx' => true],
+			'oc_bar' => ['new_idx' => true, 'old_idx' => true],
+		], $names);
+	}
+
+	public static function dataIsOptionalIndexFinding(): array {
+		$optionalIndexNames = ['oc_foo' => ['foo_idx' => true]];
+
+		return [
+			'matching missing_index' => [['table' => 'oc_foo', 'type' => 'missing_index', 'name' => 'foo_idx'], $optionalIndexNames, true],
+			'matching unexpected_index' => [['table' => 'oc_foo', 'type' => 'unexpected_index', 'name' => 'foo_idx'], $optionalIndexNames, true],
+			'different index name' => [['table' => 'oc_foo', 'type' => 'missing_index', 'name' => 'other_idx'], $optionalIndexNames, false],
+			'different table' => [['table' => 'oc_bar', 'type' => 'missing_index', 'name' => 'foo_idx'], $optionalIndexNames, false],
+			'non-index finding type' => [['table' => 'oc_foo', 'type' => 'missing_column', 'name' => 'foo_idx'], $optionalIndexNames, false],
+		];
+	}
+
+	/**
+	 * Optional-index findings must be filtered out entirely (never reach
+	 * partitionFindings() or --output=json), not just hidden from plain-text
+	 * output - Settings already has a dedicated admin-overview surface for them.
+	 */
+	#[DataProvider('dataIsOptionalIndexFinding')]
+	public function testIsOptionalIndexFinding(array $finding, array $optionalIndexNames, bool $expected): void {
+		$result = self::invokePrivate($this->schemaChecker, 'isOptionalIndexFinding', [$finding, $optionalIndexNames]);
+
+		$this->assertSame($expected, $result);
+	}
+}