From 5b7b8f8dac7d2d1e6fda6526628557ce7233257d Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 28 Sep 2016 13:59:50 +0200 Subject: [PATCH 1/3] Remove notifications upon user deletion Signed-off-by: Joas Schilling --- lib/private/User/User.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/private/User/User.php b/lib/private/User/User.php index 94ac8c1362..11c0cc8649 100644 --- a/lib/private/User/User.php +++ b/lib/private/User/User.php @@ -214,6 +214,10 @@ class User implements IUser { \OC::$server->getCommentsManager()->deleteReferencesOfActor('users', $this->uid); \OC::$server->getCommentsManager()->deleteReadMarksFromUser($this); + + $notification = \OC::$server->getNotificationManager()->createNotification(); + $notification->setUser($this->uid); + \OC::$server->getNotificationManager()->markProcessed($notification); } if ($this->emitter) { From f6ff60f4cb3800e5e7937fc18d62b99342fb7727 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Thu, 29 Sep 2016 15:01:38 +0200 Subject: [PATCH 2/3] Make sure that comments, notifications and preferences are deleted Signed-off-by: Joas Schilling --- tests/lib/User/UserTest.php | 85 ++++++++++++++++++++++++++++++++----- 1 file changed, 74 insertions(+), 11 deletions(-) diff --git a/tests/lib/User/UserTest.php b/tests/lib/User/UserTest.php index 69f75d0776..19afa3642d 100644 --- a/tests/lib/User/UserTest.php +++ b/tests/lib/User/UserTest.php @@ -11,6 +11,10 @@ namespace Test\User; use OC\Hooks\PublicEmitter; use OC\User\Database; +use OCP\Comments\ICommentsManager; +use OCP\IConfig; +use OCP\Notification\IManager as INotificationManager; +use OCP\Notification\INotification; /** * Class UserTest @@ -175,9 +179,7 @@ class UserTest extends \Test\TestCase { $backend->expects($this->any()) ->method('implementsActions') - ->will($this->returnCallback(function ($actions) { - return false; - })); + ->willReturn(false); $user = new \OC\User\User('foo', $backend); $this->assertTrue($user->canChangeAvatar()); @@ -380,9 +382,7 @@ class UserTest extends \Test\TestCase { $backend->expects($this->any()) ->method('implementsActions') - ->will($this->returnCallback(function ($actions) { - return false; - })); + ->willReturn(false); $backend->expects($this->never()) ->method('setDisplayName'); @@ -433,7 +433,18 @@ class UserTest extends \Test\TestCase { $this->assertEquals(2, $hooksCalled); } - public function testDeleteHooks() { + public function dataDeleteHooks() { + return [ + [true], + [false], + ]; + } + + /** + * @dataProvider dataDeleteHooks + * @param bool $result + */ + public function testDeleteHooks($result) { $hooksCalled = 0; $test = $this; @@ -442,7 +453,10 @@ class UserTest extends \Test\TestCase { */ $backend = $this->createMock(\Test\Util\User\Dummy::class); $backend->expects($this->once()) - ->method('deleteUser'); + ->method('deleteUser') + ->willReturn($result); + $emitter = new PublicEmitter(); + $user = new \OC\User\User('foo', $backend, $emitter); /** * @param \OC\User\User $user @@ -452,12 +466,61 @@ class UserTest extends \Test\TestCase { $test->assertEquals('foo', $user->getUID()); }; - $emitter = new PublicEmitter(); $emitter->listen('\OC\User', 'preDelete', $hook); $emitter->listen('\OC\User', 'postDelete', $hook); - $user = new \OC\User\User('foo', $backend, $emitter); - $this->assertTrue($user->delete()); + $config = $this->createMock(IConfig::class); + $commentsManager = $this->createMock(ICommentsManager::class); + $notificationManager = $this->createMock(INotificationManager::class); + + if ($result) { + $config->expects($this->once()) + ->method('deleteAllUserValues') + ->with('foo'); + + $commentsManager->expects($this->once()) + ->method('deleteReferencesOfActor') + ->with('users', 'foo'); + $commentsManager->expects($this->once()) + ->method('deleteReadMarksFromUser') + ->with($user); + + $notification = $this->createMock(INotification::class); + $notification->expects($this->once()) + ->method('setUser') + ->with('foo'); + + $notificationManager->expects($this->once()) + ->method('createNotification') + ->willReturn($notification); + $notificationManager->expects($this->once()) + ->method('markProcessed') + ->with($notification); + } else { + $config->expects($this->never()) + ->method('deleteAllUserValues'); + + $commentsManager->expects($this->never()) + ->method('deleteReferencesOfActor'); + $commentsManager->expects($this->never()) + ->method('deleteReadMarksFromUser'); + + $notificationManager->expects($this->never()) + ->method('createNotification'); + $notificationManager->expects($this->never()) + ->method('markProcessed'); + } + + $this->overwriteService('NotificationManager', $notificationManager); + $this->overwriteService('CommentsManager', $commentsManager); + $this->overwriteService('AllConfig', $config); + + $this->assertSame($result, $user->delete()); + + $this->restoreService('AllConfig'); + $this->restoreService('CommentsManager'); + $this->restoreService('NotificationManager'); + $this->assertEquals(2, $hooksCalled); } From 4d1acfd4ef7087b8dc7795396c75fd239a6245f4 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Thu, 29 Sep 2016 15:05:47 +0200 Subject: [PATCH 3/3] Only trigger postDelete hooks when the user was deleted... Signed-off-by: Joas Schilling --- lib/private/User/User.php | 6 +++--- tests/lib/User/UserTest.php | 9 +++++---- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/lib/private/User/User.php b/lib/private/User/User.php index 11c0cc8649..68787ce60f 100644 --- a/lib/private/User/User.php +++ b/lib/private/User/User.php @@ -218,10 +218,10 @@ class User implements IUser { $notification = \OC::$server->getNotificationManager()->createNotification(); $notification->setUser($this->uid); \OC::$server->getNotificationManager()->markProcessed($notification); - } - if ($this->emitter) { - $this->emitter->emit('\OC\User', 'postDelete', array($this)); + if ($this->emitter) { + $this->emitter->emit('\OC\User', 'postDelete', array($this)); + } } return !($result === false); } diff --git a/tests/lib/User/UserTest.php b/tests/lib/User/UserTest.php index 19afa3642d..5eee5d60d0 100644 --- a/tests/lib/User/UserTest.php +++ b/tests/lib/User/UserTest.php @@ -435,16 +435,17 @@ class UserTest extends \Test\TestCase { public function dataDeleteHooks() { return [ - [true], - [false], + [true, 2], + [false, 1], ]; } /** * @dataProvider dataDeleteHooks * @param bool $result + * @param int $expectedHooks */ - public function testDeleteHooks($result) { + public function testDeleteHooks($result, $expectedHooks) { $hooksCalled = 0; $test = $this; @@ -521,7 +522,7 @@ class UserTest extends \Test\TestCase { $this->restoreService('CommentsManager'); $this->restoreService('NotificationManager'); - $this->assertEquals(2, $hooksCalled); + $this->assertEquals($expectedHooks, $hooksCalled); } public function testGetCloudId() {