doctrine / orm

Doctrine Object Relational Mapper (ORM)
https://www.doctrine-project.org/projects/orm.html
MIT License
9.95k stars 2.52k forks source link

BackedEnum primary key fails to convert for association using proxy classes #10788

Open wmouwen opened 1 year ago

wmouwen commented 1 year ago

Bug Report

Q A
BC Break unsure
Version 2.15.3

Summary

For a while BackedEnums were allowed as primary keys, where Doctrine would happily convert them to their scalar value. There has been some back and forth about this in previous issues/PRs where the functionality was repaired and broken again.

https://github.com/doctrine/orm/issues/10334 https://github.com/doctrine/orm/issues/10471 https://github.com/doctrine/orm/pull/10508

Most recent events: https://github.com/doctrine/orm/issues/10745 https://github.com/doctrine/orm/pull/10758

Current behavior

Setting a BackedEnum as primary key on an entity, using that entity in an association and trying to save it with a proxy class, will throw an error as the BackedEnum is no longer converted to a scalar value.

Uncaught Error: Object of class Enum\LocaleCode could not be converted to string in /app/vendor/doctrine/dbal/src/Driver/PDO/Statement.php:43
Stack trace:
#0 /app/vendor/doctrine/dbal/src/Driver/PDO/Statement.php(43): PDOStatement->bindValue(1, Object(Enum\LocaleCode), 2)
#1 /app/vendor/doctrine/dbal/src/Statement.php(115): Doctrine\DBAL\Driver\PDO\Statement->bindValue(1, Object(Enum\LocaleCode), 2)
#2 /app/vendor/doctrine/orm/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php(276): Doctrine\DBAL\Statement->bindValue(1, Object(Enum\LocaleCode), Object(Doctrine\DBAL\Types\StringType))
#3 /app/vendor/doctrine/orm/lib/Doctrine/ORM/UnitOfWork.php(1172): Doctrine\ORM\Persisters\Entity\BasicEntityPersister->executeInserts()
#4 /app/vendor/doctrine/orm/lib/Doctrine/ORM/UnitOfWork.php(441): Doctrine\ORM\UnitOfWork->executeInserts(Object(Doctrine\ORM\Mapping\ClassMetadata))
#5 /app/vendor/doctrine/orm/lib/Doctrine/ORM/EntityManager.php(403): Doctrine\ORM\UnitOfWork->commit(NULL)
#6 /app/trigger_error.php(23): Doctrine\ORM\EntityManager->flush()
#7 {main}
  thrown in /app/vendor/doctrine/dbal/src/Driver/PDO/Statement.php on line 43

How to reproduce

See https://github.com/wmouwen/doctrine-orm-10788 for a minimal setup throwing the error.

Code snippet copy 👇 ```php code = \Enum\LocaleCode::Dutch; // Persist entity, clear manager. $entityManager->persist($locale); $entityManager->flush(); $entityManager->clear(); // Create entity with an association to the entity with BackedEnum key. // Note that the use of a proxy class is required for the error to show. $categoryLocale = new \Entity\CategoryLocale(); $categoryLocale->locale = $entityManager->getReference(\Entity\Locale::class, \Enum\LocaleCode::Dutch); // Attempt to persist, throws an error. $entityManager->persist($categoryLocale); $entityManager->flush(); ```

Expected behavior

The BackedEnum is converted and the save succeeds, as it did in version 2.15.2.

-edit- Added the stack trace, made the code snippet collapse, added reference to repository with minimal setup.

wmouwen commented 1 year ago

@Gwemox Allow me to tag you in this issue as you were the author of the initial pull requests.

Gwemox commented 1 year ago

@wmouwen Do you have the stack trace?

wmouwen commented 1 year ago

@Gwemox I've created a repository with minimal code which triggers the error. Important to the case is the use of a proxy class. Stack trace is included in the README in the repository.

https://github.com/wmouwen/doctrine-orm-10788

Gwemox commented 1 year ago

@wmouwen you can convert your backed enum to string with a doctrine custom type : https://www.doctrine-project.org/projects/doctrine-orm/en/2.15/cookbook/custom-mapping-types.html

@greg0ire @sips-richard We should think about the desired solution for 2.6 or 3.0 ?

Currently StringType does nothing.

    public function convertToDatabaseValue($value, AbstractPlatform $platform)
    {
        return $value;
    }

$value is a backed enum.

greg0ire commented 1 year ago

Well if it's a bug, for 2.15

Gwemox commented 1 year ago

We reverted because of this issue https://github.com/doctrine/orm/issues/10745

We could modify the StringType to make a ->value if it is an enum ? Or create EnumType ?

wmouwen commented 1 year ago

After having fiddled with custom types for backed enumerations, I would argue having dedicated types isn't the way to go. You would have to differ between the (kinds of) string and integers that there are default mappings for: EnumStringType, EnumIntegerType, optionally EnumSmallintType, etc.