Skip to content
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 42 additions & 8 deletions lib/BackgroundJob/SyncJob.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
namespace OCA\Mail\BackgroundJob;

use Horde_Imap_Client_Exception;
use OCA\Mail\AppInfo\Application;
use OCA\Mail\Exception\IncompleteSyncException;
use OCA\Mail\Exception\ServiceException;
use OCA\Mail\IMAP\MailboxSync;
Expand All @@ -26,21 +27,26 @@
use function sprintf;

class SyncJob extends TimedJob {
private const DEFAULT_SYNC_INTERVAL = 3600;

private IUserManager $userManager;
private AccountService $accountService;
private ImapToDbSynchronizer $syncService;
private MailboxSync $mailboxSync;
private LoggerInterface $logger;
private IJobList $jobList;
private readonly bool $forcedSyncInterval;

public function __construct(ITimeFactory $time,
public function __construct(
ITimeFactory $time,
IUserManager $userManager,
AccountService $accountService,
MailboxSync $mailboxSync,
ImapToDbSynchronizer $syncService,
LoggerInterface $logger,
IJobList $jobList,
IConfig $config) {
private readonly IConfig $config,
) {
parent::__construct($time);

$this->userManager = $userManager;
Expand All @@ -50,12 +56,15 @@ public function __construct(ITimeFactory $time,
$this->logger = $logger;
$this->jobList = $jobList;

$this->setInterval(
max(
5 * 60,
$config->getSystemValueInt('app.mail.background-sync-interval', 3600)
),
);
$configuredSyncInterval = $config->getSystemValueInt('app.mail.background-sync-interval');
if ($configuredSyncInterval > 0) {
$this->forcedSyncInterval = true;
} else {
$this->forcedSyncInterval = false;
$configuredSyncInterval = self::DEFAULT_SYNC_INTERVAL;
}

$this->setInterval(max(5 * 60, $configuredSyncInterval));
$this->setTimeSensitivity(self::TIME_SENSITIVE);
}

Expand All @@ -79,6 +88,31 @@ protected function run($argument) {
return;
}

// If an admin configured a custom sync interval, always abide by it
if (!$this->forcedSyncInterval) {
$now = $this->time->getTime();
$heartbeat = (int)$this->config->getUserValue(
$account->getUserId(),
Application::APP_ID,
'ui-heartbeat',
$now + 1, // Force negative value for $lastUsed in case of no heartbeat
);
$lastUsed = $now - $heartbeat;
if ($lastUsed > 3 * 24 * 3600) {
// User did not open the app in more than three days -> defer sync
$this->setInterval(6 * 3600);
} elseif ($lastUsed > 24 * 3600) {
// User opened the app at least once within the last three days -> default sync
$this->setInterval(self::DEFAULT_SYNC_INTERVAL);
} elseif ($lastUsed > 0) {
// User opened the app at least once within the last 24 hours -> sync more often
$this->setInterval(15 * 60);
} else {
// Default to the hourly interval in case there is no heartbeat
$this->setInterval(self::DEFAULT_SYNC_INTERVAL);
}
}

$user = $this->userManager->get($account->getUserId());
if ($user === null || !$user->isEnabled()) {
$this->logger->debug(sprintf(
Expand Down
28 changes: 17 additions & 11 deletions lib/Controller/MailboxesController.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
namespace OCA\Mail\Controller;

use Horde_Imap_Client;
use OCA\Mail\AppInfo\Application;
use OCA\Mail\Contracts\IMailManager;
use OCA\Mail\Contracts\IMailSearch;
use OCA\Mail\Exception\ClientException;
Expand All @@ -27,29 +28,27 @@
use OCP\AppFramework\Http\Attribute\OpenAPI;
use OCP\AppFramework\Http\Attribute\UserRateLimit;
use OCP\AppFramework\Http\JSONResponse;
use OCP\AppFramework\Utility\ITimeFactory;
use OCP\IConfig;
use OCP\IRequest;

#[OpenAPI(scope: OpenAPI::SCOPE_IGNORE)]
class MailboxesController extends Controller {
private AccountService $accountService;
private ?string $currentUserId;
private IMailManager $mailManager;
private SyncService $syncService;
private ?string $currentUserId;

/**
* @param string $appName
* @param IRequest $request
* @param AccountService $accountService
* @param string|null $UserId
* @param IMailManager $mailManager
* @param SyncService $syncService
*/
public function __construct(string $appName,
public function __construct(
string $appName,
IRequest $request,
AccountService $accountService,
?string $UserId,
IMailManager $mailManager,
SyncService $syncService) {
SyncService $syncService,
private readonly IConfig $config,
private readonly ITimeFactory $timeFactory,
) {
parent::__construct($appName, $request);

$this->accountService = $accountService;
Expand Down Expand Up @@ -140,6 +139,13 @@ public function sync(int $id, array $ids = [], ?int $lastMessageTimestamp = null
$account = $this->accountService->find($this->currentUserId, $mailbox->getAccountId());
$order = $sortOrder === 'newest' ? IMailSearch::ORDER_NEWEST_FIRST: IMailSearch::ORDER_OLDEST_FIRST;

$this->config->setUserValue(
$this->currentUserId,
Application::APP_ID,
'ui-heartbeat',
(string)$this->timeFactory->getTime(),
);

try {
$syncResponse = $this->syncService->syncMailbox(
$account,
Expand Down
19 changes: 17 additions & 2 deletions lib/Service/AccountService.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
namespace OCA\Mail\Service;

use OCA\Mail\Account;
use OCA\Mail\AppInfo\Application;
use OCA\Mail\BackgroundJob\PreviewEnhancementProcessingJob;
use OCA\Mail\BackgroundJob\QuotaJob;
use OCA\Mail\BackgroundJob\SyncJob;
Expand All @@ -21,7 +22,9 @@
use OCA\Mail\Exception\ServiceException;
use OCA\Mail\IMAP\IMAPClientFactory;
use OCP\AppFramework\Db\DoesNotExistException;
use OCP\AppFramework\Utility\ITimeFactory;
use OCP\BackgroundJob\IJobList;
use OCP\IConfig;
use function array_map;

class AccountService {
Expand All @@ -44,10 +47,14 @@ class AccountService {
/** @var IMAPClientFactory */
private $imapClientFactory;

public function __construct(MailAccountMapper $mapper,
public function __construct(
MailAccountMapper $mapper,
AliasesService $aliasesService,
IJobList $jobList,
IMAPClientFactory $imapClientFactory) {
IMAPClientFactory $imapClientFactory,
private readonly IConfig $config,
private readonly ITimeFactory $time,
) {
$this->mapper = $mapper;
$this->aliasesService = $aliasesService;
$this->jobList = $jobList;
Expand Down Expand Up @@ -174,6 +181,14 @@ public function save(MailAccount $newAccount): MailAccount {
$this->jobList->add(PreviewEnhancementProcessingJob::class, ['accountId' => $newAccount->getId()]);
$this->jobList->add(QuotaJob::class, ['accountId' => $newAccount->getId()]);

// Set initial heartbeat
$this->config->setUserValue(
$newAccount->getUserId(),
Application::APP_ID,
'ui-heartbeat',
(string)$this->time->getTime(),
);

return $newAccount;
}

Expand Down
6 changes: 5 additions & 1 deletion tests/Integration/MailboxSynchronizationTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
use OCA\Mail\Service\Sync\SyncService;
use OCA\Mail\Tests\Integration\Framework\ImapTest;
use OCA\Mail\Tests\Integration\Framework\ImapTestAccount;
use OCP\AppFramework\Utility\ITimeFactory;
use OCP\IConfig;
use OCP\IRequest;
use OCP\Server;
use Psr\Log\LoggerInterface;
Expand All @@ -46,7 +48,9 @@ protected function setUp(): void {
Server::get(AccountService::class),
$this->getTestAccountUserId(),
Server::get(IMailManager::class),
Server::get(SyncService::class)
Server::get(SyncService::class),
Server::get(IConfig::class),
Server::get(ITimeFactory::class),
);

$this->account = $this->createTestAccount('user12345');
Expand Down
12 changes: 11 additions & 1 deletion tests/Unit/Controller/MailboxesControllerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
use OCA\Mail\Service\AccountService;
use OCA\Mail\Service\Sync\SyncService;
use OCP\AppFramework\Http\JSONResponse;
use OCP\AppFramework\Utility\ITimeFactory;
use OCP\IConfig;
use OCP\IRequest;
use PHPUnit\Framework\MockObject\MockObject;

Expand All @@ -45,20 +47,28 @@ class MailboxesControllerTest extends TestCase {
/** @var SyncService|MockObject */
private $syncService;

private IConfig|MockObject $config;
private ITimeFactory|MockObject $timeFactory;

public function setUp(): void {
parent::setUp();

$this->request = $this->createMock(IRequest::class);
$this->accountService = $this->createMock(AccountService::class);
$this->mailManager = $this->createMock(IMailManager::class);
$this->syncService = $this->createMock(SyncService::class);
$this->config = $this->createMock(IConfig::class);
$this->timeFactory = $this->createMock(ITimeFactory::class);

$this->controller = new MailboxesController(
$this->appName,
$this->request,
$this->accountService,
$this->userId,
$this->mailManager,
$this->syncService
$this->syncService,
$this->config,
$this->timeFactory
);
}

Expand Down
19 changes: 18 additions & 1 deletion tests/Unit/Service/AccountServiceTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@
use OCA\Mail\IMAP\IMAPClientFactory;
use OCA\Mail\Service\AccountService;
use OCA\Mail\Service\AliasesService;
use OCP\AppFramework\Utility\ITimeFactory;
use OCP\BackgroundJob\IJobList;
use OCP\IConfig;
use OCP\IL10N;
use PHPUnit\Framework\MockObject\MockObject;

Expand Down Expand Up @@ -55,6 +57,9 @@ class AccountServiceTest extends TestCase {
/** @var Horde_Imap_Client_Socket|MockObject */
private $client;

private IConfig&MockObject $config;
private ITimeFactory&MockObject $time;

protected function setUp(): void {
parent::setUp();

Expand All @@ -63,11 +68,15 @@ protected function setUp(): void {
$this->aliasesService = $this->createMock(AliasesService::class);
$this->jobList = $this->createMock(IJobList::class);
$this->imapClientFactory = $this->createMock(IMAPClientFactory::class);
$this->config = $this->createMock(IConfig::class);
$this->time = $this->createMock(ITimeFactory::class);
$this->accountService = new AccountService(
$this->mapper,
$this->aliasesService,
$this->jobList,
$this->imapClientFactory
$this->imapClientFactory,
$this->config,
$this->time,
);

$this->account1 = new MailAccount();
Expand Down Expand Up @@ -155,12 +164,20 @@ public function testDeleteByAccountId() {

public function testSave() {
$account = new MailAccount();
$account->setUserId('user1');

$this->mapper->expects($this->once())
->method('save')
->with($account)
->will($this->returnArgument(0));

$this->time->expects(self::once())
->method('getTime')
->willReturn(1755850409);
$this->config->expects(self::once())
->method('setUserValue')
->with('user1', 'mail', 'ui-heartbeat', 1755850409);

$actual = $this->accountService->save($account);

$this->assertEquals($account, $actual);
Expand Down