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

Decouple public API from Doctrine\Persistence\Proxy #10832

Merged
merged 1 commit into from
Jul 15, 2023

Conversation

nicolas-grekas
Copy link
Member

Related to doctrine/persistence#334
On the path to deprecating Doctrine\Persistence\Proxy in the future.

Adding EntityManager::isUninitializedEntity() will allow userland to decouple from the Proxy interface:

before:

if ($entity instanceof Proxy && !$entity->__isInitialized()) {
    $entity->__load();
}

after:

if ($em->isUninitializedEntity($entity)) {
    $em->initializeObject($entity);
}

/**
* {@inheritDoc}
*/
public function isUninitializedEntity($obj)
Copy link
Member

Choose a reason for hiding this comment

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

you should add the parameter and return types

Copy link
Member Author

Choose a reason for hiding this comment

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

return type done, mixed for param is not compatible with PHP 7.1. @inheritDoc to the rescue, from doctrine/persistence#334

/**
* @internal
*
* @method void __setInitialized(bool $initialized)
Copy link
Member

Choose a reason for hiding this comment

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

if it is an internal API, can't we define the method in the interface directly ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not possible because this overlaps with exiting declarations on legacy Proxy interfaces / implementations.

@nicolas-grekas nicolas-grekas force-pushed the proxy-loaded-api branch 6 times, most recently from 826b87e to 89cbb6f Compare July 10, 2023 14:22
@nicolas-grekas
Copy link
Member Author

PR green 🍏 (ignoring unrelated doc failures.)

@beberlei
Copy link
Member

This changes code from not requiring the EntityManager, to requiring the entity manager to make this check.

Are we ok with this, since explicit proxy loading puts the code into "persistence context" that probably has the entity manager already?

@nicolas-grekas
Copy link
Member Author

Are we ok with this, since explicit proxy loading puts the code into "persistence context" that probably has the entity manager already?

That's my though also yes: the code shouldn't care about proxy or not (as stated in the doc). But when caring, it means you have the EM not too far away (if not already there). The concept of a Doctrine proxy is in the same domain as Doctrine ORM/ODM in this context.

stof
stof previously approved these changes Jul 12, 2023
@derrabus derrabus merged commit dca7ddf into doctrine:2.16.x Jul 15, 2023
42 checks passed
@derrabus derrabus added this to the 2.16.0 milestone Jul 15, 2023
@nicolas-grekas nicolas-grekas deleted the proxy-loaded-api branch July 15, 2023 08:59
@greg0ire greg0ire mentioned this pull request Aug 15, 2023
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