Potential conflict with mongodb bundle for directory prefix

[benglass]

This may be a non-issue but I just ran into it on SonataMediaBundle. Both the extensions for PHPCRBundle and MongoDBBundle have their extension class with the method getMappingObjectDefaultName which returns the string 'Document'.

This causes auto-mapping for both of these bundles to assume anything in the Document namespace must have accompanying mapping. You can get around this by disabling auto mapping and explicitly specifying the prefix for documents. It seems unlikely people will use both of these bundles concurrently since they are both ODM bundles but I suppose its possible.

This could also cause issues with any bundle that has a Document namespace causing exceptions because mapping info wont be found. This of course exists for the MongoDB namespace as well and of course you can get around it by not using auto mapping.

https://github.com/doctrine/DoctrineMongoDBBundle/blob/master/DependencyInjection/DoctrineMongoDBExtension.php#L343

https://github.com/doctrine/DoctrinePHPCRBundle/blob/master/DependencyInjection/DoctrinePHPCRExtension.php#L465

There may be nothing that can or should be done about this but I figured I'd log it here.

[dbu]

this is indeed true. there is not much we can do about this, autoloading is a bit of "magic" and ignoring unknown things rather than reporting would pose a severe usability drawback for people using only one of the doctrines.

the only thing i can point you to is http://symfony.com/doc/current/cookbook/doctrine/mapping_model_classes.html

if you see places in our doc where we could put a tip for this, please do a PR, we should avoid that people have to figure this out for themselves.

[dbu]

oh, and i would think that when you use xml/yml mapping and need no extra fields for phpcr, you should be able to share the same class in the same namespace with different xml/yml files per storage. its mostly a problem with annotations, no?

[benglass]

@dbu thanks for the clarification I think this is something that ideally bundle authors would handle by using the same setup the cmf bundles use of never putting things in the generic folders but instead explicitly mapping Doctrine\Phpcr\MyDocument via the compiler pass since this works with auto mapping as well as explicit mapping approaches.

I'm not sure where the right place to document this would be....

[dbu]

We built those compiler passes while refactoring FOSUserBundle, which had the same problem. The important distinction is that default is fine in project bundles, but not in reusable bundles.

