From e7ac35ed956856def713c12cad866efc6e4cc41b Mon Sep 17 00:00:00 2001 From: "Roman S. Borschel" Date: Thu, 15 Jul 2010 15:52:42 +0200 Subject: [PATCH] [DDC-119] Fixed. --- lib/Doctrine/ORM/PersistentCollection.php | 9 +- lib/Doctrine/ORM/Query/Parser.php | 1 + lib/Doctrine/ORM/UnitOfWork.php | 48 +++--- .../Tests/ORM/Functional/NotifyPolicyTest.php | 160 ++++++++++++++++++ tests/Doctrine/Tests/ORM/UnitOfWorkTest.php | 64 ++++++- 5 files changed, 250 insertions(+), 32 deletions(-) create mode 100644 tests/Doctrine/Tests/ORM/Functional/NotifyPolicyTest.php diff --git a/lib/Doctrine/ORM/PersistentCollection.php b/lib/Doctrine/ORM/PersistentCollection.php index b1cd9d40a..c2e4207b7 100644 --- a/lib/Doctrine/ORM/PersistentCollection.php +++ b/lib/Doctrine/ORM/PersistentCollection.php @@ -279,14 +279,15 @@ final class PersistentCollection implements Collection { if ( ! $this->isDirty) { $this->isDirty = true; - //if ($this->isNotifyRequired) { - //$this->em->getUnitOfWork()->scheduleCollectionUpdate($this); - //} + if ($this->association !== null && $this->association->isOwningSide && $this->association->isManyToMany() && + $this->em->getClassMetadata(get_class($this->owner))->isChangeTrackingNotify()) { + $this->em->getUnitOfWork()->scheduleForDirtyCheck($this->owner); + } } } /** - * Gets a boolean flag indicating whether this colleciton is dirty which means + * Gets a boolean flag indicating whether this collection is dirty which means * its state needs to be synchronized with the database. * * @return boolean TRUE if the collection is dirty, FALSE otherwise. diff --git a/lib/Doctrine/ORM/Query/Parser.php b/lib/Doctrine/ORM/Query/Parser.php index d17a92070..96ff91bf7 100644 --- a/lib/Doctrine/ORM/Query/Parser.php +++ b/lib/Doctrine/ORM/Query/Parser.php @@ -1656,6 +1656,7 @@ class Parser $peek = $this->_lexer->glimpse(); $supportsAlias = true; + if ($peek['value'] != '(' && $this->_lexer->lookahead['type'] === Lexer::T_IDENTIFIER) { if ($peek['value'] == '.') { // ScalarExpression diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php index b6bb85e60..a75bc277c 100644 --- a/lib/Doctrine/ORM/UnitOfWork.php +++ b/lib/Doctrine/ORM/UnitOfWork.php @@ -405,8 +405,6 @@ class UnitOfWork implements PropertyChangedListener } $oid = spl_object_hash($entity); - - //TODO: Skip this step if changetracking = NOTIFY $actualData = array(); foreach ($class->reflFields as $name => $refProp) { $value = $refProp->getValue($entity); @@ -455,7 +453,8 @@ class UnitOfWork implements PropertyChangedListener // Entity is "fully" MANAGED: it was already fully persisted before // and we have a copy of the original data $originalData = $this->originalEntityData[$oid]; - $changeSet = array(); + $isChangeTrackingNotify = isset($this->entityChangeSets[$oid]); + $changeSet = $isChangeTrackingNotify ? $this->entityChangeSets[$oid] : array(); foreach ($actualData as $propName => $actualValue) { $orgValue = isset($originalData[$propName]) ? $originalData[$propName] : null; @@ -474,6 +473,8 @@ class UnitOfWork implements PropertyChangedListener $this->collectionDeletions[] = $orgValue; } } + } else if ($isChangeTrackingNotify) { + continue; } else if (is_object($orgValue) && $orgValue !== $actualValue) { $changeSet[$propName] = array($orgValue, $actualValue); } else if ($orgValue != $actualValue || ($orgValue === null ^ $actualValue === null)) { @@ -513,13 +514,14 @@ class UnitOfWork implements PropertyChangedListener foreach ($this->identityMap as $className => $entities) { $class = $this->em->getClassMetadata($className); - // Skip class if change tracking happens through notification - if ($class->isChangeTrackingNotify() /* || $class->isReadOnly*/) { - continue; - } + // Skip class if instances are read-only + //if ($class->isReadOnly) { + // continue; + //} - // If change tracking is explicit, then only compute changes on explicitly persisted entities - $entitiesToProcess = $class->isChangeTrackingDeferredExplicit() ? + // If change tracking is explicit or happens through notification, then only compute + // changes on entities of that type that are explicitly marked for synchronization. + $entitiesToProcess = ! $class->isChangeTrackingDeferredImplicit() ? (isset($this->scheduledForDirtyCheck[$className]) ? $this->scheduledForDirtyCheck[$className] : array()) : $entities; @@ -958,6 +960,12 @@ class UnitOfWork implements PropertyChangedListener return isset($this->entityUpdates[spl_object_hash($entity)]); } + public function isScheduledForDirtyCheck($entity) + { + $rootEntityName = $this->em->getClassMetadata(get_class($entity))->rootEntityName; + return isset($this->scheduledForDirtyCheck[$rootEntityName][spl_object_hash($entity)]); + } + /** * INTERNAL: * Schedules an entity for deletion. @@ -2010,7 +2018,7 @@ class UnitOfWork implements PropertyChangedListener public function scheduleForDirtyCheck($entity) { $rootClassName = $this->em->getClassMetadata(get_class($entity))->rootEntityName; - $this->scheduledForDirtyCheck[$rootClassName][] = $entity; + $this->scheduledForDirtyCheck[$rootClassName][spl_object_hash($entity)] = $entity; } /** @@ -2119,7 +2127,6 @@ class UnitOfWork implements PropertyChangedListener * @param string $propertyName The name of the property that changed. * @param mixed $oldValue The old value of the property. * @param mixed $newValue The new value of the property. - * @todo Simply use _scheduledForDirtyCheck, otherwise a lot of behavior is potentially inconsistent. */ public function propertyChanged($entity, $propertyName, $oldValue, $newValue) { @@ -2132,24 +2139,13 @@ class UnitOfWork implements PropertyChangedListener return; // ignore non-persistent fields } - //$this->scheduleForDirtyCheck($entity); + // Update changeset and mark entity for synchronization $this->entityChangeSets[$oid][$propertyName] = array($oldValue, $newValue); - - if ($isAssocField) { - $assoc = $class->associationMappings[$propertyName]; - if ($assoc->isOneToOne() && $assoc->isOwningSide) { - $this->entityUpdates[$oid] = $entity; - } else if ($oldValue instanceof PersistentCollection) { - // A PersistentCollection was de-referenced, so delete it. - if ( ! in_array($oldValue, $this->collectionDeletions, true)) { - $this->collectionDeletions[] = $oldValue; - } - } - } else { - $this->entityUpdates[$oid] = $entity; + if ( ! isset($this->scheduledForDirtyCheck[$class->rootEntityName][$oid])) { + $this->scheduleForDirtyCheck($entity); } } - + /** * Gets the currently scheduled entity insertions in this UnitOfWork. * diff --git a/tests/Doctrine/Tests/ORM/Functional/NotifyPolicyTest.php b/tests/Doctrine/Tests/ORM/Functional/NotifyPolicyTest.php new file mode 100644 index 000000000..8d9d3ffa6 --- /dev/null +++ b/tests/Doctrine/Tests/ORM/Functional/NotifyPolicyTest.php @@ -0,0 +1,160 @@ +_schemaTool->createSchema(array( + $this->_em->getClassMetadata('Doctrine\Tests\ORM\Functional\NotifyUser'), + $this->_em->getClassMetadata('Doctrine\Tests\ORM\Functional\NotifyGroup') + )); + } catch (\Exception $e) { + // Swallow all exceptions. We do not test the schema tool here. + } + } + + public function testChangeTracking() + { + //$this->_em->getConnection()->getConfiguration()->setSQLLogger(new \Doctrine\DBAL\Logging\EchoSQLLogger); + + $user = new NotifyUser(); + $group = new NotifyGroup(); + $user->setName('roman'); + $group->setName('dev'); + + $user->getGroups()->add($group); + $group->getUsers()->add($user); + + $this->_em->persist($user); + $this->_em->persist($group); + $this->_em->flush(); + $this->_em->clear(); + + $userId = $user->getId(); + $groupId = $group->getId(); + unset($user, $group); + + $user = $this->_em->find(__NAMESPACE__.'\NotifyUser', $userId); + $this->assertEquals(1, $user->getGroups()->count()); + $group = $this->_em->find(__NAMESPACE__.'\NotifyGroup', $groupId); + $this->assertEquals(1, $group->getUsers()->count()); + + $group2 = new NotifyGroup(); + $group2->setName('nerds'); + $this->_em->persist($group2); + $user->getGroups()->add($group2); + $group2->getUsers()->add($user); + + $group->setName('geeks'); + + $this->_em->flush(); + $this->_em->clear(); + + $group2Id = $group2->getId(); + unset($group2, $user); + + $user = $this->_em->find(__NAMESPACE__.'\NotifyUser', $userId); + $this->assertEquals(2, $user->getGroups()->count()); + $group2 = $this->_em->find(__NAMESPACE__.'\NotifyGroup', $group2Id); + $this->assertEquals(1, $group2->getUsers()->count()); + $group = $this->_em->find(__NAMESPACE__.'\NotifyGroup', $groupId); + $this->assertEquals(1, $group->getUsers()->count()); + $this->assertEquals('geeks', $group->getName()); + } +} + +class NotifyBaseEntity implements NotifyPropertyChanged { + private $listeners = array(); + + public function addPropertyChangedListener(PropertyChangedListener $listener) { + $this->listeners[] = $listener; + } + + protected function onPropertyChanged($propName, $oldValue, $newValue) { + if ($this->listeners) { + foreach ($this->listeners as $listener) { + $listener->propertyChanged($this, $propName, $oldValue, $newValue); + } + } + } +} + +/** @Entity @ChangeTrackingPolicy("NOTIFY") */ +class NotifyUser extends NotifyBaseEntity { + /** @Id @Column(type="integer") @GeneratedValue */ + private $id; + + /** @Column */ + private $name; + + /** @ManyToMany(targetEntity="NotifyGroup") */ + private $groups; + + function __construct() { + $this->groups = new ArrayCollection; + } + + function getId() { + return $this->id; + } + + function getName() { + return $this->name; + } + + function setName($name) { + $this->onPropertyChanged('name', $this->name, $name); + $this->name = $name; + } + + function getGroups() { + return $this->groups; + } +} + +/** @Entity */ +class NotifyGroup extends NotifyBaseEntity { + /** @Id @Column(type="integer") @GeneratedValue */ + private $id; + + /** @Column */ + private $name; + + /** @ManyToMany(targetEntity="NotifyUser", mappedBy="groups") */ + private $users; + + function __construct() { + $this->users = new ArrayCollection; + } + + function getId() { + return $this->id; + } + + function getName() { + return $this->name; + } + + function setName($name) { + $this->onPropertyChanged('name', $this->name, $name); + $this->name = $name; + } + + function getUsers() { + return $this->users; + } +} + diff --git a/tests/Doctrine/Tests/ORM/UnitOfWorkTest.php b/tests/Doctrine/Tests/ORM/UnitOfWorkTest.php index 4a3b6a4df..5858b5a98 100644 --- a/tests/Doctrine/Tests/ORM/UnitOfWorkTest.php +++ b/tests/Doctrine/Tests/ORM/UnitOfWorkTest.php @@ -131,21 +131,44 @@ class UnitOfWorkTest extends \Doctrine\Tests\OrmTestCase { $persister = new EntityPersisterMock($this->_emMock, $this->_emMock->getClassMetadata("Doctrine\Tests\ORM\NotifyChangedEntity")); $this->_unitOfWork->setEntityPersister('Doctrine\Tests\ORM\NotifyChangedEntity', $persister); + $itemPersister = new EntityPersisterMock($this->_emMock, $this->_emMock->getClassMetadata("Doctrine\Tests\ORM\NotifyChangedRelatedItem")); + $this->_unitOfWork->setEntityPersister('Doctrine\Tests\ORM\NotifyChangedRelatedItem', $itemPersister); $entity = new NotifyChangedEntity; $entity->setData('thedata'); $this->_unitOfWork->persist($entity); $this->_unitOfWork->commit(); + $this->assertEquals(1, count($persister->getInserts())); + $persister->reset(); $this->assertTrue($this->_unitOfWork->isInIdentityMap($entity)); $entity->setData('newdata'); $entity->setTransient('newtransientvalue'); - $this->assertTrue($this->_unitOfWork->isScheduledForUpdate($entity)); + $this->assertTrue($this->_unitOfWork->isScheduledForDirtyCheck($entity)); $this->assertEquals(array('data' => array('thedata', 'newdata')), $this->_unitOfWork->getEntityChangeSet($entity)); + + $item = new NotifyChangedRelatedItem(); + $entity->getItems()->add($item); + $item->setOwner($entity); + $this->_unitOfWork->persist($item); + + $this->_unitOfWork->commit(); + $this->assertEquals(1, count($itemPersister->getInserts())); + $persister->reset(); + $itemPersister->reset(); + + + $entity->getItems()->removeElement($item); + $item->setOwner(null); + $this->assertTrue($entity->getItems()->isDirty()); + $this->_unitOfWork->commit(); + $updates = $itemPersister->getUpdates(); + $this->assertEquals(1, count($updates)); + $this->assertTrue($updates[0] === $item); } public function testGetEntityStateOnVersionedEntityWithAssignedIdentifier() @@ -192,7 +215,7 @@ class NotifyChangedEntity implements \Doctrine\Common\NotifyPropertyChanged /** * @Id * @Column(type="integer") - * @GeneratedValue(strategy="AUTO") + * @GeneratedValue */ private $id; /** @@ -202,10 +225,21 @@ class NotifyChangedEntity implements \Doctrine\Common\NotifyPropertyChanged private $transient; // not persisted + /** @OneToMany(targetEntity="NotifyChangedRelatedItem", mappedBy="owner") */ + private $items; + + public function __construct() { + $this->items = new \Doctrine\Common\Collections\ArrayCollection; + } + public function getId() { return $this->id; } + public function getItems() { + return $this->items; + } + public function setTransient($value) { if ($value != $this->transient) { $this->_onPropertyChanged('transient', $this->transient, $value); @@ -238,6 +272,32 @@ class NotifyChangedEntity implements \Doctrine\Common\NotifyPropertyChanged } } +/** @Entity */ +class NotifyChangedRelatedItem +{ + /** + * @Id + * @Column(type="integer") + * @GeneratedValue + */ + private $id; + + /** @ManyToOne(targetEntity="NotifyChangedEntity", inversedBy="items") */ + private $owner; + + public function getId() { + return $this->id; + } + + public function getOwner() { + return $this->owner; + } + + public function setOwner($owner) { + $this->owner = $owner; + } +} + /** @Entity */ class VersionedAssignedIdentifierEntity {