From dd0d5fb7a3045141d7c5a0a22ddb6dec6880b6dd Mon Sep 17 00:00:00 2001 From: "Konstantin.Myakshin" Date: Thu, 1 Sep 2016 17:38:34 +0300 Subject: [PATCH] Use preFlush event introduced in Doctrine 2.2 and minior refactoring (closes #8, #89, #123) --- EventListener/Doctrine/BaseListener.php | 4 +- EventListener/Doctrine/CleanListener.php | 15 ++--- EventListener/Doctrine/InjectListener.php | 11 ++- EventListener/Doctrine/RemoveListener.php | 25 +++---- EventListener/Doctrine/UploadListener.php | 33 ++------- Resources/doc/known_issues.md | 39 ----------- .../Doctrine/CleanListenerTest.php | 20 ++---- .../Doctrine/InjectListenerTest.php | 3 +- .../Doctrine/RemoveListenerTest.php | 3 +- .../Doctrine/UploadListenerTest.php | 67 ++----------------- 10 files changed, 46 insertions(+), 174 deletions(-) diff --git a/EventListener/Doctrine/BaseListener.php b/EventListener/Doctrine/BaseListener.php index f5f655f8..d2b128f6 100644 --- a/EventListener/Doctrine/BaseListener.php +++ b/EventListener/Doctrine/BaseListener.php @@ -55,7 +55,7 @@ public function __construct($mapping, AdapterInterface $adapter, MetadataReader /** * Checks if the given object is uploadable using the current mapping. * - * @param mixed $object The object to test. + * @param object $object The object to test. * * @return bool */ @@ -67,7 +67,7 @@ protected function isUploadable($object) /** * Returns a list of uploadable fields for the given object and mapping. * - * @param mixed $object The object to use. + * @param object $object The object to use. * * @return array A list of field names. */ diff --git a/EventListener/Doctrine/CleanListener.php b/EventListener/Doctrine/CleanListener.php index f3a33014..7dfe99e3 100644 --- a/EventListener/Doctrine/CleanListener.php +++ b/EventListener/Doctrine/CleanListener.php @@ -3,6 +3,7 @@ namespace Vich\UploaderBundle\EventListener\Doctrine; use Doctrine\Common\EventArgs; +use Doctrine\ORM\Events; /** * CleanListener @@ -10,25 +11,22 @@ * Listen to the update event to delete old files accordingly. * * @author Kévin Gomez + * @author Konstantin Myakshin */ class CleanListener extends BaseListener { /** - * The events the listener is subscribed to. - * - * @return array The array of events. + * {@inheritdoc} */ public function getSubscribedEvents() { - return array( - 'preUpdate', - ); + return array(Events::preFlush); } /** - * @param EventArgs $event The event. + * {@inheritdoc} */ - public function preUpdate(EventArgs $event) + public function preFlush(EventArgs $event) { $object = $this->adapter->getObjectFromArgs($event); @@ -38,7 +36,6 @@ public function preUpdate(EventArgs $event) foreach ($this->getUploadableFields($object) as $field) { $this->handler->clean($object, $field); - $this->adapter->recomputeChangeSet($event); } } } diff --git a/EventListener/Doctrine/InjectListener.php b/EventListener/Doctrine/InjectListener.php index 766c771d..037edfad 100644 --- a/EventListener/Doctrine/InjectListener.php +++ b/EventListener/Doctrine/InjectListener.php @@ -3,6 +3,7 @@ namespace Vich\UploaderBundle\EventListener\Doctrine; use Doctrine\Common\EventArgs; +use Doctrine\DBAL\Events; /** * InjectListener @@ -14,19 +15,15 @@ class InjectListener extends BaseListener { /** - * The events the listener is subscribed to. - * - * @return array The array of events. + * {@inheritdoc} */ public function getSubscribedEvents() { - return array( - 'postLoad', - ); + return array(Events::postLoad); } /** - * @param EventArgs $event The event. + * {@inheritdoc} */ public function postLoad(EventArgs $event) { diff --git a/EventListener/Doctrine/RemoveListener.php b/EventListener/Doctrine/RemoveListener.php index 0912e1ab..e9fc259a 100644 --- a/EventListener/Doctrine/RemoveListener.php +++ b/EventListener/Doctrine/RemoveListener.php @@ -4,6 +4,7 @@ use Doctrine\Common\EventArgs; use Doctrine\Common\Persistence\Proxy; +use Doctrine\ORM\Events; /** * RemoveListener @@ -15,34 +16,30 @@ class RemoveListener extends BaseListener { /** - * The events the listener is subscribed to. - * - * @return array The array of events. + * {@inheritdoc} */ public function getSubscribedEvents() { return array( - 'preRemove', - 'postRemove', + Events::preRemove, + Events::postRemove, ); } /** - * Ensures a proxy will be usable in the postRemove. - * - * @param EventArgs $event The event. + * {@inheritdoc} */ public function preRemove(EventArgs $event) { - $object = $this->adapter->getObjectFromArgs($event); - - if ($this->isUploadable($object) && $object instanceof Proxy) { - $object->__load(); - } + $object = $this->adapter->getObjectFromArgs($event); + // Ensures a proxy will be usable in the postRemove. + if ($this->isUploadable($object) && $object instanceof Proxy) { + $object->__load(); + } } /** - * @param EventArgs $event The event. + * {@inheritdoc} */ public function postRemove(EventArgs $event) { diff --git a/EventListener/Doctrine/UploadListener.php b/EventListener/Doctrine/UploadListener.php index b8f393ef..a6578294 100644 --- a/EventListener/Doctrine/UploadListener.php +++ b/EventListener/Doctrine/UploadListener.php @@ -3,6 +3,7 @@ namespace Vich\UploaderBundle\EventListener\Doctrine; use Doctrine\Common\EventArgs; +use Doctrine\ORM\Events; /** * UploadListener @@ -10,26 +11,22 @@ * Handles file uploads. * * @author Kévin Gomez + * @author Konstantin Myakshin */ class UploadListener extends BaseListener { /** - * The events the listener is subscribed to. - * - * @return array The array of events. + * {@inheritdoc} */ public function getSubscribedEvents() { - return array( - 'prePersist', - 'preUpdate', - ); + return array(Events::preFlush); } /** - * @param EventArgs $event The event. + * {@inheritdoc} */ - public function prePersist(EventArgs $event) + public function preFlush(EventArgs $event) { $object = $this->adapter->getObjectFromArgs($event); @@ -41,22 +38,4 @@ public function prePersist(EventArgs $event) $this->handler->upload($object, $field); } } - - /** - * @param EventArgs $event The event. - */ - public function preUpdate(EventArgs $event) - { - $object = $this->adapter->getObjectFromArgs($event); - - if (!$this->isUploadable($object)) { - return; - } - - foreach ($this->getUploadableFields($object) as $field) { - $this->handler->upload($object, $field); - } - - $this->adapter->recomputeChangeSet($event); - } } diff --git a/Resources/doc/known_issues.md b/Resources/doc/known_issues.md index 071c7e26..e65adda2 100644 --- a/Resources/doc/known_issues.md +++ b/Resources/doc/known_issues.md @@ -1,45 +1,6 @@ Known issues ============ -## The file is not updated if there are not other changes in the entity - -As the bundle is listening to Doctrine `prePersist` and `preUpdate` events, which are not fired -when there is no change on field mapped by Doctrine, the file upload is not handled if the image field -is the only updated. - -A workaround to solve this issue is to manually generate a change: - -```php -use Symfony\Component\HttpFoundation\File\File; -use Symfony\Component\HttpFoundation\File\UploadedFile; - -class Product -{ - // ... - - /** - * @ORM\Column(type="datetime") - * - * @var \DateTime - */ - private $updatedAt; - - // ... - - public function setImage(File $image = null) - { - $this->image = $image; - - // Only change the updated af if the file is really uploaded to avoid database updates. - // This is needed when the file should be set when loading the entity. - if ($this->image instanceof UploadedFile) { - $this->updatedAt = new \DateTime('now'); - } - } -} -``` -See issue [GH-123](https://github.com/dustin10/VichUploaderBundle/issues/123) - ## Catchable Fatal Error When you get the following error:`Catchable Fatal Error: ... must be an instance of Symfony\Component\HttpFoundation\File\UploadedFile, string given, ...` diff --git a/Tests/EventListener/Doctrine/CleanListenerTest.php b/Tests/EventListener/Doctrine/CleanListenerTest.php index 79309aa1..2f767bc9 100644 --- a/Tests/EventListener/Doctrine/CleanListenerTest.php +++ b/Tests/EventListener/Doctrine/CleanListenerTest.php @@ -2,6 +2,7 @@ namespace Vich\UploaderBundle\Tests\EventListener\Doctrine; +use Doctrine\ORM\Events; use Vich\UploaderBundle\EventListener\Doctrine\CleanListener; /** @@ -28,11 +29,11 @@ public function testGetSubscribedEvents() { $events = $this->listener->getSubscribedEvents(); - $this->assertSame(array('preUpdate'), $events); + $this->assertSame(array(Events::preFlush), $events); } /** - * Test the preUpdate method. + * Test the preFlush method. */ public function testPreUpdate() { @@ -55,16 +56,11 @@ public function testPreUpdate() ->method('clean') ->with($this->object, 'field_name'); - $this->adapter - ->expects($this->once()) - ->method('recomputeChangeSet') - ->with($this->event); - - $this->listener->preUpdate($this->event); + $this->listener->preFlush($this->event); } /** - * Test that preUpdate skips non uploadable entity. + * Test that preFlush skips non uploadable entity. */ public function testPreUpdateSkipsNonUploadable() { @@ -78,10 +74,6 @@ public function testPreUpdateSkipsNonUploadable() ->expects($this->never()) ->method('clean'); - $this->adapter - ->expects($this->never()) - ->method('recomputeChangeSet'); - - $this->listener->preUpdate($this->event); + $this->listener->preFlush($this->event); } } diff --git a/Tests/EventListener/Doctrine/InjectListenerTest.php b/Tests/EventListener/Doctrine/InjectListenerTest.php index a5e4c0ef..748ed61a 100644 --- a/Tests/EventListener/Doctrine/InjectListenerTest.php +++ b/Tests/EventListener/Doctrine/InjectListenerTest.php @@ -2,6 +2,7 @@ namespace Vich\UploaderBundle\Tests\EventListener\Doctrine; +use Doctrine\ORM\Events; use Vich\UploaderBundle\EventListener\Doctrine\InjectListener; /** @@ -28,7 +29,7 @@ public function testGetSubscribedEvents() { $events = $this->listener->getSubscribedEvents(); - $this->assertSame(array('postLoad'), $events); + $this->assertSame(array(Events::postLoad), $events); } /** diff --git a/Tests/EventListener/Doctrine/RemoveListenerTest.php b/Tests/EventListener/Doctrine/RemoveListenerTest.php index b610145a..1ed47199 100644 --- a/Tests/EventListener/Doctrine/RemoveListenerTest.php +++ b/Tests/EventListener/Doctrine/RemoveListenerTest.php @@ -2,6 +2,7 @@ namespace Vich\UploaderBundle\Tests\EventListener\Doctrine; +use Doctrine\ORM\Events; use Vich\UploaderBundle\EventListener\Doctrine\RemoveListener; /** @@ -28,7 +29,7 @@ public function testGetSubscribedEvents() { $events = $this->listener->getSubscribedEvents(); - $this->assertSame(array('preRemove', 'postRemove'), $events); + $this->assertSame(array(Events::preRemove, Events::postRemove), $events); } public function testPreRemove() diff --git a/Tests/EventListener/Doctrine/UploadListenerTest.php b/Tests/EventListener/Doctrine/UploadListenerTest.php index cf1f0bf0..345f1c52 100644 --- a/Tests/EventListener/Doctrine/UploadListenerTest.php +++ b/Tests/EventListener/Doctrine/UploadListenerTest.php @@ -2,6 +2,7 @@ namespace Vich\UploaderBundle\Tests\EventListener\Doctrine; +use Doctrine\ORM\Events; use Vich\UploaderBundle\EventListener\Doctrine\UploadListener; /** @@ -28,13 +29,13 @@ public function testGetSubscribedEvents() { $events = $this->listener->getSubscribedEvents(); - $this->assertSame(array('prePersist', 'preUpdate'), $events); + $this->assertSame(array(Events::preFlush), $events); } /** - * Tests the prePersist method. + * Tests the preFlush method. */ - public function testPrePersist() + public function testPreFlush() { $this->metadata ->expects($this->once()) @@ -55,11 +56,11 @@ public function testPrePersist() ->method('upload') ->with($this->object, 'field_name'); - $this->listener->prePersist($this->event); + $this->listener->preFlush($this->event); } /** - * Tests that prePersist skips non-uploadable entity. + * Tests that preFlush skips non-uploadable entity. */ public function testPrePersistSkipsNonUploadable() { @@ -73,60 +74,6 @@ public function testPrePersistSkipsNonUploadable() ->expects($this->never()) ->method('upload'); - $this->listener->prePersist($this->event); - } - - /** - * Test the preUpdate method. - */ - public function testPreUpdate() - { - $this->adapter - ->expects($this->once()) - ->method('recomputeChangeSet') - ->with($this->event); - - $this->metadata - ->expects($this->once()) - ->method('isUploadable') - ->with('Vich\UploaderBundle\Tests\DummyEntity') - ->will($this->returnValue(true)); - - $this->metadata - ->expects($this->once()) - ->method('getUploadableFields') - ->with('Vich\UploaderBundle\Tests\DummyEntity', self::MAPPING_NAME) - ->will($this->returnValue(array( - array('propertyName' => 'field_name') - ))); - - $this->handler - ->expects($this->once()) - ->method('upload') - ->with($this->object, 'field_name'); - - $this->listener->preUpdate($this->event); - } - - /** - * Test that preUpdate skips non uploadable entity. - */ - public function testPreUpdateSkipsNonUploadable() - { - $this->metadata - ->expects($this->once()) - ->method('isUploadable') - ->with('Vich\UploaderBundle\Tests\DummyEntity') - ->will($this->returnValue(false)); - - $this->adapter - ->expects($this->never()) - ->method('recomputeChangeSet'); - - $this->handler - ->expects($this->never()) - ->method('upload'); - - $this->listener->preUpdate($this->event); + $this->listener->preFlush($this->event); } }