From c69bcb8e68cd57a7771d03cd0e43c52cf1a26220 Mon Sep 17 00:00:00 2001 From: Sean Molenaar Date: Thu, 21 Mar 2019 05:16:35 +0100 Subject: Allow empty-ish lastmodified and clean up FeedFetcher Test (#458) --- composer.lock | 17 +- lib/Fetcher/FeedFetcher.php | 2 +- tests/Unit/Fetcher/FeedFetcherTest.php | 344 +++++++++++++++++++++------------ 3 files changed, 230 insertions(+), 133 deletions(-) diff --git a/composer.lock b/composer.lock index 7fe887ba5..9d0c34a1c 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "fe812fa2682a6ee30eb5ab53805f4aeb", + "content-hash": "34e3f8a6c35510d6e0f6977ddf4dc9c5", "packages": [ { "name": "arthurhoaro/favicon", @@ -1699,16 +1699,16 @@ }, { "name": "squizlabs/php_codesniffer", - "version": "3.4.0", + "version": "3.4.1", "source": { "type": "git", "url": "https://github.com/squizlabs/PHP_CodeSniffer.git", - "reference": "379deb987e26c7cd103a7b387aea178baec96e48" + "reference": "5b4333b4010625d29580eb4a41f1e53251be6baa" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/squizlabs/PHP_CodeSniffer/zipball/379deb987e26c7cd103a7b387aea178baec96e48", - "reference": "379deb987e26c7cd103a7b387aea178baec96e48", + "url": "https://api.github.com/repos/squizlabs/PHP_CodeSniffer/zipball/5b4333b4010625d29580eb4a41f1e53251be6baa", + "reference": "5b4333b4010625d29580eb4a41f1e53251be6baa", "shasum": "" }, "require": { @@ -1741,12 +1741,12 @@ } ], "description": "PHP_CodeSniffer tokenizes PHP, JavaScript and CSS files and detects violations of a defined set of coding standards.", - "homepage": "http://www.squizlabs.com/php-codesniffer", + "homepage": "https://github.com/squizlabs/PHP_CodeSniffer", "keywords": [ "phpcs", "standards" ], - "time": "2018-12-19T23:57:18+00:00" + "time": "2019-03-19T03:22:27+00:00" }, { "name": "symfony/polyfill-ctype", @@ -1906,7 +1906,8 @@ "platform": { "php": "^7.0", "ext-json": "*", - "ext-simplexml": "*" + "ext-simplexml": "*", + "ext-libxml": "*" }, "platform-dev": [] } diff --git a/lib/Fetcher/FeedFetcher.php b/lib/Fetcher/FeedFetcher.php index fe6910e5b..bcf6081e3 100755 --- a/lib/Fetcher/FeedFetcher.php +++ b/lib/Fetcher/FeedFetcher.php @@ -73,7 +73,7 @@ class FeedFetcher implements IFeedFetcher } $url = $url2->getNormalizedURL(); $this->reader->resetFilters(); - if (is_null($lastModified) || !is_string($lastModified)) { + if (empty($lastModified) || !is_string($lastModified)) { $resource = $this->reader->read($url); } else { $resource = $this->reader->readSince($url, new DateTime($lastModified)); diff --git a/tests/Unit/Fetcher/FeedFetcherTest.php b/tests/Unit/Fetcher/FeedFetcherTest.php index 0ae453c9c..5f3420810 100644 --- a/tests/Unit/Fetcher/FeedFetcherTest.php +++ b/tests/Unit/Fetcher/FeedFetcherTest.php @@ -13,6 +13,7 @@ namespace OCA\News\Tests\Unit\Fetcher; +use DateTime; use FeedIo\Feed\Item\Author; use FeedIo\Feed\Item\MediaInterface; use FeedIo\Feed\ItemInterface; @@ -132,16 +133,16 @@ class FeedFetcherTest extends TestCase protected function setUp() { - $this->l10n = $this->getMockBuilder(\OCP\IL10N::class) + $this->l10n = $this->getMockBuilder(\OCP\IL10N::class) ->disableOriginalConstructor() ->getMock(); - $this->reader = $this->getMockBuilder(\FeedIo\FeedIo::class) + $this->reader = $this->getMockBuilder(\FeedIo\FeedIo::class) ->disableOriginalConstructor() ->getMock(); - $this->favicon = $this->getMockBuilder(\Favicon\Favicon::class) + $this->favicon = $this->getMockBuilder(\Favicon\Favicon::class) ->disableOriginalConstructor() ->getMock(); - $this->result = $this->getMockBuilder(\FeedIo\Reader\Result::class) + $this->result = $this->getMockBuilder(\FeedIo\Reader\Result::class) ->disableOriginalConstructor() ->getMock(); $this->response = $this->getMockBuilder(\FeedIo\Adapter\ResponseInterface::class) @@ -163,7 +164,7 @@ class FeedFetcherTest extends TestCase $timeFactory->expects($this->any()) ->method('getTime') ->will($this->returnValue($this->time)); - $this->logger = $this->getMockBuilder(PsrLogger::class) + $this->logger = $this->getMockBuilder(PsrLogger::class) ->disableOriginalConstructor() ->getMock(); $this->fetcher = new FeedFetcher( @@ -173,33 +174,38 @@ class FeedFetcherTest extends TestCase $timeFactory, $this->logger ); - $this->url = 'http://tests/'; - - $this->permalink = 'http://permalink'; - $this->title = 'my&lt;' title'; - $this->guid = 'hey guid here'; - $this->guid_hash = 'df9a5f84e44bfe38cf44f6070d5b0250'; - $this->body = 'test]]>'; - $this->parsed_body = 'let the bodies hit the floor test'; - $this->pub = 23111; - $this->updated = 23444; - $this->author = new Author(); + $this->url = 'http://tests/'; + + $this->permalink = 'http://permalink'; + $this->title = 'my&lt;' title'; + $this->guid = 'hey guid here'; + $this->guid_hash = 'df9a5f84e44bfe38cf44f6070d5b0250'; + $this->body + = 'test]]>'; + $this->parsed_body + = 'let the bodies hit the floor test'; + $this->pub = 23111; + $this->updated = 23444; + $this->author = new Author(); $this->author->setName('<boogieman'); - $this->enclosure = 'http://enclosure.you'; + $this->enclosure = 'http://enclosure.you'; - $this->feed_title = '<a>&its a</a> title'; - $this->feed_link = 'http://tests/'; - $this->feed_image = '/an/image'; + $this->feed_title = '<a>&its a</a> title'; + $this->feed_link = 'http://tests/'; + $this->feed_image = '/an/image'; $this->web_favicon = 'http://anon.google.com'; - $this->modified = $this->getMockBuilder('\DateTime')->getMock(); + $this->modified = $this->getMockBuilder('\DateTime')->getMock(); $this->modified->expects($this->any()) ->method('getTimestamp') ->will($this->returnValue(3)); - $this->encoding = 'UTF-8'; - $this->language = 'de-DE'; - $this->rtl = false; + $this->encoding = 'UTF-8'; + $this->language = 'de-DE'; + $this->rtl = false; } + /** + * Test if the fetcher can handle a URL. + */ public function testCanHandle() { $url = 'google.de'; @@ -212,133 +218,183 @@ class FeedFetcherTest extends TestCase */ public function testNoFetchIfNotModified() { - $this->__setUpReader($this->url, false); + $this->setUpReader($this->url, '@0', false); $this->logger->expects($this->once()) ->method('debug') - ->with('Feed {url} was not modified since last fetch. old: {old}, new: {new}'); - $result = $this->fetcher->fetch($this->url, false, null, null, null); + ->with( + 'Feed {url} was not modified since last fetch. old: {old}, new: {new}' + ); + $result = $this->fetcher->fetch($this->url, false, '@0', null, null); + $this->assertSame([null, []], $result); } + /** + * Test if empty is logged when the feed remain the same. + */ + public function testFetchIfNoModifiedExists() + { + $this->setUpReader($this->url, null, true); + $item = $this->createItem(); + $feed = $this->createFeed(); + $this->mockIterator($this->feed_mock, [$this->item_mock]); + $result = $this->fetcher->fetch($this->url, false, '0', null, null); + + $this->assertEquals([$feed, [$item]], $result); + } + + /** + * Test if the fetch function fetches a feed. + */ public function testFetch() { - $this->__setUpReader($this->url); - $item = $this->_createItem(); - $feed = $this->_createFeed(); - $this->_mockIterator($this->feed_mock, [$this->item_mock]); - $result = $this->fetcher->fetch($this->url, false, null, null, null); + $this->setUpReader($this->url); + $item = $this->createItem(); + $feed = $this->createFeed(); + $this->mockIterator($this->feed_mock, [$this->item_mock]); + $result = $this->fetcher->fetch($this->url, false, '@1553118393', null, null); $this->assertEquals([$feed, [$item]], $result); } + /** + * Test if fetching a feed with authentication works. + */ public function testFetchAccount() { - $this->__setUpReader('http://account%40email.com:F9sEU%2ARt%25%3AKFK8HMHT%26@tests/'); - $item = $this->_createItem(); - $feed = $this->_createFeed('de-DE', false, 'http://account%40email.com:F9sEU%2ARt%25%3AKFK8HMHT%26@tests/'); - $this->_mockIterator($this->feed_mock, [$this->item_mock]); - $result = $this->fetcher->fetch($this->url, false, null, 'account@email.com', 'F9sEU*Rt%:KFK8HMHT&'); + $this->setUpReader('http://account%40email.com:F9sEU%2ARt%25%3AKFK8HMHT%26@tests/'); + $item = $this->createItem(); + $feed = $this->createFeed('de-DE', false, 'http://account%40email.com:F9sEU%2ARt%25%3AKFK8HMHT%26@tests/'); + $this->mockIterator($this->feed_mock, [$this->item_mock]); + $result = $this->fetcher->fetch($this->url, false, '@1553118393', 'account@email.com', 'F9sEU*Rt%:KFK8HMHT&'); $this->assertEquals([$feed, [$item]], $result); } - + /** + * Test if fetching a feed with an audio item works. + */ public function testAudioEnclosure() { - $this->__setUpReader($this->url); - $item = $this->_createItem('audio/ogg'); - $feed = $this->_createFeed(); - $this->_mockIterator($this->feed_mock, [$this->item_mock]); - $result = $this->fetcher->fetch($this->url, false, null, null, null); + $this->setUpReader($this->url); + $item = $this->createItem('audio/ogg'); + $feed = $this->createFeed(); + $this->mockIterator($this->feed_mock, [$this->item_mock]); + $result = $this->fetcher->fetch($this->url, false, '@1553118393', null, null); $this->assertEquals([$feed, [$item]], $result); } - + /** + * Test if fetching a feed with a video item works. + */ public function testVideoEnclosure() { - $this->__setUpReader($this->url); - $item = $this->_createItem('video/ogg'); - $feed = $this->_createFeed(); - $this->_mockIterator($this->feed_mock, [$this->item_mock]); - $result = $this->fetcher->fetch($this->url, false, null, null, null); + $this->setUpReader($this->url); + $item = $this->createItem('video/ogg'); + $feed = $this->createFeed(); + $this->mockIterator($this->feed_mock, [$this->item_mock]); + $result = $this->fetcher->fetch($this->url, false, '@1553118393', null, null); $this->assertEquals([$feed, [$item]], $result); } - public function testFavicon() + /** + * Test if fetching a feed with a favicon works. + */ + public function testFavicon() { - $this->__setUpReader($this->url); + $this->setUpReader($this->url); - $feed = $this->_createFeed('de-DE', true); - $item = $this->_createItem(); - $this->_mockIterator($this->feed_mock, [$this->item_mock]); - $result = $this->fetcher->fetch($this->url, true, null, null, null); + $feed = $this->createFeed('de-DE', true); + $item = $this->createItem(); + $this->mockIterator($this->feed_mock, [$this->item_mock]); + $result = $this->fetcher->fetch($this->url, true, '@1553118393', null, null); $this->assertEquals([$feed, [$item]], $result); } - public function testNoFavicon() + /** + * Test if fetching a feed without a favicon works. + */ + public function testNoFavicon() { - $this->__setUpReader($this->url); + $this->setUpReader($this->url); - $feed = $this->_createFeed(false); + $feed = $this->createFeed(false); $this->favicon->expects($this->never()) ->method('get'); - $item = $this->_createItem(); - $this->_mockIterator($this->feed_mock, [$this->item_mock]); - $result = $this->fetcher->fetch($this->url, false, null, null, null); + $item = $this->createItem(); + $this->mockIterator($this->feed_mock, [$this->item_mock]); + $result = $this->fetcher->fetch($this->url, false, '@1553118393', null, null); $this->assertEquals([$feed, [$item]], $result); } - public function testRtl() + /** + * Test if fetching a feed with a non-western language works. + */ + public function testRtl() { - $this->__setUpReader($this->url); - $this->_createFeed('he-IL'); - $this->_createItem(); - $this->_mockIterator($this->feed_mock, [$this->item_mock]); - list($feed, $items) = $this->fetcher->fetch($this->url, false, null, null, null); + $this->setUpReader($this->url); + $this->createFeed('he-IL'); + $this->createItem(); + $this->mockIterator($this->feed_mock, [$this->item_mock]); + list($feed, $items) = $this->fetcher->fetch($this->url, false, '@1553118393', null, null); $this->assertTrue($items[0]->getRtl()); } + /** + * Test if fetching a feed with a RSS pubdate works and sets the property. + */ public function testRssPubDate() { - $this->__setUpReader($this->url); - $this->_createFeed('he-IL'); - $this->_createItem(); + $this->setUpReader($this->url); + $this->createFeed('he-IL'); + $this->createItem(); $this->item_mock->expects($this->exactly(2)) ->method('getValue') - ->will($this->returnValueMap([ - ['pubDate', '2018-03-27T19:50:29Z'], - ['published', NULL], - ])); + ->will( + $this->returnValueMap( + [ + ['pubDate', '2018-03-27T19:50:29Z'], + ['published', null], + ] + ) + ); - $this->_mockIterator($this->feed_mock, [$this->item_mock]); - list($feed, $items) = $this->fetcher->fetch($this->url, false, null, null, null); + $this->mockIterator($this->feed_mock, [$this->item_mock]); + list($feed, $items) = $this->fetcher->fetch($this->url, false, '@1553118393', null, null); $this->assertSame($items[0]->getPubDate(), 1522180229); } + /** + * Test if fetching a feed with a Atom pubdate works and sets the property. + */ public function testAtomPubDate() { - $this->__setUpReader($this->url); - $this->_createFeed('he-IL'); - $this->_createItem(); + $this->setUpReader($this->url); + $this->createFeed('he-IL'); + $this->createItem(); $this->item_mock->expects($this->exactly(3)) - ->method('getValue') - ->will($this->returnValueMap([ - ['pubDate', NULL], - ['published', '2018-02-27T19:50:29Z'], - ])); + ->method('getValue') + ->will( + $this->returnValueMap( + [ + ['pubDate', null], + ['published', '2018-02-27T19:50:29Z'], + ] + ) + ); - $this->_mockIterator($this->feed_mock, [$this->item_mock]); - list($feed, $items) = $this->fetcher->fetch($this->url, false, null, null, null); + $this->mockIterator($this->feed_mock, [$this->item_mock]); + list($feed, $items) = $this->fetcher->fetch($this->url, false, '@1553118393', null, null); $this->assertSame($items[0]->getPubDate(), 1519761029); } @@ -350,7 +406,7 @@ class FeedFetcherTest extends TestCase * * @return mixed */ - private function _mockIterator($iteratorMock, array $items) + private function mockIterator($iteratorMock, array $items) { $iteratorData = new \stdClass(); $iteratorData->array = $items; @@ -419,18 +475,33 @@ class FeedFetcherTest extends TestCase return $iteratorMock; } - private function __setUpReader($url = '', $modified = true) + /** + * Set up a FeedIO mock instance + * + * @param string $url URL that will be read. + * @param string $modifiedDate Date of last fetch + * @param bool $modified If the feed will be modified + */ + private function setUpReader($url = '', $modifiedDate = '@1553118393', $modified = true) { - $this->reader->expects($this->once()) - ->method('read') - ->with($this->equalTo($url)) - ->will($this->returnValue($this->result)); + if (is_null($modifiedDate)) { + $this->reader->expects($this->once()) + ->method('read') + ->with($url) + ->will($this->returnValue($this->result)); + } else { + $this->reader->expects($this->once()) + ->method('readSince') + ->with($url, new DateTime($modifiedDate)) + ->will($this->returnValue($this->result)); + } + $this->result->expects($this->once()) ->method('getResponse') ->will($this->returnValue($this->response)); $this->response->expects($this->once()) ->method('isModified') - ->will($this->returnValue($modified)); + ->will($this->returnValue($modified !== false)); $this->location = $url; if (!$modified) { @@ -444,32 +515,35 @@ class FeedFetcherTest extends TestCase ->method('getFeed') ->will($this->returnValue($this->feed_mock)); } - - } - - private function _expectFeed($method, $return, $count = 1) - { - $this->feed_mock->expects($this->exactly($count)) - ->method($method) - ->will($this->returnValue($return)); - } - - private function _expectItem($method, $return, $count = 1) - { - $this->item_mock->expects($this->exactly($count)) - ->method($method) - ->will($this->returnValue($return)); } - - private function _createItem($enclosureType=null) + /** + * Create an item mock. + * + * @param string|null $enclosureType Media type. + * + * @return Item + */ + private function createItem($enclosureType = null) { - $this->_expectItem('getLink', $this->permalink); - $this->_expectItem('getTitle', $this->title); - $this->_expectItem('getPublicId', $this->guid); - $this->_expectItem('getDescription', $this->body); - $this->_expectItem('getLastModified', $this->modified); - $this->_expectItem('getAuthor', $this->author); + $this->item_mock->expects($this->exactly(1)) + ->method('getLink') + ->will($this->returnValue($this->permalink)); + $this->item_mock->expects($this->exactly(1)) + ->method('getTitle') + ->will($this->returnValue($this->title)); + $this->item_mock->expects($this->exactly(1)) + ->method('getPublicId') + ->will($this->returnValue($this->guid)); + $this->item_mock->expects($this->exactly(1)) + ->method('getDescription') + ->will($this->returnValue($this->body)); + $this->item_mock->expects($this->exactly(1)) + ->method('getLastModified') + ->will($this->returnValue($this->modified)); + $this->item_mock->expects($this->exactly(1)) + ->method('getAuthor') + ->will($this->returnValue($this->author)); $item = new Item(); @@ -497,8 +571,14 @@ class FeedFetcherTest extends TestCase $media2->expects($this->once()) ->method('getUrl') ->will($this->returnValue($this->enclosure)); - $this->_expectItem('hasMedia', true); - $this->_expectItem('getMedias', [$media, $media2]); + + $this->item_mock->expects($this->exactly(1)) + ->method('hasMedia') + ->will($this->returnValue(true)); + + $this->item_mock->expects($this->exactly(1)) + ->method('getMedias') + ->will($this->returnValue([$media, $media2])); $item->setEnclosureMime($enclosureType); $item->setEnclosureLink($this->enclosure); @@ -508,14 +588,30 @@ class FeedFetcherTest extends TestCase return $item; } - - private function _createFeed($lang='de-DE', $favicon=false, $url= null) + /** + * Create a mock feed. + * + * @param string $lang Feed language. + * @param bool $favicon Fetch favicon. + * @param string|null $url Feed URL. + * + * @return Feed + */ + private function createFeed($lang = 'de-DE', $favicon = false, $url = null) { $url = $url ?? $this->url; - $this->_expectFeed('getTitle', $this->feed_title, 2); - $this->_expectFeed('getLink', $this->feed_link); - $this->_expectFeed('getLastModified', $this->modified); - $this->_expectFeed('getLanguage', $lang); + $this->feed_mock->expects($this->exactly(2)) + ->method('getTitle') + ->will($this->returnValue($this->feed_title)); + $this->feed_mock->expects($this->exactly(1)) + ->method('getLink') + ->will($this->returnValue($this->feed_link)); + $this->feed_mock->expects($this->exactly(1)) + ->method('getLastModified') + ->will($this->returnValue($this->modified)); + $this->feed_mock->expects($this->exactly(1)) + ->method('getLanguage') + ->will($this->returnValue($lang)); $feed = new Feed(); -- cgit v1.2.3