From 0f84090c1991f08b3f8365be91942212b2d9f42d Mon Sep 17 00:00:00 2001 From: Loris Leiva Date: Thu, 12 Nov 2020 19:19:23 +0000 Subject: [PATCH 1/7] Register global extenders --- src/Illuminate/Container/Container.php | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/src/Illuminate/Container/Container.php b/src/Illuminate/Container/Container.php index 98d8f0ae7900..ed2521fd4d2e 100755 --- a/src/Illuminate/Container/Container.php +++ b/src/Illuminate/Container/Container.php @@ -70,6 +70,13 @@ class Container implements ArrayAccess, ContainerContract */ protected $extenders = []; + /** + * The global extension closures for services. + * + * @var array[] + */ + protected $globalExtenders = []; + /** * All of the registered tags. * @@ -378,14 +385,26 @@ public function singletonIf($abstract, $concrete = null) /** * "Extend" an abstract type in the container. * - * @param string $abstract - * @param \Closure $closure + * @param string|\Closure $abstract + * @param \Closure|null $closure * @return void * * @throws \InvalidArgumentException */ - public function extend($abstract, Closure $closure) + public function extend($abstract, $closure = null) { + if ($abstract instanceof Closure) { + $closure = $abstract; + + foreach ($this->instances as $abstract => $instance) { + $this->instances[$abstract] = $closure($instance, $this); + $this->rebound($abstract); + } + + $this->globalExtenders[] = $closure; + return; + } + $abstract = $this->getAlias($abstract); if (isset($this->instances[$abstract])) { From 43d2d9b8efe13360059dbacaf059b87bded0b7c9 Mon Sep 17 00:00:00 2001 From: Loris Leiva Date: Thu, 12 Nov 2020 19:27:07 +0000 Subject: [PATCH 2/7] Use global extenders --- src/Illuminate/Container/Container.php | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/src/Illuminate/Container/Container.php b/src/Illuminate/Container/Container.php index ed2521fd4d2e..4724b05592ac 100755 --- a/src/Illuminate/Container/Container.php +++ b/src/Illuminate/Container/Container.php @@ -64,18 +64,18 @@ class Container implements ArrayAccess, ContainerContract protected $abstractAliases = []; /** - * The extension closures for services. + * The global extension closures for services. * * @var array[] */ - protected $extenders = []; + protected $globalExtenders = []; /** - * The global extension closures for services. + * The extension closures for services. * * @var array[] */ - protected $globalExtenders = []; + protected $extenders = []; /** * All of the registered tags. @@ -1192,7 +1192,20 @@ public function getAlias($abstract) */ protected function getExtenders($abstract) { - return $this->extenders[$this->getAlias($abstract)] ?? []; + return array_merge( + $this->globalExtenders, + $this->extenders[$this->getAlias($abstract)] ?? [] + ); + } + + /** + * Remove all of the global extender callbacks. + * + * @return void + */ + public function forgetGlobalExtenders() + { + $this->extenders = []; } /** From 7b953eab102b982ad309ea7236da50d6721281aa Mon Sep 17 00:00:00 2001 From: Loris Leiva Date: Thu, 12 Nov 2020 19:44:36 +0000 Subject: [PATCH 3/7] Add tests --- tests/Container/ContainerExtendTest.php | 69 +++++++++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/tests/Container/ContainerExtendTest.php b/tests/Container/ContainerExtendTest.php index a34c5c6ef9d6..24b03eb4302b 100644 --- a/tests/Container/ContainerExtendTest.php +++ b/tests/Container/ContainerExtendTest.php @@ -187,6 +187,75 @@ public function testUnsetExtend() $this->assertSame('foo', $container->make('foo')); } + + public function testGloballyExtendedBindings() + { + // Given a simple "foo" binding. + $container = new Container; + $container['foo'] = 'foo'; + + // When we append "bar" to all bindings. + $container->extend(function ($old, $container) { + return $old . 'bar'; + }); + + // Then we resolve "foobar". + $this->assertSame('foobar', $container->make('foo')); + } + + public function testGloballyExtendedSingletons() + { + // Given an registered "foo" singleton. + $container = new Container; + $container->singleton('foo', function () { + return (object) ['name' => 'taylor']; + }); + + // When we add the age property to all bindings. + $container->extend(function ($old, $container) { + $old->age = 26; + + return $old; + }); + + // Then the "foo" singleton has the "age" property. + $result = $container->make('foo'); + $this->assertSame('taylor', $result->name); + $this->assertEquals(26, $result->age); + + // And it stays the same instance no matter how many times we resolve it. + $this->assertSame($result, $container->make('foo')); + } + + public function testGloballyExtendedInstancesArePreserved() + { + // Given a "foo" simple binding. + $container = new Container; + $container->bind('foo', function () { + return (object) ['foo' => 'bind']; + }); + + // And a "foo" instance. + $obj = (object) ['foo' => 'instance']; + $container->instance('foo', $obj); + + // When we extend all bindings twice. + $container->extend(function ($obj, $container) { + $obj->bar = 'extended_once'; + + return $obj; + }); + $container->extend(function ($obj, $container) { + $obj->baz = 'extended_twice'; + + return $obj; + }); + + // Then the "foo" instance has been extended twice. + $this->assertSame('instance', $container->make('foo')->foo); + $this->assertSame('extended_once', $container->make('foo')->bar); + $this->assertSame('extended_twice', $container->make('foo')->baz); + } } class ContainerLazyExtendStub From 2bbd9d4746fdd4f383052e593576967db7706a84 Mon Sep 17 00:00:00 2001 From: Loris Leiva Date: Thu, 12 Nov 2020 20:20:31 +0000 Subject: [PATCH 4/7] Add tests and trigger rebinding callbacks for resolved array too --- src/Illuminate/Container/Container.php | 30 ++++++--- tests/Container/ContainerExtendTest.php | 90 +++++++++++++++++++++++++ 2 files changed, 112 insertions(+), 8 deletions(-) diff --git a/src/Illuminate/Container/Container.php b/src/Illuminate/Container/Container.php index 4724b05592ac..8c3b26bfb0af 100755 --- a/src/Illuminate/Container/Container.php +++ b/src/Illuminate/Container/Container.php @@ -394,14 +394,7 @@ public function singletonIf($abstract, $concrete = null) public function extend($abstract, $closure = null) { if ($abstract instanceof Closure) { - $closure = $abstract; - - foreach ($this->instances as $abstract => $instance) { - $this->instances[$abstract] = $closure($instance, $this); - $this->rebound($abstract); - } - - $this->globalExtenders[] = $closure; + $this->addGlobalExtender($abstract); return; } @@ -419,6 +412,27 @@ public function extend($abstract, $closure = null) } } } + /** + * Add a global extender callback to the container. + * + * @param \Closure $closure + * @return void + * + * @throws \InvalidArgumentException + */ + protected function addGlobalExtender(Closure $closure) + { + foreach ($this->instances as $abstract => $instance) { + $this->instances[$abstract] = $closure($instance, $this); + $this->rebound($abstract); + } + + foreach (array_keys($this->resolved) as $abstract) { + $this->rebound($abstract); + } + + $this->globalExtenders[] = $closure; + } /** * Register an existing instance as shared in the container. diff --git a/tests/Container/ContainerExtendTest.php b/tests/Container/ContainerExtendTest.php index 24b03eb4302b..6b788d9e5f36 100644 --- a/tests/Container/ContainerExtendTest.php +++ b/tests/Container/ContainerExtendTest.php @@ -256,6 +256,96 @@ public function testGloballyExtendedInstancesArePreserved() $this->assertSame('extended_once', $container->make('foo')->bar); $this->assertSame('extended_twice', $container->make('foo')->baz); } + + public function testGlobalExtendersAreLazyInitialized() + { + // Given an uninitialized class. + ContainerLazyExtendStub::$initialized = false; + + // And a simple binding of that class. + $container = new Container; + $container->bind(ContainerLazyExtendStub::class); + + // When we add a global extender that initializes that class. + $container->extend(function ($obj, $container) { + $obj->init(); + + return $obj; + }); + + // Then the class is still not initialized. + $this->assertFalse(ContainerLazyExtendStub::$initialized); + + // but will be initialized when resolved from the container. + $container->make(ContainerLazyExtendStub::class); + $this->assertTrue(ContainerLazyExtendStub::$initialized); + } + + public function testGlobalExtendersCanBeCalledBeforeBind() + { + // Given a registered global extenders that appends "bar" to all bindings. + $container = new Container; + $container->extend(function ($old, $container) { + return $old.'bar'; + }); + + // When we, later on, bind "foo" to the container. + $container['foo'] = 'foo'; + + // Then it gets resolved to "foobar". + $this->assertSame('foobar', $container->make('foo')); + } + + public function testGlobalExtendersTriggerInstanceRebindingCallbacks() + { + // Given an indicator that the instance has no rebound. + $testRebound = false; + + // And a rebinding callback that toggles that indicator to true. + $container = new Container; + $container->rebinding('foo', function () use (&$testRebound) { + $testRebound = true; + }); + + // And a "foo" instance already bound. + $obj = new stdClass; + $container->instance('foo', $obj); + + // When we register a global extender. + $container->extend(function ($obj, $container) { + return $obj; + }); + + // Then the rebinding callback has been called. + $this->assertTrue($testRebound); + } + + public function testGlobalExtendersTriggerBindRebindingCallback() + { + // Given an indicator that the instance has no rebound. + $testRebound = false; + + // And a rebinding callback that toggles that indicator to true. + $container = new Container; + $container->rebinding('foo', function () use (&$testRebound) { + $testRebound = true; + }); + + // And an existing "foo" binding that has been resolved once. + $container->bind('foo', function () { + return new stdClass; + }); + $container->make('foo'); + $this->assertFalse($testRebound); + + // When we register a global extender. + $container->extend(function ($obj, $container) { + return $obj; + }); + + // Then the rebinding callback has been called. + $this->assertTrue($testRebound); + } } class ContainerLazyExtendStub From c443ab4b8acb74d37e77424b63c5b9d3af96e732 Mon Sep 17 00:00:00 2001 From: Loris Leiva Date: Thu, 12 Nov 2020 20:35:34 +0000 Subject: [PATCH 5/7] Add tests --- src/Illuminate/Container/Container.php | 2 +- tests/Container/ContainerExtendTest.php | 57 +++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 1 deletion(-) diff --git a/src/Illuminate/Container/Container.php b/src/Illuminate/Container/Container.php index 8c3b26bfb0af..5223dfab8537 100755 --- a/src/Illuminate/Container/Container.php +++ b/src/Illuminate/Container/Container.php @@ -1219,7 +1219,7 @@ protected function getExtenders($abstract) */ public function forgetGlobalExtenders() { - $this->extenders = []; + $this->globalExtenders = []; } /** diff --git a/tests/Container/ContainerExtendTest.php b/tests/Container/ContainerExtendTest.php index 6b788d9e5f36..b1ae6402858b 100644 --- a/tests/Container/ContainerExtendTest.php +++ b/tests/Container/ContainerExtendTest.php @@ -346,6 +346,63 @@ public function testGlobalExtendersTriggerBindRebindingCallback() // Then the rebinding callback has been called. $this->assertTrue($testRebound); } + + public function testGlobalExtensionWorksOnAliasedBindings() + { + // Given a singleton with an alias. + $container = new Container; + $container->singleton('something', function () { + return 'some value'; + }); + $container->alias('something', 'something-alias'); + + // When we register a global extender. + $container->extend(function ($value) { + return $value.' extended'; + }); + + // Then both the singleton and its alias appear to be extended. + $this->assertSame('some value extended', $container->make('something')); + $this->assertSame('some value extended', $container->make('something-alias')); + } + + public function testMultipleGlobalExtenders() + { + // Given a simple binding. + $container = new Container; + $container['foo'] = 'foo'; + + // When we register two global extenders. + $container->extend(function ($old, $container) { + return $old.'bar'; + }); + $container->extend(function ($old, $container) { + return $old.'baz'; + }); + + // Then the binding has been extended by both global extenders. + $this->assertSame('foobarbaz', $container->make('foo')); + } + + public function testForgetGlobalExtenders() + { + // Given a simple binding. + $container = new Container; + $container->bind('foo', function () { + return 'foo'; + }); + + // And a global extender that appends "bar" to all bindings. + $container->extend(function ($obj, $container) { + return $obj . 'bar'; + }); + + // When we forget all global extenders. + $container->forgetGlobalExtenders(); + + // Then the global extender is not applied when we next resolve that binding. + $this->assertSame('foo', $container->make('foo')); + } } class ContainerLazyExtendStub From 14be73c0df3eba40dc7e40f843e7256e4fcb5240 Mon Sep 17 00:00:00 2001 From: Loris Leiva Date: Thu, 12 Nov 2020 21:44:50 +0000 Subject: [PATCH 6/7] Fix style --- src/Illuminate/Container/Container.php | 2 ++ tests/Container/ContainerExtendTest.php | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/Illuminate/Container/Container.php b/src/Illuminate/Container/Container.php index 5223dfab8537..b85d2bcf0ce9 100755 --- a/src/Illuminate/Container/Container.php +++ b/src/Illuminate/Container/Container.php @@ -395,6 +395,7 @@ public function extend($abstract, $closure = null) { if ($abstract instanceof Closure) { $this->addGlobalExtender($abstract); + return; } @@ -412,6 +413,7 @@ public function extend($abstract, $closure = null) } } } + /** * Add a global extender callback to the container. * diff --git a/tests/Container/ContainerExtendTest.php b/tests/Container/ContainerExtendTest.php index b1ae6402858b..d3b7845cc08c 100644 --- a/tests/Container/ContainerExtendTest.php +++ b/tests/Container/ContainerExtendTest.php @@ -196,7 +196,7 @@ public function testGloballyExtendedBindings() // When we append "bar" to all bindings. $container->extend(function ($old, $container) { - return $old . 'bar'; + return $old.'bar'; }); // Then we resolve "foobar". @@ -394,7 +394,7 @@ public function testForgetGlobalExtenders() // And a global extender that appends "bar" to all bindings. $container->extend(function ($obj, $container) { - return $obj . 'bar'; + return $obj.'bar'; }); // When we forget all global extenders. From 0439bdce6fbab1f570a648dfa4981b88c91bed2b Mon Sep 17 00:00:00 2001 From: Loris Leiva Date: Sun, 15 Nov 2020 16:01:42 +0000 Subject: [PATCH 7/7] Global extenders should not rebind anything that has already been resolved --- src/Illuminate/Container/Container.php | 26 ++------------- tests/Container/ContainerExtendTest.php | 43 +++++++++---------------- 2 files changed, 17 insertions(+), 52 deletions(-) diff --git a/src/Illuminate/Container/Container.php b/src/Illuminate/Container/Container.php index b85d2bcf0ce9..8e3b5449958b 100755 --- a/src/Illuminate/Container/Container.php +++ b/src/Illuminate/Container/Container.php @@ -391,10 +391,10 @@ public function singletonIf($abstract, $concrete = null) * * @throws \InvalidArgumentException */ - public function extend($abstract, $closure = null) + public function extend($abstract, Closure $closure = null) { if ($abstract instanceof Closure) { - $this->addGlobalExtender($abstract); + $this->globalExtenders[] = $abstract; return; } @@ -414,28 +414,6 @@ public function extend($abstract, $closure = null) } } - /** - * Add a global extender callback to the container. - * - * @param \Closure $closure - * @return void - * - * @throws \InvalidArgumentException - */ - protected function addGlobalExtender(Closure $closure) - { - foreach ($this->instances as $abstract => $instance) { - $this->instances[$abstract] = $closure($instance, $this); - $this->rebound($abstract); - } - - foreach (array_keys($this->resolved) as $abstract) { - $this->rebound($abstract); - } - - $this->globalExtenders[] = $closure; - } - /** * Register an existing instance as shared in the container. * diff --git a/tests/Container/ContainerExtendTest.php b/tests/Container/ContainerExtendTest.php index d3b7845cc08c..f2c5353cb022 100644 --- a/tests/Container/ContainerExtendTest.php +++ b/tests/Container/ContainerExtendTest.php @@ -227,34 +227,21 @@ public function testGloballyExtendedSingletons() $this->assertSame($result, $container->make('foo')); } - public function testGloballyExtendedInstancesArePreserved() + public function testResolvedInstancesAreNotAffectedByNewGlobalExtenders() { - // Given a "foo" simple binding. + // Given an already resolved "foo" instance. $container = new Container; - $container->bind('foo', function () { - return (object) ['foo' => 'bind']; - }); - - // And a "foo" instance. - $obj = (object) ['foo' => 'instance']; - $container->instance('foo', $obj); - - // When we extend all bindings twice. - $container->extend(function ($obj, $container) { - $obj->bar = 'extended_once'; + $container->instance('foo', (object) ['foo' => 'original']); - return $obj; - }); + // When we add a new global extender. $container->extend(function ($obj, $container) { - $obj->baz = 'extended_twice'; + $obj->foo = 'extended'; return $obj; }); - // Then the "foo" instance has been extended twice. - $this->assertSame('instance', $container->make('foo')->foo); - $this->assertSame('extended_once', $container->make('foo')->bar); - $this->assertSame('extended_twice', $container->make('foo')->baz); + // Then the "foo" instance was not extended. + $this->assertSame('original', $container->make('foo')->foo); } public function testGlobalExtendersAreLazyInitialized() @@ -296,7 +283,7 @@ public function testGlobalExtendersCanBeCalledBeforeBind() $this->assertSame('foobar', $container->make('foo')); } - public function testGlobalExtendersTriggerInstanceRebindingCallbacks() + public function testGlobalExtendersDoNotTriggerInstanceRebindingCallbacks() { // Given an indicator that the instance has no rebound. $testRebound = false; @@ -311,16 +298,16 @@ public function testGlobalExtendersTriggerInstanceRebindingCallbacks() $obj = new stdClass; $container->instance('foo', $obj); - // When we register a global extender. + // When we register a new global extender. $container->extend(function ($obj, $container) { return $obj; }); - // Then the rebinding callback has been called. - $this->assertTrue($testRebound); + // Then the rebinding callback was not called. + $this->assertFalse($testRebound); } - public function testGlobalExtendersTriggerBindRebindingCallback() + public function testGlobalExtendersDoNotTriggerBindRebindingCallback() { // Given an indicator that the instance has no rebound. $testRebound = false; @@ -338,13 +325,13 @@ public function testGlobalExtendersTriggerBindRebindingCallback() $container->make('foo'); $this->assertFalse($testRebound); - // When we register a global extender. + // When we register a new global extender. $container->extend(function ($obj, $container) { return $obj; }); - // Then the rebinding callback has been called. - $this->assertTrue($testRebound); + // Then the rebinding callback was not called. + $this->assertFalse($testRebound); } public function testGlobalExtensionWorksOnAliasedBindings()