diff --git a/.idea/php.xml b/.idea/php.xml index 803509b..4284e20 100644 --- a/.idea/php.xml +++ b/.idea/php.xml @@ -51,7 +51,6 @@ - @@ -122,6 +121,9 @@ + + + @@ -135,4 +137,4 @@ - \ No newline at end of file + diff --git a/README.md b/README.md index bad03a1..9bde985 100644 --- a/README.md +++ b/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,9 +153,9 @@ services: Arxy\FilesBundle\Storage\FlysystemStorage: ~ - Arxy\FilesBundle\Storage: + Arxy\FilesBundle\Storage: alias: 'Arxy\FilesBundle\Storage\FlysystemStorage' - + Arxy\FilesBundle\Manager: $class: 'App\Entity\File' @@ -163,15 +163,15 @@ services: 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. diff --git a/composer.json b/composer.json index 8f480f6..6dfcc33 100644 --- a/composer.json +++ b/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", diff --git a/src/DependencyInjection/ArxyFilesExtension.php b/src/DependencyInjection/ArxyFilesExtension.php index b36561c..a3e2b2e 100644 --- a/src/DependencyInjection/ArxyFilesExtension.php +++ b/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: diff --git a/src/EventListener/DoctrineORMListener.php b/src/EventListener/DoctrineORMListener.php index c6766e4..588b564 100644 --- a/src/EventListener/DoctrineORMListener.php +++ b/src/EventListener/DoctrineORMListener.php @@ -6,8 +6,9 @@ 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 ReflectionObject; @@ -15,21 +16,15 @@ final class DoctrineORMListener { private ManagerInterface $manager; + /** @var class-string */ 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 @@ -37,9 +32,11 @@ 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,14 +45,51 @@ 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; } - $this->handleEmbeddable($entityManager, $entity, $this->remove); + } + + 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->clearPending(); + } + + public function onTransactionRollBack(TransactionRollBackEventArgs $eventArgs): void + { + if ($eventArgs->getConnection()->isTransactionActive()) { + return; + } + + $this->clearPending(); + } + + private function clearPending(): void + { + $this->pendingMove = []; + $this->pendingRemove = []; } public function onClear(): void { $this->manager->clear(); + $this->clearPending(); } private function supports(object $entity): bool @@ -63,11 +97,8 @@ 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 +115,7 @@ private function handleEmbeddable( if ($file === null) { continue; } - $action($file); + yield $file; } } } diff --git a/src/Manager.php b/src/Manager.php index 3e39a37..ee2c132 100644 --- a/src/Manager.php +++ b/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) { diff --git a/tests/Functional/ManagerTest.php b/tests/Functional/ManagerTest.php index 92ec52b..3795308 100644 --- a/tests/Functional/ManagerTest.php +++ b/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,38 @@ public function testSimpleDelete(): void self::assertFalse($this->flysystem->fileExists($pathname)); } + 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->commit(); + + self::assertTrue( + $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 +251,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));