Add transactions support

README.md

@@ -142,7 +142,7 @@ services:
 
     Arxy\FilesBundle\Twig\FilesExtension:
         tags:
-            - { name: twig.extension }
+            - {name: twig.extension}
 
     Arxy\FilesBundle\NamingStrategy\IdToPathStrategy: ~
     Arxy\FilesBundle\NamingStrategy\AppendExtensionStrategy:
@@ -153,25 +153,25 @@ services:
 
     Arxy\FilesBundle\Storage\FlysystemStorage: ~
 
-    Arxy\FilesBundle\Storage: 
+    Arxy\FilesBundle\Storage:
         alias: 'Arxy\FilesBundle\Storage\FlysystemStorage'
-        
+
     Arxy\FilesBundle\Manager:
         $class: 'App\Entity\File'
 
     Arxy\FilesBundle\ManagerInterface:
         alias: Arxy\FilesBundle\Manager
 
     Arxy\FilesBundle\EventListener\DoctrineORMListener:
-        arguments: [ "@Arxy\\FilesBundle\\ManagerInterface" ] # This can be omit, if using autowiring.
+        arguments: ["@Arxy\\FilesBundle\\ManagerInterface"] # This can be omit, if using autowiring.
         tags:
-            - { name: doctrine.event_listener, event: 'postPersist' }
-            - { name: doctrine.event_listener, event: 'preRemove' }
+            - {name: doctrine.event_listener, event: 'postPersist'}
+            - {name: doctrine.event_listener, event: 'preRemove'}
 
     Arxy\FilesBundle\Form\Type\FileType:
-        arguments: [ "@Arxy\\FilesBundle\\ManagerInterface" ] # This can be omit, if using autowiring.
+        arguments: ["@Arxy\\FilesBundle\\ManagerInterface"] # This can be omit, if using autowiring.
         tags: # This can be omit, if using autowiring.
-            - { name: form.type }
+            - {name: form.type}
 ```
 
 or using pure PHP
@@ -623,7 +623,7 @@ bin/console arxy:files:migrate-naming-strategy
 
 ```yaml
     MicrosoftAzure\Storage\Blob\BlobRestProxy:
-        factory: [ 'MicrosoftAzure\Storage\Blob\BlobRestProxy', 'createBlobService' ]
+        factory: ['MicrosoftAzure\Storage\Blob\BlobRestProxy', 'createBlobService']
         arguments:
             $connectionString: 'DefaultEndpointsProtocol=https;AccountName=xxxxxxxx;EndpointSuffix=core.windows.net'
 
@@ -728,7 +728,7 @@ There is also DelegatingManager, which can be used as router to different other
 
 ```yaml
     Arxy\FilesBundle\DelegatingManager:
-        $managers: [ '@manager_1', '@manager_2' ]
+        $managers: ['@manager_1', '@manager_2']
 ```
 
 Then you can do: `$manager->getManagerFor(File::class)->upload($file)`. Note: If you do
@@ -975,4 +975,4 @@ Currently, only image preview generator exists. You can add your own image previ
 
 # Known issues
 
-- If file entity is deleted within transaction and transaction is rolled back - file will be deleted. I'm waiting for DBAL 3.2.* release to be able to fix that.
+No known issues.

composer.json

