From 15bea633fa0b4eddec4a08b45f7fad4980cb5584 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Przepi=C3=B3ra?= Date: Wed, 30 Sep 2026 22:02:33 +0200 Subject: [PATCH] Only a directory named after the module is a module MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An updater that parks the previous version next to the module — as `Enterprise.poprzednia-20260930195044`, a full copy including module.json — made discover() report it as a second install of the same module, and as an ENABLED one, because enablement matches on the manifest name and the copy carries the same name. The operator saw two "OpenMES Enterprise" cards, v0.3.5 and v0.3.4, both enabled, both declaring the same provider class, with no way to tell which one "Uninstall" would remove. Worse than cosmetic: two directories claiming the same provider is a state nothing good comes out of. A module lives in a directory named after itself — that is how installFromZip() puts it there and how loadEnabled() finds it. Anything else carrying a module.json is a leftover: an updater's backup, a half-finished copy, an unpacked archive. Those are skipped now and logged, so a mismatch is visible rather than silently shaping the modules list. The module side is being fixed too — backups move out of modules/ — but core should not depend on every module getting that right. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01QRWsLoNeVWdsSHve6vUmxs --- backend/app/Services/ModuleManager.php | 19 ++++ backend/tests/Feature/ModuleDiscoveryTest.php | 89 +++++++++++++++++++ 2 files changed, 108 insertions(+) create mode 100644 backend/tests/Feature/ModuleDiscoveryTest.php diff --git a/backend/app/Services/ModuleManager.php b/backend/app/Services/ModuleManager.php index e2e566f2..228aa69e 100644 --- a/backend/app/Services/ModuleManager.php +++ b/backend/app/Services/ModuleManager.php @@ -5,6 +5,7 @@ use App\Support\CoreVersionConstraint; use Illuminate\Support\Collection; use Illuminate\Support\Facades\DB; +use Illuminate\Support\Facades\Log; class ModuleManager { @@ -44,6 +45,24 @@ public function discover(): Collection continue; } + // A module lives in a directory named after itself — that is how + // installFromZip() puts it there and how loadEnabled() finds it. + // Anything else carrying a module.json is a leftover: a backup an + // updater parked next to the module, a half-finished copy, an + // unpacked archive. Those used to show up here as a second, also + // "enabled" install of the same module, because enablement matches + // on the manifest name and the copy carries the same one — two + // cards, two directories, the same provider class, and no way for + // the operator to tell which is which. + if ($manifest['name'] !== $entry) { + Log::warning('module.directory_name_mismatch', [ + 'directory' => $entry, + 'declares' => $manifest['name'], + ]); + + continue; + } + $manifest['enabled'] = in_array($manifest['name'], $enabled); $manifest['directory'] = $entry; $manifest['has_error'] = false; diff --git a/backend/tests/Feature/ModuleDiscoveryTest.php b/backend/tests/Feature/ModuleDiscoveryTest.php new file mode 100644 index 00000000..ad1a0b4f --- /dev/null +++ b/backend/tests/Feature/ModuleDiscoveryTest.php @@ -0,0 +1,89 @@ + */ + private array $doPosprzatania = []; + + protected function setUp(): void + { + parent::setUp(); + + $this->modulesPath = base_path('modules'); + } + + protected function tearDown(): void + { + foreach ($this->doPosprzatania as $katalog) { + if (is_dir($katalog)) { + @unlink("{$katalog}/module.json"); + @rmdir($katalog); + } + } + + parent::tearDown(); + } + + private function zrobKatalogModulu(string $katalog, string $nazwaWManifescie): void + { + $sciezka = "{$this->modulesPath}/{$katalog}"; + @mkdir($sciezka, 0755, true); + + file_put_contents("{$sciezka}/module.json", json_encode([ + 'name' => $nazwaWManifescie, + 'display_name' => $nazwaWManifescie, + 'version' => '1.0.0', + 'provider' => "Modules\\{$nazwaWManifescie}\\Providers\\Provider", + ])); + + $this->doPosprzatania[] = $sciezka; + } + + public function test_katalog_nazwany_jak_modul_jest_wykrywany(): void + { + $this->zrobKatalogModulu('DiscoveryProbe', 'DiscoveryProbe'); + + $nazwy = app(ModuleManager::class)->discover()->pluck('name')->all(); + + $this->assertContains('DiscoveryProbe', $nazwy); + } + + public function test_kopia_obok_modulu_nie_jest_drugim_modulem(): void + { + $this->zrobKatalogModulu('DiscoveryProbe', 'DiscoveryProbe'); + $this->zrobKatalogModulu('DiscoveryProbe.poprzednia-20260930195044', 'DiscoveryProbe'); + + $moduly = app(ModuleManager::class)->discover() + ->where('name', 'DiscoveryProbe'); + + $this->assertCount(1, $moduly, 'kopia poprzedniej wersji pokazała się jako osobny moduł'); + $this->assertSame('DiscoveryProbe', $moduly->first()['directory']); + } + + public function test_katalog_deklarujacy_obca_nazwe_jest_pomijany(): void + { + $this->zrobKatalogModulu('DiscoveryProbe', 'ZupelnieCosInnego'); + + $nazwy = app(ModuleManager::class)->discover()->pluck('name')->all(); + + $this->assertNotContains('ZupelnieCosInnego', $nazwy); + } +}