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

EntityManager::getReference() can return null #6755

Merged
merged 1 commit into from
Oct 9, 2017

Conversation

BenMorel
Copy link
Contributor

@BenMorel BenMorel commented Oct 4, 2017

When the requested class has subclasses, a proxy cannot be returned, and the entity has to be loaded from the DB to figure out its actual class:

https://github.com/doctrine/doctrine2/blob/13f838f8bed021cc67e564ec1ff6e994a1a499a8/lib/Doctrine/ORM/EntityManager.php#L498-L500

Because find() is used, if the entity doesn't exist, EntityManager::getReference() therefore returns null.

I think that either:

  • the return type of EntityManagerInterface::getReference() should be object|null
  • or EntityManager::getReference() should throw an ORMException when the id doesn't exist

This is a PR for the first proposal, but if you prefer the second approach (BC break?) I can change the PR.

@SenseException
Copy link
Member

getReference() is a similar method as find() and should have the same behaviour. I think null should be okay.

@lcobucci lcobucci self-assigned this Oct 9, 2017
@lcobucci lcobucci added this to the 2.6.0 milestone Oct 9, 2017
@lcobucci
Copy link
Member

lcobucci commented Oct 9, 2017

@BenMorel it doesn't make much sense to thrown an exception, also because:

https://github.com/doctrine/doctrine2/blob/13f838f8bed021cc67e564ec1ff6e994a1a499a8/lib/Doctrine/ORM/EntityManager.php#L494-L496

So I'd say we simply forgot to add |null on the interface, but maybe @Ocramius or @guilhermeblanco could give us some historical context here.

@Ocramius
Copy link
Member

Ocramius commented Oct 9, 2017 via email

@lcobucci lcobucci merged commit 9e9e562 into doctrine:master Oct 9, 2017
@lcobucci
Copy link
Member

lcobucci commented Oct 9, 2017

@BenMorel 🚢

@BenMorel BenMorel deleted the patch-2 branch October 10, 2017 18:09
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.

5 participants