Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added registry service #158

Closed
wants to merge 1 commit into from
Closed

Conversation

Spea
Copy link

@Spea Spea commented Jan 9, 2013

This PR adds a new service doctrine

@Ocramius
Copy link
Member

Ocramius commented Jan 9, 2013

@Spea I'm not sure about what this is for...

@Spea
Copy link
Author

Spea commented Jan 9, 2013

I recently created a module for the JMS serializer library. There I have the possibility to use some specific metadata driver for doctrine (see here). As you can see I need an object of type ManagerRegistry for this driver.

Of course I could create this service in the module itself, but I think it makes more sense to implemente it in the ORM module directly.

@Ocramius
Copy link
Member

Ocramius commented Jan 9, 2013

@Spea the idea is nice, but shouldn't this:

  1. go to DoctrineModule?
  2. have adaptation (where needed) for ORM and ODM modules? (hard part: how do you get the ORM and the ODM modules both working with one factory?)

I think only the factory fits the ORM module.

Can you also add test coverage for these classes please?

@@ -22,6 +22,7 @@
'Doctrine\ORM\EntityManager' => 'doctrine.entitymanager.orm_default',
),
'factories' => array(
'doctrine' => new DoctrineORMModule\Service\RegistryFactory(),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should probably be doctrine.registry

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, if there is no config to be passed to the RegistryFactory, this can be a string:

'doctrine' => 'DoctrineORMModule\Service\RegistryFactory',

@Spea
Copy link
Author

Spea commented Jan 9, 2013

I will create a new PR for the registry class in the DoctrineModule then. I also add some tests for the factory.

@Ocramius
Copy link
Member

Ocramius commented Jan 9, 2013

@Spea thank you.

Please think carefully about how to handle the registry, since ORM and ODM have different keys, and we ideally would want to have a single registry (or at least a registry that knows of any defined EMs/DMs)

@Spea
Copy link
Author

Spea commented Jan 9, 2013

I'm not sure about the naming. In the symfony2 bundles the registry services are called doctrine for ORM and doctrine_mongodb for MongoDB. Here I could use doctrine.registry.(orm|odm), if this is ok.

@Spea
Copy link
Author

Spea commented Jan 9, 2013

The registry class is always somehow specific when it comes to alias namespaces. I can put the basic parts into the DoctrineModule, but in the end the ORM/ODM module must extend this registry class and implement the getAliasNamespace method.

@Ocramius
Copy link
Member

Ocramius commented Jan 9, 2013

@Spea I'd use a single registry. After all, anything depending on DoctrineModule only should not make ANY assumptions about the implementation of the ObjectManager.

Here's how it could work:

return new ManagerRegistry(array(
    'doctrine.entitymanager',
    'doctrine.documentmanager',
    'doctrine.contentmanager', //example for PHPCR
    'doctrine.graphmanager', // example for OrientDB ODM
));

The problem is indeed the getConfiguration part. Can we inject that too?

return new ManagerRegistry(array(
    'doctrine.entitymanager' => 'doctrine.configuration',
    'doctrine.documentmanager' =>  'doctrine.configuration',
    'doctrine.contentmanager' => 'doctrine.configuration', //example for PHPCR
    'doctrine.graphmanager' => 'doctrine.configuration', // example for OrientDB ODM
));

Good that this came out now, since I also notice all these collisions.

@superdweebie any ideas?

@Spea
Copy link
Author

Spea commented Jan 9, 2013

The problem with a single registry also is, that it would be impossible to pass a default connection to the registry (which one should be used).

@Ocramius
Copy link
Member

Ocramius commented Jan 9, 2013

@Spea that is up to the user configuring it I suppose. We can add defaults in the ODM and ORM modules. Last module wins :)

@Ocramius
Copy link
Member

I'm closing this after having discussed it a bit with internals. All this registry stuff is basically convoluted, and is here because we still have no clear DI policy in core. There's some movement in data-fixtures, but so far, this is way too hacky, and I prefer not to have this in here.

@Ocramius Ocramius closed this Mar 28, 2013
@eddiejaoude
Copy link

Almost 1 year on, is there another solution for this? I have the same issue as @Spea, I wish to use DoctrineTypeDriver with JMS serialiser library, but unable to get the ManagerRegistry

@superdweebie
Copy link
Contributor

@eddiejaoude while it isn't in doctrine-module itself, for the reasons cited above, there is nothing stopping you writing your own factory to return a ManagerRegistry. Use the code in the diff. It should get you most of the way.

@eddiejaoude
Copy link

I just wanted to check that nothing official has been done. Yes, I will use the diff as an example. Thanks.

@eddiejaoude
Copy link

ZF2 Module on packagist to get ManagerRegistry if anyone else needs it https://packagist.org/packages/eddiejaoude/zf2-doctrine2-manager-registry-service

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants