diff --git a/src/config/src/ConfigFactory.php b/src/config/src/ConfigFactory.php index d4641d401..94cff3440 100644 --- a/src/config/src/ConfigFactory.php +++ b/src/config/src/ConfigFactory.php @@ -21,7 +21,14 @@ public function __invoke(ContainerInterface $container) $rootConfig = $this->readConfig($configPath . '/hyperf.php'); $autoloadConfig = $this->readPaths($loadPaths, ['hyperf.php']); - $merged = array_replace_recursive(ProviderConfig::load(), $rootConfig, ...$autoloadConfig); + + // Merge all config sources: provider configs + root config + autoload configs + $allConfigs = [ProviderConfig::load(), $rootConfig, ...$autoloadConfig]; + $merged = array_reduce( + array_slice($allConfigs, 1), + ProviderConfig::mergeTwo(...), + $allConfigs[0] + ); return new Repository($merged); } diff --git a/src/config/src/ProviderConfig.php b/src/config/src/ProviderConfig.php index 82eebad41..0e8f88b20 100644 --- a/src/config/src/ProviderConfig.php +++ b/src/config/src/ProviderConfig.php @@ -6,6 +6,7 @@ use Hyperf\Collection\Arr; use Hyperf\Config\ProviderConfig as HyperfProviderConfig; +use Hyperf\Di\Definition\PriorityDefinition; use Hyperf\Support\Composer; use Hypervel\Support\ServiceProvider; use Throwable; @@ -85,4 +86,84 @@ protected static function packagesToIgnore(): array return array_merge($packages, $project); } + + /** + * Merge provider config arrays. + * + * Correctly handles: + * - Pure lists (numeric keys): appends values with deduplication + * - Associative arrays (string keys): recursively merges, later wins for scalars + * - Mixed arrays (e.g. listeners with priorities): appends numeric, merges string keys + * + * @return array + */ + protected static function merge(...$arrays): array + { + if (empty($arrays)) { + return []; + } + + $result = array_reduce( + array_slice($arrays, 1), + [static::class, 'mergeTwo'], + $arrays[0] + ); + + // Special handling for dependencies with PriorityDefinition + if (isset($result['dependencies'])) { + $result['dependencies'] = []; + foreach ($arrays as $item) { + foreach ($item['dependencies'] ?? [] as $key => $value) { + $depend = $result['dependencies'][$key] ?? null; + if (! $depend instanceof PriorityDefinition) { + $result['dependencies'][$key] = $value; + continue; + } + + if ($value instanceof PriorityDefinition) { + $depend->merge($value); + } + } + } + } + + return $result; + } + + /** + * Merge two config arrays. + * + * Correctly handles: + * - Pure lists (numeric keys): appends values with deduplication + * - Associative arrays (string keys): recursively merges, later wins for scalars + * - Mixed arrays (e.g. listeners with priorities): appends numeric, merges string keys + * + * This method is public so ConfigFactory can use the same merge semantics. + * + * @return array + */ + public static function mergeTwo(array $base, array $override): array + { + $result = $base; + + foreach ($override as $key => $value) { + if (is_int($key)) { + // Numeric key - append if not already present (deduplicate) + if (! in_array($value, $result, true)) { + $result[] = $value; + } + } elseif (! array_key_exists($key, $result)) { + // New string key - just add it + $result[$key] = $value; + } elseif (is_array($value) && is_array($result[$key])) { + // Both are arrays - recursively merge + $result[$key] = self::mergeTwo($result[$key], $value); + } else { + // Scalar or mixed types - override wins + $result[$key] = $value; + } + } + + return $result; + } } diff --git a/tests/Config/ConfigFactoryTest.php b/tests/Config/ConfigFactoryTest.php new file mode 100644 index 000000000..23fdf67b4 --- /dev/null +++ b/tests/Config/ConfigFactoryTest.php @@ -0,0 +1,192 @@ + [ + 'App\Commands\CommandA', + 'App\Commands\CommandB', + 'App\Commands\CommandC', + ], + 'listeners' => [ + 'App\Listeners\ListenerA', + 'App\Listeners\ListenerB', + ], + ]; + + // Simulates app config (e.g., config/commands.php adding custom commands) + $appConfig = [ + 'commands' => [ + 'App\Commands\CustomCommand', + ], + ]; + + // This simulates what ConfigFactory currently does + $result = $this->mergeConfigs($providerConfig, $appConfig); + + // All provider commands should still be present + $this->assertContains( + 'App\Commands\CommandA', + $result['commands'], + 'CommandA from provider config should be preserved' + ); + $this->assertContains( + 'App\Commands\CommandB', + $result['commands'], + 'CommandB from provider config should be preserved' + ); + $this->assertContains( + 'App\Commands\CommandC', + $result['commands'], + 'CommandC from provider config should be preserved' + ); + + // App's custom command should be added + $this->assertContains( + 'App\Commands\CustomCommand', + $result['commands'], + 'CustomCommand from app config should be added' + ); + + // Should have 4 commands total (3 from provider + 1 from app) + $this->assertCount(4, $result['commands'], 'Should have all 4 commands'); + } + + /** + * Test that the merge strategy correctly replaces scalar values in + * associative arrays (app config overrides provider config). + */ + public function testMergeReplacesScalarsInAssociativeArrays(): void + { + $providerConfig = [ + 'database' => [ + 'default' => 'sqlite', + 'connections' => [ + 'pgsql' => [ + 'driver' => 'pgsql', + 'host' => 'localhost', + 'port' => 5432, + ], + ], + ], + ]; + + $appConfig = [ + 'database' => [ + 'default' => 'pgsql', + 'connections' => [ + 'pgsql' => [ + 'host' => 'production-db.example.com', + ], + ], + ], + ]; + + $result = $this->mergeConfigs($providerConfig, $appConfig); + + // App's default should override provider's default + $this->assertSame('pgsql', $result['database']['default']); + + // App's host should override provider's host + $this->assertSame( + 'production-db.example.com', + $result['database']['connections']['pgsql']['host'] + ); + + // Driver should remain a string (not become an array) + $this->assertIsString( + $result['database']['connections']['pgsql']['driver'], + 'Driver should remain a string, not become an array' + ); + $this->assertSame('pgsql', $result['database']['connections']['pgsql']['driver']); + + // Provider's port should be preserved + $this->assertSame(5432, $result['database']['connections']['pgsql']['port']); + } + + /** + * Test merging multiple config arrays (simulating provider + root + autoload configs). + */ + public function testMergeMultipleConfigArrays(): void + { + $providerConfig = [ + 'commands' => ['CommandA', 'CommandB'], + 'app' => ['name' => 'Provider Default'], + ]; + + $rootConfig = [ + 'app' => ['debug' => true], + ]; + + $autoloadConfig1 = [ + 'commands' => ['CommandC'], + 'app' => ['name' => 'My App'], + ]; + + $autoloadConfig2 = [ + 'database' => ['default' => 'mysql'], + ]; + + // Merge all configs (simulating ConfigFactory behavior) + $result = $this->mergeConfigs($providerConfig, $rootConfig, $autoloadConfig1, $autoloadConfig2); + + // All commands should be combined + $this->assertContains('CommandA', $result['commands']); + $this->assertContains('CommandB', $result['commands']); + $this->assertContains('CommandC', $result['commands']); + + // Later app.name should win + $this->assertSame('My App', $result['app']['name']); + + // app.debug should be merged in + $this->assertTrue($result['app']['debug']); + + // database from autoloadConfig2 should be present + $this->assertSame('mysql', $result['database']['default']); + } + + /** + * Simulate ConfigFactory's merge behavior using ProviderConfig::merge(). + * + * ConfigFactory uses ProviderConfig::mergeTwo() directly for merging. + * We test via ProviderConfig::merge() which uses the same mergeTwo() method + * internally, ensuring identical merge semantics. + */ + private function mergeConfigs(array ...$configs): array + { + $method = new ReflectionMethod(ProviderConfig::class, 'merge'); + + return $method->invoke(null, ...$configs); + } +} diff --git a/tests/Config/MergeIntegrationTest.php b/tests/Config/MergeIntegrationTest.php new file mode 100644 index 000000000..09720b73d --- /dev/null +++ b/tests/Config/MergeIntegrationTest.php @@ -0,0 +1,291 @@ +loadConfigsFromDir(self::FIXTURES_DIR . '/base'); + $override = $this->loadConfigsFromDir(self::FIXTURES_DIR . '/override'); + $expected = $this->loadConfigsFromDir(self::FIXTURES_DIR . '/expected'); + + // Merge base and override using ProviderConfig::merge() + $merged = $this->callMerge($base, $override); + + foreach (self::CONFIG_KEYS as $key) { + $this->assertArrayHasKey($key, $merged, "Merged config should have key: {$key}"); + $this->assertArrayHasKey($key, $expected, "Expected config should have key: {$key}"); + + $this->assertSame( + $expected[$key], + $merged[$key], + "Config '{$key}' should match expected after merge" + ); + } + } + + /** + * Test database config merge - nested associative arrays with scalar overrides. + * + * Verifies: + * - Scalar values in nested arrays are overridden (not converted to arrays) + * - New nested keys are added + * - Existing nested keys are preserved when not overridden + */ + public function testDatabaseConfigMerge(): void + { + $base = ['database' => require self::FIXTURES_DIR . '/base/database.php']; + $override = ['database' => require self::FIXTURES_DIR . '/override/database.php']; + + $merged = $this->callMerge($base, $override); + + // Critical: driver must remain a string, not become an array + $this->assertIsString( + $merged['database']['connections']['pgsql']['driver'], + 'pgsql driver should be a string, not an array' + ); + $this->assertSame('pgsql', $merged['database']['connections']['pgsql']['driver']); + + // Host should be overridden + $this->assertSame('db.example.com', $merged['database']['connections']['pgsql']['host']); + + // Pool should be added from override + $this->assertArrayHasKey('pool', $merged['database']['connections']['pgsql']); + $this->assertSame(10.0, $merged['database']['connections']['pgsql']['pool']['connect_timeout']); + + // MySQL should be preserved from base + $this->assertSame('mysql', $merged['database']['connections']['mysql']['driver']); + + // SQLite should be added from override + $this->assertArrayHasKey('sqlite', $merged['database']['connections']); + $this->assertSame('sqlite', $merged['database']['connections']['sqlite']['driver']); + } + + /** + * Test listeners config merge - THE CRITICAL TEST. + * + * This tests the mixed array pattern where: + * - Numeric keys are regular listeners (appended) + * - String keys are listeners with priority values (must be preserved) + * + * This was the original bug: Arr::merge lost the string keys. + */ + public function testListenersConfigMergePreservesPriorityKeys(): void + { + $base = ['listeners' => require self::FIXTURES_DIR . '/base/listeners.php']; + $override = ['listeners' => require self::FIXTURES_DIR . '/override/listeners.php']; + + $merged = $this->callMerge($base, $override); + + // All numeric-keyed listeners should be present + $this->assertContains('App\Listeners\EventLoggerListener', $merged['listeners']); + $this->assertContains('App\Listeners\AuditListener', $merged['listeners']); + $this->assertContains('Hyperf\Command\Listener\RegisterCommandListener', $merged['listeners']); + $this->assertContains('Hyperf\ModelListener\Listener\ModelEventListener', $merged['listeners']); + $this->assertContains('Hyperf\Process\Listener\BootProcessListener', $merged['listeners']); + + // Priority listeners MUST have their string keys preserved + $this->assertArrayHasKey( + 'Hyperf\ModelListener\Listener\ModelHookEventListener', + $merged['listeners'], + 'ModelHookEventListener string key must be preserved' + ); + $this->assertSame( + 99, + $merged['listeners']['Hyperf\ModelListener\Listener\ModelHookEventListener'], + 'ModelHookEventListener priority must be 99' + ); + + $this->assertArrayHasKey( + 'Hyperf\Signal\Listener\SignalRegisterListener', + $merged['listeners'], + 'SignalRegisterListener string key must be preserved' + ); + $this->assertSame( + PHP_INT_MAX, + $merged['listeners']['Hyperf\Signal\Listener\SignalRegisterListener'], + 'SignalRegisterListener priority must be PHP_INT_MAX' + ); + + // Priority values should NOT appear as standalone numeric entries + $numericValues = array_values(array_filter( + $merged['listeners'], + fn ($v, $k) => is_int($k), + ARRAY_FILTER_USE_BOTH + )); + $this->assertNotContains(99, $numericValues, 'Priority 99 should not be a standalone entry'); + $this->assertNotContains(PHP_INT_MAX, $numericValues, 'Priority PHP_INT_MAX should not be a standalone entry'); + } + + /** + * Test commands config merge - pure list with deduplication. + */ + public function testCommandsConfigMergeDeduplicates(): void + { + $base = ['commands' => require self::FIXTURES_DIR . '/base/commands.php']; + $override = ['commands' => require self::FIXTURES_DIR . '/override/commands.php']; + + $merged = $this->callMerge($base, $override); + + // All unique commands should be present + $this->assertContains('App\Commands\MigrateCommand', $merged['commands']); + $this->assertContains('App\Commands\SeedCommand', $merged['commands']); + $this->assertContains('App\Commands\CacheCommand', $merged['commands']); + $this->assertContains('App\Commands\QueueCommand', $merged['commands']); + $this->assertContains('App\Commands\ScheduleCommand', $merged['commands']); + + // CacheCommand should appear only once (deduplicated) + $cacheCommandCount = count(array_filter( + $merged['commands'], + fn ($cmd) => $cmd === 'App\Commands\CacheCommand' + )); + $this->assertSame(1, $cacheCommandCount, 'CacheCommand should appear exactly once'); + + // Total should be 5 (3 from base + 2 new from override, 1 duplicate skipped) + $this->assertCount(5, $merged['commands']); + } + + /** + * Test cache config merge - deep nesting with type preservation. + */ + public function testCacheConfigMergePreservesTypes(): void + { + $base = ['cache' => require self::FIXTURES_DIR . '/base/cache.php']; + $override = ['cache' => require self::FIXTURES_DIR . '/override/cache.php']; + + $merged = $this->callMerge($base, $override); + + // Floats must remain floats + $this->assertIsFloat($merged['cache']['stores']['swoole']['memory_limit_buffer']); + $this->assertSame(0.05, $merged['cache']['stores']['swoole']['memory_limit_buffer']); + + $this->assertIsFloat($merged['cache']['stores']['swoole']['eviction_proportion']); + $this->assertSame(0.05, $merged['cache']['stores']['swoole']['eviction_proportion']); + + $this->assertIsFloat($merged['cache']['swoole_tables']['default']['conflict_proportion']); + $this->assertSame(0.2, $merged['cache']['swoole_tables']['default']['conflict_proportion']); + + // Integers must remain integers and be overridden correctly + $this->assertIsInt($merged['cache']['stores']['swoole']['eviction_interval']); + $this->assertSame(5000, $merged['cache']['stores']['swoole']['eviction_interval']); // Overridden + + $this->assertIsInt($merged['cache']['swoole_tables']['default']['rows']); + $this->assertSame(2048, $merged['cache']['swoole_tables']['default']['rows']); // Overridden + + // Boolean must remain boolean + $this->assertIsBool($merged['cache']['stores']['array']['serialize']); + $this->assertFalse($merged['cache']['stores']['array']['serialize']); + + // New store should be added + $this->assertArrayHasKey('database', $merged['cache']['stores']); + $this->assertSame('database', $merged['cache']['stores']['database']['driver']); + + // Numeric array inside should be preserved + $this->assertSame([2, 100], $merged['cache']['stores']['database']['lock_lottery']); + } + + /** + * Test app config merge - scalar overrides and list appending. + */ + public function testAppConfigMergeScalarsAndLists(): void + { + $base = ['app' => require self::FIXTURES_DIR . '/base/app.php']; + $override = ['app' => require self::FIXTURES_DIR . '/override/app.php']; + + $merged = $this->callMerge($base, $override); + + // Strings should be overridden + $this->assertSame('OverrideApp', $merged['app']['name']); + $this->assertSame('local', $merged['app']['env']); + + // Boolean should be overridden + $this->assertTrue($merged['app']['debug']); + + // Null should be overridden with value + $this->assertSame('base64:testkey123', $merged['app']['key']); + + // New key should be added + $this->assertArrayHasKey('new_setting', $merged['app']); + $this->assertSame('new_value', $merged['app']['new_setting']); + + // Preserved values should remain + $this->assertSame('http://localhost', $merged['app']['url']); + $this->assertSame('UTC', $merged['app']['timezone']); + + // Providers should be combined (not replaced) + $this->assertContains('App\Providers\AppServiceProvider', $merged['app']['providers']); + $this->assertContains('App\Providers\EventServiceProvider', $merged['app']['providers']); + $this->assertContains('App\Providers\RouteServiceProvider', $merged['app']['providers']); + $this->assertCount(3, $merged['app']['providers']); + } + + /** + * Load all PHP config files from a directory into a single array. + * + * @return array + */ + private function loadConfigsFromDir(string $dir): array + { + $configs = []; + $files = glob($dir . '/*.php'); + + foreach ($files as $file) { + $key = basename($file, '.php'); + $configs[$key] = require $file; + } + + return $configs; + } + + /** + * Call ProviderConfig::merge() via reflection. + * + * @return array + */ + private function callMerge(array ...$arrays): array + { + $method = new ReflectionMethod(ProviderConfig::class, 'merge'); + + return $method->invoke(null, ...$arrays); + } +} diff --git a/tests/Config/ProviderConfigTest.php b/tests/Config/ProviderConfigTest.php new file mode 100644 index 000000000..7d78cda8c --- /dev/null +++ b/tests/Config/ProviderConfigTest.php @@ -0,0 +1,766 @@ + ['driver' => 'pgsql']] + * Config B: ['database' => ['driver' => 'mysql']] + * Expected: ['database' => ['driver' => 'mysql']] + * Bug: ['database' => ['driver' => ['pgsql', 'mysql']]] + */ + public function testMergePreservesScalarValuesWithDuplicateKeys(): void + { + $configA = [ + 'database' => [ + 'default' => 'sqlite', + 'connections' => [ + 'sqlite' => [ + 'driver' => 'sqlite', + 'database' => '/path/to/database.sqlite', + ], + ], + ], + ]; + + $configB = [ + 'database' => [ + 'connections' => [ + 'sqlite' => [ + 'driver' => 'sqlite', + 'foreign_key_constraints' => true, + ], + ], + ], + ]; + + $result = $this->callMerge($configA, $configB); + + // The driver should remain a string, not become an array + $this->assertIsString( + $result['database']['connections']['sqlite']['driver'], + 'Driver should be a string, not an array. This indicates array_merge_recursive is incorrectly converting duplicate scalar keys into arrays.' + ); + $this->assertSame('sqlite', $result['database']['connections']['sqlite']['driver']); + + // Later config values should override earlier ones + $this->assertSame('sqlite', $result['database']['default']); + + // Both unique keys should be present + $this->assertSame('/path/to/database.sqlite', $result['database']['connections']['sqlite']['database']); + $this->assertTrue($result['database']['connections']['sqlite']['foreign_key_constraints']); + } + + /** + * Test that merging three configs still preserves scalar values. + */ + public function testMergeThreeConfigsPreservesScalarValues(): void + { + $configA = [ + 'app' => ['name' => 'First', 'debug' => false], + ]; + + $configB = [ + 'app' => ['name' => 'Second', 'timezone' => 'UTC'], + ]; + + $configC = [ + 'app' => ['name' => 'Third', 'debug' => true], + ]; + + $result = $this->callMerge($configA, $configB, $configC); + + // name should be the last value, not an array of all values + $this->assertIsString($result['app']['name']); + $this->assertSame('Third', $result['app']['name']); + + // debug should be the last value + $this->assertIsBool($result['app']['debug']); + $this->assertTrue($result['app']['debug']); + + // timezone from middle config should be preserved + $this->assertSame('UTC', $result['app']['timezone']); + } + + /** + * Test that numeric arrays are combined (appended), not replaced. + * + * Commands, listeners, etc. use numeric arrays and should be combined + * from all provider configs, not replaced. + */ + public function testMergeCombinesNumericArrays(): void + { + $configA = [ + 'commands' => ['CommandA', 'CommandB'], + ]; + + $configB = [ + 'commands' => ['CommandC'], + ]; + + $result = $this->callMerge($configA, $configB); + + // Numeric arrays should be combined (all commands from all packages) + $this->assertSame(['CommandA', 'CommandB', 'CommandC'], $result['commands']); + } + + /** + * Test that listeners with priority values preserve both the class and priority. + * + * Hyperf's listener config uses a mixed pattern where: + * - Simple listeners are numeric-keyed: ['ListenerA', 'ListenerB'] + * - Listeners with priority use string keys: ['PriorityListener' => 99] + */ + public function testMergePreservesListenersWithPriority(): void + { + $configA = [ + 'listeners' => [ + 'App\Listeners\ListenerA', + 'App\Listeners\ListenerB', + ], + ]; + + $configB = [ + 'listeners' => [ + 'Hyperf\ModelListener\Listener\ModelEventListener', + 'Hyperf\ModelListener\Listener\ModelHookEventListener' => 99, + ], + ]; + + $result = $this->callMerge($configA, $configB); + + // All simple listeners should be present + $this->assertContains('App\Listeners\ListenerA', $result['listeners']); + $this->assertContains('App\Listeners\ListenerB', $result['listeners']); + $this->assertContains('Hyperf\ModelListener\Listener\ModelEventListener', $result['listeners']); + + // Priority listener should have its string key preserved with the priority value + $this->assertArrayHasKey( + 'Hyperf\ModelListener\Listener\ModelHookEventListener', + $result['listeners'], + 'Priority listener class name should be preserved as a string key' + ); + $this->assertSame( + 99, + $result['listeners']['Hyperf\ModelListener\Listener\ModelHookEventListener'], + 'Priority value should be preserved' + ); + + // The priority value (99) should NOT appear as a standalone numeric entry + $numericValues = array_filter($result['listeners'], fn ($v, $k) => is_int($k), ARRAY_FILTER_USE_BOTH); + $this->assertNotContains( + 99, + $numericValues, + 'Priority value should not be a standalone entry - this indicates the string key was lost' + ); + } + + /** + * Test that merging empty arrays returns an empty array. + */ + public function testMergeWithNoArraysReturnsEmpty(): void + { + $result = $this->callMerge(); + + $this->assertSame([], $result); + } + + /** + * Test that merging a single array returns it unchanged. + */ + public function testMergeSingleArrayReturnsUnchanged(): void + { + $config = [ + 'app' => ['name' => 'MyApp', 'debug' => true], + 'commands' => ['CommandA', 'CommandB'], + ]; + + $result = $this->callMerge($config); + + $this->assertSame($config, $result); + } + + /** + * Test deeply nested config structures are merged correctly. + */ + public function testMergeDeeplyNestedConfigs(): void + { + $configA = [ + 'level1' => [ + 'level2' => [ + 'level3' => [ + 'level4' => [ + 'value' => 'original', + 'only_in_a' => 'a_value', + ], + ], + ], + ], + ]; + + $configB = [ + 'level1' => [ + 'level2' => [ + 'level3' => [ + 'level4' => [ + 'value' => 'overridden', + 'only_in_b' => 'b_value', + ], + ], + ], + ], + ]; + + $result = $this->callMerge($configA, $configB); + + // Scalar at deep level should be overridden + $this->assertSame('overridden', $result['level1']['level2']['level3']['level4']['value']); + + // Unique keys from both should be present + $this->assertSame('a_value', $result['level1']['level2']['level3']['level4']['only_in_a']); + $this->assertSame('b_value', $result['level1']['level2']['level3']['level4']['only_in_b']); + } + + /** + * Test that null values are handled correctly (replaced like any scalar). + */ + public function testMergeHandlesNullValues(): void + { + $configA = [ + 'app' => ['value' => 'not_null', 'nullable' => null], + ]; + + $configB = [ + 'app' => ['value' => null, 'nullable' => 'now_has_value'], + ]; + + $result = $this->callMerge($configA, $configB); + + // Later null should override earlier value + $this->assertNull($result['app']['value']); + + // Later value should override earlier null + $this->assertSame('now_has_value', $result['app']['nullable']); + } + + /** + * Test arrays with mixed numeric and string keys. + * + * Numeric keys are appended (with deduplication), string keys are replaced. + * This matches Hyperf's listener pattern: ['ListenerA', 'PriorityListener' => 99] + */ + public function testMergeMixedNumericAndStringKeys(): void + { + $configA = [ + 'mixed' => [ + 'numeric_0', + 'string_key' => 'value_a', + 'numeric_1', + ], + ]; + + $configB = [ + 'mixed' => [ + 'another_numeric', + 'string_key' => 'value_b', + ], + ]; + + $result = $this->callMerge($configA, $configB); + + // String key should be replaced + $this->assertSame('value_b', $result['mixed']['string_key']); + + // Numeric-keyed values should be appended + $this->assertContains('numeric_0', $result['mixed']); + $this->assertContains('numeric_1', $result['mixed']); + $this->assertContains('another_numeric', $result['mixed']); + + // Should have 4 entries: 3 numeric + 1 string key + $this->assertCount(4, $result['mixed']); + } + + /** + * Test that scalar value replaces array value (later scalar wins). + */ + public function testMergeScalarReplacesArray(): void + { + $configA = [ + 'setting' => ['complex' => 'array', 'with' => 'values'], + ]; + + $configB = [ + 'setting' => 'simple_string', + ]; + + $result = $this->callMerge($configA, $configB); + + // Scalar should completely replace array + $this->assertIsString($result['setting']); + $this->assertSame('simple_string', $result['setting']); + } + + /** + * Test that duplicate values in numeric arrays are deduplicated. + * + * The merge logic deduplicates by default, which prevents listeners + * from firing twice when multiple providers include the same class. + */ + public function testMergeDeduplicatesNumericArrays(): void + { + $configA = [ + 'listeners' => ['ListenerA', 'SharedListener'], + ]; + + $configB = [ + 'listeners' => ['SharedListener', 'ListenerB'], + ]; + + $result = $this->callMerge($configA, $configB); + + // SharedListener should only appear once (deduplicated) + $this->assertSame( + ['ListenerA', 'SharedListener', 'ListenerB'], + $result['listeners'] + ); + } + + /** + * Test that numeric string keys are converted to integers by PHP. + * + * PHP automatically converts numeric string keys like '80' to integers. + * This means they're treated as numeric keys (appended with deduplication). + * This is PHP behavior, not something we can change. + */ + public function testMergeNumericStringKeysAreConvertedToIntegers(): void + { + $configA = [ + 'ports' => [ + '80' => 'http', + '443' => 'https', + ], + ]; + + $configB = [ + 'ports' => [ + '80' => 'http_updated', + '8080' => 'alt_http', + ], + ]; + + $result = $this->callMerge($configA, $configB); + + // PHP converts '80' to int 80, so these are numeric keys + // Numeric keys append with deduplication + // 'http' and 'http_updated' are different values, so both are kept + $this->assertContains('http', $result['ports']); + $this->assertContains('https', $result['ports']); + $this->assertContains('http_updated', $result['ports']); + $this->assertContains('alt_http', $result['ports']); + } + + /** + * Test nested numeric arrays within associative structures. + */ + public function testMergeNestedNumericArraysWithinAssociative(): void + { + $configA = [ + 'annotations' => [ + 'scan' => [ + 'paths' => ['/path/a', '/path/b'], + 'collectors' => ['CollectorA'], + ], + ], + ]; + + $configB = [ + 'annotations' => [ + 'scan' => [ + 'paths' => ['/path/c'], + 'ignore_annotations' => ['IgnoreMe'], + ], + ], + ]; + + $result = $this->callMerge($configA, $configB); + + // Numeric arrays should be combined + $this->assertSame(['/path/a', '/path/b', '/path/c'], $result['annotations']['scan']['paths']); + $this->assertSame(['CollectorA'], $result['annotations']['scan']['collectors']); + $this->assertSame(['IgnoreMe'], $result['annotations']['scan']['ignore_annotations']); + } + + /** + * Test boolean values are preserved correctly. + */ + public function testMergePreservesBooleanTypes(): void + { + $configA = [ + 'flags' => ['enabled' => true, 'verbose' => false], + ]; + + $configB = [ + 'flags' => ['enabled' => false, 'debug' => true], + ]; + + $result = $this->callMerge($configA, $configB); + + $this->assertFalse($result['flags']['enabled']); + $this->assertFalse($result['flags']['verbose']); + $this->assertTrue($result['flags']['debug']); + + // Ensure they're actual booleans, not arrays + $this->assertIsBool($result['flags']['enabled']); + $this->assertIsBool($result['flags']['verbose']); + $this->assertIsBool($result['flags']['debug']); + } + + /** + * Test integer values are preserved correctly. + */ + public function testMergePreservesIntegerTypes(): void + { + $configA = [ + 'limits' => ['timeout' => 30, 'retries' => 3], + ]; + + $configB = [ + 'limits' => ['timeout' => 60, 'max_connections' => 100], + ]; + + $result = $this->callMerge($configA, $configB); + + $this->assertSame(60, $result['limits']['timeout']); + $this->assertSame(3, $result['limits']['retries']); + $this->assertSame(100, $result['limits']['max_connections']); + + // Ensure they're actual integers, not arrays + $this->assertIsInt($result['limits']['timeout']); + $this->assertIsInt($result['limits']['retries']); + $this->assertIsInt($result['limits']['max_connections']); + } + + /** + * Test the real-world database config scenario that originally caused the bug. + */ + public function testMergeRealWorldDatabaseConfigScenario(): void + { + // Simulates CoreServiceProvider config + $coreConfig = [ + 'database' => [ + 'default' => 'sqlite', + 'connections' => [ + 'sqlite' => [ + 'driver' => 'sqlite', + 'database' => '/app/database.sqlite', + 'prefix' => '', + ], + 'pgsql' => [ + 'driver' => 'pgsql', + 'host' => 'localhost', + 'port' => 5432, + ], + ], + ], + ]; + + // Simulates DatabaseServiceProvider config (overlapping sqlite config) + $databaseConfig = [ + 'database' => [ + 'connections' => [ + 'sqlite' => [ + 'driver' => 'sqlite', + 'foreign_key_constraints' => true, + ], + ], + ], + ]; + + // Simulates another package adding a mysql connection + $mysqlConfig = [ + 'database' => [ + 'connections' => [ + 'mysql' => [ + 'driver' => 'mysql', + 'host' => 'localhost', + 'port' => 3306, + ], + ], + ], + ]; + + $result = $this->callMerge($coreConfig, $databaseConfig, $mysqlConfig); + + // All drivers must be strings, not arrays + $this->assertIsString($result['database']['connections']['sqlite']['driver']); + $this->assertIsString($result['database']['connections']['pgsql']['driver']); + $this->assertIsString($result['database']['connections']['mysql']['driver']); + + $this->assertSame('sqlite', $result['database']['connections']['sqlite']['driver']); + $this->assertSame('pgsql', $result['database']['connections']['pgsql']['driver']); + $this->assertSame('mysql', $result['database']['connections']['mysql']['driver']); + + // All unique keys should be present + $this->assertSame('/app/database.sqlite', $result['database']['connections']['sqlite']['database']); + $this->assertTrue($result['database']['connections']['sqlite']['foreign_key_constraints']); + $this->assertSame(5432, $result['database']['connections']['pgsql']['port']); + $this->assertSame(3306, $result['database']['connections']['mysql']['port']); + } + + /** + * Test that PriorityDefinition objects are merged correctly. + * + * When multiple providers define the same dependency using PriorityDefinition, + * the definitions should be merged and the highest priority implementation wins. + */ + public function testMergeDependenciesWithPriorityDefinition(): void + { + $configA = [ + 'dependencies' => [ + 'SomeInterface' => new PriorityDefinition('ImplementationA', 5), + ], + ]; + + $configB = [ + 'dependencies' => [ + 'SomeInterface' => new PriorityDefinition('ImplementationB', 10), + ], + ]; + + $result = $this->callMerge($configA, $configB); + + // The result should be a PriorityDefinition + $this->assertArrayHasKey('SomeInterface', $result['dependencies']); + $definition = $result['dependencies']['SomeInterface']; + $this->assertInstanceOf(PriorityDefinition::class, $definition); + + // Higher priority (10) should win when getting the definition + $this->assertSame('ImplementationB', $definition->getDefinition()); + + // Both implementations should be tracked in the definition + $dependencies = $definition->getDependencies(); + $this->assertArrayHasKey('ImplementationA', $dependencies); + $this->assertArrayHasKey('ImplementationB', $dependencies); + $this->assertSame(5, $dependencies['ImplementationA']); + $this->assertSame(10, $dependencies['ImplementationB']); + } + + /** + * Test merging three configs with PriorityDefinition. + * + * The implementation with the highest priority should win regardless of order. + */ + public function testMergeThreeConfigsWithPriorityDefinition(): void + { + $configA = [ + 'dependencies' => [ + 'CacheInterface' => new PriorityDefinition('RedisCache', 10), + ], + ]; + + $configB = [ + 'dependencies' => [ + 'CacheInterface' => new PriorityDefinition('MemoryCache', 5), + ], + ]; + + $configC = [ + 'dependencies' => [ + 'CacheInterface' => new PriorityDefinition('FileCache', 15), + ], + ]; + + $result = $this->callMerge($configA, $configB, $configC); + + $definition = $result['dependencies']['CacheInterface']; + $this->assertInstanceOf(PriorityDefinition::class, $definition); + + // FileCache has highest priority (15), should win + $this->assertSame('FileCache', $definition->getDefinition()); + + // All three should be tracked + $dependencies = $definition->getDependencies(); + $this->assertCount(3, $dependencies); + $this->assertSame(10, $dependencies['RedisCache']); + $this->assertSame(5, $dependencies['MemoryCache']); + $this->assertSame(15, $dependencies['FileCache']); + } + + /** + * Test that plain dependency value followed by PriorityDefinition works. + * + * When a plain value is defined first and then a PriorityDefinition, + * the PriorityDefinition should take over. + */ + public function testMergePlainDependencyThenPriorityDefinition(): void + { + $configA = [ + 'dependencies' => [ + 'LoggerInterface' => 'FileLogger', + ], + ]; + + $configB = [ + 'dependencies' => [ + 'LoggerInterface' => new PriorityDefinition('DatabaseLogger', 10), + ], + ]; + + $result = $this->callMerge($configA, $configB); + + // The PriorityDefinition should take over + $definition = $result['dependencies']['LoggerInterface']; + $this->assertInstanceOf(PriorityDefinition::class, $definition); + $this->assertSame('DatabaseLogger', $definition->getDefinition()); + } + + /** + * Test that PriorityDefinition followed by plain value preserves PriorityDefinition. + * + * This is the intended Hyperf behavior: when a PriorityDefinition is defined first + * and a plain value comes later, the plain value is ignored because PriorityDefinition + * is only merged with other PriorityDefinitions. + */ + public function testMergePriorityDefinitionThenPlainDependency(): void + { + $configA = [ + 'dependencies' => [ + 'LoggerInterface' => new PriorityDefinition('DatabaseLogger', 10), + ], + ]; + + $configB = [ + 'dependencies' => [ + 'LoggerInterface' => 'FileLogger', + ], + ]; + + $result = $this->callMerge($configA, $configB); + + // The PriorityDefinition should be preserved (plain value ignored) + // This matches Hyperf's behavior - PriorityDefinition only merges with PriorityDefinition + $definition = $result['dependencies']['LoggerInterface']; + $this->assertInstanceOf(PriorityDefinition::class, $definition); + $this->assertSame('DatabaseLogger', $definition->getDefinition()); + } + + /** + * Test mixed dependencies - some plain, some with PriorityDefinition. + */ + public function testMergeMixedDependencies(): void + { + $configA = [ + 'dependencies' => [ + 'CacheInterface' => new PriorityDefinition('RedisCache', 5), + 'LoggerInterface' => 'FileLogger', + 'QueueInterface' => 'SyncQueue', + ], + ]; + + $configB = [ + 'dependencies' => [ + 'CacheInterface' => new PriorityDefinition('MemoryCache', 10), + 'LoggerInterface' => 'DatabaseLogger', + 'MailerInterface' => 'SmtpMailer', + ], + ]; + + $result = $this->callMerge($configA, $configB); + + // CacheInterface: PriorityDefinition merged, higher priority wins + $cacheDefinition = $result['dependencies']['CacheInterface']; + $this->assertInstanceOf(PriorityDefinition::class, $cacheDefinition); + $this->assertSame('MemoryCache', $cacheDefinition->getDefinition()); + + // LoggerInterface: Plain values, last wins + $this->assertSame('DatabaseLogger', $result['dependencies']['LoggerInterface']); + + // QueueInterface: Only in first config, preserved + $this->assertSame('SyncQueue', $result['dependencies']['QueueInterface']); + + // MailerInterface: Only in second config, added + $this->assertSame('SmtpMailer', $result['dependencies']['MailerInterface']); + } + + /** + * Test that mergeTwo() is callable as a public static method. + * + * This method is public so ConfigFactory can use the same merge semantics + * without duplicating the implementation. + */ + public function testMergeTwoIsPublicAndWorksDirectly(): void + { + $base = [ + 'commands' => ['CommandA', 'CommandB'], + 'database' => [ + 'default' => 'sqlite', + 'connections' => [ + 'sqlite' => ['driver' => 'sqlite'], + ], + ], + ]; + + $override = [ + 'commands' => ['CommandC'], + 'database' => [ + 'default' => 'pgsql', + 'connections' => [ + 'pgsql' => ['driver' => 'pgsql', 'host' => 'localhost'], + ], + ], + ]; + + // Call directly without reflection - this verifies it's public + $result = ProviderConfig::mergeTwo($base, $override); + + // Numeric arrays are combined + $this->assertSame(['CommandA', 'CommandB', 'CommandC'], $result['commands']); + + // Scalar values are overridden + $this->assertSame('pgsql', $result['database']['default']); + + // Nested arrays are merged recursively + $this->assertArrayHasKey('sqlite', $result['database']['connections']); + $this->assertArrayHasKey('pgsql', $result['database']['connections']); + $this->assertSame('sqlite', $result['database']['connections']['sqlite']['driver']); + $this->assertSame('pgsql', $result['database']['connections']['pgsql']['driver']); + } + + /** + * Call the protected merge method via reflection. + */ + private function callMerge(array ...$arrays): array + { + $method = new ReflectionMethod(ProviderConfig::class, 'merge'); + + return $method->invoke(null, ...$arrays); + } +} diff --git a/tests/Config/fixtures/base/app.php b/tests/Config/fixtures/base/app.php new file mode 100644 index 000000000..143aefcfc --- /dev/null +++ b/tests/Config/fixtures/base/app.php @@ -0,0 +1,26 @@ + 'BaseApp', + 'env' => 'production', + 'debug' => false, + 'url' => 'http://localhost', + 'timezone' => 'UTC', + 'locale' => 'en', + 'fallback_locale' => 'en', + 'cipher' => 'AES-256-CBC', + 'key' => null, + 'maintenance' => [ + 'driver' => 'file', + 'store' => 'redis', + ], + 'providers' => [ + 'App\Providers\AppServiceProvider', + 'App\Providers\EventServiceProvider', + ], +]; diff --git a/tests/Config/fixtures/base/cache.php b/tests/Config/fixtures/base/cache.php new file mode 100644 index 000000000..e3d44fde7 --- /dev/null +++ b/tests/Config/fixtures/base/cache.php @@ -0,0 +1,37 @@ + 'redis', + 'stores' => [ + 'array' => [ + 'driver' => 'array', + 'serialize' => false, + ], + 'redis' => [ + 'driver' => 'redis', + 'connection' => 'default', + 'lock_connection' => 'default', + ], + 'swoole' => [ + 'driver' => 'swoole', + 'table' => 'default', + 'memory_limit_buffer' => 0.05, + 'eviction_policy' => 'lru', + 'eviction_proportion' => 0.05, + 'eviction_interval' => 10000, + ], + ], + 'swoole_tables' => [ + 'default' => [ + 'rows' => 1024, + 'bytes' => 10240, + 'conflict_proportion' => 0.2, + ], + ], + 'prefix' => 'app_cache_', +]; diff --git a/tests/Config/fixtures/base/commands.php b/tests/Config/fixtures/base/commands.php new file mode 100644 index 000000000..7e2ee0e8c --- /dev/null +++ b/tests/Config/fixtures/base/commands.php @@ -0,0 +1,12 @@ + 'pgsql', + 'connections' => [ + 'pgsql' => [ + 'driver' => 'pgsql', + 'host' => 'localhost', + 'port' => 5432, + 'database' => 'app', + 'username' => 'postgres', + 'password' => '', + 'charset' => 'utf8', + 'prefix' => '', + 'prefix_indexes' => true, + 'schema' => 'public', + 'sslmode' => 'prefer', + ], + 'mysql' => [ + 'driver' => 'mysql', + 'host' => 'localhost', + 'port' => 3306, + 'database' => 'app', + 'username' => 'root', + 'password' => '', + 'charset' => 'utf8mb4', + 'collation' => 'utf8mb4_unicode_ci', + 'prefix' => '', + 'prefix_indexes' => true, + ], + ], + 'migrations' => 'migrations', + 'redis' => [ + 'default' => [ + 'host' => 'localhost', + 'port' => 6379, + 'db' => 0, + ], + ], +]; diff --git a/tests/Config/fixtures/base/listeners.php b/tests/Config/fixtures/base/listeners.php new file mode 100644 index 000000000..7e90b11b1 --- /dev/null +++ b/tests/Config/fixtures/base/listeners.php @@ -0,0 +1,15 @@ + true) + * - Null is overridden with value (key: null -> 'base64:...') + * - New keys are added (new_setting) + * - Providers list is combined (not replaced!) + */ +return [ + 'name' => 'OverrideApp', // Overridden + 'env' => 'local', // Overridden + 'debug' => true, // Overridden from false + 'url' => 'http://localhost', // Preserved + 'timezone' => 'UTC', // Preserved + 'locale' => 'en', // Preserved + 'fallback_locale' => 'en', // Preserved + 'cipher' => 'AES-256-CBC', // Preserved + 'key' => 'base64:testkey123', // Overridden from null + 'maintenance' => [ + 'driver' => 'file', + 'store' => 'redis', + ], + 'providers' => [ + // From base + 'App\Providers\AppServiceProvider', + 'App\Providers\EventServiceProvider', + // From override (appended) + 'App\Providers\RouteServiceProvider', + ], + 'new_setting' => 'new_value', // Added from override +]; diff --git a/tests/Config/fixtures/expected/cache.php b/tests/Config/fixtures/expected/cache.php new file mode 100644 index 000000000..3b010f931 --- /dev/null +++ b/tests/Config/fixtures/expected/cache.php @@ -0,0 +1,54 @@ + 'redis', + 'stores' => [ + 'array' => [ + 'driver' => 'array', + 'serialize' => false, + ], + 'redis' => [ + 'driver' => 'redis', + 'connection' => 'default', + 'lock_connection' => 'default', + ], + 'swoole' => [ + 'driver' => 'swoole', + 'table' => 'default', + 'memory_limit_buffer' => 0.05, // Float preserved + 'eviction_policy' => 'lru', + 'eviction_proportion' => 0.05, // Float preserved + 'eviction_interval' => 5000, // Overridden from 10000 + ], + 'database' => [ // Added from override + 'driver' => 'database', + 'connection' => 'pgsql', + 'table' => 'cache', + 'lock_connection' => 'pgsql', + 'lock_table' => 'cache_locks', + 'lock_lottery' => [2, 100], + 'lock_timeout' => 86400, + ], + ], + 'swoole_tables' => [ + 'default' => [ + 'rows' => 2048, // Overridden from 1024 + 'bytes' => 10240, + 'conflict_proportion' => 0.2, // Float preserved + ], + ], + 'prefix' => 'app_cache_', +]; diff --git a/tests/Config/fixtures/expected/commands.php b/tests/Config/fixtures/expected/commands.php new file mode 100644 index 000000000..31c699edc --- /dev/null +++ b/tests/Config/fixtures/expected/commands.php @@ -0,0 +1,18 @@ + 'pgsql', + 'connections' => [ + 'pgsql' => [ + 'driver' => 'pgsql', + 'host' => 'db.example.com', // Overridden + 'port' => 5432, + 'database' => 'app', + 'username' => 'postgres', + 'password' => '', + 'charset' => 'utf8', + 'prefix' => '', + 'prefix_indexes' => true, + 'schema' => 'public', + 'sslmode' => 'prefer', + 'pool' => [ // Added from override + 'min_connections' => 1, + 'max_connections' => 10, + 'connect_timeout' => 10.0, + 'wait_timeout' => 3.0, + ], + ], + 'mysql' => [ + 'driver' => 'mysql', + 'host' => 'localhost', + 'port' => 3306, + 'database' => 'app', + 'username' => 'root', + 'password' => '', + 'charset' => 'utf8mb4', + 'collation' => 'utf8mb4_unicode_ci', + 'prefix' => '', + 'prefix_indexes' => true, + ], + 'sqlite' => [ // Added from override + 'driver' => 'sqlite', + 'database' => ':memory:', + 'prefix' => '', + 'foreign_key_constraints' => true, + ], + ], + 'migrations' => 'migrations', + 'redis' => [ + 'default' => [ + 'host' => 'localhost', + 'port' => 6379, + 'db' => 0, + ], + 'queue' => [ // Added from override + 'host' => 'redis-queue.example.com', + 'port' => 6379, + 'db' => 1, + ], + ], +]; diff --git a/tests/Config/fixtures/expected/listeners.php b/tests/Config/fixtures/expected/listeners.php new file mode 100644 index 000000000..a6c80da1b --- /dev/null +++ b/tests/Config/fixtures/expected/listeners.php @@ -0,0 +1,27 @@ + 'App\Listeners\EventLoggerListener', + 1 => 'App\Listeners\AuditListener', + 2 => 'Hyperf\Command\Listener\RegisterCommandListener', + + // From override (numeric keys - appended) + 3 => 'Hyperf\ModelListener\Listener\ModelEventListener', + 4 => 'Hyperf\Process\Listener\BootProcessListener', + + // From override (string keys with priority - MUST be preserved) + 'Hyperf\ModelListener\Listener\ModelHookEventListener' => 99, + 'Hyperf\Signal\Listener\SignalRegisterListener' => PHP_INT_MAX, +]; diff --git a/tests/Config/fixtures/override/app.php b/tests/Config/fixtures/override/app.php new file mode 100644 index 000000000..ee0e3b076 --- /dev/null +++ b/tests/Config/fixtures/override/app.php @@ -0,0 +1,22 @@ + 'OverrideApp', + 'env' => 'local', + 'debug' => true, + 'key' => 'base64:testkey123', + 'new_setting' => 'new_value', + 'providers' => [ + 'App\Providers\RouteServiceProvider', + ], +]; diff --git a/tests/Config/fixtures/override/cache.php b/tests/Config/fixtures/override/cache.php new file mode 100644 index 000000000..0ba210b14 --- /dev/null +++ b/tests/Config/fixtures/override/cache.php @@ -0,0 +1,33 @@ + [ + 'swoole' => [ + 'eviction_interval' => 5000, // Override int + ], + 'database' => [ + 'driver' => 'database', + 'connection' => 'pgsql', + 'table' => 'cache', + 'lock_connection' => 'pgsql', + 'lock_table' => 'cache_locks', + 'lock_lottery' => [2, 100], + 'lock_timeout' => 86400, + ], + ], + 'swoole_tables' => [ + 'default' => [ + 'rows' => 2048, // Override int + ], + ], +]; diff --git a/tests/Config/fixtures/override/commands.php b/tests/Config/fixtures/override/commands.php new file mode 100644 index 000000000..de9bfac4a --- /dev/null +++ b/tests/Config/fixtures/override/commands.php @@ -0,0 +1,14 @@ + [ + 'pgsql' => [ + 'host' => 'db.example.com', + 'pool' => [ + 'min_connections' => 1, + 'max_connections' => 10, + 'connect_timeout' => 10.0, + 'wait_timeout' => 3.0, + ], + ], + 'sqlite' => [ + 'driver' => 'sqlite', + 'database' => ':memory:', + 'prefix' => '', + 'foreign_key_constraints' => true, + ], + ], + 'redis' => [ + 'queue' => [ + 'host' => 'redis-queue.example.com', + 'port' => 6379, + 'db' => 1, + ], + ], +]; diff --git a/tests/Config/fixtures/override/listeners.php b/tests/Config/fixtures/override/listeners.php new file mode 100644 index 000000000..0fe833506 --- /dev/null +++ b/tests/Config/fixtures/override/listeners.php @@ -0,0 +1,23 @@ + 99, + 'Hyperf\Signal\Listener\SignalRegisterListener' => PHP_INT_MAX, +];