Skip to content

Commit d5ad42d

Browse files
artongebackportbot[bot]
authored andcommitted
feat: Limit trash expire job to 30 minutes
And pick up where it left off, leveraging getSeenUsers. Signed-off-by: Louis Chemineau <louis@chmn.me> [skip ci]
1 parent 258b2d7 commit d5ad42d

File tree

2 files changed

+36
-32
lines changed

2 files changed

+36
-32
lines changed

apps/files_trashbin/lib/BackgroundJob/ExpireTrash.php

Lines changed: 21 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -31,36 +31,24 @@
3131
use OCA\Files_Trashbin\Trashbin;
3232
use OCP\AppFramework\Utility\ITimeFactory;
3333
use OCP\BackgroundJob\TimedJob;
34-
use OCP\IConfig;
35-
use OCP\IUser;
34+
use OCP\IAppConfig;
3635
use OCP\IUserManager;
3736

3837
class ExpireTrash extends TimedJob {
39-
private IConfig $config;
40-
private Expiration $expiration;
41-
private IUserManager $userManager;
4238

4339
public function __construct(
44-
IConfig $config,
45-
IUserManager $userManager,
46-
Expiration $expiration,
40+
private IAppConfig $appConfig,
41+
private IUserManager $userManager,
42+
private Expiration $expiration,
4743
ITimeFactory $time
4844
) {
4945
parent::__construct($time);
5046
// Run once per 30 minutes
5147
$this->setInterval(60 * 30);
52-
53-
$this->config = $config;
54-
$this->userManager = $userManager;
55-
$this->expiration = $expiration;
5648
}
5749

58-
/**
59-
* @param $argument
60-
* @throws \Exception
61-
*/
6250
protected function run($argument) {
63-
$backgroundJob = $this->config->getAppValue('files_trashbin', 'background_job_expire_trash', 'yes');
51+
$backgroundJob = $this->appConfig->getValueString('files_trashbin', 'background_job_expire_trash', 'yes');
6452
if ($backgroundJob === 'no') {
6553
return;
6654
}
@@ -70,15 +58,28 @@ protected function run($argument) {
7058
return;
7159
}
7260

73-
$this->userManager->callForSeenUsers(function (IUser $user) {
61+
$stopTime = time() + 60 * 30; // Stops after 30 minutes.
62+
$offset = $this->appConfig->getValueInt('files_trashbin', 'background_job_expire_trash_offset', 0);
63+
$users = $this->userManager->getSeenUsers($offset);
64+
65+
foreach ($users as $user) {
7466
$uid = $user->getUID();
7567
if (!$this->setupFS($uid)) {
76-
return;
68+
continue;
7769
}
7870
$dirContent = Helper::getTrashFiles('/', $uid, 'mtime');
7971
Trashbin::deleteExpiredFiles($dirContent, $uid);
80-
});
8172

73+
$offset++;
74+
75+
if ($stopTime < time()) {
76+
$this->appConfig->setValueInt('files_trashbin', 'background_job_expire_trash_offset', $offset);
77+
\OC_Util::tearDownFS();
78+
return;
79+
}
80+
}
81+
82+
$this->appConfig->setValueInt('files_trashbin', 'background_job_expire_trash_offset', 0);
8283
\OC_Util::tearDownFS();
8384
}
8485

apps/files_trashbin/tests/BackgroundJob/ExpireTrashTest.php

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -29,31 +29,31 @@
2929
use OCA\Files_Trashbin\Expiration;
3030
use OCP\AppFramework\Utility\ITimeFactory;
3131
use OCP\BackgroundJob\IJobList;
32-
use OCP\IConfig;
32+
use OCP\IAppConfig;
3333
use OCP\IUserManager;
3434
use PHPUnit\Framework\MockObject\MockObject;
3535
use Test\TestCase;
3636

3737
class ExpireTrashTest extends TestCase {
38-
/** @var IConfig|MockObject */
39-
private $config;
38+
/** @var IAppConfig&MockObject */
39+
private $appConfig;
4040

41-
/** @var IUserManager|MockObject */
41+
/** @var IUserManager&MockObject */
4242
private $userManager;
4343

44-
/** @var Expiration|MockObject */
44+
/** @var Expiration&MockObject */
4545
private $expiration;
4646

47-
/** @var IJobList|MockObject */
47+
/** @var IJobList&MockObject */
4848
private $jobList;
4949

50-
/** @var ITimeFactory|MockObject */
50+
/** @var ITimeFactory&MockObject */
5151
private $time;
5252

5353
protected function setUp(): void {
5454
parent::setUp();
5555

56-
$this->config = $this->createMock(IConfig::class);
56+
$this->appConfig = $this->createMock(IAppConfig::class);
5757
$this->userManager = $this->createMock(IUserManager::class);
5858
$this->expiration = $this->createMock(Expiration::class);
5959
$this->jobList = $this->createMock(IJobList::class);
@@ -69,22 +69,25 @@ protected function setUp(): void {
6969
}
7070

7171
public function testConstructAndRun(): void {
72-
$this->config->method('getAppValue')
72+
$this->appConfig->method('getValueString')
7373
->with('files_trashbin', 'background_job_expire_trash', 'yes')
7474
->willReturn('yes');
75+
$this->appConfig->method('getValueInt')
76+
->with('files_trashbin', 'background_job_expire_trash_offset', 0)
77+
->willReturn(0);
7578

76-
$job = new ExpireTrash($this->config, $this->userManager, $this->expiration, $this->time);
79+
$job = new ExpireTrash($this->appConfig, $this->userManager, $this->expiration, $this->time);
7780
$job->start($this->jobList);
7881
}
7982

8083
public function testBackgroundJobDeactivated(): void {
81-
$this->config->method('getAppValue')
84+
$this->appConfig->method('getValueString')
8285
->with('files_trashbin', 'background_job_expire_trash', 'yes')
8386
->willReturn('no');
8487
$this->expiration->expects($this->never())
8588
->method('getMaxAgeAsTimestamp');
8689

87-
$job = new ExpireTrash($this->config, $this->userManager, $this->expiration, $this->time);
90+
$job = new ExpireTrash($this->appConfig, $this->userManager, $this->expiration, $this->time);
8891
$job->start($this->jobList);
8992
}
9093
}

0 commit comments

Comments
 (0)