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

Remove ability to flush the EM partially #9485

Merged
merged 1 commit into from
Feb 7, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions UPGRADE.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,17 @@
# Upgrade to 3.0

## BC BREAK: Removed ability to partially flush/commit entity manager and unit of work

The following methods don't accept a single entity or an array of entities anymore:

* `Doctrine\ORM\EntityManager::flush()`
* `Doctrine\ORM\Decorator\EntityManagerDecorator::flush()`
* `Doctrine\ORM\UnitOfWork::commit()`

The semantics of `flush()` and `commit()` will remain the same, but the change
tracking will be performed on all entities managed by the unit of work, and not
just on the provided entities, as the parameter is now completely ignored.

## BC BREAK: Removed ability to partially clear entity manager and unit of work

* Passing an argument other than `null` to `EntityManager::clear()` will raise
Expand Down
7 changes: 0 additions & 7 deletions docs/en/reference/unitofwork.rst
Original file line number Diff line number Diff line change
Expand Up @@ -129,16 +129,10 @@ optimize the performance of the Flush Operation:
- Temporarily mark entities as read only. If you have a very large UnitOfWork
but know that a large set of entities has not changed, just mark them as read
only with ``$entityManager->getUnitOfWork()->markReadOnly($entity)``.
- Flush only a single entity with ``$entityManager->flush($entity)``.
- Use :doc:`Change Tracking Policies <change-tracking-policies>` to use more
explicit strategies of notifying the UnitOfWork what objects/properties
changed.

.. note::

Flush only a single entity with ``$entityManager->flush($entity)`` is deprecated and will be removed in ORM 3.0.
(`Details <https://github.com/doctrine/orm/issues/8459>`_)

Query Internals
---------------

Expand Down Expand Up @@ -202,4 +196,3 @@ ClassMetadataFactory
~~~~~~~~~~~~~~~~~~~~

tbr

8 changes: 0 additions & 8 deletions lib/Doctrine/ORM/Decorator/EntityManagerDecorator.php
Original file line number Diff line number Diff line change
Expand Up @@ -135,14 +135,6 @@ public function find($className, mixed $id, ?int $lockMode = null, ?int $lockVer
return $this->wrapped->find($className, $id, $lockMode, $lockVersion);
}

/**
* {@inheritdoc}
*/
public function flush($entity = null): void
{
$this->wrapped->flush($entity);
}
Copy link
Member

Choose a reason for hiding this comment

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

Why is this method removed from this class but not from EntityManager?

Copy link
Member Author

Choose a reason for hiding this comment

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

The parent class ObjectManagerDecorator already has an implementation of this method without parameters. We don't need to override it anymore.