----- Reply message -----
Von: "Ben Glassman" notifications@github.com
An: "doctrine/DoctrinePHPCRBundle" DoctrinePHPCRBundle@noreply.github.com
Cc: "David Buchmann" david@liip.ch
Betreff: [DoctrinePHPCRBundle] Potential conflict with mongodb bundle for directory prefix (#152)
Datum: Mi., Jun. 18, 2014 18:05
@dbu thanks for the clarification I think this is something that ideally bundle authors would handle by using the same setup the cmf bundles use of never putting things in the generic folders but instead explicitly mapping Doctrine\Phpcr\MyDocument via the compiler pass since this works with auto mapping as well as explicit mapping approaches.

I'm not sure where the right place to document this would be....

—
Reply to this email directly or view it on GitHub.

[benglass]

Ah thanks for the point of clarification.

Sadly since sonata media has had Mongo ODM support with documents in the Document namespace since before PHPCR was added it seems unlikely they would accept a PR to move those classes out of the document namespace since it would be a BC so it seems in that particular case it may have to fall on the user to NOT use auto mapping. Was there something you were able to do for FOSUserBundle to retain BC during your refactor?

So perhaps a section in the docs somewhere specifically for re-usable bundle authors which recommendations/best practices. It would include NOT using the default Document namespace and instaed using the compiler pass. Not sure what else might go in there.

[stof]

@benglass We don't. We applied the change in FOSUserBundle in 2.x, using a single class for all Doctrine implementations (the entity is not aware of the data mapper, so this works well when you are not using annotations)

[dbu]

@benglass moving from annotations to xml mappings should work well afaik, and its no BC break. see https://github.com/FriendsOfSymfony/FOSUserBundle/tree/master/Document - the classes are still present for BC.

[benglass]

@dbu We use xml and so does the sonata media bundle. I think its slightly different with sonata media bundle because the Document namespace classes are not PHPCR classes, they are for MongoDB. The PHPCR classes are in the PHPCR namespace so the problem is that with auto mapping enabled PHPCR bundle finds classes in the Document namespace and wants them to be mapped.

Perhaps the solution for this bundle is to get rid of the PHPCR namespace and instead point the PHPCR mappings to the classes in the Document namespace?

This problem doesnt exist in the the user bundle because the DoctrineCouchDBBundle looks for documents in the CouchDocument namespace https://github.com/doctrine/DoctrineCouchDBBundle/blob/master/DependencyInjection/DoctrineCouchDBExtension.php#L296 while the DoctrineMongoDBBundle looks for documents in the Document namespace. It seems like it might have been nice for DoctrinePHPCRBundle to look in a PHPCR or PHPCRDocument namespace by default in order to avoid this conflict but its probably too late for a BC change like that.

[dbu]

we discussed a PHPCRDocument namespace but came up with the extensions instead.

i don't think i had to disable auto mapping with FOSUserBundle. so it feels more like something is misconfigured and tries to store mongo documents with phpcr. the mere fact of having a class in Document should not be a problem i think. you can have any class that is not mapped in a folder where mapped classes could be. but if you try to persist an object of a class that is not mapped, you will have a problem.

[benglass]

I believe the way DoctrinePHPCRBundle is set up is that if you have auto mapping enabled then it will require that any classes found in the Document namespace must have mapping files.

This is because in AbstractDoctrineExtension::loadMappingInformation it will pull all bundles from the container if auto mapping is enabled and
https://github.com/symfony/DoctrineBridge/blob/master/DependencyInjection/AbstractDoctrineExtension.php#L53

It will have no 'prefix' set because it was auto mapped and and then in AbstractDoctrineExtension::getMappingDriverBundleConfigDefaults the prefix will be set to the namespace of the bundle plus the result of $this->getMappingObjectDefaultName() https://github.com/symfony/DoctrineBridge/blob/master/DependencyInjection/AbstractDoctrineExtension.php#L173

In the case of PHPCR bundle that returns just 'Document' https://github.com/doctrine/DoctrinePHPCRBundle/blob/master/DependencyInjection/DoctrinePHPCRExtension.php#L465

In the case of MongoDB bundle it also returns 'Document' and in the case of CouchDBBundle it returns 'CouchDocument'. Therefore MongoDB with auto mapping will raise exceptions if you put PHPCR docs in the document namespace and vice versa.

Please let me know if I've got some part of this wrong as I admit this code is challenging to trace.

[dbu]

i am pretty sure there is some misconception here. there are classes in
this folder like
https://github.com/sonata-project/SonataMediaBundle/blob/master/Document/MediaManager.php
which certainly are not mapped for any doctrine - this would break mongodb.

[benglass]

hmm good point... I know that with auto mapping turned on there was an exception being thrown by PHPCR that it couldnt find mapping files for the classes in the Document folder but only the BaseGallery, BaseGalleryHasMedia and BaseMedia classes and definitely not the manager classes. I guess I will have to investigate further and look at the FileLocator class which actually finds the entities based on the mappings.

[benglass]

@dbu I have determined the answer to your question as to why DocumentManager class (which is also in the Document class) does not trigger this error. The answer is that the SymfonyFileLocator which finds the classes looks for mapping files (in this case in Resources/config/doctrine). It finds BaseGallery.phpcr.xml and so then it looks for a file called BaseGallery.phpcr.xml in the Document namespace (because of the reasons I outlined earlier). While there is a class in the Document namespace which is called BaseGallery.php it is not mapped by the file BaseGallery.phpcr.xml which points at a document in the PHPCR namespace.

The issue is therefore triggered when:

1. You are using automapping
2. You have a Document namespace
3. You have phpcr mapping files in the default Resources/config/doctrine folder
4. The classes in your Document namespace are NOT phpcr documents but are named the same thing as your PHPCR documents

So the conflict can be resolved in sonata admin I think by moving the phpcr mapping files out of the Resources/config/doctrine folder and into a doctrine-phpcr folder and by adding a RegisterMappingsCompilerPass which connects this folder to the PHPCR namespace where the actual documents are stored.

Ultimately there is a problem here that is being partially caused by the DoctrineMongoDBBundle and DoctrinePHPCRBundle both claiming the Document namespace. This causes auto mapping for them to both look in the Document namespace so if you are sharing the same class in Document namespace and having it mapped for both mongo and phcpr thats fine but if you arent then this issue will occur. Figured I'd share my findings while i still (vaguely) understand them.

[dbu]

Yeah, i think you got that right. If you have an idea how to make the cookbook article more clear for next time, please do a PR. There is a logic, but its not self-evident...