Merge pull request #3763 from markstory/better-missing-controller-errors

3.0 Improve missing controller errors a bit, and fix an issue in Table::get()
cakephp · Jun 21, 2014 · f7fda78 · f7fda78
2 parents 0ef0295 + 08d8ad9
commit f7fda78
Show file tree

Hide file tree

Showing 4 changed files with 48 additions and 51 deletions.
diff --git a/src/ORM/Table.php b/src/ORM/Table.php
@@ -864,27 +864,35 @@ protected function _setFieldMatchers($options, $keys) {
 /**
  * {@inheritDoc}
  *
- * @throws \Cake\ORM\Error\RecordNotFoundException if no record can be found give
- * such primary key value.
+ * @throws \Cake\ORM\Error\RecordNotFoundException if no record can be found given
+ * a primary key value.
+ * @throws \InvalidArgumentException When $primaryKey has an incorrect number of elements.
  */
 	public function get($primaryKey, $options = []) {
 		$key = (array)$this->primaryKey();
 		$alias = $this->alias();
 		foreach ($key as $index => $keyname) {
 			$key[$index] = $alias . '.' . $keyname;
 		}
-		$conditions = array_combine($key, (array)$primaryKey);
-		$entity = $this->find('all', $options)->where($conditions)->first();
-
-		if (!$entity) {
-			throw new RecordNotFoundException(sprintf(
-				'Record "%s" not found in table "%s"',
-				implode(',', (array)$primaryKey),
-				$this->table()
+		$primaryKey = (array)$primaryKey;
+		if (count($key) !== count($primaryKey)) {
+			throw new \InvalidArgumentException(sprintf(
+				"Incorrect number of primary key values. Expected %d got %d.",
+				count($key),
+				count($primaryKey)
 			));
 		}
+		$conditions = array_combine($key, $primaryKey);
+		$entity = $this->find('all', $options)->where($conditions)->first();
 
-		return $entity;
+		if ($entity) {
+			return $entity;
+		}
+		throw new RecordNotFoundException(sprintf(
+			'Record "%s" not found in table "%s"',
+			implode(',', (array)$primaryKey),
+			$this->table()
+		));
 	}
 
 /**

diff --git a/src/Routing/Dispatcher.php b/src/Routing/Dispatcher.php
@@ -26,7 +26,6 @@
 use Cake\Event\EventManagerTrait;
 use Cake\Network\Request;
 use Cake\Network\Response;
-use Cake\Utility\Inflector;
 
 /**
  * Dispatcher converts Requests into controller actions. It uses the dispatched Request
@@ -82,9 +81,9 @@ public function dispatch(Request $request, Response $response) {
 
 		if (!($controller instanceof Controller)) {
 			throw new MissingControllerException(array(
-				'class' => Inflector::camelize($request->params['controller']),
-				'plugin' => empty($request->params['plugin']) ? null : Inflector::camelize($request->params['plugin']),
-				'prefix' => empty($request->params['prefix']) ? null : Inflector::camelize($request->params['prefix']),
+				'class' => $request->params['controller'],
+				'plugin' => empty($request->params['plugin']) ? null : $request->params['plugin'],
+				'prefix' => empty($request->params['prefix']) ? null : $request->params['prefix'],
 				'_ext' => empty($request->params['_ext']) ? null : $request->params['_ext']
 			));
 		}

diff --git a/tests/TestCase/ORM/TableTest.php b/tests/TestCase/ORM/TableTest.php
@@ -3183,39 +3183,29 @@ public function testGet() {
  * @expectedExceptionMessage Record "10" not found in table "articles"
  * @return void
  */
-	public function testGetException() {
-		$table = $this->getMock(
-			'\Cake\ORM\Table',
-			['callFinder', 'query'],
-			[[
-				'connection' => $this->connection,
-				'table' => 'articles',
-				'schema' => [
-					'id' => ['type' => 'integer'],
-					'bar' => ['type' => 'integer'],
-					'_constraints' => ['primary' => ['type' => 'primary', 'columns' => ['bar']]]
-				]
-			]]
-		);
-
-		$query = $this->getMock(
-			'\Cake\ORM\Query',
-			['addDefaultTypes', 'first', 'where'],
-			[$this->connection, $table]
-		);
-
-		$table->expects($this->once())->method('query')
-			->will($this->returnValue($query));
-		$table->expects($this->once())->method('callFinder')
-			->with('all', $query, ['contain' => ['foo']])
-			->will($this->returnValue($query));
+	public function testGetNotFoundException() {
+		$table = new Table([
+			'name' => 'Articles',
+			'connection' => $this->connection,
+			'table' => 'articles',
+		]);
+		$table->get(10);
+	}
 
-		$query->expects($this->once())->method('where')
-			->with([$table->alias() . '.bar' => 10])
-			->will($this->returnSelf());
-		$query->expects($this->once())->method('first')
-			->will($this->returnValue(false));
-		$result = $table->get(10, ['contain' => ['foo']]);
+/**
+ * Test that an exception is raised when there are not enough keys.
+ *
+ * @expectedException \InvalidArgumentException
+ * @expectedExceptionMessage Incorrect number of primary key values. Expected 1 got 0.
+ * @return void
+ */
+	public function testGetExceptionOnIncorrectData() {
+		$table = new Table([
+			'name' => 'Articles',
+			'connection' => $this->connection,
+			'table' => 'articles',
+		]);
+		$table->get(null);
 	}
 
 /**

diff --git a/tests/TestCase/Routing/DispatcherTest.php b/tests/TestCase/Routing/DispatcherTest.php
@@ -244,7 +244,7 @@ public function testMissingController() {
 		$request = new Request([
 			'url' => 'some_controller/home',
 			'params' => [
-				'controller' => 'some_controller',
+				'controller' => 'SomeController',
 				'action' => 'home',
 			]
 		]);
@@ -263,7 +263,7 @@ public function testMissingControllerInterface() {
 		$request = new Request([
 			'url' => 'dispatcher_test_interface/index',
 			'params' => [
-				'controller' => 'dispatcher_test_interface',
+				'controller' => 'DispatcherTestInterface',
 				'action' => 'index',
 			]
 		]);
@@ -283,7 +283,7 @@ public function testMissingControllerAbstract() {
 		$request = new Request([
 			'url' => 'abstract/index',
 			'params' => [
-				'controller' => 'abstract',
+				'controller' => 'Abstract',
 				'action' => 'index',
 			]
 		]);
@@ -300,7 +300,7 @@ public function testDispatchBasic() {
 		$url = new Request([
 			'url' => 'pages/home',
 			'params' => [
-				'controller' => 'pages',
+				'controller' => 'Pages',
 				'action' => 'display',
 				'pass' => ['extract'],
 				'return' => 1