From fb28fadcf6e71b4b797aa4241436ec3add543ba0 Mon Sep 17 00:00:00 2001 From: Bernhard Posselt Date: Tue, 21 Oct 2014 23:45:06 +0200 Subject: get rid of faviconfetcher and use picofeed --- CHANGELOG.md | 3 + README.md | 2 +- appinfo/application.php | 47 +++++- config/config.php | 16 +- controller/admincontroller.php | 6 +- fetcher/feedfetcher.php | 8 +- tests/unit/config/ConfigTest.php | 10 +- tests/unit/controller/AdminControllerTest.php | 6 +- tests/unit/fetcher/FeedFetcherTest.php | 8 +- tests/unit/utility/FaviconFetcherTest.php | 201 -------------------------- utility/faviconfetcher.php | 164 --------------------- 11 files changed, 76 insertions(+), 395 deletions(-) delete mode 100644 tests/unit/utility/FaviconFetcherTest.php delete mode 100644 utility/faviconfetcher.php diff --git a/CHANGELOG.md b/CHANGELOG.md index cd820ccd2..6fa8c3cbd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,6 @@ +owncloud-news (4.001) +* **Enhancement**: Use cacheDuration instead of simplePieCacheDuration in config.ini + owncloud-news (3.406) * **Enhancement**: Make config.ini editable in the admin interface diff --git a/README.md b/README.md index 0e2d14e74..3bbbae61d 100644 --- a/README.md +++ b/README.md @@ -166,7 +166,7 @@ The configuration is in **INI** format and looks like this: ```ini autoPurgeMinimumInterval = 60 autoPurgeCount = 200 -simplePieCacheDuration = 1800 +cacheDuration = 1800 feedFetcherTimeout = 60 useCronUpdates = true ``` diff --git a/appinfo/application.php b/appinfo/application.php index 27ced3c71..bef11dd44 100644 --- a/appinfo/application.php +++ b/appinfo/application.php @@ -13,6 +13,12 @@ namespace OCA\News\AppInfo; +require_once __DIR__ . '/autoload.php'; + +use \PicoFeed\Reader as PicoFeedReader; +use \PicoFeed\Config as PicoFeedConfig; +use \PicoFeed\Favicon; + use \OC\Files\View; use \OCP\AppFramework\App; use \OCP\Util; @@ -55,7 +61,6 @@ use \OCA\News\ArticleEnhancer\XPathArticleEnhancer; use \OCA\News\ArticleEnhancer\RegexArticleEnhancer; -require_once __DIR__ . '/autoload.php'; class Application extends App { @@ -415,7 +420,43 @@ class Application extends App { * Fetchers */ $container->registerService('PicoFeedConfig', function($c) { + // FIXME: move this into a separate class for testing? + $config = $c->query('Config'); + $appConfig = $c->query('AppConfig'); + + $pico = new PicoFeedConfig(); + $pico->setClientUserAgent( + 'ownCloud News/' . $appConfig->getConfig('version') . + ' (+https://owncloud.org/; 1 subscriber;)' + ) + ->setClientTimeout($config->getFeedFetcherTimeout()) + ->setMaxRedirections(10) + ->setContentFiltering(false); + + // proxy settings + $proxy = \OCP\Config::getSystemValue('proxy'); + if ($proxy) { + // we need to filter out the port -.- + $url = new \Net_URL2($proxy); + $port = $url->getPort(); + $url->setPort(false); + $host = $url->getUrl(); + + if ($port) { + $pico->setProxyPort($port); + } + + $pico->setProxyHostname($host); + } + + $proxyAuth = \OCP\Config::getSystemValue('proxyuserpwd'); + if ($proxyAuth) { + $auth = explode(':', $proxyAuth, 2); + $pico->setProxyUsername($auth[0]) + ->setProxyPassword($auth[1]); + } + return $pico; }); $container->registerService('Fetcher', function($c) { @@ -459,8 +500,8 @@ class Application extends App { }); $container->registerService('FaviconFetcher', function($c) { - return new FaviconFetcher( - $c->query('SimplePieAPIFactory') + return new Favicon( + $c->query('PicoFeedConfig') ); }); diff --git a/config/config.php b/config/config.php index 44650c3ac..73a930f97 100644 --- a/config/config.php +++ b/config/config.php @@ -24,7 +24,7 @@ class Config { // should still be kept for an // undo actions private $autoPurgeCount; // number of allowed unread articles per feed - private $simplePieCacheDuration; // seconds + private $cacheDuration; // seconds private $feedFetcherTimeout; // seconds private $useCronUpdates; // turn off updates run by owncloud cronjob private $proxyHost; @@ -39,7 +39,7 @@ class Config { $this->fileSystem = $fileSystem; $this->autoPurgeMinimumInterval = 60; $this->autoPurgeCount = 200; - $this->simplePieCacheDuration = 30*60; + $this->cacheDuration = 30*60; $this->feedFetcherTimeout = 60; $this->useCronUpdates = true; $this->logger = $logger; @@ -88,8 +88,8 @@ class Config { } - public function getSimplePieCacheDuration() { - return $this->simplePieCacheDuration; + public function getCacheDuration() { + return $this->cacheDuration; } @@ -113,8 +113,8 @@ class Config { } - public function setSimplePieCacheDuration($value) { - $this->simplePieCacheDuration = $value; + public function setCacheDuration($value) { + $this->cacheDuration = $value; } @@ -187,8 +187,8 @@ class Config { $this->autoPurgeMinimumInterval . "\n" . 'autoPurgeCount = ' . $this->autoPurgeCount . "\n" . - 'simplePieCacheDuration = ' . - $this->simplePieCacheDuration . "\n" . + 'cacheDuration = ' . + $this->cacheDuration . "\n" . 'feedFetcherTimeout = ' . $this->feedFetcherTimeout . "\n" . 'useCronUpdates = ' . diff --git a/controller/admincontroller.php b/controller/admincontroller.php index 71bcf0d29..39d75d9d2 100644 --- a/controller/admincontroller.php +++ b/controller/admincontroller.php @@ -38,7 +38,7 @@ class AdminController extends Controller { 'autoPurgeMinimumInterval' => $this->config->getAutoPurgeMinimumInterval(), 'autoPurgeCount' => $this->config->getAutoPurgeCount(), - 'cacheDuration' => $this->config->getSimplePieCacheDuration(), + 'cacheDuration' => $this->config->getCacheDuration(), 'feedFetcherTimeout' => $this->config->getFeedFetcherTimeout(), 'useCronUpdates' => $this->config->getUseCronUpdates(), ]; @@ -59,7 +59,7 @@ class AdminController extends Controller { $useCronUpdates) { $this->config->setAutoPurgeMinimumInterval($autoPurgeMinimumInterval); $this->config->setAutoPurgeCount($autoPurgeCount); - $this->config->setSimplePieCacheDuration($cacheDuration); + $this->config->setCacheDuration($cacheDuration); $this->config->setFeedFetcherTimeout($feedFetcherTimeout); $this->config->setUseCronUpdates($useCronUpdates); $this->config->write($this->configPath); @@ -68,7 +68,7 @@ class AdminController extends Controller { 'autoPurgeMinimumInterval' => $this->config->getAutoPurgeMinimumInterval(), 'autoPurgeCount' => $this->config->getAutoPurgeCount(), - 'cacheDuration' => $this->config->getSimplePieCacheDuration(), + 'cacheDuration' => $this->config->getCacheDuration(), 'feedFetcherTimeout' => $this->config->getFeedFetcherTimeout(), 'useCronUpdates' => $this->config->getUseCronUpdates(), ]; diff --git a/fetcher/feedfetcher.php b/fetcher/feedfetcher.php index c658dd36c..68036cc95 100644 --- a/fetcher/feedfetcher.php +++ b/fetcher/feedfetcher.php @@ -13,6 +13,8 @@ namespace OCA\News\Fetcher; +use \PicoFeed\Favicon; + use \OCA\News\Db\Item; use \OCA\News\Db\Feed; use \OCA\News\Utility\FaviconFetcher; @@ -35,13 +37,13 @@ class FeedFetcher implements IFeedFetcher { private $appConfig; public function __construct(SimplePieAPIFactory $simplePieFactory, - FaviconFetcher $faviconFetcher, + Favicon $faviconFetcher, $time, $cacheDirectory, Config $config, AppConfig $appConfig){ $this->cacheDirectory = $cacheDirectory; - $this->cacheDuration = $config->getSimplePieCacheDuration(); + $this->cacheDuration = $config->getCacheDuration(); $this->fetchTimeout = $config->getFeedFetcherTimeout(); $this->faviconFetcher = $faviconFetcher; $this->simplePieFactory = $simplePieFactory; @@ -211,7 +213,7 @@ class FeedFetcher implements IFeedFetcher { if ($getFavicon) { // use the favicon from the page first since most feeds use a weird // image - $favicon = $this->faviconFetcher->fetch($feed->getLink()); + $favicon = $this->faviconFetcher->find($feed->getLink()); if (!$favicon) { $favicon = $simplePieFeed->get_image_url(); diff --git a/tests/unit/config/ConfigTest.php b/tests/unit/config/ConfigTest.php index 2032c2169..9aaac57c5 100644 --- a/tests/unit/config/ConfigTest.php +++ b/tests/unit/config/ConfigTest.php @@ -42,7 +42,7 @@ class ConfigTest extends \PHPUnit_Framework_TestCase { public function testDefaults() { $this->assertEquals(60, $this->config->getAutoPurgeMinimumInterval()); $this->assertEquals(200, $this->config->getAutoPurgeCount()); - $this->assertEquals(30*60, $this->config->getSimplePieCacheDuration()); + $this->assertEquals(30*60, $this->config->getCacheDuration()); $this->assertEquals(60, $this->config->getFeedFetcherTimeout()); $this->assertEquals(true, $this->config->getUseCronUpdates()); $this->assertEquals(8080, $this->config->getProxyPort()); @@ -128,7 +128,7 @@ class ConfigTest extends \PHPUnit_Framework_TestCase { public function testWrite () { $json = 'autoPurgeMinimumInterval = 60' . "\n" . 'autoPurgeCount = 3' . "\n" . - 'simplePieCacheDuration = 1800' . "\n" . + 'cacheDuration = 1800' . "\n" . 'feedFetcherTimeout = 60' . "\n" . 'useCronUpdates = true' . "\n" . 'proxyHost = yo man' . "\n" . @@ -165,7 +165,7 @@ class ConfigTest extends \PHPUnit_Framework_TestCase { $json = 'autoPurgeMinimumInterval = 60' . "\n" . 'autoPurgeCount = 200' . "\n" . - 'simplePieCacheDuration = 1800' . "\n" . + 'cacheDuration = 1800' . "\n" . 'feedFetcherTimeout = 60' . "\n" . 'useCronUpdates = false' . "\n" . 'proxyHost = ' . "\n" . @@ -206,8 +206,8 @@ class ConfigTest extends \PHPUnit_Framework_TestCase { } public function testCacheDuration() { - $this->config->setSimplePieCacheDuration(21); - $duration = $this->config->getSimplePieCacheDuration(); + $this->config->setCacheDuration(21); + $duration = $this->config->getCacheDuration(); $this->assertSame(21, $duration); } diff --git a/tests/unit/controller/AdminControllerTest.php b/tests/unit/controller/AdminControllerTest.php index 61e510c24..b468823da 100644 --- a/tests/unit/controller/AdminControllerTest.php +++ b/tests/unit/controller/AdminControllerTest.php @@ -56,7 +56,7 @@ class AdminControllerTest extends \PHPUnit_Framework_TestCase { ->method('getAutoPurgeCount') ->will($this->returnValue($expected['autoPurgeCount'])); $this->config->expects($this->once()) - ->method('getSimplePieCacheDuration') + ->method('getCacheDuration') ->will($this->returnValue($expected['cacheDuration'])); $this->config->expects($this->once()) ->method('getFeedFetcherTimeout') @@ -92,7 +92,7 @@ class AdminControllerTest extends \PHPUnit_Framework_TestCase { ->method('setAutoPurgeCount') ->with($this->equalTo($expected['autoPurgeCount'])); $this->config->expects($this->once()) - ->method('setSimplePieCacheDuration') + ->method('setCacheDuration') ->with($this->equalTo($expected['cacheDuration'])); $this->config->expects($this->once()) ->method('setFeedFetcherTimeout') @@ -111,7 +111,7 @@ class AdminControllerTest extends \PHPUnit_Framework_TestCase { ->method('getAutoPurgeCount') ->will($this->returnValue($expected['autoPurgeCount'])); $this->config->expects($this->once()) - ->method('getSimplePieCacheDuration') + ->method('getCacheDuration') ->will($this->returnValue($expected['cacheDuration'])); $this->config->expects($this->once()) ->method('getFeedFetcherTimeout') diff --git a/tests/unit/fetcher/FeedFetcherTest.php b/tests/unit/fetcher/FeedFetcherTest.php index 2d31fb18a..ff1581082 100644 --- a/tests/unit/fetcher/FeedFetcherTest.php +++ b/tests/unit/fetcher/FeedFetcherTest.php @@ -83,7 +83,7 @@ class FeedFetcherTest extends \PHPUnit_Framework_TestCase { ->disableOriginalConstructor() ->getMock(); $this->faviconFetcher = $this->getMockBuilder( - '\OCA\News\Utility\FaviconFetcher') + '\PicoFeed\Favicon') ->disableOriginalConstructor() ->getMock(); $this->appconfig = $this->getMockBuilder( @@ -106,7 +106,7 @@ class FeedFetcherTest extends \PHPUnit_Framework_TestCase { ->disableOriginalConstructor() ->getMock(); $this->config->expects($this->any()) - ->method('getSimplePieCacheDuration') + ->method('getCacheDuration') ->will($this->returnValue($this->cacheDuration)); $this->config->expects($this->any()) ->method('getProxyHost') @@ -317,7 +317,7 @@ class FeedFetcherTest extends \PHPUnit_Framework_TestCase { if($hasWebFavicon) { $this->faviconFetcher->expects($this->once()) - ->method('fetch') + ->method('find') ->with($this->equalTo($this->feedLink)) ->will($this->returnValue($this->webFavicon)); $feed->setFaviconLink($this->webFavicon); @@ -425,7 +425,7 @@ class FeedFetcherTest extends \PHPUnit_Framework_TestCase { ->will($this->returnValue(true)); $this->faviconFetcher->expects($this->once()) - ->method('fetch') + ->method('find') ->will($this->returnValue($this->webFavicon)); $item = $this->createItem(false, 'video/ogg'); diff --git a/tests/unit/utility/FaviconFetcherTest.php b/tests/unit/utility/FaviconFetcherTest.php deleted file mode 100644 index 4ee823636..000000000 --- a/tests/unit/utility/FaviconFetcherTest.php +++ /dev/null @@ -1,201 +0,0 @@ - - * @author Bernhard Posselt - * @copyright Alessandro Cosentino 2012 - * @copyright Bernhard Posselt 2012, 2014 - */ - - -namespace OCA\News\Utility; - - -class FaviconFetcherTest extends \PHPUnit_Framework_TestCase { - - private $fetcher; - private $fileFactory; - private $png; - - protected function setUp(){ - $this->png = "\x89\x50\x4E\x47\x0D\x0A\x1A\x0A"; - $this->fileFactory = $this->getMockBuilder( - '\OCA\News\Utility\SimplePieAPIFactory') - ->disableOriginalConstructor() - ->getMock(); - $this->config = $this->getMockBuilder( - '\OCA\News\Utility\Config') - ->disableOriginalConstructor() - ->getMock(); - $this->fetcher = new FaviconFetcher($this->fileFactory); - } - - - protected function getFileMock($body='') { - $mock = $this->getMockBuilder('\SimplePie_File') - ->disableOriginalConstructor() - ->getMock(); - $mock->body = $body; - return $mock; - } - - - protected function getFileMockCallback($onEqual, $returnMock) { - $defaultMock = $this->getFileMock(); - - return function($url) use ($onEqual, $returnMock, $defaultMock) { - if($url === $onEqual){ - return $returnMock; - } else { - return $defaultMock; - } - }; - } - - - public function testFetchNoResponseReturnsNull() { - $mock = $this->getFileMock(); - - $this->fileFactory->expects($this->any()) - ->method('getFile') - ->will($this->returnValue($mock)); - - $favicon = $this->fetcher->fetch('dfdf'); - $this->assertNull($favicon); - } - - - public function testNoProxySettingsAreUsed() { - $faviconPath = "/owncloud/core/img/favicon.png"; - $html = $this->getFaviconHTML($faviconPath); - - $url = 'http://google.com'; - $pageMock = $this->getFileMock($html); - $pngMock = $this->getFileMock($this->png); - - $this->fileFactory->expects($this->at(0)) - ->method('getFile') - ->with($this->equalTo('http://google.com')) - ->will($this->returnValue($pageMock)); - - $this->fileFactory->expects($this->at(1)) - ->method('getFile') - ->with($this->equalTo( - 'http://google.com/owncloud/core/img/favicon.png'), - $this->equalTo(10), - $this->equalTo(5), - $this->equalTo(null), - $this->equalTo('Mozilla/5.0 AppleWebKit')) - ->will($this->returnValue($pngMock)); - - $favicon = $this->fetcher->fetch($url); - - $this->assertEquals('http://google.com/owncloud/core/img/favicon.png', - $favicon); - } - - - public function testFetchFaviconFaviconDotIcoHttp(){ - $url = ' sub.google.com '; - $mock = $this->getFileMock($this->png); - - $callback = $this->getFileMockCallback( - 'http://sub.google.com/favicon.ico', $mock); - - $this->fileFactory->expects($this->any()) - ->method('getFile') - ->will($this->returnCallback($callback)); - - $favicon = $this->fetcher->fetch($url); - - $this->assertEquals('http://sub.google.com/favicon.ico', $favicon); - } - - - public function testFetchFaviconFaviconDotIcoHttpBaseUrl(){ - $url = 'https://google.com/sometetst/dfladsf'; - $mock = $this->getFileMock($this->png); - - $callback = $this->getFileMockCallback( - 'https://google.com/favicon.ico', $mock); - - $this->fileFactory->expects($this->any()) - ->method('getFile') - ->will($this->returnCallback($callback)); - - $favicon = $this->fetcher->fetch($url); - - $this->assertEquals('https://google.com/favicon.ico', $favicon); - } - - - private function getFaviconHTML($faviconPath) { - return " - - - - - "; - } - - - public function testIconAbspathHTTP() { - $faviconPath = "/owncloud/core/img/favicon.png"; - $html = $this->getFaviconHTML($faviconPath); - - $url = 'http://google.com'; - $pageMock = $this->getFileMock($html); - $pngMock = $this->getFileMock($this->png); - - $this->fileFactory->expects($this->at(0)) - ->method('getFile') - ->with($this->equalTo('http://google.com')) - ->will($this->returnValue($pageMock)); - - $this->fileFactory->expects($this->at(1)) - ->method('getFile') - ->with($this->equalTo( - 'http://google.com/owncloud/core/img/favicon.png')) - ->will($this->returnValue($pngMock)); - - $favicon = $this->fetcher->fetch($url); - - $this->assertEquals('http://google.com/owncloud/core/img/favicon.png', - $favicon); - } - - - public function testEmptyFilePathDoesNotOpenFile() { - $url = ''; - - $this->fileFactory->expects($this->never()) - ->method('getFile'); - - $this->fetcher->fetch($url); - } - - public function testInvalidHostnameDoesNotOpenFile() { - $url = "a.b_c.de"; - - $this->fileFactory->expects($this->never()) - ->method('getFile'); - - $this->fetcher->fetch($url); - } - - - public function testInvalidHostnameDoesNotOpenFileHttp() { - $url = "http://a.b_c.de"; - - $this->fileFactory->expects($this->never()) - ->method('getFile'); - - $this->fetcher->fetch($url); - } - - -} diff --git a/utility/faviconfetcher.php b/utility/faviconfetcher.php deleted file mode 100644 index b837db91c..000000000 --- a/utility/faviconfetcher.php +++ /dev/null @@ -1,164 +0,0 @@ - - * @author Bernhard Posselt - * @copyright Alessandro Cosentino 2012 - * @copyright Bernhard Posselt 2012, 2014 - */ - -namespace OCA\News\Utility; - -use \ZendXml\Security; - - -class FaviconFetcher { - - private $apiFactory; - - /** - * Inject a factory to build a simplepie file object. This is needed because - * the file object contains logic in its constructor which makes it - * impossible to inject and test - */ - public function __construct(SimplePieAPIFactory $apiFactory) { - $this->apiFactory = $apiFactory; - } - - - /** - * Fetches a favicon from a given URL - * - * @param string|null $url the url where to fetch it from - * @return null|string - */ - public function fetch($url) { - try { - $url = $this->buildURL($url); - } catch (NoValidUrlException $e) { - return null; - } - - $faviconUrl = $this->extractFromPage($url); - - // check the url for a valid image - if($faviconUrl && $this->isImage($faviconUrl)) { - return $faviconUrl; - } elseif ($url) { - $base = new \Net_URL2($url); - $faviconUrl = (string) $base->resolve('/favicon.ico'); - - if($this->isImage($faviconUrl)) { - return $faviconUrl; - } - } - - return null; - } - - /** - * Get the attribute if its a DOMElement, otherwise return null - */ - private function getAttribute($item, $name) { - // used to fix scrutinizer errors - if ($item instanceof \DOMElement) { - return $item->getAttribute($name); - } else { - return null; - } - } - - - /** - * Tries to get a favicon from a page - * @param string $url the url to the page - * @return string the full url to the page - */ - protected function extractFromPage($url) { - if(!$url) { - return null; - } - - $file = $this->getFile($url); - - /** @noinspection PhpUndefinedFieldInspection */ - if($file->body !== '') { - $dom = new \DOMDocument(); - - Security::scan($file->body, $dom, function ($xml, $dom) { - return @$dom->loadHTML($xml, LIBXML_NONET); - }); - - if($dom) { - $xpath = new \DOMXpath($dom); - $elements = $xpath->query("//link[contains(@rel, 'icon')]"); - - if ($elements->length > 0) { - /** @noinspection PhpUndefinedMethodInspection */ - $iconPath = $this->getAttribute($elements->item(0), 'href'); - $base = new \Net_URL2($url); - $absPath = (string) $base->resolve($iconPath, $url); - return $absPath; - } - } - } - - return null; - } - - - private function getFile($url) { - return $this->apiFactory->getFile( - $url, 10, 5, null, 'Mozilla/5.0 AppleWebKit' - ); - } - - - /** - * Test if the file is an image - * @param string $url the url to the file - * @return bool true if image - */ - protected function isImage($url) { - // check for empty urls - if(!$url) { - return false; - } - - $file = $this->getFile($url); - /** @noinspection PhpParamsInspection */ - $sniffer = new \SimplePie_Content_Type_Sniffer($file); - return $sniffer->image() !== false; - } - - - /** - * Get HTTP or HTTPS addresses from an incomplete URL - * @param string $url the url that should be built - * @return string a string containing the http or https address - * @throws NoValidUrlException when no valid url can be returned - */ - protected function buildURL($url) { - // trim the right / from the url - $url = trim($url); - $url = rtrim($url, '/'); - - // check for http:// or https:// and validate URL - if (strpos($url, 'http://') === 0 || strpos($url, 'https://') === 0) { - if (filter_var($url, FILTER_VALIDATE_URL)) { - return $url; - } - } elseif (filter_var("http://" . $url, FILTER_VALIDATE_URL)) { - // maybe $url was something like www.example.com - return 'http://' . $url; - } - - // no valid URL was passed in or could be built from $url - throw new NoValidUrlException(); - } - -} -- cgit v1.2.3