Update phpdoc for local attachment and outbox

Signed-off-by: Daniel Kesselberg <mail@danielkesselberg.de>

7 files changed, 97 insertions, 66 deletions

diff --git a/lib/Db/LocalAttachmentMapper.php b/lib/Db/LocalAttachmentMapper.php
index 50bb25158..b3213768e 100644
--- a/lib/Db/LocalAttachmentMapper.php
+++ b/lib/Db/LocalAttachmentMapper.php
@@ -46,11 +46,12 @@ class LocalAttachmentMapper extends QBMapper {
 	/**
 	 * @return LocalAttachment[]
 	 */
-	public function findByLocalMessageId(int $localMessageId): array {
+	public function findByLocalMessageId(string $userId, int $localMessageId): array {
 		$qb = $this->db->getQueryBuilder();
 		$qb->select('*')
 			->from($this->getTableName())
-			->where(
+			->where($qb->expr()->eq('user_id', $qb->createNamedParameter($userId)))
+			->andWhere(
 				$qb->expr()->eq('local_message_id', $qb->createNamedParameter($localMessageId, IQueryBuilder::PARAM_INT), IQueryBuilder::PARAM_INT)
 			);
 		return $this->findEntities($qb);
@@ -86,12 +87,17 @@ class LocalAttachmentMapper extends QBMapper {
 		return $this->findEntity($query);
 	}
 
-	public function deleteForLocalMessage(int $localMessageId): void {
+	/**
+	 * @throws Throwable
+	 * @throws \OCP\DB\Exception
+	 */
+	public function deleteForLocalMessage(string $userId, int $localMessageId): void {
 		$this->db->beginTransaction();
 		try {
 			$qb = $this->db->getQueryBuilder();
 			$qb->delete($this->getTableName())
-				->where($qb->expr()->eq('local_message_id', $qb->createNamedParameter($localMessageId), IQueryBuilder::PARAM_INT));
+				->where($qb->expr()->eq('user_id', $qb->createNamedParameter($userId)))
+				->andWhere($qb->expr()->eq('local_message_id', $qb->createNamedParameter($localMessageId), IQueryBuilder::PARAM_INT));
 			$qb->execute();
 			$this->db->commit();
 		} catch (Throwable $e) {
@@ -100,13 +106,18 @@ class LocalAttachmentMapper extends QBMapper {
 		}
 	}
 
-	public function saveLocalMessageAttachments(int $localMessageId, array $attachmentIds) {
+	/**
+	 * @throws Throwable
+	 * @throws \OCP\DB\Exception
+	 */
+	public function saveLocalMessageAttachments(string $userId, int $localMessageId, array $attachmentIds): void {
 		$this->db->beginTransaction();
 		try {
 			$qb = $this->db->getQueryBuilder();
 			$qb->update($this->getTableName())
 				->set('local_message_id', $qb->createNamedParameter($localMessageId, IQueryBuilder::PARAM_INT))
-				->where(
+				->where($qb->expr()->eq('user_id', $qb->createNamedParameter($userId)))
+				->andWhere(
 					$qb->expr()->in('id', $qb->createNamedParameter($attachmentIds, IQueryBuilder::PARAM_INT_ARRAY), IQueryBuilder::PARAM_INT_ARRAY)
 				);
 			$qb->execute();
@@ -119,12 +130,14 @@ class LocalAttachmentMapper extends QBMapper {
 
 	/**
 	 * @return LocalAttachment[]
+	 * @throws \OCP\DB\Exception
 	 */
-	public function findByIds(array $attachmentIds): array {
+	public function findByIds(string $userId, array $attachmentIds): array {
 		$qb = $this->db->getQueryBuilder();
 		$qb->select('*')
 			->from($this->getTableName())
-			->where(
+			->where($qb->expr()->eq('user_id', $qb->createNamedParameter($userId)))
+			->andWhere(
 				$qb->expr()->in('id', $qb->createNamedParameter($attachmentIds, IQueryBuilder::PARAM_INT_ARRAY), IQueryBuilder::PARAM_INT_ARRAY)
 			);
 		return $this->findEntities($qb);
diff --git a/lib/Db/LocalMessageMapper.php b/lib/Db/LocalMessageMapper.php
index 79c8ee2f3..0aaa0f979 100644
--- a/lib/Db/LocalMessageMapper.php
+++ b/lib/Db/LocalMessageMapper.php
@@ -112,7 +112,7 @@ class LocalMessageMapper extends QBMapper {
 				$qb->expr()->eq('m.id', $qb->createNamedParameter($id, IQueryBuilder::PARAM_INT), IQueryBuilder::PARAM_INT)
 			);
 		$entity = $this->findEntity($qb);
-		$entity->setAttachments($this->attachmentMapper->findByLocalMessageId($id));
+		$entity->setAttachments($this->attachmentMapper->findByLocalMessageId($userId, $id));
 		$entity->setRecipients($this->recipientMapper->findByLocalMessageId($id));
 		return $entity;
 	}
diff --git a/lib/Service/Attachment/AttachmentService.php b/lib/Service/Attachment/AttachmentService.php
index 65dc5dcae..7670bd466 100644
--- a/lib/Service/Attachment/AttachmentService.php
+++ b/lib/Service/Attachment/AttachmentService.php
@@ -165,9 +165,9 @@ class AttachmentService implements IAttachmentService {
 	}
 
 	public function deleteLocalMessageAttachments(string $userId, int $localMessageId): void {
-		$attachments = $this->mapper->findByLocalMessageId($localMessageId);
+		$attachments = $this->mapper->findByLocalMessageId($userId, $localMessageId);
 		// delete db entries
-		$this->mapper->deleteForLocalMessage($localMessageId);
+		$this->mapper->deleteForLocalMessage($userId, $localMessageId);
 		// delete storage
 		foreach ($attachments as $attachment) {
 			$this->storage->delete($userId, $attachment->getId());
@@ -175,7 +175,7 @@ class AttachmentService implements IAttachmentService {
 	}
 
 	public function deleteLocalMessageAttachmentsById(string $userId, int $localMessageId, array $attachmentIds): void {
-		$attachments = $this->mapper->findByIds($attachmentIds);
+		$attachments = $this->mapper->findByIds($userId, $attachmentIds);
 		// delete storage
 		foreach ($attachments as $attachment) {
 			$this->mapper->delete($attachment);
@@ -187,12 +187,12 @@ class AttachmentService implements IAttachmentService {
 	 * @param int[] $attachmentIds
 	 * @return LocalAttachment[]
 	 */
-	public function saveLocalMessageAttachments(int $messageId, array $attachmentIds): array {
+	public function saveLocalMessageAttachments(string $userId, int $messageId, array $attachmentIds): array {
 		if (empty($attachmentIds)) {
 			return [];
 		}
-		$this->mapper->saveLocalMessageAttachments($messageId, $attachmentIds);
-		return $this->mapper->findByLocalMessageId($messageId);
+		$this->mapper->saveLocalMessageAttachments($userId, $messageId, $attachmentIds);
+		return $this->mapper->findByLocalMessageId($userId, $messageId);
 	}
 
 	/**
@@ -207,8 +207,8 @@ class AttachmentService implements IAttachmentService {
 
 		// no need to diff, no old attachments
 		if (empty($message->getAttachments())) {
-			$this->mapper->saveLocalMessageAttachments($message->getId(), $newAttachmentIds);
-			return $this->mapper->findByLocalMessageId($message->getId());
+			$this->mapper->saveLocalMessageAttachments($userId, $message->getId(), $newAttachmentIds);
+			return $this->mapper->findByLocalMessageId($userId, $message->getId());
 		}
 
 		$oldAttachmentIds = array_map(static function ($attachment) {
@@ -217,7 +217,7 @@ class AttachmentService implements IAttachmentService {
 
 		$add = array_diff($newAttachmentIds, $oldAttachmentIds);
 		if (!empty($add)) {
-			$this->mapper->saveLocalMessageAttachments($message->getId(), $add);
+			$this->mapper->saveLocalMessageAttachments($userId, $message->getId(), $add);
 		}
 
 		$delete = array_diff($oldAttachmentIds, $newAttachmentIds);
@@ -225,7 +225,7 @@ class AttachmentService implements IAttachmentService {
 			$this->deleteLocalMessageAttachmentsById($userId, $message->getId(), $delete);
 		}
 
-		return $this->mapper->findByLocalMessageId($message->getId());
+		return $this->mapper->findByLocalMessageId($userId, $message->getId());
 	}
 
 
diff --git a/lib/Service/OutboxService.php b/lib/Service/OutboxService.php
index ce3754849..1a37d661a 100644
--- a/lib/Service/OutboxService.php
+++ b/lib/Service/OutboxService.php
@@ -144,7 +144,7 @@ class OutboxService implements ILocalMailboxService {
 			$client->logout();
 		}
 
-		$message->setAttachments($this->attachmentService->saveLocalMessageAttachments($message->getId(), $attachmentIds));
+		$message->setAttachments($this->attachmentService->saveLocalMessageAttachments($account->getUserId(), $message->getId(), $attachmentIds));
 		return $message;
 	}
 
diff --git a/tests/Integration/Db/LocalAttachmentMapperTest.php b/tests/Integration/Db/LocalAttachmentMapperTest.php
index 25087e786..d4659f93e 100644
--- a/tests/Integration/Db/LocalAttachmentMapperTest.php
+++ b/tests/Integration/Db/LocalAttachmentMapperTest.php
@@ -54,6 +54,11 @@ class LocalAttachmentMapperTest extends TestCase {
 	/** @var array */
 	private $attachments;
 
+	/** @var string */
+	private $user1 = 'user45678';
+	/** @var string  */
+	private $user2 = 'dontFindMe';
+
 	protected function setUp(): void {
 		parent::setUp();
 
@@ -72,32 +77,39 @@ class LocalAttachmentMapperTest extends TestCase {
 		$delete = $qb->delete($this->mapper->getTableName());
 		$delete->execute();
 
-		$attachment = LocalAttachment::fromParams([
+		$attachment1 = LocalAttachment::fromParams([
 			'fileName' => 'slimes_in_the_mines.jpeg',
 			'mimeType' => 'image/jpeg',
-			'userId' => 'user45678',
+			'userId' => $this->user1,
 			'createdAt' => $this->timeFactory->getTime()
 		]);
 		$attachment2 = LocalAttachment::fromParams([
 			'fileName' => 'prismatic_shard.png',
 			'mimeType' => 'image/png',
-			'userId' => 'dontFindMe',
+			'userId' => $this->user2,
+			'createdAt' => $this->timeFactory->getTime()
+		]);
+		$attachment3 = LocalAttachment::fromParams([
+			'fileName' => 'slimes_in_the_shard.png',
+			'mimeType' => 'image/png',
+			'userId' => $this->user1,
 			'createdAt' => $this->timeFactory->getTime()
 		]);
-		$attachment = $this->mapper->insert($attachment);
+		$attachment1 = $this->mapper->insert($attachment1);
 		$attachment2 = $this->mapper->insert($attachment2);
-		$this->attachmentIds = [$attachment->getId(), $attachment2->getId()];
-
-		$message = new LocalMessage();
-		$message->setType(LocalMessage::TYPE_OUTGOING);
-		$message->setAccountId(1);
-		$message->setAliasId(3);
-		$message->setSendAt(3);
-		$message->setSubject('testSaveLocalAttachments');
-		$message->setBody('message');
-		$message->setHtml(true);
-		$message->setInReplyToMessageId('abcdefg');
-		$message = $this->localMessageMapper->insert($message);
+		$attachment3 = $this->mapper->insert($attachment3);
+		$this->attachmentIds = [$attachment1->getId(), $attachment2->getId(), $attachment3->getId()];
+
+		$message1 = new LocalMessage();
+		$message1->setType(LocalMessage::TYPE_OUTGOING);
+		$message1->setAccountId(1);
+		$message1->setAliasId(3);
+		$message1->setSendAt(3);
+		$message1->setSubject('testSaveLocalAttachments');
+		$message1->setBody('message');
+		$message1->setHtml(true);
+		$message1->setInReplyToMessageId('abcdefg');
+		$message1 = $this->localMessageMapper->insert($message1);
 		$message2 = new LocalMessage();
 		$message2->setType(LocalMessage::TYPE_OUTGOING);
 		$message2->setAccountId(1);
@@ -108,44 +120,44 @@ class LocalAttachmentMapperTest extends TestCase {
 		$message2->setHtml(true);
 		$message2->setInReplyToMessageId('abcdefg');
 		$message2 = $this->localMessageMapper->insert($message2);
-		$this->localMessageIds = [$message->getId(), $message2->getId()];
+		$this->localMessageIds = [$message1->getId(), $message2->getId()];
 	}
 
 	public function testSaveAndFindLocalAttachments(): void {
-		$this->mapper->saveLocalMessageAttachments($this->localMessageIds[0], $this->attachmentIds);
-		$foundAttachments = $this->mapper->findByLocalMessageId($this->localMessageIds[0]);
+		$this->mapper->saveLocalMessageAttachments($this->user1, $this->localMessageIds[0], $this->attachmentIds);
+		$foundAttachments = $this->mapper->findByLocalMessageId($this->user1, $this->localMessageIds[0]);
 
 		$this->assertCount(2, $foundAttachments);
 	}
 
 	public function testDeleteForLocalMessage(): void {
-		$this->mapper->saveLocalMessageAttachments($this->localMessageIds[0], $this->attachmentIds);
-		$foundAttachments = $this->mapper->findByLocalMessageId($this->localMessageIds[0]);
+		$this->mapper->saveLocalMessageAttachments($this->user1, $this->localMessageIds[0], $this->attachmentIds);
+		$foundAttachments = $this->mapper->findByLocalMessageId($this->user1, $this->localMessageIds[0]);
 
 		$this->assertCount(2, $foundAttachments);
 
-		$this->mapper->deleteForLocalMessage($this->localMessageIds[0]);
+		$this->mapper->deleteForLocalMessage($this->user1, $this->localMessageIds[0]);
 
-		$result = $this->mapper->findByLocalMessageId($this->localMessageIds[0]);
+		$result = $this->mapper->findByLocalMessageId($this->user1, $this->localMessageIds[0]);
 		$this->assertEmpty($result);
 	}
 
 	public function testFind(): void {
-		$this->mapper->saveLocalMessageAttachments($this->localMessageIds[0], $this->attachmentIds);
-		$foundAttachment = $this->mapper->find('user45678', $this->attachmentIds[0]);
+		$this->mapper->saveLocalMessageAttachments($this->user1, $this->localMessageIds[0], $this->attachmentIds);
+		$foundAttachment = $this->mapper->find($this->user1, $this->attachmentIds[0]);
 
 		$this->assertEquals('slimes_in_the_mines.jpeg', $foundAttachment->getFileName());
 		$this->assertEquals('image/jpeg', $foundAttachment->getMimeType());
 		$this->assertEquals($this->localMessageIds[0], $foundAttachment->getLocalMessageId());
-		$this->assertEquals('user45678', $foundAttachment->getUserId());
+		$this->assertEquals($this->user1, $foundAttachment->getUserId());
 
 		$this->expectException(DoesNotExistException::class);
-		$this->mapper->find('user45678', $this->attachmentIds[1]);
+		$this->mapper->find($this->user1, $this->attachmentIds[1]);
 	}
 
 	public function testFindByLocalMessageIds(): void {
-		$this->mapper->saveLocalMessageAttachments($this->localMessageIds[0], [$this->attachmentIds[0]]);
-		$this->mapper->saveLocalMessageAttachments($this->localMessageIds[1], [$this->attachmentIds[1]]);
+		$this->mapper->saveLocalMessageAttachments($this->user1, $this->localMessageIds[0], [$this->attachmentIds[0]]);
+		$this->mapper->saveLocalMessageAttachments($this->user2, $this->localMessageIds[1], [$this->attachmentIds[1]]);
 
 		$foundAttachments = $this->mapper->findByLocalMessageIds($this->localMessageIds);
 		$this->assertCount(2, $foundAttachments);
diff --git a/tests/Unit/Service/Attachment/AttachmentServiceTest.php b/tests/Unit/Service/Attachment/AttachmentServiceTest.php
index 160ddcbb9..332fd23d0 100644
--- a/tests/Unit/Service/Attachment/AttachmentServiceTest.php
+++ b/tests/Unit/Service/Attachment/AttachmentServiceTest.php
@@ -235,11 +235,11 @@ class AttachmentServiceTest extends TestCase {
 
 		$this->mapper->expects(self::once())
 			->method('findByLocalMessageId')
-			->with('10')
+			->with($userId, '10')
 			->willReturn($attachments);
 		$this->mapper->expects(self::once())
 			->method('deleteForLocalMessage')
-			->with('10');
+			->with($userId, '10');
 		$this->storage->expects(self::once())
 			->method('delete')
 			->with($userId, $attachment->getId());
@@ -248,21 +248,23 @@ class AttachmentServiceTest extends TestCase {
 	}
 
 	public function testSaveLocalMessageAttachment(): void {
+		$userId = 'linus';
 		$attachmentIds = [1,2,3];
 		$messageId = 100;
 
 		$this->mapper->expects(self::once())
 			->method('saveLocalMessageAttachments')
-			->with($messageId, $attachmentIds);
+			->with($userId, $messageId, $attachmentIds);
 		$this->mapper->expects(self::once())
 			->method('findByLocalMessageId')
-			->with($messageId)
+			->with($userId, $messageId)
 			->willReturn([$this->createMock(LocalAttachment::class)]);
 
-		$this->service->saveLocalMessageAttachments($messageId, $attachmentIds);
+		$this->service->saveLocalMessageAttachments($userId, $messageId, $attachmentIds);
 	}
 
 	public function testSaveLocalMessageAttachmentNoAttachmentIds(): void {
+		$userId = 'linus';
 		$attachmentIds = [];
 		$messageId = 100;
 
@@ -271,7 +273,7 @@ class AttachmentServiceTest extends TestCase {
 		$this->mapper->expects(self::never())
 			->method('findByLocalMessageId');
 
-		$this->service->saveLocalMessageAttachments($messageId, $attachmentIds);
+		$this->service->saveLocalMessageAttachments($userId, $messageId, $attachmentIds);
 	}
 
 	public function testhandleLocalMessageAttachment(): void {
@@ -450,10 +452,10 @@ class AttachmentServiceTest extends TestCase {
 		$attachmentIds = [4,5];
 		$this->mapper->expects(self::once())
 			->method('saveLocalMessageAttachments')
-			->with($message->getId(), $attachmentIds);
+			->with($userId, $message->getId(), $attachmentIds);
 		$this->mapper->expects(self::once())
 			->method('findByLocalMessageId')
-			->with($message->getId())
+			->with($userId, $message->getId())
 			->willReturn([$a1, $a2]);
 		$this->service->updateLocalMessageAttachments($userId, $message, $attachmentIds);
 	}
@@ -471,11 +473,11 @@ class AttachmentServiceTest extends TestCase {
 		]);
 		$this->mapper->expects(self::once())
 			->method('findByLocalMessageId')
-			->with($message->getId())
+			->with($userId, $message->getId())
 			->willReturn([$attachment]);
 		$this->mapper->expects(self::once())
 			->method('deleteForLocalMessage')
-			->with($message->getId());
+			->with($userId, $message->getId());
 		$this->storage->expects(self::once())
 			->method('delete')
 			->with($userId, 5678);
@@ -503,10 +505,10 @@ class AttachmentServiceTest extends TestCase {
 
 		$this->mapper->expects(self::once())
 			->method('saveLocalMessageAttachments')
-			->with($message->getId(), [ 1 => 4]);
+			->with($userId, $message->getId(), [ 1 => 4]);
 		$this->mapper->expects(self::once())
 			->method('findByIds')
-			->with([2])
+			->with($userId, [2])
 			->willReturn([$a1]);
 		$this->mapper->expects(self::once())
 			->method('delete')
diff --git a/tests/Unit/Service/OutboxServiceTest.php b/tests/Unit/Service/OutboxServiceTest.php
index 78b857bc5..2a4043f48 100644
--- a/tests/Unit/Service/OutboxServiceTest.php
+++ b/tests/Unit/Service/OutboxServiceTest.php
@@ -226,7 +226,9 @@ class OutboxServiceTest extends TestCase {
 		]);
 		$message2 = $message;
 		$message2->setId(10);
-		$account = $this->createMock(Account::class);
+		$account = $this->createConfiguredMock(Account::class, [
+			'getUserId' => $this->userId
+		]);
 		$client = $this->createMock(\Horde_Imap_Client_Socket::class);
 
 		$this->mapper->expects(self::once())
@@ -243,7 +245,7 @@ class OutboxServiceTest extends TestCase {
 			->willReturn($attachmentIds);
 		$this->attachmentService->expects(self::once())
 			->method('saveLocalMessageAttachments')
-			->with(10, $attachmentIds);
+			->with($this->userId, 10, $attachmentIds);
 
 		$this->outboxService->saveMessage($account, $message, $to, $cc, $bcc, $attachments);
 	}
@@ -282,7 +284,7 @@ class OutboxServiceTest extends TestCase {
 		$message2 = $message;
 		$message2->setRecipients([$rTo]);
 		$account = $this->createConfiguredMock(Account::class, [
-			'getUserId' => 'linus'
+			'getUserId' => $this->userId
 		]);
 		$client = $this->createMock(\Horde_Imap_Client_Socket::class);
 
@@ -330,7 +332,9 @@ class OutboxServiceTest extends TestCase {
 		]);
 		$message2 = $message;
 		$message2->setId(10);
-		$account = $this->createMock(Account::class);
+		$account = $this->createConfiguredMock(Account::class, [
+			'getUserId' => $this->userId
+		]);
 
 		$this->mapper->expects(self::once())
 			->method('saveWithRecipients')
@@ -338,7 +342,7 @@ class OutboxServiceTest extends TestCase {
 			->willReturn($message2);
 		$this->attachmentService->expects(self::once())
 			->method('saveLocalMessageAttachments')
-			->with(10, []);
+			->with($this->userId, 10, []);
 
 		$this->outboxService->saveMessage($account, $message, $to, $cc, $bcc);
 	}