DCOM-56: Remote autoloading from annotations completly, replacing it with its own loading mechanism #656

doctrinebot opened this Issue Jul 1, 2011 · 2 comments

2 participants


Jira issue originally created by user @beberlei:

This is a mail from me to the symfony-dev mailing-list:

I am sorry it is very late to bring this up again, but given the nasty problems I faced today with the way annotationreader works with autoload = true I have to share them (and blow of some steam). While i guess its to late to change this again before 2.0, we might still have to discuss to go away from autoload = true for 2.1. Now for the reasons:

AnnotationReader with autoload = true (which Symfony2 uses) will not only require a silent autoloader, it requires ALL autoloaders used to be silent. While this is the case for the Symfony Universal loader its not the case for the Doctrine one, and not for many others - and its not even a PSR-0 requirement. For a simple reason: Supporting silent failure means using file_exists, means a file stat, which means apc.stat = 0 is useless. While I don't care so much about it in Doctrine context, because the AnnotationReader default is NOT to autoload annotations this will cause problems for Symfony: Not every library in any Symfony2 app will be included through a silent autoloader. That means given the context users might run into problems using annotations that they have absolutely no way of fixing. And since the AnnotationReader does not know this upfront, potential failures become very real issue.

Example: I use SecurityExtraBundle and happen to have my SuperDuperLibrary XYZ. That library was developed by my company and contains tons of important business logic but unfortunately uses a non-silent autoloader (for whatever reasons). However i use Symfony to secure access to it by generating secure proxies. Now what happens if an annotation reader runs over that library? Because the current namespace is always considered, for every @var NAMESPACE."\var" is classexists'ed and then your code fatal errors. I didn't see this problem before, because i don't use annotations myself so much and always in the BC Doctrine 2.0.x way and that slipped my mind. Same goes for Validation with Annotations on external code, or maybe in the future services definitions through annotations.

All this trouble we are getting into with autoloading annotations is just because we wanted to validate that the annotations used actually exist. But since we changed to a use/import approach rather then re-using namespaces we don't even have the clashing problem anymore that got us into the trouble with class_exists before. The autoloading however also got us and users into other problems:

  1. We have to maintain a rather large map of "blacklisted" annotations that never throw failure exceptions because they are actually used as doc documentations. As with blacklists this is never a complete list.
  2. Users with intensive doc-block usage are punished by Symfony having to maintain their own blacklist of annotations that never throw exceptions so that their code actually works.
  3. Libraries have to handle the case that a reader is either using autoloading or not through special code.

While I do think it would have been nice to offer validation for annotation this is a task that should be put on IDEs and developers, since it turns out that its not possible flawlessly within the library itself. The AnnotationReader should always use classexists($name, false). Docblocks are still comments and the code shouldn't fail because classes are not available that are not even relevant in the context of the current AnnotationReader. Each part of the code that uses an AnnotationReader should require_once the annotations that it can potentially need upfront. That even works for Validation as you can grab the tags from the DIC. That way we have a single mode of operation for the AnnotationReader and not two different ones that are 180 degrees different. OF course one could argue ALWAYS to use classexists($name, true) instead, but i hope my mail showed why this is not a good idea.

If for some reason we do want autoloading for annotations then it should be a mechanism different from the PHP one, because they are both not compatible. The reader could have hooks for autoloading and validation mechanisms. Nothing we want to add for Doctrine Common 2.1, but something we could easily play around with for Common 3.

Next a mail with implementation details:

I propose to change the following on AnnotationReader:

  1. Add a method to register annotation classes: $reader->registerAnnotation("Symfony\Bridge\Doctrine\Validator\UniqueEntity"); and $reader->registerAnnotations(array(..));
  2. Add a method to register files that contain annotations: $reader->registerAnnotationFile(".."), directly loading them via require_once;
  3. Add a method to register annotation namespaces: $reader->registerAnnotationNamespace("Symfony\Component\Validator\Constraint", $dir). This is used to create a silently failing PSR-0 "autoloader" for this path + directories. It can be automatically populated from data in the UniversalClassLoader.
  4. Change the class_exists check to never use autoload = true. If a
    class is part of a registered annotation namespace try load the file based on a silent
    PSR-0 autoload for it using an autoloader impl. contained in the
    AnnotationReader (not the php autoloading mechanism), before triggering
    the class_exists($name, false)

    This way for symfony only the UniversalClassLoader data has to be used
    to populate the registered annotation namespaces.


Comment created by @beberlei:



Issue was closed with resolution "Fixed"

@beberlei beberlei was assigned by doctrinebot Dec 6, 2015
@doctrinebot doctrinebot added this to the 2.1 milestone Dec 6, 2015
@doctrinebot doctrinebot closed this Dec 6, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment