feat: adjust background sync on user activity #11149

SebastianKrupinski · 2025-05-14T21:55:22Z

Resolves

#10717

Summary

added logic to track mail app activity
added logic to skip back ground sync job if the user is active in the ui
refactored testIsRequest in IMipServiceTest to perform a fake test instead of skipping to play nice with the CI

Signed-off-by: SebastianKrupinski <[email protected]>

SebastianKrupinski · 2025-06-30T17:23:53Z

Failing tests are unrelated, failing due to missing NCU name space interfaces in NC30

kesselb · 2025-07-01T17:59:07Z

Failing tests are unrelated, failing due to missing NCU name space interfaces in NC30

I doubt it ;)

kesselb · 2025-07-01T18:00:10Z

lib/BackgroundJob/SyncJob.php

@@ -79,6 +84,12 @@ protected function run($argument) {
 			return;
 		}

+		if (($this->userConfig->getValueInt($account->getUserId(), Application::APP_ID, 'ui-hearthbeat') - $this->time->getTime()) > 900) {


Suggested change

if (($this->userConfig->getValueInt($account->getUserId(), Application::APP_ID, 'ui-hearthbeat') - $this->time->getTime()) > 900) {

if (($this->userConfig->getValueInt($account->getUserId(), Application::APP_ID, 'ui-heartbeat') - $this->time->getTime()) > 900) {

kesselb · 2025-07-01T18:00:29Z

lib/Controller/MailboxesController.php

@@ -140,6 +136,8 @@ 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->userConfig->setValueInt($this->currentUserId, Application::APP_ID, 'ui-hearthbeat', time());


Suggested change

$this->userConfig->setValueInt($this->currentUserId, Application::APP_ID, 'ui-hearthbeat', time());

$this->userConfig->setValueInt($this->currentUserId, Application::APP_ID, 'ui-heartbeat', time());

kesselb · 2025-07-01T18:03:50Z

lib/Controller/MailboxesController.php

@@ -140,6 +136,8 @@ 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->userConfig->setValueInt($this->currentUserId, Application::APP_ID, 'ui-hearthbeat', time());


Suggested change

$this->userConfig->setValueInt($this->currentUserId, Application::APP_ID, 'ui-hearthbeat', time());

$this->userConfig->setValueInt($this->currentUserId, Application::APP_ID, 'ui-hearthbeat', time());

To make it testable: Replace time() with ITimeFactoy.getTime.

kesselb · 2025-07-01T18:05:24Z

tests/Unit/Service/IMipServiceTest.php

 		if (!method_exists(IManager::class, 'handleImipRequest')) {
-			self::markTestIncomplete();
+			$this->assertTrue(true);


As this seems unrelated, please remove it.

It is unrelated that is why it is in a separate commit. And need to fix a failing nc30 test

No.

See https://github.com/nextcloud/mail/actions/runs/16008830201/job/45161666190?pr=11340 (copy of this branch, without the IMipServiceTest change => only the warning about the incomplet test is gone, but the incomplet test isn't the problem)

=> https://github.com/nextcloud/mail/pull/11149/files#r2178436374

Signed-off-by: SebastianKrupinski <[email protected]>

kesselb · 2025-07-01T20:09:16Z

lib/BackgroundJob/SyncJob.php

 		IUserManager $userManager,
 		AccountService $accountService,
 		MailboxSync $mailboxSync,
 		ImapToDbSynchronizer $syncService,
 		LoggerInterface $logger,
 		IJobList $jobList,
-		IConfig $config) {
+		IConfig $config,
+		private IUserConfig $userConfig,


IUserConfig is new in 31.
Use IConfig.getUserValue instread.

ChristophWurst

From when we discussed the feature I remember saying that we want to track when a user accesses Mail. If they access it frequently we keep the hourly sync. If they don't, we drop down to one sync in four hours or even less often.

The current change tries to detect user activity, then skips syncing but also drops the interval from 60 minutes to 15 minutes.

Can you explain why the implementation was done so different to what we discussed? Are there new findings that make the original idea impossible?

ChristophWurst · 2025-07-02T06:04:53Z

lib/BackgroundJob/SyncJob.php

@@ -79,6 +84,12 @@ protected function run($argument) {
 			return;
 		}

+		if (($this->userConfig->getValueInt($account->getUserId(), Application::APP_ID, 'ui-heartbeat') - $this->time->getTime()) > 900) {
+			$this->logger->debug('Detected user activity, skipping background sync job');
+			$this->setInterval(900);


Does setting the interval of an already running job have any consequence?

SebastianKrupinski · 2025-07-02T12:11:00Z

From when we discussed the feature I remember saying that we want to track when a user accesses Mail. If they access it frequently we keep the hourly sync. If they don't, we drop down to one sync in four hours or even less often.

The current change tries to detect user activity, then skips syncing but also drops the interval from 60 minutes to 15 minutes.

Can you explain why the implementation was done so different to what we discussed? Are there new findings that make the original idea impossible?

Yes, that is what I am trying to do with this.

The idea here is that the background sync will not run as long as there is a up to date heart beat. So as long as the mail UI has been active the background sync will not run, but will revert back to the regular configured interval when there is no activity. Essentially if you are active all day the background sync should not execute. And if you are not active often then it will revert to the default once an hour to sync the mailbox to reduce UI waiting time, and to pick things like iMip.

It only reschdules to 15min when there is a heart beat to check for another heart beat, as the sync can be configure to run every 5min (app.mail.background-sync-interval), I thought it was more sane to check less frequently. We can probably change this to 30min.

ChristophWurst · 2025-07-03T08:04:40Z

The cron sync will sync all INBOXes and mailboxes marked for background sync. Frontend will sync INBOXes and the currently open mailbox. Therefore, the background sync should also run when a user has the app open all day.

SebastianKrupinski self-assigned this May 14, 2025

SebastianKrupinski added this to 💌 📅 👥 Groupware team May 14, 2025

SebastianKrupinski added the 2. developing label May 14, 2025

github-project-automation bot moved this to 🏗️ In progress in 💌 📅 👥 Groupware team May 14, 2025

SebastianKrupinski force-pushed the fix/issue-10717-dynamic-sync-interval branch from 4109db8 to ae2c593 Compare June 30, 2025 14:30

feat: adjust background sync on user activity

ad65815

Signed-off-by: SebastianKrupinski <[email protected]>

SebastianKrupinski force-pushed the fix/issue-10717-dynamic-sync-interval branch from 71b55f7 to ad65815 Compare June 30, 2025 16:42

fix: perform fake test instead of skipping

ce40db2

Signed-off-by: SebastianKrupinski <[email protected]>

SebastianKrupinski force-pushed the fix/issue-10717-dynamic-sync-interval branch from 95d2bca to ce40db2 Compare June 30, 2025 17:16

SebastianKrupinski added 3. to review and removed 2. developing labels Jun 30, 2025

SebastianKrupinski marked this pull request as ready for review June 30, 2025 17:22

SebastianKrupinski requested review from ChristophWurst, kesselb and GretaD as code owners June 30, 2025 17:22

kesselb reviewed Jul 1, 2025

View reviewed changes

fixup! fix: perform fake test instead of skipping

5e75632

Signed-off-by: SebastianKrupinski <[email protected]>

kesselb reviewed Jul 1, 2025

View reviewed changes

ChristophWurst requested changes Jul 2, 2025

View reviewed changes

	if (($this->userConfig->getValueInt($account->getUserId(), Application::APP_ID, 'ui-hearthbeat') - $this->time->getTime()) > 900) {
	if (($this->userConfig->getValueInt($account->getUserId(), Application::APP_ID, 'ui-heartbeat') - $this->time->getTime()) > 900) {

	$this->userConfig->setValueInt($this->currentUserId, Application::APP_ID, 'ui-hearthbeat', time());
	$this->userConfig->setValueInt($this->currentUserId, Application::APP_ID, 'ui-heartbeat', time());

feat: adjust background sync on user activity #11149

Are you sure you want to change the base?

feat: adjust background sync on user activity #11149

Uh oh!

Conversation

SebastianKrupinski commented May 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Resolves

Summary

Uh oh!

SebastianKrupinski commented Jun 30, 2025

Uh oh!

kesselb commented Jul 1, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kesselb Jul 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ChristophWurst left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SebastianKrupinski commented Jul 2, 2025

Uh oh!

ChristophWurst commented Jul 3, 2025

Uh oh!

Uh oh!

SebastianKrupinski commented May 14, 2025 •

edited

Loading

kesselb Jul 1, 2025 •

edited

Loading