Skip to content

Commit

Permalink
Include plugin bootstrap on the first time.
Browse files Browse the repository at this point in the history
If a plugin is loaded through the static method it should be immediately
bootstrapped, as that is how Plugin::load() has worked for a long time.
Because many 3.x plugins rely on their bootstrap file only being executed
once per process, we need to disable the bootstrap hook so that
`app->pluginBootstrap()` doesn't re-trigger the bootstrap steps.

Refs #11816
  • Loading branch information
markstory committed Mar 16, 2018
1 parent 8ffd39a commit fa036c6
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 14 deletions.
8 changes: 4 additions & 4 deletions src/Core/Plugin.php
Expand Up @@ -174,10 +174,7 @@ public static function load($plugin, array $config = [])
);
}

// Defer the bootstrap process if an Application class exists.
// Immediate plugin bootstrapping is deprecated and will be removed in 4.x
$appClass = Configure::read('App.namespace') . '\\' . 'Application';
if ($config['bootstrap'] === true && !class_exists($appClass)) {
if ($config['bootstrap'] === true) {
static::bootstrap($plugin);
}
}
Expand Down Expand Up @@ -316,6 +313,9 @@ public static function bootstrap($name)
if (!$plugin->isEnabled('bootstrap')) {
return false;
}
// Disable bootstrapping for this plugin as it will have
// been bootstrapped.
$plugin->disable('bootstrap');

return static::_includeFile(
$plugin->getConfigPath() . 'bootstrap.php',
Expand Down
20 changes: 10 additions & 10 deletions tests/TestCase/Core/PluginTest.php
Expand Up @@ -39,7 +39,7 @@ public function tearDown()
*
* @return void
*/
public function testLoadSingle()
public function testLoad()
{
Plugin::unload();
Plugin::load('TestPlugin');
Expand Down Expand Up @@ -74,7 +74,7 @@ public function testUnload()
*
* @return void
*/
public function testLoadSingleWithAutoload()
public function testLoadWithAutoload()
{
$this->assertFalse(class_exists('Company\TestPluginFive\Utility\Hello'));
Plugin::load('Company/TestPluginFive', [
Expand All @@ -91,7 +91,7 @@ class_exists('Company\TestPluginFive\Utility\Hello'),
*
* @return void
*/
public function testLoadSingleWithAutoloadAndBootstrap()
public function testLoadWithAutoloadAndBootstrap()
{
Plugin::load(
'Company/TestPluginFive',
Expand All @@ -111,10 +111,9 @@ class_exists('Company\TestPluginFive\Utility\Hello'),
/**
* Tests loading a plugin and its bootstrap file
*
* @deprecated Immediate plugin bootstrap should be removed in 4.x
* @return void
*/
public function testLoadSingleWithBootstrap()
public function testLoadWithBootstrap()
{
Plugin::load('TestPlugin', ['bootstrap' => true]);
$this->assertTrue(Plugin::loaded('TestPlugin'));
Expand All @@ -126,17 +125,18 @@ public function testLoadSingleWithBootstrap()
}

/**
* Tests loading a plugin defers bootstrap when an application exists
* Tests loading a plugin and its bootstrap file
*
* @return void
*/
public function testLoadSingleDeferBootstrap()
public function testLoadWithBootstrapDisableBootstrapHook()
{
static::setAppNamespace('TestApp');

Plugin::load('TestPlugin', ['bootstrap' => true]);
$this->assertTrue(Plugin::loaded('TestPlugin'));
$this->assertNull(Configure::read('PluginTest.test_plugin.bootstrap'), 'Normally this value would be loaded');
$this->assertEquals('loaded plugin bootstrap', Configure::read('PluginTest.test_plugin.bootstrap'));

$plugin = Plugin::getCollection()->get('TestPlugin');
$this->assertFalse($plugin->isEnabled('bootstrap'), 'Should be disabled as hook has been run.');
}

/**
Expand Down
23 changes: 23 additions & 0 deletions tests/TestCase/Http/BaseApplicationTest.php
Expand Up @@ -159,4 +159,27 @@ public function testPluginBootstrap()
$this->assertNull($app->pluginBootstrap());
$this->assertTrue(Configure::check('PluginTest.test_plugin.bootstrap'));
}

/**
* Ensure that plugins loaded via Plugin::load()
* don't have their bootstrapping run twice.
*
* @return void
*/
public function testPluginBootstrapInteractWithPluginLoad()
{
Plugin::load('TestPlugin', ['bootstrap' => true]);
$app = $this->getMockForAbstractClass(
BaseApplication::class,
[$this->path]
);
$this->assertTrue(Configure::check('PluginTest.test_plugin.bootstrap'));
Configure::delete('PluginTest.test_plugin.bootstrap');

$this->assertNull($app->pluginBootstrap());
$this->assertFalse(
Configure::check('PluginTest.test_plugin.bootstrap'),
'Key should not be set, as plugin has already had bootstrap run'
);
}
}

0 comments on commit fa036c6

Please sign in to comment.