From 9f799accff108435b2cddab356a9a952402c20be Mon Sep 17 00:00:00 2001 From: Thomas Citharel Date: Fri, 13 Aug 2021 14:19:03 +0200 Subject: [PATCH] Delete calendar subscriptions as well when deleting user Closes #28418 Signed-off-by: Thomas Citharel --- apps/dav/lib/HookManager.php | 15 +++++++- apps/dav/tests/unit/DAV/HookManagerTest.php | 41 ++++++++++++--------- 2 files changed, 36 insertions(+), 20 deletions(-) diff --git a/apps/dav/lib/HookManager.php b/apps/dav/lib/HookManager.php index 3929fdabbd04c..02c7291c68904 100644 --- a/apps/dav/lib/HookManager.php +++ b/apps/dav/lib/HookManager.php @@ -56,6 +56,9 @@ class HookManager { /** @var array */ private $calendarsToDelete = []; + /** @var array */ + private $subscriptionsToDelete = []; + /** @var array */ private $addressBooksToDelete = []; @@ -110,9 +113,11 @@ public function postCreateUser($params) { public function preDeleteUser($params) { $uid = $params['uid']; + $userPrincipalUri = 'principals/users/' . $uid; $this->usersToDelete[$uid] = $this->userManager->get($uid); - $this->calendarsToDelete = $this->calDav->getUsersOwnCalendars('principals/users/' . $uid); - $this->addressBooksToDelete = $this->cardDav->getUsersOwnAddressBooks('principals/users/' . $uid); + $this->calendarsToDelete = $this->calDav->getUsersOwnCalendars($userPrincipalUri); + $this->subscriptionsToDelete = $this->calDav->getSubscriptionsForUser($userPrincipalUri); + $this->addressBooksToDelete = $this->cardDav->getUsersOwnAddressBooks($userPrincipalUri); } public function preUnassignedUserId($uid) { @@ -131,6 +136,12 @@ public function postDeleteUser($params) { true // Make sure the data doesn't go into the trashbin, a new user with the same UID would later see it otherwise ); } + + foreach ($this->subscriptionsToDelete as $subscription) { + $this->calDav->deleteSubscription( + $subscription['id'], + ); + } $this->calDav->deleteAllSharesByUser('principals/users/' . $uid); foreach ($this->addressBooksToDelete as $addressBook) { diff --git a/apps/dav/tests/unit/DAV/HookManagerTest.php b/apps/dav/tests/unit/DAV/HookManagerTest.php index d36db8f90c860..ae8f9e9b857dd 100644 --- a/apps/dav/tests/unit/DAV/HookManagerTest.php +++ b/apps/dav/tests/unit/DAV/HookManagerTest.php @@ -37,6 +37,7 @@ use OCP\IL10N; use OCP\IUser; use OCP\IUserManager; +use PHPUnit\Framework\MockObject\MockObject; use Symfony\Component\EventDispatcher\EventDispatcherInterface; use Test\TestCase; @@ -44,7 +45,7 @@ class HookManagerTest extends TestCase { /** @var IL10N */ private $l10n; - /** @var EventDispatcherInterface | \PHPUnit\Framework\MockObject\MockObject */ + /** @var EventDispatcherInterface | MockObject */ private $eventDispatcher; protected function setUp(): void { @@ -65,17 +66,17 @@ public function test() { ->getMock(); $user->expects($this->once())->method('getUID')->willReturn('newUser'); - /** @var IUserManager | \PHPUnit\Framework\MockObject\MockObject $userManager */ + /** @var IUserManager | MockObject $userManager */ $userManager = $this->getMockBuilder(IUserManager::class) ->disableOriginalConstructor() ->getMock(); - /** @var SyncService | \PHPUnit\Framework\MockObject\MockObject $syncService */ + /** @var SyncService | MockObject $syncService */ $syncService = $this->getMockBuilder(SyncService::class) ->disableOriginalConstructor() ->getMock(); - /** @var CalDavBackend | \PHPUnit\Framework\MockObject\MockObject $cal */ + /** @var CalDavBackend | MockObject $cal */ $cal = $this->getMockBuilder(CalDavBackend::class) ->disableOriginalConstructor() ->getMock(); @@ -84,7 +85,7 @@ public function test() { 'principals/users/newUser', 'personal', ['{DAV:}displayname' => 'Personal']); - /** @var CardDavBackend | \PHPUnit\Framework\MockObject\MockObject $card */ + /** @var CardDavBackend | MockObject $card */ $card = $this->getMockBuilder(CardDavBackend::class) ->disableOriginalConstructor() ->getMock(); @@ -103,24 +104,24 @@ public function testWithExisting() { ->getMock(); $user->expects($this->once())->method('getUID')->willReturn('newUser'); - /** @var IUserManager | \PHPUnit\Framework\MockObject\MockObject $userManager */ + /** @var IUserManager | MockObject $userManager */ $userManager = $this->getMockBuilder(IUserManager::class) ->disableOriginalConstructor() ->getMock(); - /** @var SyncService | \PHPUnit\Framework\MockObject\MockObject $syncService */ + /** @var SyncService | MockObject $syncService */ $syncService = $this->getMockBuilder(SyncService::class) ->disableOriginalConstructor() ->getMock(); - /** @var CalDavBackend | \PHPUnit\Framework\MockObject\MockObject $cal */ + /** @var CalDavBackend | MockObject $cal */ $cal = $this->getMockBuilder(CalDavBackend::class) ->disableOriginalConstructor() ->getMock(); $cal->expects($this->once())->method('getCalendarsForUserCount')->willReturn(1); $cal->expects($this->never())->method('createCalendar'); - /** @var CardDavBackend | \PHPUnit\Framework\MockObject\MockObject $card */ + /** @var CardDavBackend | MockObject $card */ $card = $this->getMockBuilder(CardDavBackend::class) ->disableOriginalConstructor() ->getMock(); @@ -137,17 +138,17 @@ public function testWithBirthdayCalendar() { ->getMock(); $user->expects($this->once())->method('getUID')->willReturn('newUser'); - /** @var IUserManager | \PHPUnit\Framework\MockObject\MockObject $userManager */ + /** @var IUserManager | MockObject $userManager */ $userManager = $this->getMockBuilder(IUserManager::class) ->disableOriginalConstructor() ->getMock(); - /** @var SyncService | \PHPUnit\Framework\MockObject\MockObject $syncService */ + /** @var SyncService | MockObject $syncService */ $syncService = $this->getMockBuilder(SyncService::class) ->disableOriginalConstructor() ->getMock(); - /** @var CalDavBackend | \PHPUnit\Framework\MockObject\MockObject $cal */ + /** @var CalDavBackend | MockObject $cal */ $cal = $this->getMockBuilder(CalDavBackend::class) ->disableOriginalConstructor() ->getMock(); @@ -156,7 +157,7 @@ public function testWithBirthdayCalendar() { 'principals/users/newUser', 'personal', ['{DAV:}displayname' => 'Personal']); - /** @var CardDavBackend | \PHPUnit\Framework\MockObject\MockObject $card */ + /** @var CardDavBackend | MockObject $card */ $card = $this->getMockBuilder(CardDavBackend::class) ->disableOriginalConstructor() ->getMock(); @@ -174,30 +175,34 @@ public function testDeleteCalendar() { ->disableOriginalConstructor() ->getMock(); - /** @var IUserManager | \PHPUnit\Framework\MockObject\MockObject $userManager */ + /** @var IUserManager | MockObject $userManager */ $userManager = $this->getMockBuilder(IUserManager::class) ->disableOriginalConstructor() ->getMock(); $userManager->expects($this->once())->method('get')->willReturn($user); - /** @var SyncService | \PHPUnit\Framework\MockObject\MockObject $syncService */ + /** @var SyncService | MockObject $syncService */ $syncService = $this->getMockBuilder(SyncService::class) ->disableOriginalConstructor() ->getMock(); $syncService->expects($this->once()) ->method('deleteUser'); - /** @var CalDavBackend | \PHPUnit\Framework\MockObject\MockObject $cal */ + /** @var CalDavBackend | MockObject $cal */ $cal = $this->getMockBuilder(CalDavBackend::class) ->disableOriginalConstructor() ->getMock(); $cal->expects($this->once())->method('getUsersOwnCalendars')->willReturn([ ['id' => 'personal'] ]); - $cal->expects($this->once())->method('deleteCalendar'); + $cal->expects($this->once())->method('getSubscriptionsForUser')->willReturn([ + ['id' => 'some-subscription'] + ]); + $cal->expects($this->once())->method('deleteCalendar')->with('personal'); + $cal->expects($this->once())->method('deleteSubscription')->with('some-subscription'); $cal->expects($this->once())->method('deleteAllSharesByUser'); - /** @var CardDavBackend | \PHPUnit\Framework\MockObject\MockObject $card */ + /** @var CardDavBackend | MockObject $card */ $card = $this->getMockBuilder(CardDavBackend::class) ->disableOriginalConstructor() ->getMock();