Skip to content

Commit

Permalink
Add version reference change and version deletion audit logs
Browse files Browse the repository at this point in the history
  • Loading branch information
Seldaek committed Jun 12, 2024
1 parent 430da64 commit bfc3a25
Show file tree
Hide file tree
Showing 8 changed files with 235 additions and 38 deletions.
30 changes: 15 additions & 15 deletions phpstan-baseline.neon
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,21 @@ parameters:
count: 1
path: src/DataFixtures/PackageFixtures.php

-
message: "#^Query error\\: Column \"audit_log\\.attributes\" expects value type string, got type array\\<string, mixed\\>$#"
count: 1
path: src/Entity/AuditRecordRepository.php

-
message: "#^Query error\\: Column \"audit_log\\.datetime\" expects value type string, got type DateTimeImmutable$#"
count: 1
path: src/Entity/AuditRecordRepository.php

-
message: "#^Query error\\: Column \"audit_log\\.id\" expects value type string, got type Symfony\\\\Component\\\\Uid\\\\Ulid$#"
count: 1
path: src/Entity/AuditRecordRepository.php

-
message: "#^Method App\\\\Entity\\\\PackageRepository\\:\\:getDependents\\(\\) should return array\\<array\\{id\\: int, name\\: string, description\\: string\\|null, language\\: string\\|null, abandoned\\: int, replacementPackage\\: string\\|null\\}\\> but returns array\\<int\\<0, max\\>, non\\-empty\\-array\\<string, mixed\\>\\>\\.$#"
count: 1
Expand All @@ -40,21 +55,6 @@ parameters:
count: 1
path: src/Entity/Version.php

-
message: "#^Query error\\: Column \"audit_log\\.attributes\" expects value type string, got type array\\<string, mixed\\>$#"
count: 1
path: src/EventListener/PackageListener.php

-
message: "#^Query error\\: Column \"audit_log\\.datetime\" expects value type string, got type DateTimeImmutable$#"
count: 1
path: src/EventListener/PackageListener.php

-
message: "#^Query error\\: Column \"audit_log\\.id\" expects value type string, got type Symfony\\\\Component\\\\Uid\\\\Ulid$#"
count: 1
path: src/EventListener/PackageListener.php

-
message: "#^Method App\\\\Model\\\\FavoriteManager\\:\\:getFavoriteCount\\(\\) should return int\\<0, max\\> but returns int\\.$#"
count: 1
Expand Down
4 changes: 2 additions & 2 deletions src/Audit/AuditRecordType.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ enum AuditRecordType: string
case PackageCreated = 'package_created';
case PackageDeleted = 'package_deleted';
case CanonicalUrlChange = 'canonical_url_change';
case VersionDeleted = 'version_deleted'; # TODO
case VersionChange = 'version_change'; # TODO
case VersionDeleted = 'version_deleted';
case VersionReferenceChange = 'version_reference_change';
case PackageAbandoned = 'package_abandoned'; # TODO
case PackageUnabandoned = 'package_unabandoned'; # TODO

Expand Down
19 changes: 19 additions & 0 deletions src/Entity/AuditRecord.php
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,25 @@ public static function canonicalUrlChange(Package $package, User|null $user, str
return new self(AuditRecordType::CanonicalUrlChange, ['name' => $package->getName(), 'repository_from' => $oldRepository, 'repository_to' => $package->getRepository(), 'actor' => self::getUserData($user)], $user?->getId(), $package->getVendor(), $package->getId());
}

public static function versionDeleted(Version $version, User|null $user): self
{
$package = $version->getPackage();

return new self(AuditRecordType::VersionDeleted, ['name' => $package->getName(), 'version' => $version->getVersion(), 'actor' => self::getUserData($user, 'automation')], $user?->getId(), $package->getVendor(), $package->getId());
}

public static function versionReferenceChange(Version $version, string|null $oldSourceReference, string|null $oldDistReference): self
{
$package = $version->getPackage();

return new self(
AuditRecordType::VersionReferenceChange,
['name' => $package->getName(), 'version' => $version->getVersion(), 'source_from' => $oldSourceReference, 'source_to' => $version->getSource()['reference'] ?? null, 'dist_from' => $oldDistReference, 'dist_to' => $version->getDist()['reference'] ?? null],
vendor: $package->getVendor(),
packageId: $package->getId()
);
}

/**
* @return array{id: int, username: string}|string
*/
Expand Down
22 changes: 22 additions & 0 deletions src/Entity/AuditRecordRepository.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@

use Doctrine\Bundle\DoctrineBundle\Repository\ServiceEntityRepository;
use Doctrine\Persistence\ManagerRegistry;
use Doctrine\DBAL\Types\Types;
use Symfony\Bridge\Doctrine\Types\UlidType;

/**
* @extends ServiceEntityRepository<AuditRecord>
Expand Down Expand Up @@ -42,4 +44,24 @@ public function remove(AuditRecord $entity, bool $flush = false): void
$this->getEntityManager()->flush();
}
}

/**
* Performs a direct insert not requiring usage of the ORM so it can be used within ORM lifecycle listeners
*/
public function insert(AuditRecord $record): void
{
$this->getEntityManager()->getConnection()->insert('audit_log', [
'id' => $record->id,
'datetime' => $record->datetime,
'type' => $record->type->value,
'attributes' => $record->attributes,
'userId' => $record->userId,
'vendor' => $record->vendor,
'packageId' => $record->packageId,
], [
'id' => UlidType::NAME,
'datetime' => Types::DATETIME_IMMUTABLE,
'attributes' => Types::JSON
]);
}
}
24 changes: 3 additions & 21 deletions src/EventListener/PackageListener.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,10 @@
use App\Entity\User;
use App\Util\DoctrineTrait;
use Doctrine\Bundle\DoctrineBundle\Attribute\AsEntityListener;
use Doctrine\DBAL\Types\Types;
use Doctrine\ORM\EntityManager;
use Doctrine\Persistence\Event\LifecycleEventArgs;
use Doctrine\ORM\Event\PreUpdateEventArgs;
use Doctrine\Persistence\ManagerRegistry;
use Symfony\Bridge\Doctrine\Types\UlidType;
use Symfony\Bundle\SecurityBundle\Security;

#[AsEntityListener(event: 'postPersist', entity: Package::class)]
Expand All @@ -47,7 +45,7 @@ public function __construct(
*/
public function postPersist(Package $package, LifecycleEventArgs $event): void
{
$this->insert(AuditRecord::packageCreated($package, $this->getUser()));
$this->getEM()->getRepository(AuditRecord::class)->insert(AuditRecord::packageCreated($package, $this->getUser()));
}

/**
Expand All @@ -74,8 +72,9 @@ public function preUpdate(Package $package, PreUpdateEventArgs $event): void
public function postUpdate(Package $package, LifecycleEventArgs $event): void
{
if ($this->buffered) {
$repo = $this->getEM()->getRepository(AuditRecord::class);
foreach ($this->buffered as $record) {
$this->insert($record);
$repo->insert($record);
}
$this->buffered = [];
}
Expand All @@ -87,21 +86,4 @@ private function getUser(): User|null

return $user instanceof User ? $user : null;
}

private function insert(AuditRecord $record): void
{
$this->getEM()->getConnection()->insert('audit_log', [
'id' => $record->id,
'datetime' => $record->datetime,
'type' => $record->type->value,
'attributes' => $record->attributes,
'userId' => $record->userId,
'vendor' => $record->vendor,
'packageId' => $record->packageId,
], [
'id' => UlidType::NAME,
'datetime' => Types::DATETIME_IMMUTABLE,
'attributes' => Types::JSON
]);
}
}
86 changes: 86 additions & 0 deletions src/EventListener/VersionListener.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
<?php declare(strict_types=1);

/*
* This file is part of Packagist.
*
* (c) Jordi Boggiano <[email protected]>
* Nils Adermann <[email protected]>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace App\EventListener;

use App\Entity\AuditRecord;
use App\Entity\User;
use App\Entity\Version;
use App\Util\DoctrineTrait;
use Doctrine\Bundle\DoctrineBundle\Attribute\AsEntityListener;
use Doctrine\ORM\EntityManager;
use Doctrine\ORM\Event\PreUpdateEventArgs;
use Doctrine\Persistence\Event\LifecycleEventArgs;
use Doctrine\Persistence\ManagerRegistry;
use Symfony\Bundle\SecurityBundle\Security;

#[AsEntityListener(event: 'preRemove', entity: Version::class)]
#[AsEntityListener(event: 'preUpdate', entity: Version::class)]
#[AsEntityListener(event: 'postUpdate', entity: Version::class)]
class VersionListener
{
use DoctrineTrait;

/** @var list<AuditRecord> */
private array $buffered = [];

public function __construct(
private ManagerRegistry $doctrine,
private Security $security,
) {
}

/**
* @param LifecycleEventArgs<EntityManager> $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()) {
$oldDistRef = $event->getOldValue('dist')['reference'] ?? null;
$oldSourceRef = $event->getOldValue('source')['reference'] ?? null;
$newDistRef = $event->getNewValue('dist')['reference'] ?? null;
$newSourceRef = $event->getNewValue('source')['reference'] ?? null;
if ($oldDistRef !== $newDistRef || $oldSourceRef !== $newSourceRef) {
// buffering things to be inserted in postUpdate once we can confirm it is done
$this->buffered[] = AuditRecord::versionReferenceChange($version, $oldSourceRef, $oldDistRef);
}
}
}

/**
* @param LifecycleEventArgs<EntityManager> $event
*/
public function postUpdate(Version $version, LifecycleEventArgs $event): void
{
if ($this->buffered) {
$repo = $this->getEM()->getRepository(AuditRecord::class);
foreach ($this->buffered as $record) {
$repo->insert($record);
}
$this->buffered = [];
}
}

private function getUser(): User|null
{
$user = $this->security->getUser();

return $user instanceof User ? $user : null;
}
}
1 change: 1 addition & 0 deletions tests/Audit/PackageAuditRecordTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ public function testPackageChangesGetRecorded(): void
self::assertSame(AuditRecordType::PackageCreated->value, $logs[0]['type']);

$package->setRepository('https://github.com/composer/packagist');
$em->persist($package);
$em->flush();

$logs = $container->get(Connection::class)->fetchAllAssociative('SELECT * FROM audit_log ORDER BY id DESC');
Expand Down
87 changes: 87 additions & 0 deletions tests/Audit/VersionAuditRecordTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
<?php declare(strict_types=1);

/*
* This file is part of Packagist.
*
* (c) Jordi Boggiano <[email protected]>
* Nils Adermann <[email protected]>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace App\Tests\Controller;

use App\Audit\AuditRecordType;
use App\Entity\Version;
use Doctrine\DBAL\Connection;
use Doctrine\Persistence\ManagerRegistry;
use App\Entity\Package;
use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase;

class VersionAuditRecordTest extends KernelTestCase
{
public function setUp(): void
{
self::bootKernel();
static::getContainer()->get(Connection::class)->beginTransaction();

parent::setUp();
}

public function tearDown(): void
{
static::getContainer()->get(Connection::class)->rollBack();

parent::tearDown();
}

public function testVersionChangesGetRecorded(): void
{
$container = static::getContainer();
$em = $container->get(ManagerRegistry::class)->getManager();

$package = new Package();
$package->setRepository('https://github.com/composer/composer');

$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([]);
$version->setDist(['reference' => 'old-dist-ref', 'type' => 'zip', 'url' => 'https://example.org/dist.zip']);

$em->persist($package);
$em->persist($version);
$em->flush();

$version->setDist(['reference' => 'new-dist-ref', 'type' => 'zip', 'url' => 'https://example.org/dist.zip']);
$version->setSource(['reference' => 'new-source-ref', 'type' => 'git', 'url' => 'git://example.org/dist.zip']);
$em->persist($version);
$em->flush();

$logs = $container->get(Connection::class)->fetchAllAssociative('SELECT * FROM audit_log ORDER BY id DESC');
self::assertCount(2, $logs); // package creation + version reference change
self::assertSame(AuditRecordType::VersionReferenceChange->value, $logs[0]['type']);
self::assertSame('{"name": "composer/composer", "dist_to": "new-dist-ref", "version": "1.0.0", "dist_from": "old-dist-ref", "source_to": "new-source-ref", "source_from": null}', $logs[0]['attributes']);

// verify that only reference changes triggers a new audit log
$version->setDist(['reference' => 'new-dist-ref', 'type' => 'zip2', 'url' => 'https://example.org/dist.zip2']);
$version->setSource(['reference' => 'new-source-ref', 'type' => 'git2', 'url' => 'git://example.org/dist.zip2']);
$em->persist($version);
$em->flush();

$logs = $container->get(Connection::class)->fetchAllAssociative('SELECT * FROM audit_log ORDER BY id DESC');
self::assertCount(2, $logs);

$em->remove($version);
$em->flush();

$logs = $container->get(Connection::class)->fetchAllAssociative('SELECT * FROM audit_log ORDER BY id DESC');
self::assertCount(3, $logs);
self::assertSame(AuditRecordType::VersionDeleted->value, $logs[0]['type']);
}
}

0 comments on commit bfc3a25

Please sign in to comment.