Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions core/Command/Db/CheckSchema.php
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ private function printDisabledAppFindings(array $byDisabledApp, OutputInterface

$output->writeln('Disabled apps (not affecting exit code):');
$output->writeln('If the schema for a disabled app differs from what is expected, this might indicate the app was updated since it was disabled. Missing migrations will be applied once the app is enabled again.');
$output->writeln("Tables listed under '(unknown app)' could not be attributed to any app - most likely leftovers from an app that was removed entirely (occ app:remove only deletes its code, not its tables or config).");
foreach ($byDisabledApp as $app => $appFindings) {
$output->writeln(" {$app}:");
foreach ($appFindings as $finding) {
Expand Down
9 changes: 7 additions & 2 deletions core/Command/Upgrade.php
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ function ($success) use ($output, $self): void {

$success = $updater->upgrade();

$this->postUpgradeCheck($input, $output);
$this->postUpgradeCheck($input, $output, $success);

if (!$success) {
return self::ERROR_FAILURE;
Expand Down Expand Up @@ -227,8 +227,9 @@ function ($success) use ($output, $self): void {
*
* @param InputInterface $input input interface
* @param OutputInterface $output output interface
* @param bool $upgradeSucceeded whether the upgrade itself completed successfully
*/
protected function postUpgradeCheck(InputInterface $input, OutputInterface $output) {
protected function postUpgradeCheck(InputInterface $input, OutputInterface $output, bool $upgradeSucceeded) {
$trustedDomains = $this->config->getSystemValue('trusted_domains', []);
if (empty($trustedDomains)) {
$output->write(
Expand All @@ -238,6 +239,10 @@ protected function postUpgradeCheck(InputInterface $input, OutputInterface $outp
);
}

if (!$upgradeSucceeded) {
return;
}

$this->checkSchema($output);
}

Expand Down
32 changes: 28 additions & 4 deletions lib/private/DB/SchemaChecker.php
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,16 @@ public function getFindings(?string $onlyTable = null): array {

// Enabled apps are already autoloaded at boot, no extra class loading needed.
foreach (array_keys($enabledApps) as $app) {
$this->applyMigrations($app, $expectedSchema);
try {
$this->applyMigrations($app, $expectedSchema);
} catch (AppPathNotFoundException) {
// Enabled in config, but the app's code is gone: occ app:remove
// only deletes an app's code, never its enabled flag, tables or
// config (see Installer::removeApp()). Nothing to replay here;
// any of its tables still in the live DB surface as
// unattributed unexpected_table findings instead of crashing
// the whole check.
}
}

// Disabled apps keep their tables, so replay their migrations too.
Expand Down Expand Up @@ -77,8 +86,20 @@ public function getFindings(?string $onlyTable = null): 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]);
if ($finding['type'] === 'unexpected_table' && $app === null) {
// An unexpected table that cannot be attributed to any known
// app is inherently informational, not a sign of drift: occ
// app:remove leaves tables and appconfig in place and only
// deletes the app's code (see Installer::removeApp()), so a
// long-uninstalled app's tables can never be attributed by
// applyDisabledMigrations() - there is no code left to replay
// migrations from. Treat them the same as residue from a
// still-present disabled app instead of blocking on them.
$finding['enabled'] = false;
} else {
// 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;
Expand Down Expand Up @@ -120,7 +141,10 @@ public function partitionFindings(array $findings): array {
} elseif ($finding['enabled']) {
$blocking[] = $finding;
} else {
$byDisabledApp[$finding['app']][] = $finding;
// $finding['app'] is null for unattributed unexpected tables
// (see getFindings()); group those under a placeholder label
// rather than coercing null to an empty-string array key.
$byDisabledApp[$finding['app'] ?? '(unknown app)'][] = $finding;
}
}
return ['blocking' => $blocking, 'byDisabledApp' => $byDisabledApp, 'optionalIndices' => $optionalIndices];
Expand Down
Loading