From a600f6b718ab81efeda1fce68e5f817c3f23504a Mon Sep 17 00:00:00 2001 From: Bernhard Posselt Date: Thu, 6 Nov 2014 15:49:16 +0100 Subject: [PATCH] try to fix delete older than threshold so we can test it properly --- appinfo/application.php | 11 +- appinfo/database.xml | 3 +- appinfo/info.xml | 2 +- db/itemmapper.php | 25 +++-- db/mapperfactory.php | 40 ------- db/postgres/itemmapper.php | 69 ------------ tests/integration/bootstrap.php | 10 +- tests/integration/db/ItemMapperTest.php | 30 ++++-- tests/integration/fixtures/feeds.json | 2 - tests/unit/db/ItemMapperTest.php | 28 ++--- tests/unit/db/MapperFactoryTest.php | 51 --------- tests/unit/db/postgres/ItemMapperTest.php | 122 ---------------------- 12 files changed, 61 insertions(+), 332 deletions(-) delete mode 100644 db/mapperfactory.php delete mode 100644 db/postgres/itemmapper.php delete mode 100644 tests/unit/db/MapperFactoryTest.php delete mode 100644 tests/unit/db/postgres/ItemMapperTest.php diff --git a/appinfo/application.php b/appinfo/application.php index 8716feac9..5c1978459 100644 --- a/appinfo/application.php +++ b/appinfo/application.php @@ -43,8 +43,8 @@ use \OCA\News\Service\ItemService; 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\Utility\OPMLExporter; use \OCA\News\Utility\Updater; @@ -230,13 +230,6 @@ class Application extends App { /** * Mappers */ - $container->registerService('MapperFactory', function($c) { - return new MapperFactory( - $c->query('DatabaseType'), - $c->query('Db') - ); - }); - $container->registerService('FolderMapper', function($c) { return new FolderMapper( $c->query('Db') @@ -250,7 +243,7 @@ class Application extends App { }); $container->registerService('ItemMapper', function($c) { - return $c->query('MapperFactory')->getItemMapper( + return new ItemMapper( $c->query('Db') ); }); diff --git a/appinfo/database.xml b/appinfo/database.xml index f7a95746e..688d04547 100644 --- a/appinfo/database.xml +++ b/appinfo/database.xml @@ -252,7 +252,8 @@ status integer 8 - false + 0 + true last_modified diff --git a/appinfo/info.xml b/appinfo/info.xml index d3ea13c0c..45e6cde33 100644 --- a/appinfo/info.xml +++ b/appinfo/info.xml @@ -4,7 +4,7 @@ News An RSS/Atom feed reader. Requires ownCloud backgroundjobs or an updater script to be enabled to update your feeds. See the README.md in the apps top directory AGPL - 3.999.4 + 3.999.5 7.0.3 Bernhard Posselt, Alessandro Cosentino, Jan-Christoph Borchardt diff --git a/db/itemmapper.php b/db/itemmapper.php index 41c2a6975..dec80894f 100644 --- a/db/itemmapper.php +++ b/db/itemmapper.php @@ -237,15 +237,17 @@ class ItemMapper extends NewsMapper { */ public function deleteReadOlderThanThreshold($threshold){ $status = StatusFlag::STARRED | StatusFlag::UNREAD; - $sql = 'SELECT COUNT(*) - `feeds`.`articles_per_update` AS `size`, ' . - '`items`.`feed_id` AS `feed_id` ' . + $params = [$status, $threshold]; + + $sql = 'SELECT (COUNT(*) - `feeds`.`articles_per_update`) AS `size`, ' . + '`feeds`.`id` AS `feed_id`, `feeds`.`articles_per_update` ' . 'FROM `*PREFIX*news_items` `items` ' . 'JOIN `*PREFIX*news_feeds` `feeds` ' . 'ON `feeds`.`id` = `items`.`feed_id` ' . 'AND NOT ((`items`.`status` & ?) > 0) ' . - 'GROUP BY `items`.`feed_id`, `feeds`.`articles_per_update` ' . + 'GROUP BY `feeds`.`id`, `feeds`.`articles_per_update` ' . 'HAVING COUNT(*) > ?'; - $params = [$status, $threshold]; + $result = $this->execute($sql, $params); while($row = $result->fetch()) { @@ -254,16 +256,21 @@ class ItemMapper extends NewsMapper { $limit = $size - $threshold; if($limit > 0) { - $params = [$status, $row['feed_id']]; + $params = [$status, $row['feed_id'], $limit]; $sql = 'DELETE FROM `*PREFIX*news_items` ' . - 'WHERE NOT ((`status` & ?) > 0) ' . - 'AND `feed_id` = ? ' . - 'ORDER BY `id` ASC'; + '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, $limit); + $this->execute($sql, $params); } } + } diff --git a/db/mapperfactory.php b/db/mapperfactory.php deleted file mode 100644 index 6bc9346ca..000000000 --- a/db/mapperfactory.php +++ /dev/null @@ -1,40 +0,0 @@ - - * @author Bernhard Posselt - * @copyright Alessandro Cosentino 2012 - * @copyright Bernhard Posselt 2012, 2014 - */ - -namespace OCA\News\Db; - -use \OCP\IDb; -use \OCA\News\Db\Postgres\ItemMapper as PostgresItemMapper; - -class MapperFactory { - - private $dbType; - private $db; - - public function __construct($dbType, IDb $db) { - $this->dbType = $dbType; - $this->db = $db; - } - - - public function getItemMapper() { - switch($this->dbType) { - case 'pgsql': - return new PostgresItemMapper($this->db); - default: - return new ItemMapper($this->db); - } - } - - -} \ No newline at end of file diff --git a/db/postgres/itemmapper.php b/db/postgres/itemmapper.php deleted file mode 100644 index f736fe54c..000000000 --- a/db/postgres/itemmapper.php +++ /dev/null @@ -1,69 +0,0 @@ - - * @author Bernhard Posselt - * @copyright Alessandro Cosentino 2012 - * @copyright Bernhard Posselt 2012, 2014 - */ - -namespace OCA\News\Db\Postgres; - -use \OCP\IDb; - -use \OCA\News\Db\StatusFlag; - - -class ItemMapper extends \OCA\News\Db\ItemMapper { - - public function __construct(IDb $db){ - parent::__construct($db); - } - - - /** - * 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(*) - `feeds`.`articles_per_update` AS `size`, ' . - '`items`.`feed_id` AS `feed_id` ' . - 'FROM `*PREFIX*news_items` `items` ' . - 'JOIN `*PREFIX*news_feeds` `feeds` ' . - 'ON `feeds`.`id` = `items`.`feed_id` ' . - 'AND NOT ((`items`.`status` & ?) > 0) ' . - 'GROUP BY `items`.`feed_id`, `feeds`.`articles_per_update` ' . - 'HAVING COUNT(*) > ?'; - $params = [$status, $threshold]; - $result = $this->execute($sql, $params); - - while($row = $result->fetch()) { - - $size = (int) $row['size']; - $limit = $size - $threshold; - - if($limit > 0) { - $params = [$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/tests/integration/bootstrap.php b/tests/integration/bootstrap.php index 815913e3a..c8d7cc719 100644 --- a/tests/integration/bootstrap.php +++ b/tests/integration/bootstrap.php @@ -24,7 +24,6 @@ class NewsIntegrationTest extends \PHPUnit_Framework_TestCase { protected function setUp() { $this->ownCloudVersion = \OCP\Util::getVersion(); $this->cleanUp(); - $this->setupUser($this->userId, $this->userPassword); $app = new Application(); $this->container = $app->getContainer(); @@ -67,13 +66,13 @@ class NewsIntegrationTest extends \PHPUnit_Framework_TestCase { // feeds in folders foreach($folders as $folder) { $newFolder = $this->createFolder($folder); - $this->folders[$newFolder->getName()] = $newFolder; + $this->folders[$folder['name']] = $newFolder; if (array_key_exists($folder['name'], $feeds)) { foreach ($feeds[$folder['name']] as $feed) { $feed['folderId'] = $newFolder->getId(); $newFeed = $this->createFeed($feed); - $this->feeds[$newFeed->getTitle()] = $newFeed; + $this->feeds[$feed['title']] = $newFeed; if (array_key_exists($feed['title'], $items)) { foreach ($items[$feed['title']] as $item) { @@ -159,7 +158,7 @@ class NewsIntegrationTest extends \PHPUnit_Framework_TestCase { } - protected function setupUser($user='test', $password='test') { + protected function setupUser($user, $password) { $userManager = \OC::$server->getUserManager(); if ($userManager->userExists($user)) { @@ -176,13 +175,16 @@ class NewsIntegrationTest extends \PHPUnit_Framework_TestCase { $session->login($user, $password); } + private function cleanUp() { + $this->setupUser($this->userId, $this->userPassword); $this->clearNewsDatabase($this->userId); $this->folders = []; $this->feeds = []; $this->items = []; } + protected function tearDown() { $this->cleanUp(); } diff --git a/tests/integration/db/ItemMapperTest.php b/tests/integration/db/ItemMapperTest.php index c86112d46..5e6452398 100644 --- a/tests/integration/db/ItemMapperTest.php +++ b/tests/integration/db/ItemMapperTest.php @@ -47,20 +47,28 @@ class ItemMapperTest extends NewsIntegrationTest { } - public function testDeleteOlderThanThreshold() { + private function deleteReadOlderThanThreshold() { $this->itemMapper->deleteReadOlderThanThreshold(1); - $item1 = $this->items['del1']; - $item2 = $this->items['del2']; - $item3 = $this->items['del3']; - $item4 = $this->items['del4']; - $this->itemMapper->find($item3->getId(), $this->userId); - $this->itemMapper->find($item4->getId(), $this->userId); + $this->itemMapper->find($this->items['del3']->getId(), $this->userId); + $this->itemMapper->find($this->items['del4']->getId(), $this->userId); + } - //$this->setExpectedException( - // 'OCP\AppFramework\Db\DoesNotExistException'); - $this->itemMapper->find($item1->getId(), $this->userId); - $this->itemMapper->find($item2->getId(), $this->userId); + + public function testDeleteOlderThanThresholdOne() { + $this->deleteReadOlderThanThreshold(); + + $this->setExpectedException( + 'OCP\AppFramework\Db\DoesNotExistException'); + $this->itemMapper->find($this->items['del1']->getId(), $this->userId); + } + + + public function testDeleteOlderThanThresholdTwo() { + $this->deleteReadOlderThanThreshold(); + $this->setExpectedException( + 'OCP\AppFramework\Db\DoesNotExistException'); + $this->itemMapper->find($this->items['del2']->getId(), $this->userId); } diff --git a/tests/integration/fixtures/feeds.json b/tests/integration/fixtures/feeds.json index b1696a9eb..685a75396 100644 --- a/tests/integration/fixtures/feeds.json +++ b/tests/integration/fixtures/feeds.json @@ -2,7 +2,6 @@ "first folder": [ { "title": "first feed", - "folderName": "first folder", "url": "http://google.de", "deletedAt": 0, "location": "http://feed.com/rss", @@ -16,7 +15,6 @@ }, { "title": "second feed", - "folderName": "first folder", "url": "http://golem.de", "deletedAt": 0, "location": "http://feed.com/rss", diff --git a/tests/unit/db/ItemMapperTest.php b/tests/unit/db/ItemMapperTest.php index 46a129ecf..a4e3fe919 100644 --- a/tests/unit/db/ItemMapperTest.php +++ b/tests/unit/db/ItemMapperTest.php @@ -361,13 +361,13 @@ class ItemMapperTest extends \Test\AppFramework\Db\MapperTestUtility { public function testDeleteReadOlderThanThresholdDoesNotDelete(){ $status = StatusFlag::STARRED | StatusFlag::UNREAD; - $sql = 'SELECT COUNT(*) - `feeds`.`articles_per_update` AS `size`, ' . - '`items`.`feed_id` AS `feed_id` ' . + $sql = 'SELECT (COUNT(*) - `feeds`.`articles_per_update`) AS `size`' . + ', `feeds`.`id` AS `feed_id`, `feeds`.`articles_per_update` ' . 'FROM `*PREFIX*news_items` `items` ' . 'JOIN `*PREFIX*news_feeds` `feeds` ' . 'ON `feeds`.`id` = `items`.`feed_id` ' . 'AND NOT ((`items`.`status` & ?) > 0) ' . - 'GROUP BY `items`.`feed_id`, `feeds`.`articles_per_update` ' . + 'GROUP BY `feeds`.`id`, `feeds`.`articles_per_update` ' . 'HAVING COUNT(*) > ?'; $threshold = 10; @@ -385,25 +385,27 @@ class ItemMapperTest extends \Test\AppFramework\Db\MapperTestUtility { $threshold = 10; $status = StatusFlag::STARRED | StatusFlag::UNREAD; - $sql1 = 'SELECT COUNT(*) - `feeds`.`articles_per_update` AS `size`, ' . - '`items`.`feed_id` AS `feed_id` ' . + $sql1 = 'SELECT (COUNT(*) - `feeds`.`articles_per_update`) AS `size`' . + ', `feeds`.`id` AS `feed_id`, `feeds`.`articles_per_update` ' . 'FROM `*PREFIX*news_items` `items` ' . 'JOIN `*PREFIX*news_feeds` `feeds` ' . 'ON `feeds`.`id` = `items`.`feed_id` ' . 'AND NOT ((`items`.`status` & ?) > 0) ' . - 'GROUP BY `items`.`feed_id`, `feeds`.`articles_per_update` ' . + 'GROUP BY `feeds`.`id`, `feeds`.`articles_per_update` ' . 'HAVING COUNT(*) > ?'; $params1 = [$status, $threshold]; + $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 = [$status, 30, 1]; $row = ['feed_id' => 30, 'size' => 11]; - - $sql2 = 'DELETE FROM `*PREFIX*news_items` ' . - 'WHERE NOT ((`status` & ?) > 0) ' . - 'AND `feed_id` = ? ' . - 'ORDER BY `id` ASC'; - $params2 = [$status, 30]; - $this->setMapperResult($sql1, $params1, [$row]); $this->setMapperResult($sql2, $params2); diff --git a/tests/unit/db/MapperFactoryTest.php b/tests/unit/db/MapperFactoryTest.php deleted file mode 100644 index b5aa76dc0..000000000 --- a/tests/unit/db/MapperFactoryTest.php +++ /dev/null @@ -1,51 +0,0 @@ - - * @author Bernhard Posselt - * @copyright Alessandro Cosentino 2012 - * @copyright Bernhard Posselt 2012, 2014 - */ - -namespace OCA\News\Db; - - -class MapperFactoryTest extends \PHPUnit_Framework_TestCase { - - private $db; - private $settings; - - public function setUp() { - $this->db = $this->getMockBuilder('\OCP\IDb') - ->disableOriginalConstructor() - ->getMock(); - } - - - public function testGetItemMapperSqlite() { - $factory = new MapperFactory('sqlite', $this->db); - - $this->assertTrue($factory->getItemMapper() instanceof ItemMapper); - } - - - public function testGetItemMapperMysql() { - $factory = new MapperFactory('mysql', $this->db); - - $this->assertTrue($factory->getItemMapper() instanceof ItemMapper); - } - - - public function testGetItemMapperPostgres() { - $factory = new MapperFactory('pgsql', $this->db); - - $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 deleted file mode 100644 index 22c6bb78c..000000000 --- a/tests/unit/db/postgres/ItemMapperTest.php +++ /dev/null @@ -1,122 +0,0 @@ - - * @author Bernhard Posselt - * @copyright Alessandro Cosentino 2012 - * @copyright Bernhard Posselt 2012, 2014 - */ - -namespace OCA\News\Db\Postgres; - -use \OCA\News\Db\Item; -use \OCA\News\Db\StatusFlag; - - -class ItemMapperTest extends \Test\AppFramework\Db\MapperTestUtility { - - private $mapper; - private $items; - private $newestItemId; - private $limit; - private $user; - private $offset; - private $updatedSince; - private $status; - - - public function setUp() { - parent::setUp(); - - $this->mapper = new ItemMapper($this->db); - - // create mock items - $item1 = new Item(); - $item2 = new Item(); - - $this->items = [$item1, $item2]; - - $this->userId = 'john'; - $this->id = 3; - $this->folderId = 2; - - $this->row = [['id' => $this->items[0]->getId()]]; - - $this->rows = [ - ['id' => $this->items[0]->getId()], - ['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; - - } - - - public function testDeleteReadOlderThanThresholdDoesntDelete(){ - $status = StatusFlag::STARRED | StatusFlag::UNREAD; - $sql = 'SELECT COUNT(*) - `feeds`.`articles_per_update` AS `size`, ' . - '`items`.`feed_id` AS `feed_id` ' . - 'FROM `*PREFIX*news_items` `items` ' . - 'JOIN `*PREFIX*news_feeds` `feeds` ' . - 'ON `feeds`.`id` = `items`.`feed_id` ' . - 'AND NOT ((`items`.`status` & ?) > 0) ' . - 'GROUP BY `items`.`feed_id`, `feeds`.`articles_per_update` ' . - 'HAVING COUNT(*) > ?'; - - $threshold = 10; - $rows = [['feed_id' => 30, 'size' => 9]]; - $params = [$status, $threshold]; - - $this->setMapperResult($sql, $params, $rows); - $this->mapper->deleteReadOlderThanThreshold($threshold); - - - } - - - public function testDeleteReadOlderThanThreshold(){ - $threshold = 10; - $status = StatusFlag::STARRED | StatusFlag::UNREAD; - - $sql1 = 'SELECT COUNT(*) - `feeds`.`articles_per_update` AS `size`, ' . - '`items`.`feed_id` AS `feed_id` ' . - 'FROM `*PREFIX*news_items` `items` ' . - 'JOIN `*PREFIX*news_feeds` `feeds` ' . - 'ON `feeds`.`id` = `items`.`feed_id` ' . - 'AND NOT ((`items`.`status` & ?) > 0) ' . - 'GROUP BY `items`.`feed_id`, `feeds`.`articles_per_update` ' . - 'HAVING COUNT(*) > ?'; - $params1 = [$status, $threshold]; - - - $row = ['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 = [$status, 30, 1]; - - - $this->setMapperResult($sql1, $params1, [$row]); - $this->setMapperResult($sql2, $params2); - - $this->mapper->deleteReadOlderThanThreshold($threshold); - } - - -}