Doctrine 1

PATCH: Docrine_Record _isValueModified should not type check when comparing integers

Details

  • Type: Bug Bug
  • Status: Resolved Resolved
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: 1.2.2
  • Fix Version/s: 1.2.3
  • Component/s: Record
  • Labels:
    None

Description

The _isValueModified function in Record.php does the following for checking whether or not an integral value was modified:

...
} else if (in_array($type, array('integer', 'int')) && is_numeric($old) && is_numeric($new)) {
return $old !== $new;
...
} else {
return $old !== $new;
...

This does not make sense, it implies strict type checking is always done for integers.
I think the first check (line 1533) should be done loosely so that a string '123' is equal to integer 123, and thereby such a field is not considered modified.

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Dennis Verspuij added a comment -

Attaching the simple patch to correct this.

Show
Dennis Verspuij added a comment - Attaching the simple patch to correct this.
Hide
Permalink
David Jeanmonod added a comment -

This ticket seems to be a duplicate of this one: http://www.doctrine-project.org/jira/browse/DC-550
Maybe we could close it... Seems to be fix in the 1.2.2

Show
David Jeanmonod added a comment - This ticket seems to be a duplicate of this one: http://www.doctrine-project.org/jira/browse/DC-550 Maybe we could close it... Seems to be fix in the 1.2.2
Hide
Permalink
David Jeanmonod added a comment -

Oh no, I misunderstood the DC-550. In fact the problem was introduce when fixing the 550. The strict type checking is a regression that have been introduce in the 550. Before the values old and new were cast to int.

Show
David Jeanmonod added a comment - Oh no, I misunderstood the DC-550 . In fact the problem was introduce when fixing the 550. The strict type checking is a regression that have been introduce in the 550. Before the values old and new were cast to int.
Hide
Permalink
David Jeanmonod added a comment - - edited

This regression can be a real problem. With this, if we set an integer value to a record, then the record is set as modified even if the value was the same as before.
Here is a little test case to demonstrate the problem:

TestCase for DC-664
<?php
require_once('doctrine/lib/Doctrine.php');
//require_once(dirname(__FILE__).'/../../lib/vendor/symfony/lib/plugins/sfDoctrinePlugin/lib/vendor/doctrine/Doctrine.php');
spl_autoload_register(array('Doctrine', 'autoload'));
$manager = Doctrine_Manager::getInstance();
$manager->setAttribute(Doctrine::ATTR_EXPORT, Doctrine::EXPORT_ALL);
$conn = Doctrine_Manager::connection('mysql://root:root@localhost/test_doctrine');
echo "Connection is set up\n";

class Record extends Doctrine_Record {
    public function setTableDefinition(){
        $this->setTableName('record');
        $this->hasColumn('number', 'integer');
    }
}

try {Doctrine::dropDatabases();}catch(Exception $e){} // Drop if exist :-)
Doctrine::createDatabases();
Doctrine::createTablesFromArray(array('Record'));
echo "Database table is create\n";

$record = new Record();
$record->number = 5;
$record->save();

$record = Doctrine::getTable('Record')->createQuery()->fetchOne();
$record->number = 5;

echo "BEFORE:";
var_dump($record->getModified(true));

echo "AFTER:";
var_dump($record->getModified());
Output
Connection is set up
Database table is create
BEFORE:array(1) {
  ["number"]=>
  string(1) "5"
}
AFTHER:array(1) {
  ["number"]=>
  int(5)
}
Show
David Jeanmonod added a comment - - edited This regression can be a real problem. With this, if we set an integer value to a record, then the record is set as modified even if the value was the same as before. Here is a little test case to demonstrate the problem: TestCase for DC-664 <?php require_once('doctrine/lib/Doctrine.php'); //require_once(dirname(__FILE__).'/../../lib/vendor/symfony/lib/plugins/sfDoctrinePlugin/lib/vendor/doctrine/Doctrine.php'); spl_autoload_register(array('Doctrine', 'autoload')); $manager = Doctrine_Manager::getInstance(); $manager->setAttribute(Doctrine::ATTR_EXPORT, Doctrine::EXPORT_ALL); $conn = Doctrine_Manager::connection('mysql: //root:root@localhost/test_doctrine'); echo "Connection is set up\n" ; class Record extends Doctrine_Record { public function setTableDefinition(){ $ this ->setTableName('record'); $ this ->hasColumn('number', 'integer'); } } try {Doctrine::dropDatabases();} catch (Exception $e){} // Drop if exist :-) Doctrine::createDatabases(); Doctrine::createTablesFromArray(array('Record')); echo "Database table is create\n" ; $record = new Record(); $record->number = 5; $record->save(); $record = Doctrine::getTable('Record')->createQuery()->fetchOne(); $record->number = 5; echo "BEFORE:" ; var_dump($record->getModified( true )); echo "AFTER:" ; var_dump($record->getModified()); Output Connection is set up Database table is create BEFORE:array(1) { [ "number" ]=> string(1) "5" } AFTHER:array(1) { [ "number" ]=> int (5) }

People

  • Assignee:
    Jonathan H. Wage
    Reporter:
    Dennis Verspuij
Vote (1)
Watch (1)

Dates

  • Created:
    Updated:
    Resolved: