Skip to content

Fix lookup server connector background jobs problem #15143

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
rullzer merged 6 commits into from
Jul 18, 2019
Merged

Fix lookup server connector background jobs problem #15143

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
3930b91
Use an application class
nickvergessen Apr 17, 2019
3a4f630
Also send a delete like on the syncronous action
nickvergessen Apr 17, 2019
9927909
Always use a background job for the update
nickvergessen Apr 17, 2019
0061516
Use public interfaces
nickvergessen Apr 17, 2019
e1e1d4d
Yay for null coalescing operator
nickvergessen Apr 17, 2019
30051e7
Make the RetryJob work on the userId only
nickvergessen Apr 17, 2019
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
Prev Previous commit
Make the RetryJob work on the userId only 
To ensure there is only 1 background job per user

Signed-off-by: Joas Schilling <[email protected]>
  • Loading branch information
@nickvergessen @MorrisJobke
nickvergessen authored and MorrisJobke committed Jul 18, 2019
commit 30051e786b7b39b95eef5a878836419125afb6df
166 changes: 114 additions & 52 deletions apps/lookup_server_connector/lib/BackgroundJobs/RetryJob.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
<?php
declare(strict_types=1);
/**
* @copyright Copyright (c) 2016 Bjoern Schiessle <[email protected]>
* @copyright Copyright (c) 2019 Joas Schilling <[email protected]>
*
* @license GNU AGPL version 3 or any later version
*
Expand All @@ -22,43 +24,55 @@
namespace OCA\LookupServerConnector\BackgroundJobs;


use OC\Security\IdentityProof\Signer;
use OCP\Accounts\IAccountManager;
use OCP\AppFramework\Utility\ITimeFactory;
use OCP\BackgroundJob\Job;
use OCP\BackgroundJob\IJobList;
use OCP\Http\Client\IClientService;
use OCP\IConfig;
use OCP\ILogger;
use OCP\IUser;
use OCP\IUserManager;

class RetryJob extends Job {
/** @var IClientService */
private $clientService;
/** @var IJobList */
private $jobList;
/** @var string */
private $lookupServer;
/** @var int how much time should be between two, will be increased for each retry */
private $interval = 100;
/** @var IConfig */
private $config;
/** @var IUserManager */
private $userManager;
/** @var IAccountManager */
private $accountManager;
/** @var Signer */
private $signer;
/** @var int */
protected $retries = 0;
/** @var bool */
protected $retainJob = false;

