Skip to content

Commit

Permalink
PR Review - Small bug Fixes
Browse files Browse the repository at this point in the history
Fix BC compatibility for any dev using fetch($currentOffset = null)
Fix SQLite Connect to return a boolean
Remove useless string cast by testing null before
Check TaskName declaration

Fix test 1783 - 64bit compatibility
On 32 bit system, PHP use a float to overflow a bigint.
On 64 bit, PHP int is the same as a database bigint, so this test is not relevant anymore
  • Loading branch information
Tybaze authored and thePanz committed Oct 17, 2022
1 parent 50cb69e commit b6546b1
Show file tree
Hide file tree
Showing 5 changed files with 17 additions and 7 deletions.
6 changes: 4 additions & 2 deletions lib/Doctrine/Connection/Sqlite.php
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ public function __construct(Doctrine_Manager $manager, $adapter)
* initializes database functions missing in sqlite
*
* @see Doctrine_Expression
* @return void
* @return boolean
*/
public function connect()
{
Expand All @@ -96,7 +96,7 @@ public function connect()
// If customer configure it
$hasConfigureStringify = (isset($this->pendingAttributes[Doctrine_Core::ATTR_STRINGIFY_FETCHES]));

parent::connect();
$connected = parent::connect();

if(!$hasConfigureStringify) {
// PHP8.1 require default to true to keep BC
Expand All @@ -109,6 +109,8 @@ public function connect()
$this->dbh->sqliteCreateFunction('concat', array('Doctrine_Expression_Sqlite', 'concatImpl'));
$this->dbh->sqliteCreateFunction('md5', 'md5', 1);
$this->dbh->sqliteCreateFunction('now', array('Doctrine_Expression_Sqlite', 'nowImpl'), 0);

return $connected;
}

/**
Expand Down
7 changes: 6 additions & 1 deletion lib/Doctrine/Connection/Statement.php
Original file line number Diff line number Diff line change
Expand Up @@ -317,10 +317,15 @@ public function execute($params = array())
*/
public function fetch($fetchMode = Doctrine_Core::FETCH_BOTH,
$cursorOrientation = Doctrine_Core::FETCH_ORI_NEXT,
$cursorOffset = 0)
$cursorOffset = null)
{
$event = new Doctrine_Event($this, Doctrine_Event::STMT_FETCH, $this->getQuery());

// null value is not an integer
if(null === $cursorOffset) {
$cursorOffset = 0;
}

$event->fetchMode = $fetchMode;
$event->cursorOrientation = $cursorOrientation;
$event->cursorOffset = $cursorOffset;
Expand Down
2 changes: 1 addition & 1 deletion lib/Doctrine/Task.php
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ public function __construct($dispatcher = null)
$taskName = (string)$this->getTaskName();

//Derive the task name only if it wasn't entered at design-time
if (! strlen($taskName)) {
if ('' === trim($taskName)) {
$taskName = self::deriveTaskName(get_class($this));
}

Expand Down
2 changes: 1 addition & 1 deletion lib/Doctrine/Validator/Notblank.php
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,6 @@ class Doctrine_Validator_Notblank extends Doctrine_Validator_Driver
*/
public function validate($value)
{
return (trim((string) $value) !== '' && $value !== null);
return ($value !== null && trim($value) !== '');
}
}
7 changes: 5 additions & 2 deletions tests/Ticket/1783TestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,12 @@ public function testValidateLargeIntegers()
$this->manager->setAttribute(Doctrine_Core::ATTR_VALIDATE, Doctrine_Core::VALIDATE_ALL);

$test = new Ticket_1783();

$test->bigint = PHP_INT_MAX + 1;

$this->assertTrue($test->isValid());

// This test works on php 32bit version because float allow to represent bigger value than a int.
// On 64bit, int is now equivalent to a database storage of a bigint
$this->assertTrue((PHP_INT_MAX == 2147483647) ? $test->isValid() : true);

$this->manager->setAttribute(Doctrine_Core::ATTR_VALIDATE, Doctrine_Core::VALIDATE_NONE);
}
Expand Down

0 comments on commit b6546b1

Please sign in to comment.