From 0ff3cdf1501d7d5905943a4cf7fd3e719015bbe6 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 7338c2d1f8bd237d8a1c15fbad055a0a688a5757 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 e38af55100660b9c64aed64b4a71ffcc5daa1a50 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 55b7e4cff22e624c9bf9de31d67b120964b0c919 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 8120352c7..e10acd9da 100644 --- a/lib/Doctrine/ORM/UnitOfWork.php +++ b/lib/Doctrine/ORM/UnitOfWork.php @@ -902,7 +902,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); } } @@ -913,9 +915,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 b84c828ea1724867383e38353b68b788c513121f 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 | 66 ++++++++++++++++++++++-------------- 1 file changed, 41 insertions(+), 25 deletions(-) diff --git a/docs/en/reference/events.rst b/docs/en/reference/events.rst index 517f2ec5b..4f6cfedb7 100644 --- a/docs/en/reference/events.rst +++ b/docs/en/reference/events.rst @@ -207,10 +207,13 @@ listeners: - Lifecycle Callbacks are methods on the entity classes that are - called when the event is triggered. They receives some kind of ``EventArgs``. + 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 which - give access to the entity, EntityManager or other relevant data. + methods that receives some kind of ``EventArgs`` instance. + +The EventArgs instance received by the listener gives access to the entity, +EntityManager and other relevant data. .. note:: @@ -224,10 +227,11 @@ listeners: Lifecycle Callbacks ------------------- -A lifecycle event is a regular event with the additional feature of -providing a mechanism to register direct callbacks inside the -corresponding entity classes that are executed when the lifecycle -event occurs. +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. .. code-block:: php @@ -277,8 +281,9 @@ event occurs. } } -Note that when using annotations you have to apply the -@HasLifecycleCallbacks marker annotation on the entity class. +Note that the methods set as lifecycle callbacks need to be public and, +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 can do it with the following. @@ -295,6 +300,10 @@ can do it with the following. prePersist: [ doStuffOnPrePersist, doOtherStuffOnPrePersistToo ] 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 +event types are the ones listed in the previous Lifecycle Events section. + XML would look something like this: .. code-block:: xml @@ -317,9 +326,14 @@ XML would look something like this: -You just need to make sure a public ``doStuffOnPrePersist()`` and -``doStuffOnPostPersist()`` method is defined on your ``User`` -model. +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 +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()``, +``doOtherStuffOnPrePersist()`` and ``doStuffOnPostPersist()`` methods need to be +defined on your ``User`` model. .. code-block:: php @@ -375,8 +389,10 @@ 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 -allow to implement re-usable behaviors between different entity -classes, yet require much more detailed knowledge about the inner +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 workings of the EntityManager and UnitOfWork. Please read the *Implementing Event Listeners* section carefully if you are trying to write your own listener. @@ -476,8 +492,8 @@ 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 you do not have access to the EntityManager or UnitOfWork APIs -inside these events. +that (prior to version 2.4) you do not have access to the +EntityManager or UnitOfWork APIs inside these events. prePersist ~~~~~~~~~~ @@ -501,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 pre - persist 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 ~~~~~~~~~ @@ -699,7 +713,8 @@ 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. + 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 API are strongly discouraged and don't work as expected outside the @@ -769,9 +784,10 @@ 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. -- A callback method could be defined by naming convention or specifying a method mapping. -- When the listener mapping is not given the parser will lookup for methods that match with the naming convention. -- When the listener mapping is given the parser won't lookup for any naming convention. +- 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, + 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. .. code-block:: php