try to fix delete older than threshold so we can test it properly

12 files changed, 63 insertions, 334 deletions

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 @@
                 <name>status</name>
                 <type>integer</type>
                 <length>8</length>
-                <notnull>false</notnull>
+                <default>0</default>
+                <notnull>true</notnull>
             </field>
             <field>
                 <name>last_modified</name>
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 @@
     <name>News</name>
     <description>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</description>
     <licence>AGPL</licence>
-    <version>3.999.4</version>
+    <version>3.999.5</version>
     <require>7.0.3</require>
     <author>Bernhard Posselt, Alessandro Cosentino, Jan-Christoph Borchardt</author>
 </info>
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 @@
-<?php
-/**
- * ownCloud - News
- *
- * This file is licensed under the Affero General Public License version 3 or
- * later. See the COPYING file.
- *
- * @author Alessandro Cosentino <cosenal@gmail.com>
- * @author Bernhard Posselt <dev@bernhard-posselt.com>
- * @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 @@
-<?php
-/**
- * ownCloud - News
- *
- * This file is licensed under the Affero General Public License version 3 or
- * later. See the COPYING file.
- *
- * @author Alessandro Cosentino <cosenal@gmail.com>
- * @author Bernhard Posselt <dev@bernhard-posselt.com>
- * @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->setExpectedException(
-        //    'OCP\AppFramework\Db\DoesNotExistException');
-        $this->itemMapper->find($item1->getId(), $this->userId);
-        $this->itemMapper->find($item2->getId(), $this->userId);
+
+        $this->itemMapper->find($this->items['del3']->getId(), $this->userId);
+        $this->itemMapper->find($this->items['del4']->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];
 
-
-        $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];
+                '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];
         $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 @@
-<?php
-/**
- * ownCloud - News
- *
- * This file is licensed under the Affero General Public License version 3 or
- * later. See the COPYING file.
- *
- * @author Alessandro Cosentino <cosenal@gmail.com>
- * @author Bernhard Posselt <dev@bernhard-posselt.com>
- * @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 @@
-<?php
-/**
- * ownCloud - News
- *
- * This file is licensed under the Affero General Public License version 3 or
- * later. See the COPYING file.
- *
- * @author Alessandro Cosentino <cosenal@gmail.com>
- * @author Bernhard Posselt <dev@bernhard-posselt.com>
- * @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);
-    }
-
-
-}