public function getEventManager(): EventManager
{
return $this->wrapped->getEventManager();
Expand Down
16 changes: 2 additions & 14 deletions lib/Doctrine/ORM/EntityManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -257,26 +257,14 @@ public function createQueryBuilder(): QueryBuilder
* If an entity is explicitly passed to this method only this entity and
* the cascade-persist semantics + scheduled inserts/removals are synchronized.
*
* @param object|mixed[]|null $entity
*
* @throws OptimisticLockException If a version check on an entity that
* makes use of optimistic locking fails.
* @throws ORMException
*/
public function flush($entity = null): void
public function flush(): void
{
if ($entity !== null) {
Deprecation::trigger(
'doctrine/orm',
'https://github.com/doctrine/orm/issues/8459',
'Calling %s() with any arguments to flush specific entities is deprecated and will not be supported in Doctrine ORM 3.0.',
__METHOD__
);
}

$this->errorIfClosed();

$this->unitOfWork->commit($entity);
$this->unitOfWork->commit();
}

/**
Expand Down
111 changes: 9 additions & 102 deletions lib/Doctrine/ORM/UnitOfWork.php
Original file line number Diff line number Diff line change
Expand Up @@ -319,21 +319,10 @@ public function __construct(EntityManagerInterface $em)
* 4) All collection updates
* 5) All entity deletions
*
* @param object|mixed[]|null $entity
*
* @throws Exception
*/
public function commit(object|array|null $entity = null): void
public function commit(): void
{
if ($entity !== null) {
Deprecation::triggerIfCalledFromOutside(
'doctrine/orm',
'https://github.com/doctrine/orm/issues/8459',
'Calling %s() with any arguments to commit specific entities is deprecated and will not be supported in Doctrine ORM 3.0.',
__METHOD__
);
}

$connection = $this->em->getConnection();

if ($connection instanceof PrimaryReadReplicaConnection) {
Expand All @@ -346,15 +335,7 @@ public function commit(object|array|null $entity = null): void
}

// Compute changes done since last commit.
if ($entity === null) {
$this->computeChangeSets();
} elseif (is_object($entity)) {
$this->computeSingleEntityChangeSet($entity);
} else {
foreach ($entity as $object) {
$this->computeSingleEntityChangeSet($object);
}
}
$this->computeChangeSets();

if (
! ($this->entityInsertions ||
Expand All @@ -367,7 +348,7 @@ public function commit(object|array|null $entity = null): void
$this->dispatchOnFlushEvent();
$this->dispatchPostFlushEvent();

$this->postCommitCleanup($entity);
$this->postCommitCleanup();

return; // Nothing to do.
}
Expand Down Expand Up @@ -436,9 +417,7 @@ public function commit(object|array|null $entity = null): void

// Commit failed silently
if ($conn->commit() === false) {
$object = is_object($entity) ? $entity : null;

throw new OptimisticLockException('Commit failed', $object);
throw new OptimisticLockException('Commit failed', null);
}
} catch (Throwable $e) {
$this->em->close();
Expand All @@ -461,13 +440,10 @@ public function commit(object|array|null $entity = null): void

$this->dispatchPostFlushEvent();

$this->postCommitCleanup($entity);
$this->postCommitCleanup();
}

/**
* @param object|object[]|null $entity
*/
private function postCommitCleanup(object|array|null $entity): void
private function postCommitCleanup(): void
{
$this->entityInsertions =
$this->entityUpdates =
Expand All @@ -477,25 +453,9 @@ private function postCommitCleanup(object|array|null $entity): void
$this->nonCascadedNewDetectedEntities =
$this->collectionDeletions =
$this->visitedCollections =
$this->orphanRemovals = [];

if ($entity === null) {
$this->entityChangeSets = $this->scheduledForSynchronization = [];

return;
}

$entities = is_object($entity)
? [$entity]
: $entity;

foreach ($entities as $object) {
$oid = spl_object_id($object);

$this->clearEntityChangeSet($oid);

unset($this->scheduledForSynchronization[$this->em->getClassMetadata(get_class($object))->rootEntityName][$oid]);
}
$this->orphanRemovals =
$this->entityChangeSets =
$this->scheduledForSynchronization = [];
}

/**
Expand All @@ -510,50 +470,6 @@ private function computeScheduleInsertsChangeSets(): void
}
}

/**
* Only flushes the given entity according to a ruleset that keeps the UoW consistent.
*
* 1. All entities scheduled for insertion, (orphan) removals and changes in collections are processed as well!
* 2. Read Only entities are skipped.
* 3. Proxies are skipped.
* 4. Only if entity is properly managed.
*
* @throws InvalidArgumentException
*/
private function computeSingleEntityChangeSet(object $entity): void
{
$state = $this->getEntityState($entity);

if ($state !== self::STATE_MANAGED && $state !== self::STATE_REMOVED) {
throw new InvalidArgumentException('Entity has to be managed or scheduled for removal for single computation ' . self::objToStr($entity));
}

$class = $this->em->getClassMetadata(get_class($entity));

if ($state === self::STATE_MANAGED && $class->isChangeTrackingDeferredImplicit()) {
$this->persist($entity);
}

// Compute changes for INSERTed entities first. This must always happen even in this case.
$this->computeScheduleInsertsChangeSets();

if ($class->isReadOnly) {
return;
}

// Ignore uninitialized proxy objects
if ($entity instanceof Proxy && ! $entity->__isInitialized()) {
return;
}

// Only MANAGED entities that are NOT SCHEDULED FOR INSERTION OR DELETION are processed here.
$oid = spl_object_id($entity);

if (! isset($this->entityInsertions[$oid]) && ! isset($this->entityDeletions[$oid]) && isset($this->entityStates[$oid])) {
$this->computeChangeSet($class, $entity);
}
}

/**
* Executes any extra updates that have been scheduled.
*/
Expand Down Expand Up @@ -3058,15 +2974,6 @@ public function registerManaged(object $entity, array $id, array $data): void
}
}

