From 7c73ac7c588ad37f4d748110d6ef4e97e216e825 Mon Sep 17 00:00:00 2001 From: Skyler Katz Date: Fri, 24 Apr 2026 10:28:08 -0500 Subject: [PATCH 1/4] sort module namespaces in decending order --- src/Support/ModuleRegistry.php | 160 ++++++++++++++++++--------------- tests/ModuleRegistryTest.php | 37 +++++++- 2 files changed, 123 insertions(+), 74 deletions(-) diff --git a/src/Support/ModuleRegistry.php b/src/Support/ModuleRegistry.php index 0353ae5..85fdd64 100644 --- a/src/Support/ModuleRegistry.php +++ b/src/Support/ModuleRegistry.php @@ -9,78 +9,92 @@ class ModuleRegistry { - protected ?Collection $modules = null; - - public function __construct( - protected string $modules_path, - protected Closure $modules_loader, + protected ?Collection $modules = null; + + protected ?array $namespace_index = null; + + public function __construct( + protected string $modules_path, + protected Closure $modules_loader, ) { - } - - public function getModulesPath(): string - { - return $this->modules_path; - } - - public function module(?string $name = null): ?ModuleConfig - { - // We want to allow for gracefully handling empty/null names - return $name ? $this->modules()->get($name) : null; - } - - public function moduleForPath(string $path): ?ModuleConfig - { - return $this->module($this->extractModuleNameFromPath($path)); - } - - public function moduleForPathOrFail(string $path): ModuleConfig - { - if ($module = $this->moduleForPath($path)) { - return $module; - } - - throw new CannotFindModuleForPathException($path); - } - - public function moduleForClass(string $fqcn): ?ModuleConfig - { - return $this->modules()->first(function(ModuleConfig $module) use ($fqcn) { - foreach ($module->namespaces as $namespace) { - if (Str::startsWith($fqcn, $namespace)) { - return true; - } - } - - return false; - }); - } - - /** @return Collection */ - public function modules(): Collection - { - return $this->modules ??= call_user_func($this->modules_loader); - } - - public function reload(): Collection - { - $this->modules = null; - - return $this->modules(); - } - - protected function extractModuleNameFromPath(string $path): string - { - // Handle Windows-style paths - $path = str_replace('\\', '/', $path); - - // If the modules directory is symlinked, we may get two paths that are actually - // in the same directory, but have different prefixes. This helps resolve that. - if (Str::startsWith($path, $this->modules_path)) { - $path = trim(Str::after($path, $this->modules_path), '/'); - } elseif (Str::startsWith($path, $modules_real_path = str_replace('\\', '/', realpath($this->modules_path)))) { - $path = trim(Str::after($path, $modules_real_path), '/'); - } - - return explode('/', $path)[0]; - } + } + + public function getModulesPath(): string + { + return $this->modules_path; + } + + public function module(?string $name = null): ?ModuleConfig + { + // We want to allow for gracefully handling empty/null names + return $name ? $this->modules()->get($name) : null; + } + + public function moduleForPath(string $path): ?ModuleConfig + { + return $this->module($this->extractModuleNameFromPath($path)); + } + + public function moduleForPathOrFail(string $path): ModuleConfig + { + if ($module = $this->moduleForPath($path)) { + return $module; + } + + throw new CannotFindModuleForPathException($path); + } + + public function moduleForClass(string $fqcn): ?ModuleConfig + { + foreach ($this->namespaceIndex() as $namespace => $module) { + if (Str::startsWith($fqcn, $namespace)) { + return $module; + } + } + + return null; + } + + /** @return Collection */ + public function modules(): Collection + { + return $this->modules ??= call_user_func($this->modules_loader); + } + + public function reload(): Collection + { + $this->modules = null; + $this->namespace_index = null; + + return $this->modules(); + } + + /** + * Namespaces sorted descending so the most-specific prefix wins on first match. + * + * @return array + */ + protected function namespaceIndex(): array + { + return $this->namespace_index ??= $this->modules() + ->flatMap(fn(ModuleConfig $module) => $module->namespaces->mapWithKeys(fn(string $namespace) => [$namespace => $module])) + ->sortKeysDesc() + ->all(); + } + + protected function extractModuleNameFromPath(string $path): string + { + // Handle Windows-style paths + $path = str_replace('\\', '/', $path); + + // If the modules directory is symlinked, we may get two paths that are actually + // in the same directory, but have different prefixes. This helps resolve that. + if (Str::startsWith($path, $this->modules_path)) { + $path = trim(Str::after($path, $this->modules_path), '/'); + } elseif (Str::startsWith($path, $modules_real_path = str_replace('\\', '/', realpath($this->modules_path)))) { + $path = trim(Str::after($path, $modules_real_path), '/'); + } + + return explode('/', $path)[0]; + } } diff --git a/tests/ModuleRegistryTest.php b/tests/ModuleRegistryTest.php index 36e2964..de2d725 100644 --- a/tests/ModuleRegistryTest.php +++ b/tests/ModuleRegistryTest.php @@ -2,6 +2,7 @@ namespace InterNACHI\Modular\Tests; +use Illuminate\Support\Collection; use InterNACHI\Modular\Support\ModuleConfig; use InterNACHI\Modular\Support\ModuleRegistry; use InterNACHI\Modular\Tests\Concerns\WritesToAppFilesystem; @@ -9,7 +10,41 @@ class ModuleRegistryTest extends TestCase { use WritesToAppFilesystem; - + + public function test_module_for_class_prefers_longest_matching_namespace(): void + { + $general = new ModuleConfig('general', '/tmp/general', new Collection([ + '/tmp/general/src' => 'App\\General\\', + ])); + + $specific = new ModuleConfig('specific', '/tmp/specific', new Collection([ + '/tmp/general-specific/src' => 'App\\General\\Specific\\', + ])); + + $specific_model = 'App\\General\\Specific\\Models\\Thing'; + + foreach ([[$general, $specific], [$specific, $general]] as $modules) { + $registry = new ModuleRegistry( + modules_path: '/tmp', + modules_loader: fn() => Collection::make($modules)->keyBy->name, + ); + + $this->assertSame( + 'specific', + $registry->moduleForClass($specific_model)?->name, + 'Expected the more-specific module to win regardless of iteration order', + ); + } + + $registry = new ModuleRegistry( + modules_path: '/tmp', + modules_loader: fn() => Collection::make([$general, $specific])->keyBy->name, + ); + + $this->assertSame('general', $registry->moduleForClass('App\\General\\Models\\Plain')?->name); + $this->assertNull($registry->moduleForClass('Unrelated\\Thing')); + } + public function test_it_resolves_modules(): void { $this->makeModule('test-module'); From 36f62ae9b2cf6d731ab9c05070393ee15360bb53 Mon Sep 17 00:00:00 2001 From: Skyler Katz Date: Mon, 27 Apr 2026 11:01:59 -0500 Subject: [PATCH 2/4] php-cs-fixer issues --- src/Support/ModuleRegistry.php | 170 ++++++++++++++++----------------- 1 file changed, 85 insertions(+), 85 deletions(-) diff --git a/src/Support/ModuleRegistry.php b/src/Support/ModuleRegistry.php index 85fdd64..da8ca20 100644 --- a/src/Support/ModuleRegistry.php +++ b/src/Support/ModuleRegistry.php @@ -9,92 +9,92 @@ class ModuleRegistry { - protected ?Collection $modules = null; + protected ?Collection $modules = null; - protected ?array $namespace_index = null; + protected ?array $namespace_index = null; - public function __construct( - protected string $modules_path, - protected Closure $modules_loader, + public function __construct( + protected string $modules_path, + protected Closure $modules_loader, ) { - } - - public function getModulesPath(): string - { - return $this->modules_path; - } - - public function module(?string $name = null): ?ModuleConfig - { - // We want to allow for gracefully handling empty/null names - return $name ? $this->modules()->get($name) : null; - } - - public function moduleForPath(string $path): ?ModuleConfig - { - return $this->module($this->extractModuleNameFromPath($path)); - } - - public function moduleForPathOrFail(string $path): ModuleConfig - { - if ($module = $this->moduleForPath($path)) { - return $module; - } - - throw new CannotFindModuleForPathException($path); - } - - public function moduleForClass(string $fqcn): ?ModuleConfig - { - foreach ($this->namespaceIndex() as $namespace => $module) { - if (Str::startsWith($fqcn, $namespace)) { - return $module; - } - } - - return null; - } - - /** @return Collection */ - public function modules(): Collection - { - return $this->modules ??= call_user_func($this->modules_loader); - } - - public function reload(): Collection - { - $this->modules = null; - $this->namespace_index = null; - - return $this->modules(); - } - - /** - * Namespaces sorted descending so the most-specific prefix wins on first match. - * - * @return array - */ - protected function namespaceIndex(): array - { - return $this->namespace_index ??= $this->modules() - ->flatMap(fn(ModuleConfig $module) => $module->namespaces->mapWithKeys(fn(string $namespace) => [$namespace => $module])) - ->sortKeysDesc() - ->all(); - } - - protected function extractModuleNameFromPath(string $path): string - { - // Handle Windows-style paths - $path = str_replace('\\', '/', $path); - - // If the modules directory is symlinked, we may get two paths that are actually - // in the same directory, but have different prefixes. This helps resolve that. - if (Str::startsWith($path, $this->modules_path)) { - $path = trim(Str::after($path, $this->modules_path), '/'); - } elseif (Str::startsWith($path, $modules_real_path = str_replace('\\', '/', realpath($this->modules_path)))) { - $path = trim(Str::after($path, $modules_real_path), '/'); - } - - return explode('/', $path)[0]; - } + } + + public function getModulesPath(): string + { + return $this->modules_path; + } + + public function module(?string $name = null): ?ModuleConfig + { + // We want to allow for gracefully handling empty/null names + return $name ? $this->modules()->get($name) : null; + } + + public function moduleForPath(string $path): ?ModuleConfig + { + return $this->module($this->extractModuleNameFromPath($path)); + } + + public function moduleForPathOrFail(string $path): ModuleConfig + { + if ($module = $this->moduleForPath($path)) { + return $module; + } + + throw new CannotFindModuleForPathException($path); + } + + public function moduleForClass(string $fqcn): ?ModuleConfig + { + foreach ($this->namespaceIndex() as $namespace => $module) { + if (Str::startsWith($fqcn, $namespace)) { + return $module; + } + } + + return null; + } + + /** @return Collection */ + public function modules(): Collection + { + return $this->modules ??= call_user_func($this->modules_loader); + } + + public function reload(): Collection + { + $this->modules = null; + $this->namespace_index = null; + + return $this->modules(); + } + + /** + * Namespaces sorted descending so the most-specific prefix wins on first match. + * + * @return array + */ + protected function namespaceIndex(): array + { + return $this->namespace_index ??= $this->modules() + ->flatMap(fn(ModuleConfig $module) => $module->namespaces->mapWithKeys(fn(string $namespace) => [$namespace => $module])) + ->sortKeysDesc() + ->all(); + } + + protected function extractModuleNameFromPath(string $path): string + { + // Handle Windows-style paths + $path = str_replace('\\', '/', $path); + + // If the modules directory is symlinked, we may get two paths that are actually + // in the same directory, but have different prefixes. This helps resolve that. + if (Str::startsWith($path, $this->modules_path)) { + $path = trim(Str::after($path, $this->modules_path), '/'); + } elseif (Str::startsWith($path, $modules_real_path = str_replace('\\', '/', realpath($this->modules_path)))) { + $path = trim(Str::after($path, $modules_real_path), '/'); + } + + return explode('/', $path)[0]; + } } From c3e656933bbffa528d29d746f76de63c11615a3e Mon Sep 17 00:00:00 2001 From: Chris Morrell Date: Mon, 27 Apr 2026 13:01:44 -0400 Subject: [PATCH 3/4] Revert some unnecessary whitespace changes --- src/Support/ModuleRegistry.php | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/src/Support/ModuleRegistry.php b/src/Support/ModuleRegistry.php index da8ca20..c1868fc 100644 --- a/src/Support/ModuleRegistry.php +++ b/src/Support/ModuleRegistry.php @@ -10,7 +10,7 @@ class ModuleRegistry { protected ?Collection $modules = null; - + protected ?array $namespace_index = null; public function __construct( @@ -18,32 +18,32 @@ public function __construct( protected Closure $modules_loader, ) { } - + public function getModulesPath(): string { return $this->modules_path; } - + public function module(?string $name = null): ?ModuleConfig { // We want to allow for gracefully handling empty/null names return $name ? $this->modules()->get($name) : null; } - + public function moduleForPath(string $path): ?ModuleConfig { return $this->module($this->extractModuleNameFromPath($path)); } - + public function moduleForPathOrFail(string $path): ModuleConfig { if ($module = $this->moduleForPath($path)) { return $module; } - + throw new CannotFindModuleForPathException($path); } - + public function moduleForClass(string $fqcn): ?ModuleConfig { foreach ($this->namespaceIndex() as $namespace => $module) { @@ -51,21 +51,21 @@ public function moduleForClass(string $fqcn): ?ModuleConfig return $module; } } - + return null; } - + /** @return Collection */ public function modules(): Collection { return $this->modules ??= call_user_func($this->modules_loader); } - + public function reload(): Collection { $this->modules = null; $this->namespace_index = null; - + return $this->modules(); } @@ -81,12 +81,12 @@ protected function namespaceIndex(): array ->sortKeysDesc() ->all(); } - + protected function extractModuleNameFromPath(string $path): string { // Handle Windows-style paths $path = str_replace('\\', '/', $path); - + // If the modules directory is symlinked, we may get two paths that are actually // in the same directory, but have different prefixes. This helps resolve that. if (Str::startsWith($path, $this->modules_path)) { @@ -94,7 +94,7 @@ protected function extractModuleNameFromPath(string $path): string } elseif (Str::startsWith($path, $modules_real_path = str_replace('\\', '/', realpath($this->modules_path)))) { $path = trim(Str::after($path, $modules_real_path), '/'); } - + return explode('/', $path)[0]; } } From af1a3f0cebc07797e0fbffaf1a2324e5179235f7 Mon Sep 17 00:00:00 2001 From: Chris Morrell Date: Mon, 27 Apr 2026 13:13:13 -0400 Subject: [PATCH 4/4] Cover a few more test cases --- tests/ModuleRegistryTest.php | 61 +++++++++++++++++++++++++----------- 1 file changed, 43 insertions(+), 18 deletions(-) diff --git a/tests/ModuleRegistryTest.php b/tests/ModuleRegistryTest.php index de2d725..08c99aa 100644 --- a/tests/ModuleRegistryTest.php +++ b/tests/ModuleRegistryTest.php @@ -15,34 +15,59 @@ public function test_module_for_class_prefers_longest_matching_namespace(): void { $general = new ModuleConfig('general', '/tmp/general', new Collection([ '/tmp/general/src' => 'App\\General\\', + '/tmp/general/tests' => 'App\\General\\Tests', ])); - + $specific = new ModuleConfig('specific', '/tmp/specific', new Collection([ '/tmp/general-specific/src' => 'App\\General\\Specific\\', + '/tmp/general-specific/tests' => 'App\\General\\Specific\\Tests', ])); - + + $loader_orders = [ + [$general, $specific], + [$specific, $general], + ]; + $specific_model = 'App\\General\\Specific\\Models\\Thing'; - - foreach ([[$general, $specific], [$specific, $general]] as $modules) { + $specific_test = 'App\\General\\Specific\\Tests\\ThingTest'; + $general_model = 'App\\General\\Models\\Thing'; + $general_test = 'App\\General\\Tests\\ThingTest'; + + foreach ($loader_orders as $modules) { $registry = new ModuleRegistry( modules_path: '/tmp', - modules_loader: fn() => Collection::make($modules)->keyBy->name, + modules_loader: fn() => Collection::make($modules)->keyBy('name'), ); - - $this->assertSame( - 'specific', - $registry->moduleForClass($specific_model)?->name, - 'Expected the more-specific module to win regardless of iteration order', + + $this->assertEquals( + $specific, + $registry->moduleForClass($specific_model), + 'Expected the more-specific module to win regardless of registration order (primary namespace)', + ); + + $this->assertEquals( + $specific, + $registry->moduleForClass($specific_test), + 'Expected the more-specific module to win regardless of registration order (secondary namespace)', + ); + + $this->assertEquals( + $general, + $registry->moduleForClass($general_model), + 'Expected the general module to return when a more-specific namespace does not match (primary namespace)', + ); + + $this->assertEquals( + $general, + $registry->moduleForClass($general_test), + 'Expected the general module to return when a more-specific namespace does not match (secondary namespace)', + ); + + $this->assertNull( + $registry->moduleForClass('Unrelated\\Thing'), + 'Expected unrelated class not to match any module regardless of order' ); } - - $registry = new ModuleRegistry( - modules_path: '/tmp', - modules_loader: fn() => Collection::make([$general, $specific])->keyBy->name, - ); - - $this->assertSame('general', $registry->moduleForClass('App\\General\\Models\\Plain')?->name); - $this->assertNull($registry->moduleForClass('Unrelated\\Thing')); } public function test_it_resolves_modules(): void