From 558d9aabab7e01c2e2b506aa362c70a568b953aa Mon Sep 17 00:00:00 2001 From: Jeremy Benoist Date: Thu, 10 Sep 2015 21:57:25 +0200 Subject: [PATCH] Move fetching content in a separate class --- .../Controller/WallabagRestController.php | 14 +--- .../CoreBundle/Controller/EntryController.php | 10 +-- .../CoreBundle/Helper/ContentProxy.php | 60 +++++++++++++ .../CoreBundle/Resources/config/services.yml | 7 ++ .../Tests/Helper/ContentProxyTest.php | 84 +++++++++++++++++++ 5 files changed, 156 insertions(+), 19 deletions(-) create mode 100644 src/Wallabag/CoreBundle/Helper/ContentProxy.php create mode 100644 src/Wallabag/CoreBundle/Tests/Helper/ContentProxyTest.php diff --git a/src/Wallabag/ApiBundle/Controller/WallabagRestController.php b/src/Wallabag/ApiBundle/Controller/WallabagRestController.php index 7452c82af..349229f38 100644 --- a/src/Wallabag/ApiBundle/Controller/WallabagRestController.php +++ b/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 = new Entry($this->getUser()); - $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']); - } + $entry = $this->get('wallabag_core.content_proxy')->updateEntry( + new Entry($this->getUser()), + $url + ); $tags = $request->request->get('tags', ''); if (!empty($tags)) { diff --git a/src/Wallabag/CoreBundle/Controller/EntryController.php b/src/Wallabag/CoreBundle/Controller/EntryController.php index e6580a579..b9e4e67e3 100644 --- a/src/Wallabag/CoreBundle/Controller/EntryController.php +++ b/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->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']); - } + $entry = $this->get('wallabag_core.content_proxy')->updateEntry($entry, $entry->getUrl()); $em = $this->getDoctrine()->getManager(); $em->persist($entry); diff --git a/src/Wallabag/CoreBundle/Helper/ContentProxy.php b/src/Wallabag/CoreBundle/Helper/ContentProxy.php new file mode 100644 index 000000000..2dd70e51d --- /dev/null +++ b/src/Wallabag/CoreBundle/Helper/ContentProxy.php @@ -0,0 +1,60 @@ +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 = '

Unable to retrieve readable content.

'; + + if (isset($content['open_graph']['og_description'])) { + $html .= '

But we found a short description:

'; + $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; + } +} diff --git a/src/Wallabag/CoreBundle/Resources/config/services.yml b/src/Wallabag/CoreBundle/Resources/config/services.yml index 6b8774f20..3beb5d0ef 100644 --- a/src/Wallabag/CoreBundle/Resources/config/services.yml +++ b/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 diff --git a/src/Wallabag/CoreBundle/Tests/Helper/ContentProxyTest.php b/src/Wallabag/CoreBundle/Tests/Helper/ContentProxyTest.php new file mode 100644 index 000000000..7c93f4607 --- /dev/null +++ b/src/Wallabag/CoreBundle/Tests/Helper/ContentProxyTest.php @@ -0,0 +1,84 @@ +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('

Unable to retrieve readable content.

', $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('

Unable to retrieve readable content.

But we found a short description:

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()); + } +}