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 From 300f293cb1b7c2e2a94e3d716c75faf4b2a9b823 Mon Sep 17 00:00:00 2001 From: Kevin Decherf Date: Mon, 27 Nov 2017 22:56:46 +0100 Subject: Add test on EntryControllerTest for #3442 Signed-off-by: Kevin Decherf --- .../CoreBundle/Controller/EntryControllerTest.php | 49 ++++++++++++++++++++++ tests/Wallabag/CoreBundle/WallabagCoreTestCase.php | 5 +++ 2 files changed, 54 insertions(+) diff --git a/tests/Wallabag/CoreBundle/Controller/EntryControllerTest.php b/tests/Wallabag/CoreBundle/Controller/EntryControllerTest.php index 4ac4548b..a02f9699 100644 --- 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 1eda5199..6e1163c5 100644 --- 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; -- cgit v1.2.3 From b457d7bd32b0eb228f6802d92630d8f0524f042f Mon Sep 17 00:00:00 2001 From: Kevin Decherf Date: Wed, 13 Dec 2017 22:49:53 +0100 Subject: Update CHANGELOG Signed-off-by: Kevin Decherf --- CHANGELOG.md | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index eb8b1f70..59d7f2d2 100644 --- 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 -- cgit v1.2.3