emit pre-update event for comments

* notifications can be cleaned up, no polluted DB
* updating comments will re-notify users or remove notifications, depending on the message

Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>

diff --git a/apps/comments/lib/EventHandler.php b/apps/comments/lib/EventHandler.php
index 6692ccc520d86c085f8a75a1ae205538497d0392..a5f312617e5113628e8c312029e889f1b34e24e8 100644 (file)
--- a/apps/comments/lib/EventHandler.php
+++ b/apps/comments/lib/EventHandler.php
@@ -57,14 +57,29 @@ class EventHandler implements ICommentsEventHandler {
                if( $eventType === CommentsEvent::EVENT_ADD
                        && $event instanceof CommentsEvent
                ) {
-                       $this->onAdd($event);
+                       $this->notificationHandler($event);
+                       $this->activityHandler($event);
+                       return;
+               }
+
+               if( $eventType === CommentsEvent::EVENT_PRE_UPDATE
+                       && $event instanceof CommentsEvent
+               ) {
+                       $this->notificationHandler($event);
+                       return;
+               }
+
+               if( $eventType === CommentsEvent::EVENT_UPDATE
+                       && $event instanceof CommentsEvent
+               ) {
+                       $this->notificationHandler($event);
                        return;
                }
 
                if( $eventType === CommentsEvent::EVENT_DELETE
                        && $event instanceof CommentsEvent
                ) {
-                       $this->onDelete($event);
+                       $this->notificationHandler($event);
                        return;
                }
        }
