fix various things

10 files changed, 160 insertions, 100 deletions

diff --git a/db/newsmapper.php b/db/newsmapper.php
index a36956b16..fd0408f77 100644
--- a/db/newsmapper.php
+++ b/db/newsmapper.php
@@ -33,14 +33,27 @@ abstract class NewsMapper extends Mapper {
      * Performs a SELECT query with all arguments appened to the WHERE clause
      * The SELECT will be performed on the current table and take the entity
      * that is related for transforming the properties into column names
+     *
+     * Important: This method does not filter marked as deleted rows!
+     *
      * @param array $search an assoc array from property to filter value
+     * @param int $limit
+     * @paran int $offset
      * @return array
      */
-    public function where(array $search) {
+    public function where(array $search=[], $limit=null, $offset=null) {
         $entity = new $this->entityClass;
 
         // turn keys into sql query filter, e.g. feedId -> feed_id = :feedId
         $filter = array_map(function ($property) use ($entity) {
+            // check if the property actually exists on the entity to prevent
+            // accidental Sql injection
+            if (!property_exists($entity, $property)) {
+                $msg = 'Property ' . $property . ' does not exist on '
+                    . $this->entityClass;
+                throw new \BadFunctionCallException($msg);
+            }
+
             $column = $entity->propertyToColumn($property);
             return $column . ' = :' . $property;
         }, array_keys($search));
@@ -53,7 +66,7 @@ abstract class NewsMapper extends Mapper {
             $sql .= 'WHERE ' . $andStatement;
         }
 
-        return $this->findEntities($sql, $search);
+        return $this->findEntities($sql, $search, $limit, $offset);
     }
 
 }
\ No newline at end of file
diff --git a/tests/integration/db/FeedMapperTest.php b/tests/integration/db/FeedMapperTest.php
index 3f554fcfe..b72cb1203 100644
--- a/tests/integration/db/FeedMapperTest.php
+++ b/tests/integration/db/FeedMapperTest.php
@@ -11,9 +11,9 @@
 
 namespace OCA\News\Db;
 
-use \OCA\News\Tests\Integration\NewsIntegrationTest;
+use \OCA\News\Tests\Integration\IntegrationTest;
 
-class FeedMapperTest extends NewsIntegrationTest {
+class FeedMapperTest extends IntegrationTest {
 
 
     public function testFind () {
diff --git a/tests/integration/db/FolderMapperTest.php b/tests/integration/db/FolderMapperTest.php
index a6aaae59c..21b6ddf41 100644
--- a/tests/integration/db/FolderMapperTest.php
+++ b/tests/integration/db/FolderMapperTest.php
@@ -11,9 +11,9 @@
  
 namespace OCA\News\Db;
 
-use \OCA\News\Tests\Integration\NewsIntegrationTest;
+use \OCA\News\Tests\Integration\IntegrationTest;
 
-class FolderMapperTest extends NewsIntegrationTest {
+class FolderMapperTest extends IntegrationTest {
 
 
     public function testFind () {
diff --git a/tests/integration/db/ItemMapperTest.php b/tests/integration/db/ItemMapperTest.php
index dcd101a9f..9e2cc2a31 100644
--- a/tests/integration/db/ItemMapperTest.php
+++ b/tests/integration/db/ItemMapperTest.php
@@ -11,40 +11,37 @@
 
 namespace OCA\News\Db;
 
-use \OCA\News\Tests\Integration\NewsIntegrationTest;
-
-class ItemMapperTest extends NewsIntegrationTest {
+use OCA\News\Tests\Integration\Fixtures\ItemFixture;
+use OCA\News\Tests\Integration\IntegrationTest;
 
+class ItemMapperTest extends IntegrationTest {
 
     public function testFind() {
-        $feedId = $this->feeds['first feed']->getId();
-
-        $item = new Item();
-        $item->setTitle('my title thats long');
-        $item->setGuid('a doner');
-        $item->setGuidHash('a doner');
-        $item->setFeedId($feedId);
-        $item->setUnread();
-        $item->setBody('Döner');
+        $item = new ItemFixture();
 
         $created = $this->itemMapper->insert($item);
-        $fetched = $this->itemMapper->find($created->getId(), $this->userId);
+        $fetched = $this->itemMapper->find($created->getId(), $this->user);
 
         $this->assertEquals($item->getTitle(), $fetched->getTitle());
-        $this->assertEquals($item->getGuid(), $fetched->getGuid());
-        $this->assertEquals($item->getGuidHash(), $fetched->getGuidHash());
-        $this->assertEquals($item->getFeedId(), $fetched->getFeedId());
-        $this->assertEquals($item->isRead(), $fetched->isRead());
-        $this->assertEquals('Döner', $fetched->getBody());
     }
 
+    /**
+     * Same as whereId with easier title search
+     * @param $title
+     * @return mixed
+     */
+    private function whereTitleId($title) {
+        return $this->findItemByTitle($title)->getId();
+    }
 
     /**
      * @expectedException OCP\AppFramework\Db\DoesNotExistException
      */
     public function testFindNotFoundWhenDeletedFeed() {
-        $id = $this->items['not found feed']->getId();
-        $this->itemMapper->find($id, $this->userId);
+        $this->loadFixtures('default');
+
+        $id = $this->whereTitleId('not found feed');
+        $this->itemMapper->find($id, $this->user);
     }
 
 
@@ -52,164 +49,170 @@ class ItemMapperTest extends NewsIntegrationTest {
      * @expectedException OCP\AppFramework\Db\DoesNotExistException
      */
     public function testFindNotFoundWhenDeletedFolder() {
-        $id = $this->items['not found folder']->getId();
-        $this->itemMapper->find($id, $this->userId);
+        $this->loadFixtures('default');
+
+        $id = $this->whereTitleId('not found folder');
+        $this->itemMapper->find($id, $this->user);
     }
 
 
     private function deleteReadOlderThanThreshold() {
+        $this->loadFixtures('default');
+
         $this->itemMapper->deleteReadOlderThanThreshold(1);
 
-        $this->itemMapper->find($this->items['a title1']->getId(),
-                                $this->userId);
-        $this->itemMapper->find($this->items['a title2']->getId(),
-                                $this->userId);
-        $this->itemMapper->find($this->items['a title3']->getId(),
-                                $this->userId);
-        $this->itemMapper->find($this->items['del3']->getId(), $this->userId);
-        $this->itemMapper->find($this->items['del4']->getId(), $this->userId);
+        $this->itemMapper->find($this->whereTitleId('a title1'), $this->user);
+        $this->itemMapper->find($this->whereTitleId('a title2'), $this->user);
+        $this->itemMapper->find($this->whereTitleId('a title3'), $this->user);
+        $this->itemMapper->find($this->whereTitleId('del3'), $this->user);
+        $this->itemMapper->find($this->whereTitleId('del4'), $this->user);
     }
 
-
+    /**
+     * @expectedException OCP\AppFramework\Db\DoesNotExistException
+     */
     public function testDeleteOlderThanThresholdOne() {
+        $this->loadFixtures('default');
+
         $this->deleteReadOlderThanThreshold();
 
-        $this->setExpectedException(
-            'OCP\AppFramework\Db\DoesNotExistException');
-        $this->itemMapper->find($this->items['del1']->getId(), $this->userId);
+        $this->itemMapper->find($this->whereTitleId('del1'), $this->user);
     }
 
-
+    /**
+     * @expectedException OCP\AppFramework\Db\DoesNotExistException
+     */
     public function testDeleteOlderThanThresholdTwo() {
+        $this->loadFixtures('default');
+
         $this->deleteReadOlderThanThreshold();
 
-        $this->setExpectedException(
-            'OCP\AppFramework\Db\DoesNotExistException');
-        $this->itemMapper->find($this->items['del2']->getId(), $this->userId);
+        $this->itemMapper->find($this->whereTitleId('del2'), $this->user);
     }
 
 
     public function testStarredCount () {
-        $count = $this->itemMapper->starredCount($this->userId);
+        $this->loadFixtures('default');
+
+        $count = $this->itemMapper->starredCount($this->user);
         $this->assertEquals(2, $count);
     }
 
 
     public function testReadAll () {
-        $this->itemMapper->readAll(PHP_INT_MAX, 10, $this->userId);
+        $this->loadFixtures('default');
+
+        $this->itemMapper->readAll(PHP_INT_MAX, 10, $this->user);
 
         $status = StatusFlag::UNREAD;
         $items = $this->itemMapper->findAll(
-            30, 0, $status, false, $this->userId
+            30, 0, $status, false, $this->user
         );
 
         $this->assertEquals(0, count($items));
 
-        $item = $this->items['a title1'];
-        $item = $this->itemMapper->find($item->getId(), $this->userId);
+        $itemId = $this->whereTitleId('a title1');
+        $item = $this->itemMapper->find($itemId, $this->user);
 
         $this->assertEquals(10, $item->getLastModified());
 
-        $item = $this->items['a title3'];
-        $item = $this->itemMapper->find($item->getId(), $this->userId);
+        $itemId = $this->whereTitleId('a title3');
+        $item = $this->itemMapper->find($itemId, $this->user);
 
         $this->assertEquals(10, $item->getLastModified());
 
-        $item = $this->items['a title9'];
-        $item = $this->itemMapper->find($item->getId(), $this->userId);
+        $itemId = $this->whereTitleId('a title9');
+        $item = $this->itemMapper->find($itemId, $this->user);
 
         $this->assertEquals(10, $item->getLastModified());
     }
 
 
     public function testReadFolder () {
+        $this->loadFixtures('default');
+
         $folderId = $this->folders['first folder']->getId();
         $this->itemMapper->readFolder(
-            $folderId, PHP_INT_MAX, 10, $this->userId
+            $folderId, PHP_INT_MAX, 10, $this->user
         );
 
         $status = StatusFlag::UNREAD;
         $items = $this->itemMapper->findAll(
-            30, 0, $status, false, $this->userId
+            30, 0, $status, false, $this->user
         );
 
         $this->assertEquals(1, count($items));
 
         $item = $this->items['a title1'];
-        $item = $this->itemMapper->find($item->getId(), $this->userId);
+        $item = $this->itemMapper->find($item->getId(), $this->user);
 
         $this->assertEquals(10, $item->getLastModified());
 
         $item = $this->items['a title3'];
-        $item = $this->itemMapper->find($item->getId(), $this->userId);
+        $item = $this->itemMapper->find($item->getId(), $this->user);
 
         $this->assertEquals(10, $item->getLastModified());
 
         $item = $this->items['a title9'];
-        $item = $this->itemMapper->find($item->getId(), $this->userId);
+        $item = $this->itemMapper->find($item->getId(), $this->user);
 
         $this->assertTrue($item->isUnread());
     }
 
 
     public function testReadFeed () {
+        $this->loadFixtures('default');
+
         $feedId = $this->feeds['third feed']->getId();
         $this->itemMapper->readFeed(
-            $feedId, PHP_INT_MAX, 10, $this->userId
+            $feedId, PHP_INT_MAX, 10, $this->user
         );
 
         $status = StatusFlag::UNREAD;
         $items = $this->itemMapper->findAll(
-            30, 0, $status, false, $this->userId
+            30, 0, $status, false, $this->user
         );
 
         $this->assertEquals(2, count($items));
 
         $item = $this->items['a title9'];
-        $item = $this->itemMapper->find($item->getId(), $this->userId);
+        $item = $this->itemMapper->find($item->getId(), $this->user);
 
         $this->assertEquals(10, $item->getLastModified());
 
         $item = $this->items['a title3'];
-        $item = $this->itemMapper->find($item->getId(), $this->userId);
+        $item = $this->itemMapper->find($item->getId(), $this->user);
         $this->assertTrue($item->isUnread());
 
 
         $item = $this->items['a title1'];
-        $item = $this->itemMapper->find($item->getId(), $this->userId);
+        $item = $this->itemMapper->find($item->getId(), $this->user);
         $this->assertTrue($item->isUnread());
     }
 
 
     public function testDeleteUser () {
-        $this->itemMapper->deleteUser($this->userId);
-        $id = $this->itemMapper->getNewestItemId($this->userId);
+        $this->loadFixtures('default');
+
+        $this->itemMapper->deleteUser($this->user);
+        $id = $this->itemMapper->getNewestItemId($this->user);
 
         $this->assertEquals(0, $id);
     }
 
-
     public function testGetNewestItemId () {
-        $id = $this->itemMapper->getNewestItemId($this->userId);
-
-        $item = $this->items['no folder'];
-        $this->assertEquals($item->getId(), $id);
-    }
-
+        $this->loadFixtures('default');
 
-    public function testFindByGuidHash () {
-        $item = $this->items['no folder'];
+        $id = $this->itemMapper->getNewestItemId($this->user);
 
-        $fetchedItem = $this->itemMapper->findByGuidHash(
-            'no folder', $item->getFeedId(), $this->userId
-        );
-
-        $this->assertEquals($item->getId(), $fetchedItem->getId());
+        $itemId = $this->whereTitleId('no folder');
+        $this->assertEquals($itemId, $id);
     }
 
-
     public function testFindAllUnreadOrStarred () {
-        $items = $this->itemMapper->findAllUnreadOrStarred($this->userId);
+        $this->loadFixtures('default');
+
+        $items = $this->itemMapper->findAllUnreadOrStarred($this->user);
         $this->assertEquals(4, count($items));
     }
 
diff --git a/tests/integration/fixtures/data/default.php b/tests/integration/fixtures/data/default.php
index b314d9496..5fc0565f3 100644
--- a/tests/integration/fixtures/data/default.php
+++ b/tests/integration/fixtures/data/default.php
@@ -19,12 +19,12 @@ return [
                     'url' => 'http://google.de',
                     'items' => [
                         ['title' => 'a title1'],
-                        ['title' => 'a title2', 'status' => 4],
-                        ['title' => 'a title3', 'status' => 6],
-                        ['title' => 'del1', 'status' => 0],
-                        ['title' => 'del2', 'status' => 0],
-                        ['title' => 'del3', 'status' => 0],
-                        ['title' => 'del4', 'status' => 0]
+                        ['title' => 'a title2', 'starred' => true],
+                        ['title' => 'a title3', 'starred' => true],
+                        ['title' => 'del1', 'read' => true],
+                        ['title' => 'del2', 'read' => true],
+                        ['title' => 'del3', 'read' => true],
+                        ['title' => 'del4', 'read' => true]
                     ]
                 ],
                 [
@@ -67,7 +67,7 @@ return [
             'title' => 'fourth feed',
             'url' => 'http://blog.fefe.de',
             'items' => [
-                ['title' => 'no folder', 'status' => 0]
+                ['title' => 'no folder', 'read' => true]
             ]
         ]
     ]
diff --git a/tests/integration/fixtures/FeedFixture.php b/tests/integration/fixtures/feedfixture.php
index 7b2d68c80..8c87a8efd 100644
--- a/tests/integration/fixtures/FeedFixture.php
+++ b/tests/integration/fixtures/feedfixture.php
@@ -48,4 +48,4 @@ class FeedFixture extends Feed {
         $this->fillDefaults($defaults);
     }
 
-}
\ No newline at end of file
+}
diff --git a/tests/integration/fixtures/Fixture.php b/tests/integration/fixtures/fixture.php
index 2e546c4a5..52015ebc2 100644
--- a/tests/integration/fixtures/Fixture.php
+++ b/tests/integration/fixtures/fixture.php
@@ -13,7 +13,7 @@
 namespace OCA\News\Tests\Integration\Fixtures;
 
 
-trait Entity {
+trait Fixture {
 
     public function fillDefaults(array $defaults=[]) {
         foreach ($defaults as $key => $value) {
diff --git a/tests/integration/fixtures/FolderFixture.php b/tests/integration/fixtures/folderfixture.php
index 872ee33b8..872ee33b8 100644
--- a/tests/integration/fixtures/FolderFixture.php
+++ b/tests/integration/fixtures/folderfixture.php
diff --git a/tests/integration/fixtures/ItemFixture.php b/tests/integration/fixtures/itemfixture.php
index 412f1c990..ac9191c4d 100644
--- a/tests/integration/fixtures/ItemFixture.php
+++ b/tests/integration/fixtures/itemfixture.php
@@ -35,9 +35,16 @@ class ItemFixture extends Item {
             'rtl' => false,
         ], $defaults);
         $this->fillDefaults($defaults);
+
+        if (!array_key_exists('guid', $defaults)) {
+            $this->setGuid($this->getTitle());
+        }
+
+        if (!array_key_exists('guidHash', $defaults)) {
+            $this->setGuidHash($this->getGuid());
+        }
+
         $this->generateSearchIndex();
-        $this->setGuid($this->getTitle());
-        $this->setGuidHash($this->getGuid());
     }
 
-}
\ No newline at end of file
+}
diff --git a/tests/integration/IntegrationTest.php b/tests/integration/integrationtest.php
index fda0ee1b1..08f146fdd 100644
--- a/tests/integration/IntegrationTest.php
+++ b/tests/integration/integrationtest.php
@@ -14,12 +14,16 @@ namespace OCA\News\Tests\Integration;
 
 use PHPUnit_Framework_TestCase;
 
+use OCA\News\Db\Feed;
+use OCA\News\Db\Item;
+use OCP\AppFramework\Db\Entity;
+use OCP\AppFramework\IAppContainer;
+
 use OCP\IDb;
 use OCP\IUserSession;
 use OCP\IUserManager;
 
 use OCA\News\AppInfo\Application;
-use OCA\News\Tests\Integration\Fixtures\Entity;
 use OCA\News\Tests\Integration\Fixtures\ItemFixture;
 use OCA\News\Tests\Integration\Fixtures\FeedFixture;
 use OCA\News\Tests\Integration\Fixtures\FolderFixture;
@@ -42,6 +46,9 @@ abstract class IntegrationTest extends PHPUnit_Framework_TestCase {
     /** @var FolderMapper */
     protected $folderMapper;
 
+    /** @var IAppContainer */
+    protected $container;
+
     protected function setUp() {
         parent::setUp();
         $app = new Application();
@@ -55,8 +62,38 @@ abstract class IntegrationTest extends PHPUnit_Framework_TestCase {
         $this->folderMapper = $this->container->query(FolderMapper::class);
     }
 
+    protected function findItemByTitle($title) {
+        // db logic in app code, negligible since its a test
+        $items = $this->itemMapper->where();
+        $feeds = $this->feedMapper->where(['userId' => $this->user]);
+
+        $feedIds = array_map(function (Feed $feed) {
+            return $feed->getId();
+        }, $feeds);
+
+        return array_filter($items, function (Item $item) use ($title, $feedIds) {
+            return $item->getTitle() === $title &&
+                in_array($item->getFeedId(), $feedIds);
+
+        })[0];
+    }
+
+    protected function findFolderByName($name) {
+        return $this->folderMapper->where([
+            'userId' => $this->user,
+            'name' => $name
+        ])[0];
+    }
+
+    protected function findFeedByTitle($title) {
+        return $this->feedMapper->where([
+            'userId' => $this->user,
+            'title' => $title
+        ])[0];
+    }
+
     /**
-     * @param $name loads fixtures from a given file
+     * @param string $name loads fixtures from a given file
      */
     protected function loadFixtures($name) {
         $fixtures = include __DIR__ . '/data/' . $name . '.php';
@@ -132,7 +169,7 @@ abstract class IntegrationTest extends PHPUnit_Framework_TestCase {
             $userManager->get($user)->delete();
         }
 
-        $this->clearNewsDatabase($user);
+        $this->clearUserNewsDatabase($user);
     }
 
     /**
@@ -141,10 +178,10 @@ abstract class IntegrationTest extends PHPUnit_Framework_TestCase {
      */
     protected function clearUserNewsDatabase($user) {
         $sql = [
-            'DELETE FROM *PREFIX*news_items WHERE feed_id IN ' .
-            '(SELECT id FROM *PREFIX*news_feeds WHERE user_id = ?)',
-            'DELETE FROM *PREFIX*news_feeds WHERE user_id = ?',
-            'DELETE FROM *PREFIX*news_folders WHERE user_id = ?'
+            'DELETE FROM `*PREFIX*news_items` WHERE `feed_id` IN
+              (SELECT `id` FROM `*PREFIX*news_feeds` WHERE `user_id` = ?)',
+            'DELETE FROM `*PREFIX*news_feeds` WHERE `user_id` = ?',
+            'DELETE FROM `*PREFIX*news_folders` WHERE `user_id` = ?'
         ];
 
         $db = $this->container->query(IDb::class);