doctrine
Inappropriate error detail for UniqueEntity annotation error
Bug Report
| Q | A |
|---|---|
| BC Break | No |
| Version | All |
Summary
I found an error trying to validate the doctrine schema:
php bin/console doctrine:schema:validate --verbose
Mapping
-------
In Constraint.php line 163:
[Symfony\Component\Validator\Exception\MissingOptionsException]
The options "fields" must be set for constraint "Symfony\Bridge\Doctrine\Validator\Constraints\UniqueEntity".
Exception trace:
at F:\websites\crayner\quoll\vendor\symfony\validator\Constraint.php:163
Symfony\Component\Validator\Constraint->normalizeOptions() at F:\websites\crayner\quoll\vendor\symfony\validator\Constraint.php:108
Symfony\Component\Validator\Constraint->__construct() at F:\websites\crayner\quoll\vendor\doctrine\annotations\lib\Doctrine\Common\Annotations\DocParser.php:827
Doctrine\Common\Annotations\DocParser->Annotation() at F:\websites\crayner\quoll\vendor\doctrine\annotations\lib\Doctrine\Common\Annotations\DocParser.php:662
Doctrine\Common\Annotations\DocParser->Annotations() at F:\websites\crayner\quoll\vendor\doctrine\annotations\lib\Doctrine\Common\Annotations\DocParser.php:354
Doctrine\Common\Annotations\DocParser->parse() at F:\websites\crayner\quoll\vendor\doctrine\annotations\lib\Doctrine\Common\Annotations\AnnotationReader.php:221
Doctrine\Common\Annotations\AnnotationReader->getClassAnnotations() at F:\websites\crayner\quoll\vendor\doctrine\annotations\lib\Doctrine\Common\Annotations\CachedReader.php:80
Doctrine\Common\Annotations\CachedReader->getClassAnnotations() at F:\websites\crayner\quoll\vendor\doctrine\persistence\lib\Doctrine\Persistence\Mapping\Driver\AnnotationDriver.php:177
Doctrine\Persistence\Mapping\Driver\AnnotationDriver->isTransient() at F:\websites\crayner\quoll\vendor\doctrine\persistence\lib\Doctrine\Persistence\Mapping\Driver\AnnotationDriver.php:245
Doctrine\Persistence\Mapping\Driver\AnnotationDriver->getAllClassNames() at F:\websites\crayner\quoll\vendor\doctrine\persistence\lib\Doctrine\Persistence\Mapping\Driver\MappingDriverChain.php:107
Doctrine\Persistence\Mapping\Driver\MappingDriverChain->getAllClassNames() at F:\websites\crayner\quoll\vendor\doctrine\persistence\lib\Doctrine\Persistence\Mapping\AbstractClassMetadataFactory.php:90
Doctrine\Persistence\Mapping\AbstractClassMetadataFactory->getAllMetadata() at F:\websites\crayner\quoll\vendor\doctrine\orm\lib\Doctrine\ORM\Tools\SchemaValidator.php:68
Doctrine\ORM\Tools\SchemaValidator->validateMapping() at F:\websites\crayner\quoll\vendor\doctrine\orm\lib\Doctrine\ORM\Tools\Console\Command\ValidateSchemaCommand.php:69
Doctrine\ORM\Tools\Console\Command\ValidateSchemaCommand->execute() at F:\websites\crayner\quoll\vendor\doctrine\doctrine-bundle\Command\Proxy\ValidateSchemaCommand.php:34
Doctrine\Bundle\DoctrineBundle\Command\Proxy\ValidateSchemaCommand->execute() at F:\websites\crayner\quoll\vendor\symfony\console\Command\Command.php:258
Symfony\Component\Console\Command\Command->run() at F:\websites\crayner\quoll\vendor\symfony\console\Application.php:929
Symfony\Component\Console\Application->doRunCommand() at F:\websites\crayner\quoll\vendor\symfony\framework-bundle\Console\Application.php:96
Symfony\Bundle\FrameworkBundle\Console\Application->doRunCommand() at F:\websites\crayner\quoll\vendor\symfony\console\Application.php:264
Symfony\Component\Console\Application->doRun() at F:\websites\crayner\quoll\vendor\symfony\framework-bundle\Console\Application.php:82
Symfony\Bundle\FrameworkBundle\Console\Application->doRun() at F:\websites\crayner\quoll\vendor\symfony\console\Application.php:140
Symfony\Component\Console\Application->run() at F:\websites\crayner\quoll\bin\console:42
The issue here is not with the error, but in the fact that the error is not correctly identifying the issue, which is a annotation parse error, causing the Contraint violation. Unfortunately, this error does not identify the entity at fault, and I had to remove ALL UniqueEntity references in 150+ entities and return them one at a time to eventually find the error. The error on the annotation looks like:
...
use Doctrine\ORM\Mapping as ORM;
use Symfony\Bridge\Doctrine\Validator\Constraints\UniqueEntity;
...
* @ORM\Table(name="Scale",
* indexes={@ORM\Index(name="lowest_acceptable",columns={"lowest_acceptable"})},
* uniqueConstraints={@ORM\UniqueConstraint(name="name",columns={"name"}),
* @ORM\UniqueConstraint(name="abbreviation",columns={"abbreviation"}),
* })
* @UniqueEntity{"name"})
* @UniqueEntity({"abbreviation"})
The error is on the "name" line, which is missing the opening bracket.
Current behavior
The current behaviour is as stated above.
How to reproduce
Remove the opening bracket on any UniqueEntity annotation will duplicate this issue.
Expected behavior
[Doctrine\Common\Annotations\AnnotationException]
[Syntax Error] Failed to see bracket set at position xxx in class App\Entity\Name.
I'm transferring the issue, IMO it does not have to do with the ORM, you could have the same issue with any other kind of annotations.
I disagree a little with the expected behavior, I would expect to have the Symfony\Component\Validator\Exception\MissingOptionsException passed as $previous argument of an exception in this package, with a message that would look like this:
Failed getting annotations from App\EntityName, or maybe something more even more precise Failed getting annotations while parsing file X, on line Y.
Possible next step: determine what pieces of information (classname? filename? line number?) we have at each level of this stack trace:
Symfony\Component\Validator\Constraint->normalizeOptions() at F:\websites\crayner\quoll\vendor\symfony\validator\Constraint.php:108
Symfony\Component\Validator\Constraint->__construct() at F:\websites\crayner\quoll\vendor\doctrine\annotations\lib\Doctrine\Common\Annotations\DocParser.php:827
Doctrine\Common\Annotations\DocParser->Annotation() at F:\websites\crayner\quoll\vendor\doctrine\annotations\lib\Doctrine\Common\Annotations\DocParser.php:662
Doctrine\Common\Annotations\DocParser->Annotations() at F:\websites\crayner\quoll\vendor\doctrine\annotations\lib\Doctrine\Common\Annotations\DocParser.php:354
Doctrine\Common\Annotations\DocParser->parse() at F:\websites\crayner\quoll\vendor\doctrine\annotations\lib\Doctrine\Common\Annotations\AnnotationReader.php:221
Doctrine\Common\Annotations\AnnotationReader->getClassAnnotations() at F:\websites\crayner\quoll\vendor\doctrine\annotations\lib\Doctrine\Common\Annotations\CachedReader.php:80
Sound good to me. As long as the message identifies an annotation failure in a particular file at a location in that file, I will be happy. Thanks for directing as well.
The annotation reader has a method/property reflection object available; therefore, it should be easy to add filename, class name and method/property name into any exception message. However, because showing a filename in production may be undesirable, I would add class and method name only (it should suffice and not reveal any sensitive information).
The line number may be complicated, because the reflection object provides start line of the method, not the docblock, so it would likely be wrong.
However, because showing a filename in production may be undesirable
Why would people display exception messages in production?
Anyway, thanks for your insight, I think this is ready to be developed. @crayner , do you want to give it a go? Here is the piece of code that should have a try… catch: https://github.com/doctrine/annotations/blob/8fc7b23076e6144d68379f3410bb7dcf15cdc902/lib/Doctrine/Common/Annotations/AnnotationReader.php#L140-L148
Thanks @greg0ire , Sorry, I find contributing very difficult as I become upset about reviews that argue about stuff that I have already stated, but people do not read or understand. So, No, I will not move this forward. Thanks for the offer.
If this error ever gets to a production site, the the developer never tested the code @jkufner as this is a HARD error. (i.e. HARD = it will fail 100% of the time, and is caused by the developer IMHO)
@crayner Yes, however, it may happen after an update to a slightly incompatible version, or after not upgrading the dependencies. We cannot predict any particular context or scenario, so it is better to be careful by default.
@crayner no problem, I understand. How about you @jkufner , would you want to give it a try?