-
Notifications
You must be signed in to change notification settings - Fork 46
add mappings compiler pass for bundles #27
add mappings compiler pass for bundles #27
Conversation
{ | ||
$arguments = array($mappings, '.couchdb.xml'); | ||
$locator = new Definition('Doctrine\Common\Persistence\Mapping\Driver\SymfonyFileLocator', $arguments); | ||
$driver = new Definition('Doctrine\ODM\CouchDB\Mapping\Driver\XmlDriver', array($locator)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there is a CouchDBBundle xml driver that has the only difference that it expects an array and creates the SymfonyFileLocator itself. Mongo and ORM don't provide this - should i rather use the one of the bundle here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it is a BC class AFAIK
this is done rather blindly, i did not get superclass to work at all. superclass fields are not stored by couchdb. i don't understand how couchdb-odm could even work, when looking at couchdb-odm ClassMetadataFactory::doLoadMetadata where it calls $parent->deriveChildMetadata which basically clones that parent and returns metadata, but the caller does not care about what is returned... anyways, the UnitOfWork::computeChangeSet has no superclass metadata anymore and thus no fields of the superclass are persisted. maybe these changes to doLoadMetadata should be picked into master? https://github.com/doctrine/couchdb-odm/pull/61/files#diff-10 - i manually copied just the changes on that file into ClassMetadataFactory and then things just work. |
*/ | ||
public function __construct($driver, $namespaces, $managerParameters, $enabledParameter = false) | ||
{ | ||
$managerParameters[] = 'doctrine_couchdb.default_document_manager'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the others have doctrine.orm.default_document_manager resp doctrine_mongodb.odm.default_document_manager here but couch does not have that parameter...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand this comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
all doctrine bundles use a container parameter to know the name of the default document manager. this is what we need to know in case there is no more specific document manager name specified in a different container parameter.
in all other orm / odm, this parameter has a .odm.
resp .orm.
in the name, but not for couchdb odm. but as things work like this, i suggest we leave it this way.
|
||
namespace Doctrine\Bundle\CouchDBBundle\DependencyInjection\Compiler; | ||
|
||
use Symfony\Bridge\Doctrine\DependencyInjection\CompilerPass\RegisterMappingsPass as BaseMappingPass; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you don't need to alias the base class as it does not conflict
fixed the commented issues. note that this is no longer "blind" but i successfully tested that the user fields are stored when working with this patch: doctrine/couchdb-odm#65 |
the couchdb-odm was fixed and this all works. ok to merge? |
It would be great if this could be merged. |
add mappings compiler pass for bundles
help bundles to provide model classes for several doctrine variants.
analog to doctrine/DoctrineBundle#177 and doctrine/DoctrineMongoDBBundle#185