From 8b0d76fd9d55f4e4e2d7c523c456e3ebeeeda0c0 Mon Sep 17 00:00:00 2001 From: Anna Larch <anna@nextcloud.com> Date: Tue, 23 Mar 2021 17:22:02 +0100 Subject: [PATCH] Add time check to hasLocks Signed-off-by: Anna Larch <anna@nextcloud.com> --- lib/Db/Mailbox.php | 21 ++++++++++++++++---- lib/Db/MailboxMapper.php | 2 +- lib/Service/Search/MailSearch.php | 10 ++++++++-- tests/Unit/Service/Search/MailSearchTest.php | 8 +++++++- 4 files changed, 33 insertions(+), 8 deletions(-) diff --git a/lib/Db/Mailbox.php b/lib/Db/Mailbox.php index 61f56b01c..eb7ca6a2e 100644 --- a/lib/Db/Mailbox.php +++ b/lib/Db/Mailbox.php @@ -82,6 +82,12 @@ class Mailbox extends Entity implements JsonSerializable { protected $specialUse; protected $syncInBackground; + /** + * @var int + * Lock timeout for sync (5 minutes) + */ + public const LOCK_TIMEOUT = 300; + public function __construct() { $this->addType('accountId', 'integer'); $this->addType('messages', 'integer'); @@ -119,10 +125,17 @@ class Mailbox extends Entity implements JsonSerializable { && $this->getSyncVanishedToken() !== null; } - public function hasLocks(): bool { - return $this->getSyncNewLock() !== null - || $this->getSyncChangedLock() !== null - || $this->getSyncVanishedLock() !== null; + public function hasLocks(int $now): bool { + if ($this->getSyncNewLock() !== null || $this->getSyncNewLock() > ($now - self::LOCK_TIMEOUT)) { + return true; + } + if ($this->getSyncChangedLock() !== null || $this->getSyncChangedLock() > ($now - self::LOCK_TIMEOUT)) { + return true; + } + if ($this->getSyncVanishedLock() !== null || $this->getSyncVanishedLock() > ($now - self::LOCK_TIMEOUT)) { + return true; + } + return false; } public function jsonSerialize() { diff --git a/lib/Db/MailboxMapper.php b/lib/Db/MailboxMapper.php index 4d5ee15c2..801fd64ca 100644 --- a/lib/Db/MailboxMapper.php +++ b/lib/Db/MailboxMapper.php @@ -148,7 +148,7 @@ class MailboxMapper extends QBMapper { $now = $this->timeFactory->getTime(); if ($lock !== null - && $lock > ($now - 5 * 60)) { + && $lock > ($now - Mailbox::LOCK_TIMEOUT)) { // Another process is syncing throw MailboxLockedException::from($mailbox); } diff --git a/lib/Service/Search/MailSearch.php b/lib/Service/Search/MailSearch.php index 582944c22..fc233689b 100644 --- a/lib/Service/Search/MailSearch.php +++ b/lib/Service/Search/MailSearch.php @@ -39,6 +39,7 @@ use OCA\Mail\Exception\ServiceException; use OCA\Mail\IMAP\PreviewEnhancer; use OCA\Mail\IMAP\Search\Provider as ImapSearchProvider; use OCP\AppFramework\Db\DoesNotExistException; +use OCP\AppFramework\Utility\ITimeFactory; use OCP\IUser; class MailSearch implements IMailSearch { @@ -58,16 +59,21 @@ class MailSearch implements IMailSearch { /** @var PreviewEnhancer */ private $previewEnhancer; + /** @var ITimeFactory */ + private $timeFactory; + public function __construct(FilterStringParser $filterStringParser, MailboxMapper $mailboxMapper, ImapSearchProvider $imapSearchProvider, MessageMapper $messageMapper, - PreviewEnhancer $previewEnhancer) { + PreviewEnhancer $previewEnhancer, + ITimeFactory $timeFactory) { $this->filterStringParser = $filterStringParser; $this->mailboxMapper = $mailboxMapper; $this->imapSearchProvider = $imapSearchProvider; $this->messageMapper = $messageMapper; $this->previewEnhancer = $previewEnhancer; + $this->timeFactory = $timeFactory; } public function findMessage(Account $account, @@ -101,7 +107,7 @@ class MailSearch implements IMailSearch { ?string $filter, ?int $cursor, ?int $limit): array { - if ($mailbox->hasLocks()) { + if ($mailbox->hasLocks($this->timeFactory->getTime())) { throw MailboxLockedException::from($mailbox); } if (!$mailbox->isCached()) { diff --git a/tests/Unit/Service/Search/MailSearchTest.php b/tests/Unit/Service/Search/MailSearchTest.php index 534bc076f..d08b581a7 100644 --- a/tests/Unit/Service/Search/MailSearchTest.php +++ b/tests/Unit/Service/Search/MailSearchTest.php @@ -39,6 +39,7 @@ use OCA\Mail\Service\Search\FilterStringParser; use OCA\Mail\Service\Search\Flag; use OCA\Mail\Service\Search\MailSearch; use OCA\Mail\Service\Search\SearchQuery; +use OCP\AppFramework\Utility\ITimeFactory; use PHPUnit\Framework\MockObject\MockObject; class MailSearchTest extends TestCase { @@ -61,6 +62,9 @@ class MailSearchTest extends TestCase { /** @var MessageMapper|MockObject */ private $messageMapper; + /** @var ITimeFactory|MockObject */ + private $timeFactory; + protected function setUp(): void { parent::setUp(); @@ -69,13 +73,15 @@ class MailSearchTest extends TestCase { $this->imapSearchProvider = $this->createMock(Provider::class); $this->messageMapper = $this->createMock(MessageMapper::class); $this->previewEnhancer = $this->createMock(PreviewEnhancer::class); + $this->timeFactory = $this->createMock(ITimeFactory::class); $this->search = new MailSearch( $this->filterStringParser, $this->mailboxMapper, $this->imapSearchProvider, $this->messageMapper, - $this->previewEnhancer + $this->previewEnhancer, + $this->timeFactory ); } -- GitLab