Move fetching content in a separate class

src/Wallabag/ApiBundle/Controller/WallabagRestController.php

@@ -146,16 +146,10 @@ class WallabagRestController extends Controller
     {
         $url = $request->request->get('url');
 
-        $content = $this->get('wallabag_core.graby')->fetchContent($url);
+        $entry = $this->get('wallabag_core.content_proxy')->updateEntry(
-
+            new Entry($this->getUser()),
-        $entry = new Entry($this->getUser());
+            $url
-        $entry->setUrl($content['url'] ?: $url);
+        );
-        $entry->setTitle($request->request->get('title') ?: $content['title']);
-        $entry->setContent($content['html']);
-        $entry->setMimetype($content['content_type']);
-        if (isset($content['open_graph']['og_image'])) {
-            $entry->setPreviewPicture($content['open_graph']['og_image']);
-        }
 
         $tags = $request->request->get('tags', '');
         if (!empty($tags)) {

src/Wallabag/CoreBundle/Controller/EntryController.php

@@ -30,15 +30,7 @@ class EntryController extends Controller
         $form->handleRequest($request);
 
         if ($form->isValid()) {
-            $content = $this->get('wallabag_core.graby')->fetchContent($entry->getUrl());
+            $entry = $this->get('wallabag_core.content_proxy')->updateEntry($entry, $entry->getUrl());
-
-            $entry->setUrl($content['url'] ?: $entry->getUrl());
-            $entry->setTitle($content['title']);
-            $entry->setContent($content['html']);
-            $entry->setMimetype($content['content_type']);
-            if (isset($content['open_graph']['og_image'])) {
-                $entry->setPreviewPicture($content['open_graph']['og_image']);
-            }
 
             $em = $this->getDoctrine()->getManager();
             $em->persist($entry);

src/Wallabag/CoreBundle/Helper/ContentProxy.php

@@ -0,0 +1,60 @@
+<?php
+
+namespace Wallabag\CoreBundle\Helper;
+
+use Graby\Graby;
+use Wallabag\CoreBundle\Entity\Entry;
+
+/**
+ * This kind of proxy class take care of getting the content from an url
+ * and update the entry with what it found
+ */
+class ContentProxy
+{
+    protected $graby;
+
+    public function __construct(Graby $graby)
+    {
+        $this->graby = $graby;
+    }
+
+    /**
+     * Fetch content using graby and hydrate given entry with results information.
+     * In case we couldn't find content, we'll try to use Open Graph data
+     *
+     * @param  Entry  $entry Entry to update
+     * @param  string $url   Url to grab content for
+     *
+     * @return Entry
+     */
+    public function updateEntry(Entry $entry, $url)
+    {
+        $content = $this->graby->fetchContent($url);
+
+        $title = $content['title'];
+        if (!$title && isset($content['open_graph']['og_title'])) {
+            $title = $content['open_graph']['og_title'];
+        }
+
+        $html = $content['html'];
+        if (false === $html) {
+            $html = '<p>Unable to retrieve readable content.</p>';
+
+            if (isset($content['open_graph']['og_description'])) {
+                $html .= '<p><i>But we found a short description: </i></p>';
+                $html .= $content['open_graph']['og_description'];
+            }
+        }
+
+        $entry->setUrl($content['url'] ?: $url);
+        $entry->setTitle($title);
+        $entry->setContent($html);
+        $entry->setMimetype($content['content_type']);
+
+        if (isset($content['open_graph']['og_image'])) {
+            $entry->setPreviewPicture($content['open_graph']['og_image']);
+        }
+
+        return $entry;
+    }
+}

src/Wallabag/CoreBundle/Resources/config/services.yml

@@ -33,3 +33,10 @@ services:
 
     wallabag_core.graby:
         class: Graby\Graby
+        arguments:
+            - { error_message: false }
+
+    wallabag_core.content_proxy:
+        class: Wallabag\CoreBundle\Helper\ContentProxy
+        arguments:
+            - @wallabag_core.graby

src/Wallabag/CoreBundle/Tests/Helper/ContentProxyTest.php

@@ -0,0 +1,84 @@
+<?php
+
+namespace Wallabag\CoreBundle\Tests\Helper;
+
+use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase;
+use Wallabag\CoreBundle\Entity\Entry;
+use Wallabag\CoreBundle\Entity\User;
+use Wallabag\CoreBundle\Helper\ContentProxy;
+
+class ContentProxyTest extends KernelTestCase
+{
+    public function testWithEmptyContent()
+    {
+        $graby = $this->getMockBuilder('Graby\Graby')
+            ->setMethods(array('fetchContent'))
+            ->disableOriginalConstructor()
+            ->getMock();
+
+        $graby->expects($this->any())
+            ->method('fetchContent')
+            ->willReturn(array('html' => false, 'title' => '', 'url' => '', 'content_type' => ''));
+
+        $proxy = new ContentProxy($graby);
+        $entry = $proxy->updateEntry(new Entry(new User()), 'http://0.0.0.0');
+
+        $this->assertEquals('http://0.0.0.0', $entry->getUrl());
+        $this->assertEmpty($entry->getTitle());
+        $this->assertEquals('<p>Unable to retrieve readable content.</p>', $entry->getContent());
+        $this->assertEmpty($entry->getPreviewPicture());
+        $this->assertEmpty($entry->getMimetype());
+    }
+
+    public function testWithEmptyContentButOG()
+    {
+        $graby = $this->getMockBuilder('Graby\Graby')
+            ->setMethods(array('fetchContent'))
+            ->disableOriginalConstructor()
+            ->getMock();
+
+        $graby->expects($this->any())
+            ->method('fetchContent')
+            ->willReturn(array('html' => false, 'title' => '', 'url' => '', 'content_type' => '', 'open_graph' => array('og_title' => 'my title', 'og_description' => 'desc')));
+
+        $proxy = new ContentProxy($graby);
+        $entry = $proxy->updateEntry(new Entry(new User()), 'http://0.0.0.0');
+
+        $this->assertEquals('http://0.0.0.0', $entry->getUrl());
+        $this->assertEquals('my title', $entry->getTitle());
+        $this->assertEquals('<p>Unable to retrieve readable content.</p><p><i>But we found a short description: </i></p>desc', $entry->getContent());
+        $this->assertEmpty($entry->getPreviewPicture());
+        $this->assertEmpty($entry->getMimetype());
+    }
+
+    public function testWithContent()
+    {
+        $graby = $this->getMockBuilder('Graby\Graby')
+            ->setMethods(array('fetchContent'))
+            ->disableOriginalConstructor()
+            ->getMock();
+
+        $graby->expects($this->any())
+            ->method('fetchContent')
+            ->willReturn(array(
+                'html' => 'this is my content',
+                'title' => 'this is my title',
+                'url' => 'http://1.1.1.1',
+                'content_type' => 'text/html',
+                'open_graph' => array(
+                    'og_title' => 'my OG title',
+                    'og_description' => 'OG desc',
+                    'og_image' => 'http://3.3.3.3/cover.jpg'
+                )
+            ));
+
+        $proxy = new ContentProxy($graby);
+        $entry = $proxy->updateEntry(new Entry(new User()), 'http://0.0.0.0');
+
+        $this->assertEquals('http://1.1.1.1', $entry->getUrl());
+        $this->assertEquals('this is my title', $entry->getTitle());
+        $this->assertEquals('this is my content', $entry->getContent());
+        $this->assertEquals('http://3.3.3.3/cover.jpg', $entry->getPreviewPicture());
+        $this->assertEquals('text/html', $entry->getMimetype());
+    }
+}