better plugin checks

Signed-off-by: Andy Miller <rhuk@mac.com>
This commit is contained in:
Andy Miller
2025-10-15 11:24:50 -06:00
parent b55e86a8ba
commit 57212ec9a5
8 changed files with 303 additions and 23 deletions

View File

@@ -185,7 +185,25 @@ class RecoveryManager
* @param array $context
* @return void
*/
private function quarantinePlugin(string $slug, array $context): void
public function disablePlugin(string $slug, array $context = []): void
{
$context += [
'message' => $context['message'] ?? 'Disabled during upgrade preflight',
'file' => $context['file'] ?? '',
'line' => $context['line'] ?? null,
'created_at' => $context['created_at'] ?? time(),
'plugin' => $context['plugin'] ?? $slug,
];
$this->quarantinePlugin($slug, $context);
}
/**
* @param string $slug
* @param array $context
* @return void
*/
protected function quarantinePlugin(string $slug, array $context): void
{
$slug = trim($slug);
if ($slug === '') {

View File

@@ -15,6 +15,9 @@ use Grav\Common\Yaml;
use InvalidArgumentException;
use RuntimeException;
use Throwable;
use RecursiveDirectoryIterator;
use RecursiveIteratorIterator;
use FilesystemIterator;
use function basename;
use function count;
use function dirname;
@@ -94,16 +97,21 @@ class SafeUpgradeService
}
$psrLogConflicts = $this->detectPsrLogConflicts();
$monologConflicts = $this->detectMonologConflicts();
if ($pending) {
$warnings[] = 'One or more plugins/themes are not up to date.';
}
if ($psrLogConflicts) {
$warnings[] = 'Potential psr/log signature conflicts detected.';
}
if ($monologConflicts) {
$warnings[] = 'Potential Monolog logger API incompatibilities detected.';
}
return [
'plugins_pending' => $pending,
'psr_log_conflicts' => $psrLogConflicts,
'monolog_conflicts' => $monologConflicts,
'warnings' => $warnings,
];
}
@@ -263,6 +271,47 @@ class SafeUpgradeService
return $conflicts;
}
/**
* Detect usage of deprecated Monolog `add*` methods removed in newer releases.
*
* @return array<string, array>
*/
protected function detectMonologConflicts(): array
{
$conflicts = [];
$pluginRoots = glob($this->rootPath . '/user/plugins/*', GLOB_ONLYDIR) ?: [];
$pattern = '/->add(?:Debug|Info|Notice|Warning|Error|Critical|Alert|Emergency)\s*\(/i';
foreach ($pluginRoots as $path) {
$iterator = new RecursiveIteratorIterator(
new RecursiveDirectoryIterator($path, FilesystemIterator::SKIP_DOTS)
);
foreach ($iterator as $file) {
/** @var \SplFileInfo $file */
if (!$file->isFile() || strtolower($file->getExtension()) !== 'php') {
continue;
}
$contents = @file_get_contents($file->getPathname());
if ($contents === false) {
continue;
}
if (preg_match($pattern, $contents, $match)) {
$slug = basename($path);
$relative = str_replace($this->rootPath . '/', '', $file->getPathname());
$conflicts[$slug][] = [
'file' => $relative,
'method' => trim($match[0]),
];
}
}
}
return $conflicts;
}
/**
* Ensure directories flagged for ignoring get hydrated from the current installation.
*

View File

@@ -25,7 +25,7 @@ class PreflightCommand extends GpmCommand
$service = $this->createSafeUpgradeService();
$report = $service->preflight();
$hasIssues = !empty($report['plugins_pending']) || !empty($report['psr_log_conflicts']) || !empty($report['warnings']);
$hasIssues = !empty($report['plugins_pending']) || !empty($report['psr_log_conflicts']) || !empty($report['monolog_conflicts']) || !empty($report['warnings']);
if ($this->getInput()->getOption('json')) {
$io->writeln(json_encode($report, JSON_PRETTY_PRINT));
@@ -60,6 +60,19 @@ class PreflightCommand extends GpmCommand
$io->newLine();
}
if (!empty($report['monolog_conflicts'])) {
$io->writeln('<comment>Potential Monolog logger conflicts</comment>');
foreach ($report['monolog_conflicts'] as $slug => $entries) {
foreach ($entries as $entry) {
$file = $entry['file'] ?? 'unknown file';
$method = $entry['method'] ?? 'add*';
$io->writeln(sprintf(' - %s (%s in %s)', $slug, $method, $file));
}
}
$io->writeln(' Update the plugin to use PSR-3 style logger calls (e.g. $logger->error()).');
$io->newLine();
}
if (!$hasIssues) {
$io->success('No blocking issues detected.');
} else {

View File

@@ -22,6 +22,7 @@ use RuntimeException;
use Symfony\Component\Console\Input\ArrayInput;
use Symfony\Component\Console\Input\InputOption;
use Symfony\Component\Console\Question\ConfirmationQuestion;
use Symfony\Component\Console\Style\SymfonyStyle;
use ZipArchive;
use function count;
use function is_callable;
@@ -289,9 +290,10 @@ class SelfupgradeCommand extends GpmCommand
$io = $this->getIO();
$pending = $preflight['plugins_pending'] ?? [];
$conflicts = $preflight['psr_log_conflicts'] ?? [];
$monologConflicts = $preflight['monolog_conflicts'] ?? [];
$warnings = $preflight['warnings'] ?? [];
if (empty($pending) && empty($conflicts)) {
if (empty($pending) && empty($conflicts) && empty($monologConflicts)) {
return true;
}
@@ -319,25 +321,98 @@ class SelfupgradeCommand extends GpmCommand
return false;
}
if ($conflicts) {
$io->newLine();
$io->writeln('<yellow>Potential psr/log incompatibilities:</yellow>');
foreach ($conflicts as $slug => $info) {
$requires = $info['requires'] ?? '*';
$io->writeln(sprintf(' - %s (requires psr/log %s)', $slug, $requires));
}
$io->writeln(' Update the plugin or add "replace": {"psr/log": "*"} to its composer.json and reinstall dependencies.');
if (!$this->all_yes) {
$question = new ConfirmationQuestion('Continue despite psr/log warnings? [y|N] ', false);
if (!$io->askQuestion($question)) {
$io->writeln('Aborting self-upgrade. Adjust composer requirements or update affected plugins.');
return false;
$handled = $this->handleConflicts(
$conflicts,
static function (SymfonyStyle $io, array $conflicts): void {
$io->newLine();
$io->writeln('<yellow>Potential psr/log incompatibilities:</yellow>');
foreach ($conflicts as $slug => $info) {
$requires = $info['requires'] ?? '*';
$io->writeln(sprintf(' - %s (requires psr/log %s)', $slug, $requires));
}
}
},
'Update the plugin or add "replace": {"psr/log": "*"} to its composer.json and reinstall dependencies.',
'Aborting self-upgrade. Adjust composer requirements or update affected plugins.',
'Proceeding with potential psr/log incompatibilities still active.',
'Disabled before upgrade because of psr/log conflict'
);
if (!$handled) {
return false;
}
$handledMonolog = $this->handleConflicts(
$monologConflicts,
static function (SymfonyStyle $io, array $conflicts): void {
$io->newLine();
$io->writeln('<yellow>Potential Monolog logger API incompatibilities:</yellow>');
foreach ($conflicts as $slug => $entries) {
foreach ($entries as $entry) {
$file = $entry['file'] ?? 'unknown file';
$method = $entry['method'] ?? 'add*';
$io->writeln(sprintf(' - %s (%s in %s)', $slug, $method, $file));
}
}
},
'Update the plugin to use PSR-3 style logger methods (e.g. $logger->error()) before upgrading.',
'Aborting self-upgrade. Update plugins to remove deprecated Monolog add* calls.',
'Proceeding with potential Monolog API incompatibilities still active.',
'Disabled before upgrade because of Monolog API conflict'
);
if (!$handledMonolog) {
return false;
}
return true;
}
/**
* @param array $conflicts
* @param callable $printer
* @param string $advice
* @param string $abortMessage
* @param string $continueMessage
* @param string $disableNote
* @return bool
*/
private function handleConflicts(array $conflicts, callable $printer, string $advice, string $abortMessage, string $continueMessage, string $disableNote): bool
{
if (empty($conflicts)) {
return true;
}
$io = $this->getIO();
$printer($io, $conflicts);
$io->writeln(' ' . $advice);
$choice = $this->all_yes ? 'abort' : $io->choice(
'How would you like to proceed?',
['disable', 'continue', 'abort'],
'abort'
);
if ($choice === 'abort') {
$io->writeln($abortMessage);
return false;
}
/** @var \Grav\Common\Recovery\RecoveryManager $recovery */
$recovery = Grav::instance()['recovery'];
if ($choice === 'disable') {
foreach (array_keys($conflicts) as $slug) {
$recovery->disablePlugin($slug, ['message' => $disableNote]);
$io->writeln(sprintf(' - Disabled plugin %s.', $slug));
}
$io->writeln('Continuing with conflicted plugins disabled.');
return true;
}
$io->writeln($continueMessage);
return true;
}

