Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 51 additions & 4 deletions src/Service/AssociationImpactAnalyzer.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,13 @@

namespace Rcsofttech\AuditTrailBundle\Service;

use Doctrine\Common\Collections\Criteria;
use Doctrine\ORM\EntityManagerInterface;
use Doctrine\ORM\Mapping\AssociationMapping;
use Doctrine\ORM\Mapping\ClassMetadata;
use Doctrine\ORM\Mapping\InverseSideMapping;
use Doctrine\ORM\Mapping\OwningSideMapping;
use Doctrine\ORM\PersistentCollection;
use Doctrine\ORM\UnitOfWork;
use Rcsofttech\AuditTrailBundle\ValueObject\AssociationImpact;

Expand Down Expand Up @@ -114,8 +116,8 @@ private function buildAssociationCollectionImpacts(
return [];
}

$relatedEntities = $metadata->getFieldValue($deletedEntity, $associationName);
if (!is_iterable($relatedEntities)) {
$relatedEntities = $this->resolveCollectionItems($metadata->getFieldValue($deletedEntity, $associationName));
if ($relatedEntities === null) {
return [];
}

Expand Down Expand Up @@ -161,11 +163,11 @@ private function buildSingleRelatedEntityImpact(
): ?AssociationImpact {
$relatedMetadata = $this->getClassMetadata($relatedEntity, $metadataByClass, $em);
$relatedCollection = $relatedMetadata->getFieldValue($relatedEntity, $counterpartField);
if (!is_iterable($relatedCollection)) {
$oldIds = $this->resolveCollectionIds($relatedCollection, $em);
if ($oldIds === null) {
return null;
}

$oldIds = $this->collectionIdExtractor->extractFromIterable($relatedCollection, $em);
$newIds = array_values(array_filter($oldIds, static fn ($id) => $id !== $deletedId));
if ($oldIds === $newIds) {
return null;
Expand All @@ -174,6 +176,51 @@ private function buildSingleRelatedEntityImpact(
return new AssociationImpact($relatedEntity, $counterpartField, $oldIds, $newIds);
}

/**
* @return iterable<mixed>|null
*/
private function resolveCollectionItems(mixed $items): ?iterable
{
if (!is_iterable($items)) {
return null;
}

if ($items instanceof PersistentCollection && $this->canReadCollectionThroughCriteria($items)) {
return $items->matching(Criteria::create());
}

return $items;
}

/**
* @return array<int, int|string>|null
*/
private function resolveCollectionIds(mixed $items, EntityManagerInterface $em): ?array
{
if (!is_iterable($items)) {
return null;
}

if ($items instanceof PersistentCollection && $this->canReadCollectionThroughCriteria($items)) {
return $this->collectionIdExtractor->extractFromPersistentCollectionCriteria($items, $em);
}

return $this->collectionIdExtractor->extractFromIterable($items, $em);
}

/**
* During onFlush delete impact analysis, a clean uninitialized collection
* can be read from the pre-delete database state without hydrating the
* original PersistentCollection. Dirty collections keep the existing path
* because their in-memory diffs must remain part of the result.
*
* @param PersistentCollection<int|string, mixed> $items
*/
private function canReadCollectionThroughCriteria(PersistentCollection $items): bool
{
return !$items->isInitialized() && !$items->isDirty();
}

/**
* @param array<class-string, ClassMetadata<object>> $metadataByClass
*
Expand Down
18 changes: 18 additions & 0 deletions src/Service/CollectionIdExtractor.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

namespace Rcsofttech\AuditTrailBundle\Service;

use Doctrine\Common\Collections\Criteria;
use Doctrine\ORM\EntityManagerInterface;
use Doctrine\ORM\PersistentCollection;
use Rcsofttech\AuditTrailBundle\Contract\EntityIdResolverInterface;
Expand Down Expand Up @@ -41,6 +42,23 @@ public function extractFromIterable(iterable $items, EntityManagerInterface $em)
return $ids;
}

/**
* Read ids through a separate Doctrine criteria result without hydrating
* the owning PersistentCollection.
*
* Use this only when the database state is the intended source of truth for
* the collection, and the collection has no pending in-memory diffs that
* must be folded into the result.
*
* @param PersistentCollection<int|string, mixed> $collection
*
* @return array<int, string>
*/
public function extractFromPersistentCollectionCriteria(PersistentCollection $collection, EntityManagerInterface $em): array
{
return $this->extractFromIterable($collection->matching(Criteria::create()), $em);
}

public function hasPendingIds(mixed $items, EntityManagerInterface $em): bool
{
if (!is_iterable($items)) {
Expand Down
10 changes: 10 additions & 0 deletions src/Service/PendingAuditPlanMaterializer.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
namespace Rcsofttech\AuditTrailBundle\Service;

use Doctrine\ORM\EntityManagerInterface;
use Doctrine\ORM\PersistentCollection;
use Rcsofttech\AuditTrailBundle\Contract\AuditServiceInterface;
use Rcsofttech\AuditTrailBundle\Entity\AuditLog;
use Rcsofttech\AuditTrailBundle\ValueObject\PendingAuditPlan;
Expand Down Expand Up @@ -33,6 +34,15 @@ public function materialize(PendingAuditPlan $plan, EntityManagerInterface $enti

foreach ($plan->deferredCollectionFields as $field) {
$currentValue = $metadata->getFieldValue($plan->entity, $field);
if ($currentValue instanceof PersistentCollection && !$currentValue->isInitialized() && !$currentValue->isDirty()) {
$newValues[$field] = $this->collectionIdExtractor->extractFromPersistentCollectionCriteria(
$currentValue,
$entityManager,
);

continue;
}

$newValues[$field] = is_iterable($currentValue)
? $this->collectionIdExtractor->extractFromIterable($currentValue, $entityManager)
: [];
Expand Down
138 changes: 138 additions & 0 deletions tests/Functional/CollectionInitializationSafetyTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,138 @@
<?php

declare(strict_types=1);

namespace Rcsofttech\AuditTrailBundle\Tests\Functional;

use Doctrine\ORM\PersistentCollection;
use Rcsofttech\AuditTrailBundle\Entity\AuditLog;
use Rcsofttech\AuditTrailBundle\Enum\AuditAction;
use Rcsofttech\AuditTrailBundle\Tests\Functional\Entity\Author;
use Rcsofttech\AuditTrailBundle\Tests\Functional\Entity\LazyChild;
use Rcsofttech\AuditTrailBundle\Tests\Functional\Entity\LazyParent;
use Rcsofttech\AuditTrailBundle\Tests\Functional\Entity\Tag;

use function array_slice;

final class CollectionInitializationSafetyTest extends AbstractFunctionalTestCase
{
public function testDeferredAuditMaterializationRecordsFlushedIdsWithoutInitializingExtraLazyCollection(): void
{
self::bootKernel();
$em = $this->getEntityManager();

$parent = new LazyParent('parent');
$em->persist($parent);
$em->flush();

$parentId = $parent->getId();
self::assertNotNull($parentId);

$em->clear();

$parent = $em->find(LazyParent::class, $parentId);
self::assertInstanceOf(LazyParent::class, $parent);

$children = $parent->getChildren();
self::assertInstanceOf(PersistentCollection::class, $children);
self::assertFalse($children->isInitialized());

$firstChild = new LazyChild('child-1', $parent);
$parent->getChildren()->add($firstChild);
$em->persist($firstChild);
$em->flush();

$firstChildId = $firstChild->getId();
self::assertNotNull($firstChildId);

$parentAudit = $em->getRepository(AuditLog::class)->findOneBy([
'entityClass' => LazyParent::class,
'entityId' => (string) $parentId,
'action' => AuditAction::Update,
], ['createdAt' => 'DESC']);

self::assertNotNull($parentAudit, 'Adding the child through the collection must create a deferred parent audit log.');
self::assertSame([(string) $firstChildId], $parentAudit->newValues['children'] ?? null);
self::assertContains('children', $parentAudit->changedFields ?? []);
self::assertFalse(
$children->isInitialized(),
'Deferred audit materialization must not initialize the EXTRA_LAZY collection.',
);

foreach (['child-2', 'child-3', 'child-4', 'child-5'] as $name) {
$em->persist(new LazyChild($name, $parent));
$em->flush();
}

self::assertSame(5, $em->getRepository(LazyChild::class)->count(['parent' => $parent]));
self::assertCount(
5,
$parent->getChildren(),
'An uninitialized EXTRA_LAZY collection must keep using a fresh database count.',
);
}

public function testDeletedAssociationImpactRecordsChangeWithoutInitializingRelatedCollection(): void
{
self::bootKernel();
$em = $this->getEntityManager();

$author = new Author('author');
$tags = [];
foreach (['tag-1', 'tag-2', 'tag-3', 'tag-4', 'tag-5'] as $label) {
$tag = new Tag($label);
$author->addTag($tag);
$tags[] = $tag;
}

$em->persist($author);
$em->flush();

$authorId = $author->getId();
self::assertNotNull($authorId);

$tagIds = [];
foreach ($tags as $tag) {
$tagId = $tag->getId();
self::assertNotNull($tagId);
$tagIds[] = $tagId;
}

$deletedTagId = $tagIds[0];
$expectedOldIds = array_map('strval', $tagIds);
$expectedNewIds = array_map('strval', array_slice($tagIds, 1));

$em->clear();

$author = $em->find(Author::class, $authorId);
$deletedTag = $em->find(Tag::class, $deletedTagId);
self::assertInstanceOf(Author::class, $author);
self::assertInstanceOf(Tag::class, $deletedTag);

$authorTags = $author->getTags();
self::assertInstanceOf(PersistentCollection::class, $authorTags);
self::assertFalse($authorTags->isInitialized());

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

$authorAudit = $em->getRepository(AuditLog::class)->findOneBy([
'entityClass' => Author::class,
'entityId' => (string) $authorId,
'action' => AuditAction::Update,
], ['createdAt' => 'DESC']);

self::assertNotNull($authorAudit, 'Deleting a tag must create an update audit log for the related author.');
$actualOldIds = $authorAudit->oldValues['tags'] ?? null;
$actualNewIds = $authorAudit->newValues['tags'] ?? null;
self::assertIsArray($actualOldIds);
self::assertIsArray($actualNewIds);
self::assertEqualsCanonicalizing($expectedOldIds, $actualOldIds);
self::assertEqualsCanonicalizing($expectedNewIds, $actualNewIds);
self::assertFalse(
$authorTags->isInitialized(),
'Deleted association impact analysis must not initialize the related owner collection.',
);
self::assertCount(4, $authorTags);
}
}
43 changes: 43 additions & 0 deletions tests/Functional/Entity/LazyChild.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
<?php

declare(strict_types=1);

namespace Rcsofttech\AuditTrailBundle\Tests\Functional\Entity;

use Doctrine\ORM\Mapping as ORM;
use Rcsofttech\AuditTrailBundle\Attribute\Auditable;

#[ORM\Entity]
#[ORM\Table(name: 'lazy_child')]
#[Auditable]
class LazyChild
{
#[ORM\Id]
#[ORM\GeneratedValue]
#[ORM\Column]
private ?int $id = null;

public function __construct(
#[ORM\Column]
private string $name,
#[ORM\ManyToOne(targetEntity: LazyParent::class, inversedBy: 'children')]
#[ORM\JoinColumn(nullable: false)]
private LazyParent $parent,
) {
}

public function getId(): ?int
{
return $this->id;
}

public function getName(): string
{
return $this->name;
}

public function getParent(): LazyParent
{
return $this->parent;
}
}
52 changes: 52 additions & 0 deletions tests/Functional/Entity/LazyParent.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
<?php

declare(strict_types=1);

namespace Rcsofttech\AuditTrailBundle\Tests\Functional\Entity;

use Doctrine\Common\Collections\ArrayCollection;
use Doctrine\Common\Collections\Collection;
use Doctrine\ORM\Mapping as ORM;
use Rcsofttech\AuditTrailBundle\Attribute\Auditable;

#[ORM\Entity]
#[ORM\Table(name: 'lazy_parent')]
#[Auditable]
class LazyParent
{
#[ORM\Id]
#[ORM\GeneratedValue]
#[ORM\Column]
private ?int $id = null;

/**
* @var Collection<int, LazyChild>
*/
#[ORM\OneToMany(mappedBy: 'parent', targetEntity: LazyChild::class, cascade: ['persist'], fetch: 'EXTRA_LAZY')]
private Collection $children;

public function __construct(
#[ORM\Column]
private string $name,
) {
$this->children = new ArrayCollection();
}

public function getId(): ?int
{
return $this->id;
}

public function getName(): string
{
return $this->name;
}

/**
* @return Collection<int, LazyChild>
*/
public function getChildren(): Collection
{
return $this->children;
}
}
Loading