summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--CHANGELOG4
-rw-r--r--businesslayer/feedbusinesslayer.php18
-rw-r--r--dependencyinjection/dicontainer.php4
-rw-r--r--tests/unit/businesslayer/FeedBusinessLayerTest.php28
-rw-r--r--tests/unit/utility/FeedFetcherTest.php16
-rw-r--r--tests/unit/utility/articleenhancer/ArticleEnhancerTest.php6
-rw-r--r--tests/unit/utility/articleenhancer/DefaultEnhancerTest.php54
-rw-r--r--tests/unit/utility/articleenhancer/EnhancerTest.php78
-rw-r--r--utility/articleenhancer/articleenhancer.php32
-rw-r--r--utility/articleenhancer/defaultenhancer.php49
-rw-r--r--utility/articleenhancer/enhancer.php33
-rw-r--r--utility/feedfetcher.php4
12 files changed, 110 insertions, 216 deletions
diff --git a/CHANGELOG b/CHANGELOG
index 25fb1b237..49e5b4be5 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,7 +1,7 @@
-owncloud-news (1.207)
+owncloud-news (1.401)
* Add possibility to hook up article enhancers which fetch article content directly from the web page
* Add article enhancer for explosm.net to directly fetch comics
-
+* Possible backwards incompatible change by using the link provided by simplepie instead of the user for the url hash. This prevents duplication of the feed when adding a slightly different feed url which points to the same feed and allows a speedup from O(n) to O(1) for article enhanchers
owncloud-news (1.206)
* Also handle URLErrors in updater script that are thrown when the domain of a feed is not found
diff --git a/businesslayer/feedbusinesslayer.php b/businesslayer/feedbusinesslayer.php
index 764cb07c6..cf79d0b22 100644
--- a/businesslayer/feedbusinesslayer.php
+++ b/businesslayer/feedbusinesslayer.php
@@ -95,14 +95,15 @@ class FeedBusinessLayer extends BusinessLayer {
public function create($feedUrl, $folderId, $userId){
// first try if the feed exists already
try {
- $this->mapper->findByUrlHash(md5($feedUrl), $userId);
- throw new BusinessLayerExistsException(
- $this->api->getTrans()->t('Can not add feed: Exists already'));
- } catch(DoesNotExistException $ex){}
-
- try {
list($feed, $items) = $this->feedFetcher->fetch($feedUrl);
+ // try again if feed exists depending on the reported link
+ try {
+ $this->mapper->findByUrlHash($feed->getUrlHash(), $userId);
+ throw new BusinessLayerExistsException(
+ $this->api->getTrans()->t('Can not add feed: Exists already'));
+ } catch(DoesNotExistException $ex){}
+
// insert feed
$feed->setFolderId($folderId);
$feed->setUserId($userId);
@@ -123,7 +124,7 @@ class FeedBusinessLayer extends BusinessLayer {
continue;
} catch(DoesNotExistException $ex){
$unreadCount += 1;
- $item = $this->enhancer->enhance($item);
+ $item = $this->enhancer->enhance($item, $feed->getLink());
$this->itemMapper->insert($item);
}
}
@@ -189,7 +190,8 @@ class FeedBusinessLayer extends BusinessLayer {
try {
$this->itemMapper->findByGuidHash($item->getGuidHash(), $feedId, $userId);
} catch(DoesNotExistException $ex){
- $item = $this->enhancer->enhance($item);
+ $item = $this->enhancer->enhance($item,
+ $existingFeed->getLink());
$this->itemMapper->insert($item);
}
}
diff --git a/dependencyinjection/dicontainer.php b/dependencyinjection/dicontainer.php
index 39f5a7be4..1d47181de 100644
--- a/dependencyinjection/dicontainer.php
+++ b/dependencyinjection/dicontainer.php
@@ -57,7 +57,6 @@ use \OCA\News\Utility\Updater;
use \OCA\News\Utility\SimplePieFileFactory;
use \OCA\News\Utility\ArticleEnhancer\Enhancer;
-use \OCA\News\Utility\ArticleEnhancer\DefaultEnhancer;
use \OCA\News\Utility\ArticleEnhancer\CyanideAndHappinessEnhancer;
@@ -234,8 +233,7 @@ class DIContainer extends BaseContainer {
// register fetchers in order
// the most generic enhancer should be the last one
- $enhancer->registerEnhancer($c['CyanideAndHappinessEnhancer']);
- $enhancer->registerEnhancer($c['DefaultEnhancer']);
+ $enhancer->registerEnhancer('explosm.net', $c['CyanideAndHappinessEnhancer']);
return $enhancer;
});
diff --git a/tests/unit/businesslayer/FeedBusinessLayerTest.php b/tests/unit/businesslayer/FeedBusinessLayerTest.php
index 7a4cf24e6..220b1c980 100644
--- a/tests/unit/businesslayer/FeedBusinessLayerTest.php
+++ b/tests/unit/businesslayer/FeedBusinessLayerTest.php
@@ -105,10 +105,6 @@ class FeedBusinessLayerTest extends \OCA\AppFramework\Utility\TestUtility {
$this->api->expects($this->once())
->method('getTrans')
->will($this->returnValue($trans));
- $this->feedMapper->expects($this->once())
- ->method('findByUrlHash')
- ->with($this->equalTo(md5($url)), $this->equalTo($this->user))
- ->will($this->throwException(new DoesNotExistException('yo')));
$this->fetcher->expects($this->once())
->method('fetch')
->with($this->equalTo($url))
@@ -123,6 +119,8 @@ class FeedBusinessLayerTest extends \OCA\AppFramework\Utility\TestUtility {
$createdFeed = new Feed();
$ex = new DoesNotExistException('yo');
$createdFeed->setUrl($url);
+ $createdFeed->setUrlHash('hsssi');
+ $createdFeed->setLink($url);
$item1 = new Item();
$item1->setGuidHash('hi');
$item2 = new Item();
@@ -134,7 +132,7 @@ class FeedBusinessLayerTest extends \OCA\AppFramework\Utility\TestUtility {
$this->feedMapper->expects($this->once())
->method('findByUrlHash')
- ->with($this->equalTo(md5($url)), $this->equalTo($this->user))
+ ->with($this->equalTo($createdFeed->getUrlHash()), $this->equalTo($this->user))
->will($this->throwException($ex));
$this->fetcher->expects($this->once())
->method('fetch')
@@ -153,7 +151,8 @@ class FeedBusinessLayerTest extends \OCA\AppFramework\Utility\TestUtility {
->will($this->throwException($ex));
$this->enhancer->expects($this->at(0))
->method('enhance')
- ->with($this->equalTo($return[1][1]))
+ ->with($this->equalTo($return[1][1]),
+ $this->equalTo($url))
->will($this->returnValue($return[1][1]));
$this->itemMapper->expects($this->at(1))
->method('insert')
@@ -167,7 +166,8 @@ class FeedBusinessLayerTest extends \OCA\AppFramework\Utility\TestUtility {
->will($this->throwException($ex));
$this->enhancer->expects($this->at(1))
->method('enhance')
- ->with($this->equalTo($return[1][0]))
+ ->with($this->equalTo($return[1][0]),
+ $this->equalTo($url))
->will($this->returnValue($return[1][0]));
$this->itemMapper->expects($this->at(3))
->method('insert')
@@ -183,9 +183,11 @@ class FeedBusinessLayerTest extends \OCA\AppFramework\Utility\TestUtility {
public function testCreateItemGuidExistsAlready(){
$url = 'http://test';
$folderId = 10;
- $createdFeed = new Feed();
$ex = new DoesNotExistException('yo');
+ $createdFeed = new Feed();
$createdFeed->setUrl($url);
+ $createdFeed->setUrlHash($url);
+ $createdFeed->setLink($url);
$item1 = new Item();
$item1->setGuidHash('hi');
$item2 = new Item();
@@ -197,7 +199,8 @@ class FeedBusinessLayerTest extends \OCA\AppFramework\Utility\TestUtility {
$this->feedMapper->expects($this->once())
->method('findByUrlHash')
- ->with($this->equalTo(md5($url)), $this->equalTo($this->user))
+ ->with($this->equalTo($createdFeed->getUrlHash()),
+ $this->equalTo($this->user))
->will($this->throwException($ex));
$this->fetcher->expects($this->once())
->method('fetch')
@@ -216,7 +219,8 @@ class FeedBusinessLayerTest extends \OCA\AppFramework\Utility\TestUtility {
->will($this->throwException($ex));
$this->enhancer->expects($this->at(0))
->method('enhance')
- ->with($this->equalTo($return[1][1]))
+ ->with($this->equalTo($return[1][1]),
+ $this->equalTo($url))
->will($this->returnValue($return[1][1]));
$this->itemMapper->expects($this->at(1))
->method('insert')
@@ -240,6 +244,7 @@ class FeedBusinessLayerTest extends \OCA\AppFramework\Utility\TestUtility {
$feed = new Feed();
$feed->setId(3);
$feed->getUrl('test');
+ $feed->setUrlHash('yo');
$item = new Item();
$item->setGuidHash(md5('hi'));
@@ -268,7 +273,8 @@ class FeedBusinessLayerTest extends \OCA\AppFramework\Utility\TestUtility {
->will($this->throwException($ex));
$this->enhancer->expects($this->at(0))
->method('enhance')
- ->with($this->equalTo($items[0]))
+ ->with($this->equalTo($items[0]),
+ $this->equalTo($feed->getUrl()))
->will($this->returnValue($items[0]));
$this->itemMapper->expects($this->once())
->method('insert')
diff --git a/tests/unit/utility/FeedFetcherTest.php b/tests/unit/utility/FeedFetcherTest.php
index eab57c3b4..af9de69e5 100644
--- a/tests/unit/utility/FeedFetcherTest.php
+++ b/tests/unit/utility/FeedFetcherTest.php
@@ -236,13 +236,13 @@ class FeedFetcherTest extends \OCA\AppFramework\Utility\TestUtility {
private function createFeed($hasFeedFavicon=false, $hasWebFavicon=false) {
$this->expectCore('get_title', $this->feedTitle);
- $this->expectCore('get_link', $this->feedLink);
+ $this->expectCore('get_permalink', $this->feedLink);
$feed = new Feed();
$feed->setTitle(html_entity_decode($this->feedTitle));
$feed->setUrl($this->url);
$feed->setLink($this->feedLink);
- $feed->setUrlHash(md5($this->url));
+ $feed->setUrlHash(md5($this->feedLink));
$feed->setAdded($this->time);
if($hasWebFavicon) {
@@ -281,13 +281,13 @@ class FeedFetcherTest extends \OCA\AppFramework\Utility\TestUtility {
public function testFetchMapItemsNoFeedTitleUsesUrl(){
$this->expectCore('get_title', '');
- $this->expectCore('get_link', $this->feedLink);
+ $this->expectCore('get_permalink', $this->feedLink);
$feed = new Feed();
$feed->setTitle($this->url);
$feed->setUrl($this->url);
$feed->setLink($this->feedLink);
- $feed->setUrlHash(md5($this->url));
+ $feed->setUrlHash(md5($this->feedLink));
$feed->setAdded($this->time);
$feed->setFaviconLink(null);
@@ -342,13 +342,13 @@ class FeedFetcherTest extends \OCA\AppFramework\Utility\TestUtility {
public function testFetchMapItemsGetFavicon() {
$this->expectCore('get_title', $this->feedTitle);
- $this->expectCore('get_link', $this->feedLink);
+ $this->expectCore('get_permalink', $this->feedLink);
$feed = new Feed();
$feed->setTitle(html_entity_decode($this->feedTitle));
$feed->setUrl($this->url);
$feed->setLink($this->feedLink);
- $feed->setUrlHash(md5($this->url));
+ $feed->setUrlHash(md5($this->feedLink));
$feed->setAdded($this->time);
$feed->setFaviconLink($this->webFavicon);
@@ -369,13 +369,13 @@ class FeedFetcherTest extends \OCA\AppFramework\Utility\TestUtility {
public function testFetchMapItemsNoGetFavicon() {
$this->expectCore('get_title', $this->feedTitle);
- $this->expectCore('get_link', $this->feedLink);
+ $this->expectCore('get_permalink', $this->feedLink);
$feed = new Feed();
$feed->setTitle(html_entity_decode($this->feedTitle));
$feed->setUrl($this->url);
$feed->setLink($this->feedLink);
- $feed->setUrlHash(md5($this->url));
+ $feed->setUrlHash(md5($this->feedLink));
$feed->setAdded($this->time);
$this->core->expects($this->once())
diff --git a/tests/unit/utility/articleenhancer/ArticleEnhancerTest.php b/tests/unit/utility/articleenhancer/ArticleEnhancerTest.php
index 8d507c0f8..c808a0e49 100644
--- a/tests/unit/utility/articleenhancer/ArticleEnhancerTest.php
+++ b/tests/unit/utility/articleenhancer/ArticleEnhancerTest.php
@@ -63,10 +63,10 @@ class ArticleEnhancerTest extends \OCA\AppFramework\Utility\TestUtility {
}
- public function testCanHandle() {
+ public function testDoesNotModifiyNotMatchingResults() {
$item = new Item();
- $item->setUrl('http://explosm.net/comics');
- $this->assertTrue($this->testEnhancer->canHandle($item));
+ $item->setUrl('http://explosm.net');
+ $this->assertEquals($item, $this->testEnhancer->enhance($item));
}
diff --git a/tests/unit/utility/articleenhancer/DefaultEnhancerTest.php b/tests/unit/utility/articleenhancer/DefaultEnhancerTest.php
deleted file mode 100644
index 901428616..000000000
--- a/tests/unit/utility/articleenhancer/DefaultEnhancerTest.php
+++ /dev/null
@@ -1,54 +0,0 @@
-<?php
-
-/**
-* ownCloud - News
-*
-* @author Alessandro Cosentino
-* @author Bernhard Posselt
-* @copyright 2012 Alessandro Cosentino cosenal@gmail.com
-* @copyright 2012 Bernhard Posselt dev@bernhard-posselt.com
-*
-* This library is free software; you can redistribute it and/or
-* modify it under the terms of the GNU AFFERO GENERAL PUBLIC LICENSE
-* License as published by the Free Software Foundation; either
-* version 3 of the License, or any later version.
-*
-* This library is distributed in the hope that it will be useful,
-* but WITHOUT ANY WARRANTY; without even the implied warranty of
-* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
-* GNU AFFERO GENERAL PUBLIC LICENSE for more details.
-*
-* You should have received a copy of the GNU Affero General Public
-* License along with this library. If not, see <http://www.gnu.org/licenses/>.
-*
-*/
-
-namespace OCA\News\Utility\ArticleEnhancer;
-
-use \OCA\News\Db\Item;
-
-require_once(__DIR__ . "/../../../classloader.php");
-
-
-class DefaultEnhancerTest extends \OCA\AppFramework\Utility\TestUtility {
-
- private $testEnhancer;
-
- protected function setUp() {
- $this->testEnhancer = new DefaultEnhancer();
- }
-
-
- public function testCanHandle() {
- $item = new Item();
- $this->assertTrue($this->testEnhancer->canHandle($item));
- }
-
-
- public function testEnhance() {
- $item = new Item();
- $this->assertEquals($item, $this->testEnhancer->enhance($item));
- }
-
-
-} \ No newline at end of file
diff --git a/tests/unit/utility/articleenhancer/EnhancerTest.php b/tests/unit/utility/articleenhancer/EnhancerTest.php
index 559722e60..769538740 100644
--- a/tests/unit/utility/articleenhancer/EnhancerTest.php
+++ b/tests/unit/utility/articleenhancer/EnhancerTest.php
@@ -42,67 +42,49 @@ class EnhancerTest extends \OCA\AppFramework\Utility\TestUtility {
'\OCA\News\Utility\ArticleEnhancer\ArticleEnhancer')
->disableOriginalConstructor()
->getMock();
- $this->articleEnhancer2 = $this->getMockBuilder(
- '\OCA\News\Utility\ArticleEnhancer\ArticleEnhancer')
- ->disableOriginalConstructor()
- ->getMock();
+ $this->enhancer->registerEnhancer('test.com', $this->articleEnhancer);
}
- public function testFetch(){
+ public function testEnhanceSetsCorrectHash(){
$item = new Item();
$item->setUrl('hi');
+ $urls = array(
+ 'https://test.com',
+ 'https://www.test.com',
+ 'https://test.com/',
+ 'http://test.com',
+ 'http://test.com/',
+ 'http://www.test.com'
+ );
+ for ($i=0; $i < count($urls); $i++) {
+ $url = $urls[$i];
+ $this->articleEnhancer->expects($this->at($i))
+ ->method('enhance')
+ ->with($this->equalTo($item))
+ ->will($this->returnValue($item));
+ }
+
+ for ($i=0; $i < count($urls); $i++) {
+ $url = $urls[$i];
+ $result = $this->enhancer->enhance($item, $url);
+ $this->assertEquals($item, $result);
+ }
- $this->articleEnhancer->expects($this->once())
- ->method('canHandle')
- ->with($this->equalTo($item))
- ->will($this->returnValue(true));
- $this->enhancer->registerEnhancer($this->articleEnhancer);
-
- $this->enhancer->enhance($item);
}
- public function testMultipleFetchers(){
+ public function testNotMatchShouldJustReturnItem() {
$item = new Item();
$item->setUrl('hi');
- $this->articleEnhancer->expects($this->once())
- ->method('canHandle')
- ->with($this->equalTo($item))
- ->will($this->returnValue(false));
- $this->articleEnhancer2->expects($this->once())
- ->method('canHandle')
- ->with($this->equalTo($item))
- ->will($this->returnValue(true));
-
- $this->enhancer->registerEnhancer($this->articleEnhancer);
- $this->enhancer->registerEnhancer($this->articleEnhancer2);
-
- $this->enhancer->enhance($item);
- }
+ $url = 'https://tests.com';
+ $this->articleEnhancer->expects($this->never())
+ ->method('enhance');
- public function testMultipleFetchersOnlyOneShouldHandle(){
- $item = new Item();
- $item->setUrl('hi');
- $return = 'zeas';
- $this->articleEnhancer->expects($this->once())
- ->method('canHandle')
- ->with($this->equalTo($item))
- ->will($this->returnValue(true));
- $this->articleEnhancer->expects($this->once())
- ->method('enhance')
- ->with($this->equalTo($item))
- ->will($this->returnValue($return));
- $this->articleEnhancer2->expects($this->never())
- ->method('canHandle');
-
- $this->enhancer->registerEnhancer($this->articleEnhancer);
- $this->enhancer->registerEnhancer($this->articleEnhancer2);
-
- $result = $this->enhancer->enhance($item);
-
- $this->assertEquals($return, $result);
+ $result = $this->enhancer->enhance($item, $url);
+ $this->assertEquals($item, $result);
+
}
diff --git a/utility/articleenhancer/articleenhancer.php b/utility/articleenhancer/articleenhancer.php
index d7701d53b..194137e72 100644
--- a/utility/articleenhancer/articleenhancer.php
+++ b/utility/articleenhancer/articleenhancer.php
@@ -60,27 +60,23 @@ abstract class ArticleEnhancer {
}
- public function canHandle($item){
- return preg_match($this->articleUrlRegex, $item->getUrl()) == true;
- }
-
-
public function enhance($item){
- $file = $this->fileFactory->getFile($item->getUrl(), $this->maximumTimeout);
- $dom = new \DOMDocument();
- @$dom->loadHTML($file->body);
- $xpath = new \DOMXpath($dom);
- $xpathResult = $xpath->evaluate($this->articleXPath);
-
- // in case it wasnt a text query assume its a single
- if(!is_string($xpathResult)) {
- $xpathResult = $this->domToString($xpathResult);
+ if(preg_match($this->articleUrlRegex, $item->getUrl())) {
+ $file = $this->fileFactory->getFile($item->getUrl(), $this->maximumTimeout);
+ $dom = new \DOMDocument();
+ @$dom->loadHTML($file->body);
+ $xpath = new \DOMXpath($dom);
+ $xpathResult = $xpath->evaluate($this->articleXPath);
+
+ // in case it wasnt a text query assume its a single
+ if(!is_string($xpathResult)) {
+ $xpathResult = $this->domToString($xpathResult);
+ }
+
+ $sanitizedResult = $this->purifier->purify($xpathResult);
+ $item->setBody($sanitizedResult);
}
- $sanitizedResult = $this->purifier->purify($xpathResult);
- $item->setBody($sanitizedResult);
-
-
return $item;
}
diff --git a/utility/articleenhancer/defaultenhancer.php b/utility/articleenhancer/defaultenhancer.php
deleted file mode 100644
index eb3045ceb..000000000
--- a/utility/articleenhancer/defaultenhancer.php
+++ /dev/null
@@ -1,49 +0,0 @@
-<?php
-
-/**
-* ownCloud - News
-*
-* @author Alessandro Cosentino
-* @author Bernhard Posselt
-* @copyright 2012 Alessandro Cosentino cosenal@gmail.com
-* @copyright 2012 Bernhard Posselt dev@bernhard-posselt.com
-*
-* This library is free software; you can redistribute it and/or
-* modify it under the terms of the GNU AFFERO GENERAL PUBLIC LICENSE
-* License as published by the Free Software Foundation; either
-* version 3 of the License, or any later version.
-*
-* This library is distributed in the hope that it will be useful,
-* but WITHOUT ANY WARRANTY; without even the implied warranty of
-* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
-* GNU AFFERO GENERAL PUBLIC LICENSE for more details.
-*
-* You should have received a copy of the GNU Affero General Public
-* License along with this library. If not, see <http://www.gnu.org/licenses/>.
-*
-*/
-
-namespace OCA\News\Utility\ArticleEnhancer;
-
-use \OCA\News\Utility\SimplePieFileFactory;
-
-
-class DefaultEnhancer extends ArticleEnhancer {
-
-
- public function __construct(){
- parent::__construct(null, new SimplePieFileFactory(), null, null, null);
- }
-
-
- public function canHandle($item){
- return true;
- }
-
-
- public function enhance($item){
- return $item;
- }
-
-
-} \ No newline at end of file
diff --git a/utility/articleenhancer/enhancer.php b/utility/articleenhancer/enhancer.php
index 059904f63..d7d96f6a9 100644
--- a/utility/articleenhancer/enhancer.php
+++ b/utility/articleenhancer/enhancer.php
@@ -28,23 +28,36 @@ namespace OCA\News\Utility\ArticleEnhancer;
class Enhancer {
- private $enhancers;
+ private $enhancers = array();
- public function __construct(){
- $this->enhancers = array();
+ public function registerEnhancer($feedUrl, ArticleEnhancer $enhancer){
+ $feedUrl = $this->removeTrailingSlash($feedUrl);
+
+ // create hashkeys for all supported protocols for quick access
+ $this->enhancers[$feedUrl] = $enhancer;
+ $this->enhancers['https://' . $feedUrl] = $enhancer;
+ $this->enhancers['http://' . $feedUrl] = $enhancer;
+ $this->enhancers['https://www.' . $feedUrl] = $enhancer;
+ $this->enhancers['http://www.' . $feedUrl] = $enhancer;
}
- public function registerEnhancer(ArticleEnhancer $enhancer){
- array_push($this->enhancers, $enhancer);
+ public function enhance($item, $feedUrl){
+ $feedUrl = $this->removeTrailingSlash($feedUrl);
+
+ if(array_key_exists($feedUrl, $this->enhancers)) {
+ return $this->enhancers[$feedUrl]->enhance($item);
+ } else {
+ return $item;
+ }
}
- public function enhance($item){
- foreach($this->enhancers as $enhancer){
- if($enhancer->canHandle($item)){
- return $enhancer->enhance($item);
- }
+ private function removeTrailingSlash($url) {
+ if($url[strlen($url)-1] === '/') {
+ return substr($url, 0, -1);
+ } else {
+ return $url;
}
}
diff --git a/utility/feedfetcher.php b/utility/feedfetcher.php
index 10a141e38..8ad800d3c 100644
--- a/utility/feedfetcher.php
+++ b/utility/feedfetcher.php
@@ -187,8 +187,8 @@ class FeedFetcher implements IFeedFetcher {
$feed->setTitle($title);
$feed->setUrl($url);
- $feed->setLink($simplePieFeed->get_link());
- $feed->setUrlHash(md5($url));
+ $feed->setLink($simplePieFeed->get_permalink());
+ $feed->setUrlHash(md5($feed->getLink()));
$feed->setAdded($this->time->getTime());
if ($getFavicon) {