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.

Sign up for GitHub

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

Ensure subquery in findNewIds never returns NULL #8590

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Ensure subquery in findNewIds never returns NULL #8590

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ed2d0da
fix #4479 by ensuring subquery in findNewIds never returns NULL
IchbinkeinReh Jul 2, 2023
e8b34b0
fixup! fix #4479 by ensuring subquery in findNewIds never returns NULL
ChristophWurst Sep 13, 2023
045e0bb
fixup! fix #4479 by ensuring subquery in findNewIds never returns NULL
ChristophWurst Sep 13, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion lib/Db/MessageMapper.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1209,8 +1209,9 @@ public function findNewIds(Mailbox $mailbox, array $ids): array {
$select = $this->db->getQueryBuilder();
$subSelect = $this->db->getQueryBuilder();

// MIN returns NULL if there are no rows selected, therefore we use COALESCE to ensure 0 is returned in this case
$subSelect
->select($subSelect->func()->min('sent_at'))
->select($subSelect->createFunction('COALESCE(MIN(' . $subSelect->getColumnName('sent_at') . '), 0)'))
->from($this->getTableName())
->where(
$subSelect->expr()->eq('mailbox_id', $select->createNamedParameter($mailbox->getId(), IQueryBuilder::PARAM_INT)),
Expand Down
25 changes: 25 additions & 0 deletions tests/Integration/Db/MessageMapperTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
use ChristophWurst\Nextcloud\Testing\DatabaseTransaction;
use ChristophWurst\Nextcloud\Testing\TestCase;
use OCA\Mail\Account;
use OCA\Mail\Db\Mailbox;
use OCA\Mail\Db\MessageMapper;
use OCA\Mail\Db\TagMapper;
use OCA\Mail\Support\PerformanceLogger;
Expand Down Expand Up @@ -140,4 +141,28 @@ public function testResetPreviewDataFlag(): void {
$result->closeCursor();
self::assertEquals(0, $cnt);
}

/**
* Verify we still find the one message of the mailbox if the passed IDs do not exist
*/
public function testFindNewIdsCoalescence(): void {
$uid = time();
$mailbox = new Mailbox();
$mailbox->setId(1);
$qb = $this->db->getQueryBuilder();
$insert = $qb->insert($this->mapper->getTableName())
->values([
'uid' => $qb->createNamedParameter($uid, IQueryBuilder::PARAM_INT),
'message_id' => $qb->createNamedParameter('<abc@123.com>'),
'mailbox_id' => $qb->createNamedParameter(1, IQueryBuilder::PARAM_INT),
'subject' => $qb->createNamedParameter('TEST'),
'sent_at' => $qb->createNamedParameter(time(), IQueryBuilder::PARAM_INT),
]);
$insert->executeStatement();
$id = $insert->getLastInsertId();

$found = $this->mapper->findNewIds($mailbox, [$id + 1]);

self::assertCount(1, $found);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this fails without COALESCE 👍

}
}
Loading