Update social avatars in chunks

lib/Cron/SocialUpdate.php

@@ -25,18 +25,39 @@
 
 use OCA\Contacts\Service\SocialApiService;
 
+use OCP\AppFramework\Http;
+use OCP\BackgroundJob\IJobList;
+
 class SocialUpdate extends \OC\BackgroundJob\QueuedJob {
 	/** @var SocialUpdateService */
 	private $social;
+	/** @var IJobList */
+	private $jobList;
 
-	public function __construct(SocialApiService $social) {
+	public function __construct(SocialApiService $social,
+								IJobList $jobList) {
 		$this->social = $social;
+		$this->jobList = $jobList;
 	}
 
 	protected function run($arguments) {
 		$userId = $arguments['userId'];
+		$offsetBook = $arguments['offsetBook'];
+		$offsetContact = $arguments['offsetContact'];
 
 		// update contacts with first available social media profile
-		$this->social->updateAddressbooks('any', $userId);
+		$result = $this->social->updateAddressbooks('any', $userId, $offsetBook, $offsetContact);
+
+		if ($result->getStatus() == Http::STATUS_PARTIAL_CONTENT) {
+			// not finished; schedule a follow-up
+			$report = $result->getData();
+			$stoppedAtBook = $report[0]['stoppedAt']['addressBook'];
+			$stoppedAtContact = $report[0]['stoppedAt']['contact'];
+			$this->jobList->add(self::class, [
+				'userId' => $userId,
+				'offsetBook' => $stoppedAtBook,
+				'offsetContact' => $stoppedAtContact
+			]);
+		}
 	}
 }

lib/Cron/SocialUpdateRegistration.php

@@ -85,7 +85,9 @@ protected function run($arguments) {
 			$bgSyncEnabledByUser = $this->config->getUserValue($user->getUID(), $this->appName, 'enableSocialSync', 'no');
 			if ($bgSyncEnabledByUser === 'yes') {
 				$this->jobList->add(SocialUpdate::class, [
-					'userId' => $user->getUID()
+					'userId' => $user->getUID(),
+					'offsetBook' => null,
+					'offsetContact' => null
 				]);
 			}
 		});

lib/Service/SocialApiService.php

@@ -38,6 +38,7 @@
 use OCA\DAV\CardDAV\ContactsManager;
 use OCP\IURLGenerator;
 use OCP\IL10N;
