Skip to content

Commit

Permalink
Port existing server code to new interface
Browse files Browse the repository at this point in the history
Signed-off-by: Carl Schwan <carl@carlschwan.eu>
  • Loading branch information
CarlSchwan committed Jun 29, 2022
1 parent ad54c7f commit b700c8b
Show file tree
Hide file tree
Showing 24 changed files with 123 additions and 204 deletions.
36 changes: 11 additions & 25 deletions apps/federatedfilesharing/lib/BackgroundJob/RetryJob.php
Original file line number Diff line number Diff line change
Expand Up @@ -41,19 +41,14 @@
* @package OCA\FederatedFileSharing\BackgroundJob
*/
class RetryJob extends Job {

/** @var bool */
private $retainJob = true;

/** @var Notifications */
private $notifications;
private bool $retainJob = true;
private Notifications $notifications;

/** @var int max number of attempts to send the request */
private $maxTry = 20;
private int $maxTry = 20;

/** @var int how much time should be between two tries (10 minutes) */
private $interval = 600;

private int $interval = 600;

public function __construct(Notifications $notifications,
ITimeFactory $time) {
Expand All @@ -62,14 +57,11 @@ public function __construct(Notifications $notifications,
}

/**
* run the job, then remove it from the jobList
*
* @param IJobList $jobList
* @param ILogger|null $logger
* Run the job, then remove it from the jobList
*/
public function execute(IJobList $jobList, ILogger $logger = null) {
public function start(IJobList $jobList): void {
if ($this->shouldRun($this->argument)) {
parent::execute($jobList, $logger);
parent::start($jobList);
$jobList->remove($this, $this->argument);
if ($this->retainJob) {
$this->reAddJob($jobList, $this->argument);
Expand All @@ -93,12 +85,9 @@ protected function run($argument) {
}

/**
* re-add background job with new arguments
*
* @param IJobList $jobList
* @param array $argument
* Re-add background job with new arguments
*/
protected function reAddJob(IJobList $jobList, array $argument) {
protected function reAddJob(IJobList $jobList, array $argument): void {
$jobList->add(RetryJob::class,
[
'remote' => $argument['remote'],
Expand All @@ -113,12 +102,9 @@ protected function reAddJob(IJobList $jobList, array $argument) {
}

/**
* test if it is time for the next run
*
* @param array $argument
* @return bool
* Test if it is time for the next run
*/
protected function shouldRun(array $argument) {
protected function shouldRun(array $argument): bool {
$lastRun = (int)$argument['lastRun'];
return (($this->time->getTime() - $lastRun) > $this->interval);
}
Expand Down
31 changes: 7 additions & 24 deletions apps/federation/lib/BackgroundJob/GetSharedSecret.php
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@
use OCP\Http\Client\IClient;
use OCP\Http\Client\IClientService;
use OCP\Http\Client\IResponse;
use OCP\ILogger;
use OCP\IURLGenerator;
use OCP\OCS\IDiscoveryService;
use Psr\Log\LoggerInterface;
Expand All @@ -60,7 +59,6 @@ class GetSharedSecret extends Job {
private LoggerInterface $logger;
protected bool $retainJob = false;
private string $defaultEndPoint = '/ocs/v2.php/apps/federation/api/v1/shared-secret';

/** 30 day = 2592000sec */
private int $maxLifespan = 2592000;

Expand All @@ -83,16 +81,13 @@ public function __construct(
}

/**
* run the job, then remove it from the joblist
*
* @param IJobList $jobList
* @param ILogger|null $logger
* Run the job, then remove it from the joblist
*/
public function execute(IJobList $jobList, ILogger $logger = null) {
public function start(IJobList $jobList): void {
$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);
parent::start($jobList);
}

$jobList->remove($this, $this->argument);
Expand All @@ -102,16 +97,6 @@ public function execute(IJobList $jobList, ILogger $logger = null) {
}
}

/**
* Call execute() method of parent
*
* @param IJobList $jobList
* @param ILogger $logger
*/
protected function parentExecute($jobList, $logger = null) {
parent::execute($jobList, $logger);
}

protected function run($argument) {
$target = $argument['url'];
$created = isset($argument['created']) ? (int)$argument['created'] : $this->time->getTime();
Expand Down Expand Up @@ -162,12 +147,10 @@ protected function run($argument) {
$status = -1; // There is no status code if we could not connect
$this->logger->info('Could not connect to ' . $target, [
'exception' => $e,
'app' => 'federation',
]);
} catch (\Throwable $e) {
$status = Http::STATUS_INTERNAL_SERVER_ERROR;
$this->logger->error($e->getMessage(), [
'app' => 'federation',
'exception' => $e,
]);
}
Expand All @@ -190,22 +173,22 @@ protected function run($argument) {
);
} else {
$this->logger->error(
'remote server "' . $target . '"" does not return a valid shared secret. Received data: ' . $body,
['app' => 'federation']
'remote server "' . $target . '"" does not return a valid shared secret. Received data: ' . $body,
['app' => 'federation']
);
$this->trustedServers->setServerStatus($target, TrustedServers::STATUS_FAILURE);
}
}
}

/**
* re-add background job
* Re-add background job
*
* @param array $argument
*/
protected function reAddJob(array $argument): void {
$url = $argument['url'];
$created = isset($argument['created']) ? (int)$argument['created'] : $this->time->getTime();
$created = $argument['created'] ?? $this->time->getTime();
$token = $argument['token'];
$this->jobList->add(
GetSharedSecret::class,
Expand Down
3 changes: 1 addition & 2 deletions apps/federation/tests/BackgroundJob/GetSharedSecretTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,7 @@ class GetSharedSecretTest extends TestCase {
/** @var \PHPUnit\Framework\MockObject\MockObject|ITimeFactory */
private $timeFactory;

/** @var GetSharedSecret */
private $getSharedSecret;
private GetSharedSecret $getSharedSecret;

protected function setUp(): void {
parent::setUp();
Expand Down
23 changes: 11 additions & 12 deletions apps/files/lib/BackgroundJob/ScanFiles.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@
namespace OCA\Files\BackgroundJob;

use OC\Files\Utils\Scanner;
use OCP\AppFramework\Utility\ITimeFactory;
use OCP\BackgroundJob\TimedJob;
use OCP\DB\QueryBuilder\IQueryBuilder;
use OCP\EventDispatcher\IEventDispatcher;
use OCP\IConfig;
Expand All @@ -37,13 +39,11 @@
*
* @package OCA\Files\BackgroundJob
*/
class ScanFiles extends \OC\BackgroundJob\TimedJob {
/** @var IConfig */
private $config;
/** @var IEventDispatcher */
private $dispatcher;
class ScanFiles extends TimedJob {
private IConfig $config;
private IEventDispatcher $dispatcher;
private LoggerInterface $logger;
private $connection;
private IDBConnection $connection;

/** Amount of users that should get scanned per execution */
public const USERS_PER_SESSION = 500;
Expand All @@ -52,8 +52,10 @@ public function __construct(
IConfig $config,
IEventDispatcher $dispatcher,
LoggerInterface $logger,
IDBConnection $connection
IDBConnection $connection,
ITimeFactory $time
) {
parent::__construct($time);
// Run once per 10 minutes
$this->setInterval(60 * 10);

Expand All @@ -63,10 +65,7 @@ public function __construct(
$this->connection = $connection;
}

/**
* @param string $user
*/
protected function runScanner(string $user) {
protected function runScanner(string $user): void {
try {
$scanner = new Scanner(
$user,
Expand Down Expand Up @@ -95,7 +94,7 @@ private function getUserToScan() {
->andWhere($query->expr()->gt('parent', $query->createNamedParameter(-1, IQueryBuilder::PARAM_INT)))
->setMaxResults(1);

return $query->execute()->fetchOne();
return $query->executeQuery()->fetchOne();
}

/**
Expand Down
2 changes: 2 additions & 0 deletions apps/files/tests/BackgroundJob/ScanFilesTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
use OC\Files\Mount\MountPoint;
use OC\Files\Storage\Temporary;
use OCA\Files\BackgroundJob\ScanFiles;
use OCP\AppFramework\Utility\ITimeFactory;
use OCP\EventDispatcher\IEventDispatcher;
use OCP\IConfig;
use OCP\IUser;
Expand Down Expand Up @@ -64,6 +65,7 @@ protected function setUp(): void {
$dispatcher,
$logger,
$connection,
$this->createMock(ITimeFactory::class)
])
->setMethods(['runScanner'])
->getMock();
Expand Down
52 changes: 17 additions & 35 deletions apps/files_trashbin/lib/BackgroundJob/ExpireTrash.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,46 +30,30 @@
use OCA\Files_Trashbin\Expiration;
use OCA\Files_Trashbin\Helper;
use OCA\Files_Trashbin\Trashbin;
use OCP\AppFramework\Utility\ITimeFactory;
use OCP\BackgroundJob\TimedJob;
use OCP\IConfig;
use OCP\IUser;
use OCP\IUserManager;

class ExpireTrash extends \OC\BackgroundJob\TimedJob {
class ExpireTrash extends TimedJob {
private IConfig $config;
private Expiration $expiration;
private IUserManager $userManager;

/** @var IConfig */
private $config;

/**
* @var Expiration
*/
private $expiration;

/**
* @var IUserManager
*/
private $userManager;

public function __construct(IConfig $config = null,
IUserManager $userManager = null,
Expiration $expiration = null) {
public function __construct(
IConfig $config,
IUserManager $userManager,
Expiration $expiration,
ITimeFactory $time
) {
parent::__construct($time);
// Run once per 30 minutes
$this->setInterval(60 * 30);

if ($config === null || $expiration === null || $userManager === null) {
$this->fixDIForJobs();
} else {
$this->config = $config;
$this->userManager = $userManager;
$this->expiration = $expiration;
}
}

protected function fixDIForJobs() {
/** @var Application $application */
$application = \OC::$server->query(Application::class);
$this->config = $application->getContainer()->get(IConfig::class);
$this->userManager = \OC::$server->getUserManager();
$this->expiration = $application->getContainer()->query('Expiration');
$this->config = $config;
$this->userManager = $userManager;
$this->expiration = $expiration;
}

/**
Expand Down Expand Up @@ -101,10 +85,8 @@ protected function run($argument) {

/**
* Act on behalf on trash item owner
* @param string $user
* @return boolean
*/
protected function setupFS($user) {
protected function setupFS(string $user): bool {
\OC_Util::tearDownFS();
\OC_Util::setupFS($user);

Expand Down
26 changes: 9 additions & 17 deletions apps/files_versions/lib/BackgroundJob/ExpireVersions.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,27 +27,21 @@

use OCA\Files_Versions\Expiration;
use OCA\Files_Versions\Storage;
use OCP\AppFramework\Utility\ITimeFactory;
use OCP\BackgroundJob\TimedJob;
use OCP\IConfig;
use OCP\IUser;
use OCP\IUserManager;

class ExpireVersions extends \OC\BackgroundJob\TimedJob {
class ExpireVersions extends TimedJob {
public const ITEMS_PER_SESSION = 1000;

/** @var IConfig */
private $config;
private IConfig $config;
private Expiration $expiration;
private IUserManager $userManager;

/**
* @var Expiration
*/
private $expiration;

/**
* @var IUserManager
*/
private $userManager;

public function __construct(IConfig $config, IUserManager $userManager, Expiration $expiration) {
public function __construct(IConfig $config, IUserManager $userManager, Expiration $expiration, ITimeFactory $time) {
parent::__construct($time);
// Run once per 30 minutes
$this->setInterval(60 * 30);

Expand Down Expand Up @@ -78,10 +72,8 @@ public function run($argument) {

/**
* Act on behalf on trash item owner
* @param string $user
* @return boolean
*/
protected function setupFS($user) {
protected function setupFS(string $user): bool {
\OC_Util::tearDownFS();
\OC_Util::setupFS($user);

Expand Down
Loading

0 comments on commit b700c8b

Please sign in to comment.