View File

@@ -120,4 +120,25 @@ class RecoveryManagerTest extends \Codeception\TestCase\Test
$manager = new RecoveryManager($this->tmpDir);
self::assertNull($manager->getContext());
}
public function testDisablePluginRecordsQuarantineWithoutFlag(): void
{
$plugin = $this->tmpDir . '/user/plugins/problem';
Folder::create($plugin);
$manager = new RecoveryManager($this->tmpDir);
$manager->disablePlugin('problem', ['message' => 'Manual disable']);
$flag = $this->tmpDir . '/system/recovery.flag';
self::assertFileDoesNotExist($flag);
$configFile = $this->tmpDir . '/user/config/plugins/problem.yaml';
self::assertFileExists($configFile);
self::assertStringContainsString('enabled: false', file_get_contents($configFile));
$quarantine = $this->tmpDir . '/user/data/upgrades/quarantine.json';
self::assertFileExists($quarantine);
$decoded = json_decode(file_get_contents($quarantine), true);
self::assertSame('Manual disable', $decoded['problem']['message']);
}
}

View File

@@ -30,6 +30,11 @@ class SafeUpgradeServiceTest extends \Codeception\TestCase\Test
public $conflicts = [
'beta' => ['requires' => '^1.0']
];
public $monolog = [
'gamma' => [
['file' => 'user/plugins/gamma/gamma.php', 'method' => '->addError(']
]
];
protected function detectPendingPluginUpdates(): array
{
@@ -40,14 +45,20 @@ class SafeUpgradeServiceTest extends \Codeception\TestCase\Test
{
return $this->conflicts;
}
protected function detectMonologConflicts(): array
{
return $this->monolog;
}
};
$result = $service->preflight();
self::assertArrayHasKey('warnings', $result);
self::assertCount(2, $result['warnings']);
self::assertCount(3, $result['warnings']);
self::assertArrayHasKey('alpha', $result['plugins_pending']);
self::assertArrayHasKey('beta', $result['psr_log_conflicts']);
self::assertArrayHasKey('gamma', $result['monolog_conflicts']);
}
public function testPreflightHandlesDetectionFailure(): void
@@ -62,12 +73,18 @@ class SafeUpgradeServiceTest extends \Codeception\TestCase\Test
{
return [];
}
protected function detectMonologConflicts(): array
{
return [];
}
};
$result = $service->preflight();
self::assertSame([], $result['plugins_pending']);
self::assertSame([], $result['psr_log_conflicts']);
self::assertSame([], $result['monolog_conflicts']);
self::assertCount(1, $result['warnings']);
self::assertStringContainsString('Cannot reach GPM', $result['warnings'][0]);
}
@@ -140,6 +157,37 @@ class SafeUpgradeServiceTest extends \Codeception\TestCase\Test
self::assertArrayHasKey('problem', $conflicts);
}
public function testDetectsMonologConflictsFromFilesystem(): void
{
[$root] = $this->prepareLiveEnvironment();
$plugin = $root . '/user/plugins/logger';
Folder::create($plugin . '/src');
$code = <<<'PHP'
<?php
class LoggerTest {
public function test(
\Monolog\Logger $logger
) {
$logger->addError('deprecated');
}
}
PHP;
file_put_contents($plugin . '/src/logger.php', $code);
$service = new SafeUpgradeService([
'root' => $root,
'staging_root' => $this->tmpDir . '/staging',
]);
$method = new ReflectionMethod(SafeUpgradeService::class, 'detectMonologConflicts');
$method->setAccessible(true);
$conflicts = $method->invoke($service);
self::assertArrayHasKey('logger', $conflicts);
self::assertNotEmpty($conflicts['logger']);
self::assertStringContainsString('addError', $conflicts['logger'][0]['method']);
}
public function testClearRecoveryFlagRemovesFile(): void
{
[$root] = $this->prepareLiveEnvironment();
@@ -190,4 +238,3 @@ class SafeUpgradeServiceTest extends \Codeception\TestCase\Test
return $package;
}
}

