Skip to content

The default objectManagerLoader doesn't work when the entity has an embedded field. #254

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

Closed
VincentLanglet opened this issue Jan 21, 2022 · 8 comments

Comments

@VincentLanglet
Copy link
Contributor

Since #253, the metadata is resolved without the need of an objectManagerLoader.

It works well, but I have the following issue:

$this->getEntityManager()->getRepository(Expedition::class)->getListOrders($context, $date);

report an error

Call to an undefined method Doctrine\ORM\EntityRepository<App\Entity\Expedition>::getListOrders().  

getRepository is considered as an EntityRepository instead of ExpeditionRepository.
Seems like that as soon as I remove the field

/**
 * @ORM\Embedded(class="App\Entity\Embeddable\Slot")
 */
private ?Slot $slot = null;

from the Expedition entity, it works well.

@VincentLanglet
Copy link
Contributor Author

For my Expedition entity this line https://github.com/phpstan/phpstan-doctrine/blob/master/src/Type/Doctrine/ObjectMetadataResolver.php#L181 returns null. (but not for others entities)

After some dump, I discovered I ends here https://github.com/phpstan/phpstan-doctrine/blob/master/src/Type/Doctrine/ObjectMetadataResolver.php#L120

With the exception message

No identifier/primary key specified for Entity "App\Entity\Embeddable\Slot". Every Entity must have an identifier/primary key.

But there is no need for identifier https://www.doctrine-project.org/projects/doctrine-orm/en/2.11/tutorials/embeddables.html

I think there is something to improve in the PHPStan\Doctrine\Mapping\ClassMetadataFactory.

@VincentLanglet
Copy link
Contributor Author

Adding

	protected function validateRuntimeMetadata($class, $parent)
	{
		return;
	}

to the Phpstan metadatafactory solve the issue.

Would you agree with this @ondrejmirtes ?

@ondrejmirtes
Copy link
Member

That just seems like a workaround.

Does this work when you provide objectManagerLoader? Can you check why?

@VincentLanglet
Copy link
Contributor Author

Does this work when you provide objectManagerLoader? Can you check why?

I use both ORM and ODM on my project.
I used my own fork with #218 with this objectManagerLoader https://github.com/phpstan/phpstan-doctrine/pull/218/files#diff-7be597212047526284ecdf31e9ea0e3cd38415a178b2ebb71af7a7204f388de9 and it was working.

To debug this, I simply run

public function __construct(EntityManagerInterface $entityManager)
    {
        $entityManager->getMetadataFactory()->getMetadataFor(Expedition::class);

        (new \PHPStan\Doctrine\Mapping\ClassMetadataFactory())->getMetadataFor(Expedition::class);

        parent::__construct();
    }

in my symfony project.

The first getMetadataFor work, but the second doesn't.

I ended up in the Doctrine\ORM\Mapping\ClassMetadataFactory::doLoadMetadata which call validateRuntimeMetadata, in which the call $class->validateIdentifier() is made.

    public function validateIdentifier()
    {
        if ($this->isMappedSuperclass || $this->isEmbeddedClass) {
            return;
        }
        // do things

When the call is made by the phpstan factory, $this->isEmbeddedClass is false (but it's true with the doctrine factory).

So I find out that the Doctrine\ORM\Mapping\Driver\AnnotationDriver::loadMetadataForClass is supposed to do the job.
It's called with Expedition and Slot for Doctrine.
It's only called with Expedition for Phpstan.

@VincentLanglet
Copy link
Contributor Author

Ok, so I found why.

Everything is done by the line

$this->driver->loadMetadataForClass($class->getName(), $class);

here: https://github.com/doctrine/orm/blob/2.11.x/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php#L133

In the MappingDriverChain, the code is

	public function loadMetadataForClass($className, ClassMetadata $metadata): void
	{
		foreach ($this->drivers as $driver) {
			if ($driver->isTransient($className)) {
				continue;
			}

			$driver->loadMetadataForClass($className, $metadata);
			return;
		}
	}

and $driver->isTransient($className) return true for embeddable entities.

Doctrine is doing $driver->loadMetadataForClass($className, $metadata) without any transient check.

I tried changing the code to

	public function loadMetadataForClass($className, ClassMetadata $metadata): void
	{
		foreach ($this->drivers as $driver) {
			$driver->loadMetadataForClass($className, $metadata);
		}
	}

but it doesn't work, I get another error. But changing it to

	public function loadMetadataForClass($className, ClassMetadata $metadata): void
	{
		foreach ($this->drivers as $driver) {
			$driver->loadMetadataForClass($className, $metadata);
                         return;
		}
	}

solve the issue for me, because I use Annotation IMHO ; it will just break for people relying on Attribute since the driver won't be use.

@ondrejmirtes
Copy link
Member

This should work: 5fa8fef

Thanks.

@VincentLanglet
Copy link
Contributor Author

This should work: 5fa8fef

Thanks.

Indeed, it works for me.

@github-actions
Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 22, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants