Skip to content

Commit aa2ca86

Browse files
julien-ncjulien-nc
committed
feat(taskprocessing): avoid generator cascade
Signed-off-by: Julien Veyssier <[email protected]>
1 parent e2c65b2 commit aa2ca86

File tree

4 files changed

+89
-80
lines changed

4 files changed

+89
-80
lines changed

‎core/Command/TaskProcessing/Cleanup.php‎

Lines changed: 49 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -9,16 +9,25 @@
99
namespace OC\Core\Command\TaskProcessing;
1010

1111
use OC\Core\Command\Base;
12+
use OC\TaskProcessing\Db\TaskMapper;
1213
use OC\TaskProcessing\Manager;
14+
use OCP\Files\AppData\IAppDataFactory;
15+
use Psr\Log\LoggerInterface;
1316
use Symfony\Component\Console\Input\InputArgument;
1417
use Symfony\Component\Console\Input\InputInterface;
1518
use Symfony\Component\Console\Output\OutputInterface;
1619

1720
class Cleanup extends Base {
21+
private \OCP\Files\IAppData $appData;
22+
1823
public function __construct(
1924
protected Manager $taskProcessingManager,
25+
private TaskMapper $taskMapper,
26+
private LoggerInterface $logger,
27+
IAppDataFactory $appDataFactory,
2028
) {
2129
parent::__construct();
30+
$this->appData = $appDataFactory->get('core');
2231
}
2332

2433
protected function configure() {
@@ -37,20 +46,48 @@ protected function configure() {
3746
protected function execute(InputInterface $input, OutputInterface $output): int {
3847
$maxAgeSeconds = $input->getArgument('maxAgeSeconds') ?? Manager::MAX_TASK_AGE_SECONDS;
3948
$output->writeln('<comment>Cleanup up tasks older than ' . $maxAgeSeconds . ' seconds and the related output files</comment>');
40-
$cleanupResult = $this->taskProcessingManager->cleanupOldTasks($maxAgeSeconds);
41-
foreach ($cleanupResult as $entry) {
42-
if (isset($entry['task_id'], $entry['file_id'], $entry['file_name'])) {
43-
$output->writeln("<info>\t - " . 'Deleted appData/core/TaskProcessing/' . $entry['file_name'] . ' (fileId: ' . $entry['file_id'] . ', taskId: ' . $entry['task_id'] . ')</info>');
44-
} elseif (isset($entry['directory_name'])) {
45-
$output->writeln("<info>\t - " . 'Deleted appData/core/' . $entry['directory_name'] . '/' . $entry['file_name'] . '</info>');
46-
} elseif (isset($entry['deleted_task_count'])) {
47-
$output->writeln("<comment>\t - " . 'Deleted ' . $entry['deleted_task_count'] . ' tasks from the database</comment>');
48-
} elseif (isset($entry['deleted_task_id_list'])) {
49-
foreach ($entry['deleted_task_id_list'] as $taskId) {
50-
$output->writeln("<info>\t - " . 'Deleted task ' . $taskId . ' from the database</info>');
51-
}
49+
50+
$taskIdsToCleanup = [];
51+
try {
52+
$fileCleanupGenerator = $this->taskProcessingManager->cleanupTaskProcessingTaskFiles($maxAgeSeconds);
53+
foreach ($fileCleanupGenerator as $cleanedUpEntry) {
54+
$output->writeln(
55+
"<info>\t - " . 'Deleted appData/core/TaskProcessing/' . $cleanedUpEntry['file_name']
56+
. ' (fileId: ' . $cleanedUpEntry['file_id'] . ', taskId: ' . $cleanedUpEntry['task_id'] . ')</info>'
57+
);
5258
}
59+
$taskIdsToCleanup = $fileCleanupGenerator->getReturn();
60+
} catch (\Exception $e) {
61+
$this->logger->warning('Failed to delete stale task processing tasks files', ['exception' => $e]);
62+
$output->writeln('<warning>Failed to delete stale task processing tasks files</warning>');
5363
}
64+
try {
65+
$deletedTaskCount = $this->taskMapper->deleteOlderThan($maxAgeSeconds);
66+
foreach ($taskIdsToCleanup as $taskId) {
67+
$output->writeln("<info>\t - " . 'Deleted task ' . $taskId . ' from the database</info>');
68+
}
69+
$output->writeln("<comment>\t - " . 'Deleted ' . $deletedTaskCount . ' tasks from the database</comment>');
70+
} catch (\OCP\DB\Exception $e) {
71+
$this->logger->warning('Failed to delete stale task processing tasks', ['exception' => $e]);
72+
$output->writeln('<warning>Failed to delete stale task processing tasks</warning>');
73+
}
74+
try {
75+
$textToImageDeletedFileNames = $this->taskProcessingManager->clearFilesOlderThan($this->appData->getFolder('text2image'), $maxAgeSeconds);
76+
foreach ($textToImageDeletedFileNames as $entry) {
77+
$output->writeln("<info>\t - " . 'Deleted appData/core/text2image/' . $entry . '</info>');
78+
}
79+
} catch (\OCP\Files\NotFoundException $e) {
80+
// noop
81+
}
82+
try {
83+
$audioToTextDeletedFileNames = $this->taskProcessingManager->clearFilesOlderThan($this->appData->getFolder('audio2text'), $maxAgeSeconds);
84+
foreach ($audioToTextDeletedFileNames as $entry) {
85+
$output->writeln("<info>\t - " . 'Deleted appData/core/audio2text/' . $entry . '</info>');
86+
}
87+
} catch (\OCP\Files\NotFoundException $e) {
88+
// noop
89+
}
90+
5491
return 0;
5592
}
5693
}

‎lib/private/TaskProcessing/Manager.php‎

Lines changed: 3 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -1490,58 +1490,18 @@ public function extractFileIdsFromTask(Task $task): array {
14901490
return $ids;
14911491
}
14921492

1493-
/**
1494-
* @param int $ageInSeconds
1495-
* @return \Generator
1496-
*/
1497-
public function cleanupOldTasks(int $ageInSeconds = self::MAX_TASK_AGE_SECONDS): \Generator {
1498-
$taskIdsToCleanup = [];
1499-
try {
1500-
$fileCleanupGenerator = $this->cleanupTaskProcessingTaskFiles($ageInSeconds);
1501-
foreach ($fileCleanupGenerator as $cleanedUpEntry) {
1502-
yield $cleanedUpEntry;
1503-
}
1504-
$taskIdsToCleanup = $fileCleanupGenerator->getReturn();
1505-
} catch (\Exception $e) {
1506-
$this->logger->warning('Failed to delete stale task processing tasks files', ['exception' => $e]);
1507-
}
1508-
try {
1509-
$deletedTaskCount = $this->taskMapper->deleteOlderThan($ageInSeconds);
1510-
yield ['deleted_task_id_list' => $taskIdsToCleanup];
1511-
yield ['deleted_task_count' => $deletedTaskCount];
1512-
} catch (\OCP\DB\Exception $e) {
1513-
$this->logger->warning('Failed to delete stale task processing tasks', ['exception' => $e]);
1514-
}
1515-
try {
1516-
$textToImageDeletedFiles = $this->clearFilesOlderThan($this->appData->getFolder('text2image'), $ageInSeconds);
1517-
foreach ($textToImageDeletedFiles as $entry) {
1518-
yield $entry;
1519-
}
1520-
} catch (\OCP\Files\NotFoundException $e) {
1521-
// noop
1522-
}
1523-
try {
1524-
$audioToTextDeletedFiles = $this->clearFilesOlderThan($this->appData->getFolder('audio2text'), $ageInSeconds);
1525-
foreach ($audioToTextDeletedFiles as $entry) {
1526-
yield $entry;
1527-
}
1528-
} catch (\OCP\Files\NotFoundException $e) {
1529-
// noop
1530-
}
1531-
}
1532-
15331493
/**
15341494
* @param ISimpleFolder $folder
15351495
* @param int $ageInSeconds
15361496
* @return \Generator
15371497
*/
1538-
private function clearFilesOlderThan(ISimpleFolder $folder, int $ageInSeconds): \Generator {
1498+
public function clearFilesOlderThan(ISimpleFolder $folder, int $ageInSeconds = self::MAX_TASK_AGE_SECONDS): \Generator {
15391499
foreach ($folder->getDirectoryListing() as $file) {
15401500
if ($file->getMTime() < time() - $ageInSeconds) {
15411501
try {
15421502
$fileName = $file->getName();
15431503
$file->delete();
1544-
yield ['directory_name' => $folder->getName(), 'file_name' => $fileName];
1504+
yield $fileName;
15451505
} catch (NotPermittedException $e) {
15461506
$this->logger->warning('Failed to delete a stale task processing file', ['exception' => $e]);
15471507
}
@@ -1558,7 +1518,7 @@ private function clearFilesOlderThan(ISimpleFolder $folder, int $ageInSeconds):
15581518
* @throws \JsonException
15591519
* @throws \OCP\Files\NotFoundException
15601520
*/
1561-
private function cleanupTaskProcessingTaskFiles(int $ageInSeconds): \Generator {
1521+
public function cleanupTaskProcessingTaskFiles(int $ageInSeconds = self::MAX_TASK_AGE_SECONDS): \Generator {
15621522
$taskIdsToCleanup = [];
15631523
foreach ($this->taskMapper->getTasksToCleanup($ageInSeconds) as $task) {
15641524
$taskIdsToCleanup[] = $task->getId();

‎lib/private/TaskProcessing/RemoveOldTasksBackgroundJob.php‎

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,25 +6,52 @@
66
*/
77
namespace OC\TaskProcessing;
88

9+
use OC\TaskProcessing\Db\TaskMapper;
910
use OCP\AppFramework\Utility\ITimeFactory;
1011
use OCP\BackgroundJob\TimedJob;
12+
use OCP\Files\AppData\IAppDataFactory;
13+
use Psr\Log\LoggerInterface;
1114

1215
class RemoveOldTasksBackgroundJob extends TimedJob {
16+
private \OCP\Files\IAppData $appData;
1317

1418
public function __construct(
1519
ITimeFactory $timeFactory,
1620
private Manager $taskProcessingManager,
21+
private TaskMapper $taskMapper,
22+
private LoggerInterface $logger,
23+
IAppDataFactory $appDataFactory,
1724
) {
1825
parent::__construct($timeFactory);
1926
$this->setInterval(60 * 60 * 24);
2027
// can be deferred to maintenance window
2128
$this->setTimeSensitivity(self::TIME_INSENSITIVE);
29+
$this->appData = $appDataFactory->get('core');
2230
}
2331

2432
/**
2533
* @inheritDoc
2634
*/
2735
protected function run($argument): void {
28-
iterator_to_array($this->taskProcessingManager->cleanupOldTasks());
36+
try {
37+
iterator_to_array($this->taskProcessingManager->cleanupTaskProcessingTaskFiles());
38+
} catch (\Exception $e) {
39+
$this->logger->warning('Failed to delete stale task processing tasks files', ['exception' => $e]);
40+
}
41+
try {
42+
$this->taskMapper->deleteOlderThan(Manager::MAX_TASK_AGE_SECONDS);
43+
} catch (\OCP\DB\Exception $e) {
44+
$this->logger->warning('Failed to delete stale task processing tasks', ['exception' => $e]);
45+
}
46+
try {
47+
iterator_to_array($this->taskProcessingManager->clearFilesOlderThan($this->appData->getFolder('text2image')));
48+
} catch (\OCP\Files\NotFoundException $e) {
49+
// noop
50+
}
51+
try {
52+
iterator_to_array($this->taskProcessingManager->clearFilesOlderThan($this->appData->getFolder('audio2text')));
53+
} catch (\OCP\Files\NotFoundException $e) {
54+
// noop
55+
}
2956
}
3057
}

‎tests/lib/TaskProcessing/TaskProcessingTest.php‎

Lines changed: 9 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -943,6 +943,11 @@ public function testOldTasksShouldBeCleanedUp(): void {
943943
$timeFactory->expects($this->any())->method('getDateTime')->willReturnCallback(fn () => $currentTime);
944944
$timeFactory->expects($this->any())->method('getTime')->willReturnCallback(fn () => $currentTime->getTimestamp());
945945

946+
$this->taskMapper = new TaskMapper(
947+
Server::get(IDBConnection::class),
948+
$timeFactory,
949+
);
950+
946951
$this->registrationContext->expects($this->any())->method('getTaskProcessingProviders')->willReturn([
947952
new ServiceRegistration('test', SuccessfulSyncProvider::class)
948953
]);
@@ -963,34 +968,14 @@ public function testOldTasksShouldBeCleanedUp(): void {
963968

964969
$task = $this->manager->getTask($task->getId());
965970

966-
$taskMapper = new TaskMapper(
967-
Server::get(IDBConnection::class),
968-
$timeFactory,
969-
);
970-
$manager = new Manager(
971-
$this->appConfig,
972-
$this->coordinator,
973-
$this->serverContainer,
974-
Server::get(LoggerInterface::class),
975-
$taskMapper,
976-
$this->jobList,
977-
Server::get(IEventDispatcher::class),
978-
Server::get(IAppDataFactory::class),
979-
Server::get(IRootFolder::class),
980-
Server::get(\OC\TextToImage\Manager::class),
981-
$this->userMountCache,
982-
Server::get(IClientService::class),
983-
Server::get(IAppManager::class),
984-
Server::get(IUserManager::class),
985-
Server::get(IUserSession::class),
986-
Server::get(ICacheFactory::class),
987-
);
988971
$currentTime = $currentTime->add(new \DateInterval('P1Y'));
989972
// run background job
990973
$bgJob = new RemoveOldTasksBackgroundJob(
991974
$timeFactory,
992-
// use a locally defined manager to make sure the taskMapper uses the mocked timeFactory
993-
$manager,
975+
$this->manager,
976+
$this->taskMapper,
977+
Server::get(LoggerInterface::class),
978+
Server::get(IAppDataFactory::class),
994979
);
995980
$bgJob->setArgument([]);
996981
$bgJob->start($this->jobList);

0 commit comments

Comments
 (0)