From af29e1bf07aabaa6a4e4653c1a3b5c10ce831bb6 Mon Sep 17 00:00:00 2001
From: Kevin Decherf <kevin@kdecherf.com>
Date: Sun, 26 Nov 2017 23:20:23 +0100
Subject: Fix empty title and domain_name when exception is thrown during fetch

Add a new helper to set a default title when it's empty:
1/ use basename part of entry's path, if any
2/ or use domain name

Fixes #2053

Signed-off-by: Kevin Decherf <kevin@kdecherf.com>
---
 .../ApiBundle/Controller/EntryRestController.php   | 16 ++++++++++
 .../CoreBundle/Controller/EntryController.php      |  8 +++++
 src/Wallabag/CoreBundle/Helper/ContentProxy.php    | 37 +++++++++++++++++++---
 .../Controller/EntryRestControllerTest.php         |  2 ++
 4 files changed, 59 insertions(+), 4 deletions(-)

diff --git a/src/Wallabag/ApiBundle/Controller/EntryRestController.php b/src/Wallabag/ApiBundle/Controller/EntryRestController.php
index 7d820c7e..acca219f 100644
--- 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 840dc254..b7fdea27 100644
--- 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 4cc20c9c..fe795d42 100644
--- 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 b0d4c4e1..5c7b988c 100644
--- 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'])) {
-- 
cgit v1.2.3