Merge pull request #24 from webgriffe/issue-17

Remove temporary files after consume with a TemporaryFilesManager

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

src/ApiClient.php

@@ -34,20 +34,34 @@ 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, '/');
         $this->username = $username;
         $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();
+    }
 }

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());
+    }
 }

src/EventSubscriber/CommandEventSubscriber.php

@@ -0,0 +1,35 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Webgriffe\SyliusAkeneoPlugin\EventSubscriber;
+
+use Symfony\Component\Console\ConsoleEvents;
+use Symfony\Component\Console\Event\ConsoleTerminateEvent;
+use Symfony\Component\EventDispatcher\EventSubscriberInterface;
+use Webgriffe\SyliusAkeneoPlugin\TemporaryFilesManagerInterface;
+
+final class CommandEventSubscriber implements EventSubscriberInterface
+{
+    /** @var TemporaryFilesManagerInterface */
+    private $temporaryFilesManager;
+
+    public function __construct(TemporaryFilesManagerInterface $temporaryFilesManager)
+    {
+        $this->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();
+        }
+    }
+}

src/Resources/config/services.xml

@@ -1,6 +1,10 @@
 <?xml version="1.0" encoding="UTF-8" ?>
 
 <container xmlns="http://symfony.com/schema/dic/services" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd">
+    <parameters>
+        <parameter key="webgriffe_sylius_akeneo.temporary_files_prefix">akeneo-</parameter>
+    </parameters>
+
     <services>
         <!-- Commands -->
         <service id="webgriffe_sylius_akeneo.command.consume" class="Webgriffe\SyliusAkeneoPlugin\Command\ConsumeCommand">
@@ -26,6 +30,7 @@
             <argument>%webgriffe_sylius_akeneo.api_client.password%</argument>
             <argument>%webgriffe_sylius_akeneo.api_client.client_id%</argument>
             <argument>%webgriffe_sylius_akeneo.api_client.secret%</argument>
+            <argument type="service" id="webgriffe_sylius_akeneo.temporary_file_manager" />
         </service>
         <service id="Webgriffe\SyliusAkeneoPlugin\ApiClientInterface" alias="webgriffe_sylius_akeneo.api_client" />
         <service id="Webgriffe\SyliusAkeneoPlugin\AttributeOptions\ApiClientInterface" alias="webgriffe_sylius_akeneo.api_client" />
@@ -42,6 +47,20 @@
         <service id="webgriffe_sylius_akeneo.slugify" class="Cocur\Slugify\Slugify" />
         <service id="Cocur\Slugify\SlugifyInterface" alias="webgriffe_sylius_akeneo.slugify" />
 
+        <service id="webgriffe_sylius_akeneo.temporary_file_manager" class="Webgriffe\SyliusAkeneoPlugin\TemporaryFilesManager">
+            <argument type="service" id="filesystem" />
+            <argument type="service">
+                <service class="Symfony\Component\Finder\Finder" />
+            </argument>
+            <argument>%webgriffe_sylius_akeneo.temporary_directory%</argument>
+            <argument>%webgriffe_sylius_akeneo.temporary_files_prefix%</argument>
+        </service>
+
+        <service id="webgriffe_sylius_akeneo.event_subscriber.command" class="Webgriffe\SyliusAkeneoPlugin\EventSubscriber\CommandEventSubscriber">
+            <argument type="service" id="webgriffe_sylius_akeneo.temporary_file_manager" />
+            <tag name="kernel.event_subscriber" />
+        </service>
+
         <!-- Product Importer -->
         <service id="webgriffe_sylius_akeneo.product.taxons_resolver" class="Webgriffe\SyliusAkeneoPlugin\Product\AlreadyExistingTaxonsResolver">
             <argument type="service" id="sylius.repository.taxon" />

src/TemporaryFilesManager.php

@@ -0,0 +1,50 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Webgriffe\SyliusAkeneoPlugin;
+
+use Symfony\Component\Filesystem\Filesystem;
+use Symfony\Component\Finder\Finder;
+
+final class TemporaryFilesManager implements TemporaryFilesManagerInterface
+{
+    /** @var Filesystem */
+    private $filesystem;
+
+    /** @var Finder */
+    private $finder;
+
+    /** @var string */
+    private $temporaryDirectory;
+
+    /** @var string */
+    private $temporaryFilesPrefix;
+
+    public function __construct(
+        Filesystem $filesystem,
+        Finder $finder,
+        string $temporaryDirectory,
+        string $temporaryFilesPrefix
+    ) {
+        $this->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());
+        }
+    }
+}

src/TemporaryFilesManagerInterface.php

@@ -0,0 +1,12 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Webgriffe\SyliusAkeneoPlugin;
+
+interface TemporaryFilesManagerInterface
+{
+    public function generateTemporaryFilePath(): string;
+
+    public function deleteAllTemporaryFiles(): void;
+}

tests/Application/config/services_test.yaml

@@ -9,6 +9,8 @@ services:
 
     webgriffe_sylius_akeneo.api_client:
         class: 'Tests\Webgriffe\SyliusAkeneoPlugin\Integration\TestDouble\ApiClientMock'
+        arguments:
+            - '@webgriffe_sylius_akeneo.temporary_file_manager'
     webgriffe_sylius_akeneo.date_time_builder:
         class: 'Tests\Webgriffe\SyliusAkeneoPlugin\Integration\TestDouble\DateTimeBuilder'
 

tests/Behat/Context/Cli/ConsumeCommandContext.php

@@ -6,7 +6,7 @@
 
 use Behat\Behat\Context\Context;
 use Symfony\Bundle\FrameworkBundle\Console\Application;
-use Symfony\Component\Console\Tester\CommandTester;
+use Symfony\Component\Console\Tester\ApplicationTester;
 use Symfony\Component\HttpKernel\KernelInterface;
 use Webgriffe\SyliusAkeneoPlugin\Command\ConsumeCommand;
 
@@ -32,9 +32,9 @@ public function __construct(
     public function iImportAllItemsInQueue()
     {
         $application = new Application($this->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']);
     }
 }

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 . '*'));
+    }
 }

tests/Behat/Resources/services.xml

@@ -35,7 +35,11 @@
             <argument type="service" id="webgriffe_sylius_akeneo_plugin.repository.queue_item" />
         </service>
 
-        <service id="webgriffe_sylius_akeneo.context.system.filesystem" class="Tests\Webgriffe\SyliusAkeneoPlugin\Behat\Context\System\FilesystemContext" />
+        <service id="webgriffe_sylius_akeneo.context.system.filesystem" class="Tests\Webgriffe\SyliusAkeneoPlugin\Behat\Context\System\FilesystemContext">
+            <argument>%webgriffe_sylius_akeneo.temporary_directory%</argument>
+            <argument>%webgriffe_sylius_akeneo.temporary_files_prefix%</argument>
+        </service>
+
         <service id="webgriffe_sylius_akeneo.context.system.datetime" class="Tests\Webgriffe\SyliusAkeneoPlugin\Behat\Context\System\DateTimeContext" />
     </services>
 </container>

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"

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());