From 9807ee7d6b003b9dbef5eb4c78136f0e72d753ed Mon Sep 17 00:00:00 2001 From: Bernhard Posselt Date: Sat, 22 Jun 2013 15:02:33 +0200 Subject: use current date when feed does not provide pubdates, disable article updates --- CHANGELOG | 2 + businesslayer/feedbusinesslayer.php | 19 +-------- tests/unit/businesslayer/FeedBusinessLayerTest.php | 49 ---------------------- tests/unit/utility/FeedFetcherTest.php | 28 +++++++++++-- utility/feedfetcher.php | 10 ++++- 5 files changed, 37 insertions(+), 71 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 38e2aa806..29c1685e0 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -8,6 +8,8 @@ ownCloud-news (1.201) * Increased allowed feed timeout from 10 to 60 seconds * Make it possible for plugins to turn off mark read on scroll * Removed HTML Purifier unit tests which made it possible to trigger XSS using a crafted URL +* Don't update existing articles anymore if the pubdate changes to prevent weird update behaviour +* If articles dont provide a pubdate, use the date when the article was saved in the database ownCloud-news (1.001) * Also use monospace for pre tag diff --git a/businesslayer/feedbusinesslayer.php b/businesslayer/feedbusinesslayer.php index 05448d7f6..2d61dfe61 100644 --- a/businesslayer/feedbusinesslayer.php +++ b/businesslayer/feedbusinesslayer.php @@ -171,24 +171,7 @@ class FeedBusinessLayer extends BusinessLayer { $item->setFeedId($existingFeed->getId()); try { - $existing = $this->itemMapper->findByGuidHash( - $item->getGuidHash(), $feedId, $userId); - - // in case of an update the existing item has to be deleted - // if the pub_date changed because we sort by id on the - // client side since this is the only reliable way to do it - // to not get weird behaviour - if((int)$existing->getPubDate() !== (int)$item->getPubDate()){ - - // because the item is being replaced we need to keep - // status flags but we want the new entry to be unread - $item->setStatus($existing->getStatus()); - $item->setUnread(); - - $this->itemMapper->delete($existing); - $this->itemMapper->insert($item); - } - + $this->itemMapper->findByGuidHash($item->getGuidHash(), $feedId, $userId); } catch(DoesNotExistException $ex){ $this->itemMapper->insert($item); } diff --git a/tests/unit/businesslayer/FeedBusinessLayerTest.php b/tests/unit/businesslayer/FeedBusinessLayerTest.php index 9f8683d9b..0e3532f16 100644 --- a/tests/unit/businesslayer/FeedBusinessLayerTest.php +++ b/tests/unit/businesslayer/FeedBusinessLayerTest.php @@ -400,55 +400,6 @@ class FeedBusinessLayerTest extends \OCA\AppFramework\Utility\TestUtility { $this->assertEquals($return, $feed); } - public function testUpdateUpdatesEntry(){ - $feed = new Feed(); - $feed->setId(3); - $feed->getUrl('test'); - - $item = new Item(); - $item->setGuidHash(md5('hi')); - $item->setPubDate(3333); - $items = array( - $item - ); - - $item2 = new Item(); - $item2->setPubDate(111); - - $fetchReturn = array($feed, $items); - - $this->feedMapper->expects($this->at(0)) - ->method('find') - ->with($this->equalTo($feed->getId()), - $this->equalTo($this->user)) - ->will($this->returnValue($feed)); - $this->fetcher->expects($this->once()) - ->method('fetch') - ->will($this->returnValue($fetchReturn)); - $this->itemMapper->expects($this->once()) - ->method('findByGuidHash') - ->with($this->equalTo($item->getGuidHash()), - $this->equalTo($feed->getId()), - $this->equalTo($this->user)) - ->will($this->returnValue($item2)); - $this->itemMapper->expects($this->at(1)) - ->method('delete') - ->with($this->equalTo($item2)); - $this->itemMapper->expects($this->at(2)) - ->method('insert') - ->with($this->equalTo($item)); - - $this->feedMapper->expects($this->at(1)) - ->method('find') - ->with($feed->getId(), $this->user) - ->will($this->returnValue($feed)); - - $return = $this->feedBusinessLayer->update($feed->getId(), $this->user); - - $this->assertEquals($return, $feed); - $this->assertTrue($item->isUnread()); - } - public function testCreateUpdateFails(){ $feed = new Feed(); diff --git a/tests/unit/utility/FeedFetcherTest.php b/tests/unit/utility/FeedFetcherTest.php index 94339fe96..1ea06e2ad 100644 --- a/tests/unit/utility/FeedFetcherTest.php +++ b/tests/unit/utility/FeedFetcherTest.php @@ -173,7 +173,7 @@ class FeedFetcherTest extends \OCA\AppFramework\Utility\TestUtility { } - private function createItem($author=false, $enclosureType=null) { + private function createItem($author=false, $enclosureType=null, $noPubDate=false) { $this->purifier->expects($this->once()) ->method('purify') ->with($this->equalTo($this->body)) @@ -182,9 +182,17 @@ class FeedFetcherTest extends \OCA\AppFramework\Utility\TestUtility { $this->expectItem('get_title', $this->title); $this->expectItem('get_id', $this->guid); $this->expectItem('get_content', $this->body); - $this->expectItem('get_date', $this->pub); $item = new Item(); + + if($noPubDate) { + $this->expectItem('get_date', 0); + $item->setPubDate($this->time); + } else { + $this->expectItem('get_date', $this->pub); + $item->setPubDate($this->pub); + } + $item->setStatus(0); $item->setUnread(); $item->setUrl($this->permalink); @@ -192,7 +200,6 @@ class FeedFetcherTest extends \OCA\AppFramework\Utility\TestUtility { $item->setGuid($this->guid); $item->setGuidHash(md5($this->guid)); $item->setBody($this->body2); - $item->setPubDate($this->pub); $item->setLastModified($this->time); if($author) { $mock = $this->getMock('author', array('get_name')); @@ -319,6 +326,20 @@ class FeedFetcherTest extends \OCA\AppFramework\Utility\TestUtility { $this->assertEquals(array($feed, array($item)), $result); } + + public function testFetchMapItemsNoPubdate(){ + $this->core->expects($this->once()) + ->method('init') + ->will($this->returnValue(true)); + $item = $this->createItem(false, true, true); + $feed = $this->createFeed(false, true); + $this->expectCore('get_items', array($this->item)); + $result = $this->fetcher->fetch($this->url); + + $this->assertEquals(array($feed, array($item)), $result); + } + + public function testFetchMapItemsGetFavicon() { $this->expectCore('get_title', $this->feedTitle); $this->expectCore('get_link', $this->feedLink); @@ -371,4 +392,5 @@ class FeedFetcherTest extends \OCA\AppFramework\Utility\TestUtility { $this->assertEquals(array($feed, array($item)), $result); } + } diff --git a/utility/feedfetcher.php b/utility/feedfetcher.php index fc301b31e..4706f7612 100644 --- a/utility/feedfetcher.php +++ b/utility/feedfetcher.php @@ -126,7 +126,15 @@ class FeedFetcher implements IFeedFetcher { $item->setBody(str_replace('purifier->purify($simplePieItem->get_content()))); - $item->setPubDate($simplePieItem->get_date('U')); + + // pubdate is not required. if not given use the current date + $date = $simplePieItem->get_date('U'); + if(!$date) { + $date = $this->time->getTime(); + } + + $item->setPubDate($date); + $item->setLastModified($this->time->getTime()); $author = $simplePieItem->get_author(); -- cgit v1.2.3