diff options
-rw-r--r-- | CHANGELOG | 4 | ||||
-rw-r--r-- | businesslayer/feedbusinesslayer.php | 18 | ||||
-rw-r--r-- | dependencyinjection/dicontainer.php | 4 | ||||
-rw-r--r-- | tests/unit/businesslayer/FeedBusinessLayerTest.php | 28 | ||||
-rw-r--r-- | tests/unit/utility/FeedFetcherTest.php | 16 | ||||
-rw-r--r-- | tests/unit/utility/articleenhancer/ArticleEnhancerTest.php | 6 | ||||
-rw-r--r-- | tests/unit/utility/articleenhancer/DefaultEnhancerTest.php | 54 | ||||
-rw-r--r-- | tests/unit/utility/articleenhancer/EnhancerTest.php | 78 | ||||
-rw-r--r-- | utility/articleenhancer/articleenhancer.php | 32 | ||||
-rw-r--r-- | utility/articleenhancer/defaultenhancer.php | 49 | ||||
-rw-r--r-- | utility/articleenhancer/enhancer.php | 33 | ||||
-rw-r--r-- | utility/feedfetcher.php | 4 |
12 files changed, 110 insertions, 216 deletions
@@ -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) { |