From 1bcd2ca8e35dca6e68e5f06506ade0a78a2beae8 Mon Sep 17 00:00:00 2001
From: Arthur Schiwon <blizzz@arthur-schiwon.de>
Date: Wed, 12 Oct 2016 18:06:22 +0200
Subject: [PATCH] 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>
---
apps/comments/lib/EventHandler.php | 27 ++++++++++----
apps/comments/lib/Notification/Listener.php | 4 +-
apps/comments/tests/Unit/EventHandlerTest.php | 37 ++-----------------
.../tests/Unit/Notification/ListenerTest.php | 2 +
apps/dav/lib/Comments/CommentNode.php | 2 +-
lib/private/Comments/Manager.php | 6 +++
lib/public/Comments/CommentsEvent.php | 7 ++--
tests/lib/Comments/ManagerTest.php | 4 +-
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();
--
GitLab