From 76c46cca63895a38aec65699787b983762fd92c8 Mon Sep 17 00:00:00 2001 From: Daniel Opitz Date: Tue, 11 Jul 2017 00:17:37 +0200 Subject: [PATCH 01/11] replaced old status with 2 flags for unread and starred --- lib/Db/FeedMapper.php | 12 +++------ lib/Db/Item.php | 53 +++++++++---------------------------- lib/Db/ItemMapper.php | 49 +++++++++++++++++++--------------- lib/Db/Mysql/ItemMapper.php | 21 ++++++--------- lib/Fetcher/FeedFetcher.php | 2 +- lib/Service/ItemService.php | 9 +++---- 6 files changed, 56 insertions(+), 90 deletions(-) diff --git a/lib/Db/FeedMapper.php b/lib/Db/FeedMapper.php index 056296e497..bd063109f5 100644 --- a/lib/Db/FeedMapper.php +++ b/lib/Db/FeedMapper.php @@ -35,8 +35,7 @@ public function find($id, $userId){ // work because prepared statements dont work. This is a // POSSIBLE SQL INJECTION RISK WHEN MODIFIED WITHOUT THOUGHT. // think twice when changing this - 'AND (`items`.`status` & ' . StatusFlag::UNREAD . ') = ' . - StatusFlag::UNREAD . ' ' . + 'AND unread = 1 ' . 'WHERE `feeds`.`id` = ? ' . 'AND `feeds`.`user_id` = ? ' . 'GROUP BY `feeds`.`id`'; @@ -57,8 +56,7 @@ public function findAllFromUser($userId){ // work because prepared statements dont work. This is a // POSSIBLE SQL INJECTION RISK WHEN MODIFIED WITHOUT THOUGHT. // think twice when changing this - 'AND (`items`.`status` & ' . StatusFlag::UNREAD . ') = ' . - StatusFlag::UNREAD . ' ' . + 'AND unread = 1 ' . 'WHERE `feeds`.`user_id` = ? ' . 'AND (`feeds`.`folder_id` = 0 ' . 'OR `folders`.`deleted_at` = 0' . @@ -82,8 +80,7 @@ public function findAll(){ // work because prepared statements dont work. This is a // POSSIBLE SQL INJECTION RISK WHEN MODIFIED WITHOUT THOUGHT. // think twice when changing this - 'AND (`items`.`status` & ' . StatusFlag::UNREAD . ') = ' . - StatusFlag::UNREAD . ' ' . + 'AND unread = 1 ' . 'WHERE (`feeds`.`folder_id` = 0 ' . 'OR `folders`.`deleted_at` = 0' . ')' . @@ -103,8 +100,7 @@ public function findByUrlHash($hash, $userId){ // work because prepared statements dont work. This is a // POSSIBLE SQL INJECTION RISK WHEN MODIFIED WITHOUT THOUGHT. // think twice when changing this - 'AND (`items`.`status` & ' . StatusFlag::UNREAD . ') = ' . - StatusFlag::UNREAD . ' ' . + 'AND unread = 1 ' . 'WHERE `feeds`.`url_hash` = ? ' . 'AND `feeds`.`user_id` = ? ' . 'GROUP BY `feeds`.`id`'; diff --git a/lib/Db/Item.php b/lib/Db/Item.php index f19843c8e6..e701e1e81a 100644 --- a/lib/Db/Item.php +++ b/lib/Db/Item.php @@ -37,14 +37,14 @@ * @method void setEnclosureLink(string $value) * @method integer getFeedId() * @method void setFeedId(integer $value) - * @method integer getStatus() - * @method void setStatus(integer $value) * @method void setRtl(boolean $value) * @method string getLastModified() * @method void setLastModified(string $value) * @method void setFingerprint(string $value) * @method void setContentHash(string $value) * @method void setSearchIndex(string $value) + * @method void setUnread(bool $value) + * @method void setStarred(bool $value) */ class Item extends Entity implements IAPI, \JsonSerializable { @@ -67,53 +67,32 @@ class Item extends Entity implements IAPI, \JsonSerializable { protected $searchIndex; protected $rtl; protected $fingerprint; + protected $unread; + protected $starred; public function __construct() { $this->addType('pubDate', 'integer'); $this->addType('updatedDate', 'integer'); $this->addType('feedId', 'integer'); - $this->addType('status', 'integer'); $this->addType('rtl', 'boolean'); - } - - public function setRead() { - $this->markFieldUpdated('status'); - $this->status &= ~StatusFlag::UNREAD; - } - - public function isRead() { - return !(($this->status & StatusFlag::UNREAD) === StatusFlag::UNREAD); - } - - public function setUnread() { - $this->markFieldUpdated('status'); - $this->status |= StatusFlag::UNREAD; + $this->addType('unread', 'boolean'); + $this->addType('starred', 'boolean'); } public function isUnread() { - return !$this->isRead(); - } - - public function setStarred() { - $this->markFieldUpdated('status'); - $this->status |= StatusFlag::STARRED; + return $this->unread; } public function isStarred() { - return ($this->status & StatusFlag::STARRED) === StatusFlag::STARRED; - } - - public function setUnstarred() { - $this->markFieldUpdated('status'); - $this->status &= ~StatusFlag::STARRED; + return (bool)$this->starred; } public function isUnstarred() { - return !$this->isStarred(); + return !$this->starred; } /** - * Turns entitie attributes into an array + * Turns entity attributes into an array */ public function jsonSerialize() { return [ @@ -196,16 +175,8 @@ public static function fromImport($import) { $item->setEnclosureMime($import['enclosureMime']); $item->setEnclosureLink($import['enclosureLink']); $item->setRtl($import['rtl']); - if ($import['unread']) { - $item->setUnread(); - } else { - $item->setRead(); - } - if ($import['starred']) { - $item->setStarred(); - } else { - $item->setUnstarred(); - } + $item->setUnread(!empty($import['unread'])); + $item->setStarred(!empty($import['starred'])); return $item; } diff --git a/lib/Db/ItemMapper.php b/lib/Db/ItemMapper.php index b49cdd5ea9..a312f2ae33 100644 --- a/lib/Db/ItemMapper.php +++ b/lib/Db/ItemMapper.php @@ -49,10 +49,19 @@ private function makeSelectQueryStatus($prependTo, $status, $oldestFirst = false, $search = [], $distinctFingerprint = false) { $status = (int)$status; + // destruct old status flag + $unread = (($status & StatusFlag::UNREAD) == StatusFlag::UNREAD); + $starred = (($status & StatusFlag::STARRED) == StatusFlag::STARRED); $count = count($search); + $sql = ''; + if ($unread) { + $sql .= 'AND `items`.`unread` = ' . $unread . ' '; + } elseif ($starred) { + $sql .= 'AND `items`.`starred` = ' . $starred . ' '; + } + // WARNING: Potential SQL injection if you change this carelessly - $sql = 'AND ((`items`.`status` & ' . $status . ') = ' . $status . ') '; $sql .= str_repeat('AND `items`.`search_index` LIKE ? ', $count); $sql .= $prependTo; @@ -88,14 +97,13 @@ public function starredCount($userId) { 'ON `feeds`.`id` = `items`.`feed_id` ' . 'AND `feeds`.`deleted_at` = 0 ' . 'AND `feeds`.`user_id` = ? ' . - 'AND ((`items`.`status` & ' . StatusFlag::STARRED . ') = ' . - StatusFlag::STARRED . ')' . + 'AND `items`.`starred` = ? ' . 'LEFT OUTER JOIN `*PREFIX*news_folders` `folders` ' . 'ON `folders`.`id` = `feeds`.`folder_id` ' . 'WHERE `feeds`.`folder_id` = 0 ' . 'OR `folders`.`deleted_at` = 0'; - $params = [$userId]; + $params = [$userId, 1]; $result = $this->execute($sql, $params)->fetch(); @@ -105,21 +113,21 @@ public function starredCount($userId) { public function readAll($highestItemId, $time, $userId) { $sql = 'UPDATE `*PREFIX*news_items` ' . - 'SET `status` = `status` & ? ' . + 'SET unread = ? ' . ', `last_modified` = ? ' . 'WHERE `feed_id` IN (' . 'SELECT `id` FROM `*PREFIX*news_feeds` ' . 'WHERE `user_id` = ? ' . ') ' . 'AND `id` <= ?'; - $params = [~StatusFlag::UNREAD, $time, $userId, $highestItemId]; + $params = [0, $time, $userId, $highestItemId]; $this->execute($sql, $params); } public function readFolder($folderId, $highestItemId, $time, $userId) { $sql = 'UPDATE `*PREFIX*news_items` ' . - 'SET `status` = `status` & ? ' . + 'SET unread = ? ' . ', `last_modified` = ? ' . 'WHERE `feed_id` IN (' . 'SELECT `id` FROM `*PREFIX*news_feeds` ' . @@ -127,7 +135,7 @@ public function readFolder($folderId, $highestItemId, $time, $userId) { 'AND `user_id` = ? ' . ') ' . 'AND `id` <= ?'; - $params = [~StatusFlag::UNREAD, $time, $folderId, $userId, + $params = [0, $time, $folderId, $userId, $highestItemId]; $this->execute($sql, $params); } @@ -135,7 +143,7 @@ public function readFolder($folderId, $highestItemId, $time, $userId) { public function readFeed($feedId, $highestItemId, $time, $userId) { $sql = 'UPDATE `*PREFIX*news_items` ' . - 'SET `status` = `status` & ? ' . + 'SET unread = ? ' . ', `last_modified` = ? ' . 'WHERE `feed_id` = ? ' . 'AND `id` <= ? ' . @@ -143,7 +151,7 @@ public function readFeed($feedId, $highestItemId, $time, $userId) { 'SELECT * FROM `*PREFIX*news_feeds` ' . 'WHERE `user_id` = ? ' . 'AND `id` = ? ) '; - $params = [~StatusFlag::UNREAD, $time, $feedId, $highestItemId, + $params = [0, $time, $feedId, $highestItemId, $userId, $feedId]; $this->execute($sql, $params); @@ -250,8 +258,7 @@ public function findAll($limit, $offset, $status, $oldestFirst, $userId, public function findAllUnreadOrStarred($userId) { $params = [$userId]; - $status = StatusFlag::UNREAD | StatusFlag::STARRED; - $sql = 'AND ((`items`.`status` & ' . $status . ') > 0) '; + $sql = 'AND `items`.`unread` = 1 OR `items`.`starred` = 1 '; $sql = $this->makeSelectQuery($sql); return $this->findEntities($sql, $params); } @@ -272,15 +279,14 @@ public function findByGuidHash($guidHash, $feedId, $userId) { * @param int $threshold the number of items that should be deleted */ public function deleteReadOlderThanThreshold($threshold) { - $status = StatusFlag::STARRED | StatusFlag::UNREAD; - $params = [$status, $threshold]; + $params = [1, 1, $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) ' . + 'AND NOT (`items`.`unread` = ? OR `items`.`starred` = ?) ' . 'GROUP BY `feeds`.`id`, `feeds`.`articles_per_update` ' . 'HAVING COUNT(*) > ?'; @@ -292,12 +298,12 @@ public function deleteReadOlderThanThreshold($threshold) { $limit = $size - $threshold; if ($limit > 0) { - $params = [$status, $row['feed_id'], $limit]; + $params = [1, 1, $row['feed_id'], $limit]; $sql = 'DELETE FROM `*PREFIX*news_items` ' . 'WHERE `id` IN (' . 'SELECT `id` FROM `*PREFIX*news_items` ' . - 'WHERE NOT ((`status` & ?) > 0) ' . + 'WHERE NOT (`items`.`unread` = ? OR `items`.`starred` = ?) ' . 'AND `feed_id` = ? ' . 'ORDER BY `id` ASC ' . 'LIMIT ?' . @@ -383,19 +389,18 @@ public function readItem($itemId, $isRead, $lastModified, $userId) { // as unread if ($isRead) { $sql = 'UPDATE `*PREFIX*news_items` - SET `status` = `status` & ?, - `last_modified` = ? + SET `unread` = ?,'. + '`last_modified` = ? WHERE `fingerprint` = ? AND `feed_id` IN ( SELECT `f`.`id` FROM `*PREFIX*news_feeds` AS `f` WHERE `f`.`user_id` = ? )'; - $params = [~StatusFlag::UNREAD, $lastModified, - $item->getFingerprint(), $userId]; + $params = [0, $lastModified, $item->getFingerprint(), $userId]; $this->execute($sql, $params); } else { $item->setLastModified($lastModified); - $item->setUnread(); + $item->setUnread(true); $this->update($item); } } diff --git a/lib/Db/Mysql/ItemMapper.php b/lib/Db/Mysql/ItemMapper.php index 8053c3d871..b3ee8aebd3 100644 --- a/lib/Db/Mysql/ItemMapper.php +++ b/lib/Db/Mysql/ItemMapper.php @@ -16,9 +16,6 @@ use OCA\News\Utility\Time; use OCP\IDBConnection; -use OCA\News\Db\StatusFlag; - - class ItemMapper extends \OCA\News\Db\ItemMapper { public function __construct(IDBConnection $db, Time $time){ @@ -32,16 +29,15 @@ public function __construct(IDBConnection $db, Time $time){ * @param int $threshold the number of items that should be deleted */ public function deleteReadOlderThanThreshold($threshold){ - $status = StatusFlag::STARRED | StatusFlag::UNREAD; $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) ' . + 'AND NOT (`items`.`unread` = ? OR `items`.`starred` = ?)' . 'GROUP BY `feeds`.`id`, `feeds`.`articles_per_update` ' . 'HAVING COUNT(*) > ?'; - $params = [$status, $threshold]; + $params = [1, 1, $threshold]; $result = $this->execute($sql, $params); while($row = $result->fetch()) { @@ -50,10 +46,10 @@ public function deleteReadOlderThanThreshold($threshold){ $limit = $size - $threshold; if($limit > 0) { - $params = [$status, $row['feed_id'], $limit]; + $params = [1, 1, $row['feed_id'], $limit]; $sql = 'DELETE FROM `*PREFIX*news_items` ' . - 'WHERE NOT ((`status` & ?) > 0) ' . + 'WHERE NOT (`items`.`unread` = ? OR `items`.`starred` = ?)' . 'AND `feed_id` = ? ' . 'ORDER BY `id` ASC ' . 'LIMIT ?'; @@ -71,16 +67,15 @@ public function readItem($itemId, $isRead, $lastModified, $userId) { $sql = 'UPDATE `*PREFIX*news_items` `items` JOIN `*PREFIX*news_feeds` `feeds` ON `feeds`.`id` = `items`.`feed_id` - SET `items`.`status` = `items`.`status` & ?, - `items`.`last_modified` = ? + SET `items`.`unread` = ?,'. + '`items`.`last_modified` = ? WHERE `items`.`fingerprint` = ? AND `feeds`.`user_id` = ?'; - $params = [~StatusFlag::UNREAD, $lastModified, - $item->getFingerprint(), $userId]; + $params = [0, $lastModified, $item->getFingerprint(), $userId]; $this->execute($sql, $params); } else { $item->setLastModified($lastModified); - $item->setUnread(); + $item->setUnread(true); $this->update($item); } } diff --git a/lib/Fetcher/FeedFetcher.php b/lib/Fetcher/FeedFetcher.php index b870017235..e63db2772a 100644 --- a/lib/Fetcher/FeedFetcher.php +++ b/lib/Fetcher/FeedFetcher.php @@ -242,7 +242,7 @@ protected function determineRtl($parsedItem, $parsedFeed) { protected function buildItem($parsedItem, $parsedFeed) { $item = new Item(); - $item->setUnread(); + $item->setUnread(true); $item->setUrl($parsedItem->getUrl()); $item->setGuid($parsedItem->getId()); $item->setGuidHash($item->getGuid()); diff --git a/lib/Service/ItemService.php b/lib/Service/ItemService.php index f657b3eff3..9051860f56 100644 --- a/lib/Service/ItemService.php +++ b/lib/Service/ItemService.php @@ -13,6 +13,7 @@ namespace OCA\News\Service; +use OCA\News\Db\Item; use OCP\IConfig; use OCP\AppFramework\Db\DoesNotExistException; @@ -122,15 +123,13 @@ public function findAll($id, $type, $limit, $offset, $showAll, $oldestFirst, */ public function star($feedId, $guidHash, $isStarred, $userId){ try { + /** @var Item $item */ $item = $this->itemMapper->findByGuidHash( $guidHash, $feedId, $userId ); - if($isStarred){ - $item->setStarred(); - } else { - $item->setUnstarred(); - } + $item->setStarred($isStarred); + $this->itemMapper->update($item); } catch(DoesNotExistException $ex) { throw new ServiceNotFoundException($ex->getMessage()); From e5adcd76fa3b85e0e15c295a5cd937c60398edf4 Mon Sep 17 00:00:00 2001 From: Daniel Opitz Date: Wed, 12 Jul 2017 23:37:51 +0200 Subject: [PATCH 02/11] add fields to db, replace int(1,0) with booleans in sql queries, removed StatusFlags class + refactor code relying to it --- appinfo/database.xml | 12 ++++ lib/Db/FeedMapper.php | 8 +-- lib/Db/ItemMapper.php | 121 ++++++++++++++++++++---------------- lib/Db/Mysql/ItemMapper.php | 16 ++--- lib/Db/StatusFlag.php | 47 -------------- lib/Service/ItemService.php | 20 ++---- 6 files changed, 98 insertions(+), 126 deletions(-) delete mode 100644 lib/Db/StatusFlag.php diff --git a/appinfo/database.xml b/appinfo/database.xml index 1044208cd6..0675bf8558 100644 --- a/appinfo/database.xml +++ b/appinfo/database.xml @@ -363,6 +363,18 @@ 0 true + + unread + boolean + true + true + + + starred + boolean + false + true + last_modified integer diff --git a/lib/Db/FeedMapper.php b/lib/Db/FeedMapper.php index bd063109f5..d3f69807eb 100644 --- a/lib/Db/FeedMapper.php +++ b/lib/Db/FeedMapper.php @@ -35,7 +35,7 @@ public function find($id, $userId){ // work because prepared statements dont work. This is a // POSSIBLE SQL INJECTION RISK WHEN MODIFIED WITHOUT THOUGHT. // think twice when changing this - 'AND unread = 1 ' . + 'AND unread = true ' . 'WHERE `feeds`.`id` = ? ' . 'AND `feeds`.`user_id` = ? ' . 'GROUP BY `feeds`.`id`'; @@ -56,7 +56,7 @@ public function findAllFromUser($userId){ // work because prepared statements dont work. This is a // POSSIBLE SQL INJECTION RISK WHEN MODIFIED WITHOUT THOUGHT. // think twice when changing this - 'AND unread = 1 ' . + 'AND unread = true ' . 'WHERE `feeds`.`user_id` = ? ' . 'AND (`feeds`.`folder_id` = 0 ' . 'OR `folders`.`deleted_at` = 0' . @@ -80,7 +80,7 @@ public function findAll(){ // work because prepared statements dont work. This is a // POSSIBLE SQL INJECTION RISK WHEN MODIFIED WITHOUT THOUGHT. // think twice when changing this - 'AND unread = 1 ' . + 'AND unread = true ' . 'WHERE (`feeds`.`folder_id` = 0 ' . 'OR `folders`.`deleted_at` = 0' . ')' . @@ -100,7 +100,7 @@ public function findByUrlHash($hash, $userId){ // work because prepared statements dont work. This is a // POSSIBLE SQL INJECTION RISK WHEN MODIFIED WITHOUT THOUGHT. // think twice when changing this - 'AND unread = 1 ' . + 'AND unread = true ' . 'WHERE `feeds`.`url_hash` = ? ' . 'AND `feeds`.`user_id` = ? ' . 'GROUP BY `feeds`.`id`'; diff --git a/lib/Db/ItemMapper.php b/lib/Db/ItemMapper.php index a312f2ae33..ae47c7eb83 100644 --- a/lib/Db/ItemMapper.php +++ b/lib/Db/ItemMapper.php @@ -45,27 +45,26 @@ private function makeSelectQuery($prependTo = '', $oldestFirst = false, 'ORDER BY `items`.`id` ' . $ordering; } - private function makeSelectQueryStatus($prependTo, $status, - $oldestFirst = false, $search = [], - $distinctFingerprint = false) { - $status = (int)$status; - // destruct old status flag - $unread = (($status & StatusFlag::UNREAD) == StatusFlag::UNREAD); - $starred = (($status & StatusFlag::STARRED) == StatusFlag::STARRED); - $count = count($search); - + /** + * check if type is feed or all items should be shown + * @param bool $showAll + * @param int|null $type + * @return string + */ + private function buildStatusQueryPart($showAll, $type = null) { $sql = ''; - if ($unread) { - $sql .= 'AND `items`.`unread` = ' . $unread . ' '; - } elseif ($starred) { - $sql .= 'AND `items`.`starred` = ' . $starred . ' '; + + if (isset($type) && $type === FeedType::STARRED) { + $sql = 'AND `items`.`starred` = true '; + } elseif (!$showAll) { + $sql .= 'AND `items`.`unread` = true '; } - // WARNING: Potential SQL injection if you change this carelessly - $sql .= str_repeat('AND `items`.`search_index` LIKE ? ', $count); - $sql .= $prependTo; + return $sql; + } - return $this->makeSelectQuery($sql, $oldestFirst, $distinctFingerprint); + private function buildSearchQueryPart(array $search = []) { + return str_repeat('AND `items`.`search_index` LIKE ? ', count($search)); } /** @@ -97,13 +96,13 @@ public function starredCount($userId) { 'ON `feeds`.`id` = `items`.`feed_id` ' . 'AND `feeds`.`deleted_at` = 0 ' . 'AND `feeds`.`user_id` = ? ' . - 'AND `items`.`starred` = ? ' . + 'AND `items`.`starred` = true ' . 'LEFT OUTER JOIN `*PREFIX*news_folders` `folders` ' . 'ON `folders`.`id` = `feeds`.`folder_id` ' . 'WHERE `feeds`.`folder_id` = 0 ' . 'OR `folders`.`deleted_at` = 0'; - $params = [$userId, 1]; + $params = [$userId]; $result = $this->execute($sql, $params)->fetch(); @@ -113,21 +112,21 @@ public function starredCount($userId) { public function readAll($highestItemId, $time, $userId) { $sql = 'UPDATE `*PREFIX*news_items` ' . - 'SET unread = ? ' . + 'SET unread = false ' . ', `last_modified` = ? ' . 'WHERE `feed_id` IN (' . 'SELECT `id` FROM `*PREFIX*news_feeds` ' . 'WHERE `user_id` = ? ' . ') ' . 'AND `id` <= ?'; - $params = [0, $time, $userId, $highestItemId]; + $params = [$time, $userId, $highestItemId]; $this->execute($sql, $params); } public function readFolder($folderId, $highestItemId, $time, $userId) { $sql = 'UPDATE `*PREFIX*news_items` ' . - 'SET unread = ? ' . + 'SET unread = false ' . ', `last_modified` = ? ' . 'WHERE `feed_id` IN (' . 'SELECT `id` FROM `*PREFIX*news_feeds` ' . @@ -135,7 +134,7 @@ public function readFolder($folderId, $highestItemId, $time, $userId) { 'AND `user_id` = ? ' . ') ' . 'AND `id` <= ?'; - $params = [0, $time, $folderId, $userId, + $params = [$time, $folderId, $userId, $highestItemId]; $this->execute($sql, $params); } @@ -143,7 +142,7 @@ public function readFolder($folderId, $highestItemId, $time, $userId) { public function readFeed($feedId, $highestItemId, $time, $userId) { $sql = 'UPDATE `*PREFIX*news_items` ' . - 'SET unread = ? ' . + 'SET unread = false ' . ', `last_modified` = ? ' . 'WHERE `feed_id` = ? ' . 'AND `id` <= ? ' . @@ -151,7 +150,7 @@ public function readFeed($feedId, $highestItemId, $time, $userId) { 'SELECT * FROM `*PREFIX*news_feeds` ' . 'WHERE `user_id` = ? ' . 'AND `id` = ? ) '; - $params = [0, $time, $feedId, $highestItemId, + $params = [$time, $feedId, $highestItemId, $userId, $feedId]; $this->execute($sql, $params); @@ -167,27 +166,33 @@ private function getOperator($oldestFirst) { } - public function findAllNew($updatedSince, $status, $userId) { - $sql = $this->makeSelectQueryStatus( - 'AND `items`.`last_modified` >= ? ', $status); + public function findAllNew($updatedSince, $type, $showAll, $userId) { + $sql = $this->buildStatusQueryPart($showAll, $type); + + $sql .= 'AND `items`.`last_modified` >= ? '; + $sql = $this->makeSelectQuery($sql); $params = [$userId, $updatedSince]; return $this->findEntities($sql, $params); } - public function findAllNewFolder($id, $updatedSince, $status, $userId) { - $sql = 'AND `feeds`.`folder_id` = ? ' . + public function findAllNewFolder($id, $updatedSince, $showAll, $userId) { + $sql = $this->buildStatusQueryPart($showAll); + + $sql .= 'AND `feeds`.`folder_id` = ? ' . 'AND `items`.`last_modified` >= ? '; - $sql = $this->makeSelectQueryStatus($sql, $status); + $sql = $this->makeSelectQuery($sql); $params = [$userId, $id, $updatedSince]; return $this->findEntities($sql, $params); } - public function findAllNewFeed($id, $updatedSince, $status, $userId) { - $sql = 'AND `items`.`feed_id` = ? ' . + public function findAllNewFeed($id, $updatedSince, $showAll, $userId) { + $sql = $this->buildStatusQueryPart($showAll); + + $sql .= 'AND `items`.`feed_id` = ? ' . 'AND `items`.`last_modified` >= ? '; - $sql = $this->makeSelectQueryStatus($sql, $status); + $sql = $this->makeSelectQuery($sql); $params = [$userId, $id, $updatedSince]; return $this->findEntities($sql, $params); } @@ -203,54 +208,60 @@ private function findEntitiesIgnoringNegativeLimit($sql, $params, $limit) { } - public function findAllFeed($id, $limit, $offset, $status, $oldestFirst, + public function findAllFeed($id, $limit, $offset, $showAll, $oldestFirst, $userId, $search = []) { $params = [$userId]; $params = array_merge($params, $this->buildLikeParameters($search)); $params[] = $id; - $sql = 'AND `items`.`feed_id` = ? '; + $sql = $this->buildStatusQueryPart($showAll); + $sql .= $this->buildSearchQueryPart($search); + + $sql .= 'AND `items`.`feed_id` = ? '; if ($offset !== 0) { $sql .= 'AND `items`.`id` ' . $this->getOperator($oldestFirst) . ' ? '; $params[] = $offset; } - $sql = $this->makeSelectQueryStatus($sql, $status, $oldestFirst, - $search); + $sql = $this->makeSelectQuery($sql, $oldestFirst, $search); return $this->findEntitiesIgnoringNegativeLimit($sql, $params, $limit); } - public function findAllFolder($id, $limit, $offset, $status, $oldestFirst, + public function findAllFolder($id, $limit, $offset, $showAll, $oldestFirst, $userId, $search = []) { $params = [$userId]; $params = array_merge($params, $this->buildLikeParameters($search)); $params[] = $id; - $sql = 'AND `feeds`.`folder_id` = ? '; + $sql = $this->buildStatusQueryPart($showAll); + $sql .= $this->buildSearchQueryPart($search); + + $sql .= 'AND `feeds`.`folder_id` = ? '; if ($offset !== 0) { $sql .= 'AND `items`.`id` ' . $this->getOperator($oldestFirst) . ' ? '; $params[] = $offset; } - $sql = $this->makeSelectQueryStatus($sql, $status, $oldestFirst, - $search); + $sql = $this->makeSelectQuery($sql, $oldestFirst, $search); return $this->findEntitiesIgnoringNegativeLimit($sql, $params, $limit); } - public function findAll($limit, $offset, $status, $oldestFirst, $userId, + public function findAll($limit, $offset, $type, $showAll, $oldestFirst, $userId, $search = []) { $params = [$userId]; $params = array_merge($params, $this->buildLikeParameters($search)); - $sql = ''; + $sql = $this->buildStatusQueryPart($showAll, $type); + $sql .= $this->buildSearchQueryPart($search); + if ($offset !== 0) { $sql .= 'AND `items`.`id` ' . $this->getOperator($oldestFirst) . ' ? '; $params[] = $offset; } - $sql = $this->makeSelectQueryStatus($sql, $status, $oldestFirst, - $search); + + $sql = $this->makeSelectQuery($sql, $oldestFirst); return $this->findEntitiesIgnoringNegativeLimit($sql, $params, $limit); } @@ -258,7 +269,7 @@ public function findAll($limit, $offset, $status, $oldestFirst, $userId, public function findAllUnreadOrStarred($userId) { $params = [$userId]; - $sql = 'AND `items`.`unread` = 1 OR `items`.`starred` = 1 '; + $sql = 'AND `items`.`unread` = true OR `items`.`starred` = true '; $sql = $this->makeSelectQuery($sql); return $this->findEntities($sql, $params); } @@ -279,14 +290,15 @@ public function findByGuidHash($guidHash, $feedId, $userId) { * @param int $threshold the number of items that should be deleted */ public function deleteReadOlderThanThreshold($threshold) { - $params = [1, 1, $threshold]; + $params = [$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`.`unread` = ? OR `items`.`starred` = ?) ' . + 'AND `items`.`unread` = false ' . + 'AND `items`.`starred` = false ' . 'GROUP BY `feeds`.`id`, `feeds`.`articles_per_update` ' . 'HAVING COUNT(*) > ?'; @@ -298,12 +310,13 @@ public function deleteReadOlderThanThreshold($threshold) { $limit = $size - $threshold; if ($limit > 0) { - $params = [1, 1, $row['feed_id'], $limit]; + $params = [$row['feed_id'], $limit]; $sql = 'DELETE FROM `*PREFIX*news_items` ' . 'WHERE `id` IN (' . 'SELECT `id` FROM `*PREFIX*news_items` ' . - 'WHERE NOT (`items`.`unread` = ? OR `items`.`starred` = ?) ' . + 'WHERE `items`.`unread` = false ' . + 'AND `items`.`starred` = false ' . 'AND `feed_id` = ? ' . 'ORDER BY `id` ASC ' . 'LIMIT ?' . @@ -389,14 +402,14 @@ public function readItem($itemId, $isRead, $lastModified, $userId) { // as unread if ($isRead) { $sql = 'UPDATE `*PREFIX*news_items` - SET `unread` = ?,'. - '`last_modified` = ? + SET `unread` = false, + `last_modified` = ? WHERE `fingerprint` = ? AND `feed_id` IN ( SELECT `f`.`id` FROM `*PREFIX*news_feeds` AS `f` WHERE `f`.`user_id` = ? )'; - $params = [0, $lastModified, $item->getFingerprint(), $userId]; + $params = [$lastModified, $item->getFingerprint(), $userId]; $this->execute($sql, $params); } else { $item->setLastModified($lastModified); diff --git a/lib/Db/Mysql/ItemMapper.php b/lib/Db/Mysql/ItemMapper.php index b3ee8aebd3..5e85400645 100644 --- a/lib/Db/Mysql/ItemMapper.php +++ b/lib/Db/Mysql/ItemMapper.php @@ -34,10 +34,11 @@ public function deleteReadOlderThanThreshold($threshold){ 'FROM `*PREFIX*news_items` `items` ' . 'JOIN `*PREFIX*news_feeds` `feeds` ' . 'ON `feeds`.`id` = `items`.`feed_id` ' . - 'AND NOT (`items`.`unread` = ? OR `items`.`starred` = ?)' . + 'AND `items`.`unread` = false ' . + 'AND `items`.`starred` = false ' . 'GROUP BY `feeds`.`id`, `feeds`.`articles_per_update` ' . 'HAVING COUNT(*) > ?'; - $params = [1, 1, $threshold]; + $params = [$threshold]; $result = $this->execute($sql, $params); while($row = $result->fetch()) { @@ -46,10 +47,11 @@ public function deleteReadOlderThanThreshold($threshold){ $limit = $size - $threshold; if($limit > 0) { - $params = [1, 1, $row['feed_id'], $limit]; + $params = [$row['feed_id'], $limit]; $sql = 'DELETE FROM `*PREFIX*news_items` ' . - 'WHERE NOT (`items`.`unread` = ? OR `items`.`starred` = ?)' . + 'WHERE `items`.`unread` = false ' . + 'AND `items`.`starred` = false ' . 'AND `feed_id` = ? ' . 'ORDER BY `id` ASC ' . 'LIMIT ?'; @@ -67,11 +69,11 @@ public function readItem($itemId, $isRead, $lastModified, $userId) { $sql = 'UPDATE `*PREFIX*news_items` `items` JOIN `*PREFIX*news_feeds` `feeds` ON `feeds`.`id` = `items`.`feed_id` - SET `items`.`unread` = ?,'. - '`items`.`last_modified` = ? + SET `items`.`unread` = false, + `items`.`last_modified` = ? WHERE `items`.`fingerprint` = ? AND `feeds`.`user_id` = ?'; - $params = [0, $lastModified, $item->getFingerprint(), $userId]; + $params = [$lastModified, $item->getFingerprint(), $userId]; $this->execute($sql, $params); } else { $item->setLastModified($lastModified); diff --git a/lib/Db/StatusFlag.php b/lib/Db/StatusFlag.php deleted file mode 100644 index 314a81d01e..0000000000 --- a/lib/Db/StatusFlag.php +++ /dev/null @@ -1,47 +0,0 @@ - - * @author Bernhard Posselt - * @copyright Alessandro Cosentino 2012 - * @copyright Bernhard Posselt 2012, 2014 - */ - -namespace OCA\News\Db; - -class StatusFlag { - const UNREAD = 0x02; - const STARRED = 0x04; - const DELETED = 0x08; - const UPDATED = 0x16; - - - /** - * Get status for query - * - * @param int $type the type that should be turned into the status - * @param bool $showAll true if it should return all read items - * @return int the status for the database - */ - public function typeToStatus($type, $showAll){ - if($type === FeedType::STARRED){ - return self::STARRED; - } else { - $status = 0; - } - - if($showAll){ - $status &= ~self::UNREAD; - } else { - $status |= self::UNREAD; - } - - return $status; - } - - -} \ No newline at end of file diff --git a/lib/Service/ItemService.php b/lib/Service/ItemService.php index 9051860f56..fee00ffc23 100644 --- a/lib/Service/ItemService.php +++ b/lib/Service/ItemService.php @@ -18,7 +18,6 @@ use OCP\AppFramework\Db\DoesNotExistException; use OCA\News\Db\ItemMapper; -use OCA\News\Db\StatusFlag; use OCA\News\Db\FeedType; use OCA\News\Config\Config; use OCA\News\Utility\Time; @@ -26,19 +25,16 @@ class ItemService extends Service { - private $statusFlag; private $config; private $timeFactory; private $itemMapper; private $systemConfig; public function __construct(ItemMapper $itemMapper, - StatusFlag $statusFlag, Time $timeFactory, Config $config, IConfig $systemConfig){ parent::__construct($itemMapper); - $this->statusFlag = $statusFlag; $this->config = $config; $this->timeFactory = $timeFactory; $this->itemMapper = $itemMapper; @@ -57,20 +53,18 @@ public function __construct(ItemMapper $itemMapper, * @return array of items */ public function findAllNew($id, $type, $updatedSince, $showAll, $userId){ - $status = $this->statusFlag->typeToStatus($type, $showAll); - switch($type){ case FeedType::FEED: return $this->itemMapper->findAllNewFeed( - $id, $updatedSince, $status, $userId + $id, $updatedSince, $showAll, $userId ); case FeedType::FOLDER: return $this->itemMapper->findAllNewFolder( - $id, $updatedSince, $status, $userId + $id, $updatedSince, $showAll, $userId ); default: return $this->itemMapper->findAllNew( - $updatedSince, $status, $userId + $updatedSince, $type, $showAll, $userId ); } } @@ -91,22 +85,20 @@ public function findAllNew($id, $type, $updatedSince, $showAll, $userId){ */ public function findAll($id, $type, $limit, $offset, $showAll, $oldestFirst, $userId, $search=[]){ - $status = $this->statusFlag->typeToStatus($type, $showAll); - switch($type){ case FeedType::FEED: return $this->itemMapper->findAllFeed( - $id, $limit, $offset, $status, $oldestFirst, $userId, + $id, $limit, $offset, $showAll, $oldestFirst, $userId, $search ); case FeedType::FOLDER: return $this->itemMapper->findAllFolder( - $id, $limit, $offset, $status, $oldestFirst, $userId, + $id, $limit, $offset, $showAll, $oldestFirst, $userId, $search ); default: return $this->itemMapper->findAll( - $limit, $offset, $status, $oldestFirst, $userId, $search + $limit, $offset, $type, $showAll, $oldestFirst, $userId, $search ); } } From 193c926ef66792269590d972e3a6403a8b7a5ea9 Mon Sep 17 00:00:00 2001 From: Daniel Opitz Date: Thu, 13 Jul 2017 00:29:13 +0200 Subject: [PATCH 03/11] add repair step for migration --- appinfo/info.xml | 8 ++++- lib/Migration/MigrateStatusFlags.php | 54 ++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+), 1 deletion(-) create mode 100644 lib/Migration/MigrateStatusFlags.php diff --git a/appinfo/info.xml b/appinfo/info.xml index 6f97c052f0..099f3d824f 100644 --- a/appinfo/info.xml +++ b/appinfo/info.xml @@ -8,7 +8,7 @@ Before you update to a new version, [check the changelog](https://github.com/nextcloud/news/blob/master/CHANGELOG.md) to avoid surprises. **Important**: To enable feed updates you will need to enable either [Nextcloud system cron](https://docs.nextcloud.com/server/10/admin_manual/configuration_server/background_jobs_configuration.html#cron) or use [an updater](https://github.com/nextcloud/news-updater) which uses the built in update API and disable cron updates. More information can be found [in the README](https://github.com/nextcloud/news).]]> - 11.0.5 + 11.0.6 agpl Bernhard Posselt Alessandro Cosentino @@ -42,6 +42,12 @@ Before you update to a new version, [check the changelog](https://github.com/nex OCA\News\Cron\Updater + + + OCA\News\Migration\MigrateStatusFlags + + + OCA\News\Settings\Admin OCA\News\Settings\Section diff --git a/lib/Migration/MigrateStatusFlags.php b/lib/Migration/MigrateStatusFlags.php new file mode 100644 index 0000000000..7f3b6f562a --- /dev/null +++ b/lib/Migration/MigrateStatusFlags.php @@ -0,0 +1,54 @@ + + * @copyright Daniel Opitz 2017 + */ + +namespace OCA\News\Migration; + +use OCP\IConfig; +use OCP\IDBConnection; +use OCP\Migration\IRepairStep; +use OCP\Migration\IOutput; + +class MigrateStatusFlags implements IRepairStep { + + /** @var IDBConnection */ + private $db; + + /** @var IConfig */ + private $config; + + /** + * @param IDBConnection $db + * @param IConfig $config + */ + public function __construct(IDBConnection $db, IConfig $config) { + $this->db = $db; + $this->config = $config; + } + + public function getName() { + return 'Migrate binary status into separate boolean fields'; + } + + public function run(IOutput $output) { + $version = $this->config->getAppValue('news', 'installed_version', '0.0.0'); + if (version_compare($version, '11.0.6', '>=')) { + return; + } + + $update = 'UPDATE `*PREFIX*news_items` ' . + 'SET unread = IF(status & 2, true, false), starred = IF(status & 4, true, false)'; + + $output->startProgress(); + $this->db->executeUpdate($update); + $output->finishProgress(); + } + +} \ No newline at end of file From 0eb2723d73b2bfe448ceb1e00868884d075d8c89 Mon Sep 17 00:00:00 2001 From: Daniel Opitz Date: Thu, 13 Jul 2017 20:52:13 +0200 Subject: [PATCH 04/11] again use integer(1,0) instead of bool in sql queries, because of sqlite doesn't support true/false --- lib/Db/FeedMapper.php | 8 ++++---- lib/Db/ItemMapper.php | 24 ++++++++++++------------ lib/Db/Mysql/ItemMapper.php | 10 +++++----- lib/Migration/MigrateStatusFlags.php | 2 +- 4 files changed, 22 insertions(+), 22 deletions(-) diff --git a/lib/Db/FeedMapper.php b/lib/Db/FeedMapper.php index d3f69807eb..bd063109f5 100644 --- a/lib/Db/FeedMapper.php +++ b/lib/Db/FeedMapper.php @@ -35,7 +35,7 @@ public function find($id, $userId){ // work because prepared statements dont work. This is a // POSSIBLE SQL INJECTION RISK WHEN MODIFIED WITHOUT THOUGHT. // think twice when changing this - 'AND unread = true ' . + 'AND unread = 1 ' . 'WHERE `feeds`.`id` = ? ' . 'AND `feeds`.`user_id` = ? ' . 'GROUP BY `feeds`.`id`'; @@ -56,7 +56,7 @@ public function findAllFromUser($userId){ // work because prepared statements dont work. This is a // POSSIBLE SQL INJECTION RISK WHEN MODIFIED WITHOUT THOUGHT. // think twice when changing this - 'AND unread = true ' . + 'AND unread = 1 ' . 'WHERE `feeds`.`user_id` = ? ' . 'AND (`feeds`.`folder_id` = 0 ' . 'OR `folders`.`deleted_at` = 0' . @@ -80,7 +80,7 @@ public function findAll(){ // work because prepared statements dont work. This is a // POSSIBLE SQL INJECTION RISK WHEN MODIFIED WITHOUT THOUGHT. // think twice when changing this - 'AND unread = true ' . + 'AND unread = 1 ' . 'WHERE (`feeds`.`folder_id` = 0 ' . 'OR `folders`.`deleted_at` = 0' . ')' . @@ -100,7 +100,7 @@ public function findByUrlHash($hash, $userId){ // work because prepared statements dont work. This is a // POSSIBLE SQL INJECTION RISK WHEN MODIFIED WITHOUT THOUGHT. // think twice when changing this - 'AND unread = true ' . + 'AND unread = 1 ' . 'WHERE `feeds`.`url_hash` = ? ' . 'AND `feeds`.`user_id` = ? ' . 'GROUP BY `feeds`.`id`'; diff --git a/lib/Db/ItemMapper.php b/lib/Db/ItemMapper.php index ae47c7eb83..08415c64b6 100644 --- a/lib/Db/ItemMapper.php +++ b/lib/Db/ItemMapper.php @@ -55,9 +55,9 @@ private function buildStatusQueryPart($showAll, $type = null) { $sql = ''; if (isset($type) && $type === FeedType::STARRED) { - $sql = 'AND `items`.`starred` = true '; + $sql = 'AND `items`.`starred` = 1 '; } elseif (!$showAll) { - $sql .= 'AND `items`.`unread` = true '; + $sql .= 'AND `items`.`unread` = 1 '; } return $sql; @@ -96,7 +96,7 @@ public function starredCount($userId) { 'ON `feeds`.`id` = `items`.`feed_id` ' . 'AND `feeds`.`deleted_at` = 0 ' . 'AND `feeds`.`user_id` = ? ' . - 'AND `items`.`starred` = true ' . + 'AND `items`.`starred` = 1 ' . 'LEFT OUTER JOIN `*PREFIX*news_folders` `folders` ' . 'ON `folders`.`id` = `feeds`.`folder_id` ' . 'WHERE `feeds`.`folder_id` = 0 ' . @@ -112,7 +112,7 @@ public function starredCount($userId) { public function readAll($highestItemId, $time, $userId) { $sql = 'UPDATE `*PREFIX*news_items` ' . - 'SET unread = false ' . + 'SET unread = 0 ' . ', `last_modified` = ? ' . 'WHERE `feed_id` IN (' . 'SELECT `id` FROM `*PREFIX*news_feeds` ' . @@ -126,7 +126,7 @@ public function readAll($highestItemId, $time, $userId) { public function readFolder($folderId, $highestItemId, $time, $userId) { $sql = 'UPDATE `*PREFIX*news_items` ' . - 'SET unread = false ' . + 'SET unread = 0 ' . ', `last_modified` = ? ' . 'WHERE `feed_id` IN (' . 'SELECT `id` FROM `*PREFIX*news_feeds` ' . @@ -142,7 +142,7 @@ public function readFolder($folderId, $highestItemId, $time, $userId) { public function readFeed($feedId, $highestItemId, $time, $userId) { $sql = 'UPDATE `*PREFIX*news_items` ' . - 'SET unread = false ' . + 'SET unread = 0 ' . ', `last_modified` = ? ' . 'WHERE `feed_id` = ? ' . 'AND `id` <= ? ' . @@ -269,7 +269,7 @@ public function findAll($limit, $offset, $type, $showAll, $oldestFirst, $userId, public function findAllUnreadOrStarred($userId) { $params = [$userId]; - $sql = 'AND `items`.`unread` = true OR `items`.`starred` = true '; + $sql = 'AND `items`.`unread` = 1 OR `items`.`starred` = 1 '; $sql = $this->makeSelectQuery($sql); return $this->findEntities($sql, $params); } @@ -297,8 +297,8 @@ public function deleteReadOlderThanThreshold($threshold) { 'FROM `*PREFIX*news_items` `items` ' . 'JOIN `*PREFIX*news_feeds` `feeds` ' . 'ON `feeds`.`id` = `items`.`feed_id` ' . - 'AND `items`.`unread` = false ' . - 'AND `items`.`starred` = false ' . + 'AND `items`.`unread` = 0 ' . + 'AND `items`.`starred` = 0 ' . 'GROUP BY `feeds`.`id`, `feeds`.`articles_per_update` ' . 'HAVING COUNT(*) > ?'; @@ -315,8 +315,8 @@ public function deleteReadOlderThanThreshold($threshold) { $sql = 'DELETE FROM `*PREFIX*news_items` ' . 'WHERE `id` IN (' . 'SELECT `id` FROM `*PREFIX*news_items` ' . - 'WHERE `items`.`unread` = false ' . - 'AND `items`.`starred` = false ' . + 'WHERE `items`.`unread` = 0 ' . + 'AND `items`.`starred` = 0 ' . 'AND `feed_id` = ? ' . 'ORDER BY `id` ASC ' . 'LIMIT ?' . @@ -402,7 +402,7 @@ public function readItem($itemId, $isRead, $lastModified, $userId) { // as unread if ($isRead) { $sql = 'UPDATE `*PREFIX*news_items` - SET `unread` = false, + SET `unread` = 0, `last_modified` = ? WHERE `fingerprint` = ? AND `feed_id` IN ( diff --git a/lib/Db/Mysql/ItemMapper.php b/lib/Db/Mysql/ItemMapper.php index 5e85400645..7b39ccc432 100644 --- a/lib/Db/Mysql/ItemMapper.php +++ b/lib/Db/Mysql/ItemMapper.php @@ -34,8 +34,8 @@ public function deleteReadOlderThanThreshold($threshold){ 'FROM `*PREFIX*news_items` `items` ' . 'JOIN `*PREFIX*news_feeds` `feeds` ' . 'ON `feeds`.`id` = `items`.`feed_id` ' . - 'AND `items`.`unread` = false ' . - 'AND `items`.`starred` = false ' . + 'AND `items`.`unread` = 0 ' . + 'AND `items`.`starred` = 0 ' . 'GROUP BY `feeds`.`id`, `feeds`.`articles_per_update` ' . 'HAVING COUNT(*) > ?'; $params = [$threshold]; @@ -50,8 +50,8 @@ public function deleteReadOlderThanThreshold($threshold){ $params = [$row['feed_id'], $limit]; $sql = 'DELETE FROM `*PREFIX*news_items` ' . - 'WHERE `items`.`unread` = false ' . - 'AND `items`.`starred` = false ' . + 'WHERE `items`.`unread` = 0 ' . + 'AND `items`.`starred` = 0 ' . 'AND `feed_id` = ? ' . 'ORDER BY `id` ASC ' . 'LIMIT ?'; @@ -69,7 +69,7 @@ public function readItem($itemId, $isRead, $lastModified, $userId) { $sql = 'UPDATE `*PREFIX*news_items` `items` JOIN `*PREFIX*news_feeds` `feeds` ON `feeds`.`id` = `items`.`feed_id` - SET `items`.`unread` = false, + SET `items`.`unread` = 0, `items`.`last_modified` = ? WHERE `items`.`fingerprint` = ? AND `feeds`.`user_id` = ?'; diff --git a/lib/Migration/MigrateStatusFlags.php b/lib/Migration/MigrateStatusFlags.php index 7f3b6f562a..8ff552907f 100644 --- a/lib/Migration/MigrateStatusFlags.php +++ b/lib/Migration/MigrateStatusFlags.php @@ -44,7 +44,7 @@ public function run(IOutput $output) { } $update = 'UPDATE `*PREFIX*news_items` ' . - 'SET unread = IF(status & 2, true, false), starred = IF(status & 4, true, false)'; + 'SET unread = IF(status & 2, 1, 0), starred = IF(status & 4, 1, 0)'; $output->startProgress(); $this->db->executeUpdate($update); From f8aa13f759b37990157481c19c90621c5005a9ed Mon Sep 17 00:00:00 2001 From: Daniel Opitz Date: Wed, 19 Jul 2017 01:00:45 +0200 Subject: [PATCH 05/11] add/fix unit tests for new boolean status --- lib/Db/Item.php | 26 +- lib/Db/ItemMapper.php | 6 +- lib/Db/Mysql/ItemMapper.php | 2 +- tests/Integration/Db/FeedMapperTest.php | 192 +++++- tests/Integration/Db/ItemMapperTest.php | 9 +- tests/Integration/Fixtures/ItemFixture.php | 3 +- tests/Integration/Fixtures/data/default.php | 14 +- tests/Integration/Fixtures/data/readitem.php | 12 +- tests/Integration/IntegrationTest.php | 5 +- tests/Unit/Db/FeedMapperTest.php | 303 ---------- tests/Unit/Db/ItemMapperTest.php | 550 ------------------ tests/Unit/Db/Mysql/ItemMapperTest.php | 119 ---- .../Unit/Migration/MigrateStatusFlagsTest.php | 65 +++ tests/Unit/Service/ItemServiceTest.php | 29 +- tests/Unit/Service/StatusFlagTest.php | 57 -- 15 files changed, 313 insertions(+), 1079 deletions(-) delete mode 100644 tests/Unit/Db/FeedMapperTest.php delete mode 100644 tests/Unit/Db/ItemMapperTest.php delete mode 100644 tests/Unit/Db/Mysql/ItemMapperTest.php create mode 100644 tests/Unit/Migration/MigrateStatusFlagsTest.php delete mode 100644 tests/Unit/Service/StatusFlagTest.php diff --git a/lib/Db/Item.php b/lib/Db/Item.php index e701e1e81a..efcd87a88b 100644 --- a/lib/Db/Item.php +++ b/lib/Db/Item.php @@ -43,8 +43,6 @@ * @method void setFingerprint(string $value) * @method void setContentHash(string $value) * @method void setSearchIndex(string $value) - * @method void setUnread(bool $value) - * @method void setStarred(bool $value) */ class Item extends Entity implements IAPI, \JsonSerializable { @@ -79,14 +77,36 @@ public function __construct() { $this->addType('starred', 'boolean'); } + public function setRead() { + $this->setUnread(false); + } + + public function isRead() { + return !$this->unread; + } + + public function setUnread($value = true) { + $this->markFieldUpdated('unread'); + $this->unread = (bool)$value; + } + public function isUnread() { - return $this->unread; + return (bool)$this->unread; + } + + public function setStarred($value = true) { + $this->markFieldUpdated('starred'); + $this->starred = (bool)$value; } public function isStarred() { return (bool)$this->starred; } + public function setUnstarred() { + $this->setStarred(false); + } + public function isUnstarred() { return !$this->starred; } diff --git a/lib/Db/ItemMapper.php b/lib/Db/ItemMapper.php index 08415c64b6..ff18ad5e65 100644 --- a/lib/Db/ItemMapper.php +++ b/lib/Db/ItemMapper.php @@ -269,7 +269,7 @@ public function findAll($limit, $offset, $type, $showAll, $oldestFirst, $userId, public function findAllUnreadOrStarred($userId) { $params = [$userId]; - $sql = 'AND `items`.`unread` = 1 OR `items`.`starred` = 1 '; + $sql = 'AND (`items`.`unread` = 1 OR `items`.`starred` = 1) '; $sql = $this->makeSelectQuery($sql); return $this->findEntities($sql, $params); } @@ -315,8 +315,8 @@ public function deleteReadOlderThanThreshold($threshold) { $sql = 'DELETE FROM `*PREFIX*news_items` ' . 'WHERE `id` IN (' . 'SELECT `id` FROM `*PREFIX*news_items` ' . - 'WHERE `items`.`unread` = 0 ' . - 'AND `items`.`starred` = 0 ' . + 'WHERE `unread` = 0 ' . + 'AND `starred` = 0 ' . 'AND `feed_id` = ? ' . 'ORDER BY `id` ASC ' . 'LIMIT ?' . diff --git a/lib/Db/Mysql/ItemMapper.php b/lib/Db/Mysql/ItemMapper.php index 7b39ccc432..b63d4f983b 100644 --- a/lib/Db/Mysql/ItemMapper.php +++ b/lib/Db/Mysql/ItemMapper.php @@ -26,7 +26,7 @@ public function __construct(IDBConnection $db, Time $time){ /** * Delete all items for feeds that have over $threshold unread and not * starred items - * @param int $threshold the number of items that should be deleted + * @param int $threshold the number of items that should be deleted */ public function deleteReadOlderThanThreshold($threshold){ $sql = 'SELECT (COUNT(*) - `feeds`.`articles_per_update`) AS `size`, ' . diff --git a/tests/Integration/Db/FeedMapperTest.php b/tests/Integration/Db/FeedMapperTest.php index 0c13280f54..b429149a44 100644 --- a/tests/Integration/Db/FeedMapperTest.php +++ b/tests/Integration/Db/FeedMapperTest.php @@ -6,49 +6,231 @@ * later. See the COPYING file. * * @author Bernhard Posselt + * @author Daniel Opitz * @copyright Bernhard Posselt 2015 + * @copyright Daniel Opitz 2017 */ namespace OCA\News\Db; -use \OCA\News\Tests\Integration\IntegrationTest; +use OCA\News\Tests\Integration\Fixtures\FeedFixture; +use OCA\News\Tests\Integration\IntegrationTest; class FeedMapperTest extends IntegrationTest { - public function testFind () { + $feed = new FeedFixture(); + $feed = $this->feedMapper->insert($feed); + + $fetched = $this->feedMapper->find($feed->getId(), $this->user); + + $this->assertInstanceOf(Feed::class, $fetched); + $this->assertEquals($feed->getLink(), $fetched->getLink()); + } + + /** + * @expectedException OCP\AppFramework\Db\DoesNotExistException + */ + public function testFindNotExisting () { + $this->feedMapper->find(0, $this->user); } - /* TBD public function testFindAll () { + $feeds = [ + [ + 'userId' => $this->user, + 'items' => [] + ], + [ + 'userId' => 'john', + 'items' => [] + ] + ]; + $this->loadFeedFixtures($feeds); + + $fetched = $this->feedMapper->findAll(); + + $this->assertInternalType('array', $fetched); + $this->assertCount(2, $fetched); + $this->assertContainsOnlyInstancesOf(Feed::class, $fetched); + + $this->tearDownUser('john'); + } + public function testFindAllEmpty () { + $feeds = $this->feedMapper->findAll(); + + $this->assertInternalType('array', $feeds); + $this->assertCount(0, $feeds); } public function testFindAllFromUser () { + $feeds = [ + [ + 'userId' => $this->user, + 'items' => [] + ], + [ + 'userId' => 'john', + 'items' => [] + ] + ]; + $this->loadFeedFixtures($feeds); + + $fetched = $this->feedMapper->findAllFromUser($this->user); + + $this->assertInternalType('array', $fetched); + $this->assertCount(1, $fetched); + $this->assertContainsOnlyInstancesOf(Feed::class, $fetched); + + $this->tearDownUser('john'); + } + + public function testFindAllFromUserNotExisting () { + $fetched = $this->feedMapper->findAllFromUser('notexistinguser'); + + $this->assertInternalType('array', $fetched); + $this->assertCount(0, $fetched); } public function testFindByUrlHash () { + $feed = new FeedFixture([ + 'urlHash' => 'someTestHash', + 'title' => 'Some Test Title' + ]); + $feed = $this->feedMapper->insert($feed); + + $fetched = $this->feedMapper->findByUrlHash($feed->getUrlHash(), $this->user); + $this->assertInstanceOf(Feed::class, $fetched); + $this->assertEquals($feed->getTitle(), $fetched->getTitle()); } + /** + * @expectedException OCP\AppFramework\Db\MultipleObjectsReturnedException + */ + public function testFindByUrlHashMoreThanOneResult () { + $feed1 = $this->feedMapper->insert(new FeedFixture([ + 'urlHash' => 'someTestHash' + ])); + $feed2 = $this->feedMapper->insert(new FeedFixture([ + 'urlHash' => 'someTestHash' + ])); + + $this->feedMapper->findByUrlHash($feed1->getUrlHash(), $this->user); + } - public function testDelete () { + /** + * @expectedException OCP\AppFramework\Db\DoesNotExistException + */ + public function testFindByUrlHashNotExisting () { + $this->feedMapper->findByUrlHash('some random hash', $this->user); } + public function testDelete () { + $this->loadFixtures('default'); + + $feeds = $this->feedMapper->findAllFromUser($this->user); + $this->assertCount(4, $feeds); + + $feed = reset($feeds); + + $items = $this->itemMapper->findAllFeed( + $feed->getId(), 100, 0, true, false, $this->user + ); + $this->assertCount(7, $items); + + $this->feedMapper->delete($feed); + + $this->assertCount(3, $this->feedMapper->findAllFromUser($this->user)); + + $items = $this->itemMapper->findAllFeed( + $feed->getId(), 100, 0, true, false, $this->user + ); + $this->assertCount(0, $items); + } + public function testGetToDelete () { + $this->loadFeedFixtures([ + ['deletedAt' => 1], + ['deletedAt' => 0], + ['deletedAt' => 1, 'userId' => 'john'], + ['deletedAt' => 1000] + ]); + + $fetched = $this->feedMapper->getToDelete(); + + $this->assertInternalType('array', $fetched); + $this->assertCount(3, $fetched); + $this->assertContainsOnlyInstancesOf(Feed::class, $fetched); + + $this->tearDownUser('john'); + } + + public function testGetToDeleteOlderThan () { + $this->loadFeedFixtures([ + ['deletedAt' => 1], + ['deletedAt' => 0], + ['deletedAt' => 1, 'userId' => 'john'], + ['deletedAt' => 1000] + ]); + + $fetched = $this->feedMapper->getToDelete(1000); + + $this->assertInternalType('array', $fetched); + $this->assertCount(2, $fetched); + $this->assertContainsOnlyInstancesOf(Feed::class, $fetched); + + $this->tearDownUser('john'); + } + public function testGetToDeleteUser () { + $this->loadFeedFixtures([ + ['deletedAt' => 1], + ['deletedAt' => 0], + ['deletedAt' => 1, 'userId' => 'john'], + ['deletedAt' => 1000] + ]); + + $fetched = $this->feedMapper->getToDelete(2000, $this->user); + + $this->assertInternalType('array', $fetched); + $this->assertCount(2, $fetched); + $this->assertContainsOnlyInstancesOf(Feed::class, $fetched); + + $this->tearDownUser('john'); } + public function testGetToDeleteEmpty () { + $fetched = $this->feedMapper->getToDelete(); + + $this->assertInternalType('array', $fetched); + $this->assertCount(0, $fetched); + } public function testDeleteUser () { + $this->loadFixtures('default'); - }*/ + $this->assertCount(4, $this->feedMapper->findAllFromUser($this->user)); + $items = $this->itemMapper->findAll(100, 0, 0, true, false, $this->user); + $this->assertCount(9, $items); + $this->feedMapper->deleteUser($this->user); + + $this->assertCount(0, $this->feedMapper->findAllFromUser($this->user)); + + $items = $this->itemMapper->findAll(100, 0, 0, true, false, $this->user); + $this->assertCount(0, $items); + } + + public function testDeleteUserNotExisting () { + $this->feedMapper->deleteUser('notexistinguser'); + } } diff --git a/tests/Integration/Db/ItemMapperTest.php b/tests/Integration/Db/ItemMapperTest.php index 6b621e0708..da66351590 100644 --- a/tests/Integration/Db/ItemMapperTest.php +++ b/tests/Integration/Db/ItemMapperTest.php @@ -112,9 +112,8 @@ public function testReadAll () { $this->itemMapper->readAll(PHP_INT_MAX, 10, $this->user); - $status = StatusFlag::UNREAD; $items = $this->itemMapper->findAll( - 30, 0, $status, false, $this->user + 30, 0, 0, false, false, $this->user ); $this->assertEquals(0, count($items)); @@ -144,9 +143,8 @@ public function testReadFolder () { $folderId, PHP_INT_MAX, 10, $this->user ); - $status = StatusFlag::UNREAD; $items = $this->itemMapper->findAll( - 30, 0, $status, false, $this->user + 30, 0, 0, false, false, $this->user ); $this->assertEquals(1, count($items)); @@ -176,9 +174,8 @@ public function testReadFeed () { $feedId, PHP_INT_MAX, 10, $this->user ); - $status = StatusFlag::UNREAD; $items = $this->itemMapper->findAll( - 30, 0, $status, false, $this->user + 30, 0, 0, false, false, $this->user ); $this->assertEquals(2, count($items)); diff --git a/tests/Integration/Fixtures/ItemFixture.php b/tests/Integration/Fixtures/ItemFixture.php index 2dfe79c280..0832b0ef49 100644 --- a/tests/Integration/Fixtures/ItemFixture.php +++ b/tests/Integration/Fixtures/ItemFixture.php @@ -29,7 +29,8 @@ public function __construct(array $defaults=[]) { 'enclosureMime' => 'video/mpeg', 'enclosureLink' => 'http://google.de/web.webm', 'feedId' => 0, - 'status' => 2, + 'unread' => true, + 'starred' => false, 'lastModified' => 113, 'rtl' => false, ], $defaults); diff --git a/tests/Integration/Fixtures/data/default.php b/tests/Integration/Fixtures/data/default.php index d87bb1e6f6..862515b12f 100644 --- a/tests/Integration/Fixtures/data/default.php +++ b/tests/Integration/Fixtures/data/default.php @@ -20,12 +20,12 @@ 'articlesPerUpdate' => 1, 'items' => [ ['title' => 'a title1', 'guid' => 'abc'], - ['title' => 'a title2', 'status' => 4, 'guid' => 'def'], - ['title' => 'a title3', 'status' => 6, 'guid' => 'gih'], - ['title' => 'del1', 'status' => 0], - ['title' => 'del2', 'status' => 0], - ['title' => 'del3', 'status' => 0], - ['title' => 'del4', 'status' => 0] + ['title' => 'a title2', 'unread' => false, 'starred' => true, 'guid' => 'def'], + ['title' => 'a title3', 'unread' => true, 'starred' => true, 'guid' => 'gih'], + ['title' => 'del1', 'unread' => false, 'starred' => false], + ['title' => 'del2', 'unread' => false, 'starred' => false], + ['title' => 'del3', 'unread' => false, 'starred' => false], + ['title' => 'del4', 'unread' => false, 'starred' => false] ] ], [ @@ -69,7 +69,7 @@ 'title' => 'fourth feed', 'url' => 'http://blog.fefe.de', 'items' => [ - ['title' => 'no folder', 'status' => 0] + ['title' => 'no folder', 'unread' => false, 'starred' => false] ] ] ] diff --git a/tests/Integration/Fixtures/data/readitem.php b/tests/Integration/Fixtures/data/readitem.php index 0a587bad7c..8f953a8450 100644 --- a/tests/Integration/Fixtures/data/readitem.php +++ b/tests/Integration/Fixtures/data/readitem.php @@ -15,18 +15,18 @@ 'title' => 'john feed', 'userId' => 'john', 'items' => [ - ['title' => 'blubb', 'status' => 2], - ['title' => 'blubb', 'status' => 2] + ['title' => 'blubb', 'unread' => true, 'starred' => false], + ['title' => 'blubb', 'unread' => true, 'starred' => false] ] ], [ 'title' => 'test feed', 'userId' => 'test', 'items' => [ - ['title' => 'blubb', 'status' => 2], - ['title' => 'blubbs', 'status' => 2], - ['title' => 'blubb', 'status' => 2], - ['title' => 'blubb', 'status' => 2] + ['title' => 'blubb', 'unread' => true, 'starred' => false], + ['title' => 'blubbs', 'unread' => true, 'starred' => false], + ['title' => 'blubb', 'unread' => true, 'starred' => false], + ['title' => 'blubb', 'unread' => true, 'starred' => false] ] ] ] diff --git a/tests/Integration/IntegrationTest.php b/tests/Integration/IntegrationTest.php index c14d3007ac..f14a1263b6 100644 --- a/tests/Integration/IntegrationTest.php +++ b/tests/Integration/IntegrationTest.php @@ -124,7 +124,10 @@ protected function loadFeedFixtures(array $feedFixtures=[], $folderId=0) { $feed = new FeedFixture($feedFixture); $feed->setFolderId($folderId); $feedId = $this->loadFixture($feed); - $this->loadItemFixtures($feedFixture['items'], $feedId); + + if (!empty($feedFixture['items'])) { + $this->loadItemFixtures($feedFixture['items'], $feedId); + } } } diff --git a/tests/Unit/Db/FeedMapperTest.php b/tests/Unit/Db/FeedMapperTest.php deleted file mode 100644 index ed74b235e1..0000000000 --- a/tests/Unit/Db/FeedMapperTest.php +++ /dev/null @@ -1,303 +0,0 @@ - - * @author Bernhard Posselt - * @copyright Alessandro Cosentino 2012 - * @copyright Bernhard Posselt 2012, 2014 - */ - -namespace OCA\News\Db; - - -use OCA\News\Utility\Time; - -class FeedMapperTest extends \OCA\News\Tests\Unit\Db\MapperTestUtility { - - private $mapper; - private $feeds; - - protected function setUp(){ - parent::setUp(); - - $this->mapper = new FeedMapper($this->db, new Time()); - - // create mock feeds - $feed1 = new Feed(); - $feed2 = new Feed(); - - $this->feeds = [$feed1, $feed2]; - $this->user = 'herman'; - } - - - public function testFind(){ - $userId = 'john'; - $id = 3; - $rows = [ - ['id' => $this->feeds[0]->getId()], - ]; - $sql = 'SELECT `feeds`.*, COUNT(`items`.`id`) AS `unread_count` ' . - 'FROM `*PREFIX*news_feeds` `feeds` ' . - 'LEFT JOIN `*PREFIX*news_items` `items` ' . - 'ON `feeds`.`id` = `items`.`feed_id` ' . - 'AND (`items`.`status` & ' . StatusFlag::UNREAD . ') = ' . - StatusFlag::UNREAD . ' ' . - 'WHERE `feeds`.`id` = ? ' . - 'AND `feeds`.`user_id` = ? ' . - 'GROUP BY `feeds`.`id`'; - $params = [$id, $userId]; - $this->setMapperResult($sql, $params, $rows); - - $result = $this->mapper->find($id, $userId); - $this->assertEquals($this->feeds[0], $result); - - } - - - public function testFindNotFound(){ - $userId = 'john'; - $id = 3; - $sql = 'SELECT `feeds`.*, COUNT(`items`.`id`) AS `unread_count` ' . - 'FROM `*PREFIX*news_feeds` `feeds` ' . - 'LEFT JOIN `*PREFIX*news_items` `items` ' . - 'ON `feeds`.`id` = `items`.`feed_id` ' . - 'AND (`items`.`status` & ' . StatusFlag::UNREAD . ') = ' . - StatusFlag::UNREAD . ' ' . - 'WHERE `feeds`.`id` = ? ' . - 'AND `feeds`.`user_id` = ? ' . - 'GROUP BY `feeds`.`id`'; - $params = [$id, $userId]; - $this->setMapperResult($sql, $params); - - $this->setExpectedException( - '\OCP\AppFramework\Db\DoesNotExistException' - ); - $this->mapper->find($id, $userId); - } - - - public function testFindMoreThanOneResultFound(){ - $userId = 'john'; - $id = 3; - $rows = [ - ['id' => $this->feeds[0]->getId()], - ['id' => $this->feeds[1]->getId()] - ]; - $sql = 'SELECT `feeds`.*, COUNT(`items`.`id`) AS `unread_count` ' . - 'FROM `*PREFIX*news_feeds` `feeds` ' . - 'LEFT JOIN `*PREFIX*news_items` `items` ' . - 'ON `feeds`.`id` = `items`.`feed_id` ' . - 'AND (`items`.`status` & ' . StatusFlag::UNREAD . ') = ' . - StatusFlag::UNREAD . ' ' . - 'WHERE `feeds`.`id` = ? ' . - 'AND `feeds`.`user_id` = ? ' . - 'GROUP BY `feeds`.`id`'; - $params = [$id, $userId]; - $this->setMapperResult($sql, $params, $rows); - - $this->setExpectedException( - '\OCP\AppFramework\Db\MultipleObjectsReturnedException' - ); - $this->mapper->find($id, $userId); - } - - - public function testFindAll(){ - $rows = [ - ['id' => $this->feeds[0]->getId()], - ['id' => $this->feeds[1]->getId()] - ]; - $sql = 'SELECT `feeds`.*, COUNT(`items`.`id`) AS `unread_count` ' . - 'FROM `*PREFIX*news_feeds` `feeds` ' . - 'LEFT OUTER JOIN `*PREFIX*news_folders` `folders` '. - 'ON `feeds`.`folder_id` = `folders`.`id` ' . - 'LEFT JOIN `*PREFIX*news_items` `items` ' . - 'ON `feeds`.`id` = `items`.`feed_id` ' . - 'AND (`items`.`status` & ' . StatusFlag::UNREAD . ') = ' . - StatusFlag::UNREAD . ' ' . - 'WHERE (`feeds`.`folder_id` = 0 ' . - 'OR `folders`.`deleted_at` = 0' . - ')' . - 'AND `feeds`.`deleted_at` = 0 ' . - 'GROUP BY `feeds`.`id`'; - - $this->setMapperResult($sql, [], $rows); - - $result = $this->mapper->findAll(); - $this->assertEquals($this->feeds, $result); - } - - - public function testFindAllFromUser(){ - $userId = 'john'; - $rows = [ - ['id' => $this->feeds[0]->getId()], - ['id' => $this->feeds[1]->getId()] - ]; - $sql = 'SELECT `feeds`.*, COUNT(`items`.`id`) AS `unread_count` ' . - 'FROM `*PREFIX*news_feeds` `feeds` ' . - 'LEFT OUTER JOIN `*PREFIX*news_folders` `folders` '. - 'ON `feeds`.`folder_id` = `folders`.`id` ' . - 'LEFT JOIN `*PREFIX*news_items` `items` ' . - 'ON `feeds`.`id` = `items`.`feed_id` ' . - 'AND (`items`.`status` & ' . StatusFlag::UNREAD . ') = ' . - StatusFlag::UNREAD . ' ' . - 'WHERE `feeds`.`user_id` = ? ' . - 'AND (`feeds`.`folder_id` = 0 ' . - 'OR `folders`.`deleted_at` = 0' . - ')' . - 'AND `feeds`.`deleted_at` = 0 ' . - 'GROUP BY `feeds`.`id`'; - $this->setMapperResult($sql, [$userId], $rows); - - $result = $this->mapper->findAllFromUser($userId); - $this->assertEquals($this->feeds, $result); - } - - - public function testFindByUrlHash(){ - $urlHash = md5('hihi'); - $row = [['id' => $this->feeds[0]->getId()]]; - $sql = 'SELECT `feeds`.*, COUNT(`items`.`id`) AS `unread_count` ' . - 'FROM `*PREFIX*news_feeds` `feeds` ' . - 'LEFT JOIN `*PREFIX*news_items` `items` ' . - 'ON `feeds`.`id` = `items`.`feed_id` ' . - 'AND (`items`.`status` & ' . StatusFlag::UNREAD . ') = ' . - StatusFlag::UNREAD . ' ' . - 'WHERE `feeds`.`url_hash` = ? ' . - 'AND `feeds`.`user_id` = ? ' . - 'GROUP BY `feeds`.`id`'; - $this->setMapperResult($sql, [$urlHash, $this->user], $row); - - $result = $this->mapper->findByUrlHash($urlHash, $this->user); - $this->assertEquals($this->feeds[0], $result); - } - - - public function testFindByUrlHashNotFound(){ - $urlHash = md5('hihi'); - $sql = 'SELECT `feeds`.*, COUNT(`items`.`id`) AS `unread_count` ' . - 'FROM `*PREFIX*news_feeds` `feeds` ' . - 'LEFT JOIN `*PREFIX*news_items` `items` ' . - 'ON `feeds`.`id` = `items`.`feed_id` ' . - 'AND (`items`.`status` & ' . StatusFlag::UNREAD . ') = ' . - StatusFlag::UNREAD . ' ' . - 'WHERE `feeds`.`url_hash` = ? ' . - 'AND `feeds`.`user_id` = ? ' . - 'GROUP BY `feeds`.`id`'; - $this->setMapperResult($sql, [$urlHash, $this->user]); - - $this->setExpectedException( - '\OCP\AppFramework\Db\DoesNotExistException' - ); - $this->mapper->findByUrlHash($urlHash, $this->user); - } - - - public function testFindByUrlHashMoreThanOneResultFound(){ - $urlHash = md5('hihi'); - $rows = [ - ['id' => $this->feeds[0]->getId()], - ['id' => $this->feeds[1]->getId()] - ]; - $sql = 'SELECT `feeds`.*, COUNT(`items`.`id`) AS `unread_count` ' . - 'FROM `*PREFIX*news_feeds` `feeds` ' . - 'LEFT JOIN `*PREFIX*news_items` `items` ' . - 'ON `feeds`.`id` = `items`.`feed_id` ' . - 'AND (`items`.`status` & ' . StatusFlag::UNREAD . ') = ' . - StatusFlag::UNREAD . ' ' . - 'WHERE `feeds`.`url_hash` = ? ' . - 'AND `feeds`.`user_id` = ? ' . - 'GROUP BY `feeds`.`id`'; - $this->setMapperResult($sql, [$urlHash, $this->user], $rows); - - $this->setExpectedException( - '\OCP\AppFramework\Db\MultipleObjectsReturnedException' - ); - $this->mapper->findByUrlHash($urlHash, $this->user); - } - - - public function testDelete(){ - $feed = new Feed(); - $feed->setId(3); - - $sql = 'DELETE FROM `*PREFIX*news_feeds` WHERE `id` = ?'; - $arguments = [$feed->getId()]; - - $sql2 = 'DELETE FROM `*PREFIX*news_items` WHERE `feed_id` = ?'; - $arguments2 = [$feed->getId()]; - - $this->setMapperResult($sql, $arguments); - $this->setMapperResult($sql2, $arguments2); - - $this->mapper->delete($feed); - - } - - - public function testGetPurgeDeleted(){ - $rows = [ - ['id' => $this->feeds[0]->getId()], - ['id' => $this->feeds[1]->getId()] - ]; - $deleteOlderThan = 110; - $sql = 'SELECT * FROM `*PREFIX*news_feeds` ' . - 'WHERE `deleted_at` > 0 ' . - 'AND `deleted_at` < ? '; - $this->setMapperResult($sql, [$deleteOlderThan], $rows); - $result = $this->mapper->getToDelete($deleteOlderThan); - - $this->assertEquals($this->feeds, $result); - } - - - public function testGetPurgeDeletedFromUser(){ - $rows = [ - ['id' => $this->feeds[0]->getId()], - ['id' => $this->feeds[1]->getId()] - ]; - $deleteOlderThan = 110; - $sql = 'SELECT * FROM `*PREFIX*news_feeds` ' . - 'WHERE `deleted_at` > 0 ' . - 'AND `deleted_at` < ? ' . - 'AND `user_id` = ?'; - $this->setMapperResult($sql, [$deleteOlderThan, $this->user], $rows); - $result = $this->mapper->getToDelete($deleteOlderThan, $this->user); - - $this->assertEquals($this->feeds, $result); - } - - - public function testGetAllPurgeDeletedFromUser(){ - $rows = [ - ['id' => $this->feeds[0]->getId()], - ['id' => $this->feeds[1]->getId()] - ]; - - $sql = 'SELECT * FROM `*PREFIX*news_feeds` ' . - 'WHERE `deleted_at` > 0 ' . - 'AND `user_id` = ?'; - $this->setMapperResult($sql, [$this->user], $rows); - $result = $this->mapper->getToDelete(null, $this->user); - - $this->assertEquals($this->feeds, $result); - } - - - public function testDeleteFromUser(){ - $userId = 'john'; - $sql = 'DELETE FROM `*PREFIX*news_feeds` WHERE `user_id` = ?'; - - $this->setMapperResult($sql, [$userId]); - - $this->mapper->deleteUser($userId); - } - - -} diff --git a/tests/Unit/Db/ItemMapperTest.php b/tests/Unit/Db/ItemMapperTest.php deleted file mode 100644 index 082ff6ff94..0000000000 --- a/tests/Unit/Db/ItemMapperTest.php +++ /dev/null @@ -1,550 +0,0 @@ - - * @author Bernhard Posselt - * @copyright Alessandro Cosentino 2012 - * @copyright Bernhard Posselt 2012, 2014 - */ - -namespace OCA\News\Db; - - -use OCA\News\Utility\Time; - -class ItemMapperTest extends \OCA\News\Tests\Unit\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, new Time()); - - // 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; - - } - - - private function makeSelectQuery($prependTo, $oldestFirst=false){ - if ($oldestFirst) { - $ordering = 'ASC'; - } else { - $ordering = 'DESC'; - } - - 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` ' . $ordering; - } - - private function makeSelectQueryStatus($prependTo, $status, - $oldestFirst=false, $search=[]) { - $status = (int) $status; - - // WARNING: Potential SQL injection if you change this carelessly - $sql = 'AND ((`items`.`status` & ' . $status . ') = ' . $status . ') '; - - foreach ($search as $_) { - $sql .= 'AND `items`.`search_index` LIKE ? '; - } - - $sql .= $prependTo; - - return $this->makeSelectQuery($sql, $oldestFirst); - } - - - public function testFind(){ - $sql = $this->makeSelectQuery('AND `items`.`id` = ? '); - - $this->setMapperResult( - $sql, [$this->userId, $this->id], $this->row - ); - - $result = $this->mapper->find($this->id, $this->userId); - $this->assertEquals($this->items[0], $result); - } - - - public function testGetStarredCount(){ - $userId = 'john'; - $row = [['size' => 9]]; - $sql = 'SELECT COUNT(*) AS size 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` = ? ' . - 'AND ((`items`.`status` & ' . StatusFlag::STARRED . ') = ' . - StatusFlag::STARRED . ')' . - 'LEFT OUTER JOIN `*PREFIX*news_folders` `folders` ' . - 'ON `folders`.`id` = `feeds`.`folder_id` ' . - 'WHERE `feeds`.`folder_id` = 0 ' . - 'OR `folders`.`deleted_at` = 0'; - - $this->setMapperResult($sql, [$userId], $row); - - $result = $this->mapper->starredCount($userId); - $this->assertEquals($row[0]['size'], $result); - } - - - public function testReadAll(){ - $sql = 'UPDATE `*PREFIX*news_items` ' . - 'SET `status` = `status` & ? ' . - ', `last_modified` = ? ' . - 'WHERE `feed_id` IN (' . - 'SELECT `id` FROM `*PREFIX*news_feeds` ' . - 'WHERE `user_id` = ? ' . - ') '. - 'AND `id` <= ?'; - $params = [~StatusFlag::UNREAD, $this->updatedSince, $this->user, 3]; - $this->setMapperResult($sql, $params); - $this->mapper->readAll(3, $this->updatedSince, $this->user); - } - - - public function testReadFolder(){ - $sql = 'UPDATE `*PREFIX*news_items` ' . - 'SET `status` = `status` & ? ' . - ', `last_modified` = ? ' . - 'WHERE `feed_id` IN (' . - 'SELECT `id` FROM `*PREFIX*news_feeds` ' . - 'WHERE `folder_id` = ? ' . - 'AND `user_id` = ? ' . - ') '. - 'AND `id` <= ?'; - $params = [~StatusFlag::UNREAD, $this->updatedSince, 3, $this->user, 6]; - $this->setMapperResult($sql, $params); - $this->mapper->readFolder(3, 6, $this->updatedSince, $this->user); - } - - - public function testReadFeed(){ - $sql = 'UPDATE `*PREFIX*news_items` ' . - 'SET `status` = `status` & ? ' . - ', `last_modified` = ? ' . - 'WHERE `feed_id` = ? ' . - 'AND `id` <= ? ' . - 'AND EXISTS (' . - 'SELECT * FROM `*PREFIX*news_feeds` ' . - 'WHERE `user_id` = ? ' . - 'AND `id` = ? ) '; - $params = [ - ~StatusFlag::UNREAD, $this->updatedSince, 3, 6, $this->user, 3 - ]; - $this->setMapperResult($sql, $params); - $this->mapper->readFeed(3, 6, $this->updatedSince, $this->user); - } - - - public function testFindAllNew(){ - $sql = 'AND `items`.`last_modified` >= ? '; - $sql = $this->makeSelectQueryStatus($sql, $this->status); - $params = [$this->user, $this->updatedSince]; - - $this->setMapperResult($sql, $params, $this->rows); - $result = $this->mapper->findAllNew($this->updatedSince, - $this->status, $this->user); - - $this->assertEquals($this->items, $result); - } - - - public function testFindAllNewFolder(){ - $sql = 'AND `feeds`.`folder_id` = ? ' . - 'AND `items`.`last_modified` >= ? '; - $sql = $this->makeSelectQueryStatus($sql, $this->status); - - $params = [$this->user, $this->id, $this->updatedSince]; - $this->setMapperResult($sql, $params, $this->rows); - $result = $this->mapper->findAllNewFolder($this->id, - $this->updatedSince, $this->status, $this->user); - - $this->assertEquals($this->items, $result); - } - - - public function testFindAllNewFeed(){ - $sql = 'AND `items`.`feed_id` = ? ' . - 'AND `items`.`last_modified` >= ? '; - $sql = $this->makeSelectQueryStatus($sql, $this->status); - $params = [$this->user, $this->id, $this->updatedSince]; - - $this->setMapperResult($sql, $params, $this->rows); - $result = $this->mapper->findAllNewFeed($this->id, $this->updatedSince, - $this->status, $this->user); - - $this->assertEquals($this->items, $result); - } - - - public function testFindAllUnreadOrStarred(){ - $status = StatusFlag::UNREAD | StatusFlag::STARRED; - $sql = 'AND ((`items`.`status` & ' . $status . ') > 0) '; - $sql = $this->makeSelectQuery($sql); - $params = [$this->user]; - $this->setMapperResult($sql, $params, $this->rows); - $result = $this->mapper->findAllUnreadOrStarred($this->user); - - $this->assertEquals($this->items, $result); - } - - - public function testFindAllFeed(){ - $sql = 'AND `items`.`feed_id` = ? ' . - 'AND `items`.`id` < ? '; - $sql = $this->makeSelectQueryStatus($sql, $this->status); - $params = [$this->user, $this->id, $this->offset]; - $this->setMapperResult($sql, $params, $this->rows, $this->limit); - $result = $this->mapper->findAllFeed($this->id, $this->limit, - $this->offset, $this->status, false, $this->user); - - $this->assertEquals($this->items, $result); - } - - - public function testFindAllFeedSearch(){ - $sql = 'AND `items`.`feed_id` = ? ' . - 'AND `items`.`id` < ? '; - $search = ['%test_\\', 'a']; - $sql = $this->makeSelectQueryStatus($sql, $this->status, false, $search); - $params = [$this->user, '%\%test\\_\\\\%', '%a%', $this->id, $this->offset]; - $this->setMapperResult($sql, $params, $this->rows, $this->limit); - $result = $this->mapper->findAllFeed($this->id, $this->limit, - $this->offset, $this->status, false, $this->user, $search); - - $this->assertEquals($this->items, $result); - } - - - public function testFindAllFeedNegativeLimit(){ - $sql = 'AND `items`.`feed_id` = ? ' . - 'AND `items`.`id` < ? '; - $sql = $this->makeSelectQueryStatus($sql, $this->status); - $params = [$this->user, $this->id, $this->offset]; - $this->setMapperResult($sql, $params, $this->rows); - $result = $this->mapper->findAllFeed($this->id, -1, - $this->offset, $this->status, false, $this->user); - - $this->assertEquals($this->items, $result); - } - - - public function testFindAllFeedOldestFirst(){ - $sql = 'AND `items`.`feed_id` = ? ' . - 'AND `items`.`id` > ? '; - $sql = $this->makeSelectQueryStatus($sql, $this->status, true); - $params = [$this->user, $this->id, $this->offset]; - $this->setMapperResult($sql, $params, $this->rows, $this->limit); - $result = $this->mapper->findAllFeed($this->id, $this->limit, - $this->offset, $this->status, true, $this->user); - - $this->assertEquals($this->items, $result); - } - - - public function testFindAllFeedOffsetZero(){ - $sql = 'AND `items`.`feed_id` = ? '; - $sql = $this->makeSelectQueryStatus($sql, $this->status); - $params = [$this->user, $this->id]; - $this->setMapperResult($sql, $params, $this->rows, $this->limit); - $result = $this->mapper->findAllFeed($this->id, $this->limit, - 0, $this->status, false, $this->user); - - $this->assertEquals($this->items, $result); - } - - - public function testFindAllFolder(){ - $sql = 'AND `feeds`.`folder_id` = ? ' . - 'AND `items`.`id` < ? '; - $sql = $this->makeSelectQueryStatus($sql, $this->status); - $params = [$this->user, $this->id, $this->offset]; - $this->setMapperResult($sql, $params, $this->rows, $this->limit); - $result = $this->mapper->findAllFolder($this->id, $this->limit, - $this->offset, $this->status, false, $this->user); - - $this->assertEquals($this->items, $result); - } - - public function testFindAllFolderSearch(){ - $sql = 'AND `feeds`.`folder_id` = ? ' . - 'AND `items`.`id` < ? '; - $search = ['%test_\\', 'a']; - $sql = $this->makeSelectQueryStatus($sql, $this->status, false, $search); - $params = [$this->user, '%\%test\\_\\\\%', '%a%', $this->id, $this->offset]; - $this->setMapperResult($sql, $params, $this->rows, $this->limit); - $result = $this->mapper->findAllFolder($this->id, $this->limit, - $this->offset, $this->status, false, $this->user, $search); - - $this->assertEquals($this->items, $result); - } - - public function testFindAllFolderNegativeLimit(){ - $sql = 'AND `feeds`.`folder_id` = ? ' . - 'AND `items`.`id` < ? '; - $sql = $this->makeSelectQueryStatus($sql, $this->status); - $params = [$this->user, $this->id, $this->offset]; - $this->setMapperResult($sql, $params, $this->rows); - $result = $this->mapper->findAllFolder($this->id, -1, - $this->offset, $this->status, false, $this->user); - - $this->assertEquals($this->items, $result); - } - - - - public function testFindAllFolderOldestFirst(){ - $sql = 'AND `feeds`.`folder_id` = ? ' . - 'AND `items`.`id` > ? '; - $sql = $this->makeSelectQueryStatus($sql, $this->status, true); - $params = [$this->user, $this->id, $this->offset]; - $this->setMapperResult($sql, $params, $this->rows, $this->limit); - $result = $this->mapper->findAllFolder($this->id, $this->limit, - $this->offset, $this->status, true, $this->user); - - $this->assertEquals($this->items, $result); - } - - - public function testFindAllFolderOffsetZero(){ - $sql = 'AND `feeds`.`folder_id` = ? '; - $sql = $this->makeSelectQueryStatus($sql, $this->status); - $params = [$this->user, $this->id]; - $this->setMapperResult($sql, $params, $this->rows, $this->limit); - $result = $this->mapper->findAllFolder($this->id, $this->limit, - 0, $this->status, false, $this->user); - - $this->assertEquals($this->items, $result); - } - - - public function testFindAll(){ - $sql = 'AND `items`.`id` < ? '; - $sql = $this->makeSelectQueryStatus($sql, $this->status); - $params = [$this->user, $this->offset]; - $this->setMapperResult($sql, $params, $this->rows, $this->limit); - $result = $this->mapper->findAll($this->limit, - $this->offset, $this->status, false, $this->user); - - $this->assertEquals($this->items, $result); - } - - - public function testFindAllSearch(){ - $sql = 'AND `items`.`id` < ? '; - $search = ['%tEst_\\', 'a']; - $params = [$this->user, '%\%test\\_\\\\%', '%a%', $this->offset]; - $sql = $this->makeSelectQueryStatus($sql, $this->status, false, $search); - $this->setMapperResult($sql, $params, $this->rows, $this->limit); - $result = $this->mapper->findAll($this->limit, - $this->offset, $this->status, false, $this->user, $search); - - $this->assertEquals($this->items, $result); - } - - - public function testFindAllNegativeLimit(){ - $sql = 'AND `items`.`id` < ? '; - $sql = $this->makeSelectQueryStatus($sql, $this->status); - $params = [$this->user, $this->offset]; - $this->setMapperResult($sql, $params, $this->rows, null); - $result = $this->mapper->findAll(-1, - $this->offset, $this->status, false, $this->user); - - $this->assertEquals($this->items, $result); - } - - - public function testFindAllOldestFirst(){ - $sql = 'AND `items`.`id` > ? '; - $sql = $this->makeSelectQueryStatus($sql, $this->status, true); - $params = [$this->user, $this->offset]; - $this->setMapperResult($sql, $params, $this->rows, $this->limit); - $result = $this->mapper->findAll($this->limit, - $this->offset, $this->status, true, $this->user); - - $this->assertEquals($this->items, $result); - } - - - public function testFindAllOffsetZero(){ - $sql = $this->makeSelectQueryStatus('', $this->status); - $params = [$this->user]; - $this->setMapperResult($sql, $params, $this->rows, $this->limit); - $result = $this->mapper->findAll($this->limit, - 0, $this->status, false, $this->user); - - $this->assertEquals($this->items, $result); - } - - - - - public function testFindByGuidHash(){ - $hash = md5('test'); - $feedId = 3; - $sql = $this->makeSelectQuery( - 'AND `items`.`guid_hash` = ? ' . - 'AND `feeds`.`id` = ? '); - - $this->setMapperResult( - $sql, [$this->userId, $hash, $feedId], $this->row); - - $result = $this->mapper->findByGuidHash($hash, $feedId, $this->userId); - $this->assertEquals($this->items[0], $result); - } - - - public function testDeleteReadOlderThanThresholdDoesNotDelete(){ - $status = StatusFlag::STARRED | StatusFlag::UNREAD; - $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 `feeds`.`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`' . - ', `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 `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]; - $this->setMapperResult($sql1, $params1, [$row]); - $this->setMapperResult($sql2, $params2); - - $this->mapper->deleteReadOlderThanThreshold($threshold); - } - - - public function testGetNewestItem() { - $sql = 'SELECT MAX(`items`.`id`) AS `max_id` ' . - 'FROM `*PREFIX*news_items` `items` '. - 'JOIN `*PREFIX*news_feeds` `feeds` ' . - 'ON `feeds`.`id` = `items`.`feed_id` '. - 'AND `feeds`.`user_id` = ?'; - $params = [$this->user]; - $rows = [['max_id' => 3]]; - - $this->setMapperResult($sql, $params, $rows); - - $result = $this->mapper->getNewestItemId($this->user); - $this->assertEquals(3, $result); - } - - - public function testGetNewestItemIdNotFound() { - $sql = 'SELECT MAX(`items`.`id`) AS `max_id` ' . - 'FROM `*PREFIX*news_items` `items` '. - 'JOIN `*PREFIX*news_feeds` `feeds` ' . - 'ON `feeds`.`id` = `items`.`feed_id` '. - 'AND `feeds`.`user_id` = ?'; - $params = [$this->user]; - $rows = []; - - $this->setMapperResult($sql, $params, $rows); - $this->setExpectedException( - '\OCP\AppFramework\Db\DoesNotExistException' - ); - - $this->mapper->getNewestItemId($this->user); - } - - - public function testDeleteFromUser(){ - $userId = 'john'; - $sql = 'DELETE FROM `*PREFIX*news_items` ' . - 'WHERE `feed_id` IN (' . - 'SELECT `feeds`.`id` FROM `*PREFIX*news_feeds` `feeds` ' . - 'WHERE `feeds`.`user_id` = ?' . - ')'; - - $this->setMapperResult($sql, [$userId]); - - $this->mapper->deleteUser($userId); - } - - -} \ No newline at end of file diff --git a/tests/Unit/Db/Mysql/ItemMapperTest.php b/tests/Unit/Db/Mysql/ItemMapperTest.php deleted file mode 100644 index e058d80b41..0000000000 --- a/tests/Unit/Db/Mysql/ItemMapperTest.php +++ /dev/null @@ -1,119 +0,0 @@ - - * @author Bernhard Posselt - * @copyright Alessandro Cosentino 2012 - * @copyright Bernhard Posselt 2012, 2014 - */ - -namespace OCA\News\Db\Mysql; - -use \OCA\News\Db\Item; -use \OCA\News\Db\StatusFlag; -use OCA\News\Utility\Time; - -class ItemMapperTest extends \OCA\News\Tests\Unit\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, new Time()); - - // 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 testDeleteReadOlderThanThresholdDoesNotDeleteBelow(){ - $status = StatusFlag::STARRED | StatusFlag::UNREAD; - $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 `feeds`.`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`' . - ', `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 `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 ' . - 'LIMIT ?'; - $params2 = [$status, 30, 1]; - - - $this->setMapperResult($sql1, $params1, [$row]); - $this->setMapperResult($sql2, $params2); - - $this->mapper->deleteReadOlderThanThreshold($threshold); - } - - -} \ No newline at end of file diff --git a/tests/Unit/Migration/MigrateStatusFlagsTest.php b/tests/Unit/Migration/MigrateStatusFlagsTest.php new file mode 100644 index 0000000000..3718b71cef --- /dev/null +++ b/tests/Unit/Migration/MigrateStatusFlagsTest.php @@ -0,0 +1,65 @@ + + * @copyright Daniel Opitz 2017 + */ + +namespace OCA\News\Migration; + +use OCP\IConfig; +use OCP\IDBConnection; +use OCP\Migration\IOutput; +use Test\TestCase; + +class MigrateStatusFlagsTest extends TestCase { + + /** @var IDBConnection|\PHPUnit_Framework_MockObject_MockObject */ + protected $db; + /** @var IConfig|\PHPUnit_Framework_MockObject_MockObject */ + protected $config; + /** @var IOutput|\PHPUnit_Framework_MockObject_MockObject */ + protected $output; + + protected function setUp() { + $this->db = $this->createMock(IDBConnection::class); + $this->config = $this->createMock(IConfig::class); + $this->output = $this->createMock(IOutput::class); + } + + public function testRun() { + $sql = $update = 'UPDATE `*PREFIX*news_items` ' . + 'SET unread = IF(status & 2, 1, 0), starred = IF(status & 4, 1, 0)'; + + $this->config->expects($this->exactly(1)) + ->method('getAppValue') + ->with('news', 'installed_version', '0.0.0') + ->willReturn('11.0.5'); + $this->db->expects($this->exactly(1)) + ->method('executeUpdate') + ->with($sql); + $this->output->expects($this->exactly(1)) + ->method('startProgress'); + $this->output->expects($this->exactly(1)) + ->method('finishProgress'); + + $migration = new MigrateStatusFlags($this->db, $this->config); + $migration->run($this->output); + } + + public function testRunNewerVersion() { + $this->config->expects($this->exactly(1)) + ->method('getAppValue') + ->with('news', 'installed_version', '0.0.0') + ->willReturn('11.1.0'); + $this->db->expects($this->exactly(0)) + ->method('executeUpdate'); + + $migration = new MigrateStatusFlags($this->db, $this->config); + $migration->run($this->output); + } +} \ No newline at end of file diff --git a/tests/Unit/Service/ItemServiceTest.php b/tests/Unit/Service/ItemServiceTest.php index 3b31971361..5e1cb4cae1 100644 --- a/tests/Unit/Service/ItemServiceTest.php +++ b/tests/Unit/Service/ItemServiceTest.php @@ -16,13 +16,13 @@ use \OCP\AppFramework\Db\DoesNotExistException; use \OCA\News\Db\Item; -use \OCA\News\Db\StatusFlag; use \OCA\News\Db\FeedType; class ItemServiceTest extends \PHPUnit_Framework_TestCase { private $mapper; + /** @var ItemService */ private $itemService; private $user; private $response; @@ -46,13 +46,6 @@ protected function setUp(){ $this->mapper = $this->getMockBuilder('\OCA\News\Db\ItemMapper') ->disableOriginalConstructor() ->getMock(); - $this->statusFlag = $this->getMockBuilder('\OCA\News\Db\StatusFlag') - ->disableOriginalConstructor() - ->getMock(); - $this->status = StatusFlag::STARRED; - $this->statusFlag->expects($this->any()) - ->method('typeToStatus') - ->will($this->returnValue($this->status)); $this->config = $this->getMockBuilder( '\OCA\News\Config\Config') ->disableOriginalConstructor() @@ -62,8 +55,7 @@ protected function setUp(){ ->disableOriginalConstructor() ->getMock(); $this->itemService = new ItemService($this->mapper, - $this->statusFlag, $this->timeFactory, $this->config, - $this->systemConfig); + $this->timeFactory, $this->config, $this->systemConfig); $this->user = 'jack'; $this->id = 3; $this->updatedSince = 20333; @@ -80,7 +72,7 @@ public function testFindAllNewFeed(){ ->method('findAllNewFeed') ->with($this->equalTo($this->id), $this->equalTo($this->updatedSince), - $this->equalTo($this->status), + $this->equalTo($this->showAll), $this->equalTo($this->user)) ->will($this->returnValue($this->response)); @@ -97,7 +89,7 @@ public function testFindAllNewFolder(){ ->method('findAllNewFolder') ->with($this->equalTo($this->id), $this->equalTo($this->updatedSince), - $this->equalTo($this->status), + $this->equalTo($this->showAll), $this->equalTo($this->user)) ->will($this->returnValue($this->response)); @@ -113,7 +105,8 @@ public function testFindAllNew(){ $this->mapper->expects($this->once()) ->method('findAllNew') ->with( $this->equalTo($this->updatedSince), - $this->equalTo($this->status), + $this->equalTo($type), + $this->equalTo($this->showAll), $this->equalTo($this->user)) ->will($this->returnValue($this->response)); @@ -131,7 +124,7 @@ public function testFindAllFeed(){ ->with($this->equalTo($this->id), $this->equalTo($this->limit), $this->equalTo($this->offset), - $this->equalTo($this->status), + $this->equalTo($this->showAll), $this->equalTo(false), $this->equalTo($this->user), $this->equalTo([])) @@ -152,7 +145,7 @@ public function testFindAllFolder(){ ->with($this->equalTo($this->id), $this->equalTo($this->limit), $this->equalTo($this->offset), - $this->equalTo($this->status), + $this->equalTo($this->showAll), $this->equalTo(true), $this->equalTo($this->user), $this->equalTo([])) @@ -172,7 +165,8 @@ public function testFindAll(){ ->method('findAll') ->with( $this->equalTo($this->limit), $this->equalTo($this->offset), - $this->equalTo($this->status), + $this->equalTo($type), + $this->equalTo($this->showAll), $this->equalTo(true), $this->equalTo($this->user), $this->equalTo([])) @@ -193,7 +187,8 @@ public function testFindAllSearch(){ ->method('findAll') ->with( $this->equalTo($this->limit), $this->equalTo($this->offset), - $this->equalTo($this->status), + $this->equalTo($type), + $this->equalTo($this->showAll), $this->equalTo(true), $this->equalTo($this->user), $this->equalTo($search)) diff --git a/tests/Unit/Service/StatusFlagTest.php b/tests/Unit/Service/StatusFlagTest.php deleted file mode 100644 index 5d672fc531..0000000000 --- a/tests/Unit/Service/StatusFlagTest.php +++ /dev/null @@ -1,57 +0,0 @@ - - * @author Bernhard Posselt - * @copyright Alessandro Cosentino 2012 - * @copyright Bernhard Posselt 2012, 2014 - */ - -namespace OCA\News\Db; - - -class StatusFlagTest extends \PHPUnit_Framework_TestCase { - - private $statusFlag; - - protected function setUp(){ - $this->statusFlag = new StatusFlag(); - } - - - public function testTypeToStatusUnreadStarred(){ - $expected = StatusFlag::STARRED; - $status = $this->statusFlag->typeToStatus(FeedType::STARRED, false); - - $this->assertEquals($expected, $status); - } - - - public function testTypeToStatusUnread(){ - $expected = StatusFlag::UNREAD; - $status = $this->statusFlag->typeToStatus(FeedType::FEED, false); - - $this->assertEquals($expected, $status); - } - - - public function testTypeToStatusReadStarred(){ - $expected = StatusFlag::STARRED; - $status = $this->statusFlag->typeToStatus(FeedType::STARRED, true); - - $this->assertEquals($expected, $status); - } - - - public function testTypeToStatusRead(){ - $expected = (~StatusFlag::UNREAD) & 0; - $status = $this->statusFlag->typeToStatus(FeedType::FEED, true); - - $this->assertEquals($expected, $status); - } - -} \ No newline at end of file From 997ac424ca6e437d5474b3b08068e42a65815768 Mon Sep 17 00:00:00 2001 From: Daniel Opitz Date: Thu, 20 Jul 2017 19:45:10 +0200 Subject: [PATCH 06/11] set unread/starred flags as statements in sql --- lib/Db/FeedMapper.php | 16 +++---- lib/Db/ItemMapper.php | 43 ++++++++++--------- lib/Db/Mysql/ItemMapper.php | 16 +++---- lib/Migration/MigrateStatusFlags.php | 20 +++++++-- .../Unit/Migration/MigrateStatusFlagsTest.php | 37 +++++++++++++--- 5 files changed, 87 insertions(+), 45 deletions(-) diff --git a/lib/Db/FeedMapper.php b/lib/Db/FeedMapper.php index bd063109f5..ad54ffc37e 100644 --- a/lib/Db/FeedMapper.php +++ b/lib/Db/FeedMapper.php @@ -35,11 +35,11 @@ public function find($id, $userId){ // work because prepared statements dont work. This is a // POSSIBLE SQL INJECTION RISK WHEN MODIFIED WITHOUT THOUGHT. // think twice when changing this - 'AND unread = 1 ' . + 'AND unread = ? ' . 'WHERE `feeds`.`id` = ? ' . 'AND `feeds`.`user_id` = ? ' . 'GROUP BY `feeds`.`id`'; - $params = [$id, $userId]; + $params = [true, $id, $userId]; return $this->findEntity($sql, $params); } @@ -56,14 +56,14 @@ public function findAllFromUser($userId){ // work because prepared statements dont work. This is a // POSSIBLE SQL INJECTION RISK WHEN MODIFIED WITHOUT THOUGHT. // think twice when changing this - 'AND unread = 1 ' . + 'AND unread = ? ' . 'WHERE `feeds`.`user_id` = ? ' . 'AND (`feeds`.`folder_id` = 0 ' . 'OR `folders`.`deleted_at` = 0' . ')' . 'AND `feeds`.`deleted_at` = 0 ' . 'GROUP BY `feeds`.`id`'; - $params = [$userId]; + $params = [true, $userId]; return $this->findEntities($sql, $params); } @@ -80,14 +80,14 @@ public function findAll(){ // work because prepared statements dont work. This is a // POSSIBLE SQL INJECTION RISK WHEN MODIFIED WITHOUT THOUGHT. // think twice when changing this - 'AND unread = 1 ' . + 'AND unread = ? ' . 'WHERE (`feeds`.`folder_id` = 0 ' . 'OR `folders`.`deleted_at` = 0' . ')' . 'AND `feeds`.`deleted_at` = 0 ' . 'GROUP BY `feeds`.`id`'; - return $this->findEntities($sql); + return $this->findEntities($sql, [true]); } @@ -100,11 +100,11 @@ public function findByUrlHash($hash, $userId){ // work because prepared statements dont work. This is a // POSSIBLE SQL INJECTION RISK WHEN MODIFIED WITHOUT THOUGHT. // think twice when changing this - 'AND unread = 1 ' . + 'AND unread = ? ' . 'WHERE `feeds`.`url_hash` = ? ' . 'AND `feeds`.`user_id` = ? ' . 'GROUP BY `feeds`.`id`'; - $params = [$hash, $userId]; + $params = [true, $hash, $userId]; return $this->findEntity($sql, $params); } diff --git a/lib/Db/ItemMapper.php b/lib/Db/ItemMapper.php index ff18ad5e65..777d0d392c 100644 --- a/lib/Db/ItemMapper.php +++ b/lib/Db/ItemMapper.php @@ -15,6 +15,7 @@ use Exception; use OCA\News\Utility\Time; +use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\IDBConnection; @@ -55,9 +56,11 @@ private function buildStatusQueryPart($showAll, $type = null) { $sql = ''; if (isset($type) && $type === FeedType::STARRED) { - $sql = 'AND `items`.`starred` = 1 '; + $sql = 'AND `items`.`starred` = '; + $sql .= $this->db->quote(true, IQueryBuilder::PARAM_BOOL) . ' '; } elseif (!$showAll) { - $sql .= 'AND `items`.`unread` = 1 '; + $sql .= 'AND `items`.`unread` = '; + $sql .= $this->db->quote(true, IQueryBuilder::PARAM_BOOL) . ' '; } return $sql; @@ -96,13 +99,13 @@ public function starredCount($userId) { 'ON `feeds`.`id` = `items`.`feed_id` ' . 'AND `feeds`.`deleted_at` = 0 ' . 'AND `feeds`.`user_id` = ? ' . - 'AND `items`.`starred` = 1 ' . + 'AND `items`.`starred` = ? ' . 'LEFT OUTER JOIN `*PREFIX*news_folders` `folders` ' . 'ON `folders`.`id` = `feeds`.`folder_id` ' . 'WHERE `feeds`.`folder_id` = 0 ' . 'OR `folders`.`deleted_at` = 0'; - $params = [$userId]; + $params = [$userId, true]; $result = $this->execute($sql, $params)->fetch(); @@ -112,21 +115,21 @@ public function starredCount($userId) { public function readAll($highestItemId, $time, $userId) { $sql = 'UPDATE `*PREFIX*news_items` ' . - 'SET unread = 0 ' . + 'SET unread = ? ' . ', `last_modified` = ? ' . 'WHERE `feed_id` IN (' . 'SELECT `id` FROM `*PREFIX*news_feeds` ' . 'WHERE `user_id` = ? ' . ') ' . 'AND `id` <= ?'; - $params = [$time, $userId, $highestItemId]; + $params = [false, $time, $userId, $highestItemId]; $this->execute($sql, $params); } public function readFolder($folderId, $highestItemId, $time, $userId) { $sql = 'UPDATE `*PREFIX*news_items` ' . - 'SET unread = 0 ' . + 'SET unread = ? ' . ', `last_modified` = ? ' . 'WHERE `feed_id` IN (' . 'SELECT `id` FROM `*PREFIX*news_feeds` ' . @@ -134,7 +137,7 @@ public function readFolder($folderId, $highestItemId, $time, $userId) { 'AND `user_id` = ? ' . ') ' . 'AND `id` <= ?'; - $params = [$time, $folderId, $userId, + $params = [false, $time, $folderId, $userId, $highestItemId]; $this->execute($sql, $params); } @@ -142,7 +145,7 @@ public function readFolder($folderId, $highestItemId, $time, $userId) { public function readFeed($feedId, $highestItemId, $time, $userId) { $sql = 'UPDATE `*PREFIX*news_items` ' . - 'SET unread = 0 ' . + 'SET unread = ? ' . ', `last_modified` = ? ' . 'WHERE `feed_id` = ? ' . 'AND `id` <= ? ' . @@ -150,7 +153,7 @@ public function readFeed($feedId, $highestItemId, $time, $userId) { 'SELECT * FROM `*PREFIX*news_feeds` ' . 'WHERE `user_id` = ? ' . 'AND `id` = ? ) '; - $params = [$time, $feedId, $highestItemId, + $params = [false, $time, $feedId, $highestItemId, $userId, $feedId]; $this->execute($sql, $params); @@ -268,8 +271,8 @@ public function findAll($limit, $offset, $type, $showAll, $oldestFirst, $userId, public function findAllUnreadOrStarred($userId) { - $params = [$userId]; - $sql = 'AND (`items`.`unread` = 1 OR `items`.`starred` = 1) '; + $params = [$userId, true, true]; + $sql = 'AND (`items`.`unread` = ? OR `items`.`starred` = ?) '; $sql = $this->makeSelectQuery($sql); return $this->findEntities($sql, $params); } @@ -290,15 +293,15 @@ public function findByGuidHash($guidHash, $feedId, $userId) { * @param int $threshold the number of items that should be deleted */ public function deleteReadOlderThanThreshold($threshold) { - $params = [$threshold]; + $params = [false, false, $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 `items`.`unread` = 0 ' . - 'AND `items`.`starred` = 0 ' . + 'AND `items`.`unread` = ? ' . + 'AND `items`.`starred` = ? ' . 'GROUP BY `feeds`.`id`, `feeds`.`articles_per_update` ' . 'HAVING COUNT(*) > ?'; @@ -310,13 +313,13 @@ public function deleteReadOlderThanThreshold($threshold) { $limit = $size - $threshold; if ($limit > 0) { - $params = [$row['feed_id'], $limit]; + $params = [false, false, $row['feed_id'], $limit]; $sql = 'DELETE FROM `*PREFIX*news_items` ' . 'WHERE `id` IN (' . 'SELECT `id` FROM `*PREFIX*news_items` ' . - 'WHERE `unread` = 0 ' . - 'AND `starred` = 0 ' . + 'WHERE `unread` = ? ' . + 'AND `starred` = ? ' . 'AND `feed_id` = ? ' . 'ORDER BY `id` ASC ' . 'LIMIT ?' . @@ -402,14 +405,14 @@ public function readItem($itemId, $isRead, $lastModified, $userId) { // as unread if ($isRead) { $sql = 'UPDATE `*PREFIX*news_items` - SET `unread` = 0, + SET `unread` = ?, `last_modified` = ? WHERE `fingerprint` = ? AND `feed_id` IN ( SELECT `f`.`id` FROM `*PREFIX*news_feeds` AS `f` WHERE `f`.`user_id` = ? )'; - $params = [$lastModified, $item->getFingerprint(), $userId]; + $params = [false, $lastModified, $item->getFingerprint(), $userId]; $this->execute($sql, $params); } else { $item->setLastModified($lastModified); diff --git a/lib/Db/Mysql/ItemMapper.php b/lib/Db/Mysql/ItemMapper.php index b63d4f983b..09fa8910b0 100644 --- a/lib/Db/Mysql/ItemMapper.php +++ b/lib/Db/Mysql/ItemMapper.php @@ -34,11 +34,11 @@ public function deleteReadOlderThanThreshold($threshold){ 'FROM `*PREFIX*news_items` `items` ' . 'JOIN `*PREFIX*news_feeds` `feeds` ' . 'ON `feeds`.`id` = `items`.`feed_id` ' . - 'AND `items`.`unread` = 0 ' . - 'AND `items`.`starred` = 0 ' . + 'AND `items`.`unread` = ? ' . + 'AND `items`.`starred` = ? ' . 'GROUP BY `feeds`.`id`, `feeds`.`articles_per_update` ' . 'HAVING COUNT(*) > ?'; - $params = [$threshold]; + $params = [false, false, $threshold]; $result = $this->execute($sql, $params); while($row = $result->fetch()) { @@ -47,11 +47,11 @@ public function deleteReadOlderThanThreshold($threshold){ $limit = $size - $threshold; if($limit > 0) { - $params = [$row['feed_id'], $limit]; + $params = [false, false, $row['feed_id'], $limit]; $sql = 'DELETE FROM `*PREFIX*news_items` ' . - 'WHERE `items`.`unread` = 0 ' . - 'AND `items`.`starred` = 0 ' . + 'WHERE `items`.`unread` = ? ' . + 'AND `items`.`starred` = ? ' . 'AND `feed_id` = ? ' . 'ORDER BY `id` ASC ' . 'LIMIT ?'; @@ -69,11 +69,11 @@ public function readItem($itemId, $isRead, $lastModified, $userId) { $sql = 'UPDATE `*PREFIX*news_items` `items` JOIN `*PREFIX*news_feeds` `feeds` ON `feeds`.`id` = `items`.`feed_id` - SET `items`.`unread` = 0, + SET `items`.`unread` = ?, `items`.`last_modified` = ? WHERE `items`.`fingerprint` = ? AND `feeds`.`user_id` = ?'; - $params = [$lastModified, $item->getFingerprint(), $userId]; + $params = [false, $lastModified, $item->getFingerprint(), $userId]; $this->execute($sql, $params); } else { $item->setLastModified($lastModified); diff --git a/lib/Migration/MigrateStatusFlags.php b/lib/Migration/MigrateStatusFlags.php index 8ff552907f..a5073e38ca 100644 --- a/lib/Migration/MigrateStatusFlags.php +++ b/lib/Migration/MigrateStatusFlags.php @@ -11,6 +11,7 @@ namespace OCA\News\Migration; +use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\IConfig; use OCP\IDBConnection; use OCP\Migration\IRepairStep; @@ -43,11 +44,22 @@ public function run(IOutput $output) { return; } - $update = 'UPDATE `*PREFIX*news_items` ' . - 'SET unread = IF(status & 2, 1, 0), starred = IF(status & 4, 1, 0)'; - $output->startProgress(); - $this->db->executeUpdate($update); + + $qb = $this->db->getQueryBuilder(); + + $qb->update('news_items') + ->set('unread', $qb->createParameter('unread_value')) + ->where('(status & 2)') + ->setParameter('unread_value', true, IQueryBuilder::PARAM_BOOL) + ->execute(); + + $qb->update('news_items') + ->set('starred', $qb->createParameter('starred_value')) + ->where('(status & 4)') + ->setParameter('starred_value', true, IQueryBuilder::PARAM_BOOL) + ->execute(); + $output->finishProgress(); } diff --git a/tests/Unit/Migration/MigrateStatusFlagsTest.php b/tests/Unit/Migration/MigrateStatusFlagsTest.php index 3718b71cef..0f612fe5ef 100644 --- a/tests/Unit/Migration/MigrateStatusFlagsTest.php +++ b/tests/Unit/Migration/MigrateStatusFlagsTest.php @@ -11,6 +11,8 @@ namespace OCA\News\Migration; +use OCP\DB\QueryBuilder\IParameter; +use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\IConfig; use OCP\IDBConnection; use OCP\Migration\IOutput; @@ -32,16 +34,41 @@ protected function setUp() { } public function testRun() { - $sql = $update = 'UPDATE `*PREFIX*news_items` ' . - 'SET unread = IF(status & 2, 1, 0), starred = IF(status & 4, 1, 0)'; + $queryBuilder = $this->createMock(IQueryBuilder::class); + $queryBuilder->expects($this->exactly(2)) + ->method('createParameter') + ->with($this->logicalOr('unread_value', 'starred_value')) + ->willReturn($this->createMock(IParameter::class)); + $queryBuilder->expects($this->exactly(2)) + ->method('update') + ->with('news_items') + ->willReturnSelf(); + $setParam = $this->logicalOr('unread', 'starred'); + $queryBuilder->expects($this->exactly(2)) + ->method('set') + ->with($setParam, $this->isInstanceOf(IParameter::class)) + ->willReturnSelf(); + $queryBuilder->expects($this->exactly(2)) + ->method('where') + ->with($this->logicalOr('(status & 2)', '(status & 4)')) + ->willReturnSelf(); + $setParameterName = $this->logicalOr('unread_value', 'starred_value'); + $queryBuilder->expects($this->exactly(2)) + ->method('setParameter') + ->with($setParameterName, true, IQueryBuilder::PARAM_BOOL) + ->willReturnSelf(); + $queryBuilder->expects($this->exactly(2)) + ->method('execute') + ->with(); $this->config->expects($this->exactly(1)) ->method('getAppValue') ->with('news', 'installed_version', '0.0.0') ->willReturn('11.0.5'); $this->db->expects($this->exactly(1)) - ->method('executeUpdate') - ->with($sql); + ->method('getQueryBuilder') + ->with() + ->willReturn($queryBuilder); $this->output->expects($this->exactly(1)) ->method('startProgress'); $this->output->expects($this->exactly(1)) @@ -57,7 +84,7 @@ public function testRunNewerVersion() { ->with('news', 'installed_version', '0.0.0') ->willReturn('11.1.0'); $this->db->expects($this->exactly(0)) - ->method('executeUpdate'); + ->method('getQueryBuilder'); $migration = new MigrateStatusFlags($this->db, $this->config); $migration->run($this->output); From 08a9b5c76a9841c6da6903ebbb4e95c330891ac8 Mon Sep 17 00:00:00 2001 From: Daniel Opitz Date: Sun, 23 Jul 2017 20:21:31 +0200 Subject: [PATCH 07/11] fixed mysql unknown column items.unread, fixed marking of read items on repair step --- lib/Db/Mysql/ItemMapper.php | 4 ++-- lib/Migration/MigrateStatusFlags.php | 6 ++++++ tests/Unit/Migration/MigrateStatusFlagsTest.php | 16 ++++++++-------- 3 files changed, 16 insertions(+), 10 deletions(-) diff --git a/lib/Db/Mysql/ItemMapper.php b/lib/Db/Mysql/ItemMapper.php index 09fa8910b0..ad373eafb7 100644 --- a/lib/Db/Mysql/ItemMapper.php +++ b/lib/Db/Mysql/ItemMapper.php @@ -50,8 +50,8 @@ public function deleteReadOlderThanThreshold($threshold){ $params = [false, false, $row['feed_id'], $limit]; $sql = 'DELETE FROM `*PREFIX*news_items` ' . - 'WHERE `items`.`unread` = ? ' . - 'AND `items`.`starred` = ? ' . + 'WHERE `unread` = ? ' . + 'AND `starred` = ? ' . 'AND `feed_id` = ? ' . 'ORDER BY `id` ASC ' . 'LIMIT ?'; diff --git a/lib/Migration/MigrateStatusFlags.php b/lib/Migration/MigrateStatusFlags.php index a5073e38ca..724ea7846f 100644 --- a/lib/Migration/MigrateStatusFlags.php +++ b/lib/Migration/MigrateStatusFlags.php @@ -54,6 +54,12 @@ public function run(IOutput $output) { ->setParameter('unread_value', true, IQueryBuilder::PARAM_BOOL) ->execute(); + $qb->update('news_items') + ->set('unread', $qb->createParameter('unread_value')) + ->where('(NOT status & 2)') + ->setParameter('unread_value', false, IQueryBuilder::PARAM_BOOL) + ->execute(); + $qb->update('news_items') ->set('starred', $qb->createParameter('starred_value')) ->where('(status & 4)') diff --git a/tests/Unit/Migration/MigrateStatusFlagsTest.php b/tests/Unit/Migration/MigrateStatusFlagsTest.php index 0f612fe5ef..effebafac8 100644 --- a/tests/Unit/Migration/MigrateStatusFlagsTest.php +++ b/tests/Unit/Migration/MigrateStatusFlagsTest.php @@ -35,29 +35,29 @@ protected function setUp() { public function testRun() { $queryBuilder = $this->createMock(IQueryBuilder::class); - $queryBuilder->expects($this->exactly(2)) + $queryBuilder->expects($this->exactly(3)) ->method('createParameter') ->with($this->logicalOr('unread_value', 'starred_value')) ->willReturn($this->createMock(IParameter::class)); - $queryBuilder->expects($this->exactly(2)) + $queryBuilder->expects($this->exactly(3)) ->method('update') ->with('news_items') ->willReturnSelf(); $setParam = $this->logicalOr('unread', 'starred'); - $queryBuilder->expects($this->exactly(2)) + $queryBuilder->expects($this->exactly(3)) ->method('set') ->with($setParam, $this->isInstanceOf(IParameter::class)) ->willReturnSelf(); - $queryBuilder->expects($this->exactly(2)) + $queryBuilder->expects($this->exactly(3)) ->method('where') - ->with($this->logicalOr('(status & 2)', '(status & 4)')) + ->with($this->logicalOr('(status & 2)', '(status & 4)', '(NOT status & 2)')) ->willReturnSelf(); $setParameterName = $this->logicalOr('unread_value', 'starred_value'); - $queryBuilder->expects($this->exactly(2)) + $queryBuilder->expects($this->exactly(3)) ->method('setParameter') - ->with($setParameterName, true, IQueryBuilder::PARAM_BOOL) + ->with($setParameterName, $this->logicalOr(true, false), IQueryBuilder::PARAM_BOOL) ->willReturnSelf(); - $queryBuilder->expects($this->exactly(2)) + $queryBuilder->expects($this->exactly(3)) ->method('execute') ->with(); From 58937b9a062ebb19583f3f0f15e1975dfcc44fdc Mon Sep 17 00:00:00 2001 From: Daniel Opitz Date: Sun, 23 Jul 2017 20:25:44 +0200 Subject: [PATCH 08/11] remove unnecessary bool casts --- lib/Db/Item.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/Db/Item.php b/lib/Db/Item.php index efcd87a88b..138077fc4c 100644 --- a/lib/Db/Item.php +++ b/lib/Db/Item.php @@ -87,20 +87,20 @@ public function isRead() { public function setUnread($value = true) { $this->markFieldUpdated('unread'); - $this->unread = (bool)$value; + $this->unread = $value; } public function isUnread() { - return (bool)$this->unread; + return $this->unread; } public function setStarred($value = true) { $this->markFieldUpdated('starred'); - $this->starred = (bool)$value; + $this->starred = $value; } public function isStarred() { - return (bool)$this->starred; + return $this->starred; } public function setUnstarred() { From 216917eb23d88786d92f39d44348d9bc57ff6a3e Mon Sep 17 00:00:00 2001 From: Daniel Opitz Date: Wed, 2 Aug 2017 23:18:58 +0200 Subject: [PATCH 09/11] add empty checks to Items::is* methods --- lib/Db/Item.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/Db/Item.php b/lib/Db/Item.php index 138077fc4c..ad01d82030 100644 --- a/lib/Db/Item.php +++ b/lib/Db/Item.php @@ -82,7 +82,7 @@ public function setRead() { } public function isRead() { - return !$this->unread; + return empty($this->unread); } public function setUnread($value = true) { @@ -91,7 +91,7 @@ public function setUnread($value = true) { } public function isUnread() { - return $this->unread; + return !empty($this->unread); } public function setStarred($value = true) { @@ -100,7 +100,7 @@ public function setStarred($value = true) { } public function isStarred() { - return $this->starred; + return !empty($this->starred); } public function setUnstarred() { @@ -108,7 +108,7 @@ public function setUnstarred() { } public function isUnstarred() { - return !$this->starred; + return empty($this->starred); } /** From f65378b0e581dc19a6db693d0de54397dcce95b2 Mon Sep 17 00:00:00 2001 From: Daniel Opitz Date: Sun, 13 Aug 2017 17:50:37 +0200 Subject: [PATCH 10/11] update migration to use native sql instead of the querybuilder --- lib/Migration/MigrateStatusFlags.php | 30 ++------ .../Unit/Migration/MigrateStatusFlagsTest.php | 75 ++++++++++--------- 2 files changed, 48 insertions(+), 57 deletions(-) diff --git a/lib/Migration/MigrateStatusFlags.php b/lib/Migration/MigrateStatusFlags.php index 724ea7846f..acf53ebba9 100644 --- a/lib/Migration/MigrateStatusFlags.php +++ b/lib/Migration/MigrateStatusFlags.php @@ -44,29 +44,13 @@ public function run(IOutput $output) { return; } - $output->startProgress(); + $sql = 'UPDATE `*PREFIX*news_items` ' + . 'SET `unread` = ((`status` & 2) = 2), ' + . '`starred` = ((`status` & 4) = 4)'; + $query = $this->db->prepare($sql); - $qb = $this->db->getQueryBuilder(); - - $qb->update('news_items') - ->set('unread', $qb->createParameter('unread_value')) - ->where('(status & 2)') - ->setParameter('unread_value', true, IQueryBuilder::PARAM_BOOL) - ->execute(); - - $qb->update('news_items') - ->set('unread', $qb->createParameter('unread_value')) - ->where('(NOT status & 2)') - ->setParameter('unread_value', false, IQueryBuilder::PARAM_BOOL) - ->execute(); - - $qb->update('news_items') - ->set('starred', $qb->createParameter('starred_value')) - ->where('(status & 4)') - ->setParameter('starred_value', true, IQueryBuilder::PARAM_BOOL) - ->execute(); - - $output->finishProgress(); + if (!$query->execute()) { + throw new \Exception('Could not migrate status'); + } } - } \ No newline at end of file diff --git a/tests/Unit/Migration/MigrateStatusFlagsTest.php b/tests/Unit/Migration/MigrateStatusFlagsTest.php index effebafac8..bee3532c15 100644 --- a/tests/Unit/Migration/MigrateStatusFlagsTest.php +++ b/tests/Unit/Migration/MigrateStatusFlagsTest.php @@ -11,8 +11,7 @@ namespace OCA\News\Migration; -use OCP\DB\QueryBuilder\IParameter; -use OCP\DB\QueryBuilder\IQueryBuilder; +use Doctrine\DBAL\Driver\Statement; use OCP\IConfig; use OCP\IDBConnection; use OCP\Migration\IOutput; @@ -34,45 +33,53 @@ protected function setUp() { } public function testRun() { - $queryBuilder = $this->createMock(IQueryBuilder::class); - $queryBuilder->expects($this->exactly(3)) - ->method('createParameter') - ->with($this->logicalOr('unread_value', 'starred_value')) - ->willReturn($this->createMock(IParameter::class)); - $queryBuilder->expects($this->exactly(3)) - ->method('update') - ->with('news_items') - ->willReturnSelf(); - $setParam = $this->logicalOr('unread', 'starred'); - $queryBuilder->expects($this->exactly(3)) - ->method('set') - ->with($setParam, $this->isInstanceOf(IParameter::class)) - ->willReturnSelf(); - $queryBuilder->expects($this->exactly(3)) - ->method('where') - ->with($this->logicalOr('(status & 2)', '(status & 4)', '(NOT status & 2)')) - ->willReturnSelf(); - $setParameterName = $this->logicalOr('unread_value', 'starred_value'); - $queryBuilder->expects($this->exactly(3)) - ->method('setParameter') - ->with($setParameterName, $this->logicalOr(true, false), IQueryBuilder::PARAM_BOOL) - ->willReturnSelf(); - $queryBuilder->expects($this->exactly(3)) + $statement = $this->createMock(Statement::class); + $statement->expects($this->exactly(1)) ->method('execute') - ->with(); + ->with() + ->willReturn(true); $this->config->expects($this->exactly(1)) ->method('getAppValue') ->with('news', 'installed_version', '0.0.0') ->willReturn('11.0.5'); + + $sql = 'UPDATE `*PREFIX*news_items` ' + . 'SET `unread` = ((`status` & 2) = 2), ' + . '`starred` = ((`status` & 4) = 4)'; + $this->db->expects($this->exactly(1)) - ->method('getQueryBuilder') + ->method('prepare') + ->with($sql) + ->willReturn($statement); + + $migration = new MigrateStatusFlags($this->db, $this->config); + $migration->run($this->output); + } + + /** + * @expectedException \Exception + */ + public function testRunException() { + $statement = $this->createMock(Statement::class); + $statement->expects($this->exactly(1)) + ->method('execute') ->with() - ->willReturn($queryBuilder); - $this->output->expects($this->exactly(1)) - ->method('startProgress'); - $this->output->expects($this->exactly(1)) - ->method('finishProgress'); + ->willReturn(false); + + $this->config->expects($this->exactly(1)) + ->method('getAppValue') + ->with('news', 'installed_version', '0.0.0') + ->willReturn('11.0.5'); + + $sql = 'UPDATE `*PREFIX*news_items` ' + . 'SET `unread` = ((`status` & 2) = 2), ' + . '`starred` = ((`status` & 4) = 4)'; + + $this->db->expects($this->exactly(1)) + ->method('prepare') + ->with($sql) + ->willReturn($statement); $migration = new MigrateStatusFlags($this->db, $this->config); $migration->run($this->output); @@ -84,7 +91,7 @@ public function testRunNewerVersion() { ->with('news', 'installed_version', '0.0.0') ->willReturn('11.1.0'); $this->db->expects($this->exactly(0)) - ->method('getQueryBuilder'); + ->method('prepare'); $migration = new MigrateStatusFlags($this->db, $this->config); $migration->run($this->output); From 9bf795ef486e4cb3a6f4f11a11df17bfea243be0 Mon Sep 17 00:00:00 2001 From: Daniel Opitz Date: Mon, 14 Aug 2017 00:40:51 +0200 Subject: [PATCH 11/11] don't cast the flags manually, let the api do the work --- appinfo/database.xml | 2 +- lib/Db/Item.php | 40 ++++--------------- lib/Service/FeedService.php | 2 +- tests/Integration/Db/ItemMapperTest.php | 4 +- .../Controller/EntityApiSerializerTest.php | 10 ++--- tests/Unit/Db/ItemTest.php | 31 +++++++------- tests/Unit/Fetcher/FeedFetcherTest.php | 2 +- tests/Unit/Service/FeedServiceTest.php | 14 +++---- tests/Unit/Service/ItemServiceTest.php | 15 +++---- 9 files changed, 49 insertions(+), 71 deletions(-) diff --git a/appinfo/database.xml b/appinfo/database.xml index 0675bf8558..e9c65a9471 100644 --- a/appinfo/database.xml +++ b/appinfo/database.xml @@ -366,7 +366,7 @@ unread boolean - true + false true diff --git a/lib/Db/Item.php b/lib/Db/Item.php index ad01d82030..a781048fd5 100644 --- a/lib/Db/Item.php +++ b/lib/Db/Item.php @@ -43,6 +43,8 @@ * @method void setFingerprint(string $value) * @method void setContentHash(string $value) * @method void setSearchIndex(string $value) + * @method void setUnread(bool $value) + * @method void setStarred(bool $value) */ class Item extends Entity implements IAPI, \JsonSerializable { @@ -65,8 +67,8 @@ class Item extends Entity implements IAPI, \JsonSerializable { protected $searchIndex; protected $rtl; protected $fingerprint; - protected $unread; - protected $starred; + protected $unread = false; + protected $starred = false; public function __construct() { $this->addType('pubDate', 'integer'); @@ -77,38 +79,12 @@ public function __construct() { $this->addType('starred', 'boolean'); } - public function setRead() { - $this->setUnread(false); - } - - public function isRead() { - return empty($this->unread); - } - - public function setUnread($value = true) { - $this->markFieldUpdated('unread'); - $this->unread = $value; - } - public function isUnread() { - return !empty($this->unread); - } - - public function setStarred($value = true) { - $this->markFieldUpdated('starred'); - $this->starred = $value; + return $this->unread; } public function isStarred() { - return !empty($this->starred); - } - - public function setUnstarred() { - $this->setStarred(false); - } - - public function isUnstarred() { - return empty($this->starred); + return $this->starred; } /** @@ -195,8 +171,8 @@ public static function fromImport($import) { $item->setEnclosureMime($import['enclosureMime']); $item->setEnclosureLink($import['enclosureLink']); $item->setRtl($import['rtl']); - $item->setUnread(!empty($import['unread'])); - $item->setStarred(!empty($import['starred'])); + $item->setUnread($import['unread']); + $item->setStarred($import['starred']); return $item; } diff --git a/lib/Service/FeedService.php b/lib/Service/FeedService.php index f17b759e3c..d24fd259e1 100644 --- a/lib/Service/FeedService.php +++ b/lib/Service/FeedService.php @@ -264,7 +264,7 @@ public function update($feedId, $userId, $forceUpdate=false){ // update modes: 0 nothing, 1 set unread if ($existingFeed->getUpdateMode() === 1) { - $dbItem->setUnread(); + $dbItem->setUnread(true); } $this->itemMapper->update($dbItem); diff --git a/tests/Integration/Db/ItemMapperTest.php b/tests/Integration/Db/ItemMapperTest.php index da66351590..fa9cc7d250 100644 --- a/tests/Integration/Db/ItemMapperTest.php +++ b/tests/Integration/Db/ItemMapperTest.php @@ -243,7 +243,7 @@ public function testReadItem() { // assert that all test user's same items are read $items = $this->itemMapper->where(['feedId' => $feed->getId(), 'title' => 'blubb']); foreach ($items as $item) { - $this->assertTrue($item->isRead()); + $this->assertFalse($item->isUnread()); } // assert that a different item is not read @@ -274,7 +274,7 @@ public function testUnreadItem() { if ($item->getId() === $duplicateItem->getId()) { $this->assertTrue($item->isUnread()); } else { - $this->assertTrue($item->isRead()); + $this->assertFalse($item->isUnread()); } } diff --git a/tests/Unit/Controller/EntityApiSerializerTest.php b/tests/Unit/Controller/EntityApiSerializerTest.php index ec357e7f2c..63de1ed7ee 100644 --- a/tests/Unit/Controller/EntityApiSerializerTest.php +++ b/tests/Unit/Controller/EntityApiSerializerTest.php @@ -29,7 +29,7 @@ class EntityApiSerializerTest extends \PHPUnit_Framework_TestCase { public function testSerializeSingle() { $item = new Item(); - $item->setUnread(); + $item->setUnread(true); $serializer = new EntityApiSerializer('items'); $result = $serializer->serialize($item); @@ -40,10 +40,10 @@ public function testSerializeSingle() { public function testSerializeMultiple() { $item = new Item(); - $item->setUnread(); + $item->setUnread(true); $item2 = new Item(); - $item2->setRead(); + $item2->setUnread(false); $serializer = new EntityApiSerializer('items'); @@ -66,10 +66,10 @@ public function testResponseNoChange() { public function testCompleteArraysTransformed() { $item = new Item(); - $item->setUnread(); + $item->setUnread(true); $item2 = new Item(); - $item2->setRead(); + $item2->setUnread(false); $serializer = new EntityApiSerializer('items'); diff --git a/tests/Unit/Db/ItemTest.php b/tests/Unit/Db/ItemTest.php index f26cf6ff95..6f0b48e5ab 100644 --- a/tests/Unit/Db/ItemTest.php +++ b/tests/Unit/Db/ItemTest.php @@ -16,6 +16,7 @@ class ItemTest extends \PHPUnit_Framework_TestCase { + /** @var Item */ private $item; protected function setUp(){ @@ -25,30 +26,30 @@ protected function setUp(){ public function testSetRead(){ - $this->item->setRead(); + $this->item->setUnread(false); - $this->assertTrue($this->item->isRead()); + $this->assertFalse($this->item->isUnread()); } public function testSetUnread(){ - $this->item->setUnread(); + $this->item->setUnread(true); $this->assertTrue($this->item->isUnread()); } public function testSetStarred(){ - $this->item->setStarred(); + $this->item->setStarred(true); $this->assertTrue($this->item->isStarred()); } public function testSetUnstarred(){ - $this->item->setUnstarred(); + $this->item->setStarred(false); - $this->assertTrue($this->item->isUnstarred()); + $this->assertFalse($this->item->isStarred()); } @@ -68,8 +69,8 @@ public function testToAPI() { $item->setRtl(true); $item->setFeedId(1); $item->setStatus(0); - $item->setUnread(); - $item->setStarred(); + $item->setUnread(true); + $item->setStarred(true); $item->setLastModified('1111111111234567'); $item->setFingerprint('fingerprint'); $item->setContentHash('contentHash'); @@ -113,9 +114,9 @@ public function testJSONSerialize() { $item->setFeedId(1); $item->setStatus(0); $item->setRtl(true); - $item->setUnread(); + $item->setUnread(true); $item->setFingerprint('fingerprint'); - $item->setStarred(); + $item->setStarred(true); $item->setLastModified(321); $this->assertEquals([ @@ -156,8 +157,8 @@ public function testToExport() { $item->setFeedId(1); $item->setRtl(true); $item->setStatus(0); - $item->setRead(); - $item->setStarred(); + $item->setUnread(false); + $item->setStarred(true); $item->setLastModified(321); $feed = new Feed(); @@ -194,13 +195,13 @@ private function createImportItem($isRead) { $item->setBody('body'); $item->setEnclosureMime('audio/ogg'); $item->setEnclosureLink('enclink'); - $item->setStarred(); + $item->setStarred(true); $item->setRtl(true); if ($isRead) { - $item->setUnread(); + $item->setUnread(true); } else { - $item->setRead(); + $item->setUnread(false); } return $item; diff --git a/tests/Unit/Fetcher/FeedFetcherTest.php b/tests/Unit/Fetcher/FeedFetcherTest.php index ce09bb0e35..16d26cb66d 100644 --- a/tests/Unit/Fetcher/FeedFetcherTest.php +++ b/tests/Unit/Fetcher/FeedFetcherTest.php @@ -231,7 +231,7 @@ private function createItem($enclosureType=null) { $item->setUpdatedDate($this->updated); $item->setStatus(0); - $item->setUnread(); + $item->setUnread(true); $item->setUrl($this->permalink); $item->setTitle('my<\' title'); $item->setGuid($this->guid); diff --git a/tests/Unit/Service/FeedServiceTest.php b/tests/Unit/Service/FeedServiceTest.php index 68d25df9e5..6df02e51c2 100644 --- a/tests/Unit/Service/FeedServiceTest.php +++ b/tests/Unit/Service/FeedServiceTest.php @@ -388,7 +388,7 @@ private function createUpdateItem() { $item->setTitle('hey'); $item->setAuthor('aut'); $item->setBody('new'); - $item->setRead(); + $item->setUnread(false); return $item; } @@ -401,7 +401,7 @@ private function createUpdateItem2() { $item->setTitle('ho'); $item->setAuthor('auto'); $item->setBody('old'); - $item->setRead(); + $item->setUnread(false); return $item; } @@ -448,7 +448,7 @@ public function testUpdateSetsUnreadIfModeIsOne() { $item = $this->createUpdateItem(); $item2 = $this->createUpdateItem2(); $item3 = $this->createUpdateItem(); - $item3->setUnread(); + $item3->setUnread(true); $items = [$item]; @@ -760,8 +760,8 @@ public function testImportArticles(){ $item->setBody('come over'); $item->setEnclosureMime('mime'); $item->setEnclosureLink('lin'); - $item->setUnread(); - $item->setUnstarred(); + $item->setUnread(true); + $item->setStarred(false); $item->generateSearchIndex(); $json = $item->toExport(['feed3' => $feed]); @@ -816,8 +816,8 @@ public function testImportArticlesCreatesOwnFeedWhenNotFound(){ $item->setBody('come over'); $item->setEnclosureMime('mime'); $item->setEnclosureLink('lin'); - $item->setUnread(); - $item->setUnstarred(); + $item->setUnread(true); + $item->setStarred(false); $item->generateSearchIndex(); $json = $item->toExport(['feed3' => $feed]); diff --git a/tests/Unit/Service/ItemServiceTest.php b/tests/Unit/Service/ItemServiceTest.php index 5e1cb4cae1..4beebbb63f 100644 --- a/tests/Unit/Service/ItemServiceTest.php +++ b/tests/Unit/Service/ItemServiceTest.php @@ -211,11 +211,11 @@ public function testStar(){ $item = new Item(); $item->setStatus(128); $item->setId($itemId); - $item->setUnstarred(); + $item->setStarred(false); $expectedItem = new Item(); $expectedItem->setStatus(128); - $expectedItem->setStarred(); + $expectedItem->setStarred(true); $expectedItem->setId($itemId); $this->mapper->expects($this->once()) @@ -244,11 +244,12 @@ public function testUnstar(){ $item = new Item(); $item->setStatus(128); $item->setId($itemId); - $item->setStarred(); + $item->setStarred(true); $expectedItem = new Item(); $expectedItem->setStatus(128); - $expectedItem->setUnstarred(); + $expectedItem->setStarred(true); //workaround to set starred as updated field + $expectedItem->setStarred(false); $expectedItem->setId($itemId); $this->mapper->expects($this->once()) @@ -265,7 +266,7 @@ public function testUnstar(){ $this->itemService->star($feedId, $guidHash, false, $this->user); - $this->assertTrue($item->isUnstarred()); + $this->assertFalse($item->isStarred()); } public function testRead(){ @@ -273,11 +274,11 @@ public function testRead(){ $item = new Item(); $item->setStatus(128); $item->setId($itemId); - $item->setUnread(); + $item->setUnread(true); $expectedItem = new Item(); $expectedItem->setStatus(128); - $expectedItem->setRead(); + $expectedItem->setUnread(false); $expectedItem->setId($itemId); $expectedItem->setLastModified($this->time);