Support multiple object managers? #53
Comments
|
Probably internally we should just return the whole doctrine registry from the php file and use https://github.com/doctrine/persistence/blob/master/lib/Doctrine/Persistence/ManagerRegistry.php#L87 ? |
|
Hi, I see two possible options:
|
different databases 😉
doctrine bundle can handle multiple managers and we can differentiate it based on the context in our app What do you think about my comment to use the doctrine registry for this in phpstan? is there any drawback? Thats the source of truth that knows which entity is managed by which object manager 😊 |
What's the "context"? I really don't know how the ManagerRegistry works - and I don't think that all applications using Doctrine use it. It seems too Symfony-specific. |
|
Also, when analysing DQL, it really doesn't go through ClassMetadata, I already need to have an ObjectManager (EntityManager) to run the query on. |
I meant based on bundles/namespaces in our app.
Yeah I guess choosing the "correct" ObjectManager for analyzing DQL could be tricky indeed 😕 Maybe running different phpstan configurations on different namespaces/folders is an option. Will have a look. |
If you differentiate based on that, it sounds usable :) I'm gonna keep this open since more people might have this problem, so it should be somehow supported. |
The concept of having multiple entity managers is not limited to Symfony, nor is it an anti-pattern. People who use multiple object managers can use a registry to fetch a manager by name or to fetch the manager responsible for a given entity/class. In fact, this bundle even supports returning the correct repository type for a given object: #46. The option of having multiple configurations is a workaround, not a solution. For example, in one of our applications we use both MongoDB ODM and ORM. This also means that classes might very well use an |
|
My point is that in Doctrine itself, there's only the interface and an abstract class. Not all projects use the ManagerRegistry. BTW: It looks just like a service locator pattern under a different name to me ;) There's some problems around this if I wanted to add support - I have no way of testing it. And I don't know what are the common patterns: Is that you inject ManagerRegistry into your class-that-uses-EM (just call it service from now on) like this? /** @var ManagerRegistry */
private $managerRegistry;
public function __construct(ManagerRegistry $managerRegistry)
{
$this->managerRegistry = $managerRegistry;
}And use it later like this: $this->managerRegistry->getManager('a')->getRepository(FooEntity::class)->...Or is it rather like this? /** @var EntityManager */
private $entityManager;
public function __construct(ManagerRegistry $managerRegistry)
{
$this->entityManager = $managerRegistry->getManager('a');
}Which makes it really difficult when the EM is used later in the class - PHPStan has no idea which one is used because it just sees the property phpDoc. It would have to be annotated like I can imagine some other combinations too. My point is that it's really difficult to support all of them generally so I must hard-code every pattern like this... |
|
In our app we
So without checking the DI config there is no way to always know which manager is used inside some class for phpstan I'm afraid 😢 |
|
We, for example, have separate symfony bundle, which uses different database and has it's own doctrine config. In services.yml we have: services:
_defaults:
...
autowire: true
bind:
$em: '@doctrine.orm.exchange_entity_manager'So everywhere in this bundle where we have Also ran into that issue today while updating phpstan-doctrine to get rid of these :) parameters:
ignoreErrors:
- '#Call to an undefined method Doctrine\\ORM\\EntityRepository<[a-zA-Z0-9\\_:]+>::[a-zA-Z0-9_]+\(\)#'
- '#Method .+? should return [a-zA-Z0-9\\]+ but returns Doctrine\\ORM\\EntityRepository<[a-zA-Z0-9\\_:]+>#'
- '#Method .+? should return [a-zA-Z0-9\\]+ but returns Doctrine\\Common\\Persistence\\ObjectRepository#'
- '#Method .+? should return [a-zA-Z0-9\\]+ but returns Doctrine\\Common\\Persistence\\ObjectRepository#'
- '#Property [a-zA-Z0-9\\]+::\$[a-zA-Z]+Repository \([a-zA-Z0-9\\]+\) does not accept Doctrine\\ORM\\EntityRepository<[a-zA-Z0-9\\]+>#' |
|
Just an idea: accept an array of entity managers and retry with each of them if |
|
@ondrejmirtes of course catching exceptions is a hacky workaround. This should be better: $entityManager->getConfiguration()->getMetadataDriverImpl()->getAllClassNames();Which returns an array of FQCN's of entites managed by that particular entity manager. Not sure about the ODM, but there must be something similar. So we'll end up with returning one entity manager, or an array of managers in What do you think? |
The Symfony Bundle for the MongoDB ODM contains the same manager registry logic as the ORM bundle. They're separate registries but the functionality is exactly the same. |
|
I personally always inject ManagerRegistry, the entity managers can be replaced, e.g. if you have an SQL error and reconnect. It probably makes sense to change the file we use for phpstan to return the manager registry which can then give us the correct entity manager pet entity. I guess for generic dql you can use the default manager and most of the time this will be correct |
|
Just reminding that this is still a huge issue, as having more than one object manager (simply connection) makes impossible to use all the cool related functionality. :) |
|
Anyone is welcome to suggest and/or contribute a solution. I haven’t come up with one yet. |
|
@ondrejmirtes, I suggested one possible solution above in #53 (comment), but you didn't answer whether you like it or not and why. The other one is to use ManagerRegistry, which you seem to not like. I'm not sure there is option 3. :) |
|
How would DQL support work with your suggestion? |
|
@ondrejmirtes You'll need to have a list of entities and object managers that manage them, and pick a corresponding object manager before analyzing DQL. But I didn't look at how you have it implemented at the moment. That's more of a question to you: would DQL support work if you'd know which entity is managed by which object manager? |
|
Currently there's no easy way to parse DQL and find which entities are used in it. |
|
@ondrejmirtes First thing is to watch for entity name in from() calls, the second is to get entity name from knowing which repo is for which entity if there's no from() call. Since it's not possible to join across different connections, the same OM can be used for joined entities when analyzing. Or that is exactly what you mean by saying "not easy"? Just to clarify, I just want the discussion to go on. Not trying to demand anyone to fix anything. |
|
Proper DQL AST is in development for Doctrine ORM 3.0. Authors themselves expressed that the current shape is not easy to parse. |
|
Oops, I thought about query builders for some reason (probably because we don't use DQL). But anyway, should phpstan-doctrine parse all the DQL propperly or just couple of FROM statement variations will be enought: \Fully\Qualyfied\Class\Name and AcmeBundle:EntityName? Maybe just lock the implementation to FQCN, at least it will work then. |
|
Seems like it would be valid for the moment at least to just use the default manager for DQL |
|
No news on this issue? I have an app which uses both the ODM and the ORM, and PHPStan mistakes calls to the ODM to calls to the ORM... |
|
Hi, I have no experience with multiple EMs nor with ODM. I’d need a sample
project with some errors produced so I know if it’s fixable and how to do
it.
On Mon, 1 Jul 2019 at 10:13, Alessandro Lai ***@***.***> wrote:
No news on this issue? I have an app which uses both the ODM and the ORM,
and PHPStan mistakes calls to the ODM to calls to the ORM...
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#53?email_source=notifications&email_token=AAAZTOGJBKDME2FS727XJP3P5G4BFA5CNFSM4GXIX2F2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODY5LFTA#issuecomment-507163340>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAAZTOAR335U4QZTZXRCBEDP5G4BFANCNFSM4GXIX2FQ>
.
--
Ondřej Mirtes
|
|
I personally use direct DI and type against |
|
I’m not sure how this is possible:
and PHPStan mistakes calls to the ODM to calls to the ORM...
I’d really need a sample project to be able to verify the solutions...
On Mon, 1 Jul 2019 at 11:58, Alessandro Lai ***@***.***> wrote:
I personally use direct DI and type against EntityManager and
DocumentManager, so for me the fix would be a simple "duplication" of the
type mapping. But it's obviously not a general solution.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#53?email_source=notifications&email_token=AAAZTOFP4KKA5C6S2CFGRXTP5HIN3A5CNFSM4GXIX2F2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODY5UB6I#issuecomment-507199737>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAAZTOHCCKHTZOMSBNWEAPDP5HIN3ANCNFSM4GXIX2FQ>
.
--
Ondřej Mirtes
|
That's because I added the snippet of code to get the ORM from the Symfony container, so the extension has only that, and it tries to use it with ODM calls too. I'll see if I'll have some time to create an example project. |
|
@ondrejmirtes I've prepared a small test project: https://github.com/Jean85/phpstan-doctrine-demo The docker setup is maybe a bit broken, I'll probably fix it tomorrow, but the PHPStan analysis works fine, in the sense that it reports the same error I get locally. |
|
Hey, we use PHPStan on level 8 with the latest Symfony 5.0 stack and have discovered a new behaviour which appeared along the PHPStan 0.12.9 release :X. In fact, we already knew that the Doctrine The problem here is that using it that way does obviously trigger the following error, despite the fact that it is a totally legit use of entityManagers ;) : We used to ignore this expected error with the following regex under the But using the What could we do next :) ? |
|
@ZarcoX Open a new issue and post a stacktrace + code sample that causes this. |
|
@ondrejmirtes Actually, this is totally related to this issue. Your suggestion on using the What happens here is that any class attempting to call my second entityManager will error out because of the following code : Which is normal since the |
|
Any news on this one? @OskarStark and I are running into the same issue in a project with multiple databases and multiple entity managers. |
|
Ha, for what it's worth, @ondrejmirtes' recommendation to use a configuration per entity manager works for me. |
|
@localheinz which recommendation? |
For us, option one works in a particular project. It's not the best solution, but it works - that's what counts. I can see that there are projects where it will not work, and I'm looking forward to a solution that works with a single configuration, a single run, and multiple entity managers. |
|
It's not fixed yet because I don't see how an ideal solution should work. I'm glad those workarounds work. |
|
Hi, having this problem too. |
|
@Mika56 Feel free to try it and send a PR. We could detect whether |
|
Here's an With this, <?php
use Doctrine\Bundle\DoctrineBundle\Registry;
use Doctrine\Persistence\Mapping\ClassMetadataFactory;
use Doctrine\Persistence\ObjectManager;
require __DIR__.'/../config/bootstrap.php';
$kernel = new \App\Kernel($_SERVER['APP_ENV'], (bool) $_SERVER['APP_DEBUG']);
$kernel->boot();
$doctrine = $kernel->getContainer()->get('doctrine');
$metadataFactory = new class($doctrine) implements ClassMetadataFactory
{
private Registry $doctrine;
public function __construct(Registry $doctrine)
{
$this->doctrine = $doctrine;
}
public function getAllMetadata()
{
$all = [];
foreach ($this->doctrine->getManagers() as $manager) {
$all = array_merge($all, $manager->getMetadataFactory()->getAllMetadata());
}
return $all;
}
public function getMetadataFor($className)
{
return $this->doctrine->getManagerForClass($className)->getClassMetadata($className);
}
public function isTransient($className)
{
$isTransient = true;
foreach ($this->doctrine->getManagers() as $manager) {
$isTransient = $isTransient && $manager->getMetadataFactory()->isTransient($className);
}
return $isTransient;
}
public function hasMetadataFor($className)
{
$hasMetadata = false;
foreach ($this->doctrine->getManagers() as $manager) {
$hasMetadata = $hasMetadata || $manager->getMetadataFactory()->hasMetadataFor($className);
}
return $hasMetadata;
}
public function setMetadataFor($className, $class)
{
throw new \Exception(__FILE__);
}
};
return new class($doctrine, $metadataFactory) implements ObjectManager
{
private Registry $doctrine;
private ClassMetadataFactory $metadataFactory;
public function __construct(Registry $doctrine, ClassMetadataFactory $metadataFactory)
{
$this->doctrine = $doctrine;
$this->metadataFactory = $metadataFactory;
}
public function getRepository($className)
{
return $this->doctrine->getRepository($className);
}
public function getClassMetadata($className)
{
return $this->doctrine->getManagerForClass($className)->getClassMetadata($className);
}
public function getMetadataFactory()
{
return $this->metadataFactory;
}
public function find($className, $id)
{
throw new \Exception(__FILE__);
}
public function persist($object)
{
throw new \Exception(__FILE__);
}
public function remove($object)
{
throw new \Exception(__FILE__);
}
public function merge($object)
{
throw new \Exception(__FILE__);
}
public function clear($objectName = null)
{
throw new \Exception(__FILE__);
}
public function detach($object)
{
throw new \Exception(__FILE__);
}
public function refresh($object)
{
throw new \Exception(__FILE__);
}
public function flush()
{
throw new \Exception(__FILE__);
}
public function initializeObject($obj)
{
throw new \Exception(__FILE__);
}
public function contains($object)
{
throw new \Exception(__FILE__);
}
}; |
works perfect for me |
|
Hi, I have a project using both ORM and ODM and encountered the same issue. I tried a phpstan-loader, this way: By following the previous suggestion, and expecting phpstan to use the orm or odm according to the entity/document. Am I doing something wrong in my object loader or something should/could be changed in the phpstan-doctrine extension implementation in order to allow such an object loader implementation. |
|
Most phpstan-doctrine features will work without providing objectManagerLoader in phpstan-doctrine 1.2.0 thanks to: #253 It should solve this use-case too. |
|
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
In my project I have multiple objectmanagers.
This currently leads to errors like:
because its using the wrong objectManager (the one returned in
object-manager.php) for some entities.The text was updated successfully, but these errors were encountered: