[stable32] fix(trash): allow to configure the trash retention in hours

lib/Cron/DeleteCron.php

@@ -14,13 +14,15 @@
 use OCA\Deck\Db\StackMapper;
 use OCA\Deck\InvalidAttachmentType;
 use OCA\Deck\Service\AttachmentService;
+use OCA\Deck\Service\ConfigService;
 use OCA\Deck\Sharing\DeckShareProvider;
 use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\BackgroundJob\IJob;
 use OCP\BackgroundJob\TimedJob;
 
 class DeleteCron extends TimedJob {
 
+	private $configService;
 	/** @var BoardMapper */
 	private $boardMapper;
 	/** @var CardMapper */
@@ -36,6 +38,7 @@ class DeleteCron extends TimedJob {
 
 	public function __construct(
 		ITimeFactory $time,
+		ConfigService $configService,
 		BoardMapper $boardMapper,
 		CardMapper $cardMapper,
 		AttachmentService $attachmentService,
@@ -44,14 +47,15 @@ public function __construct(
 		DeckShareProvider $deckShareProvider,
 	) {
 		parent::__construct($time);
+		$this->configService = $configService;
 		$this->boardMapper = $boardMapper;
 		$this->cardMapper = $cardMapper;
 		$this->attachmentService = $attachmentService;
 		$this->attachmentMapper = $attachmentMapper;
 		$this->stackMapper = $stackMapper;
 		$this->deckShareProvider = $deckShareProvider;
 
-		$this->setInterval(60 * 60 * 24);
+		$this->setInterval(60 * 60); // Run once every hour
 		$this->setTimeSensitivity(IJob::TIME_INSENSITIVE);
 	}
 
@@ -60,18 +64,19 @@ public function __construct(
 	 * @SuppressWarnings(PHPMD.UnusedFormalParameter)
 	 */
 	protected function run($argument) {
-		$boards = $this->boardMapper->findToDelete();
+		$timeLimit = time() - $this->configService->getTrashRetention();
+
+		$boards = $this->boardMapper->findToDelete($timeLimit);
 		foreach ($boards as $board) {
 			$this->boardMapper->delete($board);
 		}
 
-		$timeLimit = time() - (60 * 5); // 5 min buffer
 		$cards = $this->cardMapper->findToDelete($timeLimit, 500);
 		foreach ($cards as $card) {
 			$this->cardMapper->delete($card);
 		}
 
-		$attachments = $this->attachmentMapper->findToDelete();
+		$attachments = $this->attachmentMapper->findToDelete($timeLimit);
 		foreach ($attachments as $attachment) {
 			try {
 				$service = $this->attachmentService->getService($attachment->getType());
@@ -88,7 +93,7 @@ protected function run($argument) {
 			$this->deckShareProvider->delete($share);
 		}
 
-		$stacks = $this->stackMapper->findToDelete();
+		$stacks = $this->stackMapper->findToDelete($timeLimit);
 		foreach ($stacks as $stack) {
 			$this->stackMapper->delete($stack);
 		}

lib/Db/AttachmentMapper.php

@@ -90,9 +90,7 @@ public function findAll($cardId) {
 	 * @param bool $withOffset
 	 * @return array
 	 */
-	public function findToDelete($cardId = null, $withOffset = true) {
-		// add buffer of 5 min
-		$timeLimit = time() - (60 * 5);
+	public function findToDelete(int $timeLimit, $cardId = null, $withOffset = true) {
 		$qb = $this->db->getQueryBuilder();
 		$qb->select('*')
 			->from($this->getTableName())

lib/Db/BoardMapper.php

@@ -440,9 +440,7 @@ public function findAll(): array {
 		return $this->findEntities($qb);
 	}
 
-	public function findToDelete() {
-		// add buffer of 5 min
-		$timeLimit = time() - (60 * 5);
+	public function findToDelete(int $timeLimit) {
 		$qb = $this->db->getQueryBuilder();
 		$qb->select('id', 'title', 'owner', 'color', 'archived', 'deleted_at', 'last_modified')
 			->from('deck_boards')

lib/Db/StackMapper.php

@@ -165,9 +165,7 @@ public function findBoardId($id): ?int {
 		return $result !== false ? $result : null;
 	}
 
-	public function findToDelete(): array {
-		// add buffer of 5 min
-		$timeLimit = time() - (60 * 5);
+	public function findToDelete(int $timeLimit): array {
 		$qb = $this->db->getQueryBuilder();
 		$qb->select('*')
 			->from($this->getTableName())

lib/Service/AttachmentService.php

@@ -27,6 +27,7 @@
 use OCP\IUserManager;
 
 class AttachmentService {
+	private $configService;
 	private $attachmentMapper;
 	private $cardMapper;
 	private $permissionService;
@@ -49,6 +50,7 @@ class AttachmentService {
 	private AttachmentServiceValidator $attachmentServiceValidator;
 
 	public function __construct(
+		ConfigService $configService,
 		AttachmentMapper $attachmentMapper,
 		CardMapper $cardMapper,
 		IUserManager $userManager,
@@ -61,6 +63,7 @@ public function __construct(
 		ActivityManager $activityManager,
 		AttachmentServiceValidator $attachmentServiceValidator,
 	) {
+		$this->configService = $configService;
 		$this->attachmentMapper = $attachmentMapper;
 		$this->cardMapper = $cardMapper;
 		$this->permissionService = $permissionService;
@@ -115,7 +118,8 @@ public function findAll($cardId, $withDeleted = false) {
 
 		$attachments = $this->attachmentMapper->findAll($cardId);
 		if ($withDeleted) {
-			$attachments = array_merge($attachments, $this->attachmentMapper->findToDelete($cardId, false));
+			$timeLimit = time() - $this->configService->getTrashRetention();
+			$attachments = array_merge($attachments, $this->attachmentMapper->findToDelete($timeLimit, $cardId, false));
 		}
 
 		foreach (array_keys($this->services) as $attachmentType) {

lib/Service/ConfigService.php

@@ -227,4 +227,10 @@ public function setAttachmentFolder(?string $userId, string $path): void {
 
 		$this->config->setUserValue($userId ?? $this->getUserId(), 'deck', 'attachment_folder', $path);
 	}
+
+	public function getTrashRetention(): int {
+		$value = $this->config->getAppValue(Application::APP_ID, 'trashRetentionHours', '5');
+		$hours = (int)$value > 0 ? (int)$value : 5;
+		return 60 * 60 * $hours;
+	}
 }

tests/unit/Cron/DeleteCronTest.php

@@ -34,6 +34,7 @@
 use OCA\Deck\Db\StackMapper;
 use OCA\Deck\InvalidAttachmentType;
 use OCA\Deck\Service\AttachmentService;
+use OCA\Deck\Service\ConfigService;
 use OCA\Deck\Service\IAttachmentService;
 use OCA\Deck\Sharing\DeckShareProvider;
 use OCP\AppFramework\Utility\ITimeFactory;
@@ -44,6 +45,8 @@ class DeleteCronTest extends TestCase {
 
 	/** @var ITimeFactory|MockObject */
 	private $timeFactory;
+	/** @var ConfigService|MockObject */
+	private $configService;
 	/** @var BoardMapper|MockObject */
 	protected $boardMapper;
 	/** @var CardMapper|\PHPUnit\Framework\MockObject\MockObject */
@@ -62,6 +65,7 @@ class DeleteCronTest extends TestCase {
 	public function setUp(): void {
 		parent::setUp();
 		$this->timeFactory = $this->createMock(ITimeFactory::class);
+		$this->configService = $this->createMock(ConfigService::class);
 		$this->boardMapper = $this->createMock(BoardMapper::class);
 		$this->cardMapper = $this->createMock(CardMapper::class);
 		$this->attachmentService = $this->createMock(AttachmentService::class);
@@ -70,6 +74,7 @@ public function setUp(): void {
 		$this->deckShareProvider = $this->createMock(DeckShareProvider::class);
 		$this->deleteCron = new DeleteCron(
 			$this->timeFactory,
+			$this->configService,
 			$this->boardMapper,
 			$this->cardMapper,
 			$this->attachmentService,

tests/unit/Db/AttachmentMapperTest.php

@@ -110,8 +110,9 @@ public function testFindToDelete() {
 			$attachment->resetUpdatedFields();
 		}
 
-		$this->assertEquals([$attachmentsToDelete[0]], $this->attachmentMapper->findToDelete(1));
-		$this->assertEquals([$attachmentsToDelete[2]], $this->attachmentMapper->findToDelete(2));
+		$timeLimit = time() - (60 * 60 * 5);
+		$this->assertEquals([$attachmentsToDelete[0]], $this->attachmentMapper->findToDelete($timeLimit, 1));
+		$this->assertEquals([$attachmentsToDelete[2]], $this->attachmentMapper->findToDelete($timeLimit, 2));
 	}
 
 	public function testIsOwner() {

tests/unit/Db/BoardMapperTest.php

@@ -152,7 +152,8 @@ public function testFindAllToDelete() {
 		$this->boards[0]->setDeletedAt(1);
 		$this->boards[0] = $this->boardMapper->update($this->boards[0]);
 
-		$actual = $this->boardMapper->findToDelete();
+		$timeLimit = time() - (60 * 60 * 5);
+		$actual = $this->boardMapper->findToDelete($timeLimit);
 		$this->boards[0]->resetUpdatedFields();
 		$this->assertEquals([$this->boards[0]], $actual);
 

tests/unit/Service/AttachmentServiceTest.php

@@ -65,6 +65,8 @@ class AttachmentServiceTest extends TestCase {
 
 	/** @var IUserManager|MockObject */
 	private $userManager;
+	/** @var ConfigService */
+	private $configService;
 	/** @var AttachmentMapper|MockObject */
 	private $attachmentMapper;
 	/** @var CardMapper|MockObject */
@@ -107,6 +109,8 @@ public function setUp(): void {
 
 		$this->appContainer = $this->createMock(IAppContainer::class);
 
+		$this->configService = $this->createMock(ConfigService::class);
+
 		$this->userManager = $this->createMock(IUserManager::class);
 		$this->attachmentMapper = $this->createMock(AttachmentMapper::class);
 		$this->cardMapper = $this->createMock(CardMapper::class);
@@ -135,6 +139,7 @@ public function setUp(): void {
 		$this->attachmentServiceValidator = $this->createMock(AttachmentServiceValidator::class);
 
 		$this->attachmentService = new AttachmentService(
+			$this->configService,
 			$this->attachmentMapper,
 			$this->cardMapper,
 			$this->userManager,
@@ -171,7 +176,7 @@ public function testRegisterAttachmentService() {
 		$application->expects($this->any())
 			->method('getContainer')
 			->willReturn($appContainer);
-		$attachmentService = new AttachmentService($this->attachmentMapper, $this->cardMapper, $this->userManager, $this->changeHelper, $this->permissionService, $application, $this->attachmentCacheHelper, $this->userId, $this->l10n, $this->activityManager, $this->attachmentServiceValidator);
+		$attachmentService = new AttachmentService($this->configService, $this->attachmentMapper, $this->cardMapper, $this->userManager, $this->changeHelper, $this->permissionService, $application, $this->attachmentCacheHelper, $this->userId, $this->l10n, $this->activityManager, $this->attachmentServiceValidator);
 		$attachmentService->registerAttachmentService('custom', MyAttachmentService::class);
 		$this->assertEquals($fileServiceMock, $attachmentService->getService('deck_file'));
 		$this->assertEquals(MyAttachmentService::class, get_class($attachmentService->getService('custom')));
@@ -201,7 +206,7 @@ public function testRegisterAttachmentServiceNotExisting() {
 			->method('getContainer')
 			->willReturn($appContainer);
 
-		$attachmentService = new AttachmentService($this->attachmentMapper, $this->cardMapper, $this->userManager, $this->changeHelper, $this->permissionService, $application, $this->attachmentCacheHelper, $this->userId, $this->l10n, $this->activityManager, $this->attachmentServiceValidator);
+		$attachmentService = new AttachmentService($this->configService, $this->attachmentMapper, $this->cardMapper, $this->userManager, $this->changeHelper, $this->permissionService, $application, $this->attachmentCacheHelper, $this->userId, $this->l10n, $this->activityManager, $this->attachmentServiceValidator);
 		$attachmentService->registerAttachmentService('custom', MyAttachmentService::class);
 		$attachmentService->getService('deck_file_invalid');
 	}
@@ -262,9 +267,12 @@ public function testFindAllWithDeleted() {
 			->method('findAll')
 			->with(123)
 			->willReturn($attachments);
+		$this->configService->expects($this->once())
+			->method('getTrashRetention')
+			->willReturn(3600);
 		$this->attachmentMapper->expects($this->once())
 			->method('findToDelete')
-			->with(123, false)
+			->with($this->anything(), 123, false)
 			->willReturn($attachmentsDeleted);
 
 		$this->attachmentServiceImpl->expects($this->exactly(4))

tests/unit/Service/Importer/BoardImportServiceTest.php

@@ -29,6 +29,7 @@
 use OCA\Deck\Db\Assignment;
 use OCA\Deck\Db\AssignmentMapper;
 use OCA\Deck\Db\AttachmentMapper;
+use OCA\Deck\Db\Board;
 use OCA\Deck\Db\BoardMapper;
 use OCA\Deck\Db\Card;
 use OCA\Deck\Db\CardMapper;
@@ -151,6 +152,12 @@ public function testImportSuccess() {
 		$this->userManager->method('userExists')
 			->willReturn(true);
 
+		$board = new Board();
+		$board->setOwner('admin');
+		$this->trelloJsonService
+			->method('getBoard')
+			->willReturn($board);
+
 		$this->boardMapper
 			->expects($this->once())
 			->method('insert');

tests/unit/Service/PermissionServiceTest.php

@@ -358,7 +358,7 @@ public function testFindUsers() {
 		$this->userManager->expects($this->any())
 			->method('userExists')
 			->withConsecutive(['user1'], ['user2'])
-			->willReturnOnConsecutiveCalls($user1, $user2);
+			->willReturnOnConsecutiveCalls(true, true);
 
 		$group = $this->createMock(IGroup::class);
 		$group->expects($this->once())