Skip to content

Commit

Permalink
BUG Fix nesting bug in Kernel
Browse files Browse the repository at this point in the history
  • Loading branch information
Damian Mooyman committed Jun 22, 2017
1 parent 188ce35 commit 9379834
Show file tree
Hide file tree
Showing 3 changed files with 96 additions and 2 deletions.
5 changes: 5 additions & 0 deletions src/Core/Config/ConfigLoader.php
Expand Up @@ -37,6 +37,11 @@ public static function inst()
*/
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");
}
Expand Down
12 changes: 10 additions & 2 deletions src/Core/Injector/InjectorLoader.php
Expand Up @@ -36,6 +36,11 @@ public static function inst()
*/
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");
}
Expand Down Expand Up @@ -87,8 +92,8 @@ public function countManifests()
*/
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;
Expand All @@ -101,9 +106,12 @@ public function nest()

/**
* Mark this instance as the current instance
*
* @return $this
*/
public function activate()
{
static::$instance = $this;
return $this;
}
}
81 changes: 81 additions & 0 deletions tests/php/Core/KernelTest.php
@@ -0,0 +1,81 @@
<?php

namespace SilverStripe\Core\Tests;

use BadMethodCallException;
use SilverStripe\Control\Director;
use SilverStripe\Core\Config\Config;
use SilverStripe\Core\Config\ConfigLoader;
use SilverStripe\Core\Injector\Injector;
use SilverStripe\Core\Injector\InjectorLoader;
use SilverStripe\Core\Kernel;
use SilverStripe\Dev\SapphireTest;

class KernelTest extends SapphireTest
{
public function testNesting()
{
/** @var Kernel $kernel */
$kernel = Injector::inst()->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();
}
}

0 comments on commit 9379834

Please sign in to comment.