View File

@@ -14,6 +14,7 @@ class PreflightCommandTest extends \Codeception\TestCase\Test
$service = new StubSafeUpgradeService([
'plugins_pending' => [],
'psr_log_conflicts' => [],
'monolog_conflicts' => [],
'warnings' => []
]);
$command = new TestPreflightCommand($service);
@@ -31,6 +32,7 @@ class PreflightCommandTest extends \Codeception\TestCase\Test
$service = new StubSafeUpgradeService([
'plugins_pending' => ['alpha' => ['type' => 'plugin', 'current' => '1', 'available' => '2']],
'psr_log_conflicts' => ['beta' => ['requires' => '^1']],
'monolog_conflicts' => ['gamma' => [['file' => 'user/plugins/gamma/gamma.php', 'method' => '->addError(']]],
'warnings' => ['pending updates']
]);
$command = new TestPreflightCommand($service);
@@ -42,6 +44,7 @@ class PreflightCommandTest extends \Codeception\TestCase\Test
$output = implode("\n", $style->messages);
self::assertStringContainsString('pending updates', $output);
self::assertStringContainsString('beta', $output);
self::assertStringContainsString('gamma', $output);
}
/**

View File

@@ -1,5 +1,6 @@
<?php
use Codeception\Util\Fixtures;
use Grav\Console\Gpm\SelfupgradeCommand;
use Symfony\Component\Console\Input\ArrayInput;
use Symfony\Component\Console\Input\InputInterface;
@@ -43,7 +44,7 @@ class SelfupgradeCommandTest extends \Codeception\TestCase\Test
public function testHandlePreflightReportAbortsOnPendingWhenDeclined(): void
{
$command = new TestSelfupgradeCommand();
[$style] = $this->injectIo($command, [false]);
[$style] = $this->injectIo($command);
$this->setAllYes($command, false);
$result = $command->runHandle([
@@ -59,7 +60,7 @@ class SelfupgradeCommandTest extends \Codeception\TestCase\Test
public function testHandlePreflightReportAbortsOnConflictWhenDeclined(): void
{
$command = new TestSelfupgradeCommand();
[$style] = $this->injectIo($command, [false]);
[$style] = $this->injectIo($command, ['abort']);
$this->setAllYes($command, false);
$result = $command->runHandle([
@@ -72,6 +73,50 @@ class SelfupgradeCommandTest extends \Codeception\TestCase\Test
self::assertStringContainsString('Adjust composer requirements', implode("\n", $style->messages));
}
public function testHandlePreflightReportDisablesPluginsWhenRequested(): void
{
$gravFactory = Fixtures::get('grav');
$grav = $gravFactory();
$stub = new class {
public $disabled = [];
public function disablePlugin(string $slug, array $context = []): void
{
$this->disabled[] = $slug;
}
};
$grav['recovery'] = $stub;
$command = new TestSelfupgradeCommand();
[$style] = $this->injectIo($command, ['disable']);
$result = $command->runHandle([
'plugins_pending' => [],
'psr_log_conflicts' => ['foo' => ['requires' => '^1.0']],
'warnings' => []
]);
self::assertTrue($result);
self::assertSame(['foo'], $stub->disabled);
$output = implode("\n", $style->messages);
self::assertStringContainsString('Continuing with conflicted plugins disabled.', $output);
}
public function testHandlePreflightReportContinuesWhenRequested(): void
{
$command = new TestSelfupgradeCommand();
[$style] = $this->injectIo($command, ['continue']);
$result = $command->runHandle([
'plugins_pending' => [],
'psr_log_conflicts' => ['foo' => ['requires' => '^1.0']],
'warnings' => []
]);
self::assertTrue($result);
$output = implode("\n", $style->messages);
self::assertStringContainsString('Proceeding with potential psr/log incompatibilities still active.', $output);
}
/**
* @param TestSelfupgradeCommand $command
* @param array<int, mixed> $responses
@@ -156,4 +201,13 @@ class SelfUpgradeMemoryStyle extends SymfonyStyle
return parent::askQuestion($question);
}
public function choice($question, array $choices, $default = null, $attempts = null, $errorMessage = 'Invalid value.')
{
if ($this->responses) {
return array_shift($this->responses);
}
return parent::choice($question, $choices, $default, $attempts, $errorMessage);
}
}