From 19910df42e93ef370048e8d0d2609af71bf46676 Mon Sep 17 00:00:00 2001 From: Bernhard Posselt Date: Mon, 22 Sep 2014 12:14:23 +0200 Subject: fix #454, allow global enhancers --- CHANGELOG.md | 2 + README.md | 2 +- appinfo/application.php | 7 +++ articleenhancer/enhancer.php | 21 +++++++- articleenhancer/globalarticleenhancer.php | 56 ++++++++++++++++++++++ tests/unit/articleenhancer/EnhancerTest.php | 35 ++++++++++++-- .../articleenhancer/GlobalArticleEnhancerTest.php | 49 +++++++++++++++++++ tests/unit/utility/ConfigTest.php | 45 ++++++++++------- utility/config.php | 12 +++-- 9 files changed, 201 insertions(+), 28 deletions(-) create mode 100644 articleenhancer/globalarticleenhancer.php create mode 100644 tests/unit/articleenhancer/GlobalArticleEnhancerTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 7c4f0f722..e4eb463a6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,8 @@ owncloud-news (3.003) * **Enhancement**: Less padding right on mobile phone * **Enhancement**: Expanded view: remove date on mobile phone * **Enhancement**: Compact view: click on title should remove ellipsis +* **Enhancement**: Ignore autoPurgeMinimumInterval setting if it is below 60 seconds since anything lower than that may hurt user experience +* **Enhancement**: Remove YouTube autoplay from all articles owncloud-news (3.002) * **Bugfix**: If a folder is selected, the f and d shortcuts will jump to the previous or next folder subfeeds diff --git a/README.md b/README.md index cc603add3..8773f212b 100644 --- a/README.md +++ b/README.md @@ -157,7 +157,7 @@ proxyPassword = ``` -* **autoPurgeMinimumInterval**: Minimum amount of seconds after deleted feeds and folders are removed from the database. +* **autoPurgeMinimumInterval**: Minimum amount of seconds after deleted feeds and folders are removed from the database. Values below 60 seconds are ignored * **autoPurgeCount**: Defines the minimum amount of articles that can be unread per feed before they get deleted * **simplePieCacheDuration**: Amount of seconds to cache feeds * **feedFetcherTimeout**: Maximum number of seconds to wait for an RSS or Atom feed to load. If a feed takes longer than that number of seconds to update, the update will be aborted diff --git a/appinfo/application.php b/appinfo/application.php index 5b3414a52..37dace4c4 100644 --- a/appinfo/application.php +++ b/appinfo/application.php @@ -49,6 +49,7 @@ use \OCA\News\Fetcher\Fetcher; use \OCA\News\Fetcher\FeedFetcher; use \OCA\News\ArticleEnhancer\Enhancer; +use \OCA\News\ArticleEnhancer\GlobalArticleEnhancer; use \OCA\News\ArticleEnhancer\XPathArticleEnhancer; use \OCA\News\ArticleEnhancer\RegexArticleEnhancer; @@ -350,6 +351,10 @@ class Application extends App { return new \HTMLPurifier($config); }); + $container->registerService('GlobalArticleEnhancer', function($c) { + return new GlobalArticleEnhancer(); + }); + $container->registerService('Enhancer', function($c) { $enhancer = new Enhancer(); @@ -377,6 +382,8 @@ class Application extends App { } } + $enhancer->registerGlobalEnhancer($c->query('GlobalArticleEnhancer')); + return $enhancer; }); diff --git a/articleenhancer/enhancer.php b/articleenhancer/enhancer.php index 9aa2153d1..e0ad2ab2d 100644 --- a/articleenhancer/enhancer.php +++ b/articleenhancer/enhancer.php @@ -17,6 +17,7 @@ namespace OCA\News\ArticleEnhancer; class Enhancer { private $enhancers = []; + private $globalEnhancers = []; /** * @param string $feedUrl @@ -34,6 +35,16 @@ class Enhancer { } + /** + * Registers enhancers that are run for every item and after all previous + * enhancers have been run + * @param ArticleEnhancer $enhancer + */ + public function registerGlobalEnhancer (ArticleEnhancer $enhancer) { + $this->globalEnhancers[] = $enhancer; + } + + /** * @param \OCA\News\Db\Item $item * @param string $feedUrl @@ -43,10 +54,16 @@ class Enhancer { $feedUrl = $this->removeTrailingSlash($feedUrl); if(array_key_exists($feedUrl, $this->enhancers)) { - return $this->enhancers[$feedUrl]->enhance($item); + $result = $this->enhancers[$feedUrl]->enhance($item); } else { - return $item; + $result = $item; + } + + foreach ($this->globalEnhancers as $enhancer) { + $result = $enhancer->enhance($result); } + + return $result; } diff --git a/articleenhancer/globalarticleenhancer.php b/articleenhancer/globalarticleenhancer.php new file mode 100644 index 000000000..cfefb882b --- /dev/null +++ b/articleenhancer/globalarticleenhancer.php @@ -0,0 +1,56 @@ + + * @author Bernhard Posselt + * @copyright Alessandro Cosentino 2012 + * @copyright Bernhard Posselt 2012, 2014 + */ + +namespace OCA\News\ArticleEnhancer; + +use \OCA\News\Db\Item; + + +class GlobalArticleEnhancer implements ArticleEnhancer { + + + /** + * This method is run after all enhancers and for every item + */ + public function enhance(Item $item) { + $dom = new \DOMDocument(); + @$dom->loadHTML($item->getBody(), LIBXML_HTML_NOIMPLIED | + LIBXML_HTML_NODEFDTD); + $xpath = new \DOMXpath($dom); + + // remove youtube autoplay + // NOTE: PHP supports only XPath 1.0 so no matches() function :( + $youtubeIframes = "//iframe[contains(@src, 'youtube.com')]"; + + $elements = $xpath->query($youtubeIframes); + foreach ($elements as $element) { + + // src needs to be matched against regex to prevent false positives + // and because theres no XPath matches function available + $src = $element->getAttribute('src'); + $regex = '%^(http://|https://|//)(www\.)?youtube.com/.*autoplay=1.*%i'; + + if (preg_match($regex, $src)) { + $replaced = str_replace('autoplay=1', 'autoplay=0', $src); + $element->setAttribute('src', $replaced); + } + } + + // save all changes back to the item + $item->setBody(trim($dom->saveHTML())); + + return $item; + } + + +} \ No newline at end of file diff --git a/tests/unit/articleenhancer/EnhancerTest.php b/tests/unit/articleenhancer/EnhancerTest.php index 129b88fd7..2223b1d06 100644 --- a/tests/unit/articleenhancer/EnhancerTest.php +++ b/tests/unit/articleenhancer/EnhancerTest.php @@ -16,6 +16,15 @@ namespace OCA\News\ArticleEnhancer; use \OCA\News\Db\Item; +class AddEnhancer implements ArticleEnhancer { + public function enhance(Item $item) { + $body = $item->getBody(); + $item->setBody($body += 1); + return $item; + } +} + + class EnhancerTest extends \PHPUnit_Framework_TestCase { private $enhancer; @@ -43,19 +52,19 @@ class EnhancerTest extends \PHPUnit_Framework_TestCase { 'http://test.com/', 'http://www.test.com' ]; - for ($i=0; $i < count($urls); $i++) { + for ($i=0; $i < count($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++) { + for ($i=0; $i < count($urls); $i++) { $url = $urls[$i]; $result = $this->enhancer->enhance($item, $url); $this->assertEquals($item, $result); } - + } @@ -67,10 +76,26 @@ class EnhancerTest extends \PHPUnit_Framework_TestCase { $this->articleEnhancer->expects($this->never()) ->method('enhance'); - $result = $this->enhancer->enhance($item, $url); + $result = $this->enhancer->enhance($item, $url); $this->assertEquals($item, $result); - } + public function testGlobalEnhancer() { + $this->enhancer->registerGlobalEnhancer( + new AddEnhancer() + ); + + $this->enhancer->registerGlobalEnhancer( + new AddEnhancer() + ); + + $item = new Item(); + $item->setBody(1); + + $result = $this->enhancer->enhance($item, 'test'); + + $this->assertEquals(3, $item->getBody()); + } + } \ No newline at end of file diff --git a/tests/unit/articleenhancer/GlobalArticleEnhancerTest.php b/tests/unit/articleenhancer/GlobalArticleEnhancerTest.php new file mode 100644 index 000000000..4b0db31a1 --- /dev/null +++ b/tests/unit/articleenhancer/GlobalArticleEnhancerTest.php @@ -0,0 +1,49 @@ + + * @author Bernhard Posselt + * @copyright Alessandro Cosentino 2012 + * @copyright Bernhard Posselt 2012, 2014 + */ + +namespace OCA\News\ArticleEnhancer; + +use \OCA\News\Db\Item; + + +class GlobalArticleEnhancerTest extends \PHPUnit_Framework_TestCase { + + private $enhancer; + + protected function setUp() { + $this->enhancer = new GlobalArticleEnhancer(); + } + + + public function testNoReplaceYoutubeAutoplay() { + $body = ''; + $expected = ''; + $item = new Item(); + $item->setBody($body); + + $result = $this->enhancer->enhance($item); + $this->assertEquals($expected, $result->getBody()); + } + + + public function testReplaceYoutubeAutoplay() { + $body = 'test '; + $expected = '

test

'; + $item = new Item(); + $item->setBody($body); + + $result = $this->enhancer->enhance($item); + $this->assertEquals($expected, $result->getBody()); + } + +} diff --git a/tests/unit/utility/ConfigTest.php b/tests/unit/utility/ConfigTest.php index 578949b4e..ad26035f5 100644 --- a/tests/unit/utility/ConfigTest.php +++ b/tests/unit/utility/ConfigTest.php @@ -59,11 +59,24 @@ class ConfigTest extends \PHPUnit_Framework_TestCase { $this->config->read($this->configPath); - $this->assertTrue(3 === $this->config->getAutoPurgeCount()); - $this->assertTrue(true === $this->config->getUseCronUpdates()); + $this->assertSame(3, $this->config->getAutoPurgeCount()); + $this->assertSame(true, $this->config->getUseCronUpdates()); } + public function testReadIgnoresVeryLowPurgeInterval () { + $this->fileSystem->expects($this->once()) + ->method('file_get_contents') + ->with($this->equalTo($this->configPath)) + ->will($this->returnValue("autoPurgeMinimumInterval = 59")); + + $this->config->read($this->configPath); + + $this->assertSame(60, $this->config->getAutoPurgeMinimumInterval()); + } + + + public function testReadBool () { $this->fileSystem->expects($this->once()) ->method('file_get_contents') @@ -72,8 +85,8 @@ class ConfigTest extends \PHPUnit_Framework_TestCase { $this->config->read($this->configPath); - $this->assertTrue(3 === $this->config->getAutoPurgeCount()); - $this->assertTrue(false === $this->config->getUseCronUpdates()); + $this->assertSame(3, $this->config->getAutoPurgeCount()); + $this->assertSame(false, $this->config->getUseCronUpdates()); } @@ -84,8 +97,8 @@ class ConfigTest extends \PHPUnit_Framework_TestCase { ->will($this->returnValue('autoPurgeCounts = 3')); $this->logger->expects($this->once()) ->method('warning') - ->with($this->equalTo('Configuration value "autoPurgeCounts" ' . - 'does not exist. Ignored value.'), + ->with($this->equalTo('Configuration value "autoPurgeCounts" ' . + 'does not exist. Ignored value.'), $this->equalTo($this->loggerParams)); $this->config->read($this->configPath); @@ -99,7 +112,7 @@ class ConfigTest extends \PHPUnit_Framework_TestCase { ->will($this->returnValue('')); $this->logger->expects($this->once()) ->method('warning') - ->with($this->equalTo('Configuration invalid. Ignoring values.'), + ->with($this->equalTo('Configuration invalid. Ignoring values.'), $this->equalTo($this->loggerParams)); $this->config->read($this->configPath); @@ -107,10 +120,10 @@ class ConfigTest extends \PHPUnit_Framework_TestCase { public function testWrite () { - $json = "autoPurgeMinimumInterval = 60\n" . - "autoPurgeCount = 3\n" . - "simplePieCacheDuration = 1800\n" . - "feedFetcherTimeout = 60\n" . + $json = "autoPurgeMinimumInterval = 60\n" . + "autoPurgeCount = 3\n" . + "simplePieCacheDuration = 1800\n" . + "feedFetcherTimeout = 60\n" . "useCronUpdates = true\n" . "proxyHost = yo man\n" . "proxyPort = 12\n" . @@ -141,13 +154,13 @@ class ConfigTest extends \PHPUnit_Framework_TestCase { ->method('file_exists') ->with($this->equalTo($this->configPath)) ->will($this->returnValue(false)); - + $this->config->setUseCronUpdates(false); - $json = "autoPurgeMinimumInterval = 60\n" . - "autoPurgeCount = 200\n" . - "simplePieCacheDuration = 1800\n" . - "feedFetcherTimeout = 60\n" . + $json = "autoPurgeMinimumInterval = 60\n" . + "autoPurgeCount = 200\n" . + "simplePieCacheDuration = 1800\n" . + "feedFetcherTimeout = 60\n" . "useCronUpdates = false\n" . "proxyHost = \n" . "proxyPort = 8080\n" . diff --git a/utility/config.php b/utility/config.php index 1e2719818..94e71442b 100644 --- a/utility/config.php +++ b/utility/config.php @@ -18,7 +18,7 @@ use \OCP\ILogger; class Config { - private $fileSystem; + private $fileSystem; private $autoPurgeMinimumInterval; // seconds, used to define how // long deleted folders and feeds // should still be kept for an @@ -75,7 +75,11 @@ class Config { } public function getAutoPurgeMinimumInterval() { - return $this->autoPurgeMinimumInterval; + if ($this->autoPurgeMinimumInterval > 60) { + return $this->autoPurgeMinimumInterval; + } else { + return 60; + } } @@ -162,7 +166,7 @@ class Config { settype($value, $type); $this->$key = $value; } else { - $this->logger->warning('Configuration value "' . $key . + $this->logger->warning('Configuration value "' . $key . '" does not exist. Ignored value.' , $this->loggerParams); } } @@ -173,7 +177,7 @@ class Config { public function write($configPath) { - $ini = + $ini = "autoPurgeMinimumInterval = " . $this->autoPurgeMinimumInterval . "\n" . "autoPurgeCount = " . $this->autoPurgeCount . "\n" . "simplePieCacheDuration = " . $this->simplePieCacheDuration . "\n" . -- cgit v1.2.3