From 2995d69b4dcde53f0e02ea16d3b31006536166f7 Mon Sep 17 00:00:00 2001 From: Divarion-D Date: Tue, 30 Jun 2026 22:41:46 +0300 Subject: [PATCH] fix(modules): improve dependency handling and diagnostics for module loading --- src/Core/Module/ModuleLoader.php | 49 ++++++++++++++ src/Core/Module/ModuleManager.php | 79 +++++++++++++++++++++- src/Public/Views/admin/api.php | 14 ++-- src/Public/Views/admin/modules.php | 7 ++ tests/Unit/ModuleLoaderPriorityTest.php | 9 +-- tests/Unit/ModuleLoaderTest.php | 28 ++++++-- tests/Unit/ModuleManagerMigrationsTest.php | 70 +++++++++++++++++++ 7 files changed, 238 insertions(+), 18 deletions(-) diff --git a/src/Core/Module/ModuleLoader.php b/src/Core/Module/ModuleLoader.php index 6513d9bc..fcd050ec 100644 --- a/src/Core/Module/ModuleLoader.php +++ b/src/Core/Module/ModuleLoader.php @@ -523,6 +523,17 @@ class ModuleLoader { protected function resolveLoadOrder(array $discovered): array { $order = []; $state = []; // 1 = visiting, 2 = visited + + // Drop modules whose required dependencies are unavailable (missing on + // disk, disabled/uninstalled via config/modules.php, or filtered out by + // environment) before sorting. A single unsatisfiable module must not + // abort the whole load: we skip it — and, transitively, anything that + // requires it — with a warning, so the rest of the panel and the CLI keep + // working. Without this, e.g. disabling 'watch' while 'plex' (which + // requires it) stays enabled would throw ModuleNotFoundException out of + // loadAll() and brick both the admin panel and console.php. + $discovered = $this->pruneUnsatisfiableModules($discovered); + $names = array_keys($discovered); // Sort by priority desc, then alphabetically for determinism within same priority @@ -542,6 +553,44 @@ class ModuleLoader { return $order; } + /** + * Removes modules whose required dependencies are not present in the + * discovered set, cascading transitively. + * + * Discovery may legitimately exclude a module (disabled/uninstalled via + * config/modules.php, or filtered out for the current environment). When that + * module is a *required* dependency of another, the dependent cannot load — + * but that is not a fatal condition for the whole panel: we drop the dependent + * (and anything depending on it, in turn) and log a warning, leaving every + * still-satisfiable module loadable. This keeps the admin panel and CLI alive + * instead of throwing ModuleNotFoundException out of loadAll(). + * + * Optional dependencies are ignored here — they are allowed to be absent. + * + * @param array $discovered Discovered modules (name => [path, manifest]). + * @return array Pruned discovered set containing only satisfiable modules. + */ + protected function pruneUnsatisfiableModules(array $discovered): array { + do { + $removed = false; + foreach ($discovered as $name => $info) { + foreach ($info['manifest']['dependencies'] as $dependency) { + if (!isset($discovered[$dependency])) { + error_log( + "ModuleLoader: skipping module '{$name}' — required dependency " + . "'{$dependency}' is not available (missing, disabled, or wrong environment)" + ); + unset($discovered[$name]); + $removed = true; + break; + } + } + } + } while ($removed); + + return $discovered; + } + /** * DFS traversal to visit a module and its dependencies in dependency order. * diff --git a/src/Core/Module/ModuleManager.php b/src/Core/Module/ModuleManager.php index 5041de57..8a6d64a4 100644 --- a/src/Core/Module/ModuleManager.php +++ b/src/Core/Module/ModuleManager.php @@ -91,6 +91,34 @@ class ModuleManager { return $out; } + /** + * Names of currently-loadable (enabled) installed modules that declare $name + * as a required dependency. + * + * Used to guard disabling: a dependent that is itself disabled won't be loaded + * either, so disabling $name under it is harmless and must not be blocked. + * Only enabled dependents would be broken (ModuleLoader skips them on the next + * boot), so only those count. + * + * @return string[] + */ + private function enabledDependentsOf(string $name): array { + $out = []; + foreach ($this->listModules() as $module) { + if (($module['installed_version'] ?? '') === '') { + continue; // not installed — its requirements don't apply + } + $state = $module['state'] ?? null; + if (!($state instanceof \XcVm\Core\Enum\ModuleState) || !$state->isLoadable()) { + continue; // already disabled/failed — disabling its dep won't break it + } + if (in_array($name, $module['dependencies'] ?? [], true)) { + $out[] = $module['name']; + } + } + return $out; + } + /** * Install any on-disk module that has never been installed. * @@ -171,17 +199,44 @@ class ModuleManager { * Scans the modules directory for module.json files, merges with * config/modules.php overrides, and returns sorted results. * - * @return array Module list. + * @return array Module list. */ public function listModules(): array { $overrides = $this->readOverrides(); $items = []; $jsonFiles = glob($this->modulesPath . '/*/module.json') ?: []; + + // Pre-resolve each module's state by name so the dependency diagnostics + // below can see the full set while building items. + $stateByName = []; + foreach ($jsonFiles as $jsonFile) { + $depName = basename(dirname($jsonFile)); + $stateByName[$depName] = \XcVm\Core\Enum\ModuleState::fromRaw( + $overrides[$depName]['state'] ?? ($overrides[$depName]['enabled'] ?? null) + ); + } + foreach ($jsonFiles as $jsonFile) { $name = basename(dirname($jsonFile)); $meta = json_decode((string) @file_get_contents($jsonFile), true) ?: []; - $state = \XcVm\Core\Enum\ModuleState::fromRaw($overrides[$name]['state'] ?? ($overrides[$name]['enabled'] ?? null)); + $state = $stateByName[$name]; + + $dependencies = is_array($meta['dependencies'] ?? null) ? $meta['dependencies'] : []; + + // Flag a module that is nominally Enabled but won't actually load: + // ModuleLoader skips it when a required dependency is missing or not + // loadable (e.g. plex is Enabled but watch is Failed/Disabled). Mirrors + // ModuleLoader::pruneUnsatisfiableModules(). + $dependencyWarnings = []; + foreach ($dependencies as $dep) { + if (!isset($stateByName[$dep])) { + $dependencyWarnings[] = "Required dependency '{$dep}' is missing."; + } elseif (!$stateByName[$dep]->isLoadable()) { + $dependencyWarnings[] = "Required dependency '{$dep}' is not enabled (" + . $stateByName[$dep]->value . ').'; + } + } $items[] = [ 'name' => $name, @@ -190,7 +245,7 @@ class ModuleManager { 'requires_core' => $meta['requires_core'] ?? '', 'environment' => $meta['environment'] ?? 'main', 'priority' => (int) ($meta['priority'] ?? 0), - 'dependencies' => is_array($meta['dependencies'] ?? null) ? $meta['dependencies'] : [], + 'dependencies' => $dependencies, 'optional_dependencies' => is_array($meta['optional_dependencies'] ?? null) ? $meta['optional_dependencies'] : [], 'has_navbar' => (bool) ($meta['has_navbar'] ?? false), 'has_settings' => (bool) ($meta['has_settings'] ?? false), @@ -200,6 +255,7 @@ class ModuleManager { 'installed_version' => $overrides[$name]['installed_version'] ?? '', 'source' => $overrides[$name]['source'] ?? '', 'previous_version' => $overrides[$name]['previous_version'] ?? '', + 'dependency_warnings' => $dependencyWarnings, ]; } @@ -350,6 +406,23 @@ class ModuleManager { */ public function setState(string $name, \XcVm\Core\Enum\ModuleState $state): void { $name = $this->sanitizeModuleName($name); + + // Refuse to disable a module that still-enabled dependents rely on + // (e.g. plex requires watch — watch cannot be disabled under it, or + // ModuleLoader would skip plex on the next boot). Mirrors the guard in + // uninstallModule(). Scoped strictly to a deliberate Disabled transition: + // the internal lifecycle states (Installing, Failed) are also non-loadable + // but are set by installModule() itself and must never be blocked. + if ($state === \XcVm\Core\Enum\ModuleState::Disabled) { + $dependents = $this->enabledDependentsOf($name); + if (!empty($dependents)) { + throw new \RuntimeException( + "Cannot disable '{$name}': still required by " . implode(', ', $dependents) + . '. Disable ' . (count($dependents) === 1 ? 'it' : 'them') . ' first.' + ); + } + } + $overrides = $this->readOverrides(); if (!isset($overrides[$name]) || !is_array($overrides[$name])) { diff --git a/src/Public/Views/admin/api.php b/src/Public/Views/admin/api.php index 3a29d76a..0df19528 100644 --- a/src/Public/Views/admin/api.php +++ b/src/Public/Views/admin/api.php @@ -1375,15 +1375,15 @@ if (isset($_SESSION['hash'])) { $db->query('SELECT `server_id`, `time`, `cpu`, `iostat_info`, `total_mem_used_percent`, `connections`, `streams`, `users`, `total_users`, `bytes_received`, `bytes_sent` FROM `servers_stats` WHERE `server_id` IN (SELECT `id` FROM `servers` WHERE `server_type` = 0) AND `time` >= ? ORDER BY `time` DESC;', $rNearestRange); } - if (0 >= $db->num_rows()) { - } else { + if ($db->num_rows() > 0) { foreach ($db->get_rows() as $rRow) { - if (!$rServers[$rRow['server_id']]['server_online']) { - } else { + // servers_stats may reference a server_id absent from $rServers (orphan stat + // row from a deleted server); empty() skips the missing-key / null-offset + // case without a warning — same intent: unknown or offline server → skip. + if (!empty($rServers[$rRow['server_id']]['server_online'])) { $rNearest = AdminHelpers::getNearest($rStatsRange, intval($rRow['time'])); - if (isset($rStatsRange[$rNearest][intval($rRow['server_id'])])) { - } else { + if (!isset($rStatsRange[$rNearest][intval($rRow['server_id'])])) { $rServerStats[$rNearest][intval($rRow['server_id'])] = $rRow; } } @@ -2104,7 +2104,7 @@ if (isset($_SESSION['hash'])) { exit(); } -if (RequestManager::getAll()['action'] == 'listdir') { + if (RequestManager::getAll()['action'] == 'listdir') { if (Authorization::check('adv', 'add_episode') || Authorization::check('adv', 'edit_episode') || Authorization::check('adv', 'add_movie') || Authorization::check('adv', 'edit_movie') || Authorization::check('adv', 'create_channel') || Authorization::check('adv', 'edit_cchannel') || Authorization::check('adv', 'folder_watch_add')) { if (RequestManager::getAll()['filter'] == 'video') { $rFilter = array('mp4', 'mkv', 'avi', 'mpg', 'flv', '3gp', 'm4v', 'wmv', 'mov', 'ts'); diff --git a/src/Public/Views/admin/modules.php b/src/Public/Views/admin/modules.php index 50fff9c6..6e807319 100644 --- a/src/Public/Views/admin/modules.php +++ b/src/Public/Views/admin/modules.php @@ -94,6 +94,13 @@ if (!empty($_SERVER['HTTP_X_REQUESTED_WITH']) && strtolower($_SERVER['HTTP_X_REQ data-module=""> + + + + +
diff --git a/tests/Unit/ModuleLoaderPriorityTest.php b/tests/Unit/ModuleLoaderPriorityTest.php index 826ed215..07b06dcf 100644 --- a/tests/Unit/ModuleLoaderPriorityTest.php +++ b/tests/Unit/ModuleLoaderPriorityTest.php @@ -85,15 +85,16 @@ final class ModuleLoaderPriorityTest extends TestCase { ); } - public function testRequiredDependencyStillThrowsWhenMissing(): void { + public function testRequiredDependencyMissingSkipsDependentWithoutThrowing(): void { $root = $this->createModulesRoot(); $this->createModule($root, 'needs-ghost', ['dependencies' => ['ghost-module']]); $loader = new ModuleLoader(); - - $this->expectException(RuntimeException::class); - $this->expectExceptionMessageMatches('/requires missing dependency/'); $loader->loadAll($root); + + // A missing required dependency must skip only the dependent, never abort + // the whole load. + $this->assertFalse($loader->isLoaded('needs-ghost')); } // ── Helpers ─────────────────────────────────────────────── diff --git a/tests/Unit/ModuleLoaderTest.php b/tests/Unit/ModuleLoaderTest.php index a750dfca..dc429bdb 100644 --- a/tests/Unit/ModuleLoaderTest.php +++ b/tests/Unit/ModuleLoaderTest.php @@ -9,17 +9,37 @@ use XcVm\Core\Http\Router; use PHPUnit\Framework\TestCase; final class ModuleLoaderTest extends TestCase { - public function testLoadAllThrowsWhenDependencyMissing(): void { + public function testLoadAllSkipsModuleWithMissingDependencyInsteadOfThrowing(): void { $root = $this->createModulesRoot(); + // 'missing-alpha' requires a module that does not exist; a healthy sibling + // must still load. A missing required dependency must not abort the whole + // load (which would brick the panel and CLI). $this->createModule($root, 'missing-alpha', [ 'dependencies' => ['missing-module'], ]); + $this->createModule($root, 'healthy-beta'); $loader = new ModuleLoader(); - - $this->expectException(RuntimeException::class); - $this->expectExceptionMessage('requires missing dependency missing-module'); $loader->loadAll($root); + + $this->assertFalse($loader->isLoaded('missing-alpha')); + $this->assertTrue($loader->isLoaded('healthy-beta')); + } + + public function testLoadAllSkipsTransitiveDependentsOfMissingDependency(): void { + $root = $this->createModulesRoot(); + // chain-gamma -> chain-beta -> missing-module (absent). Both gamma and beta + // must be skipped; an unrelated module must still load. + $this->createModule($root, 'chain-beta', ['dependencies' => ['missing-module']]); + $this->createModule($root, 'chain-gamma', ['dependencies' => ['chain-beta']]); + $this->createModule($root, 'chain-solo'); + + $loader = new ModuleLoader(); + $loader->loadAll($root); + + $this->assertFalse($loader->isLoaded('chain-beta')); + $this->assertFalse($loader->isLoaded('chain-gamma')); + $this->assertTrue($loader->isLoaded('chain-solo')); } public function testLoadAllThrowsWhenDependenciesAreCyclic(): void { diff --git a/tests/Unit/ModuleManagerMigrationsTest.php b/tests/Unit/ModuleManagerMigrationsTest.php index c2caba84..dca946a2 100644 --- a/tests/Unit/ModuleManagerMigrationsTest.php +++ b/tests/Unit/ModuleManagerMigrationsTest.php @@ -137,10 +137,80 @@ final class ModuleManagerMigrationsTest extends TestCase { // ── Helpers ─────────────────────────────────────────────────────────── + // ── listModules() dependency diagnostics ────────────────────────────── + + public function testListModulesFlagsDisabledRequiredDependency(): void { + // dep-consumer requires dep-base, which is present but disabled. + $this->createModuleWithDeps('dep-base', '1.0.0', []); + $this->createModuleWithDeps('dep-consumer', '1.0.0', ['dep-base']); + $this->writeOverrides(['dep-base' => ['state' => 'disabled']]); + + $byName = $this->modulesByName(); + + $this->assertSame([], $byName['dep-base']['dependency_warnings']); + $this->assertCount(1, $byName['dep-consumer']['dependency_warnings']); + $this->assertStringContainsString('dep-base', $byName['dep-consumer']['dependency_warnings'][0]); + $this->assertStringContainsString('not enabled', $byName['dep-consumer']['dependency_warnings'][0]); + } + + public function testListModulesFlagsMissingRequiredDependency(): void { + $this->createModuleWithDeps('needs-absent', '1.0.0', ['ghost-module']); + + $byName = $this->modulesByName(); + + $this->assertCount(1, $byName['needs-absent']['dependency_warnings']); + $this->assertStringContainsString('missing', $byName['needs-absent']['dependency_warnings'][0]); + } + + public function testListModulesNoWarningWhenDependencyEnabled(): void { + $this->createModuleWithDeps('ok-base', '1.0.0', []); + $this->createModuleWithDeps('ok-consumer', '1.0.0', ['ok-base']); + + $byName = $this->modulesByName(); + + $this->assertSame([], $byName['ok-consumer']['dependency_warnings']); + } + private function manager(): ModuleManager { return new ModuleManager($this->modulesPath, $this->overridesPath, ServiceContainer::getInstance()); } + /** @return array listModules() keyed by module name. */ + private function modulesByName(): array { + $byName = []; + foreach ($this->manager()->listModules() as $module) { + $byName[$module['name']] = $module; + } + return $byName; + } + + /** Create a plain module whose manifest declares the given required dependencies. */ + private function createModuleWithDeps(string $name, string $version, array $dependencies): void { + $dir = $this->modulesPath . '/' . $name; + $pascal = $this->pascal($name); + $className = $pascal . 'Module'; + $namespace = 'XcVm\\Module\\' . $pascal; + mkdir($dir, 0775, true); + + $manifest = $this->manifest($name, $version); + $manifest['dependencies'] = $dependencies; + + file_put_contents( + $dir . '/module.json', + json_encode($manifest, JSON_PRETTY_PRINT | JSON_UNESCAPED_SLASHES) + ); + + file_put_contents($dir . '/' . $className . '.php', + "modulesPath . '/' . $name;