Merge pull request #4 from w3c/inverse

Allow monitoring inverse side of relationships
w3c · Mar 30, 2017 · e7e1bb9 · e7e1bb9
2 parents db8bafd + f35e940
commit e7e1bb9
Show file tree

Hide file tree

Showing 16 changed files with 2,220 additions and 121 deletions.
diff --git a/Annotation/Change.php b/Annotation/Change.php
@@ -23,4 +23,9 @@ class Change
      */
     public $class = LifecyclePropertyChangedEvent::class;
 
+    /**
+     * @var bool
+     * @deprecated to be removed in next major version and the class will always act as if it was set to true
+     */
+    public $monitor_owning = false;
 }
diff --git a/Annotation/Update.php b/Annotation/Update.php
@@ -27,4 +27,6 @@ class Update
      * @var bool
      */
     public $monitor_collections = true;
+
+    public $monitor_owning = false;
 }
diff --git a/EventListener/LifecycleEventsListener.php b/EventListener/LifecycleEventsListener.php
diff --git a/EventListener/LifecyclePropertyEventsListener.php b/EventListener/LifecyclePropertyEventsListener.php
@@ -2,12 +2,11 @@
 
 namespace W3C\LifecycleEventsBundle\EventListener;
 
-use Doctrine\Common\Annotations\Reader;
 use Doctrine\Common\Util\ClassUtils;
 use Doctrine\ORM\Event\PreUpdateEventArgs;
-use Doctrine\ORM\Mapping\ClassMetadata;
 use Doctrine\ORM\PersistentCollection;
 use W3C\LifecycleEventsBundle\Annotation\Change;
+use W3C\LifecycleEventsBundle\Services\AnnotationGetter;
 use W3C\LifecycleEventsBundle\Services\LifecycleEventsDispatcher;
 
 /**
@@ -25,20 +24,20 @@ class LifecyclePropertyEventsListener
     private $dispatcher;
 
     /**
-     * @var Reader
+     * @var AnnotationGetter
      */
-    private $reader;
+    private $annotationGetter;
 
     /**
      * Constructs a new instance
      *
-     * @param LifecycleEventsDispatcher $dispatcher the dispatcher to fed
-     * @param Reader $reader
+     * @param LifecycleEventsDispatcher $dispatcher the dispatcher to feed
+     * @param AnnotationGetter $annotationGetter
      */
-    public function __construct(LifecycleEventsDispatcher $dispatcher, Reader $reader)
+    public function __construct(LifecycleEventsDispatcher $dispatcher, AnnotationGetter $annotationGetter)
     {
-        $this->dispatcher = $dispatcher;
-        $this->reader     = $reader;
+        $this->dispatcher       = $dispatcher;
+        $this->annotationGetter = $annotationGetter;
     }
 
     public function preUpdate(PreUpdateEventArgs $args)
@@ -57,7 +56,8 @@ private function addPropertyChanges(PreUpdateEventArgs $args)
         $classMetadata = $args->getEntityManager()->getClassMetadata($realClass);
 
         foreach ($args->getEntityChangeSet() as $property => $change) {
-            $annotation = $this->getChangeAnnotation($classMetadata, $property);
+            /** @var Change $annotation */
+            $annotation = $this->annotationGetter->getPropertyAnnotation($classMetadata, $property, Change::class);
 
             if ($annotation) {
                 $this->dispatcher->addPropertyChange(
@@ -89,7 +89,8 @@ private function addCollectionChanges(PreUpdateEventArgs $args)
             }
 
             $property   = $update->getMapping()['fieldName'];
-            $annotation = $this->getChangeAnnotation($classMetadata, $property);
+            /** @var Change $annotation */
+            $annotation = $this->annotationGetter->getPropertyAnnotation($classMetadata, $property, Change::class);
 
             // Make sure $u belongs to the entity we are working on
             if (!isset($annotation)) {
@@ -105,29 +106,4 @@ private function addCollectionChanges(PreUpdateEventArgs $args)
             );
         }
     }