/**
* INTERNAL:
* Clears the property changeset of the entity with the given OID.
*/
public function clearEntityChangeSet(int $oid): void
{
unset($this->entityChangeSets[$oid]);
}

/* PropertyChangedListener implementation */

/**
Expand Down
5 changes: 0 additions & 5 deletions phpstan-baseline.neon
Original file line number Diff line number Diff line change
Expand Up @@ -105,11 +105,6 @@ parameters:
count: 1
path: lib/Doctrine/ORM/Cache/TimestampQueryCacheValidator.php

-
message: "#^Method Doctrine\\\\Persistence\\\\ObjectManager\\:\\:flush\\(\\) invoked with 1 parameter, 0 required\\.$#"
count: 1
path: lib/Doctrine/ORM/Decorator/EntityManagerDecorator.php

-
message: "#^Return type \\(Doctrine\\\\ORM\\\\Mapping\\\\ClassMetadataFactory\\) of method Doctrine\\\\ORM\\\\Decorator\\\\EntityManagerDecorator\\:\\:getMetadataFactory\\(\\) should be compatible with return type \\(Doctrine\\\\Persistence\\\\Mapping\\\\ClassMetadataFactory\\<Doctrine\\\\Persistence\\\\Mapping\\\\ClassMetadata\\<object\\>\\>\\) of method Doctrine\\\\Persistence\\\\ObjectManager\\:\\:getMetadataFactory\\(\\)$#"
count: 1
Expand Down
6 changes: 0 additions & 6 deletions psalm-baseline.xml
Original file line number Diff line number Diff line change
Expand Up @@ -235,12 +235,6 @@
<DeprecatedClass occurrences="1">
<code>ProxyFactory</code>
</DeprecatedClass>
<MissingParamType occurrences="1">
<code>$entity</code>
</MissingParamType>
<TooManyArguments occurrences="1">
<code>flush</code>
</TooManyArguments>
</file>
<file src="lib/Doctrine/ORM/EntityManager.php">
<ArgumentTypeCoercion occurrences="1">
Expand Down
14 changes: 0 additions & 14 deletions tests/Doctrine/Tests/ORM/EntityManagerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@

use Doctrine\Common\EventManager;
use Doctrine\DBAL\Connection;
use Doctrine\Deprecations\PHPUnit\VerifyDeprecations;
use Doctrine\ORM\Configuration;
use Doctrine\ORM\EntityManager;
use Doctrine\ORM\EntityManagerInterface;
Expand All @@ -19,16 +18,13 @@
use Doctrine\ORM\QueryBuilder;
use Doctrine\ORM\UnitOfWork;
use Doctrine\Tests\Models\CMS\CmsUser;
use Doctrine\Tests\Models\GeoNames\Country;
use Doctrine\Tests\OrmTestCase;
use Generator;
use stdClass;
use TypeError;

class EntityManagerTest extends OrmTestCase
{
use VerifyDeprecations;

/** @var EntityManager */
private $entityManager;

Expand Down Expand Up @@ -222,14 +218,4 @@ public function testWrapInTransactionReThrowsThrowables(): void
self::assertFalse($this->entityManager->isOpen());
}
}

public function testDeprecatedFlushWithArguments(): void
{
$entity = new Country(456, 'United Kingdom');
$this->entityManager->persist($entity);

$this->expectDeprecationWithIdentifier('https://github.com/doctrine/orm/issues/8459');

$this->entityManager->flush($entity);
}
}
Loading