6 files changed, 165 insertions, 108 deletions

diff --git a/src/Wallabag/CoreBundle/Controller/EntryController.php b/src/Wallabag/CoreBundle/Controller/EntryController.php
index 97bb3d12..3f4eb17d 100644
--- a/src/Wallabag/CoreBundle/Controller/EntryController.php
+++ b/src/Wallabag/CoreBundle/Controller/EntryController.php
@@ -13,6 +13,8 @@ use Wallabag\CoreBundle\Form\Type\EntryFilterType;
 use Wallabag\CoreBundle\Form\Type\EditEntryType;
 use Wallabag\CoreBundle\Form\Type\NewEntryType;
 use Sensio\Bundle\FrameworkExtraBundle\Configuration\Cache;
+use Wallabag\CoreBundle\Event\EntrySavedEvent;
+use Wallabag\CoreBundle\Event\EntryDeletedEvent;
 
 class EntryController extends Controller
 {
@@ -81,6 +83,9 @@ class EntryController extends Controller
             $em->persist($entry);
             $em->flush();
 
+            // entry saved, dispatch event about it!
+            $this->get('event_dispatcher')->dispatch(EntrySavedEvent::NAME, new EntrySavedEvent($entry));
+
             return $this->redirect($this->generateUrl('homepage'));
         }
 
@@ -107,6 +112,9 @@ class EntryController extends Controller
             $em = $this->getDoctrine()->getManager();
             $em->persist($entry);
             $em->flush();
+
+            // entry saved, dispatch event about it!
+            $this->get('event_dispatcher')->dispatch(EntrySavedEvent::NAME, new EntrySavedEvent($entry));
         }
 
         return $this->redirect($this->generateUrl('homepage'));
@@ -343,6 +351,9 @@ class EntryController extends Controller
         $em->persist($entry);
         $em->flush();
 
+        // entry saved, dispatch event about it!
+        $this->get('event_dispatcher')->dispatch(EntrySavedEvent::NAME, new EntrySavedEvent($entry));
+
         return $this->redirect($this->generateUrl('view', ['id' => $entry->getId()]));
     }
 
@@ -431,6 +442,9 @@ class EntryController extends Controller
             UrlGeneratorInterface::ABSOLUTE_PATH
         );
 
+        // entry deleted, dispatch event about it!
+        $this->get('event_dispatcher')->dispatch(EntryDeletedEvent::NAME, new EntryDeletedEvent($entry));
+
         $em = $this->getDoctrine()->getManager();
         $em->remove($entry);
         $em->flush();
diff --git a/src/Wallabag/CoreBundle/Event/EntryDeletedEvent.php b/src/Wallabag/CoreBundle/Event/EntryDeletedEvent.php
new file mode 100644
index 00000000..e9061d04
--- /dev/null
+++ b/src/Wallabag/CoreBundle/Event/EntryDeletedEvent.php
@@ -0,0 +1,26 @@
+<?php
+
+namespace Wallabag\CoreBundle\Event;
+
+use Symfony\Component\EventDispatcher\Event;
+use Wallabag\CoreBundle\Entity\Entry;
+
+/**
+ * This event is fired as soon as an entry is deleted.
+ */
+class EntryDeletedEvent extends Event
+{
+    const NAME = 'entry.deleted';
+
+    protected $entry;
+
+    public function __construct(Entry $entry)
+    {
+        $this->entry = $entry;
+    }
+
+    public function getEntry()
+    {
+        return $this->entry;
+    }
+}
diff --git a/src/Wallabag/CoreBundle/Event/EntrySavedEvent.php b/src/Wallabag/CoreBundle/Event/EntrySavedEvent.php
new file mode 100644
index 00000000..5fdb5221
--- /dev/null
+++ b/src/Wallabag/CoreBundle/Event/EntrySavedEvent.php
@@ -0,0 +1,26 @@
+<?php
+
+namespace Wallabag\CoreBundle\Event;
+
+use Symfony\Component\EventDispatcher\Event;
+use Wallabag\CoreBundle\Entity\Entry;
+
+/**
+ * This event is fired as soon as an entry was saved.
+ */
+class EntrySavedEvent extends Event
+{
+    const NAME = 'entry.saved';
+
+    protected $entry;
+
+    public function __construct(Entry $entry)
+    {
+        $this->entry = $entry;
+    }
+
+    public function getEntry()
+    {
+        return $this->entry;
+    }
+}
diff --git a/src/Wallabag/CoreBundle/Event/Subscriber/DownloadImagesSubscriber.php b/src/Wallabag/CoreBundle/Event/Subscriber/DownloadImagesSubscriber.php
index 6fddcea9..4ebe837b 100644
--- a/src/Wallabag/CoreBundle/Event/Subscriber/DownloadImagesSubscriber.php
+++ b/src/Wallabag/CoreBundle/Event/Subscriber/DownloadImagesSubscriber.php
@@ -2,110 +2,85 @@
 
 namespace Wallabag\CoreBundle\Event\Subscriber;
 
