Skip to content

Commit

Permalink
Fix issue where 'shareInstances' resolve but fail to propagate (#201)
Browse files Browse the repository at this point in the history
* #200 - Expand shareInstances test to replicate propagation issue

* #200 - Fix issue where 'shareInstances' resolve but fail to propagate

Because `$share` is passed by reference to `matchParam()`, and
`matchParam()` removes matching objects from its `$search` array,
instances may be removed from `$share` before it is passed to `expand()`
or `create()`. Depending on the order of constructor parameters and the
relative placement of `shareInstances` dependencies in the object tree,
this may result in multiple instances of these dependencies being
created.

Fixed by passing a copy of the `$share` array to `matchParam()`.
  • Loading branch information
lkrms authored Mar 28, 2022
1 parent 3249d52 commit e04c98d
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 2 deletions.
2 changes: 1 addition & 1 deletion Dice.php
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ private function getParams(\ReflectionMethod $method, array $rule) {
$parameters[] = $match;
}
// Do the same with $share
else if ($share && ($match = $this->matchParam($param, $class, $share)) !== false) {
else if (($copy = $share) && ($match = $this->matchParam($param, $class, $copy)) !== false) {
$parameters[] = $match;
}
// When nothing from $args or $share matches but a class is type hinted, create an instance to use, using a substitution if set
Expand Down
10 changes: 10 additions & 0 deletions tests/ShareInstancesTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,9 @@ public function testShareInstances() {
$this->assertInstanceOf('SharedInstanceTest1', $shareTest->share1);
$this->assertInstanceOf('SharedInstanceTest2', $shareTest->share2);

$this->assertSame($shareTest->shared, $shareTest->share1->shared);
$this->assertSame($shareTest->share1->shared, $shareTest->share2->shared);
$this->assertEquals($shareTest->shared->uniq, $shareTest->share1->shared->uniq);
$this->assertEquals($shareTest->share1->shared->uniq, $shareTest->share2->shared->uniq);

}
Expand All @@ -41,7 +43,9 @@ public function testNamedShareInstances() {
$this->assertInstanceOf('SharedInstanceTest1', $shareTest->share1);
$this->assertInstanceOf('SharedInstanceTest2', $shareTest->share2);

$this->assertSame($shareTest->shared, $shareTest->share1->shared);
$this->assertSame($shareTest->share1->shared, $shareTest->share2->shared);
$this->assertEquals($shareTest->shared->uniq, $shareTest->share1->shared->uniq);
$this->assertEquals($shareTest->share1->shared->uniq, $shareTest->share2->shared->uniq);


Expand Down Expand Up @@ -72,15 +76,21 @@ public function testShareInstancesMultiple() {
$this->assertInstanceOf('SharedInstanceTest1', $shareTest->share1);
$this->assertInstanceOf('SharedInstanceTest2', $shareTest->share2);

$this->assertSame($shareTest->shared, $shareTest->share1->shared);
$this->assertSame($shareTest->share1->shared, $shareTest->share2->shared);
$this->assertEquals($shareTest->shared->uniq, $shareTest->share1->shared->uniq);
$this->assertEquals($shareTest->share1->shared->uniq, $shareTest->share2->shared->uniq);


$shareTest2 = $dice->create('TestSharedInstancesTop');
$this->assertSame($shareTest2->shared, $shareTest2->share1->shared);
$this->assertSame($shareTest2->share1->shared, $shareTest2->share2->shared);
$this->assertEquals($shareTest2->shared->uniq, $shareTest2->share1->shared->uniq);
$this->assertEquals($shareTest2->share1->shared->uniq, $shareTest2->share2->shared->uniq);

$this->assertNotSame($shareTest->shared, $shareTest2->shared);
$this->assertNotSame($shareTest->share1->shared, $shareTest2->share2->shared);
$this->assertNotEquals($shareTest->shared->uniq, $shareTest2->shared->uniq);
$this->assertNotEquals($shareTest->share1->shared->uniq, $shareTest2->share2->shared->uniq);

}
Expand Down
4 changes: 3 additions & 1 deletion tests/TestData/ShareInstances.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,12 @@
* @license http:// www.opensource.org/licenses/bsd-license.php BSD License *
* @version 3.0 */
class TestSharedInstancesTop {
public $shared;
public $share1;
public $share2;

public function __construct(SharedInstanceTest1 $share1, SharedInstanceTest2 $share2) {
public function __construct(Shared $shared, SharedInstanceTest1 $share1, SharedInstanceTest2 $share2) {
$this->shared = $shared;
$this->share1 = $share1;
$this->share2 = $share2;
}
Expand Down

0 comments on commit e04c98d

Please sign in to comment.