From 263822fd191d647a0a2e997d07fc0449553b826b Mon Sep 17 00:00:00 2001 From: Thomas Lallement Date: Tue, 18 Mar 2014 15:00:43 +0100 Subject: [PATCH 1/5] Failing Test (since commit 53a5a48aed7d87aa1533c0bcbd72e41b686527d8) Hi, It seems to be a regression since the commit https://github.com/doctrine/doctrine2/commit/53a5a48aed7d87aa1533c0bcbd72e41b686527d8 Doctrine\ORM\PersistentCollection can be populated in $changeSet if you set a PreUpdate and PostUpdate event. Original issue: http://www.doctrine-project.org/jira/browse/DDC-3033 --- .../ORM/Functional/Ticket/DDC3033Test.php | 219 ++++++++++++++++++ 1 file changed, 219 insertions(+) create mode 100644 tests/Doctrine/Tests/ORM/Functional/Ticket/DDC3033Test.php diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC3033Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC3033Test.php new file mode 100644 index 000000000..177fb0976 --- /dev/null +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC3033Test.php @@ -0,0 +1,219 @@ +_schemaTool->createSchema(array( + $this->_em->getClassMetadata(__NAMESPACE__ . '\\DDC3033User'), + $this->_em->getClassMetadata(__NAMESPACE__ . '\\DDC3033Product'), + )); + + $user = new DDC3033User(); + $user->setTitle("Test User"); + $this->_em->persist($user); + + $user2 = new DDC3033User(); + $user2->setTitle("Test User 2"); + $this->_em->persist($user2); + + $product = new DDC3033Product(); + $product->setTitle("Test product"); + $product->addBuyer($user); + + $this->_em->persist($product); + $this->_em->flush(); + + $product->setTitle("Test Change title"); + $product->addBuyer($user2); + + $this->_em->persist($product); + $this->_em->flush(); + + $expect = array( + 'title' => array( + 0 => 'Test product', + 1 => 'Test Change title', + ), + ); + + $this->assertEquals(print_r($expect, true), print_r($product->changeSet, true)); + } +} + +/** + * @Table + * @Entity @HasLifecycleCallbacks + */ +class DDC3033Product +{ + public $changeSet = array(); + + /** + * @var integer $id + * + * @Column(name="id", type="integer") + * @Id + * @GeneratedValue(strategy="AUTO") + */ + private $id; + + /** + * @var string $title + * + * @Column(name="title", type="string", length=255) + */ + private $title; + + /** + * @ManyToMany(targetEntity="DDC3033User") + * @JoinTable( + * name="user_purchases", + * joinColumns={@JoinColumn(name="product_id", referencedColumnName="id")}, + * inverseJoinColumns={@JoinColumn(name="user_id", referencedColumnName="id")} + * ) + */ + private $buyers; + + /** + * Default constructor + */ + public function __construct() + { + $this->buyers = new ArrayCollection(); + } + + /** + * @return integer + */ + public function getId() + { + return $this->id; + } + + /** + * @param string $title + */ + public function setTitle($title) + { + $this->title = $title; + } + + /** + * Get title + * + * @return string + */ + public function getTitle() + { + return $this->title; + } + + /** + * @param string $buyers + */ + public function setBuyers($buyers) + { + $this->buyers = $buyers; + } + + /** + * @return string + */ + public function getBuyers() + { + return $this->buyers; + } + + /** + * @param DDC3033User $buyer + */ + public function addBuyer(DDC3033User $buyer) + { + $this->buyers[] = $buyer; + } + + /** + * @PreUpdate + */ + public function preUpdate(LifecycleEventArgs $eventArgs) + { + } + + /** + * @PostUpdate + */ + public function postUpdate(LifecycleEventArgs $eventArgs) + { + $em = $eventArgs->getEntityManager(); + $uow = $em->getUnitOfWork(); + $entity = $eventArgs->getEntity(); + $classMetadata = $em->getClassMetadata(get_class($entity)); + + $uow->computeChangeSet($classMetadata, $entity); + $this->changeSet = $uow->getEntityChangeSet($entity); + } +} + +/** + * @Table + * @Entity @HasLifecycleCallbacks + */ +class DDC3033User +{ + /** + * @var integer + * + * @Column(name="id", type="integer") + * @Id + * @GeneratedValue(strategy="AUTO") + */ + private $id; + + /** + * @var string + * + * @Column(name="title", type="string", length=255) + */ + private $title; + + /** + * Get id + * + * @return integer + */ + public function getId() + { + return $this->id; + } + + /** + * Set title + * + * @param string $title + */ + public function setTitle($title) + { + $this->title = $title; + } + + /** + * Get title + * + * @return string + */ + public function getTitle() + { + return $this->title; + } +} From 7bf2bcb0172f5874b24ff67d6030fb9ba2802ad9 Mon Sep 17 00:00:00 2001 From: Thomas Lallement Date: Tue, 18 Mar 2014 15:04:48 +0100 Subject: [PATCH 2/5] Update DDC3033Test.php --- .../ORM/Functional/Ticket/DDC3033Test.php | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC3033Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC3033Test.php index 177fb0976..6c675cb4e 100644 --- a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC3033Test.php +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC3033Test.php @@ -31,23 +31,23 @@ class DDC3033Test extends \Doctrine\Tests\OrmFunctionalTestCase $product->setTitle("Test product"); $product->addBuyer($user); - $this->_em->persist($product); + $this->_em->persist($product); $this->_em->flush(); - $product->setTitle("Test Change title"); - $product->addBuyer($user2); + $product->setTitle("Test Change title"); + $product->addBuyer($user2); - $this->_em->persist($product); - $this->_em->flush(); + $this->_em->persist($product); + $this->_em->flush(); - $expect = array( + $expect = array( 'title' => array( - 0 => 'Test product', - 1 => 'Test Change title', - ), - ); + 0 => 'Test product', + 1 => 'Test Change title', + ), + ); - $this->assertEquals(print_r($expect, true), print_r($product->changeSet, true)); + $this->assertEquals(print_r($expect, true), print_r($product->changeSet, true)); } } @@ -160,7 +160,7 @@ class DDC3033Product $entity = $eventArgs->getEntity(); $classMetadata = $em->getClassMetadata(get_class($entity)); - $uow->computeChangeSet($classMetadata, $entity); + $uow->computeChangeSet($classMetadata, $entity); $this->changeSet = $uow->getEntityChangeSet($entity); } } From 6bbc07ddbf4b6be42ff717ee4bd1f4e4687a6f40 Mon Sep 17 00:00:00 2001 From: Thomas Lallement Date: Tue, 18 Mar 2014 22:10:15 +0100 Subject: [PATCH 3/5] Update DDC3033Test.php --- .../ORM/Functional/Ticket/DDC3033Test.php | 106 +++--------------- 1 file changed, 13 insertions(+), 93 deletions(-) diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC3033Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC3033Test.php index 6c675cb4e..cdc6c7b6c 100644 --- a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC3033Test.php +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC3033Test.php @@ -20,22 +20,22 @@ class DDC3033Test extends \Doctrine\Tests\OrmFunctionalTestCase )); $user = new DDC3033User(); - $user->setTitle("Test User"); + $user->title = "Test User"; $this->_em->persist($user); $user2 = new DDC3033User(); - $user2->setTitle("Test User 2"); + $user2->title = "Test User 2"; $this->_em->persist($user2); $product = new DDC3033Product(); - $product->setTitle("Test product"); - $product->addBuyer($user); + $product->title = "Test product"; + $product->buyers[] = $user; $this->_em->persist($product); $this->_em->flush(); - $product->setTitle("Test Change title"); - $product->addBuyer($user2); + $product->title = "Test Change title"; + $product->buyers[] = $user2; $this->_em->persist($product); $this->_em->flush(); @@ -47,7 +47,7 @@ class DDC3033Test extends \Doctrine\Tests\OrmFunctionalTestCase ), ); - $this->assertEquals(print_r($expect, true), print_r($product->changeSet, true)); + $this->assertEquals($expect, $product->changeSet); } } @@ -66,24 +66,24 @@ class DDC3033Product * @Id * @GeneratedValue(strategy="AUTO") */ - private $id; + public $id; /** * @var string $title * * @Column(name="title", type="string", length=255) */ - private $title; + public $title; /** * @ManyToMany(targetEntity="DDC3033User") * @JoinTable( - * name="user_purchases", + * name="user_purchases_3033", * joinColumns={@JoinColumn(name="product_id", referencedColumnName="id")}, * inverseJoinColumns={@JoinColumn(name="user_id", referencedColumnName="id")} * ) */ - private $buyers; + public $buyers; /** * Default constructor @@ -93,56 +93,6 @@ class DDC3033Product $this->buyers = new ArrayCollection(); } - /** - * @return integer - */ - public function getId() - { - return $this->id; - } - - /** - * @param string $title - */ - public function setTitle($title) - { - $this->title = $title; - } - - /** - * Get title - * - * @return string - */ - public function getTitle() - { - return $this->title; - } - - /** - * @param string $buyers - */ - public function setBuyers($buyers) - { - $this->buyers = $buyers; - } - - /** - * @return string - */ - public function getBuyers() - { - return $this->buyers; - } - - /** - * @param DDC3033User $buyer - */ - public function addBuyer(DDC3033User $buyer) - { - $this->buyers[] = $buyer; - } - /** * @PreUpdate */ @@ -178,42 +128,12 @@ class DDC3033User * @Id * @GeneratedValue(strategy="AUTO") */ - private $id; + public $id; /** * @var string * * @Column(name="title", type="string", length=255) */ - private $title; - - /** - * Get id - * - * @return integer - */ - public function getId() - { - return $this->id; - } - - /** - * Set title - * - * @param string $title - */ - public function setTitle($title) - { - $this->title = $title; - } - - /** - * Get title - * - * @return string - */ - public function getTitle() - { - return $this->title; - } + public $title; } From 396337bd0df071d055f5f90586e19c8690e58528 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Sun, 23 Mar 2014 12:35:54 +0100 Subject: [PATCH 4/5] [DDC-3033] Fix bug in UnitOfWork#recomputeSingleEntityChangeSet. The fix for DDC-2624 had a side effect on recomputation of changesets in preUpdate events. The method wasn't adjusted to the changes in its sister method computeChangeSet() and had wrong assumptions about the computation. Especially: 1. Collections have to be skipped 2. Comparison was changed to strict equality only. --- lib/Doctrine/ORM/UnitOfWork.php | 8 ++++---- .../Doctrine/Tests/ORM/Functional/Ticket/DDC3033Test.php | 6 +++--- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php index a61630d1f..26e446ce0 100644 --- a/lib/Doctrine/ORM/UnitOfWork.php +++ b/lib/Doctrine/ORM/UnitOfWork.php @@ -918,7 +918,9 @@ class UnitOfWork implements PropertyChangedListener $actualData = array(); foreach ($class->reflFields as $name => $refProp) { - if ( ! $class->isIdentifier($name) || ! $class->isIdGeneratorIdentity()) { + if (( ! $class->isIdentifier($name) || ! $class->isIdGeneratorIdentity()) + && ($name !== $class->versionField) + && ! $class->isCollectionValuedAssociation($name)) { $actualData[$name] = $refProp->getValue($entity); } } @@ -929,9 +931,7 @@ class UnitOfWork implements PropertyChangedListener foreach ($actualData as $propName => $actualValue) { $orgValue = isset($originalData[$propName]) ? $originalData[$propName] : null; - if (is_object($orgValue) && $orgValue !== $actualValue) { - $changeSet[$propName] = array($orgValue, $actualValue); - } else if ($orgValue != $actualValue || ($orgValue === null ^ $actualValue === null)) { + if ($orgValue !== $actualValue) { $changeSet[$propName] = array($orgValue, $actualValue); } } diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC3033Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC3033Test.php index cdc6c7b6c..4cb78d399 100644 --- a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC3033Test.php +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC3033Test.php @@ -20,11 +20,11 @@ class DDC3033Test extends \Doctrine\Tests\OrmFunctionalTestCase )); $user = new DDC3033User(); - $user->title = "Test User"; + $user->name = "Test User"; $this->_em->persist($user); $user2 = new DDC3033User(); - $user2->title = "Test User 2"; + $user2->name = "Test User 2"; $this->_em->persist($user2); $product = new DDC3033Product(); @@ -135,5 +135,5 @@ class DDC3033User * * @Column(name="title", type="string", length=255) */ - public $title; + public $name; } From ecc2857e2d744787a3a9a5699127c200cc84d7fd Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Sun, 23 Mar 2014 12:37:56 +0100 Subject: [PATCH 5/5] [DDC-3033] Clarify restrictions in events. --- docs/en/reference/events.rst | 36 +++++++++++++++++------------------- 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/docs/en/reference/events.rst b/docs/en/reference/events.rst index c37c0fc73..f315f66b1 100644 --- a/docs/en/reference/events.rst +++ b/docs/en/reference/events.rst @@ -205,12 +205,12 @@ These can be hooked into by two different types of event listeners: - Lifecycle Callbacks are methods on the entity classes that are - called when the event is triggered. As of v2.4 they receive some kind + called when the event is triggered. As of v2.4 they receive some kind of ``EventArgs`` instance. - Lifecycle Event Listeners and Subscribers are classes with specific callback methods that receives some kind of ``EventArgs`` instance. -The EventArgs instance received by the listener gives access to the entity, +The EventArgs instance received by the listener gives access to the entity, EntityManager and other relevant data. .. note:: @@ -225,9 +225,9 @@ EntityManager and other relevant data. Lifecycle Callbacks ------------------- -Lifecycle Callbacks are defined on an entity class. They allow you to -trigger callbacks whenever an instance of that entity class experiences -a relevant lifecycle event. More than one callback can be defined for each +Lifecycle Callbacks are defined on an entity class. They allow you to +trigger callbacks whenever an instance of that entity class experiences +a relevant lifecycle event. More than one callback can be defined for each lifecycle event. Lifecycle Callbacks are best used for simple operations specific to a particular entity class's lifecycle. @@ -280,7 +280,7 @@ specific to a particular entity class's lifecycle. } Note that the methods set as lifecycle callbacks need to be public and, -when using these annotations, you have to apply the +when using these annotations, you have to apply the ``@HasLifecycleCallbacks`` marker annotation on the entity class. If you want to register lifecycle callbacks from YAML or XML you @@ -299,7 +299,7 @@ can do it with the following. postPersist: [ doStuffOnPostPersist ] In YAML the ``key`` of the lifecycleCallbacks entry is the event that you -are triggering on and the value is the method (or methods) to call. The allowed +are triggering on and the value is the method (or methods) to call. The allowed event types are the ones listed in the previous Lifecycle Events section. XML would look something like this: @@ -325,11 +325,11 @@ XML would look something like this: In XML the ``type`` of the lifecycle-callback entry is the event that you -are triggering on and the ``method`` is the method to call. The allowed event +are triggering on and the ``method`` is the method to call. The allowed event types are the ones listed in the previous Lifecycle Events section. -When using YAML or XML you need to remember to create public methods to match the -callback names you defined. E.g. in these examples ``doStuffOnPrePersist()``, +When using YAML or XML you need to remember to create public methods to match the +callback names you defined. E.g. in these examples ``doStuffOnPrePersist()``, ``doOtherStuffOnPrePersist()`` and ``doStuffOnPostPersist()`` methods need to be defined on your ``User`` model. @@ -389,7 +389,7 @@ Listening and subscribing to Lifecycle Events Lifecycle event listeners are much more powerful than the simple lifecycle callbacks that are defined on the entity classes. They -sit at a level above the entities and allow you to implement re-usable +sit at a level above the entities and allow you to implement re-usable behaviors across different entity classes. Note that they require much more detailed knowledge about the inner @@ -492,7 +492,7 @@ data and lost updates/persists/removes. For the described events that are also lifecycle callback events the restrictions apply as well, with the additional restriction -that (prior to version 2.4) you do not have access to the +that (prior to version 2.4) you do not have access to the EntityManager or UnitOfWork APIs inside these events. prePersist @@ -517,11 +517,9 @@ The following restrictions apply to ``prePersist``: - If you are using a PrePersist Identity Generator such as sequences the ID value will *NOT* be available within any PrePersist events. -- Doctrine will not recognize changes made to relations in a prePersist - event called by "reachability" through a cascade persist unless you - use the internal ``UnitOfWork`` API. We do not recommend such - operations in the persistence by reachability context, so do - this at your own risk and possibly supported by unit-tests. +- Doctrine will not recognize changes made to relations in a prePersist + event. This includes modifications to + collections such as additions, removals or replacement. preRemove ~~~~~~~~~ @@ -715,7 +713,7 @@ Restrictions for this event: recognized by the flush operation anymore. - Changes to fields of the passed entities are not recognized by the flush operation anymore, use the computed change-set passed to - the event to modify primitive field values, e.g. use + the event to modify primitive field values, e.g. use ``$eventArgs->setNewValue($field, $value);`` as in the Alice to Bob example above. - Any calls to ``EntityManager#persist()`` or ``EntityManager#remove()``, even in combination with the UnitOfWork @@ -787,7 +785,7 @@ An ``Entity Listener`` could be any class, by default it should be a class with - Different from :ref:`reference-events-implementing-listeners` an ``Entity Listener`` is invoked just to the specified entity - An entity listener method receives two arguments, the entity instance and the lifecycle event. - The callback method can be defined by naming convention or specifying a method mapping. -- When a listener mapping is not given the parser will use the naming convention to look for a matching method, +- When a listener mapping is not given the parser will use the naming convention to look for a matching method, e.g. it will look for a public ``preUpdate()`` method if you are listening to the ``preUpdate`` event. - When a listener mapping is given the parser will not look for any methods using the naming convention.