Skip to content

Commit

Permalink
Raise an exception when reconfiguring objects that already exist.
Browse files Browse the repository at this point in the history
Instead of silently discarding the reconfiugration method call, an
exception should be raised so the developer knows that they have
probably made a mistake somewhere.

This behavior is applied to helpers/components/behaviors but not other
registry types as those are less frequently reconfigured.

Refs #4693
  • Loading branch information
markstory committed Sep 27, 2014
1 parent 7ad3186 commit 0e4c50e
Show file tree
Hide file tree
Showing 6 changed files with 94 additions and 7 deletions.
13 changes: 12 additions & 1 deletion src/Controller/Controller.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
use Cake\Utility\MergeVariablesTrait;
use Cake\View\ViewVarsTrait;
use LogicException;
use RuntimeException;

/**
* Application controller class for organization of business logic.
Expand Down Expand Up @@ -347,7 +348,17 @@ public function addComponent($name, array $config = []) {
*/
public function loadComponent($name, array $config = []) {
list(, $prop) = pluginSplit($name);
$this->{$prop} = $this->components()->load($name, $config);
$components = $this->components();

if (isset($components->$name) && $config && $components->$name->config() !== $config) {
$msg = sprintf(
'The "%s" component has already been loaded with the following config: %s',
$name,
var_export($components->{$name}->config(), true)
);
throw new RuntimeException($msg);
}
$this->{$prop} = $components->load($name, $config);
return $this->{$prop};
}

Expand Down
13 changes: 12 additions & 1 deletion src/ORM/Table.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
use Cake\ORM\Marshaller;
use Cake\Utility\Inflector;
use Cake\Validation\Validator;
use RuntimeException;

/**
* Represents a single database table.
Expand Down Expand Up @@ -498,10 +499,20 @@ public function entityClass($name = null) {
* @param string $name The name of the behavior. Can be a short class reference.
* @param array $options The options for the behavior to use.
* @return void
* @throws \RuntimeException If a behavior is being reloaded.
* @see \Cake\ORM\Behavior
*/
public function addBehavior($name, array $options = []) {
$this->_behaviors->load($name, $options);
$behaviors = $this->_behaviors;
if (isset($behaviors->$name) && $behaviors->{$name}->config() !== $options) {
$msg = sprintf(
'The "%s" behavior has already been loaded with the following config: %s',
$name,
var_export($behaviors->{$name}->config(), true)
);
throw new RuntimeException($msg);
}
$behaviors->load($name, $options);
}

/**
Expand Down
18 changes: 14 additions & 4 deletions src/View/View.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
use Cake\View\ViewVarsTrait;
use InvalidArgumentException;
use LogicException;
use RuntimeException;

/**
* View, the V in the MVC triad. View interacts with Helpers and view variables passed
Expand Down Expand Up @@ -771,14 +772,23 @@ public function addHelper($helperName, array $config = []) {
/**
* Loads a helper. Delegates to the `HelperRegistry::load()` to load the helper
*
* @param string $helperName Name of the helper to load.
* @param string $name Name of the helper to load.
* @param array $config Settings for the helper
* @return Helper a constructed helper object.
* @see HelperRegistry::load()
*/
public function loadHelper($helperName, array $config = []) {
list(, $class) = pluginSplit($helperName);
return $this->{$class} = $this->helpers()->load($helperName, $config);
public function loadHelper($name, array $config = []) {
list(, $class) = pluginSplit($name);
$helpers = $this->helpers();
if ($helpers->loaded($class) && $config && $helpers->$class->config() !== $config) {
$msg = sprintf(
'The "%s" helper has already been loaded with the following config: %s',
$name,
var_export($helpers->{$class}->config(), true)
);
throw new RuntimeException($msg);
}
return $this->{$class} = $helpers->load($name, $config);
}

/**
Expand Down
23 changes: 22 additions & 1 deletion tests/TestCase/Controller/ControllerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -812,7 +812,7 @@ public function testComponents() {
*
* @return void
*/
public function testAddComponent() {
public function testLoadComponent() {
$request = new Request('/');
$response = $this->getMock('Cake\Network\Response');

Expand All @@ -825,4 +825,25 @@ public function testAddComponent() {
$this->assertTrue(isset($registry->Paginator));
}

/**
* Test adding a component that is a duplicate.
*
* @return void
*/
public function testLoadComponentDuplicate() {
$request = new Request('/');
$response = $this->getMock('Cake\Network\Response');

$controller = new TestController($request, $response);
$this->assertNotEmpty($controller->loadComponent('Paginator'));
$this->assertNotEmpty($controller->loadComponent('Paginator'));
try {
$controller->loadComponent('Paginator', ['bad' => 'settings']);
$this->fail('No exception');
} catch (\RuntimeException $e) {
$this->assertContains('The "Paginator" component has already been loaded', $e->getMessage());
}
}


}
17 changes: 17 additions & 0 deletions tests/TestCase/ORM/TableTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -1069,6 +1069,23 @@ public function testAddBehavior() {
$table->addBehavior('Sluggable');
}

/**
* Test adding a behavior that is a duplicate.
*
* @return void
*/
public function testAddBehaviorDuplicate() {
$table = new Table(['table' => 'articles']);
$this->assertNull($table->addBehavior('Sluggable', ['test' => 'value']));
$this->assertNull($table->addBehavior('Sluggable', ['test' => 'value']));
try {
$table->addBehavior('Sluggable');
$this->fail('No exception raised');
} catch (\RuntimeException $e) {
$this->assertContains('The "Sluggable" behavior has already been loaded', $e->getMessage());
}
}

/**
* Test removing a behavior from a table.
*
Expand Down
17 changes: 17 additions & 0 deletions tests/TestCase/View/ViewTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -920,6 +920,23 @@ public function testLoadHelper() {
$this->assertEquals('bar', $config['foo']);
}

/**
* Test loading helper when duplicate.
*
* @return void
*/
public function testLoadHelperDuplicate() {
$View = new View();

$this->assertNotEmpty($View->loadHelper('Html', ['foo' => 'bar']));
try {
$View->loadHelper('Html', ['test' => 'value']);
$this->fail('No exception');
} catch (\RuntimeException $e) {
$this->assertContains('The "Html" helper has already been loaded', $e->getMessage());
}
}

/**
* Test loadHelpers method
*
Expand Down

0 comments on commit 0e4c50e

Please sign in to comment.