diff --git a/src/Service/AssociationImpactAnalyzer.php b/src/Service/AssociationImpactAnalyzer.php index 5ef8cc0..7b547c9 100644 --- a/src/Service/AssociationImpactAnalyzer.php +++ b/src/Service/AssociationImpactAnalyzer.php @@ -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; @@ -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 []; } @@ -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; @@ -174,6 +176,51 @@ private function buildSingleRelatedEntityImpact( return new AssociationImpact($relatedEntity, $counterpartField, $oldIds, $newIds); } + /** + * @return iterable|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|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 $items + */ + private function canReadCollectionThroughCriteria(PersistentCollection $items): bool + { + return !$items->isInitialized() && !$items->isDirty(); + } + /** * @param array> $metadataByClass * diff --git a/src/Service/CollectionIdExtractor.php b/src/Service/CollectionIdExtractor.php index 0344eb5..6e46f91 100644 --- a/src/Service/CollectionIdExtractor.php +++ b/src/Service/CollectionIdExtractor.php @@ -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; @@ -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 $collection + * + * @return array + */ + 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)) { diff --git a/src/Service/PendingAuditPlanMaterializer.php b/src/Service/PendingAuditPlanMaterializer.php index 6c555c1..8978133 100644 --- a/src/Service/PendingAuditPlanMaterializer.php +++ b/src/Service/PendingAuditPlanMaterializer.php @@ -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; @@ -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) : []; diff --git a/tests/Functional/CollectionInitializationSafetyTest.php b/tests/Functional/CollectionInitializationSafetyTest.php new file mode 100644 index 0000000..86b509e --- /dev/null +++ b/tests/Functional/CollectionInitializationSafetyTest.php @@ -0,0 +1,138 @@ +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); + } +} diff --git a/tests/Functional/Entity/LazyChild.php b/tests/Functional/Entity/LazyChild.php new file mode 100644 index 0000000..f161d75 --- /dev/null +++ b/tests/Functional/Entity/LazyChild.php @@ -0,0 +1,43 @@ +id; + } + + public function getName(): string + { + return $this->name; + } + + public function getParent(): LazyParent + { + return $this->parent; + } +} diff --git a/tests/Functional/Entity/LazyParent.php b/tests/Functional/Entity/LazyParent.php new file mode 100644 index 0000000..1d0bd9c --- /dev/null +++ b/tests/Functional/Entity/LazyParent.php @@ -0,0 +1,52 @@ + + */ + #[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 + */ + public function getChildren(): Collection + { + return $this->children; + } +}