diff options
author | call-me-matt <nextcloud@matthiasheinisch.de> | 2020-08-11 23:38:06 +0200 |
---|---|---|
committer | call-me-matt <nextcloud@matthiasheinisch.de> | 2020-08-14 12:25:04 +0200 |
commit | 1831bc4470ff551f16236c3f6d89368c8e6a4df2 (patch) | |
tree | dfd647e4925ee9596be99c2878b4decb7863373a | |
parent | 752bb14cd88acb200582724aa430887a76b5b864 (diff) |
update social avatars in chunks
Signed-off-by: call-me-matt <nextcloud@matthiasheinisch.de>
-rw-r--r-- | lib/Cron/SocialUpdate.php | 25 | ||||
-rw-r--r-- | lib/Cron/SocialUpdateRegistration.php | 4 | ||||
-rw-r--r-- | lib/Service/SocialApiService.php | 80 | ||||
-rw-r--r-- | tests/unit/Service/SocialApiServiceTest.php | 66 |
4 files changed, 161 insertions, 14 deletions
diff --git a/lib/Cron/SocialUpdate.php b/lib/Cron/SocialUpdate.php index fe245cdc..af9da459 100644 --- a/lib/Cron/SocialUpdate.php +++ b/lib/Cron/SocialUpdate.php @@ -25,18 +25,39 @@ namespace OCA\Contacts\Cron; 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 + ]); + } } } diff --git a/lib/Cron/SocialUpdateRegistration.php b/lib/Cron/SocialUpdateRegistration.php index 58c7a51a..1f4e1497 100644 --- a/lib/Cron/SocialUpdateRegistration.php +++ b/lib/Cron/SocialUpdateRegistration.php @@ -85,7 +85,9 @@ class SocialUpdateRegistration extends \OC\BackgroundJob\TimedJob { $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 ]); } }); diff --git a/lib/Service/SocialApiService.php b/lib/Service/SocialApiService.php index f0339c07..0d08921a 100644 --- a/lib/Service/SocialApiService.php +++ b/lib/Service/SocialApiService.php @@ -38,6 +38,7 @@ use OCA\DAV\CardDAV\CardDavBackend; 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 @@ class SocialApiService { 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 @@ class SocialApiService { $this->l10n = $l10n; $this->urlGen = $urlGen; $this->davBackend = $davBackend; + $this->timeFactory = $timeFactory; } @@ -254,6 +259,33 @@ class SocialApiService { 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 @@ class SocialApiService { * * @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 @@ class SocialApiService { $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,18 +324,32 @@ class SocialApiService { 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 - $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!) + if (Util::getVersion()[0] < 20) { + //TODO: remove this branch when dependency for contacts is min NCv20 (see info.xml) + $contacts = $addressBook->search('', ['UID'], ['types' => true]); + } else { + $contacts = $addressBook->search('', ['X-SOCIALPROFILE'], ['types' => true]); + } + 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); @@ -309,6 +357,18 @@ class SocialApiService { } 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); diff --git a/tests/unit/Service/SocialApiServiceTest.php b/tests/unit/Service/SocialApiServiceTest.php index a2033a41..47442c88 100644 --- a/tests/unit/Service/SocialApiServiceTest.php +++ b/tests/unit/Service/SocialApiServiceTest.php @@ -37,6 +37,7 @@ use OCA\DAV\CardDAV\CardDavBackend; 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,6 +91,7 @@ class SocialApiServiceTest extends TestCase { $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, @@ -95,7 +99,8 @@ class SocialApiServiceTest extends TestCase { $this->clientService, $this->l10n, $this->urlGen, - $this->davBackend + $this->davBackend, + $this->timeFactory ); } @@ -264,6 +269,9 @@ class SocialApiServiceTest extends TestCase { $this->config ->method('getUserValue') ->willReturn($bgSyncEnabledByUser); + $this->timeFactory + ->method('getTime') + ->willReturn(10); $this->setupAddressbooks(); @@ -284,4 +292,60 @@ class SocialApiServiceTest extends TestCase { $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']); + } + } } |