Yes, I know this is a problem, but even in the case currently all the
annotation classes have to be made available (through an autoloader).
Registering all possible annotations either with their class name in a
hashmap or already calling require_once on them is just another
mechanism that is not as convenient but allows more robust docblock
parsing. Also the "hack" to doctrine annotations loading is only
necessary, because these classes are organized in a non psr-0 structure.
So these classes are pre-loaded in a hackish way, because there is no
other mechanism for the autoload = true case.
We already set annotations: true for validators and registering the
ExtraBundles is also a statement "pro annotation", so we do have simple
ways in Symfony2 to collect the class-names of all possible annotations.
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 (only directly in the
exact namespace, no subnamspacing), 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.
greetings,
Benjamin
On Fri, 01 Jul 2011 09:24:42 +0200, Christophe COEVOET wrote:
Le 01/07/2011 01:42, Benjamin Eberlei a écrit :
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 class_exists'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
class_exists($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 class_exists($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.
greetings,
Benjamin
The issue about not autloading the annotations appears when sharing
the same annotation reader between several drivers using it (which is
good for performance reasons as it parses the annotations only once).
If each driver requires the annotations it uses, it means you have to
call ALL drivers before being able to use one of them. This is why we
currently have a hack in DoctrineBundle that autoload the
AnnotationDriver during the boot.
--
Christophe | Stof
--
If you want to report a vulnerability issue on symfony, please send
it to security at symfony-project.com
You received this message because you are subscribed to the Google
Groups "symfony developers" group.
To post to this group, send email to [email protected]
To unsubscribe from this group, send email to
[email protected]
For more options, visit this group at
http://groups.google.com/group/symfony-devs?hl=en [1]
Links:
------
[1] http://groups.google.com/group/symfony-devs?hl=en
--
If you want to report a vulnerability issue on symfony, please send it to
security at symfony-project.com
You received this message because you are subscribed to the Google
Groups "symfony developers" group.
To post to this group, send email to [email protected]
To unsubscribe from this group, send email to
[email protected]
For more options, visit this group at
http://groups.google.com/group/symfony-devs?hl=en
