feat(modules): harden module platform with event constants, validation checks, and CLI improvements
- Add ModuleEvents constants class with all 8 event strings; update dispatch call sites - ValidateCommand: check AuthorizationPolicy container registration (check 20) - ValidateCommand: warn on unknown event_listeners keys (check 21) - make:module --simple flag for minimal manifests - module:deactivate reverse-dependency warning - 12 new PHPUnit tests covering all improvements Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -35,17 +35,22 @@ final class ModuleCommand extends Command
|
||||
public function usage(): string
|
||||
{
|
||||
return <<<'USAGE'
|
||||
Usage: php bin/console make:module <module-id>
|
||||
Usage: php bin/console make:module <module-id> [--simple]
|
||||
|
||||
Arguments:
|
||||
module-id Lowercase identifier (e.g. notifications, activity-log)
|
||||
Must match [a-z][a-z0-9-]* pattern.
|
||||
|
||||
Options:
|
||||
--simple Generate a minimal manifest with only essential fields.
|
||||
Ideal for lightweight UI-only modules.
|
||||
|
||||
Example:
|
||||
php bin/console make:module notifications
|
||||
php bin/console make:module help-panel --simple
|
||||
|
||||
Generates the full module directory structure with:
|
||||
- module.php manifest (all fields, sensible defaults)
|
||||
- module.php manifest (all fields or minimal with --simple)
|
||||
- ContainerRegistrar stub
|
||||
- AuthorizationPolicy stub
|
||||
- Empty pages/, templates/, web/css/, web/js/, tests/ directories
|
||||
@@ -96,8 +101,11 @@ final class ModuleCommand extends Command
|
||||
}
|
||||
|
||||
// Generate files
|
||||
$simple = $options['simple'] ?? false;
|
||||
$files = [
|
||||
'module.php' => self::manifestTemplate($moduleId, $pascalName, $snakeName),
|
||||
'module.php' => $simple
|
||||
? self::simpleManifestTemplate($moduleId, $pascalName)
|
||||
: self::manifestTemplate($moduleId, $pascalName, $snakeName),
|
||||
"lib/Module/{$pascalName}/{$pascalName}ContainerRegistrar.php" => self::registrarTemplate($pascalName),
|
||||
"lib/Module/{$pascalName}/{$pascalName}AuthorizationPolicy.php" => self::policyTemplate($pascalName, $snakeName),
|
||||
];
|
||||
@@ -199,6 +207,37 @@ final class ModuleCommand extends Command
|
||||
PHP;
|
||||
}
|
||||
|
||||
private static function simpleManifestTemplate(string $id, string $pascalName): string
|
||||
{
|
||||
$registrarClass = "\\MintyPHP\\Module\\{$pascalName}\\{$pascalName}ContainerRegistrar::class";
|
||||
$policyClass = "\\MintyPHP\\Module\\{$pascalName}\\{$pascalName}AuthorizationPolicy::class";
|
||||
|
||||
return <<<PHP
|
||||
<?php
|
||||
|
||||
/**
|
||||
* {$pascalName} module manifest (minimal).
|
||||
*
|
||||
* Extend with routes, permissions, ui_slots, etc. as needed.
|
||||
* See docs/howto-modul-erstellen.md for all available fields.
|
||||
*/
|
||||
return [
|
||||
'id' => '{$id}',
|
||||
'version' => '0.1.0',
|
||||
'enabled_by_default' => false,
|
||||
'load_order' => 100,
|
||||
'requires' => [],
|
||||
'container_registrars' => [
|
||||
{$registrarClass},
|
||||
],
|
||||
'authorization_policies' => [
|
||||
{$policyClass},
|
||||
],
|
||||
];
|
||||
|
||||
PHP;
|
||||
}
|
||||
|
||||
private static function registrarTemplate(string $pascalName): string
|
||||
{
|
||||
return <<<PHP
|
||||
|
||||
@@ -8,6 +8,7 @@ use MintyPHP\App\Module\Contracts\LayoutContextProvider;
|
||||
use MintyPHP\App\Module\Contracts\ModuleDeactivationHandler;
|
||||
use MintyPHP\App\Module\Contracts\SearchResourceProvider;
|
||||
use MintyPHP\App\Module\Contracts\SessionProvider;
|
||||
use MintyPHP\App\Module\ModuleEvents;
|
||||
use MintyPHP\App\Module\ModuleManifestLoader;
|
||||
use MintyPHP\Console\Command;
|
||||
use MintyPHP\Service\Access\AuthorizationPolicyInterface;
|
||||
@@ -292,9 +293,63 @@ final class ValidateCommand extends Command
|
||||
}
|
||||
}
|
||||
|
||||
// 20. AuthorizationPolicy classes referenced in ContainerRegistrar source
|
||||
$registrarSources = $this->loadRegistrarSources($moduleDir, $manifest->containerRegistrars);
|
||||
foreach ($manifest->authorizationPolicies as $policyClass) {
|
||||
if (!class_exists($policyClass)) {
|
||||
continue; // Already reported by check 9
|
||||
}
|
||||
$shortName = (new \ReflectionClass($policyClass))->getShortName();
|
||||
$found = false;
|
||||
foreach ($registrarSources as $source) {
|
||||
if (str_contains($source, $shortName)) {
|
||||
$found = true;
|
||||
break;
|
||||
}
|
||||
}
|
||||
if (!$found) {
|
||||
$warnings[] = "AuthorizationPolicy '{$shortName}' is not referenced in any ContainerRegistrar — may not be registered in the container";
|
||||
}
|
||||
}
|
||||
|
||||
// 21. Event listener keys reference known events
|
||||
$knownEvents = ModuleEvents::all();
|
||||
foreach ($manifest->eventListeners as $eventName => $handlers) {
|
||||
if (!in_array($eventName, $knownEvents, true)) {
|
||||
$warnings[] = "Event listener subscribes to unknown event '{$eventName}' — not defined in ModuleEvents";
|
||||
}
|
||||
}
|
||||
|
||||
return [$errors, $warnings];
|
||||
}
|
||||
|
||||
/**
|
||||
* @param list<class-string> $registrarClasses
|
||||
* @return list<string> Source code of each registrar file
|
||||
*/
|
||||
private function loadRegistrarSources(string $moduleDir, array $registrarClasses): array
|
||||
{
|
||||
$sources = [];
|
||||
foreach ($registrarClasses as $class) {
|
||||
if (!class_exists($class)) {
|
||||
continue;
|
||||
}
|
||||
try {
|
||||
$refClass = new \ReflectionClass($class);
|
||||
$file = $refClass->getFileName();
|
||||
if ($file !== false && is_file($file)) {
|
||||
$content = file_get_contents($file);
|
||||
if ($content !== false) {
|
||||
$sources[] = $content;
|
||||
}
|
||||
}
|
||||
} catch (\ReflectionException) {
|
||||
// Skip unresolvable classes
|
||||
}
|
||||
}
|
||||
return $sources;
|
||||
}
|
||||
|
||||
/**
|
||||
* @param list<string> &$errors
|
||||
*/
|
||||
|
||||
@@ -227,25 +227,31 @@ final class ModuleRunner implements ModuleRunnerInterface
|
||||
return 1;
|
||||
}
|
||||
|
||||
$manifest = ModuleManifestLoader::loadFromDisk(
|
||||
ModuleCliRuntime::projectRoot() . '/modules',
|
||||
$moduleId
|
||||
);
|
||||
$modulesDir = ModuleCliRuntime::projectRoot() . '/modules';
|
||||
$manifest = ModuleManifestLoader::loadFromDisk($modulesDir, $moduleId);
|
||||
|
||||
// Check for reverse dependencies (other modules that require this one)
|
||||
$dependents = $this->findReverseDependencies($modulesDir, $moduleId);
|
||||
$warning = '';
|
||||
if ($dependents !== []) {
|
||||
$dependentList = implode(', ', $dependents);
|
||||
$warning = "Warning: module(s) [{$dependentList}] declare '{$moduleId}' in their requires — deactivation may break them.\n";
|
||||
}
|
||||
|
||||
$handlerClass = $manifest->deactivationHandler;
|
||||
if ($handlerClass === null) {
|
||||
$message = "Module '{$moduleId}' has no deactivation_handler declared — nothing to do.";
|
||||
$message = $warning . "Module '{$moduleId}' has no deactivation_handler declared — nothing to do.";
|
||||
return 0;
|
||||
}
|
||||
|
||||
if (!class_exists($handlerClass)) {
|
||||
$message = "Error: deactivation handler class '{$handlerClass}' not found.";
|
||||
$message = $warning . "Error: deactivation handler class '{$handlerClass}' not found.";
|
||||
return 1;
|
||||
}
|
||||
|
||||
$container = $GLOBALS['minty_app_container'] ?? null;
|
||||
if (!$container instanceof AppContainer) {
|
||||
$message = 'Error: app container is not initialized.';
|
||||
$message = $warning . 'Error: app container is not initialized.';
|
||||
return 1;
|
||||
}
|
||||
$handler = $container->has($handlerClass)
|
||||
@@ -253,17 +259,17 @@ final class ModuleRunner implements ModuleRunnerInterface
|
||||
: new $handlerClass();
|
||||
|
||||
if (!$handler instanceof ModuleDeactivationHandler) {
|
||||
$message = "Error: '{$handlerClass}' does not implement ModuleDeactivationHandler.";
|
||||
$message = $warning . "Error: '{$handlerClass}' does not implement ModuleDeactivationHandler.";
|
||||
return 1;
|
||||
}
|
||||
|
||||
if ($dryRun) {
|
||||
$message = "Dry-run: handler '{$handlerClass}' found and valid for module '{$moduleId}'.";
|
||||
$message = $warning . "Dry-run: handler '{$handlerClass}' found and valid for module '{$moduleId}'.";
|
||||
return 0;
|
||||
}
|
||||
|
||||
$handler->deactivate($container);
|
||||
$message = "Deactivation handler for module '{$moduleId}' completed.";
|
||||
$message = $warning . "Deactivation handler for module '{$moduleId}' completed.";
|
||||
|
||||
return 0;
|
||||
});
|
||||
@@ -345,4 +351,34 @@ final class ModuleRunner implements ModuleRunnerInterface
|
||||
'message' => $step['error'] ?? $message,
|
||||
];
|
||||
}
|
||||
|
||||
/**
|
||||
* @return list<string> Module IDs that declare $targetId in their requires
|
||||
*/
|
||||
private function findReverseDependencies(string $modulesDir, string $targetId): array
|
||||
{
|
||||
$dependents = [];
|
||||
$entries = is_dir($modulesDir) ? (scandir($modulesDir) ?: []) : [];
|
||||
|
||||
foreach ($entries as $entry) {
|
||||
if ($entry === '.' || $entry === '..' || $entry === $targetId) {
|
||||
continue;
|
||||
}
|
||||
$manifestFile = $modulesDir . '/' . $entry . '/module.php';
|
||||
if (!is_file($manifestFile)) {
|
||||
continue;
|
||||
}
|
||||
try {
|
||||
$manifest = ModuleManifestLoader::loadFromDisk($modulesDir, $entry);
|
||||
if (in_array($targetId, $manifest->requires, true)) {
|
||||
$dependents[] = $entry;
|
||||
}
|
||||
} catch (\Throwable) {
|
||||
// Skip unparseable manifests
|
||||
}
|
||||
}
|
||||
|
||||
sort($dependents);
|
||||
return $dependents;
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user