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>

8 files changed, 40 insertions, 49 deletions

diff --git a/apps/comments/lib/EventHandler.php b/apps/comments/lib/EventHandler.php
index 6692ccc520d..a5f312617e5 100644
--- 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 5e979fd9bf0..68705085023 100644
--- 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 21b701ea8cc..f377c01b3c9 100644
--- 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 98a0375e49a..5926264fa08 100644
--- 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 101e3d87721..f247921be79 100644
--- 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 f7b23dd5f58..b3ecab731e1 100644
--- 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 a0bff349fb4..0d8a783c107 100644
--- 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 9c0f791c0ca..71c918af6c7 100644
--- 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();