From c3f8d07c92ff4c2777b1aaf4f47d0988c1a45bfb Mon Sep 17 00:00:00 2001 From: Martin Chaine Date: Tue, 3 Jun 2025 09:42:44 +0200 Subject: [PATCH 1/8] introduce the EntryDeletion entity --- migrations/Version20250530183653.php | 37 ++++++ src/Entity/EntryDeletion.php | 107 ++++++++++++++++++ .../Subscriber/EntryDeletionSubscriber.php | 36 ++++++ .../EntryDeletionSubscriberTest.php | 45 ++++++++ 4 files changed, 225 insertions(+) create mode 100644 migrations/Version20250530183653.php create mode 100644 src/Entity/EntryDeletion.php create mode 100644 src/Event/Subscriber/EntryDeletionSubscriber.php create mode 100644 tests/Event/Subscriber/EntryDeletionSubscriberTest.php diff --git a/migrations/Version20250530183653.php b/migrations/Version20250530183653.php new file mode 100644 index 000000000..4f8eb201c --- /dev/null +++ b/migrations/Version20250530183653.php @@ -0,0 +1,37 @@ +addSql(<<<'SQL' + CREATE TABLE "wallabag_entry_deletion" (id INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL, user_id INTEGER DEFAULT NULL, entry_id INTEGER NOT NULL, deleted_at DATETIME NOT NULL, CONSTRAINT FK_D91765D5A76ED395 FOREIGN KEY (user_id) REFERENCES "wallabag_user" (id) ON DELETE CASCADE NOT DEFERRABLE INITIALLY IMMEDIATE) + SQL); + $this->addSql(<<<'SQL' + CREATE INDEX IDX_D91765D5A76ED395 ON "wallabag_entry_deletion" (user_id) + SQL); + $this->addSql(<<<'SQL' + CREATE INDEX IDX_D91765D54AF38FD1 ON "wallabag_entry_deletion" (deleted_at) + SQL); + $this->addSql(<<<'SQL' + CREATE INDEX IDX_D91765D5A76ED3954AF38FD1 ON "wallabag_entry_deletion" (user_id, deleted_at) + SQL); + } + + public function down(Schema $schema): void + { + $this->addSql(<<<'SQL' + DROP TABLE "wallabag_entry_deletion" + SQL); + } +} diff --git a/src/Entity/EntryDeletion.php b/src/Entity/EntryDeletion.php new file mode 100644 index 000000000..95f1d4c39 --- /dev/null +++ b/src/Entity/EntryDeletion.php @@ -0,0 +1,107 @@ +user = $user; + $this->entryId = $entryId; + $this->deletedAt = new \DateTime(); + } + + /** + * Get id. + * + * @return int + */ + public function getId() + { + return $this->id; + } + + /** + * Get entryId. + * + * @return int + */ + public function getEntryId() + { + return $this->entryId; + } + + /** + * @return User + */ + public function getUser() + { + return $this->user; + } + + /** + * @return \DateTimeInterface + */ + public function getDeletedAt() + { + return $this->deletedAt; + } + + /** + * Set deleted_at. + * + * @return EntryDeletion + */ + public function setDeletedAt(\DateTimeInterface $deletedAt) + { + $this->deletedAt = $deletedAt; + + return $this; + } + + /** + * Create an EntryDeletion from an Entry that's being deleted. + */ + public static function createFromEntry(Entry $entry): self + { + return new self($entry->getUser(), $entry->getId()); + } +} diff --git a/src/Event/Subscriber/EntryDeletionSubscriber.php b/src/Event/Subscriber/EntryDeletionSubscriber.php new file mode 100644 index 000000000..e9252559e --- /dev/null +++ b/src/Event/Subscriber/EntryDeletionSubscriber.php @@ -0,0 +1,36 @@ + 'onEntryDeleted', + ]; + } + + /** + * Create a deletion event record for the entry. + */ + public function onEntryDeleted(EntryDeletedEvent $event): void + { + $entry = $event->getEntry(); + + $deletionEvent = EntryDeletion::createFromEntry($entry); + + $this->em->persist($deletionEvent); + $this->em->flush(); + } +} diff --git a/tests/Event/Subscriber/EntryDeletionSubscriberTest.php b/tests/Event/Subscriber/EntryDeletionSubscriberTest.php new file mode 100644 index 000000000..eb0eb2037 --- /dev/null +++ b/tests/Event/Subscriber/EntryDeletionSubscriberTest.php @@ -0,0 +1,45 @@ +getProperty('id'); + $property->setAccessible(true); + $property->setValue($entry, 123); + + $em = $this->createMock(EntityManagerInterface::class); + + // when the event is triggered, an EntryDeletion should be persisted and flushed + $em->expects($this->once()) + ->method('persist') + ->with($this->callback(function ($deletion) use ($entry) { + return $deletion instanceof EntryDeletion + && $deletion->getEntryId() === $entry->getId() + && $deletion->getUser() === $entry->getUser(); + })); + $em->expects($this->atLeastOnce()) + ->method('flush'); + + // trigger the event to run the mocked up persist and flush + /** @var EntityManagerInterface $em */ + $subscriber = new EntryDeletionSubscriber($em); + $event = new EntryDeletedEvent($entry); + $subscriber->onEntryDeleted($event); + } +} From a245434d740b55455c978b69d8a5332e84bed887 Mon Sep 17 00:00:00 2001 From: Martin Chaine Date: Tue, 3 Jun 2025 09:45:26 +0200 Subject: [PATCH 2/8] add attributes to define OpenAPI schema more conveniently --- src/OpenApi/Attribute/OrderParameter.php | 23 +++++++ .../Attribute/PagerFanta/JsonContent.php | 60 +++++++++++++++++++ .../Attribute/PagerFanta/PageParameter.php | 23 +++++++ .../Attribute/PagerFanta/PerPageParameter.php | 23 +++++++ 4 files changed, 129 insertions(+) create mode 100644 src/OpenApi/Attribute/OrderParameter.php create mode 100644 src/OpenApi/Attribute/PagerFanta/JsonContent.php create mode 100644 src/OpenApi/Attribute/PagerFanta/PageParameter.php create mode 100644 src/OpenApi/Attribute/PagerFanta/PerPageParameter.php diff --git a/src/OpenApi/Attribute/OrderParameter.php b/src/OpenApi/Attribute/OrderParameter.php new file mode 100644 index 000000000..1b6e67ecf --- /dev/null +++ b/src/OpenApi/Attribute/OrderParameter.php @@ -0,0 +1,23 @@ + Date: Tue, 3 Jun 2025 09:46:55 +0200 Subject: [PATCH 3/8] add an API endpoint to expose EntryDeletion --- fixtures/EntryDeletionFixtures.php | 53 ++++++++++++ .../Api/EntryDeletionRestController.php | 80 +++++++++++++++++++ src/Repository/EntryDeletionRepository.php | 43 ++++++++++ src/Security/Voter/EntryDeletionVoter.php | 43 ++++++++++ .../Api/EntryDeletionRestControllerTest.php | 44 ++++++++++ 5 files changed, 263 insertions(+) create mode 100644 fixtures/EntryDeletionFixtures.php create mode 100644 src/Controller/Api/EntryDeletionRestController.php create mode 100644 src/Repository/EntryDeletionRepository.php create mode 100644 src/Security/Voter/EntryDeletionVoter.php create mode 100644 tests/Controller/Api/EntryDeletionRestControllerTest.php diff --git a/fixtures/EntryDeletionFixtures.php b/fixtures/EntryDeletionFixtures.php new file mode 100644 index 000000000..5547b8f29 --- /dev/null +++ b/fixtures/EntryDeletionFixtures.php @@ -0,0 +1,53 @@ +getReference('admin-user', User::class); + $bobUser = $this->getReference('bob-user', User::class); + + $deletions = [ + [ + 'user' => $adminUser, + 'entry_id' => 1004, + 'deleted_at' => new \DateTime('-4 day'), + ], + [ + 'user' => $adminUser, + 'entry_id' => 1001, + 'deleted_at' => new \DateTime('-1 day'), + ], + [ + 'user' => $bobUser, + 'entry_id' => 1003, + 'deleted_at' => new \DateTime('-3 days'), + ], + ]; + + foreach ($deletions as $deletionData) { + $deletion = new EntryDeletion($deletionData['user'], $deletionData['entry_id']); + $deletion->setDeletedAt($deletionData['deleted_at']); + + $manager->persist($deletion); + } + + $manager->flush(); + } + + public function getDependencies(): array + { + return [ + UserFixtures::class, + EntryFixtures::class, + ]; + } +} diff --git a/src/Controller/Api/EntryDeletionRestController.php b/src/Controller/Api/EntryDeletionRestController.php new file mode 100644 index 000000000..65f1593f6 --- /dev/null +++ b/src/Controller/Api/EntryDeletionRestController.php @@ -0,0 +1,80 @@ + 'json'])] + #[OA\Get( + summary: 'Retrieve all entry deletions for the current user.', + tags: ['EntryDeletions'], + parameters: [ + new OA\Parameter( + name: 'since', + in: 'query', + description: 'The timestamp (in seconds) since when you want entry deletions.', + required: false, + schema: new OA\Schema(type: 'integer') + ), + new WOA\OrderParameter(), + new WOA\PagerFanta\PageParameter(), + new WOA\PagerFanta\PerPageParameter(default: 100) + ], + responses: [ + new OA\Response( + response: 200, + description: 'Returned when successful', + content: new WOA\PagerFanta\JsonContent(EntryDeletion::class) + ) + ] + )] + #[IsGranted('LIST_ENTRIES')] + public function getEntryDeletionsAction(Request $request, EntryDeletionRepository $entryDeletionRepository) + { + $this->validateAuthentication(); + $userId = $this->getUser()->getId(); + + $page = $request->query->get('page', 1); + $perPage = $request->query->get('perPage', 100); + $order = $request->query->get('order', 'desc'); + $since = $request->query->get('since'); + + if (!\in_array($order, ['asc', 'desc'], true)) { + $order = 'desc'; + } + + $pager = $entryDeletionRepository->findEntryDeletions($userId, $since, $order); + $pager->setMaxPerPage($perPage); + $pager->setCurrentPage($page); + + $pagerfantaFactory = new PagerfantaFactory('page', 'perPage'); + $paginatedCollection = $pagerfantaFactory->createRepresentation( + $pager, + new HateoasRoute( + 'api_get_entry_deletions', + [ + 'page' => $page, + 'perPage' => $perPage, + 'order' => $order, + 'since' => $since, + ], + true + ), + ); + + return $this->sendResponse($paginatedCollection); + } +} diff --git a/src/Repository/EntryDeletionRepository.php b/src/Repository/EntryDeletionRepository.php new file mode 100644 index 000000000..613de69d6 --- /dev/null +++ b/src/Repository/EntryDeletionRepository.php @@ -0,0 +1,43 @@ +createQueryBuilder('de') + ->where('de.user = :userId')->setParameter('userId', $userId) + ->orderBy('de.deletedAt', $order); + + if ($since > 0) { + $qb->andWhere('de.deletedAt >= :since') + ->setParameter('since', new \DateTime(date('Y-m-d H:i:s', $since))); + } + + $pagerAdapter = new DoctrineORMAdapter($qb, true, false); + $pager = new Pagerfanta($pagerAdapter); + + return $pager; + } +} diff --git a/src/Security/Voter/EntryDeletionVoter.php b/src/Security/Voter/EntryDeletionVoter.php new file mode 100644 index 000000000..becd90209 --- /dev/null +++ b/src/Security/Voter/EntryDeletionVoter.php @@ -0,0 +1,43 @@ +getUser(); + + if (!$user instanceof User) { + return false; + } + + return match ($attribute) { + self::VIEW, self::LIST => $user === $subject->getUser(), + default => false, + }; + } +} diff --git a/tests/Controller/Api/EntryDeletionRestControllerTest.php b/tests/Controller/Api/EntryDeletionRestControllerTest.php new file mode 100644 index 000000000..941711c3a --- /dev/null +++ b/tests/Controller/Api/EntryDeletionRestControllerTest.php @@ -0,0 +1,44 @@ +client->request('GET', '/api/entry-deletions'); + $this->assertSame(200, $this->client->getResponse()->getStatusCode()); + + $content = json_decode($this->client->getResponse()->getContent(), true); + // check that only the items for the current user are returned + $this->assertEquals(2, \count($content['_embedded']['items'])); + + // validate the deletion schema on the first item + $deletionData = $content['_embedded']['items'][0]; + $this->assertArrayHasKey('id', $deletionData); + $this->assertArrayHasKey('entry_id', $deletionData); + $this->assertArrayHasKey('deleted_at', $deletionData); + $this->assertArrayNotHasKey('user_id', $deletionData); + } + + public function testGetEntryDeletionsSince() + { + // Test date range filter - get deletions from last 2 days + $since = (new \DateTime('-2 days'))->getTimestamp(); + $this->client->request('GET', "/api/entry-deletions?since={$since}"); + $this->assertSame(200, $this->client->getResponse()->getStatusCode()); + + $content = json_decode($this->client->getResponse()->getContent(), true); + $this->assertGreaterThanOrEqual(1, \count($content['_embedded']['items'])); + } +} From 5c823c91a3e79220b683640f007a37333b78b4a4 Mon Sep 17 00:00:00 2001 From: Martin Chaine Date: Tue, 3 Jun 2025 09:47:49 +0200 Subject: [PATCH 4/8] add a Command to purge expired records of EntryDeletion --- src/Command/PurgeEntryDeletionsCommand.php | 94 +++++++++++++++++++ src/Repository/EntryDeletionRepository.php | 20 ++++ .../PurgeEntryDeletionsCommandTest.php | 75 +++++++++++++++ 3 files changed, 189 insertions(+) create mode 100644 src/Command/PurgeEntryDeletionsCommand.php create mode 100644 tests/Command/PurgeEntryDeletionsCommandTest.php diff --git a/src/Command/PurgeEntryDeletionsCommand.php b/src/Command/PurgeEntryDeletionsCommand.php new file mode 100644 index 000000000..c9a416437 --- /dev/null +++ b/src/Command/PurgeEntryDeletionsCommand.php @@ -0,0 +1,94 @@ +addOption( + 'older-than', + null, + InputOption::VALUE_REQUIRED, + 'Purge records older than this date (format: YYYY-MM-DD)', + '-30 days' + ) + ->addOption( + 'dry-run', + null, + InputOption::VALUE_NONE, + 'Do not actually delete records, just show what would be deleted' + ); + } + + protected function execute(InputInterface $input, OutputInterface $output): int + { + $io = new SymfonyStyle($input, $output); + + $olderThan = $input->getOption('older-than'); + $dryRun = (bool) $input->getOption('dry-run'); + + try { + $date = new \DateTime($olderThan); + } catch (\Exception $e) { + $io->error(sprintf('Invalid date format: %s.\nYou can use any format supported by PHP (e.g. YYYY-MM-DD).', $olderThan)); + + return 1; + } + + $count = $this->entryDeletionRepository->countAllBefore($date); + + if ($dryRun) { + $io->text('Dry run mode enabled (no records will be deleted)'); + + return 0; + } + + + if (0 === $count) { + $io->success('No entry deletion records found.'); + + return 0; + } + + if ($dryRun) { + $io->success(sprintf('Would have deleted %d records.', $count)); + + return 0; + } + + $confirmMessage = sprintf( + 'Are you sure you want to delete records older than %s? (count: %d)', + $date->format('Y-m-d'), + $count, + ); + if (!$io->confirm($confirmMessage)) { + return 0; + } + + $this->entryDeletionRepository->deleteAllBefore($date); + + $io->success(sprintf('Successfully deleted %d records.', $count)); + + return 0; + } +} diff --git a/src/Repository/EntryDeletionRepository.php b/src/Repository/EntryDeletionRepository.php index 613de69d6..22d70c3b5 100644 --- a/src/Repository/EntryDeletionRepository.php +++ b/src/Repository/EntryDeletionRepository.php @@ -40,4 +40,24 @@ class EntryDeletionRepository extends ServiceEntityRepository return $pager; } + + public function countAllBefore(\DateTime $date): int + { + return $this->createQueryBuilder('de') + ->select('COUNT(de.id)') + ->where('de.deletedAt < :date') + ->setParameter('date', $date) + ->getQuery() + ->getSingleScalarResult(); + } + + public function deleteAllBefore(\DateTime $date) + { + $this->createQueryBuilder('de') + ->delete() + ->where('de.deletedAt < :date') + ->setParameter('date', $date) + ->getQuery() + ->execute(); + } } diff --git a/tests/Command/PurgeEntryDeletionsCommandTest.php b/tests/Command/PurgeEntryDeletionsCommandTest.php new file mode 100644 index 000000000..e3a07ac96 --- /dev/null +++ b/tests/Command/PurgeEntryDeletionsCommandTest.php @@ -0,0 +1,75 @@ +getTestClient()->getKernel()); + $command = $application->find('wallabag:purge-entry-deletions'); + $dateStr = '-2 days'; + + $tester = new CommandTester($command); + $tester->execute([ + '--older-than' => $dateStr, + '--dry-run' => true, + ]); + + $this->assertStringContainsString('Dry run mode enabled', $tester->getDisplay()); + $this->assertSame(0, $tester->getStatusCode()); + + $em = $this->getEntityManager(); + $count = $em->getRepository(EntryDeletion::class)->countAllBefore(new \DateTime($dateStr)); + $this->assertSame(2, $count); + } + + public function testRunPurgeEntryDeletionsCommand() + { + $application = new Application($this->getTestClient()->getKernel()); + $command = $application->find('wallabag:purge-entry-deletions'); + $dateStr = '-2 days'; + + $tester = new CommandTester($command); + $tester->setInputs(['yes']); // confirm deletion + $tester->execute(['--older-than' => $dateStr]); + + $this->assertStringContainsString('Successfully deleted 2 records', $tester->getDisplay()); + $this->assertSame(0, $tester->getStatusCode()); + + $em = $this->getEntityManager(); + + $count = $em->getRepository(EntryDeletion::class)->countAllBefore(new \DateTime($dateStr)); + $this->assertSame(0, $count); + + $count = $em->getRepository(EntryDeletion::class)->countAllBefore(new \DateTime('now')); + $this->assertSame(1, $count); + } + + public function testRunPurgeEntryDeletionsCommandWithNoRecords() + { + $application = new Application($this->getTestClient()->getKernel()); + $command = $application->find('wallabag:purge-entry-deletions'); + + $tester = new CommandTester($command); + $tester->execute([ + '--older-than' => '-1 year', + ]); + + $this->assertStringContainsString('No entry deletion records found', $tester->getDisplay()); + $this->assertSame(0, $tester->getStatusCode()); + } +} From e08c10af5dc68ef5c1cddd06523cfd5970bbfa9b Mon Sep 17 00:00:00 2001 From: Martin Chaine Date: Tue, 3 Jun 2025 17:20:22 +0200 Subject: [PATCH 5/8] handle expiration time using a parameter --- app/config/services.yml | 6 +++ app/config/wallabag.yml | 1 + src/Command/PurgeEntryDeletionsCommand.php | 25 +++------- src/Helper/EntryDeletionExpirationConfig.php | 33 +++++++++++++ .../PurgeEntryDeletionsCommandTest.php | 49 ++++++++++++------- 5 files changed, 76 insertions(+), 38 deletions(-) create mode 100644 src/Helper/EntryDeletionExpirationConfig.php diff --git a/app/config/services.yml b/app/config/services.yml index 3cca8b30f..f0acf08a2 100644 --- a/app/config/services.yml +++ b/app/config/services.yml @@ -31,6 +31,7 @@ services: $supportUrl: '@=service(''craue_config'').get(''wallabag_support_url'')' $fonts: '%wallabag.fonts%' $defaultIgnoreOriginInstanceRules: '%wallabag.default_ignore_origin_instance_rules%' + $entryDeletionExpirationDays: '%wallabag.entry_deletion_expiration_days%' Wallabag\: resource: '../../src/*' @@ -253,6 +254,11 @@ services: Wallabag\Helper\DownloadImages: arguments: $baseFolder: "%kernel.project_dir%/web/assets/images" + + + Wallabag\Helper\EntryDeletionExpirationConfig: + arguments: + $defaultExpirationDays: '%wallabag.entry_deletion_expiration_days%' Wallabag\Command\InstallCommand: arguments: diff --git a/app/config/wallabag.yml b/app/config/wallabag.yml index 02106a33b..bef3ae4c0 100644 --- a/app/config/wallabag.yml +++ b/app/config/wallabag.yml @@ -32,6 +32,7 @@ parameters: wallabag.action_mark_as_read: 1 wallabag.list_mode: 0 wallabag.display_thumbnails: 1 + wallabag.entry_deletion_expiration_days: 90 wallabag.fetching_error_message_title: 'No title found' wallabag.fetching_error_message: | wallabag can't retrieve contents for this article. Please troubleshoot this issue. diff --git a/src/Command/PurgeEntryDeletionsCommand.php b/src/Command/PurgeEntryDeletionsCommand.php index c9a416437..4d7566eb1 100644 --- a/src/Command/PurgeEntryDeletionsCommand.php +++ b/src/Command/PurgeEntryDeletionsCommand.php @@ -8,6 +8,7 @@ use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Input\InputOption; use Symfony\Component\Console\Output\OutputInterface; use Symfony\Component\Console\Style\SymfonyStyle; +use Wallabag\Helper\EntryDeletionExpirationConfig; use Wallabag\Repository\EntryDeletionRepository; class PurgeEntryDeletionsCommand extends Command @@ -18,6 +19,7 @@ class PurgeEntryDeletionsCommand extends Command public function __construct( private readonly EntityManagerInterface $entityManager, private readonly EntryDeletionRepository $entryDeletionRepository, + private readonly EntryDeletionExpirationConfig $expirationConfig, ) { parent::__construct(); } @@ -25,13 +27,6 @@ class PurgeEntryDeletionsCommand extends Command protected function configure() { $this - ->addOption( - 'older-than', - null, - InputOption::VALUE_REQUIRED, - 'Purge records older than this date (format: YYYY-MM-DD)', - '-30 days' - ) ->addOption( 'dry-run', null, @@ -44,18 +39,10 @@ class PurgeEntryDeletionsCommand extends Command { $io = new SymfonyStyle($input, $output); - $olderThan = $input->getOption('older-than'); $dryRun = (bool) $input->getOption('dry-run'); - try { - $date = new \DateTime($olderThan); - } catch (\Exception $e) { - $io->error(sprintf('Invalid date format: %s.\nYou can use any format supported by PHP (e.g. YYYY-MM-DD).', $olderThan)); - - return 1; - } - - $count = $this->entryDeletionRepository->countAllBefore($date); + $cutoff = $this->expirationConfig->getCutoffDate(); + $count = $this->entryDeletionRepository->countAllBefore($cutoff); if ($dryRun) { $io->text('Dry run mode enabled (no records will be deleted)'); @@ -78,14 +65,14 @@ class PurgeEntryDeletionsCommand extends Command $confirmMessage = sprintf( 'Are you sure you want to delete records older than %s? (count: %d)', - $date->format('Y-m-d'), + $cutoff->format('Y-m-d'), $count, ); if (!$io->confirm($confirmMessage)) { return 0; } - $this->entryDeletionRepository->deleteAllBefore($date); + $this->entryDeletionRepository->deleteAllBefore($cutoff); $io->success(sprintf('Successfully deleted %d records.', $count)); diff --git a/src/Helper/EntryDeletionExpirationConfig.php b/src/Helper/EntryDeletionExpirationConfig.php new file mode 100644 index 000000000..e6d764b24 --- /dev/null +++ b/src/Helper/EntryDeletionExpirationConfig.php @@ -0,0 +1,33 @@ +expirationDays = $defaultExpirationDays; + } + + public function getCutoffDate(): \DateTime + { + return new \DateTime("-{$this->expirationDays} days"); + } + + public function getExpirationDays(): int + { + return $this->expirationDays; + } + + /** + * Override the expiration days parameter. + * This is mostly useful for testing purposes and should not be used in other contexts. + */ + public function setExpirationDays(int $days): self + { + $this->expirationDays = $days; + return $this; + } +} diff --git a/tests/Command/PurgeEntryDeletionsCommandTest.php b/tests/Command/PurgeEntryDeletionsCommandTest.php index e3a07ac96..3fa8872f5 100644 --- a/tests/Command/PurgeEntryDeletionsCommandTest.php +++ b/tests/Command/PurgeEntryDeletionsCommandTest.php @@ -2,10 +2,13 @@ namespace Tests\Wallabag\Command; +use Doctrine\ORM\EntityManagerInterface; use Symfony\Bundle\FrameworkBundle\Console\Application; +use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase; use Symfony\Component\Console\Tester\CommandTester; -use Tests\Wallabag\WallabagTestCase; use Wallabag\Entity\EntryDeletion; +use Wallabag\Helper\EntryDeletionExpirationConfig; +use Wallabag\Repository\EntryDeletionRepository; /** * Test the purge entry deletions command. @@ -15,59 +18,67 @@ use Wallabag\Entity\EntryDeletion; * - Admin user: 1 deletion from 1 day ago (entry_id: 1001) * - Bob user: 1 deletion from 3 days ago (entry_id: 1003) */ -class PurgeEntryDeletionsCommandTest extends WallabagTestCase +class PurgeEntryDeletionsCommandTest extends KernelTestCase { + private EntryDeletionExpirationConfig $expirationConfig; + private EntryDeletionRepository $entryDeletionRepository; + + protected function setUp(): void + { + self::bootKernel(); + + $this->expirationConfig = self::getContainer()->get(EntryDeletionExpirationConfig::class); + $this->expirationConfig->setExpirationDays(2); + + $em = self::getContainer()->get(EntityManagerInterface::class); + $this->entryDeletionRepository = $em->getRepository(EntryDeletion::class); + } + public function testRunPurgeEntryDeletionsCommandWithDryRun() { - $application = new Application($this->getTestClient()->getKernel()); + $application = new Application(self::$kernel); $command = $application->find('wallabag:purge-entry-deletions'); - $dateStr = '-2 days'; $tester = new CommandTester($command); $tester->execute([ - '--older-than' => $dateStr, '--dry-run' => true, ]); $this->assertStringContainsString('Dry run mode enabled', $tester->getDisplay()); $this->assertSame(0, $tester->getStatusCode()); - $em = $this->getEntityManager(); - $count = $em->getRepository(EntryDeletion::class)->countAllBefore(new \DateTime($dateStr)); + $count = $this->entryDeletionRepository->countAllBefore($this->expirationConfig->getCutoffDate()); $this->assertSame(2, $count); } public function testRunPurgeEntryDeletionsCommand() { - $application = new Application($this->getTestClient()->getKernel()); + $application = new Application(self::$kernel); $command = $application->find('wallabag:purge-entry-deletions'); - $dateStr = '-2 days'; $tester = new CommandTester($command); $tester->setInputs(['yes']); // confirm deletion - $tester->execute(['--older-than' => $dateStr]); + $tester->execute([]); $this->assertStringContainsString('Successfully deleted 2 records', $tester->getDisplay()); $this->assertSame(0, $tester->getStatusCode()); - $em = $this->getEntityManager(); - - $count = $em->getRepository(EntryDeletion::class)->countAllBefore(new \DateTime($dateStr)); + $count = $this->entryDeletionRepository->countAllBefore($this->expirationConfig->getCutoffDate()); $this->assertSame(0, $count); - $count = $em->getRepository(EntryDeletion::class)->countAllBefore(new \DateTime('now')); - $this->assertSame(1, $count); + $countAll = $this->entryDeletionRepository->countAllBefore(new \DateTime('now')); + $this->assertSame(1, $countAll); } public function testRunPurgeEntryDeletionsCommandWithNoRecords() { - $application = new Application($this->getTestClient()->getKernel()); + $this->expirationConfig->setExpirationDays(10); + + $application = new Application(self::$kernel); $command = $application->find('wallabag:purge-entry-deletions'); $tester = new CommandTester($command); - $tester->execute([ - '--older-than' => '-1 year', - ]); + $tester->execute([]); $this->assertStringContainsString('No entry deletion records found', $tester->getDisplay()); $this->assertSame(0, $tester->getStatusCode()); From 1883ff138042dce42abaf541fe777662190904c2 Mon Sep 17 00:00:00 2001 From: Martin Chaine Date: Tue, 3 Jun 2025 18:45:01 +0200 Subject: [PATCH 6/8] refuse requests for deletions records that are too old --- .../Api/EntryDeletionRestController.php | 47 +++++++++++++++++-- .../Api/EntryDeletionRestControllerTest.php | 20 +++++++- 2 files changed, 62 insertions(+), 5 deletions(-) diff --git a/src/Controller/Api/EntryDeletionRestController.php b/src/Controller/Api/EntryDeletionRestController.php index 65f1593f6..7ec58197c 100644 --- a/src/Controller/Api/EntryDeletionRestController.php +++ b/src/Controller/Api/EntryDeletionRestController.php @@ -7,8 +7,10 @@ use Hateoas\Representation\Factory\PagerfantaFactory; use OpenApi\Attributes as OA; use Sensio\Bundle\FrameworkExtraBundle\Configuration\IsGranted; use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\HttpKernel\Exception\GoneHttpException; use Symfony\Component\Routing\Annotation\Route; use Wallabag\Entity\EntryDeletion; +use Wallabag\Helper\EntryDeletionExpirationConfig; use Wallabag\Repository\EntryDeletionRepository; use Wallabag\OpenApi\Attribute as WOA; @@ -36,26 +38,63 @@ class EntryDeletionRestController extends WallabagRestController responses: [ new OA\Response( response: 200, - description: 'Returned when successful', + description: 'Returned when successful.', content: new WOA\PagerFanta\JsonContent(EntryDeletion::class) + ), + new OA\Response( + response: 410, + description: 'Returned when the since date is before the cutoff date.', + content: new OA\JsonContent( + type: 'object', + properties: [ + 'message' => new OA\Property(type: 'string'), + ] + ), + headers: [ + new OA\Header( + header: 'X-Wallabag-Entry-Deletion-Cutoff', + description: 'The furthest cutoff timestamp possible for entry deletions.', + schema: new OA\Schema(type: 'integer') + ), + ] ) ] )] #[IsGranted('LIST_ENTRIES')] - public function getEntryDeletionsAction(Request $request, EntryDeletionRepository $entryDeletionRepository) - { + public function getEntryDeletionsAction( + Request $request, + EntryDeletionRepository $entryDeletionRepository, + EntryDeletionExpirationConfig $expirationConfig + ) { $this->validateAuthentication(); $userId = $this->getUser()->getId(); $page = $request->query->get('page', 1); $perPage = $request->query->get('perPage', 100); $order = $request->query->get('order', 'desc'); - $since = $request->query->get('since'); + $since = (int)$request->query->get('since', 0); if (!\in_array($order, ['asc', 'desc'], true)) { $order = 'desc'; } + if (0 < $since) { + $cutoff = $expirationConfig->getCutoffDate()->getTimestamp(); + if ($since < $cutoff) { + // Using a JSON response rather than a GoneHttpException to preserve the error message in production + return $this->json( + [ + 'message' => "The requested since date ({$since}) is before the data retention cutoff date ({$cutoff}).\n" + . "You can get the cutoff date programmatically from the X-Wallabag-Entry-Deletion-Cutoff header.", + ], + 410, + headers: [ + 'X-Wallabag-Entry-Deletion-Cutoff' => $cutoff, + ] + ); + } + } + $pager = $entryDeletionRepository->findEntryDeletions($userId, $since, $order); $pager->setMaxPerPage($perPage); $pager->setCurrentPage($page); diff --git a/tests/Controller/Api/EntryDeletionRestControllerTest.php b/tests/Controller/Api/EntryDeletionRestControllerTest.php index 941711c3a..f2a5359b5 100644 --- a/tests/Controller/Api/EntryDeletionRestControllerTest.php +++ b/tests/Controller/Api/EntryDeletionRestControllerTest.php @@ -33,7 +33,6 @@ class EntryDeletionRestControllerTest extends WallabagApiTestCase public function testGetEntryDeletionsSince() { - // Test date range filter - get deletions from last 2 days $since = (new \DateTime('-2 days'))->getTimestamp(); $this->client->request('GET', "/api/entry-deletions?since={$since}"); $this->assertSame(200, $this->client->getResponse()->getStatusCode()); @@ -41,4 +40,23 @@ class EntryDeletionRestControllerTest extends WallabagApiTestCase $content = json_decode($this->client->getResponse()->getContent(), true); $this->assertGreaterThanOrEqual(1, \count($content['_embedded']['items'])); } + + public function testGetEntryDeletionsWithSinceBeforeCutoffDate() + { + $sinceBeforeCutoff = (new \DateTime('-410 days'))->getTimestamp(); + $this->client->request('GET', "/api/entry-deletions?since={$sinceBeforeCutoff}"); + $this->assertSame(410, $this->client->getResponse()->getStatusCode()); + + $content = $this->client->getResponse()->getContent(); + $this->assertStringContainsString('The requested since date', $content); + $this->assertStringContainsString('is before the data retention cutoff date', $content); + $this->assertStringContainsString('X-Wallabag-Entry-Deletion-Cutoff', $content); + + $response = $this->client->getResponse(); + $this->assertTrue($response->headers->has('X-Wallabag-Entry-Deletion-Cutoff')); + + $cutoffTimestamp = $response->headers->get('X-Wallabag-Entry-Deletion-Cutoff'); + $this->assertIsNumeric($cutoffTimestamp); + $this->assertGreaterThan($sinceBeforeCutoff, (int) $cutoffTimestamp); + } } From 1fb17a17b6ff7281ca39c3d9895047f818a9d2ef Mon Sep 17 00:00:00 2001 From: Martin Chaine Date: Wed, 4 Jun 2025 11:36:44 +0200 Subject: [PATCH 7/8] round of `make phpstan fix-cs` --- src/Command/PurgeEntryDeletionsCommand.php | 18 +++++++----------- .../Api/EntryDeletionRestController.php | 19 +++++++++---------- src/Helper/EntryDeletionExpirationConfig.php | 1 + src/OpenApi/Attribute/OrderParameter.php | 3 +-- .../Attribute/PagerFanta/JsonContent.php | 9 ++++----- .../Attribute/PagerFanta/PageParameter.php | 3 +-- .../Attribute/PagerFanta/PerPageParameter.php | 3 +-- src/Repository/EntryDeletionRepository.php | 8 +------- .../PurgeEntryDeletionsCommandTest.php | 2 +- .../Api/EntryDeletionRestControllerTest.php | 6 +++--- 10 files changed, 29 insertions(+), 43 deletions(-) diff --git a/src/Command/PurgeEntryDeletionsCommand.php b/src/Command/PurgeEntryDeletionsCommand.php index 4d7566eb1..4c76d8d3a 100644 --- a/src/Command/PurgeEntryDeletionsCommand.php +++ b/src/Command/PurgeEntryDeletionsCommand.php @@ -2,7 +2,6 @@ namespace Wallabag\Command; -use Doctrine\ORM\EntityManagerInterface; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Input\InputOption; @@ -17,7 +16,6 @@ class PurgeEntryDeletionsCommand extends Command protected static $defaultDescription = 'Purge old entry deletion records'; public function __construct( - private readonly EntityManagerInterface $entityManager, private readonly EntryDeletionRepository $entryDeletionRepository, private readonly EntryDeletionExpirationConfig $expirationConfig, ) { @@ -46,24 +44,22 @@ class PurgeEntryDeletionsCommand extends Command if ($dryRun) { $io->text('Dry run mode enabled (no records will be deleted)'); + if (0 === $count) { + $io->success('No entry deletion records found.'); + } else { + $io->success(\sprintf('Would have deleted %d records.', $count)); + } return 0; } - if (0 === $count) { $io->success('No entry deletion records found.'); return 0; } - if ($dryRun) { - $io->success(sprintf('Would have deleted %d records.', $count)); - - return 0; - } - - $confirmMessage = sprintf( + $confirmMessage = \sprintf( 'Are you sure you want to delete records older than %s? (count: %d)', $cutoff->format('Y-m-d'), $count, @@ -74,7 +70,7 @@ class PurgeEntryDeletionsCommand extends Command $this->entryDeletionRepository->deleteAllBefore($cutoff); - $io->success(sprintf('Successfully deleted %d records.', $count)); + $io->success(\sprintf('Successfully deleted %d records.', $count)); return 0; } diff --git a/src/Controller/Api/EntryDeletionRestController.php b/src/Controller/Api/EntryDeletionRestController.php index 7ec58197c..29d8edacd 100644 --- a/src/Controller/Api/EntryDeletionRestController.php +++ b/src/Controller/Api/EntryDeletionRestController.php @@ -7,12 +7,11 @@ use Hateoas\Representation\Factory\PagerfantaFactory; use OpenApi\Attributes as OA; use Sensio\Bundle\FrameworkExtraBundle\Configuration\IsGranted; use Symfony\Component\HttpFoundation\Request; -use Symfony\Component\HttpKernel\Exception\GoneHttpException; use Symfony\Component\Routing\Annotation\Route; use Wallabag\Entity\EntryDeletion; use Wallabag\Helper\EntryDeletionExpirationConfig; -use Wallabag\Repository\EntryDeletionRepository; use Wallabag\OpenApi\Attribute as WOA; +use Wallabag\Repository\EntryDeletionRepository; class EntryDeletionRestController extends WallabagRestController { @@ -33,7 +32,7 @@ class EntryDeletionRestController extends WallabagRestController ), new WOA\OrderParameter(), new WOA\PagerFanta\PageParameter(), - new WOA\PagerFanta\PerPageParameter(default: 100) + new WOA\PagerFanta\PerPageParameter(default: 100), ], responses: [ new OA\Response( @@ -57,22 +56,22 @@ class EntryDeletionRestController extends WallabagRestController schema: new OA\Schema(type: 'integer') ), ] - ) + ), ] )] #[IsGranted('LIST_ENTRIES')] public function getEntryDeletionsAction( Request $request, EntryDeletionRepository $entryDeletionRepository, - EntryDeletionExpirationConfig $expirationConfig + EntryDeletionExpirationConfig $expirationConfig, ) { $this->validateAuthentication(); $userId = $this->getUser()->getId(); - $page = $request->query->get('page', 1); - $perPage = $request->query->get('perPage', 100); - $order = $request->query->get('order', 'desc'); - $since = (int)$request->query->get('since', 0); + $page = $request->query->getInt('page', 1); + $perPage = $request->query->getInt('perPage', 100); + $order = strtolower($request->query->get('order', 'desc')); + $since = $request->query->getInt('since', 0); if (!\in_array($order, ['asc', 'desc'], true)) { $order = 'desc'; @@ -85,7 +84,7 @@ class EntryDeletionRestController extends WallabagRestController return $this->json( [ 'message' => "The requested since date ({$since}) is before the data retention cutoff date ({$cutoff}).\n" - . "You can get the cutoff date programmatically from the X-Wallabag-Entry-Deletion-Cutoff header.", + . 'You can get the cutoff date programmatically from the X-Wallabag-Entry-Deletion-Cutoff header.', ], 410, headers: [ diff --git a/src/Helper/EntryDeletionExpirationConfig.php b/src/Helper/EntryDeletionExpirationConfig.php index e6d764b24..55d981693 100644 --- a/src/Helper/EntryDeletionExpirationConfig.php +++ b/src/Helper/EntryDeletionExpirationConfig.php @@ -28,6 +28,7 @@ class EntryDeletionExpirationConfig public function setExpirationDays(int $days): self { $this->expirationDays = $days; + return $this; } } diff --git a/src/OpenApi/Attribute/OrderParameter.php b/src/OpenApi/Attribute/OrderParameter.php index 1b6e67ecf..2bd856060 100644 --- a/src/OpenApi/Attribute/OrderParameter.php +++ b/src/OpenApi/Attribute/OrderParameter.php @@ -3,9 +3,8 @@ namespace Wallabag\OpenApi\Attribute; use OpenApi\Attributes as OA; -use Attribute; -#[Attribute(Attribute::TARGET_METHOD)] +#[\Attribute(\Attribute::TARGET_METHOD)] class OrderParameter extends OA\Parameter { public function __construct( diff --git a/src/OpenApi/Attribute/PagerFanta/JsonContent.php b/src/OpenApi/Attribute/PagerFanta/JsonContent.php index eb0850056..2e27b2daf 100644 --- a/src/OpenApi/Attribute/PagerFanta/JsonContent.php +++ b/src/OpenApi/Attribute/PagerFanta/JsonContent.php @@ -4,9 +4,8 @@ namespace Wallabag\OpenApi\Attribute\PagerFanta; use Nelmio\ApiDocBundle\Attribute\Model; use OpenApi\Attributes as OA; -use Attribute; -#[Attribute(Attribute::TARGET_CLASS)] +#[\Attribute(\Attribute::TARGET_CLASS)] class JsonContent extends OA\JsonContent { public function __construct(string|array|null $modelClass = null) @@ -21,7 +20,7 @@ class JsonContent extends OA\JsonContent property: 'items', type: 'array', items: new OA\Items(ref: new Model(type: $modelClass)) - ) + ), ] ), new OA\Property(property: 'page', type: 'integer'), @@ -51,9 +50,9 @@ class JsonContent extends OA\JsonContent property: 'next', type: 'object', properties: [new OA\Property(property: 'href', type: 'string')] - ) + ), ] - ) + ), ] ); } diff --git a/src/OpenApi/Attribute/PagerFanta/PageParameter.php b/src/OpenApi/Attribute/PagerFanta/PageParameter.php index 49460cb5f..110699383 100644 --- a/src/OpenApi/Attribute/PagerFanta/PageParameter.php +++ b/src/OpenApi/Attribute/PagerFanta/PageParameter.php @@ -3,9 +3,8 @@ namespace Wallabag\OpenApi\Attribute\PagerFanta; use OpenApi\Attributes as OA; -use Attribute; -#[Attribute(Attribute::TARGET_METHOD)] +#[\Attribute(\Attribute::TARGET_METHOD)] class PageParameter extends OA\Parameter { public function __construct( diff --git a/src/OpenApi/Attribute/PagerFanta/PerPageParameter.php b/src/OpenApi/Attribute/PagerFanta/PerPageParameter.php index 098410151..77d72ef19 100644 --- a/src/OpenApi/Attribute/PagerFanta/PerPageParameter.php +++ b/src/OpenApi/Attribute/PagerFanta/PerPageParameter.php @@ -3,9 +3,8 @@ namespace Wallabag\OpenApi\Attribute\PagerFanta; use OpenApi\Attributes as OA; -use Attribute; -#[Attribute(Attribute::TARGET_METHOD)] +#[\Attribute(\Attribute::TARGET_METHOD)] class PerPageParameter extends OA\Parameter { public function __construct( diff --git a/src/Repository/EntryDeletionRepository.php b/src/Repository/EntryDeletionRepository.php index 22d70c3b5..2107d07dd 100644 --- a/src/Repository/EntryDeletionRepository.php +++ b/src/Repository/EntryDeletionRepository.php @@ -17,14 +17,8 @@ class EntryDeletionRepository extends ServiceEntityRepository /** * Find deletions for a specific user since a given date. The result is paginated. - * - * @param int $userId - * @param int $since - * @param int $page - * @param int $perPage - * @param string $order */ - public function findEntryDeletions($userId, $since = 0, $order = 'asc'): Pagerfanta + public function findEntryDeletions(int $userId, int $since = 0, string $order = 'asc'): Pagerfanta { $qb = $this->createQueryBuilder('de') ->where('de.user = :userId')->setParameter('userId', $userId) diff --git a/tests/Command/PurgeEntryDeletionsCommandTest.php b/tests/Command/PurgeEntryDeletionsCommandTest.php index 3fa8872f5..53a472e72 100644 --- a/tests/Command/PurgeEntryDeletionsCommandTest.php +++ b/tests/Command/PurgeEntryDeletionsCommandTest.php @@ -12,7 +12,7 @@ use Wallabag\Repository\EntryDeletionRepository; /** * Test the purge entry deletions command. - * + * * The fixtures set up the following entry deletions: * - Admin user: 1 deletion from 4 days ago (entry_id: 1004) * - Admin user: 1 deletion from 1 day ago (entry_id: 1001) diff --git a/tests/Controller/Api/EntryDeletionRestControllerTest.php b/tests/Controller/Api/EntryDeletionRestControllerTest.php index f2a5359b5..104cdf0e8 100644 --- a/tests/Controller/Api/EntryDeletionRestControllerTest.php +++ b/tests/Controller/Api/EntryDeletionRestControllerTest.php @@ -4,12 +4,12 @@ namespace Tests\Wallabag\Controller\Api; /** * Test the entry deletion REST API endpoints. - * + * * The fixtures set up the following entry deletions: * - Admin user: 1 deletion from 4 days ago (entry_id: 1004) * - Admin user: 1 deletion from 1 day ago (entry_id: 1001) * - Bob user: 1 deletion from 3 days ago (entry_id: 1003) - * + * * The logged in user is admin. */ class EntryDeletionRestControllerTest extends WallabagApiTestCase @@ -21,7 +21,7 @@ class EntryDeletionRestControllerTest extends WallabagApiTestCase $content = json_decode($this->client->getResponse()->getContent(), true); // check that only the items for the current user are returned - $this->assertEquals(2, \count($content['_embedded']['items'])); + $this->assertCount(2, $content['_embedded']['items']); // validate the deletion schema on the first item $deletionData = $content['_embedded']['items'][0]; From c1a8772c215de0f9c07deeecd9aa361592836885 Mon Sep 17 00:00:00 2001 From: Martin Chaine Date: Wed, 4 Jun 2025 12:48:54 +0200 Subject: [PATCH 8/8] add swagger-php to composer.json since it's used directly now --- composer.json | 3 ++- composer.lock | 6 +++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/composer.json b/composer.json index fd95d1d4a..0bd37380b 100644 --- a/composer.json +++ b/composer.json @@ -156,7 +156,8 @@ "wallabag/rulerz": "dev-master", "wallabag/rulerz-bundle": "dev-master", "willdurand/hateoas": "^3.12", - "willdurand/hateoas-bundle": "^2.7" + "willdurand/hateoas-bundle": "^2.7", + "zircote/swagger-php": "^4.11.1 || ^5.0" }, "require-dev": { "dama/doctrine-test-bundle": "^8.2.2", diff --git a/composer.lock b/composer.lock index 7141dd535..c2f52ebf3 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "00b8f95df6ec0572c06ad6f34847405c", + "content-hash": "b47338ba84f717112fd3f36632c65e03", "packages": [ { "name": "babdev/pagerfanta-bundle", @@ -19449,6 +19449,6 @@ "ext-tokenizer": "*", "ext-xml": "*" }, - "platform-dev": [], - "plugin-api-version": "2.3.0" + "platform-dev": {}, + "plugin-api-version": "2.6.0" }