Skip to content

Commit 4b8c7e4

Browse files
committed
refactor: extract shared mergeTwo() to eliminate duplication
- Make ProviderConfig::mergeTwo() public static so ConfigFactory can use it - Remove duplicate mergeTwo() implementation from ConfigFactory (32 lines) - Add test verifying mergeTwo() works as public API - Update docblocks to reflect the shared architecture Both classes now use identical merge semantics from a single source of truth.
1 parent c71cc77 commit 4b8c7e4

File tree

4 files changed

+59
-38
lines changed

4 files changed

+59
-38
lines changed

src/config/src/ConfigFactory.php

Lines changed: 1 addition & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ public function __invoke(ContainerInterface $container)
2626
$allConfigs = [ProviderConfig::load(), $rootConfig, ...$autoloadConfig];
2727
$merged = array_reduce(
2828
array_slice($allConfigs, 1),
29-
[$this, 'mergeTwo'],
29+
ProviderConfig::mergeTwo(...),
3030
$allConfigs[0]
3131
);
3232

@@ -63,37 +63,4 @@ private function readPaths(array $paths, array $excludes = []): array
6363

6464
return $configs;
6565
}
66-
67-
/**
68-
* Merge two config arrays.
69-
*
70-
* Correctly handles:
71-
* - Pure lists (numeric keys): appends values with deduplication
72-
* - Associative arrays (string keys): recursively merges, later wins for scalars
73-
* - Mixed arrays (e.g. listeners with priorities): appends numeric, merges string keys
74-
*/
75-
private function mergeTwo(array $base, array $override): array
76-
{
77-
$result = $base;
78-
79-
foreach ($override as $key => $value) {
80-
if (is_int($key)) {
81-
// Numeric key - append if not already present (deduplicate)
82-
if (! in_array($value, $result, true)) {
83-
$result[] = $value;
84-
}
85-
} elseif (! array_key_exists($key, $result)) {
86-
// New string key - just add it
87-
$result[$key] = $value;
88-
} elseif (is_array($value) && is_array($result[$key])) {
89-
// Both are arrays - recursively merge
90-
$result[$key] = $this->mergeTwo($result[$key], $value);
91-
} else {
92-
// Scalar or mixed types - override wins
93-
$result[$key] = $value;
94-
}
95-
}
96-
97-
return $result;
98-
}
9966
}

src/config/src/ProviderConfig.php

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -132,8 +132,17 @@ protected static function merge(...$arrays): array
132132

133133
/**
134134
* Merge two config arrays.
135+
*
136+
* Correctly handles:
137+
* - Pure lists (numeric keys): appends values with deduplication
138+
* - Associative arrays (string keys): recursively merges, later wins for scalars
139+
* - Mixed arrays (e.g. listeners with priorities): appends numeric, merges string keys
140+
*
141+
* This method is public so ConfigFactory can use the same merge semantics.
142+
*
143+
* @return array<string, mixed>
135144
*/
136-
private static function mergeTwo(array $base, array $override): array
145+
public static function mergeTwo(array $base, array $override): array
137146
{
138147
$result = $base;
139148

tests/Config/ConfigFactoryTest.php

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -177,10 +177,11 @@ public function testMergeMultipleConfigArrays(): void
177177
}
178178

179179
/**
180-
* Use ProviderConfig::merge() since ConfigFactory uses the same merge logic.
180+
* Simulate ConfigFactory's merge behavior using ProviderConfig::merge().
181181
*
182-
* Both ConfigFactory and ProviderConfig need identical merge semantics,
183-
* so we test the shared behavior via ProviderConfig's merge method.
182+
* ConfigFactory uses ProviderConfig::mergeTwo() directly for merging.
183+
* We test via ProviderConfig::merge() which uses the same mergeTwo() method
184+
* internally, ensuring identical merge semantics.
184185
*/
185186
private function mergeConfigs(array ...$configs): array
186187
{

tests/Config/ProviderConfigTest.php

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -710,6 +710,50 @@ public function testMergeMixedDependencies(): void
710710
$this->assertSame('SmtpMailer', $result['dependencies']['MailerInterface']);
711711
}
712712

713+
/**
714+
* Test that mergeTwo() is callable as a public static method.
715+
*
716+
* This method is public so ConfigFactory can use the same merge semantics
717+
* without duplicating the implementation.
718+
*/
719+
public function testMergeTwoIsPublicAndWorksDirectly(): void
720+
{
721+
$base = [
722+
'commands' => ['CommandA', 'CommandB'],
723+
'database' => [
724+
'default' => 'sqlite',
725+
'connections' => [
726+
'sqlite' => ['driver' => 'sqlite'],
727+
],
728+
],
729+
];
730+
731+
$override = [
732+
'commands' => ['CommandC'],
733+
'database' => [
734+
'default' => 'pgsql',
735+
'connections' => [
736+
'pgsql' => ['driver' => 'pgsql', 'host' => 'localhost'],
737+
],
738+
],
739+
];
740+
741+
// Call directly without reflection - this verifies it's public
742+
$result = ProviderConfig::mergeTwo($base, $override);
743+
744+
// Numeric arrays are combined
745+
$this->assertSame(['CommandA', 'CommandB', 'CommandC'], $result['commands']);
746+
747+
// Scalar values are overridden
748+
$this->assertSame('pgsql', $result['database']['default']);
749+
750+
// Nested arrays are merged recursively
751+
$this->assertArrayHasKey('sqlite', $result['database']['connections']);
752+
$this->assertArrayHasKey('pgsql', $result['database']['connections']);
753+
$this->assertSame('sqlite', $result['database']['connections']['sqlite']['driver']);
754+
$this->assertSame('pgsql', $result['database']['connections']['pgsql']['driver']);
755+
}
756+
713757
/**
714758
* Call the protected merge method via reflection.
715759
*/

0 commit comments

Comments
 (0)