Browse Source

Merge pull request #4229 from cakephp/3.0-router-fix

3.0 Fix router parsing and associated deletes
José Lorenzo Rodríguez 11 years ago
parent
commit
bea84ed7a8

+ 1 - 1
src/Database/Query.php

@@ -1276,7 +1276,7 @@ class Query implements ExpressionInterface, IteratorAggregate {
  * Can be combined with from(), where() and other methods to
  * create delete queries with specific conditions.
  *
- * @param string $table The table to use when deleting. This
+ * @param string $table The table to use when deleting.
  * @return $this
  */
 	public function delete($table = null) {

+ 24 - 1
src/Database/QueryCompiler.php

@@ -32,7 +32,6 @@ class QueryCompiler {
  * @var array
  */
 	protected $_templates = [
-		'delete' => 'DELETE',
 		'update' => 'UPDATE %s',
 		'where' => ' WHERE %s',
 		'group' => ' GROUP BY %s ',
@@ -160,6 +159,30 @@ class QueryCompiler {
 	}
 
 /**
+ * Helper function used to build the string representation of a DELETE clause.
+ *
+ * If the table is aliased, a `DELETE x` fragment will be returned
+ *
+ * @param array $parts list of tables to be transformed to string
+ * @param \Cake\Database\Query $query The query that is being compiled
+ * @param \Cake\Database\ValueBinder $generator the placeholder generator to be used in expressions
+ * @return string
+ */
+	protected function _buildDeletePart($parts, $query, $generator) {
+		$delete = 'DELETE';
+		$aliases = [];
+		foreach ($query->clause('from') as $k => $v) {
+			if (is_string($k)) {
+				$aliases[] = $k;
+			}
+		}
+		if (!empty($aliases)) {
+			$delete = trim($delete . ' ' . implode(',', $aliases));
+		}
+		return $delete;
+	}
+
+/**
  * Helper function used to build the string representation of a FROM clause,
  * it constructs the tables list taking care of aliasing and
  * converting expression objects to string.

+ 3 - 2
src/ORM/Query.php

@@ -690,8 +690,9 @@ class Query extends DatabaseQuery {
  * @return $this
  */
 	public function delete($table = null) {
-		$table = $this->repository()->table();
-		return parent::delete($table);
+		$repo = $this->repository();
+		$this->from([$repo->alias() => $repo->table()]);
+		return parent::delete();
 	}
 
 /**

+ 2 - 2
src/ORM/Table.php

@@ -1012,8 +1012,8 @@ class Table implements RepositoryInterface, EventListener {
  * {@inheritDoc}
  */
 	public function deleteAll($conditions) {
-		$query = $this->query();
-		$query->delete()
+		$query = $this->query()
+			->delete()
 			->where($conditions);
 		$statement = $query->execute();
 		$success = $statement->rowCount() > 0;

+ 2 - 0
src/Routing/RouteBuilder.php

@@ -346,6 +346,8 @@ class RouteBuilder {
 			unset($options['routeClass']);
 
 			$route = str_replace('//', '/', $this->_path . $route);
+			$route = $route === '/' ? $route : rtrim($route, '/');
+
 			foreach ($this->_params as $param => $val) {
 				if (isset($defaults[$param]) && $defaults[$param] !== $val) {
 					$msg = 'You cannot define routes that conflict with the scope. ' .

+ 20 - 0
tests/TestCase/Database/QueryTest.php

@@ -1885,6 +1885,26 @@ class QueryTest extends TestCase {
 	}
 
 /**
+ * Test delete with from and alias.
+ *
+ * @return void
+ */
+	public function testDeleteWithAliasedFrom() {
+		$query = new Query($this->connection);
+
+		$query->delete()
+			->from(['a ' => 'authors'])
+			->where(['a.id <' => 99]);
+
+		$result = $query->sql();
+		$this->assertQuotedQuery('DELETE <a> FROM <authors> AS <a>', $result, true);
+
+		$result = $query->execute();
+		$this->assertInstanceOf('Cake\Database\StatementInterface', $result);
+		$this->assertCount(self::AUTHOR_COUNT, $result);
+	}
+
+/**
  * Test a basic delete with no from() call.
  *
  * @return void

+ 19 - 0
tests/TestCase/ORM/TableTest.php

@@ -575,6 +575,25 @@ class TableTest extends \Cake\TestSuite\TestCase {
 	}
 
 /**
+ * Test deleting many records with conditions using the alias
+ *
+ * @return void
+ */
+	public function testDeleteAllAliasedConditions() {
+		$table = new Table([
+			'table' => 'users',
+			'alias' => 'Managers',
+			'connection' => $this->connection,
+		]);
+		$result = $table->deleteAll(['Managers.id <' => 4]);
+		$this->assertTrue($result);
+
+		$result = $table->find('all')->toArray();
+		$this->assertCount(1, $result, 'Only one record should remain');
+		$this->assertEquals(4, $result[0]['id']);
+	}
+
+/**
  * Test that exceptions from the Query bubble up.
  *
  * @expectedException \Cake\Database\Exception

+ 14 - 0
tests/TestCase/Routing/RouteBuilderTest.php

@@ -113,6 +113,20 @@ class RouteBuilderTest extends TestCase {
 	}
 
 /**
+ * Test that compiling a route results in an trailing / optional pattern.
+ *
+ * @return void
+ */
+	public function testConnectTrimTrailingSlash() {
+		$routes = new RouteBuilder($this->collection, '/articles', ['controller' => 'Articles']);
+		$routes->connect('/', ['action' => 'index']);
+
+		$expected = ['plugin' => null, 'controller' => 'Articles', 'action' => 'index', 'pass' => []];
+		$this->assertEquals($expected, $this->collection->parse('/articles'));
+		$this->assertEquals($expected, $this->collection->parse('/articles/'));
+	}
+
+/**
  * Test extensions being connected to routes.
  *
  * @return void