-use Doctrine\Common\EventSubscriber;
-use Doctrine\ORM\Event\LifecycleEventArgs;
+use Symfony\Component\EventDispatcher\EventSubscriberInterface;
 use Psr\Log\LoggerInterface;
 use Wallabag\CoreBundle\Helper\DownloadImages;
 use Wallabag\CoreBundle\Entity\Entry;
+use Wallabag\CoreBundle\Event\EntrySavedEvent;
+use Wallabag\CoreBundle\Event\EntryDeletedEvent;
 use Doctrine\ORM\EntityManager;
-use Craue\ConfigBundle\Util\Config;
 
-class DownloadImagesSubscriber implements EventSubscriber
+class DownloadImagesSubscriber implements EventSubscriberInterface
 {
-    private $configClass;
+    private $em;
     private $downloadImages;
+    private $enabled;
     private $logger;
 
-    /**
-     * We inject the class instead of the service otherwise it generates a circular reference with the EntityManager.
-     * So we build the service ourself when we got the EntityManager (in downloadImages).
-     */
-    public function __construct(DownloadImages $downloadImages, $configClass, LoggerInterface $logger)
+    public function __construct(EntityManager $em, DownloadImages $downloadImages, $enabled, LoggerInterface $logger)
     {
+        $this->em = $em;
         $this->downloadImages = $downloadImages;
-        $this->configClass = $configClass;
+        $this->enabled = $enabled;
         $this->logger = $logger;
     }
 
-    public function getSubscribedEvents()
+    public static function getSubscribedEvents()
     {
-        return array(
-            'prePersist',
-            'preUpdate',
-        );
+        return [
+            EntrySavedEvent::NAME => 'onEntrySaved',
+            EntryDeletedEvent::NAME => 'onEntryDeleted',
+        ];
     }
 
     /**
-     * In case of an entry has been updated.
-     * We won't update the content field if it wasn't updated.
+     * Download images and updated the data into the entry.
      *
-     * @param LifecycleEventArgs $args
+     * @param EntrySavedEvent $event
      */
-    public function preUpdate(LifecycleEventArgs $args)
+    public function onEntrySaved(EntrySavedEvent $event)
     {
-        $entity = $args->getEntity();
+        if (!$this->enabled) {
+            $this->logger->debug('DownloadImagesSubscriber: disabled.');
 
-        if (!$entity instanceof Entry) {
             return;
         }
 
-        $config = new $this->configClass();
-        $config->setEntityManager($args->getEntityManager());
-
-        if (!$config->get('download_images_enabled')) {
-            return;
-        }
+        $entry = $event->getEntry();
 
-        // field content has been updated
-        if ($args->hasChangedField('content')) {
-            $html = $this->downloadImages($config, $entity);
+        $html = $this->downloadImages($entry);
+        if (false !== $html) {
+            $this->logger->debug('DownloadImagesSubscriber: updated html.');
 
-            if (false !== $html) {
-                $args->setNewValue('content', $html);
-            }
+            $entry->setContent($html);
         }
 
-        // field preview picture has been updated
-        if ($args->hasChangedField('previewPicture')) {
-            $previewPicture = $this->downloadPreviewImage($config, $entity);
+        // update preview picture
+        $previewPicture = $this->downloadPreviewImage($entry);
+        if (false !== $previewPicture) {
+            $this->logger->debug('DownloadImagesSubscriber: update preview picture.');
 
-            if (false !== $previewPicture) {
-                $entity->setPreviewPicture($previewPicture);
-            }
+            $entry->setPreviewPicture($previewPicture);
         }
+
+        $this->em->persist($entry);
+        $this->em->flush();
     }
 
     /**
-     * When a new entry is saved.
+     * Remove images related to the entry.
      *
-     * @param LifecycleEventArgs $args
+     * @param EntryDeletedEvent $event
      */
-    public function prePersist(LifecycleEventArgs $args)
+    public function onEntryDeleted(EntryDeletedEvent $event)
     {
-        $entity = $args->getEntity();
-
-        if (!$entity instanceof Entry) {
-            return;
-        }
-
-        $config = new $this->configClass();
-        $config->setEntityManager($args->getEntityManager());
+        if (!$this->enabled) {
+            $this->logger->debug('DownloadImagesSubscriber: disabled.');
 
-        if (!$config->get('download_images_enabled')) {
             return;
         }
 
-        // update all images inside the html
-        $html = $this->downloadImages($config, $entity);
-        if (false !== $html) {
-            $entity->setContent($html);
-        }
-
-        // update preview picture
-        $previewPicture = $this->downloadPreviewImage($config, $entity);
-        if (false !== $previewPicture) {
-            $entity->setPreviewPicture($previewPicture);
-        }
+        $this->downloadImages->removeImages($event->getEntry()->getId());
     }
 
     /**
@@ -113,16 +88,14 @@ class DownloadImagesSubscriber implements EventSubscriber
      *
      * @todo If we want to add async download, it should be done in that method
      *
-     * @param Config $config
-     * @param Entry  $entry
+     * @param Entry $entry
      *
      * @return string|false False in case of async
      */
-    public function downloadImages(Config $config, Entry $entry)
+    private function downloadImages(Entry $entry)
     {
-        $this->downloadImages->setWallabagUrl($config->get('wallabag_url'));
-
         return $this->downloadImages->processHtml(
+            $entry->getId(),
             $entry->getContent(),
             $entry->getUrl()
         );
@@ -133,16 +106,14 @@ class DownloadImagesSubscriber implements EventSubscriber
      *
      * @todo If we want to add async download, it should be done in that method
      *
-     * @param Config $config
-     * @param Entry  $entry
+     * @param Entry $entry
      *
      * @return string|false False in case of async
      */
-    public function downloadPreviewImage(Config $config, Entry $entry)
+    private function downloadPreviewImage(Entry $entry)
     {
-        $this->downloadImages->setWallabagUrl($config->get('wallabag_url'));
-
         return $this->downloadImages->processSingleImage(
+            $entry->getId(),
             $entry->getPreviewPicture(),
             $entry->getUrl()
         );
diff --git a/src/Wallabag/CoreBundle/Helper/DownloadImages.php b/src/Wallabag/CoreBundle/Helper/DownloadImages.php
index e7982c56..c5298236 100644
--- a/src/Wallabag/CoreBundle/Helper/DownloadImages.php
+++ b/src/Wallabag/CoreBundle/Helper/DownloadImages.php
@@ -6,6 +6,7 @@ use Psr\Log\LoggerInterface;
 use Symfony\Component\DomCrawler\Crawler;
 use GuzzleHttp\Client;
 use Symfony\Component\HttpFoundation\File\MimeType\MimeTypeExtensionGuesser;
+use Symfony\Component\Finder\Finder;
 
 class DownloadImages
 {
@@ -17,10 +18,11 @@ class DownloadImages
     private $mimeGuesser;
     private $wallabagUrl;
 
-    public function __construct(Client $client, $baseFolder, LoggerInterface $logger)
+    public function __construct(Client $client, $baseFolder, $wallabagUrl, LoggerInterface $logger)
     {
         $this->client = $client;
         $this->baseFolder = $baseFolder;
+        $this->wallabagUrl = rtrim($wallabagUrl, '/');
         $this->logger = $logger;
         $this->mimeGuesser = new MimeTypeExtensionGuesser();
 
@@ -28,17 +30,6 @@ class DownloadImages
     }
 
     /**
-     * Since we can't inject CraueConfig service because it'll generate a circular reference when injected in the subscriber
-     * we use a different way to inject the current wallabag url.
-     *
-     * @param string $url Usually from `$config->get('wallabag_url')`
-     */
-    public function setWallabagUrl($url)
-    {
-        $this->wallabagUrl = rtrim($url, '/');
-    }
-
-    /**
      * Setup base folder where all images are going to be saved.
      */
     private function setFolder()
@@ -52,23 +43,24 @@ class DownloadImages
     /**
      * Process the html and extract image from it, save them to local and return the updated html.
      *
+     * @param int    $entryId ID of the entry
      * @param string $html
-     * @param string $url  Used as a base path for relative image and folder
+     * @param string $url     Used as a base path for relative image and folder
      *
      * @return string
      */
-    public function processHtml($html, $url)
+    public function processHtml($entryId, $html, $url)
     {
         $crawler = new Crawler($html);
         $result = $crawler
             ->filterXpath('//img')
             ->extract(array('src'));
 
-        $relativePath = $this->getRelativePath($url);
+        $relativePath = $this->getRelativePath($entryId);
 
         // download and save the image to the folder
         foreach ($result as $image) {
-            $imagePath = $this->processSingleImage($image, $url, $relativePath);
+            $imagePath = $this->processSingleImage($entryId, $image, $url, $relativePath);
 
             if (false === $imagePath) {
                 continue;
@@ -86,24 +78,27 @@ class DownloadImages
      *     - re-saved it (for security reason)
      *     - return the new local path.
      *
+     * @param int    $entryId      ID of the entry
      * @param string $imagePath    Path to the image to retrieve
      * @param string $url          Url from where the image were found
      * @param string $relativePath Relative local path to saved the image
      *
      * @return string Relative url to access the image from the web
      */
-    public function processSingleImage($imagePath, $url, $relativePath = null)
+    public function processSingleImage($entryId, $imagePath, $url, $relativePath = null)
     {
-        if (null == $relativePath) {
-            $relativePath = $this->getRelativePath($url);
+        if (null === $relativePath) {
+            $relativePath = $this->getRelativePath($entryId);
         }
 
+        $this->logger->debug('DownloadImages: working on image: '.$imagePath);
+
         $folderPath = $this->baseFolder.'/'.$relativePath;
 
         // build image path
         $absolutePath = $this->getAbsoluteLink($url, $imagePath);
         if (false === $absolutePath) {
-            $this->logger->log('error', 'Can not determine the absolute path for that image, skipping.');
+            $this->logger->error('DownloadImages: Can not determine the absolute path for that image, skipping.');
 
             return false;
         }
@@ -111,15 +106,15 @@ class DownloadImages
         try {
             $res = $this->client->get($absolutePath);
         } catch (\Exception $e) {
-            $this->logger->log('error', 'Can not retrieve image, skipping.', ['exception' => $e]);
+            $this->logger->error('DownloadImages: Can not retrieve image, skipping.', ['exception' => $e]);
 
             return false;
         }
 
         $ext = $this->mimeGuesser->guess($res->getHeader('content-type'));
-        $this->logger->log('debug', 'Checking extension', ['ext' => $ext, 'header' => $res->getHeader('content-type')]);
+        $this->logger->debug('DownloadImages: Checking extension', ['ext' => $ext, 'header' => $res->getHeader('content-type')]);
         if (!in_array($ext, ['jpeg', 'jpg', 'gif', 'png'], true)) {
-            $this->logger->log('error', 'Processed image with not allowed extension. Skipping '.$imagePath);
+            $this->logger->error('DownloadImages: Processed image with not allowed extension. Skipping '.$imagePath);
 
             return false;
         }
@@ -133,7 +128,7 @@ class DownloadImages
         }
 
         if (false === $im) {
-            $this->logger->log('error', 'Error while regenerating image', ['path' => $localPath]);
+            $this->logger->error('DownloadImages: Error while regenerating image', ['path' => $localPath]);
 
             return false;
         }
@@ -141,16 +136,16 @@ class DownloadImages
         switch ($ext) {
             case 'gif':
                 $result = imagegif($im, $localPath);
-                $this->logger->log('debug', 'Re-creating gif');
+                $this->logger->debug('DownloadImages: Re-creating gif');
                 break;
             case 'jpeg':
             case 'jpg':
                 $result = imagejpeg($im, $localPath, self::REGENERATE_PICTURES_QUALITY);
-                $this->logger->log('debug', 'Re-creating jpg');
+                $this->logger->debug('DownloadImages: Re-creating jpg');
                 break;
             case 'png':
                 $result = imagepng($im, $localPath, ceil(self::REGENERATE_PICTURES_QUALITY / 100 * 9));
-                $this->logger->log('debug', 'Re-creating png');
+                $this->logger->debug('DownloadImages: Re-creating png');
         }
 
         imagedestroy($im);
@@ -159,23 +154,46 @@ class DownloadImages
     }
 
     /**
+     * Remove all images for the given entry id.
+     *
+     * @param int $entryId ID of the entry
+     */
+    public function removeImages($entryId)
+    {
+        $relativePath = $this->getRelativePath($entryId);
+        $folderPath = $this->baseFolder.'/'.$relativePath;
+
+        $finder = new Finder();
+        $finder
+            ->files()
+            ->ignoreDotFiles(true)
+            ->in($folderPath);
+
+        foreach ($finder as $file) {
+            @unlink($file->getRealPath());
+        }
+
+        @rmdir($folderPath);
+    }
+
+    /**
      * Generate the folder where we are going to save images based on the entry url.
      *
-     * @param string $url
+     * @param int $entryId ID of the entry
      *
      * @return string
      */
-    private function getRelativePath($url)
+    private function getRelativePath($entryId)
     {
-        $hashUrl = hash('crc32', $url);
-        $relativePath = $hashUrl[0].'/'.$hashUrl[1].'/'.$hashUrl;
+        $hashId = hash('crc32', $entryId);
+        $relativePath = $hashId[0].'/'.$hashId[1].'/'.$hashId;
         $folderPath = $this->baseFolder.'/'.$relativePath;
 
         if (!file_exists($folderPath)) {
             mkdir($folderPath, 0777, true);
         }
 
-        $this->logger->log('debug', 'Folder used for that url', ['folder' => $folderPath, 'url' => $url]);
+        $this->logger->debug('DownloadImages: Folder used for that Entry id', ['folder' => $folderPath, 'entryId' => $entryId]);
 
         return $relativePath;
     }
@@ -208,7 +226,7 @@ class DownloadImages
             return $absolute->get_uri();
         }
 
-        $this->logger->log('error', 'Can not make an absolute link', ['base' => $base, 'url' => $url]);
+        $this->logger->error('DownloadImages: Can not make an absolute link', ['base' => $base, 'url' => $url]);
 
         return false;
     }
diff --git a/src/Wallabag/CoreBundle/Resources/config/services.yml b/src/Wallabag/CoreBundle/Resources/config/services.yml
index 1fb81a46..56d776ad 100644
--- a/src/Wallabag/CoreBundle/Resources/config/services.yml
+++ b/src/Wallabag/CoreBundle/Resources/config/services.yml
@@ -140,17 +140,19 @@ services:
     wallabag_core.subscriber.download_images:
         class: Wallabag\CoreBundle\Event\Subscriber\DownloadImagesSubscriber
         arguments:
+            - "@doctrine.orm.default_entity_manager"
             - "@wallabag_core.entry.download_images"
-            - "%craue_config.config.class%"
+            - '@=service(''craue_config'').get(''download_images_enabled'')'
             - "@logger"
         tags:
-            - { name: doctrine.event_subscriber }
+            - { name: kernel.event_subscriber }
 
     wallabag_core.entry.download_images:
         class: Wallabag\CoreBundle\Helper\DownloadImages
         arguments:
             - "@wallabag_core.entry.download_images.client"
             - "%kernel.root_dir%/../web/assets/images"
+            - '@=service(''craue_config'').get(''wallabag_url'')'
             - "@logger"
 
     wallabag_core.entry.download_images.client: