-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Update DefaultRepositoryFactory.php #1159
Conversation
getRepository() does not support different databases for the same entity class due to the lookup in repositoryList. By adding the database name (or any other unique name for the specific database/connection) the lookup works for identical entities in different databases.
We don't support multiple DBs per single The edit looks invalid to me. |
Sorry. Not multiple DBs per EntityManager. Multiple EntityManagers.
The second repo will be the same as the first one due to the if-statement on (original) line 46. |
Seems more like a hack to me, as we do not have a reliable way to differentiate |
@akexakex why not just using the |
In this case, I think you should use a separate repository factory for each entity manager |
@stof the problem still stands: the API doesn't protect from misuse (and it should) |
@Ocramius reusing the same instances for several entity managers will break in many other places as well (I'm not even sure it is safe to reuse the same Configuration instance) |
That's exactly why it exploded in this case. I think we should provide a fix :-) |
Some background info: I configure two EntityManagers, one for each database. I then get a repository (entity is called Transfer) from each EntityManager. Personally I easily resolved this by implementing my own RepositoryFactory. I just wanted to inform you of a potential bug. |
I'm working on a cleaner fix+test. Marco Pivetta On 13 October 2014 18:47, akexakex [email protected] wrote:
|
@akexakex you should not share the same Configuration instance between both EntityManager instances. doctrine does not take this case into account currently |
@stof I actually have few systems relying on this as well, so I'll have to go through the different use-cases here. |
I have two configuration instances as well. The problem is with the default RepositoryFactory implementation which uses an internal lookup-array that only uses the Entity as key. |
@akexakex are you sharing the same RepositoryFactory instance between your 2 configurations ? |
It is shared by default |
@Ocramius this will not share it between different Configuration instances |
and your link is not even related to the RepositoryFactory |
Yes the RepositoryFactory is shared. |
@akexakex as I said previously, doctrine does not support the sharing between entity managers properly for its stack yet. You should use 2 instances for now |
…tRepositoryFactory` implementation
…toryFactory` caches instantiated repositories locally
…toryFactory` considers custom repository class from metadata when instantiating repositories
…ould create different repositories for different entity managers
…eps separate caches per entity manager used to build repositories
…Factory` API by making it `final` and its `protected` members `private`
…y\DefaultRepositoryFactory` becoming `final`
…RepositoryFactory` tests
See #1160 |
…anagers-per-repository-factory #1159 - multiple entity managers per repository factory should be supported
getRepository() does not support different databases for the same entity class due to the lookup in repositoryList. By adding the database name (or any other unique name for the specific database/connection) the lookup works for identical entities in different databases.