diff --git a/features/importing_products_from_queue.feature b/features/importing_products_from_queue.feature index 932d0168..d5c319d4 100644 --- a/features/importing_products_from_queue.feature +++ b/features/importing_products_from_queue.feature @@ -36,3 +36,12 @@ Feature: Importing products from queue And the product variant "braided-hat-m" of product "model-braided-hat" should exists with the right data And the queue item with identifier "braided-hat-m" for the "Product" importer has been marked as imported And the queue item with identifier "NOT_EXISTS" for the "Product" importer has not been marked as imported + + @cli + Scenario: Importing products with images should not leave temporary files in temporary files directory + Given the store operates on a single channel + And the store is also available in "it_IT" + And there is one item to import with identifier "braided-hat-m" for the "Product" importer in the Akeneo queue + And there is one item to import with identifier "Braided-hat-l" for the "Product" importer in the Akeneo queue + When I import all items in queue + Then there should not be any temporary file in the temporary files directory diff --git a/src/ApiClient.php b/src/ApiClient.php index 8ca6a19c..f1f3896c 100644 --- a/src/ApiClient.php +++ b/src/ApiClient.php @@ -34,13 +34,17 @@ final class ApiClient implements ApiClientInterface, AttributeOptionsApiClientIn /** @var string */ private $secret; + /** @var TemporaryFilesManagerInterface|null */ + private $temporaryFilesManager; + public function __construct( ClientInterface $httpClient, string $baseUrl, string $username, string $password, string $clientId, - string $secret + string $secret, + TemporaryFilesManagerInterface $temporaryFilesManager = null ) { $this->httpClient = $httpClient; $this->baseUrl = rtrim($baseUrl, '/'); @@ -48,6 +52,16 @@ public function __construct( $this->password = $password; $this->clientId = $clientId; $this->secret = $secret; + if (null === $temporaryFilesManager) { + trigger_deprecation( + 'webgriffe/sylius-akeneo-plugin', + '1.3', + 'Not passing a temporary files manager to %s is deprecated and will not be possible anymore in %s', + __CLASS__, + '2.0' + ); + } + $this->temporaryFilesManager = $temporaryFilesManager; } /** @@ -128,8 +142,7 @@ public function downloadFile(string $code): \SplFileInfo throw new \HttpException($responseResult['message'], $responseResult['code']); } - $tempName = tempnam(sys_get_temp_dir(), 'akeneo-'); - Assert::string($tempName); + $tempName = $this->generateTempFilePath(); file_put_contents($tempName, $bodyContents); return new File($tempName); @@ -243,4 +256,16 @@ private function traversePagination(array $responseResult): array return $items; } + + private function generateTempFilePath(): string + { + if (null === $this->temporaryFilesManager) { + $tempName = tempnam(sys_get_temp_dir(), 'akeneo-'); + Assert::string($tempName); + + return $tempName; + } + + return $this->temporaryFilesManager->generateTemporaryFilePath(); + } } diff --git a/src/DependencyInjection/WebgriffeSyliusAkeneoExtension.php b/src/DependencyInjection/WebgriffeSyliusAkeneoExtension.php index 2fb6d0e8..af636a88 100644 --- a/src/DependencyInjection/WebgriffeSyliusAkeneoExtension.php +++ b/src/DependencyInjection/WebgriffeSyliusAkeneoExtension.php @@ -117,6 +117,7 @@ public function process(ContainerBuilder $container): void { $this->addTaggedValueHandlersToResolver($container); $this->addTaggedImportersToRegistry($container); + $this->registerTemporaryDirectoryParameter($container); } private function createValueHandlersDefinitionsAndPriorities(array $valueHandlers): array @@ -185,4 +186,13 @@ private function addTaggedImportersToRegistry(ContainerBuilder $container): void $importerRegistryDefinition->addMethodCall('add', [new Reference($id)]); } } + + private function registerTemporaryDirectoryParameter(ContainerBuilder $container): void + { + $parameterKey = 'webgriffe_sylius_akeneo.temporary_directory'; + if ($container->hasParameter($parameterKey)) { + return; + } + $container->setParameter($parameterKey, sys_get_temp_dir()); + } } diff --git a/src/EventSubscriber/CommandEventSubscriber.php b/src/EventSubscriber/CommandEventSubscriber.php new file mode 100644 index 00000000..9305fca6 --- /dev/null +++ b/src/EventSubscriber/CommandEventSubscriber.php @@ -0,0 +1,35 @@ +temporaryFilesManager = $temporaryFilesManager; + } + + public static function getSubscribedEvents(): array + { + return [ConsoleEvents::TERMINATE => ['onTerminateCommand']]; + } + + public function onTerminateCommand(ConsoleTerminateEvent $event): void + { + $command = $event->getCommand(); + + if ($command !== null && $command->getName() === 'webgriffe:akeneo:consume') { + $this->temporaryFilesManager->deleteAllTemporaryFiles(); + } + } +} diff --git a/src/Resources/config/services.xml b/src/Resources/config/services.xml index 70e669ea..300cea54 100644 --- a/src/Resources/config/services.xml +++ b/src/Resources/config/services.xml @@ -1,6 +1,10 @@ + + akeneo- + + @@ -26,6 +30,7 @@ %webgriffe_sylius_akeneo.api_client.password% %webgriffe_sylius_akeneo.api_client.client_id% %webgriffe_sylius_akeneo.api_client.secret% + @@ -42,6 +47,20 @@ + + + + + + %webgriffe_sylius_akeneo.temporary_directory% + %webgriffe_sylius_akeneo.temporary_files_prefix% + + + + + + + diff --git a/src/TemporaryFilesManager.php b/src/TemporaryFilesManager.php new file mode 100644 index 00000000..34216f7c --- /dev/null +++ b/src/TemporaryFilesManager.php @@ -0,0 +1,50 @@ +filesystem = $filesystem; + $this->finder = $finder; + $this->temporaryDirectory = $temporaryDirectory; + $this->temporaryFilesPrefix = $temporaryFilesPrefix; + } + + public function generateTemporaryFilePath(): string + { + return $this->filesystem->tempnam($this->temporaryDirectory, $this->temporaryFilesPrefix); + } + + public function deleteAllTemporaryFiles(): void + { + $tempFiles = $this->finder->in($this->temporaryDirectory)->depth('== 0')->files()->name( + $this->temporaryFilesPrefix . '*' + ); + foreach ($tempFiles as $tempFile) { + $this->filesystem->remove($tempFile->getPathname()); + } + } +} diff --git a/src/TemporaryFilesManagerInterface.php b/src/TemporaryFilesManagerInterface.php new file mode 100644 index 00000000..2b7ad08c --- /dev/null +++ b/src/TemporaryFilesManagerInterface.php @@ -0,0 +1,12 @@ +kernel); + $application->setAutoExit(false); $application->add($this->consumeCommand); - $command = $application->find('webgriffe:akeneo:consume'); - $commandTester = new CommandTester($command); - $commandTester->execute(['command' => 'webgriffe:akeneo:consume']); + $applicationTester = new ApplicationTester($application); + $applicationTester->run(['command' => 'webgriffe:akeneo:consume']); } } diff --git a/tests/Behat/Context/System/FilesystemContext.php b/tests/Behat/Context/System/FilesystemContext.php index 915388dc..c98a9b31 100644 --- a/tests/Behat/Context/System/FilesystemContext.php +++ b/tests/Behat/Context/System/FilesystemContext.php @@ -14,6 +14,18 @@ final class FilesystemContext implements Context /** @var vfsStreamContainer */ private $vfsStream; + /** @var string */ + private $temporaryDirectory; + + /** @var string */ + private $temporaryFilesPrefix; + + public function __construct(string $temporaryDirectory, string $temporaryFilesPrefix) + { + $this->temporaryDirectory = $temporaryDirectory; + $this->temporaryFilesPrefix = $temporaryFilesPrefix; + } + /** * @BeforeScenario */ @@ -39,4 +51,12 @@ public function thereIsAFileWithNameThatContains($filename, $content) $actualFileContent = file_get_contents($file); Assert::same($actualFileContent, $content); } + + /** + * @Then /^there should not be any temporary file in the temporary files directory$/ + */ + public function thereShouldNotBeAnyTemporaryFileInTheTemporaryFilesDirectory() + { + Assert::isEmpty(glob(rtrim($this->temporaryDirectory, '/') . '/' . $this->temporaryFilesPrefix . '*')); + } } diff --git a/tests/Behat/Resources/services.xml b/tests/Behat/Resources/services.xml index 0653d1aa..3c30aab4 100644 --- a/tests/Behat/Resources/services.xml +++ b/tests/Behat/Resources/services.xml @@ -35,7 +35,11 @@ - + + %webgriffe_sylius_akeneo.temporary_directory% + %webgriffe_sylius_akeneo.temporary_files_prefix% + + diff --git a/tests/Behat/Resources/suites.yml b/tests/Behat/Resources/suites.yml index 8202d2b7..8386a530 100644 --- a/tests/Behat/Resources/suites.yml +++ b/tests/Behat/Resources/suites.yml @@ -13,6 +13,7 @@ default: - webgriffe_sylius_akeneo.context.db.product - webgriffe_sylius_akeneo.context.db.queue + - webgriffe_sylius_akeneo.context.system.filesystem filters: tags: "@importing_products && @cli" diff --git a/tests/Integration/DependencyInjection/CompilerPassTest.php b/tests/Integration/DependencyInjection/CompilerPassTest.php index d1c7344d..d8d12951 100644 --- a/tests/Integration/DependencyInjection/CompilerPassTest.php +++ b/tests/Integration/DependencyInjection/CompilerPassTest.php @@ -65,6 +65,28 @@ public function adds_tagged_importers_to_registry(): void ); } + /** + * @test + */ + public function registers_temporary_directory_parameter(): void + { + $this->compile(); + + $this->assertContainerBuilderHasParameter('webgriffe_sylius_akeneo.temporary_directory', sys_get_temp_dir()); + } + + /** + * @test + */ + public function does_not_register_temporary_directory_parameter_if_it_is_already_defined(): void + { + $this->container->setParameter('webgriffe_sylius_akeneo.temporary_directory', '/tmp/my-custom-path'); + + $this->compile(); + + $this->assertContainerBuilderHasParameter('webgriffe_sylius_akeneo.temporary_directory', '/tmp/my-custom-path'); + } + protected function registerCompilerPass(ContainerBuilder $container): void { $container->addCompilerPass(new WebgriffeSyliusAkeneoExtension()); diff --git a/tests/Integration/TemporaryFilesManagerTest.php b/tests/Integration/TemporaryFilesManagerTest.php new file mode 100644 index 00000000..ddf8e8ea --- /dev/null +++ b/tests/Integration/TemporaryFilesManagerTest.php @@ -0,0 +1,66 @@ +temporaryFileManager = new TemporaryFilesManager( + new Filesystem(), + new Finder(), + vfsStream::url('root'), + 'akeneo-' + ); + } + + /** + * @test + */ + public function it_generates_temporary_file_path() + { + $this->assertRegExp( + '|' . vfsStream::url('root') . '/akeneo-.*|', + $this->temporaryFileManager->generateTemporaryFilePath() + ); + } + + /** + * @test + */ + public function it_deletes_all_temporary_files() + { + touch(vfsStream::url('root') . '/akeneo-temp1'); + touch(vfsStream::url('root') . '/akeneo-temp2'); + touch(vfsStream::url('root') . '/akeneo-temp3'); + + $this->temporaryFileManager->deleteAllTemporaryFiles(); + + $this->assertFileNotExists(vfsStream::url('root') . '/akeneo-temp1'); + $this->assertFileNotExists(vfsStream::url('root') . '/akeneo-temp2'); + $this->assertFileNotExists(vfsStream::url('root') . '/akeneo-temp3'); + } + + /** + * @test + */ + public function it_does_not_delete_not_managed_temporary_files() + { + touch(vfsStream::url('root') . '/not-managed-temp-file'); + + $this->temporaryFileManager->deleteAllTemporaryFiles(); + + $this->assertFileExists(vfsStream::url('root') . '/not-managed-temp-file'); + } +} diff --git a/tests/Integration/TestDouble/ApiClientMock.php b/tests/Integration/TestDouble/ApiClientMock.php index 8e769bf8..ddd7b7d0 100644 --- a/tests/Integration/TestDouble/ApiClientMock.php +++ b/tests/Integration/TestDouble/ApiClientMock.php @@ -7,11 +7,20 @@ use Symfony\Component\HttpFoundation\File\File; use Webgriffe\SyliusAkeneoPlugin\ApiClientInterface; use Webgriffe\SyliusAkeneoPlugin\AttributeOptions\ApiClientInterface as AttributeOptionsApiClientInterface; +use Webgriffe\SyliusAkeneoPlugin\TemporaryFilesManagerInterface; final class ApiClientMock implements ApiClientInterface, AttributeOptionsApiClientInterface { private $productsUpdatedAt = []; + /** @var TemporaryFilesManagerInterface */ + private $temporaryFilesManager; + + public function __construct(TemporaryFilesManagerInterface $temporaryFilesManager) + { + $this->temporaryFilesManager = $temporaryFilesManager; + } + public function findProductModel(string $code): ?array { return $this->jsonDecodeOrNull(__DIR__ . '/../DataFixtures/ApiClientMock/ProductModel/' . $code . '.json'); @@ -65,7 +74,7 @@ public function downloadFile(string $code): \SplFileInfo if (!file_exists($path)) { throw new \RuntimeException("File '$path' does not exists."); } - $tempName = tempnam(sys_get_temp_dir(), 'akeneo-'); + $tempName = $this->temporaryFilesManager->generateTemporaryFilePath(); file_put_contents($tempName, file_get_contents($path)); return new File($tempName);