From d197085b432f36d91f4bd7d86d6ee540db5c6f4c Mon Sep 17 00:00:00 2001 From: saifulislamferoz Date: Thu, 15 May 2025 11:30:46 +0600 Subject: [PATCH 01/14] feat: remove ClassUtils dependency --- Resolver/DoctrineObjectEventResolver.php | 8 +++--- Subscriber/DoctrineSubscriber.php | 35 ++++++++++++++---------- 2 files changed, 24 insertions(+), 19 deletions(-) diff --git a/Resolver/DoctrineObjectEventResolver.php b/Resolver/DoctrineObjectEventResolver.php index 765e671..f701f6c 100644 --- a/Resolver/DoctrineObjectEventResolver.php +++ b/Resolver/DoctrineObjectEventResolver.php @@ -12,10 +12,10 @@ namespace Xiidea\EasyAuditBundle\Resolver; use Doctrine\Persistence\ManagerRegistry; -use Doctrine\Common\Util\ClassUtils; +use Doctrine\Persistence\Proxy; use Symfony\Contracts\EventDispatcher\Event; -use Xiidea\EasyAuditBundle\Events\DoctrineObjectEvent; use Xiidea\EasyAuditBundle\Events\DoctrineEvents; +use Xiidea\EasyAuditBundle\Events\DoctrineObjectEvent; /** Custom Event Resolver Example Class */ class DoctrineObjectEventResolver implements EventResolverInterface @@ -81,7 +81,7 @@ protected function getSingleIdentity() /** * @param DoctrineObjectEvent $event - * @param string $eventName + * @param string $eventName */ private function initialize(DoctrineObjectEvent $event, $eventName) { @@ -175,7 +175,7 @@ protected function getName() */ protected function getReflectionClassFromObject($object) { - return new \ReflectionClass(ClassUtils::getClass($object)); + return new \ReflectionClass($object instanceof Proxy ? get_parent_class($object) : get_class($object)); } /** diff --git a/Subscriber/DoctrineSubscriber.php b/Subscriber/DoctrineSubscriber.php index f4c6198..b34891f 100644 --- a/Subscriber/DoctrineSubscriber.php +++ b/Subscriber/DoctrineSubscriber.php @@ -11,8 +11,8 @@ namespace Xiidea\EasyAuditBundle\Subscriber; -use Doctrine\Common\Util\ClassUtils; use Doctrine\Persistence\Event\LifecycleEventArgs; +use Doctrine\Persistence\Proxy; use Symfony\Component\EventDispatcher\EventDispatcherInterface; use Xiidea\EasyAuditBundle\Attribute\SubscribeDoctrineEvents; use Xiidea\EasyAuditBundle\Events\DoctrineEvents; @@ -21,7 +21,7 @@ class DoctrineSubscriber { private array $toBeDeleted = []; - private $dispatcher = null; + private $dispatcher = null; public function __construct(private array $entities = []) { @@ -43,7 +43,7 @@ public function preRemove(LifecycleEventArgs $args) return; } - $className = ClassUtils::getClass($args->getObject()); + $className = $this->getClass($args->getObject()); if (!isset($this->toBeDeleted[$className])) { $this->toBeDeleted[$className] = []; @@ -64,21 +64,21 @@ public function postRemove(LifecycleEventArgs $args) private function getToBeDeletedId($entity) { if ($this->isScheduledForDelete($entity)) { - return $this->toBeDeleted[ClassUtils::getClass($entity)][spl_object_hash($entity)]; + return $this->toBeDeleted[$this->getClass($entity)][spl_object_hash($entity)]; } return null; } /** - * @param string $eventName - * @param LifecycleEventArgs $args + * @param string $eventName + * @param LifecycleEventArgs $args */ private function handleEvent($eventName, LifecycleEventArgs $args) { if (true === $this->isConfiguredToTrack($args->getObject(), $eventName)) { $this->dispatcher->dispatch( - new DoctrineObjectEvent($args, $this->getIdentity($args, ClassUtils::getClass($args->getObject()))), + new DoctrineObjectEvent($args, $this->getIdentity($args, $this->getClass($args->getObject()))), $eventName ); } @@ -86,13 +86,13 @@ private function handleEvent($eventName, LifecycleEventArgs $args) /** * @param $entity - * @param string $eventName + * @param string $eventName * * @return bool */ private function isConfiguredToTrack($entity, $eventName = '') { - $class = ClassUtils::getClass($entity); + $class = $this->getClass($entity); $eventType = DoctrineEvents::getShortEventType($eventName); if (null !== $track = $this->isAttributedEvent($entity, $eventType)) { @@ -131,8 +131,8 @@ protected function isAttributedEvent($entity, $eventType) /** - * @param string $eventType - * @param string $class + * @param string $eventType + * @param string $class * * @return bool */ @@ -142,7 +142,7 @@ private function shouldTrackEventType($eventType, $class) } /** - * @param string $class + * @param string $class * * @return bool */ @@ -152,7 +152,7 @@ private function shouldTrackAllEventType($class) } /** - * @param LifecycleEventArgs $args + * @param LifecycleEventArgs $args * @param $className * * @return array @@ -169,7 +169,7 @@ protected function getIdentity(LifecycleEventArgs $args, $className) */ private function isScheduledForDelete($entity) { - $originalClassName = ClassUtils::getClass($entity); + $originalClassName = $this->getClass($entity); return isset($this->toBeDeleted[$originalClassName]) && isset( $this->toBeDeleted[$originalClassName][spl_object_hash( @@ -179,10 +179,15 @@ private function isScheduledForDelete($entity) } /** - * @param EventDispatcherInterface $dispatcher + * @param EventDispatcherInterface $dispatcher */ public function setDispatcher($dispatcher) { $this->dispatcher = $dispatcher; } + + private function getClass($entity): string + { + return $entity instanceof Proxy ? get_parent_class($entity) : get_class($entity); + } } From 977960a59b4f11393a4418705db9c18a64850b3b Mon Sep 17 00:00:00 2001 From: saifulislamferoz Date: Thu, 15 May 2025 11:38:28 +0600 Subject: [PATCH 02/14] fix: php 8.4 deprecations --- Logger/Logger.php | 12 ++++++------ Logger/MonologLogger.php | 2 +- Tests/Fixtures/ORM/UserEntity.php | 8 ++++---- .../Bundle/TestBundle/Logger/FileLogger.php | 2 +- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/Logger/Logger.php b/Logger/Logger.php index 67a32f5..a3027a9 100644 --- a/Logger/Logger.php +++ b/Logger/Logger.php @@ -18,14 +18,14 @@ class Logger implements LoggerInterface { - private $entityDeleteLogs = []; + private array $entityDeleteLogs = []; public function __construct(private ManagerRegistry $doctrine) { } #[\Override] - public function log(AuditLog $event = null) + public function log(?AuditLog $event = null): void { if (empty($event)) { return; @@ -43,7 +43,7 @@ public function log(AuditLog $event = null) /** * @return ObjectManager */ - protected function getManager() + protected function getManager(): ObjectManager { return $this->getDoctrine()->getManager(); } @@ -51,7 +51,7 @@ protected function getManager() /** * @return ManagerRegistry */ - public function getDoctrine() + public function getDoctrine(): ManagerRegistry { return $this->doctrine; } @@ -59,13 +59,13 @@ public function getDoctrine() /** * @param AuditLog $event */ - protected function saveLog(AuditLog $event) + protected function saveLog(AuditLog $event): void { $this->getManager()->persist($event); $this->getManager()->flush($event); } - public function savePendingLogs() + public function savePendingLogs(): void { foreach ($this->entityDeleteLogs as $log) { $this->saveLog($log); diff --git a/Logger/MonologLogger.php b/Logger/MonologLogger.php index 40dbbe4..a215c15 100644 --- a/Logger/MonologLogger.php +++ b/Logger/MonologLogger.php @@ -24,7 +24,7 @@ public function __construct(private \Psr\Log\LoggerInterface $logger) } #[\Override] - public function log(AuditLog $event = null) + public function log(?AuditLog $event = null) { if (null === $event) { return; diff --git a/Tests/Fixtures/ORM/UserEntity.php b/Tests/Fixtures/ORM/UserEntity.php index 40a7b04..3760ac2 100644 --- a/Tests/Fixtures/ORM/UserEntity.php +++ b/Tests/Fixtures/ORM/UserEntity.php @@ -212,7 +212,7 @@ public function isSuperAdmin() * * @return static */ - public function setLastLogin(\DateTime $time = null) + public function setLastLogin(?\DateTime $time = null) { return $this; } @@ -225,7 +225,7 @@ public function setLastLogin(\DateTime $time = null) * * @return string The password */ - public function getPassword() + public function getPassword(): string { return; } @@ -237,7 +237,7 @@ public function getPassword() * * @return string|null The salt */ - public function getSalt() + public function getSalt(): void { return; } @@ -358,7 +358,7 @@ public function setConfirmationToken($confirmationToken) * * @return static */ - public function setPasswordRequestedAt(\DateTime $date = null) + public function setPasswordRequestedAt(?\DateTime $date = null) { return $this; } diff --git a/Tests/Functional/Bundle/TestBundle/Logger/FileLogger.php b/Tests/Functional/Bundle/TestBundle/Logger/FileLogger.php index 3740e8c..3e66a65 100644 --- a/Tests/Functional/Bundle/TestBundle/Logger/FileLogger.php +++ b/Tests/Functional/Bundle/TestBundle/Logger/FileLogger.php @@ -24,7 +24,7 @@ public function __construct($dir) $this->dir = $dir; } - public function log(AuditLog $event = null) + public function log(?AuditLog $event = null) { if (empty($event)) { return; From d2076f1e8aa313115fda9537acc5b1c94254b258 Mon Sep 17 00:00:00 2001 From: saifulislamferoz Date: Thu, 15 May 2025 11:38:58 +0600 Subject: [PATCH 03/14] ci: add php 8.4 version strategy matrix --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2c892f8..70d282a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -9,7 +9,7 @@ jobs: fail-fast: false matrix: operating-system: [ ubuntu-latest ] - php-versions: [ '8.1', '8.2', '8.3' ] + php-versions: [ '8.1', '8.2', '8.3', '8.4' ] steps: - name: Checkout From d2294368f2b6d6f51985b9f56542b030e3d0d27c Mon Sep 17 00:00:00 2001 From: saifulislamferoz Date: Thu, 15 May 2025 11:43:17 +0600 Subject: [PATCH 04/14] feat: add return type --- Tests/Fixtures/ORM/UserEntity.php | 31 ++++++++++++------------------- 1 file changed, 12 insertions(+), 19 deletions(-) diff --git a/Tests/Fixtures/ORM/UserEntity.php b/Tests/Fixtures/ORM/UserEntity.php index 3760ac2..4b2398c 100644 --- a/Tests/Fixtures/ORM/UserEntity.php +++ b/Tests/Fixtures/ORM/UserEntity.php @@ -58,7 +58,7 @@ public function getId() return $this->id; } - public function __toString() + public function __toString(): string { return $this->getUsername(); } @@ -81,7 +81,7 @@ public function getRoles(): array * * @see AccountExpiredException */ - public function isAccountNonExpired() + public function isAccountNonExpired(): bool { return true; } @@ -96,7 +96,7 @@ public function isAccountNonExpired() * * @see LockedException */ - public function isAccountNonLocked() + public function isAccountNonLocked(): bool { return true; } @@ -111,7 +111,7 @@ public function isAccountNonLocked() * * @see CredentialsExpiredException */ - public function isCredentialsNonExpired() + public function isCredentialsNonExpired(): bool { return true; } @@ -126,7 +126,7 @@ public function isCredentialsNonExpired() * * @see DisabledException */ - public function isEnabled() + public function isEnabled(): bool { return true; } @@ -156,7 +156,7 @@ public function serialize() * * @since 5.1.0 */ - public function unserialize($serialized) + public function unserialize($serialized): void { return; } @@ -166,7 +166,7 @@ public function unserialize($serialized) * * @return static */ - public function setSalt($salt) + public function setSalt($salt): static { return $this; } @@ -178,7 +178,7 @@ public function setSalt($salt) * * @return static */ - public function setPlainPassword($password) + public function setPlainPassword($password): static { return $this; } @@ -190,7 +190,7 @@ public function setPlainPassword($password) * * @return static */ - public function setPassword($password) + public function setPassword($password): static { return $this; } @@ -200,7 +200,7 @@ public function setPassword($password) * * @return bool */ - public function isSuperAdmin() + public function isSuperAdmin(): bool { return true; } @@ -217,15 +217,8 @@ public function setLastLogin(?\DateTime $time = null) return $this; } - /** - * Returns the password used to authenticate the user. - * - * This should be the encoded password. On authentication, a plain-text - * password will be salted, encoded, and then compared to this value. - * - * @return string The password - */ - public function getPassword(): string + + public function getPassword(): void { return; } From be18310f40a961352ea891f16767939a3ed4c801 Mon Sep 17 00:00:00 2001 From: saifulislamferoz Date: Thu, 15 May 2025 11:45:36 +0600 Subject: [PATCH 05/14] fix: test case failed for return type --- Tests/Fixtures/ORM/UnitOfWork.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/Fixtures/ORM/UnitOfWork.php b/Tests/Fixtures/ORM/UnitOfWork.php index a093e6c..7077658 100644 --- a/Tests/Fixtures/ORM/UnitOfWork.php +++ b/Tests/Fixtures/ORM/UnitOfWork.php @@ -15,7 +15,7 @@ class UnitOfWork implements PropertyChangedListener * @param mixed $oldValue the old value of the property that changed * @param mixed $newValue the new value of the property that changed */ - public function propertyChanged($sender, $propertyName, $oldValue, $newValue) + public function propertyChanged(object $sender, string $propertyName, $oldValue, $newValue) { } From 863989b14816ba2f050771452a6544648227795a Mon Sep 17 00:00:00 2001 From: saifulislamferoz Date: Thu, 15 May 2025 11:49:02 +0600 Subject: [PATCH 06/14] fix: test case failed for return type odm --- Tests/Fixtures/ODM/UnitOfWork.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/Fixtures/ODM/UnitOfWork.php b/Tests/Fixtures/ODM/UnitOfWork.php index 4d78e60..c20ad77 100644 --- a/Tests/Fixtures/ODM/UnitOfWork.php +++ b/Tests/Fixtures/ODM/UnitOfWork.php @@ -15,7 +15,7 @@ class UnitOfWork implements PropertyChangedListener * @param mixed $oldValue the old value of the property that changed * @param mixed $newValue the new value of the property that changed */ - public function propertyChanged($sender, $propertyName, $oldValue, $newValue) + public function propertyChanged(object $sender, string $propertyName, $oldValue, $newValue) { } From b3899e738fde7f159589cb76c02a8377ae6e46a1 Mon Sep 17 00:00:00 2001 From: saifulislamferoz Date: Thu, 15 May 2025 11:51:07 +0600 Subject: [PATCH 07/14] fix: testcase failed for data type --- Tests/Fixtures/ODM/UnitOfWork.php | 2 +- Tests/Fixtures/ORM/UnitOfWork.php | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Tests/Fixtures/ODM/UnitOfWork.php b/Tests/Fixtures/ODM/UnitOfWork.php index c20ad77..46d43cf 100644 --- a/Tests/Fixtures/ODM/UnitOfWork.php +++ b/Tests/Fixtures/ODM/UnitOfWork.php @@ -15,7 +15,7 @@ class UnitOfWork implements PropertyChangedListener * @param mixed $oldValue the old value of the property that changed * @param mixed $newValue the new value of the property that changed */ - public function propertyChanged(object $sender, string $propertyName, $oldValue, $newValue) + public function propertyChanged(object $sender, string $propertyName, mixed $oldValue, mixed $newValue) { } diff --git a/Tests/Fixtures/ORM/UnitOfWork.php b/Tests/Fixtures/ORM/UnitOfWork.php index 7077658..d0c9824 100644 --- a/Tests/Fixtures/ORM/UnitOfWork.php +++ b/Tests/Fixtures/ORM/UnitOfWork.php @@ -10,12 +10,12 @@ class UnitOfWork implements PropertyChangedListener /** * Collect information about a property change. * - * @param object $sender the object on which the property changed + * @param object $sender the object on which the property changed * @param string $propertyName the name of the property that changed - * @param mixed $oldValue the old value of the property that changed - * @param mixed $newValue the new value of the property that changed + * @param mixed $oldValue the old value of the property that changed + * @param mixed $newValue the new value of the property that changed */ - public function propertyChanged(object $sender, string $propertyName, $oldValue, $newValue) + public function propertyChanged(object $sender, string $propertyName, mixed $oldValue, mixed $newValue) { } From f75aba2244772bc5974959e88bbe640988c02ba7 Mon Sep 17 00:00:00 2001 From: saifulislamferoz Date: Thu, 15 May 2025 11:54:07 +0600 Subject: [PATCH 08/14] feat: add void return type --- Tests/Fixtures/ODM/UnitOfWork.php | 8 ++++---- Tests/Fixtures/ORM/UnitOfWork.php | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Tests/Fixtures/ODM/UnitOfWork.php b/Tests/Fixtures/ODM/UnitOfWork.php index 46d43cf..51f495a 100644 --- a/Tests/Fixtures/ODM/UnitOfWork.php +++ b/Tests/Fixtures/ODM/UnitOfWork.php @@ -10,12 +10,12 @@ class UnitOfWork implements PropertyChangedListener /** * Collect information about a property change. * - * @param object $sender the object on which the property changed + * @param object $sender the object on which the property changed * @param string $propertyName the name of the property that changed - * @param mixed $oldValue the old value of the property that changed - * @param mixed $newValue the new value of the property that changed + * @param mixed $oldValue the old value of the property that changed + * @param mixed $newValue the new value of the property that changed */ - public function propertyChanged(object $sender, string $propertyName, mixed $oldValue, mixed $newValue) + public function propertyChanged(object $sender, string $propertyName, mixed $oldValue, mixed $newValue): void { } diff --git a/Tests/Fixtures/ORM/UnitOfWork.php b/Tests/Fixtures/ORM/UnitOfWork.php index d0c9824..a15014f 100644 --- a/Tests/Fixtures/ORM/UnitOfWork.php +++ b/Tests/Fixtures/ORM/UnitOfWork.php @@ -15,7 +15,7 @@ class UnitOfWork implements PropertyChangedListener * @param mixed $oldValue the old value of the property that changed * @param mixed $newValue the new value of the property that changed */ - public function propertyChanged(object $sender, string $propertyName, mixed $oldValue, mixed $newValue) + public function propertyChanged(object $sender, string $propertyName, mixed $oldValue, mixed $newValue): void { } From 1ae3011b5550d57fd6881cac29f9a16233ccafdb Mon Sep 17 00:00:00 2001 From: saifulislamferoz Date: Thu, 15 May 2025 12:08:26 +0600 Subject: [PATCH 09/14] test: increase test coverage --- Tests/Fixtures/ORM/TestMovie.php | 59 +++++++++++++++++++++ Tests/Subscriber/DoctrineSubscriberTest.php | 6 ++- 2 files changed, 64 insertions(+), 1 deletion(-) create mode 100644 Tests/Fixtures/ORM/TestMovie.php diff --git a/Tests/Fixtures/ORM/TestMovie.php b/Tests/Fixtures/ORM/TestMovie.php new file mode 100644 index 0000000..be25ece --- /dev/null +++ b/Tests/Fixtures/ORM/TestMovie.php @@ -0,0 +1,59 @@ + + * + * This source file is subject to the MIT license that is bundled + * with this source code in the file LICENSE. + */ + +namespace Xiidea\EasyAuditBundle\Tests\Fixtures\ORM; + +use Doctrine\ORM\Mapping as ORM; +use Xiidea\EasyAuditBundle\Attribute\SubscribeDoctrineEvents; + +#[ORM\Entity] +class TestMovie +{ + /** + * @ORM\Id + * @ORM\Column(type="integer") + * @ORM\GeneratedValue(strategy="AUTO") + */ + #[ORM\Id] + #[ORM\Column(type: 'integer')] + #[ORM\GeneratedValue(strategy: "AUTO")] + protected $id; + + /** + * @ORM\Column(type="string") + */ + #[ORM\Column(type: 'string')] + protected $name; + + public function __construct($id = 1, $name = 'car') + { + $this->id = $id; + $this->name = $name; + } + + public function getName() + { + return $this->name; + } + + public function setName($name) + { + $this->name = $name; + } + + /** + * @return mixed + */ + public function getId() + { + return $this->id; + } +} diff --git a/Tests/Subscriber/DoctrineSubscriberTest.php b/Tests/Subscriber/DoctrineSubscriberTest.php index 47f072e..c60f601 100644 --- a/Tests/Subscriber/DoctrineSubscriberTest.php +++ b/Tests/Subscriber/DoctrineSubscriberTest.php @@ -13,12 +13,13 @@ use Doctrine\Persistence\Event\LifecycleEventArgs; use Doctrine\Persistence\Mapping\ClassMetadata; +use Doctrine\Persistence\ObjectManager; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Xiidea\EasyAuditBundle\Subscriber\DoctrineSubscriber; use Xiidea\EasyAuditBundle\Tests\Fixtures\ORM\DummyEntity; use Xiidea\EasyAuditBundle\Tests\Fixtures\ORM\Movie; -use Doctrine\Persistence\ObjectManager; +use Xiidea\EasyAuditBundle\Tests\Fixtures\ORM\TestMovie; class DoctrineSubscriberTest extends TestCase { @@ -70,12 +71,14 @@ public function testCreateEventForEntityConfiguredToTrackAllEvents() $subscriber = new DoctrineSubscriber(array('Xiidea\EasyAuditBundle\Tests\Fixtures\ORM\Movie' => array())); $this->invokeCreatedEventCall($subscriber); + $this->invokeCreatedEventCall($subscriber, new TestMovie()); } public function testUpdateEventForEntityNotConfiguredToTrack() { $subscriber = new DoctrineSubscriber(array()); $this->invokeUpdatedEventCall($subscriber, new DummyEntity()); + $this->invokeUpdatedEventCall($subscriber, new TestMovie()); } public function testRemovedEventForEntityNotConfiguredToTrack() @@ -123,6 +126,7 @@ private function invokeDeletedEventCall($subscriber) $subscriber->preRemove(new LifecycleEventArgs($movie, $this->entityManager)); $subscriber->postRemove(new LifecycleEventArgs($movie, $this->entityManager)); $this->assertTrue(true); + $subscriber->postRemove(new LifecycleEventArgs(new TestMovie(), $this->entityManager)); } private function mockMetaData($data = ['id' => 1]) From c660c7ae885ff52d2451a6133a1120e429ed68d4 Mon Sep 17 00:00:00 2001 From: saifulislamferoz Date: Mon, 19 May 2025 17:24:11 +0600 Subject: [PATCH 10/14] refactor: use utility class for class name --- Common/ClassUtils.php | 17 +++++++++++++++++ Resolver/DoctrineObjectEventResolver.php | 4 ++-- Subscriber/DoctrineSubscriber.php | 16 ++++++++-------- Tests/Functional/ImpersonatingUserTest.php | 8 ++++---- 4 files changed, 31 insertions(+), 14 deletions(-) create mode 100644 Common/ClassUtils.php diff --git a/Common/ClassUtils.php b/Common/ClassUtils.php new file mode 100644 index 0000000..2b14d47 --- /dev/null +++ b/Common/ClassUtils.php @@ -0,0 +1,17 @@ +getClass($args->getObject()); + $className = ClassUtils::getClass($args->getObject()); if (!isset($this->toBeDeleted[$className])) { $this->toBeDeleted[$className] = []; @@ -64,7 +64,7 @@ public function postRemove(LifecycleEventArgs $args) private function getToBeDeletedId($entity) { if ($this->isScheduledForDelete($entity)) { - return $this->toBeDeleted[$this->getClass($entity)][spl_object_hash($entity)]; + return $this->toBeDeleted[ClassUtils::getClass($entity)][spl_object_hash($entity)]; } return null; @@ -74,11 +74,11 @@ private function getToBeDeletedId($entity) * @param string $eventName * @param LifecycleEventArgs $args */ - private function handleEvent($eventName, LifecycleEventArgs $args) + private function handleEvent($eventName, LifecycleEventArgs $args): void { if (true === $this->isConfiguredToTrack($args->getObject(), $eventName)) { $this->dispatcher->dispatch( - new DoctrineObjectEvent($args, $this->getIdentity($args, $this->getClass($args->getObject()))), + new DoctrineObjectEvent($args, $this->getIdentity($args, ClassUtils::getClass($args->getObject()))), $eventName ); } @@ -92,7 +92,7 @@ private function handleEvent($eventName, LifecycleEventArgs $args) */ private function isConfiguredToTrack($entity, $eventName = '') { - $class = $this->getClass($entity); + $class = ClassUtils::getClass($entity); $eventType = DoctrineEvents::getShortEventType($eventName); if (null !== $track = $this->isAttributedEvent($entity, $eventType)) { @@ -169,7 +169,7 @@ protected function getIdentity(LifecycleEventArgs $args, $className) */ private function isScheduledForDelete($entity) { - $originalClassName = $this->getClass($entity); + $originalClassName = ClassUtils::getClass($entity); return isset($this->toBeDeleted[$originalClassName]) && isset( $this->toBeDeleted[$originalClassName][spl_object_hash( @@ -188,6 +188,6 @@ public function setDispatcher($dispatcher) private function getClass($entity): string { - return $entity instanceof Proxy ? get_parent_class($entity) : get_class($entity); + return ClassUtils::getClass($entity); } } diff --git a/Tests/Functional/ImpersonatingUserTest.php b/Tests/Functional/ImpersonatingUserTest.php index dd98317..a060a68 100644 --- a/Tests/Functional/ImpersonatingUserTest.php +++ b/Tests/Functional/ImpersonatingUserTest.php @@ -11,9 +11,9 @@ namespace Xiidea\EasyAuditBundle\Tests\Functional; +use Symfony\Bundle\FrameworkBundle\Test\WebTestCase; use Symfony\Component\DomCrawler\Crawler; use Xiidea\EasyAuditBundle\Tests\Functional\Bundle\TestBundle\Controller\DefaultController; -use Symfony\Bundle\FrameworkBundle\Test\WebTestCase; class ImpersonatingUserTest extends WebTestCase { @@ -38,7 +38,7 @@ public function testSecuredEventWithImpersonatingUser() $name = 'simple.event'; $this->client->request('GET', "/some-secure-url/{$name}", [], [], [ - "HTTP_AUTHORIZATION" => "Basic " . base64_encode("admin:login") + "HTTP_AUTHORIZATION" => "Basic ".base64_encode("admin:login"), ]); $crawler = $this->client->request('GET', "/some-secure-url/{$name}?_switch_user=user"); $this->assertTrue($this->client->getResponse()->isSuccessful()); @@ -71,7 +71,7 @@ private function getEventArrayFromResponse(Crawler $crawler): array { $html = $crawler->html(); $parts = explode(DefaultController::RESPONSE_BOUNDARY, $html); - $event = unserialize($parts[1]); - return $event; + + return unserialize($parts[1]); } } From 0cc63433ad73fc306852f862c8a061d90983825d Mon Sep 17 00:00:00 2001 From: saifulislamferoz Date: Mon, 19 May 2025 18:21:12 +0600 Subject: [PATCH 11/14] test: add testcase for class util test --- Tests/Common/ClassUtilTest.php | 40 ++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) create mode 100644 Tests/Common/ClassUtilTest.php diff --git a/Tests/Common/ClassUtilTest.php b/Tests/Common/ClassUtilTest.php new file mode 100644 index 0000000..d778228 --- /dev/null +++ b/Tests/Common/ClassUtilTest.php @@ -0,0 +1,40 @@ +assertSame(DummyClass::class, $result); + } + + public function testGetClassWithProxyObject() + { + $proxy = new DummyProxyClass(); + $result = ClassUtils::getClass($proxy); + $this->assertSame(DummyClass::class, $result); + } +} \ No newline at end of file From e9574eaea67b255b352bc1a0b6be69c5e3d185c7 Mon Sep 17 00:00:00 2001 From: saifulislamferoz Date: Mon, 19 May 2025 18:23:19 +0600 Subject: [PATCH 12/14] fix(test): error for return type --- Tests/Common/ClassUtilTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Tests/Common/ClassUtilTest.php b/Tests/Common/ClassUtilTest.php index d778228..6c62fb1 100644 --- a/Tests/Common/ClassUtilTest.php +++ b/Tests/Common/ClassUtilTest.php @@ -12,11 +12,11 @@ class DummyClass class DummyProxyClass extends DummyClass implements Proxy { - public function __load() + public function __load(): void { } - public function __isInitialized() + public function __isInitialized(): bool { return true; } From 39eaabdba9622433ac88c1d287fc99651b8ba942 Mon Sep 17 00:00:00 2001 From: saifulislamferoz Date: Mon, 19 May 2025 18:25:51 +0600 Subject: [PATCH 13/14] refactor: remove unused method --- Subscriber/DoctrineSubscriber.php | 5 ----- 1 file changed, 5 deletions(-) diff --git a/Subscriber/DoctrineSubscriber.php b/Subscriber/DoctrineSubscriber.php index 518b0da..4f7669c 100644 --- a/Subscriber/DoctrineSubscriber.php +++ b/Subscriber/DoctrineSubscriber.php @@ -185,9 +185,4 @@ public function setDispatcher($dispatcher) { $this->dispatcher = $dispatcher; } - - private function getClass($entity): string - { - return ClassUtils::getClass($entity); - } } From 23fee6b316c36f581d1afd72bc9ac1625e7b2a21 Mon Sep 17 00:00:00 2001 From: saifulislamferoz Date: Wed, 19 Nov 2025 16:35:40 +0600 Subject: [PATCH 14/14] fix: fire created event though event configured for update --- Attribute/SubscribeDoctrineEvents.php | 33 +++++++---- .../Attribute/SubscribeDoctrineEventsTest.php | 24 ++------ Tests/Fixtures/ORM/Book.php | 56 +++++++++++++++++++ Tests/Fixtures/ORM/Machine.php | 54 ++++++++++++++++++ Tests/Fixtures/ORM/Movie.php | 11 +--- Tests/Subscriber/DoctrineSubscriberTest.php | 48 ++++++++-------- 6 files changed, 167 insertions(+), 59 deletions(-) create mode 100644 Tests/Fixtures/ORM/Book.php create mode 100644 Tests/Fixtures/ORM/Machine.php diff --git a/Attribute/SubscribeDoctrineEvents.php b/Attribute/SubscribeDoctrineEvents.php index 81e2760..6577d1e 100644 --- a/Attribute/SubscribeDoctrineEvents.php +++ b/Attribute/SubscribeDoctrineEvents.php @@ -16,22 +16,35 @@ * * @author Roni Saha */ - #[\Attribute(\Attribute::TARGET_CLASS)] -/* @final */ class SubscribeDoctrineEvents +/* @final */ +class SubscribeDoctrineEvents { - public $events = array(); + public array $events = []; - public function __construct(array $values) + public function __construct(array|string $values) { - if (isset($values['value'])) { - $values['events'] = $values['value']; - } - if (!isset($values['events'])) { + $validValues = [ + 'created', + 'updated', + ]; + $valueValueStr = [ + 'created,updated', + 'created, updated', + 'updated,created', + 'updated, created', + ]; + if (!empty($values) && is_string($values) && !in_array($values, $valueValueStr)) { return; } - - $this->events = is_array($values['events']) ? $values['events'] : array_map('trim', explode(',', $values['events'])); + if (!empty($values) && is_array($values)) { + foreach ($values as $value) { + if (!in_array($value, $validValues)) { + return; + } + } + } + $this->events = is_array($values) ? $values : array_map('trim', explode(',', $values)); $this->events = array_filter($this->events); } diff --git a/Tests/Attribute/SubscribeDoctrineEventsTest.php b/Tests/Attribute/SubscribeDoctrineEventsTest.php index 1be9671..a165ad2 100644 --- a/Tests/Attribute/SubscribeDoctrineEventsTest.php +++ b/Tests/Attribute/SubscribeDoctrineEventsTest.php @@ -18,7 +18,7 @@ class SubscribeDoctrineEventsTest extends TestCase { public function testConstructWithoutData() { - $annotation = new SubscribeDoctrineEvents(array()); + $annotation = new SubscribeDoctrineEvents([]); $this->assertTrue(is_array($annotation->events)); $this->assertEmpty($annotation->events); @@ -26,10 +26,10 @@ public function testConstructWithoutData() public function testConstructWithInvalidData() { - $data = array( + $data = [ 'unknown' => 'foo', - 'array' => array('bar' => 'bar'), - ); + 'array' => ['bar' => 'bar'], + ]; $annotation = new SubscribeDoctrineEvents($data); @@ -39,25 +39,13 @@ public function testConstructWithInvalidData() public function testConstructWithValue() { - $data = array('value' => 'updated,created'); + $data = 'created,updated'; $annotation = new SubscribeDoctrineEvents($data); $this->assertTrue(is_array($annotation->events)); $this->assertNotEmpty($annotation->events); - $this->assertEquals(explode(',', $data['value']), $annotation->events); - } - - public function testConstructWithEvent() - { - $data = array('events' => 'updated,created'); - - $annotation = new SubscribeDoctrineEvents($data); - - $this->assertTrue(is_array($annotation->events)); - $this->assertNotEmpty($annotation->events); - - $this->assertEquals(explode(',', $data['events']), $annotation->events); + $this->assertEquals(explode(',', $data), $annotation->events); } } diff --git a/Tests/Fixtures/ORM/Book.php b/Tests/Fixtures/ORM/Book.php new file mode 100644 index 0000000..cc5c74e --- /dev/null +++ b/Tests/Fixtures/ORM/Book.php @@ -0,0 +1,56 @@ + + * + * This source file is subject to the MIT license that is bundled + * with this source code in the file LICENSE. + */ + +namespace Xiidea\EasyAuditBundle\Tests\Fixtures\ORM; + +use Doctrine\ORM\Mapping as ORM; +use Xiidea\EasyAuditBundle\Attribute\SubscribeDoctrineEvents; + +/** + * @ORM\Entity + */ +#[SubscribeDoctrineEvents(['updated'])] +#[ORM\Entity] +class Book +{ + + #[ORM\Id] + #[ORM\Column(type: 'integer')] + #[ORM\GeneratedValue(strategy: "AUTO")] + protected $id; + + #[ORM\Column(type: 'string')] + protected $name; + + public function __construct($id = 1, $name = 'car') + { + $this->id = $id; + $this->name = $name; + } + + public function getName() + { + return $this->name; + } + + public function setName($name) + { + $this->name = $name; + } + + /** + * @return mixed + */ + public function getId() + { + return $this->id; + } +} diff --git a/Tests/Fixtures/ORM/Machine.php b/Tests/Fixtures/ORM/Machine.php new file mode 100644 index 0000000..5091999 --- /dev/null +++ b/Tests/Fixtures/ORM/Machine.php @@ -0,0 +1,54 @@ + + * + * This source file is subject to the MIT license that is bundled + * with this source code in the file LICENSE. + */ + +namespace Xiidea\EasyAuditBundle\Tests\Fixtures\ORM; + +use Doctrine\ORM\Mapping as ORM; +use Xiidea\EasyAuditBundle\Attribute\SubscribeDoctrineEvents; + + +#[SubscribeDoctrineEvents([])] +#[ORM\Entity] +class Machine +{ + + #[ORM\Id] + #[ORM\Column(type: 'integer')] + #[ORM\GeneratedValue(strategy: "AUTO")] + protected $id; + + #[ORM\Column(type: 'string')] + protected $name; + + public function __construct($id = 1, $name = 'car') + { + $this->id = $id; + $this->name = $name; + } + + public function getName() + { + return $this->name; + } + + public function setName($name) + { + $this->name = $name; + } + + /** + * @return mixed + */ + public function getId() + { + return $this->id; + } +} diff --git a/Tests/Fixtures/ORM/Movie.php b/Tests/Fixtures/ORM/Movie.php index 12e57bd..0954520 100644 --- a/Tests/Fixtures/ORM/Movie.php +++ b/Tests/Fixtures/ORM/Movie.php @@ -17,23 +17,16 @@ /** * @ORM\Entity */ -#[SubscribeDoctrineEvents([])] +#[SubscribeDoctrineEvents(['created'])] #[ORM\Entity] class Movie { - /** - * @ORM\Id - * @ORM\Column(type="integer") - * @ORM\GeneratedValue(strategy="AUTO") - */ + #[ORM\Id] #[ORM\Column(type: 'integer')] #[ORM\GeneratedValue(strategy: "AUTO")] protected $id; - /** - * @ORM\Column(type="string") - */ #[ORM\Column(type: 'string')] protected $name; diff --git a/Tests/Subscriber/DoctrineSubscriberTest.php b/Tests/Subscriber/DoctrineSubscriberTest.php index c60f601..e320f5c 100644 --- a/Tests/Subscriber/DoctrineSubscriberTest.php +++ b/Tests/Subscriber/DoctrineSubscriberTest.php @@ -17,16 +17,15 @@ use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Xiidea\EasyAuditBundle\Subscriber\DoctrineSubscriber; -use Xiidea\EasyAuditBundle\Tests\Fixtures\ORM\DummyEntity; +use Xiidea\EasyAuditBundle\Tests\Fixtures\ORM\Book; +use Xiidea\EasyAuditBundle\Tests\Fixtures\ORM\Machine; use Xiidea\EasyAuditBundle\Tests\Fixtures\ORM\Movie; -use Xiidea\EasyAuditBundle\Tests\Fixtures\ORM\TestMovie; class DoctrineSubscriberTest extends TestCase { /** @var MockObject */ private $dispatcher; - /** @var MockObject */ private $entityManager; @@ -46,74 +45,79 @@ public function setUp(): void public function testCreateEventForAttributedEntity() { - $subscriber = new DoctrineSubscriber(array()); + $subscriber = new DoctrineSubscriber([]); $this->invokeCreatedEventCall($subscriber); + $this->assertTrue(true); } public function testCreateEventForEntityNotConfiguredToTrack() { - $subscriber = new DoctrineSubscriber(array()); - $this->invokeCreatedEventCall($subscriber, new DummyEntity()); + $subscriber = new DoctrineSubscriber([]); + $this->invokeCreatedEventCall($subscriber); + $this->assertTrue(true); } public function testCreateEventForEntityConfiguredToTrack() { - $subscriber = new DoctrineSubscriber( - array('Xiidea\EasyAuditBundle\Tests\Fixtures\ORM\Movie' => array('created')) - ); + $subscriber = new DoctrineSubscriber(['Xiidea\EasyAuditBundle\Tests\Fixtures\ORM\Movie' => ['created']]); $this->invokeCreatedEventCall($subscriber); + $this->assertTrue(true); } public function testCreateEventForEntityConfiguredToTrackAllEvents() { - $subscriber = new DoctrineSubscriber(array('Xiidea\EasyAuditBundle\Tests\Fixtures\ORM\Movie' => array())); + $subscriber = new DoctrineSubscriber(['Xiidea\EasyAuditBundle\Tests\Fixtures\ORM\Movie' => []]); $this->invokeCreatedEventCall($subscriber); - $this->invokeCreatedEventCall($subscriber, new TestMovie()); + $this->assertTrue(true); } public function testUpdateEventForEntityNotConfiguredToTrack() { - $subscriber = new DoctrineSubscriber(array()); - $this->invokeUpdatedEventCall($subscriber, new DummyEntity()); - $this->invokeUpdatedEventCall($subscriber, new TestMovie()); + $subscriber = new DoctrineSubscriber([]); + $this->invokeUpdatedEventCall($subscriber); + $this->assertTrue(true); } public function testRemovedEventForEntityNotConfiguredToTrack() { - $subscriber = new DoctrineSubscriber(array()); + $subscriber = new DoctrineSubscriber([]); $this->invokeDeletedEventCall($subscriber); + $this->assertTrue(true); } public function testRemovedEventForEntityConfiguredToTrackAllEvent() { $this->mockMetaData(); - $subscriber = new DoctrineSubscriber(array('Xiidea\EasyAuditBundle\Tests\Fixtures\ORM\Movie' => array())); + $subscriber = new DoctrineSubscriber(['Xiidea\EasyAuditBundle\Tests\Fixtures\ORM\Movie' => []]); $this->invokeDeletedEventCall($subscriber); + $this->assertTrue(true); } /** * @param DoctrineSubscriber $subscriber */ - private function invokeCreatedEventCall($subscriber, $entity = null) + private function invokeCreatedEventCall($subscriber) { $subscriber->setDispatcher($this->dispatcher); - $subscriber->postPersist(new LifecycleEventArgs($entity ?? new Movie(), $this->entityManager)); + + $subscriber->postPersist(new LifecycleEventArgs(new Movie(), $this->entityManager)); + $subscriber->postPersist(new LifecycleEventArgs(new Machine(), $this->entityManager)); $this->assertTrue(true); } /** * @param DoctrineSubscriber $subscriber */ - private function invokeUpdatedEventCall($subscriber, $entity = null) + private function invokeUpdatedEventCall($subscriber) { $subscriber->setDispatcher($this->dispatcher); - $subscriber->postUpdate(new LifecycleEventArgs($entity ?? new Movie(), $this->entityManager)); - $this->assertTrue(true); + $subscriber->postUpdate(new LifecycleEventArgs(new Book(), $this->entityManager)); + $subscriber->postUpdate(new LifecycleEventArgs(new Machine(), $this->entityManager)); } /** @@ -122,11 +126,11 @@ private function invokeUpdatedEventCall($subscriber, $entity = null) private function invokeDeletedEventCall($subscriber) { $subscriber->setDispatcher($this->dispatcher); + $movie = new Movie(); $subscriber->preRemove(new LifecycleEventArgs($movie, $this->entityManager)); $subscriber->postRemove(new LifecycleEventArgs($movie, $this->entityManager)); $this->assertTrue(true); - $subscriber->postRemove(new LifecycleEventArgs(new TestMovie(), $this->entityManager)); } private function mockMetaData($data = ['id' => 1])