/**
* @param ITimeFactory $time
* @param IClientService $clientService
* @param IJobList $jobList
* @param IConfig $config
* @param IUserManager $userManager
* @param IAccountManager $accountManager
* @param Signer $signer
*/
public function __construct(ITimeFactory $time,
IClientService $clientService,
IJobList $jobList,
IConfig $config) {
IConfig $config,
IUserManager $userManager,
IAccountManager $accountManager,
Signer $signer) {
parent::__construct($time);
$this->clientService = $clientService;
$this->jobList = $jobList;
$this->config = $config;

if ($config->getSystemValue('has_internet_connection', true) === false) {
return;
}
$this->userManager = $userManager;
$this->accountManager = $accountManager;
$this->signer = $signer;

$this->lookupServer = $config->getSystemValue('lookup_server', 'https://lookup.nextcloud.com');
if (!empty($this->lookupServer)) {
Expand All @@ -74,77 +88,125 @@ public function __construct(ITimeFactory $time,
* @param ILogger|null $logger
*/
public function execute($jobList, ILogger $logger = null): void {
if ($this->shouldRun($this->argument)) {
parent::execute($jobList, $logger);
if (!isset($this->argument['userId'])) {
// Old background job without user id, just drop it.
$jobList->remove($this, $this->argument);
return;
}

$this->retries = (int) $this->config->getUserValue($this->argument['userId'], 'lookup_server_connector', 'update_retries', 0);

if ($this->shouldRemoveBackgroundJob()) {
$jobList->remove($this, $this->argument);
return;
}

if ($this->shouldRun()) {
parent::execute($jobList, $logger);
if (!$this->retainJob) {
$jobList->remove($this, $this->argument);
}
}
}

/**
* Check if we should kill the background job:
*
* - internet connection is disabled
* - no valid lookup server URL given
* - lookup server was disabled by the admin
* - max retries are reached (set to 5)
*
* @return bool
*/
protected function shouldRemoveBackgroundJob(): bool {
return $this->config->getSystemValueBool('has_internet_connection', true) === false ||
$this->config->getSystemValueString('lookup_server', 'https://lookup.nextcloud.com') === '' ||
$this->config->getAppValue('files_sharing', 'lookupServerUploadEnabled', 'yes') !== 'yes' ||
$this->retries >= 5;
}

protected function shouldRun(): bool {
$delay = 100 * 6 ** $this->retries;
return ($this->time->getTime() - $this->lastRun) > $delay;
}

protected function run($argument): void {
if ($this->killBackgroundJob((int)$argument['retryNo'])) {
$user = $this->userManager->get($this->argument['userId']);
if (!$user instanceof IUser) {
// User does not exist anymore
return;
}

$data = $this->getUserAccountData($user);
$signedData = $this->signer->sign('lookupserver', $data, $user);
$client = $this->clientService->newClient();

try {
if (count($argument['dataArray']) === 1) {
if (count($data) === 1) {
// No public data, just the federation Id
$client->delete($this->lookupServer,
[
'body' => json_encode($argument['dataArray']),
'body' => json_encode($signedData),
'timeout' => 10,
'connect_timeout' => 3,
]
);
} else {
$client->post($this->lookupServer,
[
'body' => json_encode($argument['dataArray']),
'body' => json_encode($signedData),
'timeout' => 10,
'connect_timeout' => 3,
]
);
}
} catch (\Exception $e) {
$this->jobList->add(self::class,
[
'dataArray' => $argument['dataArray'],
'retryNo' => $argument['retryNo'] + 1,
'lastRun' => $this->time->getTime(),
]

// Reset retry counter
$this->config->deleteUserValue(
$user->getUID(),
'lookup_server_connector',
'update_retries'
);

} catch (\Exception $e) {
// An error occurred, retry later
$this->retainJob = true;
$this->config->setUserValue(
$user->getUID(),
'lookup_server_connector',
'update_retries',
$this->retries + 1
);
}
}

/**
* test if it is time for the next run
*
* @param array $argument
* @return bool
*/
protected function shouldRun(array $argument): bool {
$retryNo = (int)$argument['retryNo'];
$delay = $this->interval * 6 ** $retryNo;
return !isset($argument['lastRun']) || (($this->time->getTime() - $argument['lastRun']) > $delay);
}
protected function getUserAccountData(IUser $user): array {
$account = $this->accountManager->getAccount($user);

/**
* check if we should kill the background job
*
* The lookup server should no longer be contacted if:
*
* - max retries are reached (set to 5)
* - lookup server was disabled by the admin
* - no valid lookup server URL given
*
* @param int $retryCount
* @return bool
*/
protected function killBackgroundJob(int $retryCount): bool {
$maxTriesReached = $retryCount >= 5;
$lookupServerDisabled = $this->config->getAppValue('files_sharing', 'lookupServerUploadEnabled', 'yes') !== 'yes';
$publicData = [];
foreach ($account->getProperties() as $property) {
if ($property->getScope() === IAccountManager::VISIBILITY_PUBLIC) {
$publicData[$property->getName()] = $property->getValue();
}
}

$data = ['federationId' => $user->getCloudId()];
if (!empty($publicData)) {
$data['name'] = $publicData[IAccountManager::PROPERTY_DISPLAYNAME]['value'] ?? '';
$data['email'] = $publicData[IAccountManager::PROPERTY_EMAIL]['value'] ?? '';
$data['address'] = $publicData[IAccountManager::PROPERTY_ADDRESS]['value'] ?? '';
$data['website'] = $publicData[IAccountManager::PROPERTY_WEBSITE]['value'] ?? '';
$data['twitter'] = $publicData[IAccountManager::PROPERTY_TWITTER]['value'] ?? '';
$data['phone'] = $publicData[IAccountManager::PROPERTY_PHONE]['value'] ?? '';
$data['twitter_signature'] = $publicData[IAccountManager::PROPERTY_TWITTER]['signature'] ?? '';
$data['website_signature'] = $publicData[IAccountManager::PROPERTY_WEBSITE]['signature'] ?? '';
$data['verificationStatus'] = [
IAccountManager::PROPERTY_WEBSITE => $publicData[IAccountManager::PROPERTY_WEBSITE]['verified'] ?? '',
IAccountManager::PROPERTY_TWITTER => $publicData[IAccountManager::PROPERTY_TWITTER]['verified'] ?? '',
];
}

return $maxTriesReached || $lookupServerDisabled || empty($this->lookupServer);
return $data;
}
}
94 changes: 17 additions & 77 deletions apps/lookup_server_connector/lib/UpdateLookupServer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
<?php
declare(strict_types=1);
/**
* @copyright Copyright (c) 2016 Bjoern Schiessle <[email protected]>
* @copyright Copyright (c) 2016 Lukas Reschke <[email protected]>
Expand All @@ -22,11 +23,8 @@

namespace OCA\LookupServerConnector;

use OC\Accounts\AccountManager;
use OC\Security\IdentityProof\Signer;
use OCA\LookupServerConnector\BackgroundJobs\RetryJob;
use OCP\BackgroundJob\IJobList;
use OCP\Http\Client\IClientService;
use OCP\IConfig;
use OCP\IUser;

Expand All @@ -36,109 +34,51 @@
* @package OCA\LookupServerConnector
*/
class UpdateLookupServer {
/** @var AccountManager */
private $accountManager;
/** @var Signer */
private $signer;
/** @var IConfig */
private $config;
/** @var IJobList */
private $jobList;
/** @var string URL point to lookup server */
private $lookupServer;
/** @var bool */
private $lookupServerEnabled;

/**
* @param AccountManager $accountManager
* @param Signer $signer
* @param IJobList $jobList
* @param IConfig $config
*/
public function __construct(AccountManager $accountManager,
Signer $signer,
IJobList $jobList,
public function __construct(IJobList $jobList,
IConfig $config) {
$this->accountManager = $accountManager;
$this->signer = $signer;
$this->config = $config;
$this->jobList = $jobList;

if($config->getSystemValue('has_internet_connection', true) === false) {
return;
}

$this->lookupServerEnabled = $config->getAppValue('files_sharing', 'lookupServerUploadEnabled', 'yes') === 'yes';

$this->lookupServer = $config->getSystemValue('lookup_server', 'https://lookup.nextcloud.com');
if(!empty($this->lookupServer)) {
$this->lookupServer = rtrim($this->lookupServer, '/');
$this->lookupServer .= '/users';
}
}

/**
* @param IUser $user
*/
public function userUpdated(IUser $user): void {

if (!$this->shouldUpdateLookupServer()) {
return;
}

$userData = $this->accountManager->getUser($user);
$publicData = [];

foreach ($userData as $key => $data) {
if ($data['scope'] === AccountManager::VISIBILITY_PUBLIC) {
$publicData[$key] = $data;
}
}

$this->sendToLookupServer($user, $publicData);
}

/**
* send public user data to the lookup server
*
* @param IUser $user
* @param array $publicData
*/
protected function sendToLookupServer(IUser $user, array $publicData): void {

$dataArray = ['federationId' => $user->getCloudId()];

if (!empty($publicData)) {
$dataArray['name'] = $publicData[AccountManager::PROPERTY_DISPLAYNAME]['value'] ?? '';
$dataArray['email'] = $publicData[AccountManager::PROPERTY_EMAIL]['value'] ?? '';
$dataArray['address'] = $publicData[AccountManager::PROPERTY_ADDRESS]['value'] ?? '';
$dataArray['website'] = $publicData[AccountManager::PROPERTY_WEBSITE]['value'] ?? '';
$dataArray['twitter'] = $publicData[AccountManager::PROPERTY_TWITTER]['value'] ?? '';
$dataArray['phone'] = $publicData[AccountManager::PROPERTY_PHONE]['value'] ?? '';
$dataArray['twitter_signature'] = $publicData[AccountManager::PROPERTY_TWITTER]['signature'] ?? '';
$dataArray['website_signature'] = $publicData[AccountManager::PROPERTY_WEBSITE]['signature'] ?? '';
$dataArray['verificationStatus'] = [
AccountManager::PROPERTY_WEBSITE => $publicData[AccountManager::PROPERTY_WEBSITE]['verified'] ?? '',
AccountManager::PROPERTY_TWITTER => $publicData[AccountManager::PROPERTY_TWITTER]['verified'] ?? '',
];
}

$dataArray = $this->signer->sign('lookupserver', $dataArray, $user);
$this->jobList->add(RetryJob::class,
[
'dataArray' => $dataArray,
'retryNo' => 0,
]
// Reset retry counter
$this->config->deleteUserValue(
$user->getUID(),
'lookup_server_connector',
'update_retries'
);
$this->jobList->add(RetryJob::class, ['userId' => $user->getUID()]);
}

/**
* check if we should update the lookup server, we only do it if
*
* * we have a valid URL
* * the lookup server update was enabled by the admin
* + we have an internet connection
* + the lookup server update was not disabled by the admin
* + we have a valid lookup server URL
*
* @return bool
*/
private function shouldUpdateLookupServer(): bool {
return $this->lookupServerEnabled && !empty($this->lookupServer);
return $this->config->getSystemValueBool('has_internet_connection', true) === true &&
$this->config->getAppValue('files_sharing', 'lookupServerUploadEnabled', 'yes') === 'yes' &&
$this->config->getSystemValueString('lookup_server', 'https://lookup.nextcloud.com') !== '';
}

}