+use OCP\AppFramework\Utility\ITimeFactory;
 
 class SocialApiService {
 	private $appName;
@@ -55,6 +56,8 @@ class SocialApiService {
 	private $urlGen;
 	/** @var CardDavBackend */
 	private $davBackend;
+	/** @var ITimeFactory */
+	private $timeFactory;
 
 
 	public function __construct(
@@ -64,7 +67,8 @@ public function __construct(
 					IClientService $clientService,
 					IL10N $l10n,
 					IURLGenerator $urlGen,
-					CardDavBackend $davBackend) {
+					CardDavBackend $davBackend,
+					ITimeFactory $timeFactory) {
 		$this->appName = Application::APP_ID;
 		$this->socialProvider = $socialProvider;
 		$this->manager = $manager;
@@ -73,6 +77,7 @@ public function __construct(
 		$this->l10n = $l10n;
 		$this->urlGen = $urlGen;
 		$this->davBackend = $davBackend;
+		$this->timeFactory = $timeFactory;
 	}
 
 
@@ -254,6 +259,33 @@ protected function registerUpdateResult(array $report, string $entry, string $st
 		return $report;
 	}
 
+	/**
+	 * @NoAdminRequired
+	 *
+	 * sorts an array of address books
+	 *
+	 * @param {IAddressBook} a
+	 * @param {IAddressBook} b
+	 *
+	 * @returns {bool} comparison by URI
+	 */
+	protected function sortAddressBooks(IAddressBook $a, IAddressBook $b) {
+		return strcmp($a->getURI(), $b->getURI());
+	}
+
+	/**
+	 * @NoAdminRequired
+	 *
+	 * sorts an array of contacts
+	 *
+	 * @param {array} a
+	 * @param {array} b
+	 *
+	 * @returns {bool} comparison by UID
+	 */
+	protected function sortContacts(array $a, array $b) {
+		return strcmp($a['UID'], $b['UID']);
+	}
 
 	/**
 	 * @NoAdminRequired
@@ -265,7 +297,7 @@ protected function registerUpdateResult(array $report, string $entry, string $st
 	 *
 	 * @returns {JSONResponse} JSONResponse with the list of changed and failed contacts
 	 */
-	public function updateAddressbooks(string $network, string $userId) : JSONResponse {
+	public function updateAddressbooks(string $network, string $userId, string $offsetBook = null, string $offsetContact = null) : JSONResponse {
 
 		// double check!
 		$syncAllowedByAdmin = $this->config->getAppValue($this->appName, 'allowSocialSync', 'yes');
@@ -276,10 +308,12 @@ public function updateAddressbooks(string $network, string $userId) : JSONRespon
 
 		$delay = 1;
 		$response = [];
+		$startTime = $this->timeFactory->getTime();
 
 		// get corresponding addressbook
 		$this->registerAddressbooks($userId, $this->manager);
 		$addressBooks = $this->manager->getUserAddressBooks();
+		usort($addressBooks, [$this, 'sortAddressBooks']); // make sure the order stays the same in consecutive calls
 
 		foreach ($addressBooks as $addressBook) {
 			if ((is_null($addressBook) ||
@@ -290,25 +324,55 @@ public function updateAddressbooks(string $network, string $userId) : JSONRespon
 				continue;
 			}
 
+			// in case this is a follow-up, jump to the last stopped address book
+			if (!is_null($offsetBook)) {
+				if ($addressBook->getURI() !== $offsetBook) {
+					continue;
+				}
+				$offsetBook = null;
+			}
+
 			// get contacts in that addressbook
+			//TODO: activate this optimization when nextcloud/server#22085 is merged
+			/*
+			if (Util::getVersion()[0] < 21) {
+				//TODO: remove this branch when dependency for contacts is min NCv21 (see info.xml)
+				$contacts = $addressBook->search('', ['UID'], ['types' => true]);
+			} else {
+				$contacts = $addressBook->search('', ['X-SOCIALPROFILE'], ['types' => true]);
+			}
+			*/
 			$contacts = $addressBook->search('', ['UID'], ['types' => true]);
-			// TODO: can be optimized by:
-			// $contacts = $addressBook->search('', ['X-SOCIALPROFILE'], ['types' => true]);
-			// but see https://github.com/nextcloud/contacts/pull/1722#discussion_r463782429
-			// and the referenced PR before activating this (index has to be re-created!)
+			usort($contacts, [$this, 'sortContacts']); // make sure the order stays the same in consecutive calls
 
 			// update one contact after another
 			foreach ($contacts as $contact) {
-				// delay to prevent rate limiting issues
-				// TODO: do we need to send an Http::STATUS_PROCESSING ?
-				sleep($delay);
+				// in case this is a follow-up, jump to the last stopped contact
+				if (!is_null($offsetContact)) {
+					if ($contact['UID'] !== $offsetContact) {
+						continue;
+					}
+					$offsetContact = null;
+				}
 
 				try {
 					$r = $this->updateContact($addressBook->getURI(), $contact['UID'], $network);
 					$response = $this->registerUpdateResult($response, $contact['FN'], $r->getStatus());
 				} catch (Exception $e) {
 					$response = $this->registerUpdateResult($response, $contact['FN'], '-1');
 				}
+
+				// stop after 15sec (to be continued with next chunk)
+				if (($this->timeFactory->getTime() - $startTime) > 15) {
+					$response['stoppedAt'] = [
+						'addressBook' => $addressBook->getURI(),
+						'contact' => $contact['UID'],
+					];
+					return new JSONResponse([$response], Http::STATUS_PARTIAL_CONTENT);
+				}
+
+				// delay to prevent rate limiting issues
+				sleep($delay);
 			}
 		}
 		return new JSONResponse([$response], Http::STATUS_OK);

tests/unit/Service/SocialApiServiceTest.php

@@ -37,6 +37,7 @@
 use OCP\IURLGenerator;
 use OCP\IL10N;
 use OCP\Util;
+use OCP\AppFramework\Utility\ITimeFactory;
 
 use PHPUnit\Framework\MockObject\MockObject;
 use ChristophWurst\Nextcloud\Testing\TestCase;
@@ -58,6 +59,8 @@ class SocialApiServiceTest extends TestCase {
 	private $urlGen;
 	/** @var CardDavBackend|MockObject */
 	private $davBackend;
+	/** @var ITimeFactory|MockObject */
+	private $timeFactory;
 
 	public function socialProfileProvider() {
 		return [
@@ -88,14 +91,16 @@ public function setUp() {
 		$this->l10n = $this->createMock(IL10N::class);
 		$this->urlGen = $this->createMock(IURLGenerator::class);
 		$this->davBackend = $this->createMock(CardDavBackend::class);
+		$this->timeFactory = $this->createMock(ITimeFactory::class);
 		$this->service = new SocialApiService(
 			$this->socialProvider,
 			$this->manager,
 			$this->config,
 			$this->clientService,
 			$this->l10n,
 			$this->urlGen,
-			$this->davBackend
+			$this->davBackend,
+			$this->timeFactory
 		);
 	}
 
@@ -264,6 +269,9 @@ public function testUpdateAddressbooks($syncAllowedByAdmin, $bgSyncEnabledByUser
 		$this->config
 			->method('getUserValue')
 			->willReturn($bgSyncEnabledByUser);
+		$this->timeFactory
+			->method('getTime')
+			->willReturn(10);
 
 		$this->setupAddressbooks();
 
@@ -284,4 +292,60 @@ public function testUpdateAddressbooks($syncAllowedByAdmin, $bgSyncEnabledByUser
 			$this->assertNotContains('Empty Contact', $report[0]['failed']['404']);
 		}
 	}
+
+
+	public function testUpdateAddressbooksTimeout() {
+		$this->config
+			->method('getAppValue')
+			->willReturn('yes');
+		$this->config
+			->method('getUserValue')
+			->willReturn('yes');
+		$this->timeFactory
+			->method('getTime')
+			->willReturnOnConsecutiveCalls(10, 11, 999);
+
+		$this->setupAddressbooks();
+
+		$result = $this->service->updateAddressbooks('any', 'msstest');
+
+		$this->assertEquals(Http::STATUS_PARTIAL_CONTENT, $result->getStatus());
+
+		$report = $result->getData();
+
+		$this->assertArrayHasKey('0', $report);
+		$this->assertArrayHasKey('stoppedAt', $report[0]);
+		$this->assertArrayHasKey('addressBook', $report[0]['stoppedAt']);
+		$this->assertArrayHasKey('contact', $report[0]['stoppedAt']);
+	}
+
+	/**
+	 * @dataProvider updateAddressbookProvider
+	 */
+	public function testUpdateAddressbooksContinued($syncAllowedByAdmin, $bgSyncEnabledByUser, $expected) {
+		$this->config
+			->method('getAppValue')
+			->willReturn($syncAllowedByAdmin);
+		$this->config
+			->method('getUserValue')
+			->willReturn($bgSyncEnabledByUser);
+		$this->timeFactory
+			->method('getTime')
+			->willReturn(10);
+
+		$this->setupAddressbooks();
+
+		$result = $this->service->updateAddressbooks('any', 'mrstest', 'contacts2', '22222222-2222-2222-2222-222222222222');
+
+		$this->assertEquals($expected, $result->getStatus());
+
+		if (($syncAllowedByAdmin === 'yes') && ($bgSyncEnabledByUser === 'yes')) {
+			$report = $result->getData();
+			$this->assertArrayHasKey('0', $report);
+			$this->assertArrayHasKey('updated', $report[0]);
+			$this->assertNotContains('Valid Contact One', $report[0]['updated']);
+			$this->assertArrayHasKey('checked', $report[0]);
+			$this->assertContains('Valid Contact Two', $report[0]['checked']);
+		}
+	}
 }