Fixing issues in DboSource::defaultConditions() and DboSource::conditions() where doubly deleting a record from the beforeDelete and delete() could create incorrect conditions that would delete all records in a table. Fixes #250

This commit is contained in:
Mark Story 2010-01-22 10:30:22 -05:00
parent 317463096a
commit 5d35fd8d38
2 changed files with 46 additions and 6 deletions

View file

@ -1378,6 +1378,7 @@ class DboSource extends DataSource {
} else { } else {
$combined = array_combine($fields, $values); $combined = array_combine($fields, $values);
} }
$fields = implode(', ', $this->_prepareUpdateFields($model, $combined, empty($conditions))); $fields = implode(', ', $this->_prepareUpdateFields($model, $combined, empty($conditions)));
$alias = $joins = null; $alias = $joins = null;
@ -1605,18 +1606,27 @@ class DboSource extends DataSource {
} }
/** /**
* Creates a default set of conditions from the model if $conditions is null/empty. * Creates a default set of conditions from the model if $conditions is null/empty.
* If conditions are supplied then they will be returned. If a model doesn't exist and no conditions
* were provided either null or false will be returned based on what was input.
* *
* @param object $model * @param object $model
* @param mixed $conditions * @param mixed $conditions Array of conditions, conditions string, null or false. If an array of conditions,
* or string conditions those conditions will be returned. With other values the model's existance will be checked.
* If the model doesn't exist a null or false will be returned depending on the input value.
* @param boolean $useAlias Use model aliases rather than table names when generating conditions * @param boolean $useAlias Use model aliases rather than table names when generating conditions
* @return mixed * @return mixed Either null, false, $conditions or an array of default conditions to use.
* @see DboSource::update()
* @see DboSource::conditions()
*/ */
function defaultConditions(&$model, $conditions, $useAlias = true) { function defaultConditions(&$model, $conditions, $useAlias = true) {
if (!empty($conditions)) { if (!empty($conditions)) {
return $conditions; return $conditions;
} }
if (!$model->exists()) { $exists = $model->exists();
if (!$exists && $conditions !== null) {
return false; return false;
} elseif (!$exists) {
return null;
} }
$alias = $model->alias; $alias = $model->alias;
@ -1741,9 +1751,11 @@ class DboSource extends DataSource {
return array_unique($fields); return array_unique($fields);
} }
/** /**
* Creates a WHERE clause by parsing given conditions data. * Creates a WHERE clause by parsing given conditions data. If an array or string
* conditions are provided those conditions will be parsed and quoted. If a boolean
* is given it will be integer cast as condition. Null will return 1 = 1.
* *
* @param mixed $conditions Array or string of conditions * @param mixed $conditions Array or string of conditions, or any value.
* @param boolean $quoteValues If true, values should be quoted * @param boolean $quoteValues If true, values should be quoted
* @param boolean $where If true, "WHERE " will be prepended to the return value * @param boolean $where If true, "WHERE " will be prepended to the return value
* @param Model $model A reference to the Model instance making the query * @param Model $model A reference to the Model instance making the query
@ -1764,8 +1776,11 @@ class DboSource extends DataSource {
} }
return $clause . implode(' AND ', $out); return $clause . implode(' AND ', $out);
} }
if ($conditions === false || $conditions === true) {
return $clause . (int)$conditions . ' = 1';
}
if (empty($conditions) || trim($conditions) == '' || $conditions === true) { if (empty($conditions) || trim($conditions) == '') {
return $clause . '1 = 1'; return $clause . '1 = 1';
} }
$clauses = '/^WHERE\\x20|^GROUP\\x20BY\\x20|^HAVING\\x20|^ORDER\\x20BY\\x20/i'; $clauses = '/^WHERE\\x20|^GROUP\\x20BY\\x20|^HAVING\\x20|^ORDER\\x20BY\\x20/i';

View file

@ -2096,6 +2096,30 @@ class DboSourceTest extends CakeTestCase {
$expected = array('DISTINCT `Vendor`.`id`', '`Vendor`.`name`'); $expected = array('DISTINCT `Vendor`.`id`', '`Vendor`.`name`');
$this->assertEqual($result, $expected); $this->assertEqual($result, $expected);
} }
/**
* test that booleans and null make logical condition strings.
*
* @return void
*/
function testBooleanNullConditionsParsing() {
$result = $this->testDb->conditions(true);
$this->assertEqual($result, ' WHERE 1 = 1', 'true conditions failed %s');
$result = $this->testDb->conditions(false);
$this->assertEqual($result, ' WHERE 0 = 1', 'false conditions failed %s');
$result = $this->testDb->conditions(null);
$this->assertEqual($result, ' WHERE 1 = 1', 'null conditions failed %s');
$result = $this->testDb->conditions(array());
$this->assertEqual($result, ' WHERE 1 = 1', 'array() conditions failed %s');
$result = $this->testDb->conditions('');
$this->assertEqual($result, ' WHERE 1 = 1', '"" conditions failed %s');
$result = $this->testDb->conditions(' ', '" " conditions failed %s');
$this->assertEqual($result, ' WHERE 1 = 1');
}
/** /**
* testStringConditionsParsing method * testStringConditionsParsing method
* *
@ -3093,6 +3117,7 @@ class DboSourceTest extends CakeTestCase {
$result = $this->testDb->renderStatement('delete', array('fields' => 'value=2', 'table' => 'table', 'conditions' => 'WHERE 1=1', 'alias' => 'alias', 'joins' => '')); $result = $this->testDb->renderStatement('delete', array('fields' => 'value=2', 'table' => 'table', 'conditions' => 'WHERE 1=1', 'alias' => 'alias', 'joins' => ''));
$this->assertPattern('/^\s*DELETE\s+alias\s+FROM\s+table\s+AS\s+alias\s+WHERE\s+1=1\s*$/', $result); $this->assertPattern('/^\s*DELETE\s+alias\s+FROM\s+table\s+AS\s+alias\s+WHERE\s+1=1\s*$/', $result);
} }
/** /**
* testStatements method * testStatements method
* *