Merge pull request #1745 from call-me-matt/enh/social-chunks
This commit is contained in:
John Molakvoæ
GitHub
commit
96f05d6a5c
|
|
@ -25,18 +25,49 @@ 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'] ?? null;
|
||||
$offsetContact = $arguments['offsetContact'] ?? null;
|
||||
|
||||
// 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'];
|
||||
|
||||
// make sure the offset contact/address book are still existing
|
||||
if ($this->social->existsAddressBook($stoppedAtBook, $userId) == false) {
|
||||
$stoppedAtBook = null;
|
||||
}
|
||||
if ($this->social->existsContact($stoppedAtContact, $stoppedAtBook, $userId) == false) {
|
||||
$stoppedAtContact = null;
|
||||
}
|
||||
// TODO: can we check the userId still exists?
|
||||
|
||||
$this->jobList->add(self::class, [
|
||||
'userId' => $userId,
|
||||
'offsetBook' => $stoppedAtBook,
|
||||
'offsetContact' => $stoppedAtContact
|
||||
]);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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
|
||||
]);
|
||||
}
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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,12 +77,11 @@ class SocialApiService {
|
|||
$this->l10n = $l10n;
|
||||
$this->urlGen = $urlGen;
|
||||
$this->davBackend = $davBackend;
|
||||
$this->timeFactory = $timeFactory;
|
||||
}
|
||||
|
||||
|
||||
/**
|
||||
* @NoAdminRequired
|
||||
*
|
||||
* returns an array of supported social networks
|
||||
*
|
||||
* @returns {array} array of the supported social networks
|
||||
|
|
@ -93,8 +96,6 @@ class SocialApiService {
|
|||
|
||||
|
||||
/**
|
||||
* @NoAdminRequired
|
||||
*
|
||||
* Adds/updates photo for contact
|
||||
*
|
||||
* @param {pointer} contact reference to the contact to update
|
||||
|
|
@ -123,17 +124,19 @@ class SocialApiService {
|
|||
|
||||
|
||||
/**
|
||||
* @NoAdminRequired
|
||||
*
|
||||
* Gets the addressbook of an addressbookId
|
||||
*
|
||||
* @param {String} addressbookId the identifier of the addressbook
|
||||
* @param {IManager} manager optional a ContactManager to use
|
||||
*
|
||||
* @returns {IAddressBook} the corresponding addressbook or null
|
||||
*/
|
||||
protected function getAddressBook(string $addressbookId) : ?IAddressBook {
|
||||
protected function getAddressBook(string $addressbookId, IManager $manager = null) : ?IAddressBook {
|
||||
$addressBook = null;
|
||||
$addressBooks = $this->manager->getUserAddressBooks();
|
||||
if ($manager === null) {
|
||||
$manager = $this->manager;
|
||||
}
|
||||
$addressBooks = $manager->getUserAddressBooks();
|
||||
foreach ($addressBooks as $ab) {
|
||||
if ($ab->getUri() === $addressbookId) {
|
||||
$addressBook = $ab;
|
||||
|
|
@ -144,8 +147,6 @@ class SocialApiService {
|
|||
|
||||
|
||||
/**
|
||||
* @NoAdminRequired
|
||||
*
|
||||
* Retrieves and initiates all addressbooks from a user
|
||||
*
|
||||
* @param {string} userId the user to query
|
||||
|
|
@ -158,8 +159,6 @@ class SocialApiService {
|
|||
}
|
||||
|
||||
/**
|
||||
* @NoAdminRequired
|
||||
*
|
||||
* Retrieves social profile data for a contact and updates the entry
|
||||
*
|
||||
* @param {String} addressbookId the addressbook identifier
|
||||
|
|
@ -217,8 +216,45 @@ class SocialApiService {
|
|||
}
|
||||
|
||||
/**
|
||||
* @NoAdminRequired
|
||||
* checks an addressbook is existing
|
||||
*
|
||||
* @param {string} searchBookId the UID of the addressbook to verify
|
||||
* @param {string} userId the user that should have access
|
||||
*
|
||||
* @returns {bool} true if the addressbook exists
|
||||
*/
|
||||
public function existsAddressBook(string $searchBookId, string $userId): bool {
|
||||
$manager = $this->manager;
|
||||
$coma = new ContactsManager($this->davBackend, $this->l10n);
|
||||
$coma->setupContactsProvider($manager, $userId, $this->urlGen);
|
||||
$addressBooks = $manager->getUserAddressBooks();
|
||||
return $this->getAddressBook($searchBookId, $manager) !== null;
|
||||
}
|
||||
|
||||
/**
|
||||
* checks a contact exists in an addressbook
|
||||
*
|
||||
* @param string searchContactId the UID of the contact to verify
|
||||
* @param string searchBookId the UID of the addressbook to look in
|
||||
* @param string userId the user that should have access
|
||||
*
|
||||
* @returns bool true if the contact exists
|
||||
*/
|
||||
public function existsContact(string $searchContactId, string $searchBookId, string $userId): bool {
|
||||
// load address books for the user
|
||||
$manager = $this->manager;
|
||||
$coma = new ContactsManager($this->davBackend, $this->l10n);
|
||||
$coma->setupContactsProvider($manager, $userId, $this->urlGen);
|
||||
$addressBook = $this->getAddressBook($searchBookId, $manager);
|
||||
if ($addressBook == null) {
|
||||
return false;
|
||||
}
|
||||
|
||||
$check = $addressBook->search($searchContactId, ['UID'], ['types' => true]);
|
||||
return !empty($check);
|
||||
}
|
||||
|
||||
/**
|
||||
* Stores the result of social avatar updates for each contact
|
||||
* (used during batch updates in updateAddressbooks)
|
||||
*
|
||||
|
|
@ -254,10 +290,31 @@ class SocialApiService {
|
|||
return $report;
|
||||
}
|
||||
|
||||
/**
|
||||
* 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']);
|
||||
}
|
||||
|
||||
/**
|
||||
* Updates social profile data for all contacts of an addressbook
|
||||
*
|
||||
* @param {String} network the social network to use (fallback: take first match)
|
||||
|
|
@ -265,7 +322,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 +333,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 +349,36 @@ 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
|
||||
//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);
|
||||
|
|
@ -309,6 +386,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);
|
||||
|
|
|
|||
|
|
@ -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,80 @@ 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']);
|
||||
}
|
||||
}
|
||||
|
||||
public function testExistsContact() {
|
||||
$this->setupAddressbooks();
|
||||
|
||||
// all good:
|
||||
$result = $this->service->existsContact('11111111-1111-1111-1111-111111111111', 'contacts1', 'admin');
|
||||
$this->assertEquals(true, $result);
|
||||
|
||||
// wrong address book:
|
||||
$result = $this->service->existsContact('22222222-2222-2222-2222-222222222222', 'contacts1', 'admin');
|
||||
$this->assertEquals(false, $result);
|
||||
|
||||
// invalid contactId:
|
||||
$result = $this->service->existsContact('not-existing', 'contacts1', 'admin');
|
||||
$this->assertEquals(false, $result);
|
||||
|
||||
// invalid addressbookId:
|
||||
$result = $this->service->existsContact('11111111-1111-1111-1111-111111111111', 'not-existing', 'admin');
|
||||
$this->assertEquals(false, $result);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue