From ee861fa6dd80378be60cb76d2fb28c5e0e808ab6 Mon Sep 17 00:00:00 2001 From: Bernhard Posselt Date: Sat, 7 Sep 2013 23:00:34 +0200 Subject: extra work for postgres, fix #338 --- CHANGELOG | 3 + appinfo/info.xml | 2 +- appinfo/version | 2 +- db/itemmapper.php | 2 - db/mapperfactory.php | 52 ++++++++ db/postgres/itemmapper.php | 80 +++++++++++++ dependencyinjection/dicontainer.php | 7 +- tests/unit/db/ItemMapperTest.php | 3 - tests/unit/db/MapperFactoryTest.php | 75 ++++++++++++ tests/unit/db/postgres/ItemMapperTest.php | 191 ++++++++++++++++++++++++++++++ 10 files changed, 409 insertions(+), 8 deletions(-) create mode 100644 db/mapperfactory.php create mode 100644 db/postgres/itemmapper.php create mode 100644 tests/unit/db/MapperFactoryTest.php create mode 100644 tests/unit/db/postgres/ItemMapperTest.php diff --git a/CHANGELOG b/CHANGELOG index f9c9b5c9b..aa7015dc6 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,3 +1,6 @@ +owncloud-news (1.404) +* Fix bug on postgres databases that would not delete old articles + owncloud-news (1.403) * Respect encoding in feed enhancers * Hotfix for update on posgresql databases diff --git a/appinfo/info.xml b/appinfo/info.xml index 714ec16ec..15ddf6ee5 100644 --- a/appinfo/info.xml +++ b/appinfo/info.xml @@ -5,6 +5,6 @@ An RSS/Atom feed reader. Requires the App Framework app and backgroundjobs need to be enabled. See the README.rst in the apps top directory AGPL Alessandro Cosentino, Bernhard Posselt, Jan-Christoph Borchardt. Powered by SimplePie (Ryan Parman, Geoffrey Sneddon, Ryan McCue and contributors). - 1.403 + 1.404 5.0.6 diff --git a/appinfo/version b/appinfo/version index dd2037e0e..9539f398d 100644 --- a/appinfo/version +++ b/appinfo/version @@ -1 +1 @@ -1.403 \ No newline at end of file +1.404 \ No newline at end of file diff --git a/db/itemmapper.php b/db/itemmapper.php index eaf24dc49..0a78b02df 100644 --- a/db/itemmapper.php +++ b/db/itemmapper.php @@ -257,7 +257,6 @@ class ItemMapper extends Mapper implements IMapper { $params = array($status, $threshold); $result = $this->execute($sql, $params); - /* FIXME: this is broken on posgres while($row = $result->fetchRow()) { $size = (int) $row['size']; @@ -274,7 +273,6 @@ class ItemMapper extends Mapper implements IMapper { $this->execute($sql, $params, $limit); } } - */ } diff --git a/db/mapperfactory.php b/db/mapperfactory.php new file mode 100644 index 000000000..cf64f90d6 --- /dev/null +++ b/db/mapperfactory.php @@ -0,0 +1,52 @@ +. +* +*/ + +namespace OCA\News\Db; + +use \OCA\AppFramework\Core\API; + + +class MapperFactory { + + private $api; + + public function __construct(API $api) { + $this->api = $api; + } + + + public function getItemMapper() { + switch($this->api->getSystemValue('dbtype')) { + case 'pgsql': + return new \OCA\News\Db\Postgres\ItemMapper($this->api); + break; + default: + return new ItemMapper($this->api); + break; + } + } + + +} \ No newline at end of file diff --git a/db/postgres/itemmapper.php b/db/postgres/itemmapper.php new file mode 100644 index 000000000..ddf83ae10 --- /dev/null +++ b/db/postgres/itemmapper.php @@ -0,0 +1,80 @@ +. +* +*/ + +namespace OCA\News\Db\Postgres; + +use \OCA\AppFramework\Db\DoesNotExistException; +use \OCA\AppFramework\Db\MultipleObjectsReturnedException; +use \OCA\AppFramework\Db\Mapper; +use \OCA\AppFramework\Core\API; + +use \OCA\News\Db\StatusFlag; + + +class ItemMapper extends \OCA\News\Db\ItemMapper { + + public function __construct(API $api){ + parent::__construct($api); + } + + + /** + * Delete all items for feeds that have over $threshold unread and not + * starred items + */ + public function deleteReadOlderThanThreshold($threshold){ + $status = StatusFlag::STARRED | StatusFlag::UNREAD; + $sql = 'SELECT COUNT(*) `size`, `feed_id` ' . + 'FROM `*PREFIX*news_items` ' . + 'WHERE NOT ((`status` & ?) > 0) ' . + 'GROUP BY `feed_id` ' . + 'HAVING COUNT(*) > ?'; + $params = array($status, $threshold); + $result = $this->execute($sql, $params); + + while($row = $result->fetchRow()) { + + $size = (int) $row['size']; + $limit = $size - $threshold; + + if($limit > 0) { + $params = array($status, $row['feed_id'], $limit); + + $sql = 'DELETE FROM `*PREFIX*news_items` ' . + 'WHERE `id` IN (' . + 'SELECT `id` FROM `*PREFIX*news_items` ' . + 'WHERE NOT ((`status` & ?) > 0) ' . + 'AND `feed_id` = ? ' . + 'ORDER BY `id` ASC ' . + 'LIMIT ?' . + ')'; + + $this->execute($sql, $params); + } + } + + } + + +} \ No newline at end of file diff --git a/dependencyinjection/dicontainer.php b/dependencyinjection/dicontainer.php index 4d0d2d7ad..8ad816f8d 100644 --- a/dependencyinjection/dicontainer.php +++ b/dependencyinjection/dicontainer.php @@ -45,6 +45,7 @@ use \OCA\News\Db\FolderMapper; use \OCA\News\Db\FeedMapper; use \OCA\News\Db\ItemMapper; use \OCA\News\Db\StatusFlag; +use \OCA\News\Db\MapperFactory; use \OCA\News\External\NewsAPI; use \OCA\News\External\FolderAPI; @@ -208,6 +209,10 @@ class DIContainer extends BaseContainer { /** * MAPPERS */ + $this['MapperFactory'] = $this->share(function($c){ + return new MapperFactory($c['API']); + }); + $this['FolderMapper'] = $this->share(function($c){ return new FolderMapper($c['API']); }); @@ -217,7 +222,7 @@ class DIContainer extends BaseContainer { }); $this['ItemMapper'] = $this->share(function($c){ - return new ItemMapper($c['API']); + return $c['MapperFactory']->getItemMapper($c['API']); }); diff --git a/tests/unit/db/ItemMapperTest.php b/tests/unit/db/ItemMapperTest.php index 23896727e..eb04b1514 100644 --- a/tests/unit/db/ItemMapperTest.php +++ b/tests/unit/db/ItemMapperTest.php @@ -331,9 +331,6 @@ class ItemMapperTest extends \OCA\AppFramework\Utility\MapperTestUtility { public function testDeleteReadOlderThanThreshold(){ - $this->markTestIncomplete( - 'Fix on postgres first' - ); $threshold = 10; $status = StatusFlag::STARRED | StatusFlag::UNREAD; diff --git a/tests/unit/db/MapperFactoryTest.php b/tests/unit/db/MapperFactoryTest.php new file mode 100644 index 000000000..c1be2095b --- /dev/null +++ b/tests/unit/db/MapperFactoryTest.php @@ -0,0 +1,75 @@ +. +* +*/ + +namespace OCA\News\Db; + +require_once(__DIR__ . "/../../classloader.php"); + + + +class MapperTest extends \PHPUnit_Framework_TestCase { + + + public function setUp() { + $this->api = $this->getMockBuilder('\OCA\AppFramework\Core\API') + ->disableOriginalConstructor() + ->getMock(); + } + + + public function testGetItemMapperSqlite() { + $this->api->expects($this->once()) + ->method('getSystemValue') + ->with($this->equalTo('dbtype')) + ->will($this->returnValue('sqlite')); + $factory = new MapperFactory($this->api); + + $this->assertTrue($factory->getItemMapper() instanceof ItemMapper); + } + + + public function testGetItemMapperMysql() { + $this->api->expects($this->once()) + ->method('getSystemValue') + ->with($this->equalTo('dbtype')) + ->will($this->returnValue('mysql')); + $factory = new MapperFactory($this->api); + + $this->assertTrue($factory->getItemMapper() instanceof ItemMapper); + } + + + public function testGetItemMapperPostgres() { + $this->api->expects($this->once()) + ->method('getSystemValue') + ->with($this->equalTo('dbtype')) + ->will($this->returnValue('pgsql')); + $factory = new MapperFactory($this->api); + + $this->assertTrue($factory->getItemMapper() instanceof \OCA\News\Db\Postgres\ItemMapper); + } + + +} \ No newline at end of file diff --git a/tests/unit/db/postgres/ItemMapperTest.php b/tests/unit/db/postgres/ItemMapperTest.php new file mode 100644 index 000000000..3e9fb8867 --- /dev/null +++ b/tests/unit/db/postgres/ItemMapperTest.php @@ -0,0 +1,191 @@ +. +* +*/ + +namespace OCA\News\Db\Postgres; + +use \OCA\News\Db\Item; +use \OCA\News\Db\StatusFlag; + + +require_once(__DIR__ . "/../../../classloader.php"); + + +class ItemMapperTest extends \OCA\AppFramework\Utility\MapperTestUtility { + + private $mapper; + private $items; + private $newestItemId; + private $limit; + private $user; + private $offset; + private $updatedSince; + private $status; + + + public function setUp() + { + $this->beforeEach(); + + $this->mapper = new ItemMapper($this->api); + + // create mock items + $item1 = new Item(); + $item2 = new Item(); + + $this->items = array( + $item1, + $item2 + ); + + $this->userId = 'john'; + $this->id = 3; + $this->folderId = 2; + + $this->row = array( + array('id' => $this->items[0]->getId()), + ); + + $this->rows = array( + array('id' => $this->items[0]->getId()), + array('id' => $this->items[1]->getId()) + ); + + $this->user = 'john'; + $this->limit = 10; + $this->offset = 3; + $this->id = 11; + $this->status = 333; + $this->updatedSince = 323; + $this->newestItemId = 2; + + } + + + private function makeSelectQuery($prependTo){ + return 'SELECT `items`.* FROM `*PREFIX*news_items` `items` '. + 'JOIN `*PREFIX*news_feeds` `feeds` ' . + 'ON `feeds`.`id` = `items`.`feed_id` '. + 'AND `feeds`.`deleted_at` = 0 ' . + 'AND `feeds`.`user_id` = ? ' . + $prependTo . + 'LEFT OUTER JOIN `*PREFIX*news_folders` `folders` ' . + 'ON `folders`.`id` = `feeds`.`folder_id` ' . + 'WHERE `feeds`.`folder_id` = 0 ' . + 'OR `folders`.`deleted_at` = 0 ' . + 'ORDER BY `items`.`id` DESC'; + } + + private function makeSelectQueryStatus($prependTo, $status) { + $status = (int) $status; + + return $this->makeSelectQuery( + 'AND ((`items`.`status` & ' . $status . ') = ' . $status . ') ' . + $prependTo + ); + } + + + public function testDeleteReadOlderThanThresholdDoesNotDeleteBelowThreshold(){ + $status = StatusFlag::STARRED | StatusFlag::UNREAD; + $sql = 'SELECT COUNT(*) `size`, `feed_id` ' . + 'FROM `*PREFIX*news_items` ' . + 'WHERE NOT ((`status` & ?) > 0) ' . + 'GROUP BY `feed_id` ' . + 'HAVING COUNT(*) > ?'; + + $threshold = 10; + $rows = array(array('feed_id' => 30, 'size' => 9)); + $params = array($status, $threshold); + + $this->setMapperResult($sql, $params, $rows); + $this->mapper->deleteReadOlderThanThreshold($threshold); + + + } + + + public function testDeleteReadOlderThanThreshold(){ + $threshold = 10; + $status = StatusFlag::STARRED | StatusFlag::UNREAD; + + $sql1 = 'SELECT COUNT(*) `size`, `feed_id` ' . + 'FROM `*PREFIX*news_items` ' . + 'WHERE NOT ((`status` & ?) > 0) ' . + 'GROUP BY `feed_id` ' . + 'HAVING COUNT(*) > ?'; + $params1 = array($status, $threshold); + + + $row = array('feed_id' => 30, 'size' => 11); + + $sql2 = 'DELETE FROM `*PREFIX*news_items` ' . + 'WHERE `id` IN (' . + 'SELECT `id` FROM `*PREFIX*news_items` ' . + 'WHERE NOT ((`status` & ?) > 0) ' . + 'AND `feed_id` = ? ' . + 'ORDER BY `id` ASC ' . + 'LIMIT ?' . + ')'; + $params2 = array($status, 30, 1); + + + $pdoResult = $this->getMock('Result', + array('fetchRow')); + + $pdoResult->expects($this->at(0)) + ->method('fetchRow') + ->will($this->returnValue($row)); + $pdoResult->expects($this->at(1)) + ->method('fetchRow') + ->will($this->returnValue(false)); + + $query = $this->getMock('Query', + array('execute')); + $query->expects($this->at(0)) + ->method('execute') + ->with($this->equalTo($params1)) + ->will($this->returnValue($pdoResult)); + + $this->api->expects($this->at(0)) + ->method('prepareQuery') + ->with($this->equalTo($sql1)) + ->will(($this->returnValue($query))); + + $query2 = $this->getMock('Query', + array('execute')); + $query2->expects($this->at(0)) + ->method('execute') + ->with($this->equalTo($params2)); + + $this->api->expects($this->at(1)) + ->method('prepareQuery') + ->with($this->equalTo($sql2)) + ->will($this->returnValue($query2)); + + $result = $this->mapper->deleteReadOlderThanThreshold($threshold); + } + + +} -- cgit v1.2.3