From cb27fe090d72a6afa4d61d43d86f514ffd3b657a Mon Sep 17 00:00:00 2001 From: fs Date: Thu, 19 Mar 2026 08:26:45 +0100 Subject: [PATCH] =?UTF-8?q?refactor:=20simplify=20module=20platform=20?= =?UTF-8?q?=E2=80=94=20drop=20custom=20autoloader,=20layout=5Fcapabilities?= =?UTF-8?q?=20indirection,=20and=20absolute=20template=20paths?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Replace ModuleAutoloader with Composer ClassLoader (addPsr4 via spl_autoload_functions) - Eliminate layout_capabilities mapping; UI slots reference ability strings directly - Resolve relative template paths in ModuleRegistry instead of baking __DIR__ into manifests Co-Authored-By: Claude Opus 4.6 (1M context) --- lib/App/Module/ModuleAutoloader.php | 56 ------------ lib/App/Module/ModuleManifest.php | 4 - lib/App/Module/ModuleRegistry.php | 90 +++++++++++++------ lib/Service/Access/UiCapabilityMap.php | 4 +- modules/addressbook/module.php | 12 +-- modules/bookmarks/module.php | 26 +++--- .../ModuleStructureContractTest.php | 67 +++++++------- 7 files changed, 115 insertions(+), 144 deletions(-) delete mode 100644 lib/App/Module/ModuleAutoloader.php diff --git a/lib/App/Module/ModuleAutoloader.php b/lib/App/Module/ModuleAutoloader.php deleted file mode 100644 index 48ed37f..0000000 --- a/lib/App/Module/ModuleAutoloader.php +++ /dev/null @@ -1,56 +0,0 @@ -/lib and use the same - * MintyPHP namespace root as core classes. - */ -final class ModuleAutoloader -{ - /** @var list */ - private static array $moduleLibDirs = []; - - private static bool $registered = false; - - /** - * @param list $manifests - */ - public static function register(array $manifests): void - { - $dirs = self::$moduleLibDirs; - foreach ($manifests as $manifest) { - $libDir = rtrim($manifest->basePath, '/') . '/lib'; - if (is_dir($libDir)) { - $dirs[] = $libDir; - } - } - self::$moduleLibDirs = array_values(array_unique($dirs)); - - if (self::$registered) { - return; - } - - spl_autoload_register([self::class, 'autoload'], true, true); - self::$registered = true; - } - - private static function autoload(string $class): void - { - $prefix = 'MintyPHP\\'; - if (!str_starts_with($class, $prefix)) { - return; - } - - $relativePath = str_replace('\\', '/', substr($class, strlen($prefix))) . '.php'; - foreach (self::$moduleLibDirs as $libDir) { - $candidate = $libDir . '/' . $relativePath; - if (is_file($candidate)) { - require_once $candidate; - return; - } - } - } -} diff --git a/lib/App/Module/ModuleManifest.php b/lib/App/Module/ModuleManifest.php index e8ddb6b..25a420f 100644 --- a/lib/App/Module/ModuleManifest.php +++ b/lib/App/Module/ModuleManifest.php @@ -72,9 +72,6 @@ final class ModuleManifest /** @var list AuthorizationPolicyInterface FQCNs */ public readonly array $authorizationPolicies; - /** @var array UI-capability-key → ability-string for layout authorization */ - public readonly array $layoutCapabilities; - public readonly ?string $migrationsPath; public readonly string $basePath; @@ -109,7 +106,6 @@ final class ModuleManifest $this->sessionProviders = self::listOf($raw, 'session_providers'); $this->permissions = self::normalizePermissions($raw['permissions'] ?? [], $this->id); $this->authorizationPolicies = self::listOf($raw, 'authorization_policies'); - $this->layoutCapabilities = is_array($raw['layout_capabilities'] ?? null) ? $raw['layout_capabilities'] : []; $migrationsPath = $raw['migrations_path'] ?? null; $this->migrationsPath = is_string($migrationsPath) && trim($migrationsPath) !== '' diff --git a/lib/App/Module/ModuleRegistry.php b/lib/App/Module/ModuleRegistry.php index 1a90b4f..74dd03e 100644 --- a/lib/App/Module/ModuleRegistry.php +++ b/lib/App/Module/ModuleRegistry.php @@ -2,6 +2,7 @@ namespace MintyPHP\App\Module; +use Composer\Autoload\ClassLoader; use RuntimeException; /** @@ -66,9 +67,6 @@ final class ModuleRegistry /** @var list merged authorization policy FQCNs */ private array $mergedAuthorizationPolicies = []; - /** @var array merged layout capabilities: UI-key → ability */ - private array $mergedLayoutCapabilities = []; - /** * @var listloadOrder <=> $b->loadOrder ?: strcmp($a->id, $b->id); }); - // Allow module-local classes (modules//lib/...) to autoload. - ModuleAutoloader::register($manifests); + // Register module lib dirs with Composer's PSR-4 autoloader so that + // module classes (MintyPHP\Module\\*) resolve without a custom loader. + self::registerModuleAutoloading($manifests); // Register and merge foreach ($manifests as $manifest) { @@ -192,6 +191,33 @@ final class ModuleRegistry return array_values(array_unique(array_filter($fromConfig, static fn (string $id): bool => trim($id) !== ''))); } + /** + * Register module lib/ directories with Composer's ClassLoader. + * + * @param list $manifests + */ + private static function registerModuleAutoloading(array $manifests): void + { + $composerLoader = null; + foreach (spl_autoload_functions() as $autoloader) { + if (is_array($autoloader) && $autoloader[0] instanceof ClassLoader) { + $composerLoader = $autoloader[0]; + break; + } + } + + if ($composerLoader === null) { + return; + } + + foreach ($manifests as $manifest) { + $libDir = rtrim($manifest->basePath, '/') . '/lib'; + if (is_dir($libDir)) { + $composerLoader->addPsr4('MintyPHP\\', [$libDir]); + } + } + } + private function registerModule(ModuleManifest $manifest): void { if (isset($this->modules[$manifest->id])) { @@ -240,7 +266,7 @@ final class ModuleRegistry } $contributionList = is_array($contributions) ? $contributions : [$contributions]; foreach ($contributionList as $contribution) { - $normalizedContribution = $this->normalizeUiSlotContribution($slotName, $contribution, $manifest->id); + $normalizedContribution = $this->normalizeUiSlotContribution($slotName, $contribution, $manifest->id, $manifest->basePath); $key = (string) $normalizedContribution['key']; foreach ($this->mergedUiSlots[$slotName] as $existing) { $existingKey = is_array($existing) ? ($existing['key'] ?? null) : null; @@ -339,23 +365,6 @@ final class ModuleRegistry $this->mergedAuthorizationPolicies[] = $policyClass; } - // — Merge layout capabilities (fail-fast on duplicate key) ——— - foreach ($manifest->layoutCapabilities as $uiKey => $ability) { - $uiKey = trim((string) $uiKey); - $ability = trim((string) $ability); - if ($uiKey === '' || $ability === '') { - throw new RuntimeException( - "Layout capability in module '{$manifest->id}' must have non-empty key and ability." - ); - } - if (isset($this->mergedLayoutCapabilities[$uiKey])) { - throw new RuntimeException( - "Layout capability conflict: key '{$uiKey}' from module '{$manifest->id}' is already registered." - ); - } - $this->mergedLayoutCapabilities[$uiKey] = $ability; - } - // — Merge scheduler jobs ————————————————————————— foreach ($manifest->schedulerJobs as $schedulerJob) { $jobKey = trim((string) $schedulerJob['job_key']); @@ -450,7 +459,7 @@ final class ModuleRegistry /** * @return array */ - private function normalizeUiSlotContribution(string $slotName, mixed $rawContribution, string $moduleId): array + private function normalizeUiSlotContribution(string $slotName, mixed $rawContribution, string $moduleId, string $basePath): array { if (!is_array($rawContribution)) { throw new RuntimeException( @@ -465,6 +474,17 @@ final class ModuleRegistry ); } + // Resolve relative template paths to absolute using the module's basePath. + foreach (['panel_template', 'template'] as $templateKey) { + if (!isset($rawContribution[$templateKey])) { + continue; + } + $templatePath = trim((string) $rawContribution[$templateKey]); + if ($templatePath !== '' && !str_starts_with($templatePath, '/')) { + $rawContribution[$templateKey] = rtrim($basePath, '/') . '/' . $templatePath; + } + } + foreach (self::UI_SLOT_REQUIRED_KEYS[$slotName] ?? [] as $requiredKey) { $requiredValue = trim((string) ($rawContribution[$requiredKey] ?? '')); if ($requiredValue === '') { @@ -660,10 +680,26 @@ final class ModuleRegistry return $this->mergedAuthorizationPolicies; } - /** @return array UI-capability-key → ability-string */ - public function getLayoutCapabilities(): array + /** + * Collect unique ability strings from all module UI slot contributions. + * + * Used by UiAccessService to resolve module abilities into the layout auth map. + * Each ability maps to itself (key = ability, value = ability). + * + * @return array + */ + public function getModuleUiAbilities(): array { - return $this->mergedLayoutCapabilities; + $abilities = []; + foreach ($this->mergedUiSlots as $items) { + foreach ($items as $item) { + $ability = trim((string) ($item['permission'] ?? '')); + if ($ability !== '') { + $abilities[$ability] = $ability; + } + } + } + return $abilities; } /** diff --git a/lib/Service/Access/UiCapabilityMap.php b/lib/Service/Access/UiCapabilityMap.php index 92f80b6..f2aed8f 100644 --- a/lib/Service/Access/UiCapabilityMap.php +++ b/lib/Service/Access/UiCapabilityMap.php @@ -7,8 +7,8 @@ final class UiCapabilityMap /** * Core layout capabilities — admin panel visibility checks. * - * Module-contributed capabilities are registered via the module manifest's - * 'layout_capabilities' field and merged dynamically in UiAccessService. + * Module abilities are collected from UI slot 'permission' fields and + * merged dynamically in UiAccessService::layoutCapabilities(). * * @var array */ diff --git a/modules/addressbook/module.php b/modules/addressbook/module.php index 25949f7..67aaa6f 100644 --- a/modules/addressbook/module.php +++ b/modules/addressbook/module.php @@ -32,8 +32,8 @@ return [ 'label' => 'Address book', 'icon' => 'bi-people', 'href' => 'address-book', - 'permission' => 'can_view_address_book', - 'panel_template' => __DIR__ . '/templates/aside-people-panel.phtml', + 'permission' => 'addressbook.view', + 'panel_template' => 'templates/aside-people-panel.phtml', 'details_storage' => 'aside-people-tenant', 'details_open_active' => true, 'order' => 110, @@ -44,7 +44,7 @@ return [ 'key' => 'address-book', 'label' => 'Address book', 'base_url' => 'address-book', - 'permission' => 'can_view_address_book', + 'permission' => 'addressbook.view', 'order' => 110, ], ], @@ -54,7 +54,7 @@ return [ 'type' => 'link', 'label' => 'View in address book', 'href' => 'address-book/view/{user_uuid}', - 'permission' => 'can_view_address_book', + 'permission' => 'addressbook.view', 'order' => 110, ], ], @@ -64,10 +64,6 @@ return [ \MintyPHP\Module\AddressBook\AddressBookAuthorizationPolicy::class, ], - 'layout_capabilities' => [ - 'can_view_address_book' => 'addressbook.view', - ], - 'search_resources' => [ \MintyPHP\Module\AddressBook\Providers\AddressBookSearchProvider::class, ], diff --git a/modules/bookmarks/module.php b/modules/bookmarks/module.php index da7c118..0815dbb 100644 --- a/modules/bookmarks/module.php +++ b/modules/bookmarks/module.php @@ -33,8 +33,8 @@ return [ 'label' => 'Bookmarks', 'icon' => 'bi-bookmark-star', 'href' => '', - 'permission' => 'can_use_bookmarks', - 'panel_template' => __DIR__ . '/templates/aside-bookmarks-panel.phtml', + 'permission' => 'bookmarks.use', + 'panel_template' => 'templates/aside-bookmarks-panel.phtml', 'details_storage' => 'aside-bookmarks-groups-v1', 'order' => 120, ], @@ -42,16 +42,16 @@ return [ 'topbar.right_item' => [ [ 'key' => 'bookmarks-toggle', - 'template' => __DIR__ . '/templates/topbar-bookmark-button.phtml', - 'permission' => 'can_use_bookmarks', + 'template' => 'templates/topbar-bookmark-button.phtml', + 'permission' => 'bookmarks.use', 'order' => 120, ], ], 'layout.body_end_template' => [ [ 'key' => 'bookmarks-dialogs', - 'template' => __DIR__ . '/templates/bookmark-dialogs.phtml', - 'permission' => 'can_use_bookmarks', + 'template' => 'templates/bookmark-dialogs.phtml', + 'permission' => 'bookmarks.use', 'order' => 120, ], ], @@ -59,13 +59,13 @@ return [ [ 'key' => 'bookmarks-form-style', 'path' => 'modules/bookmarks/css/components/app-bookmark-form.css', - 'permission' => 'can_use_bookmarks', + 'permission' => 'bookmarks.use', 'order' => 120, ], [ 'key' => 'bookmarks-aside-style', 'path' => 'modules/bookmarks/css/layout/app-sidebar-bookmarks.css', - 'permission' => 'can_use_bookmarks', + 'permission' => 'bookmarks.use', 'order' => 121, ], ], @@ -74,7 +74,7 @@ return [ 'key' => 'bookmarks', 'label' => 'Bookmarks', 'base_url' => '#', - 'permission' => 'can_use_bookmarks', + 'permission' => 'bookmarks.use', 'order' => 130, ], ], @@ -91,7 +91,7 @@ return [ 'groupDialogSelector' => '[data-app-bookmark-group-dialog]', ], 'phase' => 'late', - 'permission' => 'can_use_bookmarks', + 'permission' => 'bookmarks.use', 'order' => 120, ], [ @@ -104,7 +104,7 @@ return [ 'selector' => '[data-app-bookmark-panel]', ], 'phase' => 'late', - 'permission' => 'can_use_bookmarks', + 'permission' => 'bookmarks.use', 'order' => 121, ], ], @@ -114,10 +114,6 @@ return [ \MintyPHP\Module\Bookmarks\BookmarksAuthorizationPolicy::class, ], - 'layout_capabilities' => [ - 'can_use_bookmarks' => 'bookmarks.use', - ], - 'search_resources' => [ \MintyPHP\Module\Bookmarks\Providers\BookmarksSearchProvider::class, ], diff --git a/tests/Architecture/ModuleStructureContractTest.php b/tests/Architecture/ModuleStructureContractTest.php index 9b9bb95..51b9778 100644 --- a/tests/Architecture/ModuleStructureContractTest.php +++ b/tests/Architecture/ModuleStructureContractTest.php @@ -229,38 +229,21 @@ final class ModuleStructureContractTest extends TestCase } } - // ─── Layout capabilities ───────────────────────────────────────── + // ─── UI slot abilities ────────────────────────────────────────── - public function testLayoutCapabilitiesHaveNonEmptyKeysAndAbilities(): void + /** + * Every 'permission' value in a module's UI slots must reference an ability + * declared as a constant on one of the module's authorization policies. + */ + public function testUiSlotAbilitiesMatchPolicySupports(): void { foreach (self::$modules as $module) { - foreach ($module['manifest']->layoutCapabilities as $uiKey => $ability) { - self::assertNotEmpty( - trim((string) $uiKey), - "Module '{$module['id']}' has empty layout_capabilities key" - ); - self::assertNotEmpty( - trim((string) $ability), - "Module '{$module['id']}' layout_capabilities key '{$uiKey}' has empty ability" - ); - } - } - } - - public function testLayoutCapabilityAbilitiesMatchPolicySupports(): void - { - foreach (self::$modules as $module) { - if (empty($module['manifest']->layoutCapabilities)) { - continue; - } - // Collect all abilities supported by this module's policies $supportedAbilities = []; foreach ($module['manifest']->authorizationPolicies as $policyClass) { if (!class_exists($policyClass)) { continue; } - // Check each layout capability ability against this policy $reflection = new \ReflectionClass($policyClass); foreach ($reflection->getConstants() as $constValue) { if (is_string($constValue)) { @@ -269,13 +252,28 @@ final class ModuleStructureContractTest extends TestCase } } - foreach ($module['manifest']->layoutCapabilities as $uiKey => $ability) { - self::assertContains( - $ability, - $supportedAbilities, - "Module '{$module['id']}' layout_capabilities['{$uiKey}'] references ability '{$ability}' " - . "but no module authorization policy declares it as a constant" - ); + if ($supportedAbilities === []) { + continue; + } + + // Check every permission value from UI slots + foreach ($module['manifest']->uiSlots as $slotName => $contributions) { + $contributionList = is_array($contributions) ? $contributions : [$contributions]; + foreach ($contributionList as $idx => $contribution) { + if (!is_array($contribution)) { + continue; + } + $ability = trim((string) ($contribution['permission'] ?? '')); + if ($ability === '') { + continue; + } + self::assertContains( + $ability, + $supportedAbilities, + "Module '{$module['id']}' ui_slots['{$slotName}'][{$idx}] permission '{$ability}' " + . "is not declared as a constant on any module authorization policy" + ); + } } } } @@ -410,9 +408,14 @@ final class ModuleStructureContractTest extends TestCase if ($template === null) { continue; } + // Resolve relative paths against module basePath + $templatePath = (string) $template; + if (!str_starts_with($templatePath, '/')) { + $templatePath = $module['manifest']->basePath . '/' . $templatePath; + } self::assertFileExists( - (string) $template, - "Module '{$module['id']}' {$slotName} {$templateKey} does not exist: {$template}" + $templatePath, + "Module '{$module['id']}' {$slotName} {$templateKey} does not exist: {$templatePath}" ); } }