Adjust delete logic and ann unit tests

Signed-off-by: Lukas Reschke <lukas@statuscode.ch>

2 files changed, 66 insertions, 35 deletions

diff --git a/apps/dav/lib/HookManager.php b/apps/dav/lib/HookManager.php
index f82e642c00d..03728eaf59f 100644
--- a/apps/dav/lib/HookManager.php
+++ b/apps/dav/lib/HookManager.php
@@ -40,12 +40,6 @@ class HookManager {
 	/** @var IUser[] */
 	private $usersToDelete;
 
-	/** @var array */
-	private $calendarsToDelete;
-
-	/** @var array */
-	private $addressBooksToDelete;
-
 	/** @var CalDavBackend */
 	private $calDav;
 
@@ -91,12 +85,7 @@ class HookManager {
 	}
 
 	public function preDeleteUser($params) {
-		$user = $this->userManager->get($params['uid']);
-
-		$this->usersToDelete[$params['uid']] = $user;
-
-		$this->calendarsToDelete = $this->calDav->getCalendarsForUser('principals/users/' . $user->getUID());
-		$this->addressBooksToDelete = $this->cardDav->getAddressBooksForUser('principals/users/' . $user->getUID());
+		$this->usersToDelete[$params['uid']] = $this->userManager->get($params['uid']);
 	}
 
 	public function postDeleteUser($params) {
@@ -104,15 +93,15 @@ class HookManager {
 		if (isset($this->usersToDelete[$uid])){
 			$this->syncService->deleteUser($this->usersToDelete[$uid]);
 		}
-		if (!is_null($this->calendarsToDelete)) {
-			foreach ($this->calendarsToDelete as $calendar) {
-				$this->calDav->deleteCalendar($calendar['id']);
-			}
+
+		$calendarsToDelete = $this->calDav->getCalendarsForUser('principals/users/' . $uid);
+		foreach ($calendarsToDelete as $calendar) {
+			$this->calDav->deleteCalendar($calendar['id']);
 		}
-		if (!is_null($this->addressBooksToDelete)) {
-			foreach ($this->addressBooksToDelete as $addressBook) {
-				$this->cardDav->deleteAddressBook($addressBook['id']);
-			}
+
+		$addressBooksToDelete = $this->cardDav->getAddressBooksForUser('principals/users/' . $uid);
+		foreach ($addressBooksToDelete as $addressBook) {
+			$this->cardDav->deleteAddressBook($addressBook['id']);
 		}
 	}
 
diff --git a/apps/dav/tests/unit/DAV/HookManagerTest.php b/apps/dav/tests/unit/DAV/HookManagerTest.php
index 0db66bf76c1..05384813f02 100644
--- a/apps/dav/tests/unit/DAV/HookManagerTest.php
+++ b/apps/dav/tests/unit/DAV/HookManagerTest.php
@@ -28,29 +28,30 @@ use OCA\DAV\CalDAV\CalDavBackend;
 use OCA\DAV\CardDAV\CardDavBackend;
 use OCA\DAV\CardDAV\SyncService;
 use OCA\DAV\HookManager;
+use OCP\IUser;
 use OCP\IUserManager;
 use Test\TestCase;
 
 class HookManagerTest extends TestCase {
 	public function test() {
-		$user = $this->getMockBuilder('\OCP\IUser')
+		$user = $this->getMockBuilder(IUser::class)
 			->disableOriginalConstructor()
 			->getMock();
 		$user->expects($this->once())->method('getUID')->willReturn('newUser');
 
 		/** @var IUserManager | \PHPUnit_Framework_MockObject_MockObject $userManager */
-		$userManager = $this->getMockBuilder('\OCP\IUserManager')
+		$userManager = $this->getMockBuilder(IUserManager::class)
 			->disableOriginalConstructor()
 			->getMock();
 		$userManager->expects($this->once())->method('get')->willReturn($user);
 
 		/** @var SyncService | \PHPUnit_Framework_MockObject_MockObject $syncService */
-		$syncService = $this->getMockBuilder('OCA\DAV\CardDAV\SyncService')
+		$syncService = $this->getMockBuilder(SyncService::class)
 			->disableOriginalConstructor()
 			->getMock();
 
 		/** @var CalDavBackend | \PHPUnit_Framework_MockObject_MockObject $cal */
-		$cal = $this->getMockBuilder('OCA\DAV\CalDAV\CalDavBackend')
+		$cal = $this->getMockBuilder(CalDavBackend::class)
 			->disableOriginalConstructor()
 			->getMock();
 		$cal->expects($this->once())->method('getCalendarsForUserCount')->willReturn(0);
@@ -59,7 +60,7 @@ class HookManagerTest extends TestCase {
 			'personal', ['{DAV:}displayname' => 'Personal']);
 
 		/** @var CardDavBackend | \PHPUnit_Framework_MockObject_MockObject $card */
-		$card = $this->getMockBuilder('OCA\DAV\CardDAV\CardDavBackend')
+		$card = $this->getMockBuilder(CardDavBackend::class)
 			->disableOriginalConstructor()
 			->getMock();
 		$card->expects($this->once())->method('getAddressBooksForUserCount')->willReturn(0);
@@ -72,31 +73,31 @@ class HookManagerTest extends TestCase {
 	}
 
 	public function testWithExisting() {
-		$user = $this->getMockBuilder('\OCP\IUser')
+		$user = $this->getMockBuilder(IUser::class)
 			->disableOriginalConstructor()
 			->getMock();
 		$user->expects($this->once())->method('getUID')->willReturn('newUser');
 
 		/** @var IUserManager | \PHPUnit_Framework_MockObject_MockObject $userManager */
-		$userManager = $this->getMockBuilder('\OCP\IUserManager')
+		$userManager = $this->getMockBuilder(IUserManager::class)
 			->disableOriginalConstructor()
 			->getMock();
 		$userManager->expects($this->once())->method('get')->willReturn($user);
 
 		/** @var SyncService | \PHPUnit_Framework_MockObject_MockObject $syncService */
-		$syncService = $this->getMockBuilder('OCA\DAV\CardDAV\SyncService')
+		$syncService = $this->getMockBuilder(SyncService::class)
 			->disableOriginalConstructor()
 			->getMock();
 
 		/** @var CalDavBackend | \PHPUnit_Framework_MockObject_MockObject $cal */
-		$cal = $this->getMockBuilder('OCA\DAV\CalDAV\CalDavBackend')
+		$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 */
-		$card = $this->getMockBuilder('OCA\DAV\CardDAV\CardDavBackend')
+		$card = $this->getMockBuilder(CardDavBackend::class)
 			->disableOriginalConstructor()
 			->getMock();
 		$card->expects($this->once())->method('getAddressBooksForUserCount')->willReturn(1);
@@ -107,24 +108,24 @@ class HookManagerTest extends TestCase {
 	}
 
 	public function testWithBirthdayCalendar() {
-		$user = $this->getMockBuilder('\OCP\IUser')
+		$user = $this->getMockBuilder(IUser::class)
 			->disableOriginalConstructor()
 			->getMock();
 		$user->expects($this->once())->method('getUID')->willReturn('newUser');
 
 		/** @var IUserManager | \PHPUnit_Framework_MockObject_MockObject $userManager */
-		$userManager = $this->getMockBuilder('\OCP\IUserManager')
+		$userManager = $this->getMockBuilder(IUserManager::class)
 			->disableOriginalConstructor()
 			->getMock();
 		$userManager->expects($this->once())->method('get')->willReturn($user);
 
 		/** @var SyncService | \PHPUnit_Framework_MockObject_MockObject $syncService */
-		$syncService = $this->getMockBuilder('OCA\DAV\CardDAV\SyncService')
+		$syncService = $this->getMockBuilder(SyncService::class)
 			->disableOriginalConstructor()
 			->getMock();
 
 		/** @var CalDavBackend | \PHPUnit_Framework_MockObject_MockObject $cal */
-		$cal = $this->getMockBuilder('OCA\DAV\CalDAV\CalDavBackend')
+		$cal = $this->getMockBuilder(CalDavBackend::class)
 			->disableOriginalConstructor()
 			->getMock();
 		$cal->expects($this->once())->method('getCalendarsForUserCount')->willReturn(0);
@@ -133,7 +134,7 @@ class HookManagerTest extends TestCase {
 			'personal', ['{DAV:}displayname' => 'Personal']);
 
 		/** @var CardDavBackend | \PHPUnit_Framework_MockObject_MockObject $card */
-		$card = $this->getMockBuilder('OCA\DAV\CardDAV\CardDavBackend')
+		$card = $this->getMockBuilder(CardDavBackend::class)
 			->disableOriginalConstructor()
 			->getMock();
 		$card->expects($this->once())->method('getAddressBooksForUserCount')->willReturn(0);
@@ -144,4 +145,45 @@ class HookManagerTest extends TestCase {
 		$hm = new HookManager($userManager, $syncService, $cal, $card);
 		$hm->postLogin(['uid' => 'newUser']);
 	}
+
+	public function testDeleteCalendar() {
+		$user = $this->getMockBuilder(IUser::class)
+			->disableOriginalConstructor()
+			->getMock();
+
+		/** @var IUserManager | \PHPUnit_Framework_MockObject_MockObject $userManager */
+		$userManager = $this->getMockBuilder(IUserManager::class)
+			->disableOriginalConstructor()
+			->getMock();
+		$userManager->expects($this->once())->method('get')->willReturn($user);
+
+		/** @var SyncService | \PHPUnit_Framework_MockObject_MockObject $syncService */
+		$syncService = $this->getMockBuilder(SyncService::class)
+			->disableOriginalConstructor()
+			->getMock();
+		$syncService->expects($this->once())
+			->method('deleteUser');
+
+		/** @var CalDavBackend | \PHPUnit_Framework_MockObject_MockObject $cal */
+		$cal = $this->getMockBuilder(CalDavBackend::class)
+			->disableOriginalConstructor()
+			->getMock();
+		$cal->expects($this->once())->method('getCalendarsForUser')->willReturn([
+			['id' => 'personal']
+		]);
+		$cal->expects($this->once())->method('deleteCalendar');
+
+		/** @var CardDavBackend | \PHPUnit_Framework_MockObject_MockObject $card */
+		$card = $this->getMockBuilder(CardDavBackend::class)
+			->disableOriginalConstructor()
+			->getMock();
+		$card->expects($this->once())->method('getAddressBooksForUser')->willReturn([
+			['id' => 'personal']
+		]);
+		$card->expects($this->once())->method('deleteAddressBook');
+
+		$hm = new HookManager($userManager, $syncService, $cal, $card, $this->l10n);
+		$hm->preDeleteUser(['uid' => 'newUser']);
+		$hm->postDeleteUser(['uid' => 'newUser']);
+	}
 }