mbk-lab
/
nextcloud
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Merge pull request #23388 from owncloud/issue-22887-infinite-background-job-loop-for-old-versions 

Do not create a new job when federation failed to connect but use existing job
This commit is contained in:
Thomas Müller 2016-03-21 18:04:02 +01:00
parent 61629ad9ad b7f7fc7241
commit 36e1476270
4 changed files with 68 additions and 33 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

25
apps/federation/backgroundjob/getsharedsecret.php
View File

@ -25,12 +25,13 @@ namespace OCA\Federation\BackgroundJob;
use GuzzleHttp\Exception\ClientException;
use OC\BackgroundJob\JobList;
use OC\BackgroundJob\QueuedJob;
use OC\BackgroundJob\Job;
use OCA\Federation\DbHandler;
use OCA\Federation\TrustedServers;
use OCP\AppFramework\Http;
use OCP\BackgroundJob\IJobList;
use OCP\Http\Client\IClient;
use OCP\Http\Client\IResponse;
use OCP\ILogger;
use OCP\IURLGenerator;
@ -41,7 +42,7 @@ use OCP\IURLGenerator;
*
* @package OCA\Federation\Backgroundjob
*/
class GetSharedSecret extends QueuedJob{
class GetSharedSecret extends Job{
/** @var IClient */
private $httpClient;
@ -61,6 +62,9 @@ class GetSharedSecret extends QueuedJob{
/** @var ILogger */
private $logger;
/** @var bool */
protected $retainJob = false;
private $endPoint = '/ocs/v2.php/apps/federation/api/v1/shared-secret?format=json';
/**
@ -79,7 +83,7 @@ class GetSharedSecret extends QueuedJob{
IJobList $jobList = null,
TrustedServers $trustedServers = null,
ILogger $logger = null,
dbHandler $dbHandler = null
DbHandler $dbHandler = null
) {
$this->logger = $logger ? $logger : \OC::$server->getLogger();
$this->httpClient = $httpClient ? $httpClient : \OC::$server->getHTTPClientService()->newClient();
@ -108,12 +112,15 @@ class GetSharedSecret extends QueuedJob{
* @param ILogger $logger
*/
public function execute($jobList, ILogger $logger = null) {
$jobList->remove($this, $this->argument);
$target = $this->argument['url'];
// only execute if target is still in the list of trusted domains
if ($this->trustedServers->isTrustedServer($target)) {
$this->parentExecute($jobList, $logger);
}
if (!$this->retainJob) {
$jobList->remove($this, $this->argument);
}
}
/**
@ -132,6 +139,7 @@ class GetSharedSecret extends QueuedJob{
$source = rtrim($source, '/');
$token = $argument['token'];
$result = null;
try {
$result = $this->httpClient->get(
$target . $this->endPoint,
@ -156,7 +164,7 @@ class GetSharedSecret extends QueuedJob{
$this->logger->logException($e, ['app' => 'federation']);
}
} catch (\Exception $e) {
$status = HTTP::STATUS_INTERNAL_SERVER_ERROR;
$status = Http::STATUS_INTERNAL_SERVER_ERROR;
$this->logger->logException($e, ['app' => 'federation']);
}
@ -165,16 +173,13 @@ class GetSharedSecret extends QueuedJob{
$status !== Http::STATUS_OK
&& $status !== Http::STATUS_FORBIDDEN
) {
$this->jobList->add(
'OCA\Federation\BackgroundJob\GetSharedSecret',
$argument
);
$this->retainJob = true;
} else {
// reset token if we received a valid response
$this->dbHandler->addToken($target, '');
}
if ($status === Http::STATUS_OK) {
if ($status === Http::STATUS_OK && $result instanceof IResponse) {
$body = $result->getBody();
$result = json_decode($body, true);
if (isset($result['ocs']['data']['sharedSecret'])) {

23
apps/federation/backgroundjob/requestsharedsecret.php
View File

@ -27,7 +27,7 @@ namespace OCA\Federation\BackgroundJob;
use GuzzleHttp\Exception\ClientException;
use OC\BackgroundJob\JobList;
use OC\BackgroundJob\QueuedJob;
use OC\BackgroundJob\Job;
use OCA\Federation\DbHandler;
use OCA\Federation\TrustedServers;
use OCP\AppFramework\Http;
@ -43,7 +43,7 @@ use OCP\IURLGenerator;
*
* @package OCA\Federation\Backgroundjob
*/
class RequestSharedSecret extends QueuedJob {
class RequestSharedSecret extends Job {
/** @var IClient */
private $httpClient;
@ -65,6 +65,9 @@ class RequestSharedSecret extends QueuedJob {
/** @var ILogger */
private $logger;
/** @var bool */
protected $retainJob = false;
/**
* RequestSharedSecret constructor.
*
@ -79,7 +82,7 @@ class RequestSharedSecret extends QueuedJob {
IURLGenerator $urlGenerator = null,
IJobList $jobList = null,
TrustedServers $trustedServers = null,
dbHandler $dbHandler = null
DbHandler $dbHandler = null
) {
$this->httpClient = $httpClient ? $httpClient : \OC::$server->getHTTPClientService()->newClient();
$this->jobList = $jobList ? $jobList : \OC::$server->getJobList();
@ -109,15 +112,20 @@ class RequestSharedSecret extends QueuedJob {
* @param ILogger $logger
*/
public function execute($jobList, ILogger $logger = null) {
$jobList->remove($this, $this->argument);
$target = $this->argument['url'];
// only execute if target is still in the list of trusted domains
if ($this->trustedServers->isTrustedServer($target)) {
$this->parentExecute($jobList, $logger);
}
if (!$this->retainJob) {
$jobList->remove($this, $this->argument);
}
}
/**
* call execute() method of parent
*
* @param JobList $jobList
* @param ILogger $logger
*/
@ -155,7 +163,7 @@ class RequestSharedSecret extends QueuedJob {
$this->logger->logException($e, ['app' => 'federation']);
}
} catch (\Exception $e) {
$status = HTTP::STATUS_INTERNAL_SERVER_ERROR;
$status = Http::STATUS_INTERNAL_SERVER_ERROR;
$this->logger->logException($e, ['app' => 'federation']);
}
@ -164,10 +172,7 @@ class RequestSharedSecret extends QueuedJob {
$status !== Http::STATUS_OK
&& $status !== Http::STATUS_FORBIDDEN
) {
$this->jobList->add(
'OCA\Federation\BackgroundJob\RequestSharedSecret',
$argument
);
$this->retainJob = true;
}
if ($status === Http::STATUS_FORBIDDEN) {

27
apps/federation/tests/backgroundjob/getsharedsecrettest.php
View File

@ -95,8 +95,9 @@ class GetSharedSecretTest extends TestCase {
* @dataProvider dataTestExecute
*
* @param bool $isTrustedServer
* @param bool $retainBackgroundJob
*/
public function testExecute($isTrustedServer) {
public function testExecute($isTrustedServer, $retainBackgroundJob) {
/** @var GetSharedSecret |\PHPUnit_Framework_MockObject_MockObject $getSharedSecret */
$getSharedSecret = $this->getMockBuilder('OCA\Federation\BackgroundJob\GetSharedSecret')
->setConstructorArgs(
@ -111,7 +112,6 @@ class GetSharedSecretTest extends TestCase {
)->setMethods(['parentExecute'])->getMock();
$this->invokePrivate($getSharedSecret, 'argument', [['url' => 'url']]);
$this->jobList->expects($this->once())->method('remove');
$this->trustedServers->expects($this->once())->method('isTrustedServer')
->with('url')->willReturn($isTrustedServer);
if ($isTrustedServer) {
@ -119,6 +119,12 @@ class GetSharedSecretTest extends TestCase {
} else {
$getSharedSecret->expects($this->never())->method('parentExecute');
}
$this->invokePrivate($getSharedSecret, 'retainJob', [$retainBackgroundJob]);
if ($retainBackgroundJob) {
$this->jobList->expects($this->never())->method('remove');
} else {
$this->jobList->expects($this->once())->method('remove');
}
$getSharedSecret->execute($this->jobList);
@ -126,8 +132,9 @@ class GetSharedSecretTest extends TestCase {
public function dataTestExecute() {
return [
[true],
[false]
[true, true],
[true, false],
[false, false],
];
}
@ -167,12 +174,9 @@ class GetSharedSecretTest extends TestCase {
$statusCode !== Http::STATUS_OK
&& $statusCode !== Http::STATUS_FORBIDDEN
) {
$this->jobList->expects($this->once())->method('add')
->with('OCA\Federation\BackgroundJob\GetSharedSecret', $argument);
$this->dbHandler->expects($this->never())->method('addToken');
} else {
$this->dbHandler->expects($this->once())->method('addToken')->with($target, '');
$this->jobList->expects($this->never())->method('add');
}
if ($statusCode === Http::STATUS_OK) {
@ -185,6 +189,15 @@ class GetSharedSecretTest extends TestCase {
}
$this->invokePrivate($this->getSharedSecret, 'run', [$argument]);
if (
$statusCode !== Http::STATUS_OK
&& $statusCode !== Http::STATUS_FORBIDDEN
) {
$this->assertTrue($this->invokePrivate($this->getSharedSecret, 'retainJob'));
} else {
$this->assertFalse($this->invokePrivate($this->getSharedSecret, 'retainJob'));
}
}
public function dataTestRun() {

26
apps/federation/tests/backgroundjob/requestsharedsecrettest.php
View File

@ -75,8 +75,9 @@ class RequestSharedSecretTest extends TestCase {
* @dataProvider dataTestExecute
*
* @param bool $isTrustedServer
* @param bool $retainBackgroundJob
*/
public function testExecute($isTrustedServer) {
public function testExecute($isTrustedServer, $retainBackgroundJob) {
/** @var RequestSharedSecret |\PHPUnit_Framework_MockObject_MockObject $requestSharedSecret */
$requestSharedSecret = $this->getMockBuilder('OCA\Federation\BackgroundJob\RequestSharedSecret')
->setConstructorArgs(
@ -90,7 +91,6 @@ class RequestSharedSecretTest extends TestCase {
)->setMethods(['parentExecute'])->getMock();
$this->invokePrivate($requestSharedSecret, 'argument', [['url' => 'url']]);
$this->jobList->expects($this->once())->method('remove');
$this->trustedServers->expects($this->once())->method('isTrustedServer')
->with('url')->willReturn($isTrustedServer);
if ($isTrustedServer) {
@ -98,6 +98,12 @@ class RequestSharedSecretTest extends TestCase {
} else {
$requestSharedSecret->expects($this->never())->method('parentExecute');
}
$this->invokePrivate($requestSharedSecret, 'retainJob', [$retainBackgroundJob]);
if ($retainBackgroundJob) {
$this->jobList->expects($this->never())->method('remove');
} else {
$this->jobList->expects($this->once())->method('remove');
}
$requestSharedSecret->execute($this->jobList);
@ -105,8 +111,9 @@ class RequestSharedSecretTest extends TestCase {
public function dataTestExecute() {
return [
[true],
[false]
[true, true],
[true, false],
[false, false],
];
}
@ -146,17 +153,22 @@ class RequestSharedSecretTest extends TestCase {
$statusCode !== Http::STATUS_OK
&& $statusCode !== Http::STATUS_FORBIDDEN
) {
$this->jobList->expects($this->once())->method('add')
->with('OCA\Federation\BackgroundJob\RequestSharedSecret', $argument);
$this->dbHandler->expects($this->never())->method('addToken');
}
if ($statusCode === Http::STATUS_FORBIDDEN) {
$this->jobList->expects($this->never())->method('add');
$this->dbHandler->expects($this->once())->method('addToken')->with($target, '');
}
$this->invokePrivate($this->requestSharedSecret, 'run', [$argument]);
if (
$statusCode !== Http::STATUS_OK
&& $statusCode !== Http::STATUS_FORBIDDEN
) {
$this->assertTrue($this->invokePrivate($this->requestSharedSecret, 'retainJob'));
} else {
$this->assertFalse($this->invokePrivate($this->requestSharedSecret, 'retainJob'));
}
}
public function dataTestRun() {