@@ -13,8 +13,9 @@
         "gabrielelana/byte-units": "^0.5.0"
     },
     "require-dev": {
+        "doctrine/dbal": "3.2.x-dev",
         "symfony/validator": "*",
-        "doctrine/orm": "*",
+        "doctrine/orm": "3.0.x-dev",
         "symfony/form": "*",
         "symfony/http-foundation": "*",
         "twig/twig": "*",
@@ -29,7 +30,7 @@
         "symfony/dependency-injection": "^5.2",
         "infection/infection": "^0.21.4",
         "symfony/symfony": "^4.4 | ^5.2",
-        "doctrine/doctrine-bundle": "^2.3",
+        "doctrine/doctrine-bundle": "2.5.x-dev",
         "liip/imagine-bundle": "^2.6",
         "vimeo/psalm": "^4.7",
         "league/flysystem-bundle": "^2.0",

src/DependencyInjection/ArxyFilesExtension.php

@@ -12,6 +12,8 @@
 use Arxy\FilesBundle\Storage;
 use Arxy\FilesBundle\Twig\FilesExtension;
 use Arxy\FilesBundle\Twig\FilesRuntime;
+use Doctrine\DBAL\Events as DbalEvents;
+use Doctrine\ORM\Events as OrmEvents;
 use LogicException;
 use Psr\EventDispatcher\EventDispatcherInterface;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
@@ -136,9 +138,17 @@ private function createListenerDefinition(string $driver, string $serviceId): De
             case 'orm':
                 $definition = new Definition(DoctrineORMListener::class);
                 $definition->setArgument('$manager', new Reference($serviceId));
-                $definition->addTag('doctrine.event_listener', ['event' => 'postPersist', 'lazy' => true]);
-                $definition->addTag('doctrine.event_listener', ['event' => 'postRemove', 'lazy' => true]);
-                $definition->addTag('doctrine.event_listener', ['event' => 'onClear', 'lazy' => true]);
+                $definition->addTag('doctrine.event_listener', ['event' => OrmEvents::postPersist, 'lazy' => true]);
+                $definition->addTag('doctrine.event_listener', ['event' => OrmEvents::postRemove, 'lazy' => true]);
+                $definition->addTag('doctrine.event_listener', ['event' => OrmEvents::onClear, 'lazy' => true]);
+                $definition->addTag(
+                    'doctrine.event_listener',
+                    ['event' => DbalEvents::onTransactionCommit, 'lazy' => true]
+                );
+                $definition->addTag(
+                    'doctrine.event_listener',
+                    ['event' => DbalEvents::onTransactionRollBack, 'lazy' => true]
+                );
 
                 return $definition;
             default:

src/EventListener/DoctrineORMListener.php

@@ -6,40 +6,38 @@
 
 use Arxy\FilesBundle\ManagerInterface;
 use Arxy\FilesBundle\Model\File;
-use Closure;
 use Doctrine\Common\Util\ClassUtils;
+use Doctrine\DBAL\Event\TransactionCommitEventArgs;
+use Doctrine\DBAL\Event\TransactionRollBackEventArgs;
 use Doctrine\ORM\EntityManagerInterface;
 use Doctrine\ORM\Event\LifecycleEventArgs;
+use Doctrine\ORM\Event\OnClearEventArgs;
 use ReflectionObject;
 
 final class DoctrineORMListener
 {
     private ManagerInterface $manager;
+    /** @var class-string<File> */
     private string $class;
-    private Closure $move;
-    private Closure $remove;
+    private array $pendingMove = [];
+    private array $pendingRemove = [];
 
     public function __construct(ManagerInterface $manager)
     {
         $this->class = $manager->getClass();
         $this->manager = $manager;
-
-        $this->move = static function (File $file) use ($manager): void {
-            $manager->moveFile($file);
-        };
-        $this->remove = static function (File $file) use ($manager): void {
-            $manager->remove($file);
-        };
     }
 
     public function postPersist(LifecycleEventArgs $eventArgs): void
     {
         $entity = $eventArgs->getEntity();
         $entityManager = $eventArgs->getEntityManager();
         if ($this->supports($entity)) {
-            ($this->move)($entity);
+            $this->pendingMove[] = $entity;
+        }
+        foreach ($this->handleEmbeddable($entityManager, $entity) as $file) {
+            $this->pendingMove[] = $file;
         }
-        $this->handleEmbeddable($entityManager, $entity, $this->move);
     }
 
     public function postRemove(LifecycleEventArgs $eventArgs): void
@@ -48,26 +46,63 @@ public function postRemove(LifecycleEventArgs $eventArgs): void
         $entityManager = $eventArgs->getEntityManager();
 
         if ($this->supports($entity)) {
-            ($this->remove)($entity);
+            $this->pendingRemove[] = $entity;
+        }
+        foreach ($this->handleEmbeddable($entityManager, $entity) as $file) {
+            $this->pendingRemove[] = $file;
+        }
+    }
+
+    public function onTransactionCommit(TransactionCommitEventArgs $eventArgs): void
+    {
+        if ($eventArgs->getConnection()->isTransactionActive()) {
+            return;
+        }
+
+        $pendingMove = $this->pendingMove;
+        foreach ($pendingMove as $file) {
+            $this->manager->moveFile($file);
+        }
+
+        $pendingRemove = $this->pendingRemove;
+        foreach ($pendingRemove as $file) {
+            $this->manager->remove($file);
         }
-        $this->handleEmbeddable($entityManager, $entity, $this->remove);
+
+        $this->clearPending();
+    }
+
+    public function onTransactionRollBack(TransactionRollBackEventArgs $eventArgs): void
+    {
+        if ($eventArgs->getConnection()->isTransactionActive()) {
+            return;
+        }
+
+        $this->clearPending();
     }
 
-    public function onClear(): void
+    private function clearPending(): void
     {
+        $this->pendingMove = [];
+        $this->pendingRemove = [];
+    }
+
+    public function onClear(OnClearEventArgs $eventArgs): void
+    {
+        if ($eventArgs->getEntityManager()->getConnection()->isTransactionActive()) {
+            return;
+        }
         $this->manager->clear();
+        $this->clearPending();
     }
 
     private function supports(object $entity): bool
     {
         return $entity instanceof $this->class;
     }
 
-    private function handleEmbeddable(
-        EntityManagerInterface $entityManager,
-        object $entity,
-        Closure $action
-    ): void {
+    private function handleEmbeddable(EntityManagerInterface $entityManager, object $entity): iterable
+    {
         $classMetadata = $entityManager->getClassMetadata(ClassUtils::getClass($entity));
 
         foreach ($classMetadata->embeddedClasses as $property => $embeddedClass) {
@@ -84,7 +119,7 @@ private function handleEmbeddable(
             if ($file === null) {
                 continue;
             }
-            $action($file);
+            yield $file;
         }
     }
 }

src/Manager.php

@@ -170,11 +170,7 @@ public function moveFile(File $file): void
 
         /** @psalm-suppress RedundantCondition */
         if (is_resource($stream)) {
-            try {
-                ErrorHandler::wrap(static fn (): bool => fclose($stream));
-            } catch (ErrorException $e) {
-                // nothing we can do
-            }
+            fclose($stream);
         }
 
         if ($this->eventDispatcher !== null) {

tests/Functional/ManagerTest.php

@@ -10,6 +10,8 @@
 use SplFileObject;
 use SplTempFileObject;
 
+use function md5;
+
 class ManagerTest extends AbstractFunctionalTest
 {
     protected ManagerInterface $embeddableManager;
@@ -44,6 +46,10 @@ public function testSimpleUpload(): File
         $file = $this->manager->upload(new SplFileObject(__DIR__ . '/../files/image1.jpg'));
 
         $this->entityManager->persist($file);
+
+        self::assertFalse(
+            $this->flysystem->fileExists('9aa1c5fc7c9388166d7ce7fd46648dd1')
+        );
         $this->entityManager->flush();
 
         self::assertTrue(
@@ -191,16 +197,64 @@ public function testSimpleDelete(): void
         self::assertFalse($this->flysystem->fileExists($pathname));
     }
 
+    public function testFileUploadedWithClear(): void
+    {
+        $file = $this->manager->upload(new SplFileObject(__DIR__ . '/../files/image1.jpg'));
+
+        $this->entityManager->beginTransaction();
+
+        $this->entityManager->persist($file);
+
+        self::assertFalse(
+            $this->flysystem->fileExists('9aa1c5fc7c9388166d7ce7fd46648dd1')
+        );
+        $this->entityManager->flush();
+
+        self::assertFalse(
+            $this->flysystem->fileExists('9aa1c5fc7c9388166d7ce7fd46648dd1')
+        );
+
+        $this->entityManager->clear();
+
+        $this->entityManager->commit();
+
+        self::assertTrue(
+            $this->flysystem->fileExists('9aa1c5fc7c9388166d7ce7fd46648dd1')
+        );
+    }
+
+    public function testFileNotUploadedWithRollBack(): void
+    {
+        $file = $this->manager->upload(new SplFileObject(__DIR__ . '/../files/image1.jpg'));
+
+        $this->entityManager->beginTransaction();
+
+        $this->entityManager->persist($file);
+
+        self::assertFalse(
+            $this->flysystem->fileExists('9aa1c5fc7c9388166d7ce7fd46648dd1')
+        );
+        $this->entityManager->flush();
+
+        self::assertFalse(
+            $this->flysystem->fileExists('9aa1c5fc7c9388166d7ce7fd46648dd1')
+        );
+
+        $this->entityManager->rollback();
+
+        self::assertFalse(
+            $this->flysystem->fileExists('9aa1c5fc7c9388166d7ce7fd46648dd1')
+        );
+    }
+
     /**
      * @depends testSimpleUpload
      */
     public function testFileNotDeletedWithRollback(): void
     {
-        self::markTestSkipped('Not implemented yet. Waiting DBAL 3.2.X Release');
-
         $file = $this->testSimpleUpload();
 
-        $filepath = '9aa1c5fc/7c938816/6d7ce7fd/46648dd1/9aa1c5fc7c9388166d7ce7fd46648dd1';
+        $filepath = '9aa1c5fc7c9388166d7ce7fd46648dd1';
         self::assertTrue($this->flysystem->fileExists($filepath));
 
         $this->entityManager->beginTransaction();
@@ -223,11 +277,9 @@ public function testFileNotDeletedWithRollback(): void
      */
     public function testFileDeletedWithCommit(): void
     {
-        self::markTestSkipped('Not implemented yet. Waiting DBAL 3.2.X Release');
-
         $file = $this->testSimpleUpload();
 
-        $filepath = '9aa1c5fc/7c938816/6d7ce7fd/46648dd1/9aa1c5fc7c9388166d7ce7fd46648dd1';
+        $filepath = '9aa1c5fc7c9388166d7ce7fd46648dd1';
 
         self::assertTrue($this->flysystem->fileExists($filepath));