-
-    /**
-     * @param ClassMetadata $classMetadata
-     * @param string $property
-     *
-     * @return Change
-     * @throws \ReflectionException
-     */
-    private function getChangeAnnotation(ClassMetadata $classMetadata, $property)
-    {
-        $reflProperty = $classMetadata->getReflectionProperty($property);
-
-        if ($reflProperty) {
-            /** @var Change $annotation */
-            $annotation = $this->reader->getPropertyAnnotation(
-                $classMetadata->getReflectionProperty($property),
-                Change::class
-            );
-            return $annotation;
-        }
-
-        throw new \ReflectionException(
-            $classMetadata->getName() . '.' . $property . ' not found. Could this be a private field of a parent class?'
-        );
-    }
 }
diff --git a/README.md b/README.md
@@ -13,7 +13,7 @@ This Symfony bundle is meant to capture and dispatch events that happen througho
 Doctrine already provides such events, but using them directly has a few shortcomings:
 - You don't decide at which point in a action you want to dispatch events. Events are fired during a flush.
 - When Doctrine events are fired, you are not assured that the entities have actually been saved in the database. 
-This is obvious for preUpdate (sent before persisting the changes), but postPersist and postRemove have the same issue: 
+This is obvious for preUpdate (sent before persisting the changes), but postPersist and preRemove have the same issue: 
 if you persist two new entities in a single transaction, the first insert could work (thus an event would be sent) but 
 not the second, resulting in no entities being saved at all
 
@@ -112,6 +112,7 @@ must have a constructor with the following signature:
 public function __construct($entity, array $propertiesChangeSet = null, array $collectionsChangeSet = null)
 ```
 - `monitor_collections`: whether the annotation should monitor changes to collection fields. Defaults to true
+- `monitor_owning`: whether owning side relationship changes should be also monitored as inverse side changes. Defaults to false
 
 #### `@On\Change`
 
@@ -144,11 +145,16 @@ and for collections:
  */
 public function __construct($entity, $property, $deletedElements = null, $insertedElements = null)
 ```
+- `monitor_owning`: whether to record changes to this field when owning sides change (defaults to `false`). Using
+`@On\Change` on inverse side of relationships won't trigger any events unless this paramter is set to true. This
+parameter is likely to be removed in the next major version and act as if it was set to `true` since when the 
+annotation is added to the inverse side of relationship, it is obvious it means that you want changes to owning side to
+be monitored here
 
 #### `@On\IgnoreClassUpdates`
 
 This annotation is a bit different. When placed on a field (property or collection), it prevents `@On\Update` from 
