From 9379834cb4b2e5177a2600049feec05bf111c16b Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Thu, 22 Jun 2017 15:50:47 +1200 Subject: [PATCH] BUG Fix nesting bug in Kernel --- src/Core/Config/ConfigLoader.php | 5 ++ src/Core/Injector/InjectorLoader.php | 12 ++++- tests/php/Core/KernelTest.php | 81 ++++++++++++++++++++++++++++ 3 files changed, 96 insertions(+), 2 deletions(-) create mode 100644 tests/php/Core/KernelTest.php diff --git a/src/Core/Config/ConfigLoader.php b/src/Core/Config/ConfigLoader.php index 500a62447..811fc062f 100644 --- a/src/Core/Config/ConfigLoader.php +++ b/src/Core/Config/ConfigLoader.php @@ -37,6 +37,11 @@ class ConfigLoader */ public function getManifest() { + if ($this !== self::$instance) { + throw new BadMethodCallException( + "Non-current config manifest cannot be accessed. Please call ->activate() first" + ); + } if (empty($this->manifests)) { throw new BadMethodCallException("No config manifests available"); } diff --git a/src/Core/Injector/InjectorLoader.php b/src/Core/Injector/InjectorLoader.php index 7bed66e90..f618dccd7 100644 --- a/src/Core/Injector/InjectorLoader.php +++ b/src/Core/Injector/InjectorLoader.php @@ -36,6 +36,11 @@ class InjectorLoader */ public function getManifest() { + if ($this !== self::$instance) { + throw new BadMethodCallException( + "Non-current injector manifest cannot be accessed. Please call ->activate() first" + ); + } if (empty($this->manifests)) { throw new BadMethodCallException("No injector manifests available"); } @@ -87,8 +92,8 @@ class InjectorLoader */ public function nest() { - // Nest config - $manifest = $this->getManifest()->nest(); + // Nest injector (note: Don't call getManifest()->nest() since that self-pushes a new manifest) + $manifest = clone $this->getManifest(); // Create new blank loader with new stack (top level nesting) $newLoader = new static; @@ -101,9 +106,12 @@ class InjectorLoader /** * Mark this instance as the current instance + * + * @return $this */ public function activate() { static::$instance = $this; + return $this; } } diff --git a/tests/php/Core/KernelTest.php b/tests/php/Core/KernelTest.php new file mode 100644 index 000000000..58c98b051 --- /dev/null +++ b/tests/php/Core/KernelTest.php @@ -0,0 +1,81 @@ +get(Kernel::class); + + $nested1 = $kernel->nest(); + Director::config()->set('alternate_base_url', '/mysite/'); + $this->assertEquals($nested1->getConfigLoader(), ConfigLoader::inst()); + $this->assertEquals($nested1->getInjectorLoader(), InjectorLoader::inst()); + $this->assertEquals(1, ConfigLoader::inst()->countManifests()); + $this->assertEquals(1, InjectorLoader::inst()->countManifests()); + + // Re-nest + $nested2 = $nested1->nest(); + + // Nesting config / injector should increase this count + Injector::nest(); + Config::nest(); + $this->assertEquals($nested2->getConfigLoader(), ConfigLoader::inst()); + $this->assertEquals($nested2->getInjectorLoader(), InjectorLoader::inst()); + $this->assertEquals(2, ConfigLoader::inst()->countManifests()); + $this->assertEquals(2, InjectorLoader::inst()->countManifests()); + Director::config()->set('alternate_base_url', '/anothersite/'); + + // Nesting always resets sub-loaders to 1 + $nested2->nest(); + $this->assertEquals(1, ConfigLoader::inst()->countManifests()); + $this->assertEquals(1, InjectorLoader::inst()->countManifests()); + + // Calling ->activate() on a previous kernel restores + $nested1->activate(); + $this->assertEquals($nested1->getConfigLoader(), ConfigLoader::inst()); + $this->assertEquals($nested1->getInjectorLoader(), InjectorLoader::inst()); + $this->assertEquals('/mysite/', Director::config()->get('alternate_base_url')); + $this->assertEquals(1, ConfigLoader::inst()->countManifests()); + $this->assertEquals(1, InjectorLoader::inst()->countManifests()); + } + + public function testInvalidInjectorDetection() + { + $this->expectException(BadMethodCallException::class); + $this->expectExceptionMessage( + "Non-current injector manifest cannot be accessed. Please call ->activate() first" + ); + + /** @var Kernel $kernel */ + $kernel = Injector::inst()->get(Kernel::class); + $kernel->nest(); // $kernel is no longer current kernel + + $kernel->getInjectorLoader()->getManifest(); + } + + public function testInvalidConfigDetection() + { + $this->expectException(BadMethodCallException::class); + $this->expectExceptionMessage( + "Non-current config manifest cannot be accessed. Please call ->activate() first" + ); + + /** @var Kernel $kernel */ + $kernel = Injector::inst()->get(Kernel::class); + $kernel->nest(); // $kernel is no longer current kernel + + $kernel->getConfigLoader()->getManifest(); + } +}