Browse Source

Consolidate duplicated code and fix a few issues.

* Fixup the message to be more generic.
* Exclude 'enabled' from config comparison. Components are often loaded
  by other components with enabled=false which should not cause errors.
* Fix loggers being rebuilt each time a config is defined and used.
Mark Story 11 years ago
parent
commit
a44fb76201

+ 1 - 12
src/Controller/Controller.php

@@ -30,7 +30,6 @@ use Cake\Utility\Inflector;
 use Cake\Utility\MergeVariablesTrait;
 use Cake\View\ViewVarsTrait;
 use LogicException;
-use RuntimeException;
 
 /**
  * Application controller class for organization of business logic.
@@ -348,17 +347,7 @@ class Controller implements EventListener {
  */
 	public function loadComponent($name, array $config = []) {
 		list(, $prop) = pluginSplit($name);
-		$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);
+		$this->{$prop} = $this->components()->load($name, $config);
 		return $this->{$prop};
 	}
 

+ 43 - 1
src/Core/ObjectRegistry.php

@@ -14,6 +14,8 @@
  */
 namespace Cake\Core;
 
+use RuntimeException;
+
 /**
  * Acts as a registry/factory for objects.
  *
@@ -65,9 +67,14 @@ abstract class ObjectRegistry {
  */
 	public function load($objectName, $config = []) {
 		list($plugin, $name) = pluginSplit($objectName);
-		if (isset($this->_loaded[$name])) {
+		$loaded = isset($this->_loaded[$name]);
+		if ($loaded && !empty($config)) {
+			$this->_checkDuplicate($name, $config);
+		}
+		if ($loaded) {
 			return $this->_loaded[$name];
 		}
+
 		if (is_array($config) && isset($config['className'])) {
 			$objectName = $config['className'];
 		}
@@ -82,6 +89,41 @@ abstract class ObjectRegistry {
 	}
 
 /**
+ * Check for duplicate object loading.
+ *
+ * If a duplicate is being loaded and has different configuration, that is
+ * bad and an exception will be raised.
+ *
+ * An exception is raised, as replacing the object will not update any
+ * references other objects may have. Additionally, simply updating the runtime
+ * configuration is not a good option as we may be missing important constructor
+ * logic dependent on the configuration.
+ *
+ * @param string $name The name of the alias in the registry.
+ * @param array $config The config data for the new instance.
+ * @return void
+ * @throws \RuntimeException When a duplicate is found.
+ */
+	protected function _checkDuplicate($name, $config) {
+		$existing = $this->_loaded[$name];
+		$msg = sprintf('The "%s" alias has already been loaded', $name);
+		$hasConfig = false;
+		if (method_exists($existing, 'config')) {
+			$hasConfig = true;
+		}
+		if (!$hasConfig) {
+			throw new RuntimeException($msg);
+		}
+		unset($config['enabled']);
+		if ($hasConfig && array_diff_assoc($config, $existing->config()) != []) {
+			$msg .= ' with the following config: ';
+			$msg .= var_export($this->{$name}->config(), true);
+			$msg .= ' which differs from ' . var_export($config, true);
+			throw new RuntimeException($msg);
+		}
+	}
+
+/**
  * Should resolve the classname for a given object type.
  *
  * @param string $class The class to resolve.

+ 1 - 1
src/Core/StaticConfigTrait.php

@@ -107,7 +107,7 @@ trait StaticConfigTrait {
 		if (!isset(static::$_config[$config])) {
 			return false;
 		}
-		if (isset(static::$_registry) && isset(static::$_registry->{$config})) {
+		if (isset(static::$_registry)) {
 			static::$_registry->unload($config);
 		}
 		unset(static::$_config[$config]);

+ 3 - 1
src/Log/Log.php

@@ -180,7 +180,9 @@ class Log {
 			if (isset($properties['engine'])) {
 				$properties['className'] = $properties['engine'];
 			}
-			static::$_registry->load($name, $properties);
+			if (!static::$_registry->loaded($name)) {
+				static::$_registry->load($name, $properties);
+			}
 		}
 	}
 

+ 1 - 10
src/ORM/Table.php

@@ -503,16 +503,7 @@ class Table implements RepositoryInterface, EventListener {
  * @see \Cake\ORM\Behavior
  */
 	public function addBehavior($name, array $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);
+		$this->_behaviors->load($name, $options);
 	}
 
 /**

+ 0 - 9
src/View/View.php

@@ -29,7 +29,6 @@ use Cake\View\CellTrait;
 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
@@ -780,14 +779,6 @@ class View {
 	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);
 	}
 

+ 1 - 1
tests/TestCase/Controller/ControllerTest.php

@@ -841,7 +841,7 @@ class ControllerTest extends TestCase {
 			$controller->loadComponent('Paginator', ['bad' => 'settings']);
 			$this->fail('No exception');
 		} catch (\RuntimeException $e) {
-			$this->assertContains('The "Paginator" component has already been loaded', $e->getMessage());
+			$this->assertContains('The "Paginator" alias has already been loaded', $e->getMessage());
 		}
 	}
 

+ 2 - 2
tests/TestCase/ORM/TableTest.php

@@ -1079,10 +1079,10 @@ class TableTest extends \Cake\TestSuite\TestCase {
 		$this->assertNull($table->addBehavior('Sluggable', ['test' => 'value']));
 		$this->assertNull($table->addBehavior('Sluggable', ['test' => 'value']));
 		try {
-			$table->addBehavior('Sluggable');
+			$table->addBehavior('Sluggable', ['thing' => 'thing']);
 			$this->fail('No exception raised');
 		} catch (\RuntimeException $e) {
-			$this->assertContains('The "Sluggable" behavior has already been loaded', $e->getMessage());
+			$this->assertContains('The "Sluggable" alias has already been loaded', $e->getMessage());
 		}
 	}
 

+ 1 - 1
tests/TestCase/View/ViewTest.php

@@ -933,7 +933,7 @@ class ViewTest extends TestCase {
 			$View->loadHelper('Html', ['test' => 'value']);
 			$this->fail('No exception');
 		} catch (\RuntimeException $e) {
-			$this->assertContains('The "Html" helper has already been loaded', $e->getMessage());
+			$this->assertContains('The "Html" alias has already been loaded', $e->getMessage());
 		}
 	}