-firing events related to this field. `@On\Change' ones will still work. This annotation does not allow any parameters.
+firing events related to this field. `@On\Change` ones will still work. This annotation does not allow any parameters.
 
 #### Example class
 

diff --git a/Resources/config/services.yml b/Resources/config/services.yml
@@ -4,24 +4,28 @@ parameters:
     w3c_lifecycle_events.dispatcher.class: W3C\LifecycleEventsBundle\Services\LifecycleEventsDispatcher
     w3c_lifecycle_events.post_flush_listener.class: W3C\LifecycleEventsBundle\EventListener\PostFlushListener
     w3c_lifecycle_events.auto_dispatch: ''
+    w3c_lifecycle_events.annotation_getter.class: W3C\LifecycleEventsBundle\Services\AnnotationGetter
 
 services:
+    w3c_lifecycle_events.annotation_getter:
+        class: "%w3c_lifecycle_events.annotation_getter.class%"
+        arguments: ["@annotation_reader"]
     w3c_lifecycle_events.dispatcher:
         class: "%w3c_lifecycle_events.dispatcher.class%"
         arguments: ['@event_dispatcher', '%w3c_lifecycle_events.auto_dispatch%']
     w3c_lifecycle_events.listener:
         class: "%w3c_lifecycle_events.listener.class%"
         tags:
             - { name: doctrine.event_listener, event: postPersist }
-            - { name: doctrine.event_listener, event: postRemove }
-            - { name: doctrine.event_listener, event: postSoftDelete }
+            - { name: doctrine.event_listener, event: preRemove }
+            - { name: doctrine.event_listener, event: preSoftDelete }
             - { name: doctrine.event_listener, event: preUpdate }
-        arguments: ['@w3c_lifecycle_events.dispatcher', '@annotation_reader']
+        arguments: ['@w3c_lifecycle_events.dispatcher', '@w3c_lifecycle_events.annotation_getter']
     w3c_lifecycle_events.property_listener:
         class: "%w3c_lifecycle_events.property_listener.class%"
         tags:
             - { name: doctrine.event_listener, event: preUpdate }
-        arguments: ['@w3c_lifecycle_events.dispatcher', '@annotation_reader']
+        arguments: ['@w3c_lifecycle_events.dispatcher', '@w3c_lifecycle_events.annotation_getter']
     w3c_lifecycle_events.post_flush_listener:
         class: "%w3c_lifecycle_events.post_flush_listener.class%"
         tags:

diff --git a/Services/AnnotationGetter.php b/Services/AnnotationGetter.php
@@ -0,0 +1,64 @@
+<?php
+
+namespace W3C\LifecycleEventsBundle\Services;
+
+use Doctrine\Common\Annotations\Reader;
+use Doctrine\ORM\Mapping\ClassMetadata;
+
+/**
+ * Convenient class to get lifecycle annotations more easily
+ *
+ * @author Jean-Guilhem Rouel <jean-gui@w3.org>
+ */
+class AnnotationGetter
+{
+    /**
+     * @var Reader
+     */
+    private $reader;
+
+    public function __construct(Reader $reader)
+    {
+        $this->reader = $reader;
+    }
+
+    /**
+     * Get a class-level annotation
+     *
+     * @param string $class Class to get annotation of
+     * @param string $annotationClass Class of the annotation to get
+     *
+     * @return object|null object of same class as $annotationClass or null if no annotation is found
+     */
+    public function getAnnotation($class, $annotationClass)
+    {
+        $annotation = $this->reader->getClassAnnotation(
+            new \ReflectionClass($class),
+            $annotationClass
+        );
+        return $annotation;
+    }
+
+    /**
+     * Get a field-level annotation
+     *
+     * @param ClassMetadata $classMetadata Metadata of the class to get annotation of
+     * @param string $field Name of the field to get annotation of
+     * @param string $annotationClass Class of the annotation to get
+     *
+     * @return object|null object of same class as $annotationClass or null if no annotation is found
+     * @throws \ReflectionException if the field does not exist
+     */
+    public function getPropertyAnnotation(ClassMetadata $classMetadata, $field, $annotationClass)
+    {
+        $reflProperty = $classMetadata->getReflectionProperty($field);
+
+        if ($reflProperty) {
+            return $this->reader->getPropertyAnnotation($reflProperty, $annotationClass);
+        }
+
+        throw new \ReflectionException(
+            $classMetadata->getName() . '.' . $field . ' not found. Could this be a private field of a parent class?'
+        );
+    }
+}
diff --git a/Services/LifecycleEventsDispatcher.php b/Services/LifecycleEventsDispatcher.php
@@ -234,7 +234,24 @@ public function getUpdates()
 
     public function addUpdate(Update $annotation, $entity, array $propertyChangeSet = null, array $collectionChangeSet = null)
     {
-        $this->updates[] = [$annotation, $entity, $propertyChangeSet, $collectionChangeSet];
+        if (list($key, $update) = $this->getUpdate($entity)) {
+            $update[2] = array_merge_recursive((array)$update[2], (array)$propertyChangeSet);
+            $update[3] = array_merge_recursive((array)$update[3], (array)$collectionChangeSet);
+            $this->updates[$key] = $update;
+        } else {
+            $this->updates[] = [$annotation, $entity, $propertyChangeSet, $collectionChangeSet];
+        }
+    }
+
+    public function getUpdate($entity)
+    {
+        foreach ($this->updates as $key => $update) {
+            if ($update[1] === $entity) {
+                return [$key, $update];
+            }
+        }
+
+        return null;
     }
 
     public function getPropertyChanges()
@@ -252,9 +269,26 @@ public function getCollectionChanges()
         return $this->collectionChanges;
     }
 
-    public function addCollectionChange(Change $annotation, $entity, $property, $deletedElements = null, $insertedElements = null)
+    public function addCollectionChange(Change $annotation, $entity, $property, $deletedElements = [], $insertedElements = [])
     {
-        $this->collectionChanges[] = [$annotation, $entity, $property, $deletedElements, $insertedElements];
+        if (list($key, $change) = $this->getCollectionChange($entity, $property)) {
+            $change[3] = array_merge_recursive((array)$change[3], (array)$deletedElements);
+            $change[4] = array_merge_recursive((array)$change[4], (array)$insertedElements);
+            $this->collectionChanges[$key] = $change;
+        } else {
+            $this->collectionChanges[] = [$annotation, $entity, $property, $deletedElements, $insertedElements];
+        }
+    }
+
+    public function getCollectionChange($entity, $property)
+    {
+        foreach ($this->collectionChanges as $key => $update) {
+            if ($update[1] === $entity && $update[2] === $property) {
+                return [$key, $update];
+            }
+        }
+
+        return null;
     }
 
     /**

diff --git a/Tests/Annotation/Fixtures/Person.php b/Tests/Annotation/Fixtures/Person.php
@@ -0,0 +1,48 @@
+<?php
+
+namespace W3C\LifecycleEventsBundle\Tests\Annotation\Fixtures;
+
+use W3C\LifecycleEventsBundle\Annotation as On;
+
+/**
+ * @On\Update(monitor_owning=true)
+ *
+ * @author Jean-Guilhem Rouel <jean-gui@w3.org>
+ */
+class Person
+{
+    public $name;
+
+    /**
+     * @ ORM\ManyToMany(targetEntity="Person", inversedBy="friendOf")
+     */
+    public $friends;
+
+    /**
+     * @ ORM\ManyToMany(targetEntity="Person", mappedBy="friends")
+     * @On\Change(monitor_owning=true)
+     */
+    public $friendOf;
+
+    /**
+     * @ ORM\ManyToOne(targetEntity="Person", inversedBy="sons")
+     */
+    public $father;
+
+    /**
+     * @ ORM\OneToMany(targetEntity="Person", mappedBy="father")
+     * @On\Change(monitor_owning=true)
+     */
+    public $sons;
+
+    /**
+     * @ ORM\OneToOne(targetEntity="Person", inversedBy="mentoring")
+     */
+    public $mentor;
+
+    /**
+     * @ ORM\OneToOne(targetEntity="Person", mappedBy="mentor")
+     * @On\Change(monitor_owning=true)
+     */
+    public $mentoring;
+}
diff --git a/Tests/Annotation/Fixtures/PersonNoMonitor.php b/Tests/Annotation/Fixtures/PersonNoMonitor.php
@@ -0,0 +1,46 @@
+<?php
+
+namespace W3C\LifecycleEventsBundle\Tests\Annotation\Fixtures;
+
+use W3C\LifecycleEventsBundle\Annotation as On;
+
+/**
+ * @On\Update()
+ *
+ * @author Jean-Guilhem Rouel <jean-gui@w3.org>
+ */
+class PersonNoMonitor
+{
+    public $name;
+
+    /**
+     * @ ORM\ManyToMany(targetEntity="Person", inversedBy="friendOf")
+     */
+    public $friends;
+
+    /**
+     * @ ORM\ManyToMany(targetEntity="Person", mappedBy="friends")
+     */
+    public $friendOf;
+
+    /**
+     * @ ORM\ManyToOne(targetEntity="Person", inversedBy="sons")
+     * @On\Change()
+     */
+    public $father;
+
+    /**
+     * @ ORM\OneToMany(targetEntity="Person", mappedBy="father")
+     */
+    public $sons;
+
+    /**
+     * @ ORM\OneToOne(targetEntity="Person", inversedBy="mentoring")
+     */
+    public $mentor;
+
+    /**
+     * @ ORM\OneToOne(targetEntity="Person", mappedBy="mentor")
+     */
+    public $mentoring;
+}