use current date when feed does not provide pubdates, disable article updates

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('<a', '<a target="_blank"',
 		// escape XSS
 		$this->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();