Merge pull request #3442 from wallabag/empty-entry

Fix empty title and domain_name when exception is thrown during fetch

diff --git a/CHANGELOG.md b/CHANGELOG.md
index eb8b1f704fd08be37789b65d0c062cebf89eb280..59d7f2d20016d8e6b1575a6a8d60080f1cd925eb 100644 (file)
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,6 +1,13 @@
 # Changelog
 
-##  [2.3.0](https://github.com/wallabag/wallabag/tree/2.3.0) (2017-10-21)
+## [unreleased](https://github.com/wallabag/wallabag/tree/master)
+   [Full Changelog](https://github.com/wallabag/wallabag/compare/2.3.0...master)
+
+### Fixes
+
+ - Fix empty title and domain_name when exception is thrown during fetch [#3442](https://github.com/wallabag/wallabag/pull/3442)
+
+##  [2.3.0](https://github.com/wallabag/wallabag/tree/2.3.0) (2017-12-11)
    [Full Changelog](https://github.com/wallabag/wallabag/compare/2.2.3...2.3.0)
 
 ### API

diff --git a/src/Wallabag/ApiBundle/Controller/EntryRestController.php b/src/Wallabag/ApiBundle/Controller/EntryRestController.php
index 7d820c7e606195e15697cd652442308e56643e8d..acca219fec733aabaf809d346dcfbb102c64891a 100644 (file)
--- a/src/Wallabag/ApiBundle/Controller/EntryRestController.php
+++ b/src/Wallabag/ApiBundle/Controller/EntryRestController.php
@@ -381,6 +381,14 @@ class EntryRestController extends WallabagRestController
             }
         }
 
+        if (empty($entry->getDomainName())) {
+            $this->get('wallabag_core.content_proxy')->setEntryDomainName($entry);
+        }
+
+        if (empty($entry->getTitle())) {
+            $this->get('wallabag_core.content_proxy')->setDefaultEntryTitle($entry);
+        }
+
         $em = $this->getDoctrine()->getManager();
         $em->persist($entry);
         $em->flush();
@@ -490,6 +498,14 @@ class EntryRestController extends WallabagRestController
             $entry->setOriginUrl($data['origin_url']);
         }
 
+        if (empty($entry->getDomainName())) {
+            $this->get('wallabag_core.content_proxy')->setEntryDomainName($entry);
+        }
+
+        if (empty($entry->getTitle())) {
+            $this->get('wallabag_core.content_proxy')->setDefaultEntryTitle($entry);
+        }
+
         $em = $this->getDoctrine()->getManager();
         $em->persist($entry);
         $em->flush();

diff --git a/src/Wallabag/CoreBundle/Controller/EntryController.php b/src/Wallabag/CoreBundle/Controller/EntryController.php
index 840dc254420794f5a2147329fd6cc9209a8bd06d..b7fdea27942099e821bc836b1f0e081843550e2c 100644 (file)
--- a/src/Wallabag/CoreBundle/Controller/EntryController.php
+++ b/src/Wallabag/CoreBundle/Controller/EntryController.php
@@ -502,6 +502,14 @@ class EntryController extends Controller
             $message = 'flashes.entry.notice.' . $prefixMessage . '_failed';
         }
 
+        if (empty($entry->getDomainName())) {
+            $this->get('wallabag_core.content_proxy')->setEntryDomainName($entry);
+        }
+
+        if (empty($entry->getTitle())) {
+            $this->get('wallabag_core.content_proxy')->setDefaultEntryTitle($entry);
+        }
+
         $this->get('session')->getFlashBag()->add('notice', $message);
     }
 

diff --git a/src/Wallabag/CoreBundle/Helper/ContentProxy.php b/src/Wallabag/CoreBundle/Helper/ContentProxy.php
index 4cc20c9cc4c2c2cb3115538bf0d5259ddc3f23f8..fe795d42b5669c967368d685cc334dd0a193e52f 100644 (file)
--- a/src/Wallabag/CoreBundle/Helper/ContentProxy.php
+++ b/src/Wallabag/CoreBundle/Helper/ContentProxy.php
@@ -144,6 +144,38 @@ class ContentProxy
         }
     }
 
+    /**
+     * Helper to extract and save host from entry url.
+     *
+     * @param Entry $entry
+     */
+    public function setEntryDomainName(Entry $entry)
+    {
+        $domainName = parse_url($entry->getUrl(), PHP_URL_HOST);
+        if (false !== $domainName) {
+            $entry->setDomainName($domainName);
+        }
+    }
+
+    /**
+     * Helper to set a default title using:
+     * - url basename, if applicable
+     * - hostname.
+     *
+     * @param Entry $entry
+     */
+    public function setDefaultEntryTitle(Entry $entry)
+    {
+        $url = parse_url($entry->getUrl());
+        $path = pathinfo($url['path'], PATHINFO_BASENAME);
+
+        if (empty($path)) {
+            $path = $url['host'];
+        }
+
+        $entry->setTitle($path);
+    }
+
     /**
      * Stock entry with fetched or imported content.
      * Will fall back to OpenGraph data if available.
@@ -155,10 +187,7 @@ class ContentProxy
     {
         $entry->setUrl($content['url']);
 
-        $domainName = parse_url($entry->getUrl(), PHP_URL_HOST);
-        if (false !== $domainName) {
-            $entry->setDomainName($domainName);
-        }
+        $this->setEntryDomainName($entry);
 
         if (!empty($content['title'])) {
             $entry->setTitle($content['title']);

diff --git a/tests/Wallabag/ApiBundle/Controller/EntryRestControllerTest.php b/tests/Wallabag/ApiBundle/Controller/EntryRestControllerTest.php
index b0d4c4e1a609bfb8778fc1da9b310fe6cb64b082..5c7b988c97b609396b6ad481615abf252152ec4a 100644 (file)
--- a/tests/Wallabag/ApiBundle/Controller/EntryRestControllerTest.php
+++ b/tests/Wallabag/ApiBundle/Controller/EntryRestControllerTest.php
@@ -501,6 +501,8 @@ class EntryRestControllerTest extends WallabagApiTestCase
             $content = json_decode($this->client->getResponse()->getContent(), true);
             $this->assertGreaterThan(0, $content['id']);
             $this->assertSame('http://www.example.com/', $content['url']);
+            $this->assertSame('www.example.com', $content['domain_name']);
+            $this->assertSame('www.example.com', $content['title']);
         } finally {
             // Remove the created entry to avoid side effects on other tests
             if (isset($content['id'])) {

diff --git a/tests/Wallabag/CoreBundle/Controller/EntryControllerTest.php b/tests/Wallabag/CoreBundle/Controller/EntryControllerTest.php
index 4ac4548b2624b2f75035e525a4f4e0fb5ec577f9..a02f969911f5a1ed44c2341d7f6ab9c22199b726 100644 (file)
--- a/tests/Wallabag/CoreBundle/Controller/EntryControllerTest.php
+++ b/tests/Wallabag/CoreBundle/Controller/EntryControllerTest.php
@@ -6,6 +6,7 @@ use Tests\Wallabag\CoreBundle\WallabagCoreTestCase;
 use Wallabag\CoreBundle\Entity\Config;
 use Wallabag\CoreBundle\Entity\Entry;
 use Wallabag\CoreBundle\Entity\SiteCredential;
+use Wallabag\CoreBundle\Helper\ContentProxy;
 
 class EntryControllerTest extends WallabagCoreTestCase
 {
@@ -1429,4 +1430,52 @@ class EntryControllerTest extends WallabagCoreTestCase
 
         $client->getContainer()->get('craue_config')->set('restricted_access', 0);
     }
+
+    public function testPostEntryWhenFetchFails()
+    {
+        $url = 'http://example.com/papers/email_tracking.pdf';
+        $this->logInAs('admin');
+        $client = $this->getClient();
+
+        $container = $client->getContainer();
+        $contentProxy = $this->getMockBuilder(ContentProxy::class)
+            ->disableOriginalConstructor()
+            ->setMethods(['updateEntry'])
+            ->getMock();
+        $contentProxy->expects($this->any())
+            ->method('updateEntry')
+            ->willThrowException(new \Exception('Test Fetch content fails'));
+
+        $crawler = $client->request('GET', '/new');
+
+        $this->assertSame(200, $client->getResponse()->getStatusCode());
+
+        $form = $crawler->filter('form[name=entry]')->form();
+
+        $data = [
+            'entry[url]' => $url,
+        ];
+
+        /**
+         * We generate a new client to be able to use Mock ContentProxy
+         * Also we reinject the cookie from the previous client to keep the
+         * session.
+         */
+        $cookie = $client->getCookieJar()->all();
+        $client = $this->getNewClient();
+        $client->getCookieJar()->set($cookie[0]);
+        $client->getContainer()->set('wallabag_core.content_proxy', $contentProxy);
+        $client->submit($form, $data);
+
+        $this->assertSame(302, $client->getResponse()->getStatusCode());
+
+        $content = $client->getContainer()
+            ->get('doctrine.orm.entity_manager')
+            ->getRepository('WallabagCoreBundle:Entry')
+            ->findByUrlAndUserId($url, $this->getLoggedInUserId());
+
+        $authors = $content->getPublishedBy();
+        $this->assertSame('email_tracking.pdf', $content->getTitle());
+        $this->assertSame('example.com', $content->getDomainName());
+    }
 }

diff --git a/tests/Wallabag/CoreBundle/WallabagCoreTestCase.php b/tests/Wallabag/CoreBundle/WallabagCoreTestCase.php
index 1eda519946883052dc967584d180fd2c4edb4dd7..6e1163c56ce2a285cf8499e448ed578e16c87839 100644 (file)
--- a/tests/Wallabag/CoreBundle/WallabagCoreTestCase.php
+++ b/tests/Wallabag/CoreBundle/WallabagCoreTestCase.php
@@ -25,6 +25,11 @@ abstract class WallabagCoreTestCase extends WebTestCase
         $this->client = static::createClient();
     }
 
+    public function getNewClient()
+    {
+        return $this->client = static::createClient();
+    }
+
     public function getClient()
     {
         return $this->client;