From af29e1bf07aabaa6a4e4653c1a3b5c10ce831bb6 Mon Sep 17 00:00:00 2001 From: Kevin Decherf 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 --- .../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