Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update phpdoc for local attachment and outbox #6600

Merged
merged 1 commit into from May 31, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Update phpdoc for local attachment and outbox
Signed-off-by: Daniel Kesselberg <mail@danielkesselberg.de>
  • Loading branch information
kesselb committed May 31, 2022
commit 6dd2527be8d4f6788b449c8a8f5577628b990605
29 changes: 21 additions & 8 deletions lib/Db/LocalAttachmentMapper.php
Expand Up @@ -46,11 +46,12 @@ public function __construct(IDBConnection $db) {
/**
* @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);
Expand Down Expand Up @@ -86,12 +87,17 @@ public function find(string $userId, int $id): LocalAttachment {
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) {
Expand All @@ -100,13 +106,18 @@ public function deleteForLocalMessage(int $localMessageId): void {
}
}

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();
Expand All @@ -119,12 +130,14 @@ public function saveLocalMessageAttachments(int $localMessageId, array $attachme

/**
* @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);
Expand Down
2 changes: 1 addition & 1 deletion lib/Db/LocalMessageMapper.php
Expand Up @@ -112,7 +112,7 @@ public function findById(int $id, string $userId): LocalMessage {
$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;
}
Expand Down
20 changes: 10 additions & 10 deletions lib/Service/Attachment/AttachmentService.php
Expand Up @@ -165,17 +165,17 @@ public function deleteAttachment(string $userId, int $id) {
}

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());
}
}

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);
Expand All @@ -187,12 +187,12 @@ public function deleteLocalMessageAttachmentsById(string $userId, int $localMess
* @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);
}

/**
Expand All @@ -207,8 +207,8 @@ public function updateLocalMessageAttachments(string $userId, LocalMessage $mess

// 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) {
Expand All @@ -217,15 +217,15 @@ public function updateLocalMessageAttachments(string $userId, LocalMessage $mess

$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);
if (!empty($delete)) {
$this->deleteLocalMessageAttachmentsById($userId, $message->getId(), $delete);
}

return $this->mapper->findByLocalMessageId($message->getId());
return $this->mapper->findByLocalMessageId($userId, $message->getId());
}


Expand Down
2 changes: 1 addition & 1 deletion lib/Service/OutboxService.php
Expand Up @@ -144,7 +144,7 @@ public function saveMessage(Account $account, LocalMessage $message, array $to,
$client->logout();
}

$message->setAttachments($this->attachmentService->saveLocalMessageAttachments($message->getId(), $attachmentIds));
$message->setAttachments($this->attachmentService->saveLocalMessageAttachments($account->getUserId(), $message->getId(), $attachmentIds));
return $message;
}

Expand Down
70 changes: 41 additions & 29 deletions tests/Integration/Db/LocalAttachmentMapperTest.php
Expand Up @@ -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();

Expand All @@ -72,32 +77,39 @@ protected function setUp(): void {
$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);
Expand All @@ -108,44 +120,44 @@ protected function setUp(): void {
$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);
Expand Down
26 changes: 14 additions & 12 deletions tests/Unit/Service/Attachment/AttachmentServiceTest.php
Expand Up @@ -235,11 +235,11 @@ public function testDeleteLocalMessageAttachment() : void {

$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());
Expand All @@ -248,21 +248,23 @@ public function testDeleteLocalMessageAttachment() : void {
}

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;

Expand All @@ -271,7 +273,7 @@ public function testSaveLocalMessageAttachmentNoAttachmentIds(): void {
$this->mapper->expects(self::never())
->method('findByLocalMessageId');

$this->service->saveLocalMessageAttachments($messageId, $attachmentIds);
$this->service->saveLocalMessageAttachments($userId, $messageId, $attachmentIds);
}

public function testhandleLocalMessageAttachment(): void {
Expand Down Expand Up @@ -450,10 +452,10 @@ public function testUpdateLocalMessageAttachments(): void {
$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);
}
Expand All @@ -471,11 +473,11 @@ public function testUpdateLocalMessageAttachmentsNoAttachments(): void {
]);
$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);
Expand Down Expand Up @@ -503,10 +505,10 @@ public function testUpdateLocalMessageAttachmentsDiffAttachments(): void {

$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')
Expand Down