mirrors/wallabag
1
0
Fork 0
mirror of https://github.com/wallabag/wallabag.git synced 2024-11-26 19:11:07 +00:00
Code Issues Projects Releases Wiki Activity

Rewrote code & fix tests

Browse source
This commit is contained in:
Jeremy Benoist 2017-06-01 11:31:45 +02:00
parent 843182c7cf
commit 6acadf8e98
No known key found for this signature in database
GPG key ID: FB413A58C7715C86
11 changed files with 71 additions and 101 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

60
src/Wallabag/CoreBundle/Helper/ContentProxy.php
View file

@ -7,7 +7,6 @@ use Psr\Log\LoggerInterface;
use Wallabag\CoreBundle\Entity\Entry; use Wallabag\CoreBundle\Entity\Entry;
use Wallabag\CoreBundle\Tools\Utils; use Wallabag\CoreBundle\Tools\Utils;
use Symfony\Component\HttpFoundation\File\MimeType\MimeTypeExtensionGuesser; use Symfony\Component\HttpFoundation\File\MimeType\MimeTypeExtensionGuesser;
use Symfony\Component\Config\Definition\Exception\Exception;
/** /**
* This kind of proxy class take care of getting the content from an url * This kind of proxy class take care of getting the content from an url
@ -32,57 +31,40 @@ class ContentProxy
} }
/** /**
* Update existing entry by fetching from URL using Graby. * Update entry using either fetched or provided content.
*
* @param Entry $entry Entry to update
* @param string $url Url to grab content for
*/
public function updateEntry(Entry $entry, $url)
{
$content = $this->graby->fetchContent($url);
// be sure to keep the url in case of error
// so we'll be able to refetch it in the future
$content['url'] = $content['url'] ?: $url;
$this->stockEntry($entry, $content);
}
/**
* Import entry using either fetched or provided content.
* *
* @param Entry $entry Entry to update * @param Entry $entry Entry to update
* @param string $url Url of the content
* @param array $content Array with content provided for import with AT LEAST keys title, html, url to skip the fetchContent from the url * @param array $content Array with content provided for import with AT LEAST keys title, html, url to skip the fetchContent from the url
* @param bool $disableContentUpdate Whether to skip trying to fetch content using Graby * @param bool $disableContentUpdate Whether to skip trying to fetch content using Graby
*/ */
public function importEntry(Entry $entry, array $content, $disableContentUpdate = false) public function updateEntry(Entry $entry, $url, array $content = [], $disableContentUpdate = false)
{ {
try { if (!empty($content['html'])) {
$this->validateContent($content); $content['html'] = $this->graby->cleanupHtml($content['html'], $url);
} catch (\Exception $e) {
// validation failed but do we want to disable updating content?
if (true === $disableContentUpdate) {
throw $e;
}
} }
if (false === $disableContentUpdate) { if ((empty($content) || false === $this->validateContent($content)) && false === $disableContentUpdate) {
try { try {
$fetchedContent = $this->graby->fetchContent($content['url']); $fetchedContent = $this->graby->fetchContent($url);
} catch (\Exception $e) { } catch (\Exception $e) {
$this->logger->error('Error while trying to fetch content from URL.', [ $this->logger->error('Error while trying to fetch content from URL.', [
'entry_url' => $content['url'], 'entry_url' => $url,
'error_msg' => $e->getMessage(), 'error_msg' => $e->getMessage(),
]); ]);
} }
// when content is imported, we have information in $content // when content is imported, we have information in $content
// in case fetching content goes bad, we'll keep the imported information instead of overriding them // in case fetching content goes bad, we'll keep the imported information instead of overriding them
if ($fetchedContent['html'] !== $this->fetchingErrorMessage) { if (empty($content) || $fetchedContent['html'] !== $this->fetchingErrorMessage) {
$content = $fetchedContent; $content = $fetchedContent;
} }
} }
// be sure to keep the url in case of error
// so we'll be able to refetch it in the future
$content['url'] = !empty($content['url']) ? $content['url'] : $url;
$this->stockEntry($entry, $content); $this->stockEntry($entry, $content);
} }
@ -126,7 +108,7 @@ class ContentProxy
try { try {
$entry->setPublishedAt(new \DateTime($date)); $entry->setPublishedAt(new \DateTime($date));
} catch (\Exception $e) { } catch (\Exception $e) {
$this->logger->warning('Error while defining date', ['e' => $e, 'url' => $url, 'date' => $content['date']]); $this->logger->warning('Error while defining date', ['e' => $e, 'url' => $content['url'], 'date' => $content['date']]);
} }
} }
@ -170,19 +152,11 @@ class ContentProxy
* Validate that the given content has at least a title, an html and a url. * Validate that the given content has at least a title, an html and a url.
* *
* @param array $content * @param array $content
*
* @return bool true if valid otherwise false
*/ */
private function validateContent(array $content) private function validateContent(array $content)
{ {
if (empty($content['title'])) { return !empty($content['title']) && !empty($content['html']) && !empty($content['url']);
throw new Exception('Missing title from imported entry!');
}
if (empty($content['url'])) {
throw new Exception('Missing URL from imported entry!');
}
if (empty($content['html'])) {
throw new Exception('Missing html from imported entry!');
}
} }
} }

7
src/Wallabag/ImportBundle/Import/AbstractImport.php
View file

@ -115,14 +115,11 @@ abstract class AbstractImport implements ImportInterface
*/ */
protected function fetchContent(Entry $entry, $url, array $content = []) protected function fetchContent(Entry $entry, $url, array $content = [])
{ {
// be sure to set at least the given url
$content['url'] = isset($content['url']) ? $content['url'] : $url;
try { try {
$this->contentProxy->importEntry($entry, $content, $this->disableContentUpdate); $this->contentProxy->updateEntry($entry, $url, $content, $this->disableContentUpdate);
} catch (\Exception $e) { } catch (\Exception $e) {
$this->logger->error('Error trying to import an entry.', [ $this->logger->error('Error trying to import an entry.', [
'entry_url' => $content['url'], 'entry_url' => $url,
'error_msg' => $e->getMessage(), 'error_msg' => $e->getMessage(),
]); ]);
} }

27
tests/Wallabag/CoreBundle/Helper/ContentProxyTest.php
View file

@ -11,8 +11,6 @@ use Wallabag\CoreBundle\Entity\Tag;
use Wallabag\UserBundle\Entity\User; use Wallabag\UserBundle\Entity\User;
use Wallabag\CoreBundle\Helper\RuleBasedTagger; use Wallabag\CoreBundle\Helper\RuleBasedTagger;
use Graby\Graby; use Graby\Graby;
use Monolog\Handler\TestHandler;
use Monolog\Logger;
class ContentProxyTest extends \PHPUnit_Framework_TestCase class ContentProxyTest extends \PHPUnit_Framework_TestCase
{ {
@ -259,12 +257,13 @@ class ContentProxyTest extends \PHPUnit_Framework_TestCase
->method('tag'); ->method('tag');
$logHandler = new TestHandler(); $logHandler = new TestHandler();
$logger = new Logger('test', array($logHandler)); $logger = new Logger('test', [$logHandler]);
$proxy = new ContentProxy((new Graby()), $tagger, $logger, $this->fetchingErrorMessage); $proxy = new ContentProxy((new Graby()), $tagger, $logger, $this->fetchingErrorMessage);
$entry = new Entry(new User()); $entry = new Entry(new User());
$proxy->importEntry( $proxy->updateEntry(
$entry, $entry,
'http://1.1.1.1',
[ [
'html' => str_repeat('this is my content', 325), 'html' => str_repeat('this is my content', 325),
'title' => 'this is my title', 'title' => 'this is my title',
@ -299,6 +298,7 @@ class ContentProxyTest extends \PHPUnit_Framework_TestCase
$entry = new Entry(new User()); $entry = new Entry(new User());
$proxy->updateEntry( $proxy->updateEntry(
$entry, $entry,
'http://1.1.1.1',
[ [
'html' => str_repeat('this is my content', 325), 'html' => str_repeat('this is my content', 325),
'title' => 'this is my title', 'title' => 'this is my title',
@ -326,26 +326,24 @@ class ContentProxyTest extends \PHPUnit_Framework_TestCase
public function testTaggerThrowException() public function testTaggerThrowException()
{ {
$graby = $this->getMockBuilder('Graby\Graby')
->disableOriginalConstructor()
->getMock();
$tagger = $this->getTaggerMock(); $tagger = $this->getTaggerMock();
$tagger->expects($this->once()) $tagger->expects($this->once())
->method('tag') ->method('tag')
->will($this->throwException(new \Exception())); ->will($this->throwException(new \Exception()));
$proxy = new ContentProxy($graby, $tagger, $this->getLogger(), $this->fetchingErrorMessage); $proxy = new ContentProxy((new Graby()), $tagger, $this->getLogger(), $this->fetchingErrorMessage);
$entry = new Entry(new User()); $entry = new Entry(new User());
$content = array( $proxy->updateEntry(
$entry,
'http://1.1.1.1',
[
'html' => str_repeat('this is my content', 325), 'html' => str_repeat('this is my content', 325),
'title' => 'this is my title', 'title' => 'this is my title',
'url' => 'http://1.1.1.1', 'url' => 'http://1.1.1.1',
'content_type' => 'text/html', 'content_type' => 'text/html',
'language' => 'fr', 'language' => 'fr',
]
); );
$proxy->importEntry($entry, $content, true);
$this->assertCount(0, $entry->getTags()); $this->assertCount(0, $entry->getTags());
} }
@ -374,8 +372,9 @@ class ContentProxyTest extends \PHPUnit_Framework_TestCase
->method('tag'); ->method('tag');
$proxy = new ContentProxy((new Graby()), $tagger, $this->getLogger(), $this->fetchingErrorMessage); $proxy = new ContentProxy((new Graby()), $tagger, $this->getLogger(), $this->fetchingErrorMessage);
$entry = $proxy->updateEntry( $entry = new Entry(new User());
new Entry(new User()), $proxy->updateEntry(
$entry,
'http://1.1.1.1', 'http://1.1.1.1',
[ [
'html' => $html, 'html' => $html,

8
tests/Wallabag/ImportBundle/Import/ChromeImportTest.php
View file

@ -89,7 +89,7 @@ class ChromeImportTest extends \PHPUnit_Framework_TestCase
$this->contentProxy $this->contentProxy
->expects($this->exactly(1)) ->expects($this->exactly(1))
->method('importEntry') ->method('updateEntry')
->willReturn($entry); ->willReturn($entry);
$res = $chromeImport->import(); $res = $chromeImport->import();
@ -118,7 +118,7 @@ class ChromeImportTest extends \PHPUnit_Framework_TestCase
$this->contentProxy $this->contentProxy
->expects($this->exactly(1)) ->expects($this->exactly(1))
->method('importEntry') ->method('updateEntry')
->willReturn(new Entry($this->user)); ->willReturn(new Entry($this->user));
// check that every entry persisted are archived // check that every entry persisted are archived
@ -158,7 +158,7 @@ class ChromeImportTest extends \PHPUnit_Framework_TestCase
$this->contentProxy $this->contentProxy
->expects($this->never()) ->expects($this->never())
->method('importEntry'); ->method('updateEntry');
$producer = $this->getMockBuilder('OldSound\RabbitMqBundle\RabbitMq\Producer') $producer = $this->getMockBuilder('OldSound\RabbitMqBundle\RabbitMq\Producer')
->disableOriginalConstructor() ->disableOriginalConstructor()
@ -198,7 +198,7 @@ class ChromeImportTest extends \PHPUnit_Framework_TestCase
$this->contentProxy $this->contentProxy
->expects($this->never()) ->expects($this->never())
->method('importEntry'); ->method('updateEntry');
$factory = new RedisMockFactory(); $factory = new RedisMockFactory();
$redisMock = $factory->getAdapter('Predis\Client', true); $redisMock = $factory->getAdapter('Predis\Client', true);

8
tests/Wallabag/ImportBundle/Import/FirefoxImportTest.php
View file

@ -89,7 +89,7 @@ class FirefoxImportTest extends \PHPUnit_Framework_TestCase
$this->contentProxy $this->contentProxy
->expects($this->exactly(2)) ->expects($this->exactly(2))
->method('importEntry') ->method('updateEntry')
->willReturn($entry); ->willReturn($entry);
$res = $firefoxImport->import(); $res = $firefoxImport->import();
@ -118,7 +118,7 @@ class FirefoxImportTest extends \PHPUnit_Framework_TestCase
$this->contentProxy $this->contentProxy
->expects($this->exactly(1)) ->expects($this->exactly(1))
->method('importEntry') ->method('updateEntry')
->willReturn(new Entry($this->user)); ->willReturn(new Entry($this->user));
// check that every entry persisted are archived // check that every entry persisted are archived
@ -158,7 +158,7 @@ class FirefoxImportTest extends \PHPUnit_Framework_TestCase
$this->contentProxy $this->contentProxy
->expects($this->never()) ->expects($this->never())
->method('importEntry'); ->method('updateEntry');
$producer = $this->getMockBuilder('OldSound\RabbitMqBundle\RabbitMq\Producer') $producer = $this->getMockBuilder('OldSound\RabbitMqBundle\RabbitMq\Producer')
->disableOriginalConstructor() ->disableOriginalConstructor()
@ -198,7 +198,7 @@ class FirefoxImportTest extends \PHPUnit_Framework_TestCase
$this->contentProxy $this->contentProxy
->expects($this->never()) ->expects($this->never())
->method('importEntry'); ->method('updateEntry');
$factory = new RedisMockFactory(); $factory = new RedisMockFactory();
$redisMock = $factory->getAdapter('Predis\Client', true); $redisMock = $factory->getAdapter('Predis\Client', true);

8
tests/Wallabag/ImportBundle/Import/InstapaperImportTest.php
View file

@ -104,7 +104,7 @@ class InstapaperImportTest extends \PHPUnit_Framework_TestCase
$this->contentProxy $this->contentProxy
->expects($this->exactly(4)) ->expects($this->exactly(4))
->method('importEntry') ->method('updateEntry')
->willReturn($entry); ->willReturn($entry);
$res = $instapaperImport->import(); $res = $instapaperImport->import();
@ -133,7 +133,7 @@ class InstapaperImportTest extends \PHPUnit_Framework_TestCase
$this->contentProxy $this->contentProxy
->expects($this->once()) ->expects($this->once())
->method('importEntry') ->method('updateEntry')
->willReturn(new Entry($this->user)); ->willReturn(new Entry($this->user));
// check that every entry persisted are archived // check that every entry persisted are archived
@ -173,7 +173,7 @@ class InstapaperImportTest extends \PHPUnit_Framework_TestCase
$this->contentProxy $this->contentProxy
->expects($this->never()) ->expects($this->never())
->method('importEntry'); ->method('updateEntry');
$producer = $this->getMockBuilder('OldSound\RabbitMqBundle\RabbitMq\Producer') $producer = $this->getMockBuilder('OldSound\RabbitMqBundle\RabbitMq\Producer')
->disableOriginalConstructor() ->disableOriginalConstructor()
@ -213,7 +213,7 @@ class InstapaperImportTest extends \PHPUnit_Framework_TestCase
$this->contentProxy $this->contentProxy
->expects($this->never()) ->expects($this->never())
->method('importEntry'); ->method('updateEntry');
$factory = new RedisMockFactory(); $factory = new RedisMockFactory();
$redisMock = $factory->getAdapter('Predis\Client', true); $redisMock = $factory->getAdapter('Predis\Client', true);

10
tests/Wallabag/ImportBundle/Import/PocketImportTest.php
View file

@ -282,7 +282,7 @@ class PocketImportTest extends \PHPUnit_Framework_TestCase
$this->contentProxy $this->contentProxy
->expects($this->once()) ->expects($this->once())
->method('importEntry') ->method('updateEntry')
->willReturn($entry); ->willReturn($entry);
$pocketImport->setClient($client); $pocketImport->setClient($client);
@ -377,7 +377,7 @@ class PocketImportTest extends \PHPUnit_Framework_TestCase
$this->contentProxy $this->contentProxy
->expects($this->exactly(2)) ->expects($this->exactly(2))
->method('importEntry') ->method('updateEntry')
->willReturn($entry); ->willReturn($entry);
$pocketImport->setClient($client); $pocketImport->setClient($client);
@ -450,7 +450,7 @@ JSON;
$this->contentProxy $this->contentProxy
->expects($this->never()) ->expects($this->never())
->method('importEntry'); ->method('updateEntry');
$producer = $this->getMockBuilder('OldSound\RabbitMqBundle\RabbitMq\Producer') $producer = $this->getMockBuilder('OldSound\RabbitMqBundle\RabbitMq\Producer')
->disableOriginalConstructor() ->disableOriginalConstructor()
@ -536,7 +536,7 @@ JSON;
$this->contentProxy $this->contentProxy
->expects($this->never()) ->expects($this->never())
->method('ImportEntry'); ->method('updateEntry');
$factory = new RedisMockFactory(); $factory = new RedisMockFactory();
$redisMock = $factory->getAdapter('Predis\Client', true); $redisMock = $factory->getAdapter('Predis\Client', true);
@ -621,7 +621,7 @@ JSON;
$this->contentProxy $this->contentProxy
->expects($this->once()) ->expects($this->once())
->method('importEntry') ->method('updateEntry')
->will($this->throwException(new \Exception())); ->will($this->throwException(new \Exception()));
$pocketImport->setClient($client); $pocketImport->setClient($client);

8
tests/Wallabag/ImportBundle/Import/ReadabilityImportTest.php
View file

@ -89,7 +89,7 @@ class ReadabilityImportTest extends \PHPUnit_Framework_TestCase
$this->contentProxy $this->contentProxy
->expects($this->exactly(3)) ->expects($this->exactly(3))
->method('importEntry') ->method('updateEntry')
->willReturn($entry); ->willReturn($entry);
$res = $readabilityImport->import(); $res = $readabilityImport->import();
@ -118,7 +118,7 @@ class ReadabilityImportTest extends \PHPUnit_Framework_TestCase
$this->contentProxy $this->contentProxy
->expects($this->exactly(1)) ->expects($this->exactly(1))
->method('importEntry') ->method('updateEntry')
->willReturn(new Entry($this->user)); ->willReturn(new Entry($this->user));
// check that every entry persisted are archived // check that every entry persisted are archived
@ -158,7 +158,7 @@ class ReadabilityImportTest extends \PHPUnit_Framework_TestCase
$this->contentProxy $this->contentProxy
->expects($this->never()) ->expects($this->never())
->method('importEntry'); ->method('updateEntry');
$producer = $this->getMockBuilder('OldSound\RabbitMqBundle\RabbitMq\Producer') $producer = $this->getMockBuilder('OldSound\RabbitMqBundle\RabbitMq\Producer')
->disableOriginalConstructor() ->disableOriginalConstructor()
@ -198,7 +198,7 @@ class ReadabilityImportTest extends \PHPUnit_Framework_TestCase
$this->contentProxy $this->contentProxy
->expects($this->never()) ->expects($this->never())
->method('importEntry'); ->method('updateEntry');
$factory = new RedisMockFactory(); $factory = new RedisMockFactory();
$redisMock = $factory->getAdapter('Predis\Client', true); $redisMock = $factory->getAdapter('Predis\Client', true);

8
tests/Wallabag/ImportBundle/Import/WallabagV1ImportTest.php
View file

@ -113,7 +113,7 @@ class WallabagV1ImportTest extends \PHPUnit_Framework_TestCase
$this->contentProxy $this->contentProxy
->expects($this->exactly(1)) ->expects($this->exactly(1))
->method('importEntry') ->method('updateEntry')
->willReturn($entry); ->willReturn($entry);
$res = $wallabagV1Import->import(); $res = $wallabagV1Import->import();
@ -142,7 +142,7 @@ class WallabagV1ImportTest extends \PHPUnit_Framework_TestCase
$this->contentProxy $this->contentProxy
->expects($this->exactly(3)) ->expects($this->exactly(3))
->method('importEntry') ->method('updateEntry')
->willReturn(new Entry($this->user)); ->willReturn(new Entry($this->user));
// check that every entry persisted are archived // check that every entry persisted are archived
@ -182,7 +182,7 @@ class WallabagV1ImportTest extends \PHPUnit_Framework_TestCase
$this->contentProxy $this->contentProxy
->expects($this->never()) ->expects($this->never())
->method('importEntry'); ->method('updateEntry');
$producer = $this->getMockBuilder('OldSound\RabbitMqBundle\RabbitMq\Producer') $producer = $this->getMockBuilder('OldSound\RabbitMqBundle\RabbitMq\Producer')
->disableOriginalConstructor() ->disableOriginalConstructor()
@ -222,7 +222,7 @@ class WallabagV1ImportTest extends \PHPUnit_Framework_TestCase
$this->contentProxy $this->contentProxy
->expects($this->never()) ->expects($this->never())
->method('importEntry'); ->method('updateEntry');
$factory = new RedisMockFactory(); $factory = new RedisMockFactory();
$redisMock = $factory->getAdapter('Predis\Client', true); $redisMock = $factory->getAdapter('Predis\Client', true);

10
tests/Wallabag/ImportBundle/Import/WallabagV2ImportTest.php
View file

@ -100,7 +100,7 @@ class WallabagV2ImportTest extends \PHPUnit_Framework_TestCase
$this->contentProxy $this->contentProxy
->expects($this->exactly(2)) ->expects($this->exactly(2))
->method('importEntry') ->method('updateEntry')
->willReturn(new Entry($this->user)); ->willReturn(new Entry($this->user));
$res = $wallabagV2Import->import(); $res = $wallabagV2Import->import();
@ -129,7 +129,7 @@ class WallabagV2ImportTest extends \PHPUnit_Framework_TestCase
$this->contentProxy $this->contentProxy
->expects($this->exactly(2)) ->expects($this->exactly(2))
->method('importEntry') ->method('updateEntry')
->willReturn(new Entry($this->user)); ->willReturn(new Entry($this->user));
// check that every entry persisted are archived // check that every entry persisted are archived
@ -165,7 +165,7 @@ class WallabagV2ImportTest extends \PHPUnit_Framework_TestCase
$this->contentProxy $this->contentProxy
->expects($this->never()) ->expects($this->never())
->method('importEntry'); ->method('updateEntry');
$producer = $this->getMockBuilder('OldSound\RabbitMqBundle\RabbitMq\Producer') $producer = $this->getMockBuilder('OldSound\RabbitMqBundle\RabbitMq\Producer')
->disableOriginalConstructor() ->disableOriginalConstructor()
@ -201,7 +201,7 @@ class WallabagV2ImportTest extends \PHPUnit_Framework_TestCase
$this->contentProxy $this->contentProxy
->expects($this->never()) ->expects($this->never())
->method('importEntry'); ->method('updateEntry');
$factory = new RedisMockFactory(); $factory = new RedisMockFactory();
$redisMock = $factory->getAdapter('Predis\Client', true); $redisMock = $factory->getAdapter('Predis\Client', true);
@ -278,7 +278,7 @@ class WallabagV2ImportTest extends \PHPUnit_Framework_TestCase
$this->contentProxy $this->contentProxy
->expects($this->exactly(2)) ->expects($this->exactly(2))
->method('importEntry') ->method('updateEntry')
->will($this->throwException(new \Exception())); ->will($this->throwException(new \Exception()));
$res = $wallabagV2Import->import(); $res = $wallabagV2Import->import();