tonyair/silverstripe-framework
1
0
Fork 0
mirror of https://github.com/silverstripe/silverstripe-framework synced 2024-10-22 14:05:37 +02:00
Code Issues Projects Releases Wiki Activity

FIX: Throw deprecation notice on limit=0

Browse Source 
The SS4 behaviour of limit=0 is unlikely to be the SS5 behaviour.
To clear the limit limit=null is recommended.

In addition, there’s a bit tighter type maintenance in the internal
limit data (ensure things are int).

Fixes #2950.
This commit is contained in:
Sam Minnee 2018-10-02 10:37:45 +13:00
parent 755907d117
commit 638e6ec281
2 changed files with 23 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

22
src/ORM/Queries/SQLSelect.php
View File

@ -238,8 +238,8 @@ class SQLSelect extends SQLConditionalExpression
* Pass LIMIT clause either as SQL snippet or in array format.
* Internally, limit will always be stored as a map containing the keys 'start' and 'limit'
*
* @param int|string|array $limit If passed as a string or array, assumes SQL escaped data.
* Only applies for positive values, or if an $offset is set as well.
* @param int|string|array|null $limit If passed as a string or array, assumes SQL escaped data.
* Only applies for positive values.
* @param int $offset
* @throws InvalidArgumentException
* @return $this Self reference
@ -250,10 +250,18 @@ class SQLSelect extends SQLConditionalExpression
throw new InvalidArgumentException("SQLSelect::setLimit() only takes positive values");
}
if ($limit === 0) {
Deprecation::notice(
'4.3',
"setLimit(0) is deprecated in SS4. To clear limit, call setLimit(null). " .
"In SS5 a limit of 0 will instead return no records."
);
}
if (is_numeric($limit) && ($limit || $offset)) {
$this->limit = array(
'start' => $offset,
'limit' => $limit,
'start' => (int)$offset,
'limit' => (int)$limit,
);
} elseif ($limit && is_string($limit)) {
if (strpos($limit, ',') !== false) {
@ -263,12 +271,12 @@ class SQLSelect extends SQLConditionalExpression
}
$this->limit = array(
'start' => trim($start),
'limit' => trim($innerLimit),
'start' => (int)$start,
'limit' => (int)$innerLimit,
);
} elseif ($limit === null && $offset) {
$this->limit = array(
'start' => $offset,
'start' => (int)$offset,
'limit' => $limit
);
} else {

18
tests/php/ORM/SQLSelectTest.php
View File

@ -267,20 +267,23 @@ class SQLSelectTest extends SapphireTest
);
}
/**
* @expectedException PHPUnit_Framework_Error
*/
public function testZeroLimit()
{
Deprecation::notification_version('4.3.0');
$query = new SQLSelect();
$query->setFrom("MyTable");
$query->setLimit(0);
$this->assertSQLEquals(
'SELECT * FROM MyTable',
$query->sql($parameters)
);
}
/**
* @expectedException PHPUnit_Framework_Error
*/
public function testZeroLimitWithOffset()
{
Deprecation::notification_version('4.3.0');
if (!(DB::get_conn() instanceof MySQLDatabase || DB::get_conn() instanceof SQLite3Database
|| DB::get_conn() instanceof PostgreSQLDatabase)
) {
@ -290,11 +293,6 @@ class SQLSelectTest extends SapphireTest
$query = new SQLSelect();
$query->setFrom("MyTable");
$query->setLimit(0, 99);
$this->assertSQLEquals(
'SELECT * FROM MyTable LIMIT 0 OFFSET 99',
$query->sql($parameters)
);
}
/**