Delete calendar subscriptions as well when deleting user

Closes #28418 Signed-off-by: Thomas Citharel <[email protected]>
nextcloud · Nov 27, 2021 · 9f799ac · 9f799ac
1 parent 0b3d763
commit 9f799ac
Show file tree

Hide file tree

Showing 2 changed files with 36 additions and 20 deletions.
diff --git 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
@@ -37,14 +37,15 @@
 use OCP\IL10N;
 use OCP\IUser;
 use OCP\IUserManager;
+use PHPUnit\Framework\MockObject\MockObject;
 use Symfony\Component\EventDispatcher\EventDispatcherInterface;
 use Test\TestCase;
 
 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();