diff --git a/src/Command/CleanSpamPackagesCommand.php b/src/Command/CleanSpamPackagesCommand.php index eca211d41..9de69551f 100644 --- a/src/Command/CleanSpamPackagesCommand.php +++ b/src/Command/CleanSpamPackagesCommand.php @@ -66,7 +66,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int } $output->write('.'); foreach ($package->getVersions() as $version) { - $versionRepo->remove($version); + $versionRepo->remove($version, false); } $this->providerManager->deletePackage($package); diff --git a/src/Entity/VersionRepository.php b/src/Entity/VersionRepository.php index be5973aa2..aa85714da 100644 --- a/src/Entity/VersionRepository.php +++ b/src/Entity/VersionRepository.php @@ -20,6 +20,7 @@ use Doctrine\ORM\QueryBuilder; use Doctrine\Persistence\ManagerRegistry; use Predis\Client; +use Symfony\Bundle\SecurityBundle\Security; /** * @author Jordi Boggiano @@ -34,6 +35,7 @@ public function __construct( ManagerRegistry $registry, private Client $redisCache, private VersionIdCache $versionIdCache, + private readonly Security $security, ) { parent::__construct($registry, Version::class); } @@ -44,7 +46,7 @@ public function getEntityManager(): EntityManagerInterface return parent::getEntityManager(); } - public function remove(Version $version): void + public function remove(Version $version, bool $createAuditRecord = true): void { $em = $this->getEntityManager(); $package = $version->getPackage(); @@ -66,6 +68,12 @@ public function remove(Version $version): void $em->getConnection()->executeQuery('DELETE FROM php_stat WHERE version=:version AND depth = :depth AND package_id=:packageId', ['version' => $version->getId(), 'depth' => PhpStat::DEPTH_EXACT, 'packageId' => $version->getPackage()->getId()]); $em->remove($version); + + if ($createAuditRecord) { + $user = $this->security->getUser(); + $record = AuditRecord::versionDeleted($version, $user instanceof User ? $user : null); + $em->persist($record); + } } /** diff --git a/src/EventListener/VersionListener.php b/src/EventListener/VersionListener.php index de7b80fce..9fb134130 100644 --- a/src/EventListener/VersionListener.php +++ b/src/EventListener/VersionListener.php @@ -23,7 +23,6 @@ use Doctrine\Persistence\ManagerRegistry; use Symfony\Bundle\SecurityBundle\Security; -#[AsEntityListener(event: 'preRemove', entity: Version::class)] #[AsEntityListener(event: 'preUpdate', entity: Version::class)] #[AsEntityListener(event: 'postPersist', entity: Version::class)] #[AsEntityListener(event: 'postUpdate', entity: Version::class)] @@ -50,16 +49,6 @@ public function postPersist(Version $version, LifecycleEventArgs $event): void $this->getEM()->getRepository(AuditRecord::class)->insert($record); } - /** - * @param LifecycleEventArgs $event - */ - public function preRemove(Version $version, LifecycleEventArgs $event): void - { - $record = AuditRecord::versionDeleted($version, $this->getUser()); - $this->getEM()->persist($record); - // let the record be flushed together with the entity - } - public function preUpdate(Version $version, PreUpdateEventArgs $event): void { if (($event->hasChangedField('source') || $event->hasChangedField('dist')) && !$version->isDevelopment()) { diff --git a/src/Model/PackageManager.php b/src/Model/PackageManager.php index b1a171c38..57ca9e98c 100644 --- a/src/Model/PackageManager.php +++ b/src/Model/PackageManager.php @@ -62,7 +62,7 @@ public function deletePackage(Package $package): void { $versionRepo = $this->doctrine->getRepository(Version::class); foreach ($package->getVersions() as $version) { - $versionRepo->remove($version); + $versionRepo->remove($version, false); } if ($package->getAutoUpdated() === Package::AUTO_GITHUB_HOOK) { diff --git a/src/Package/Updater.php b/src/Package/Updater.php index c6bb7f6da..9cd777587 100644 --- a/src/Package/Updater.php +++ b/src/Package/Updater.php @@ -128,7 +128,6 @@ public function update(IOInterface $io, Config $config, Package $package, VcsRep $deleteDate = new \DateTimeImmutable('-1day'); $em = $this->getEM(); - $rootIdentifier = null; $driver = $repository->getDriver(); if (!$driver) { diff --git a/tests/Audit/VersionAuditRecordTest.php b/tests/Audit/VersionAuditRecordTest.php index 0fb01c81f..53cf694d7 100644 --- a/tests/Audit/VersionAuditRecordTest.php +++ b/tests/Audit/VersionAuditRecordTest.php @@ -102,13 +102,6 @@ public function testVersionChangesGetRecorded(): void $logs = $container->get(Connection::class)->fetchAllAssociative('SELECT * FROM audit_log ORDER BY id DESC'); self::assertCount(3, $logs); - - $em->getRepository(Version::class)->remove($version); - $em->flush(); - - $logs = $container->get(Connection::class)->fetchAllAssociative('SELECT * FROM audit_log ORDER BY id DESC'); - self::assertCount(4, $logs); - self::assertSame(AuditRecordType::VersionDeleted->value, $logs[0]['type']); } private function createPackageAndVersion(): Version diff --git a/tests/Entity/VersionRepositoryTest.php b/tests/Entity/VersionRepositoryTest.php new file mode 100644 index 000000000..8d166db56 --- /dev/null +++ b/tests/Entity/VersionRepositoryTest.php @@ -0,0 +1,74 @@ + + * Nils Adermann + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace App\Tests\Entity; + +use App\Audit\AuditRecordType; +use App\Entity\AuditRecord; +use App\Entity\Version; +use App\Entity\VersionRepository; +use App\Tests\IntegrationTestCase; +use PHPUnit\Framework\Attributes\TestWith; + +class VersionRepositoryTest extends IntegrationTestCase +{ + private VersionRepository $versionRepository; + + protected function setUp(): void + { + parent::setUp(); + + $this->versionRepository = self::getEM()->getRepository(Version::class); + } + + #[TestWith([false])] + #[TestWith([true])] + public function testRemoveVersionMarksForRemovalWithAuditRecord(bool $createAuditRecord): void + { + $em = self::getEM(); + + $package = self::createPackage('vendor/package', 'https://github.com/vendor/package'); + + $version = new Version(); + $version->setPackage($package); + $version->setName($package->getName()); + $version->setVersion('1.0.0'); + $version->setNormalizedVersion('1.0.0.0'); + $version->setDevelopment(false); + $version->setLicense([]); + $version->setAutoload([]); + $package->getVersions()->add($version); + + $this->store($package, $version); + + $versionId = $version->getId(); + $this->versionRepository->remove($version, $createAuditRecord); + + $em->flush(); + $em->clear(); + + $this->assertNull($this->versionRepository->find($versionId), 'Version was not deleted'); + + $auditRecord = $em->getRepository(AuditRecord::class)->findOneBy([ + 'type' => AuditRecordType::VersionDeleted->value, + 'packageId' => $package->getId(), + 'actorId' => null, + ]); + + if ($createAuditRecord) { + $this->assertNotNull($auditRecord, 'No audit record for version deletion created'); + } else { + $this->assertNull($auditRecord, 'Audit record for version deleted created'); + } + + } +}