@@ -72,13 +87,9 @@ class EventHandler implements ICommentsEventHandler {
        /**
         * @param CommentsEvent $event
         */
-       private function onAdd(CommentsEvent $event) {
+       private function activityHandler(CommentsEvent $event) {
                $c = $this->app->getContainer();
 
-               /** @var NotificationListener $notificationListener */
-               $notificationListener = $c->query(NotificationListener::class);
-               $notificationListener->evaluate($event);
-
                /** @var ActivityListener $listener */
                $activityListener = $c->query(ActivityListener::class);
                $activityListener->commentEvent($event);
@@ -87,7 +98,7 @@ class EventHandler implements ICommentsEventHandler {
        /**
         * @param CommentsEvent $event
         */
-       private function onDelete(CommentsEvent $event) {
+       private function notificationHandler(CommentsEvent $event) {
                $c = $this->app->getContainer();
 
                /** @var NotificationListener $notificationListener */

diff --git a/apps/comments/lib/Notification/Listener.php b/apps/comments/lib/Notification/Listener.php
index 5e979fd9bf0bd9f22a451007c0b6f4561e23ad95..68705085023846c847a1c526d2939ba05655a2ac 100644 (file)
--- a/apps/comments/lib/Notification/Listener.php
+++ b/apps/comments/lib/Notification/Listener.php
@@ -85,7 +85,9 @@ class Listener {
                        }
 
                        $notification->setUser($user);
-                       if($event->getEvent() === CommentsEvent::EVENT_DELETE) {
+                       if(    $event->getEvent() === CommentsEvent::EVENT_DELETE
+                               || $event->getEvent() === CommentsEvent::EVENT_PRE_UPDATE)
+                       {
                                $this->notificationManager->markProcessed($notification);
                        } else {
                                $this->notificationManager->notify($notification);

diff --git a/apps/comments/tests/Unit/EventHandlerTest.php b/apps/comments/tests/Unit/EventHandlerTest.php
index 21b701ea8ccadfd3bba4a4f059f0dbfaf3630462..f377c01b3c9b75dc4917b1ef622a01490bdf04a6 100644 (file)
--- a/apps/comments/tests/Unit/EventHandlerTest.php
+++ b/apps/comments/tests/Unit/EventHandlerTest.php
@@ -69,42 +69,11 @@ class EventHandlerTest extends TestCase {
                $this->eventHandler->handle($event);
        }
 
-       public function notHandledProvider() {
-               return [
-                       [CommentsEvent::EVENT_UPDATE]
-               ];
-       }
-
-       /**
-        * @dataProvider notHandledProvider
-        * @param string $eventType
-        */
-       public function testNotHandled($eventType) {
-               /** @var IComment|\PHPUnit_Framework_MockObject_MockObject $comment */
-               $comment = $this->getMockBuilder(IComment::class)->getMock();
-               $comment->expects($this->once())
-                       ->method('getObjectType')
-                       ->willReturn('files');
-
-               /** @var CommentsEvent|\PHPUnit_Framework_MockObject_MockObject $event */
-               $event = $this->getMockBuilder(CommentsEvent::class)
-                       ->disableOriginalConstructor()
-                       ->getMock();
-               $event->expects($this->once())
-                       ->method('getComment')
-                       ->willReturn($comment);
-               $event->expects($this->once())
-                       ->method('getEvent')
-                       ->willReturn($eventType);
-
-               // further processing does not happen, because $event methods cannot be
-               // access more than once.
-               $this->eventHandler->handle($event);
-       }
-
        public function handledProvider() {
                return [
                        [CommentsEvent::EVENT_DELETE],
+                       [CommentsEvent::EVENT_UPDATE],
+                       [CommentsEvent::EVENT_PRE_UPDATE],
                        [CommentsEvent::EVENT_ADD]
                ];
        }
@@ -152,7 +121,7 @@ class EventHandlerTest extends TestCase {
                        ->withConsecutive([NotificationListener::class], [ActivityListener::class])
                        ->willReturnOnConsecutiveCalls($notificationListener, $activityListener);
 
-               $this->app->expects($this->once())
+               $this->app->expects($this->atLeastOnce())
                        ->method('getContainer')
                        ->willReturn($c);
 

diff --git a/apps/comments/tests/Unit/Notification/ListenerTest.php b/apps/comments/tests/Unit/Notification/ListenerTest.php
index 98a0375e49ae8406a204827daeb9bb0029a99ab5..5926264fa08ae9650356b6071f7e604c0fede88c 100644 (file)
--- a/apps/comments/tests/Unit/Notification/ListenerTest.php
+++ b/apps/comments/tests/Unit/Notification/ListenerTest.php
@@ -60,6 +60,8 @@ class ListenerTest extends TestCase {
        public function eventProvider() {
                return [
                        [CommentsEvent::EVENT_ADD, 'notify'],
+                       [CommentsEvent::EVENT_UPDATE, 'notify'],
+                       [CommentsEvent::EVENT_PRE_UPDATE, 'markProcessed'],
                        [CommentsEvent::EVENT_DELETE, 'markProcessed']
                ];
        }

diff --git a/apps/dav/lib/Comments/CommentNode.php b/apps/dav/lib/Comments/CommentNode.php
index 101e3d8772175d76b0ec7ab1c51671e2de569dd9..f247921be792f8e0f0ef237dea11eb14cb257dfc 100644 (file)
--- a/apps/dav/lib/Comments/CommentNode.php
+++ b/apps/dav/lib/Comments/CommentNode.php
@@ -173,7 +173,7 @@ class CommentNode implements \Sabre\DAV\INode, \Sabre\DAV\IProperties {
         * @param $propertyValue
         * @return bool
         * @throws BadRequest
-        * @throws Forbidden
+        * @throws \Exception
         */
        public function updateComment($propertyValue) {
                $this->checkWriteAccessOnComment();

diff --git a/lib/private/Comments/Manager.php b/lib/private/Comments/Manager.php
index f7b23dd5f580d9c06a260bced8f5a1d59b34a4a9..b3ecab731e1825d93441cd87bc4721d5715e107a 100644 (file)
--- a/lib/private/Comments/Manager.php
+++ b/lib/private/Comments/Manager.php
@@ -536,6 +536,12 @@ class Manager implements ICommentsManager {
         * @throws NotFoundException
         */
        protected function update(IComment $comment) {
+               // for properly working preUpdate Events we need the old comments as is
+               // in the DB and overcome caching. Also avoid that outdated information stays.
+               $this->uncache($comment->getId());
+               $this->sendEvent(CommentsEvent::EVENT_PRE_UPDATE, $this->get($comment->getId()));
+               $this->uncache($comment->getId());
+
                $qb = $this->dbConn->getQueryBuilder();
                $affectedRows = $qb
                        ->update('comments')

diff --git a/lib/public/Comments/CommentsEvent.php b/lib/public/Comments/CommentsEvent.php
index a0bff349fb4edd1e02c1d85228766e010105e16a..0d8a783c107fd46d27eb53ab637f2e962aeadc04 100644 (file)
--- a/lib/public/Comments/CommentsEvent.php
+++ b/lib/public/Comments/CommentsEvent.php
@@ -32,9 +32,10 @@ use Symfony\Component\EventDispatcher\Event;
  */
 class CommentsEvent extends Event {
 
-       const EVENT_ADD = 'OCP\Comments\ICommentsManager::addComment';
-       const EVENT_UPDATE = 'OCP\Comments\ICommentsManager::updateComment';
-       const EVENT_DELETE = 'OCP\Comments\ICommentsManager::deleteComment';
+       const EVENT_ADD        = 'OCP\Comments\ICommentsManager::addComment';
+       const EVENT_PRE_UPDATE = 'OCP\Comments\ICommentsManager::preUpdateComment';
+       const EVENT_UPDATE     = 'OCP\Comments\ICommentsManager::updateComment';
+       const EVENT_DELETE     = 'OCP\Comments\ICommentsManager::deleteComment';
 
        /** @var string */
        protected $event;

diff --git a/tests/lib/Comments/ManagerTest.php b/tests/lib/Comments/ManagerTest.php
index 9c0f791c0ca0057d3baf625aea31d8f507e1ae81..71c918af6c7deb85ae4b1455990dd12f052ed0f1 100644 (file)
--- a/tests/lib/Comments/ManagerTest.php
+++ b/tests/lib/Comments/ManagerTest.php
@@ -635,11 +635,11 @@ class ManagerTest extends TestCase {
 
        public function testSendEvent() {
                $handler1 = $this->getMockBuilder(ICommentsEventHandler::class)->getMock();
-               $handler1->expects($this->exactly(3))
+               $handler1->expects($this->exactly(4))
                        ->method('handle');
 
                $handler2 = $this->getMockBuilder(ICommentsEventHandler::class)->getMock();
-               $handler1->expects($this->exactly(3))
+               $handler1->expects($this->exactly(4))
                        ->method('handle');
 
                $manager = $this->getManager();