From db633ac2f8ad5b2d9a41b6d33db16cd56e9c412e Mon Sep 17 00:00:00 2001 From: Hamza Date: Mon, 15 Jun 2026 15:36:01 +0200 Subject: [PATCH 1/2] fix(delegation): clean delegations on user deletion Signed-off-by: Hamza --- lib/Db/DelegationMapper.php | 7 +++++ lib/Listener/UserDeletedListener.php | 5 ++++ .../Unit/Listener/UserDeletedListenerTest.php | 29 ++++++++++++++++++- 3 files changed, 40 insertions(+), 1 deletion(-) diff --git a/lib/Db/DelegationMapper.php b/lib/Db/DelegationMapper.php index 86406f3393..5790220be3 100644 --- a/lib/Db/DelegationMapper.php +++ b/lib/Db/DelegationMapper.php @@ -46,6 +46,13 @@ public function find(int $accountId, string $uid): Delegation { return $this->findEntity($qb); } + public function deleteByUserId(string $userId): void { + $qb = $this->db->getQueryBuilder(); + $qb->delete($this->getTableName()) + ->where($qb->expr()->eq('user_id', $qb->createNamedParameter($userId))); + $qb->executeStatement(); + } + /** * @throws DoesNotExistException */ diff --git a/lib/Listener/UserDeletedListener.php b/lib/Listener/UserDeletedListener.php index 5e7ed8222d..f4a2fa30ce 100644 --- a/lib/Listener/UserDeletedListener.php +++ b/lib/Listener/UserDeletedListener.php @@ -9,6 +9,7 @@ namespace OCA\Mail\Listener; +use OCA\Mail\Db\DelegationMapper; use OCA\Mail\Exception\ClientException; use OCA\Mail\Service\AccountService; use OCA\Mail\Service\TextBlockService; @@ -30,6 +31,7 @@ class UserDeletedListener implements IEventListener { public function __construct( AccountService $accountService, private TextBlockService $textBlockService, + private DelegationMapper $delegationMapper, LoggerInterface $logger, ) { $this->accountService = $accountService; @@ -59,5 +61,8 @@ public function handle(Event $event): void { $this->textBlockService->deleteByUserId( $user->getUID() ); + $this->delegationMapper->deleteByUserId( + $user->getUID() + ); } } diff --git a/tests/Unit/Listener/UserDeletedListenerTest.php b/tests/Unit/Listener/UserDeletedListenerTest.php index b74463ed51..e0646e33f6 100644 --- a/tests/Unit/Listener/UserDeletedListenerTest.php +++ b/tests/Unit/Listener/UserDeletedListenerTest.php @@ -11,6 +11,7 @@ use ChristophWurst\Nextcloud\Testing\TestCase; use OCA\Mail\Account; +use OCA\Mail\Db\DelegationMapper; use OCA\Mail\Db\MailAccount; use OCA\Mail\Exception\ClientException; use OCA\Mail\Listener\UserDeletedListener; @@ -26,6 +27,7 @@ class UserDeletedListenerTest extends TestCase { private AccountService&MockObject $accountService; private TextBlockService&MockObject $textBlockService; + private DelegationMapper&MockObject $delegationMapper; private LoggerInterface&MockObject $logger; private UserDeletedListener $listener; @@ -35,10 +37,12 @@ protected function setUp(): void { $this->accountService = $this->createMock(AccountService::class); $this->logger = $this->createMock(LoggerInterface::class); $this->textBlockService = $this->createMock(TextBlockService::class); + $this->delegationMapper = $this->createMock(DelegationMapper::class); $this->listener = new UserDeletedListener( $this->accountService, $this->textBlockService, + $this->delegationMapper, $this->logger ); } @@ -68,6 +72,9 @@ public function testHandleUnrelated(): void { $this->textBlockService->expects($this->never()) ->method('deleteByUserId'); + $this->delegationMapper->expects($this->never()) + ->method('deleteByUserId'); + $this->listener->handle($event); $this->addToAssertionCount(1); @@ -92,6 +99,10 @@ public function testHandleUserDeletedWithNoAccounts(): void { ->method('deleteByUserId') ->with('test-user'); + $this->delegationMapper->expects($this->once()) + ->method('deleteByUserId') + ->with('test-user'); + $this->listener->handle($event); } @@ -109,7 +120,6 @@ public function testHandleUserDeletedWithSingleAccount(): void { ->method('delete') ->with('test-user', 42); - $this->logger->expects($this->never()) ->method('error'); @@ -117,6 +127,10 @@ public function testHandleUserDeletedWithSingleAccount(): void { ->method('deleteByUserId') ->with('test-user'); + $this->delegationMapper->expects($this->once()) + ->method('deleteByUserId') + ->with('test-user'); + $this->listener->handle($event); } @@ -146,6 +160,10 @@ public function testHandleUserDeletedWithMultipleAccounts(): void { ->method('deleteByUserId') ->with('test-user'); + $this->delegationMapper->expects($this->once()) + ->method('deleteByUserId') + ->with('test-user'); + $this->listener->handle($event); } @@ -177,6 +195,10 @@ public function testHandleUserDeletedWithClientException(): void { ->method('deleteByUserId') ->with('test-user'); + $this->delegationMapper->expects($this->once()) + ->method('deleteByUserId') + ->with('test-user'); + $this->listener->handle($event); } @@ -209,10 +231,15 @@ public function testHandleUserDeletedWithPartialFailure(): void { 'Could not delete user\'s Mail account: Failed to delete account 2', ['exception' => $exception] ); + $this->textBlockService->expects($this->once()) ->method('deleteByUserId') ->with('test-user'); + $this->delegationMapper->expects($this->once()) + ->method('deleteByUserId') + ->with('test-user'); + $this->listener->handle($event); } } From f3de342bc9e41f552e7e8aa14d161d54bb271723 Mon Sep 17 00:00:00 2001 From: Hamza Date: Tue, 23 Jun 2026 17:48:43 +0200 Subject: [PATCH 2/2] test(integration): delegation mapper Signed-off-by: Hamza --- lib/Listener/UserDeletedListener.php | 6 +- lib/Service/DelegationService.php | 4 + tests/Integration/Db/DelegationMapperTest.php | 137 ++++++++++++++++++ .../Unit/Listener/UserDeletedListenerTest.php | 20 +-- 4 files changed, 154 insertions(+), 13 deletions(-) create mode 100644 tests/Integration/Db/DelegationMapperTest.php diff --git a/lib/Listener/UserDeletedListener.php b/lib/Listener/UserDeletedListener.php index f4a2fa30ce..8bc376e167 100644 --- a/lib/Listener/UserDeletedListener.php +++ b/lib/Listener/UserDeletedListener.php @@ -9,9 +9,9 @@ namespace OCA\Mail\Listener; -use OCA\Mail\Db\DelegationMapper; use OCA\Mail\Exception\ClientException; use OCA\Mail\Service\AccountService; +use OCA\Mail\Service\DelegationService; use OCA\Mail\Service\TextBlockService; use OCP\EventDispatcher\Event; use OCP\EventDispatcher\IEventListener; @@ -31,7 +31,7 @@ class UserDeletedListener implements IEventListener { public function __construct( AccountService $accountService, private TextBlockService $textBlockService, - private DelegationMapper $delegationMapper, + private DelegationService $delegationService, LoggerInterface $logger, ) { $this->accountService = $accountService; @@ -61,7 +61,7 @@ public function handle(Event $event): void { $this->textBlockService->deleteByUserId( $user->getUID() ); - $this->delegationMapper->deleteByUserId( + $this->delegationService->deleteByUserId( $user->getUID() ); } diff --git a/lib/Service/DelegationService.php b/lib/Service/DelegationService.php index 5728543500..a484ace858 100644 --- a/lib/Service/DelegationService.php +++ b/lib/Service/DelegationService.php @@ -61,6 +61,10 @@ public function delegate(Account $account, string $userId, string $currentUserId return $result; } + public function deleteByUserId(string $userId): void { + $this->delegationMapper->deleteByUserId($userId); + } + public function findDelegatedToUsersForAccount(int $accountId): array { return array_map(function (Delegation $delegation) { $displayName = $this->userManager->get($delegation->getUserId())?->getDisplayName(); diff --git a/tests/Integration/Db/DelegationMapperTest.php b/tests/Integration/Db/DelegationMapperTest.php new file mode 100644 index 0000000000..a49bb4cf15 --- /dev/null +++ b/tests/Integration/Db/DelegationMapperTest.php @@ -0,0 +1,137 @@ +db = Server::get(IDBConnection::class); + $this->mapper = new DelegationMapper($this->db); + $this->accountMapper = new MailAccountMapper($this->db); + } + + private function createDelegation(int $accountId, string $userId): Delegation { + $delegation = new Delegation(); + $delegation->setAccountId($accountId); + $delegation->setUserId($userId); + return $this->mapper->insert($delegation); + } + + private function createAccount(string $ownerUid): MailAccount { + $account = new MailAccount(); + $account->setName('Owner'); + $account->setEmail('owner@example.com'); + $account->setUserId($ownerUid); + return $this->accountMapper->insert($account); + } + + public function testFindDelegatedToUsers(): void { + $accountId = $this->createAccount('owner-a')->getId(); + $otherAccountId = $this->createAccount('owner-b')->getId(); + $this->createDelegation($accountId, 'alice'); + $this->createDelegation($accountId, 'bob'); + $this->createDelegation($otherAccountId, 'carol'); + + $result = $this->mapper->findDelegatedToUsers($accountId); + + $this->assertCount(2, $result); + $uids = array_map(static fn (Delegation $d) => $d->getUserId(), $result); + $this->assertEqualsCanonicalizing(['alice', 'bob'], $uids); + } + + public function testFindDelegatedToUsersReturnsEmpty(): void { + $accountId = $this->createAccount('owner-empty')->getId(); + + $this->assertSame([], $this->mapper->findDelegatedToUsers($accountId)); + } + + public function testFind(): void { + $accountId = $this->createAccount('owner-c')->getId(); + $inserted = $this->createDelegation($accountId, 'dave'); + + $result = $this->mapper->find($accountId, 'dave'); + + $this->assertSame($inserted->getId(), $result->getId()); + $this->assertSame('dave', $result->getUserId()); + $this->assertSame($accountId, $result->getAccountId()); + } + + public function testFindThrowsWhenMissing(): void { + $accountId = $this->createAccount('owner-d')->getId(); + + $this->expectException(DoesNotExistException::class); + + $this->mapper->find($accountId, 'nobody'); + } + + public function testFindAccountOwnerForDelegatedUser(): void { + $account = $this->createAccount('owner-uid'); + $this->createDelegation($account->getId(), 'delegate'); + + $owner = $this->mapper->findAccountOwnerForDelegatedUser($account->getId(), 'delegate'); + + $this->assertSame('owner-uid', $owner); + } + + public function testFindAccountOwnerThrowsWhenNoDelegation(): void { + $account = $this->createAccount('owner-uid'); + + $this->expectException(DoesNotExistException::class); + + $this->mapper->findAccountOwnerForDelegatedUser($account->getId(), 'delegate'); + } + + public function testDeleteByUserId(): void { + $accountA = $this->createAccount('owner-a')->getId(); + $accountB = $this->createAccount('owner-b')->getId(); + $this->createDelegation($accountA, 'delegate'); + $this->createDelegation($accountB, 'delegate'); + $this->createDelegation($accountA, 'other'); + + $this->mapper->deleteByUserId('delegate'); + + $remaining = array_map( + static fn (Delegation $d) => $d->getUserId(), + $this->mapper->findDelegatedToUsers($accountA), + ); + $this->assertSame(['other'], $remaining); + $this->assertSame([], $this->mapper->findDelegatedToUsers($accountB)); + } + + public function testDeleteByUserIdNoMatch(): void { + $accountId = $this->createAccount('owner-uid')->getId(); + $this->createDelegation($accountId, 'delegate'); + + $this->mapper->deleteByUserId('nobody'); + + $this->assertCount(1, $this->mapper->findDelegatedToUsers($accountId)); + } +} diff --git a/tests/Unit/Listener/UserDeletedListenerTest.php b/tests/Unit/Listener/UserDeletedListenerTest.php index e0646e33f6..1175b7f1e5 100644 --- a/tests/Unit/Listener/UserDeletedListenerTest.php +++ b/tests/Unit/Listener/UserDeletedListenerTest.php @@ -11,11 +11,11 @@ use ChristophWurst\Nextcloud\Testing\TestCase; use OCA\Mail\Account; -use OCA\Mail\Db\DelegationMapper; use OCA\Mail\Db\MailAccount; use OCA\Mail\Exception\ClientException; use OCA\Mail\Listener\UserDeletedListener; use OCA\Mail\Service\AccountService; +use OCA\Mail\Service\DelegationService; use OCA\Mail\Service\TextBlockService; use OCP\EventDispatcher\Event; use OCP\IUser; @@ -27,7 +27,7 @@ class UserDeletedListenerTest extends TestCase { private AccountService&MockObject $accountService; private TextBlockService&MockObject $textBlockService; - private DelegationMapper&MockObject $delegationMapper; + private DelegationService&MockObject $delegationService; private LoggerInterface&MockObject $logger; private UserDeletedListener $listener; @@ -37,12 +37,12 @@ protected function setUp(): void { $this->accountService = $this->createMock(AccountService::class); $this->logger = $this->createMock(LoggerInterface::class); $this->textBlockService = $this->createMock(TextBlockService::class); - $this->delegationMapper = $this->createMock(DelegationMapper::class); + $this->delegationService = $this->createMock(DelegationService::class); $this->listener = new UserDeletedListener( $this->accountService, $this->textBlockService, - $this->delegationMapper, + $this->delegationService, $this->logger ); } @@ -72,7 +72,7 @@ public function testHandleUnrelated(): void { $this->textBlockService->expects($this->never()) ->method('deleteByUserId'); - $this->delegationMapper->expects($this->never()) + $this->delegationService->expects($this->never()) ->method('deleteByUserId'); $this->listener->handle($event); @@ -99,7 +99,7 @@ public function testHandleUserDeletedWithNoAccounts(): void { ->method('deleteByUserId') ->with('test-user'); - $this->delegationMapper->expects($this->once()) + $this->delegationService->expects($this->once()) ->method('deleteByUserId') ->with('test-user'); @@ -127,7 +127,7 @@ public function testHandleUserDeletedWithSingleAccount(): void { ->method('deleteByUserId') ->with('test-user'); - $this->delegationMapper->expects($this->once()) + $this->delegationService->expects($this->once()) ->method('deleteByUserId') ->with('test-user'); @@ -160,7 +160,7 @@ public function testHandleUserDeletedWithMultipleAccounts(): void { ->method('deleteByUserId') ->with('test-user'); - $this->delegationMapper->expects($this->once()) + $this->delegationService->expects($this->once()) ->method('deleteByUserId') ->with('test-user'); @@ -195,7 +195,7 @@ public function testHandleUserDeletedWithClientException(): void { ->method('deleteByUserId') ->with('test-user'); - $this->delegationMapper->expects($this->once()) + $this->delegationService->expects($this->once()) ->method('deleteByUserId') ->with('test-user'); @@ -236,7 +236,7 @@ public function testHandleUserDeletedWithPartialFailure(): void { ->method('deleteByUserId') ->with('test-user'); - $this->delegationMapper->expects($this->once()) + $this->delegationService->expects($this->once()) ->method('deleteByUserId') ->with('test-user');