doctrine / DoctrineModule

Doctrine Module for Laminas
http://www.doctrine-project.org/projects/doctrine-module.html
MIT License
398 stars 269 forks source link

Make Hydrator\DoctrineObject::handleTypeConversions handle all (simple) defaults #622

Closed XCame closed 6 years ago

XCame commented 6 years ago

Extend DoctrineModule\Stdlib\Hydrator\DoctrineObject::handleTypeConversions() to handle all basic conversions according to documentation instead of just DateTime object related values

Stumbled across this, because I hadn't typecast some bools in an Entity setter method. That caused the UoW to detect changes e.g true === 1 and always updated the Entity although nothing changed (logically).

There are default type mappings from Doctrine to PHP via the hydrator, but not vice versa. (see http://docs.doctrine-project.org/projects/doctrine-orm/en/latest/reference/basic-mapping.html#doctrine-mapping-types)

Now there are (at least the simple ones).

Before:

Column definition is boolean $value in DB is 1 (TINYINT) $value extracted from hydrator is true <== this should be the same in both directions $value used in Form becomes 1 $value given to hydrator is 1 $value hydrated is 1 <== but it isn't

Now:

Column definition is boolean $value in DB is 1 (TINYINT) $value extracted from hydrator is true <== this should be the same in both directions $value used in Form becomes 1 $value given to hydrator is 1 $value hydrated is true <== now it is

TomHAnderson commented 6 years ago

Nice work! I'm a fan of this kind of typecasting. Can you provide a unit test to show that true === true and not === 1?

XCame commented 6 years ago

Besides the fact that I'm anything but good at setting up unit tests, I can try to provide some. But I think it won't happen until next month. I'm kind of overbooked at the moment.

Ocramius commented 6 years ago

@XCame this applies to almost everyone working on OSS, we included. Simply send over the tests when you have a chance.

TomHAnderson commented 6 years ago

@XCame Thanks. Once we have tests for this we'll be very close to a 2.0 release.

TomHAnderson commented 6 years ago

Moved to https://github.com/doctrine/DoctrineModule/pull/626 due to lack of unit tests by contributor.