Skip to content

Commit

Permalink
Split binary to booleans (#203)
Browse files Browse the repository at this point in the history 
* replaced old status with 2 flags for unread and starred

* add fields to db, replace int(1,0) with booleans in sql queries, removed StatusFlags class + refactor code relying to it

* add repair step for migration

* again use integer(1,0) instead of bool in sql queries, because of sqlite doesn't support true/false

* add/fix unit tests for new boolean status

* set unread/starred flags as statements in sql

* fixed mysql unknown column items.unread, fixed marking of read items on repair step

* remove unnecessary bool casts

* add empty checks to Items::is* methods

* update migration to use native sql instead of the querybuilder

* don't cast the flags manually, let the api do the work
  • Loading branch information
@danopz @BernhardPosselt
danopz authored and BernhardPosselt committed Aug 14, 2017
1 parent 7d8a85c commit a97dd58
Show file tree
Hide file tree
Showing 27 changed files with 548 additions and 1,299 deletions.
12 changes: 12 additions & 0 deletions appinfo/database.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -363,6 +363,18 @@
<default>0</default>
<notnull>true</notnull>
</field>
<field>
<name>unread</name>
<type>boolean</type>
<default>false</default>
<notnull>true</notnull>
</field>
<field>
<name>starred</name>
<type>boolean</type>
<default>false</default>
<notnull>true</notnull>
</field>
<field>
<name>last_modified</name>
<type>integer</type>
Expand Down
8 changes: 7 additions & 1 deletion appinfo/info.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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).]]></description>
<version>11.0.5</version>
<version>11.0.6</version>
<licence>agpl</licence>
<author>Bernhard Posselt</author>
<author>Alessandro Cosentino</author>
Expand Down Expand Up @@ -42,6 +42,12 @@ Before you update to a new version, [check the changelog](https://github.com/nex
<job>OCA\News\Cron\Updater</job>
</background-jobs>

<repair-steps>
<post-migration>
<step>OCA\News\Migration\MigrateStatusFlags</step>
</post-migration>
</repair-steps>

<settings>
<admin>OCA\News\Settings\Admin</admin>
<admin-section>OCA\News\Settings\Section</admin-section>
Expand Down
20 changes: 8 additions & 12 deletions lib/Db/FeedMapper.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +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 (`items`.`status` & ' . StatusFlag::UNREAD . ') = ' .
StatusFlag::UNREAD . ' ' .
'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);
}
Expand All @@ -57,15 +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 (`items`.`status` & ' . StatusFlag::UNREAD . ') = ' .
StatusFlag::UNREAD . ' ' .
'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);
}
Expand All @@ -82,15 +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 (`items`.`status` & ' . StatusFlag::UNREAD . ') = ' .
StatusFlag::UNREAD . ' ' .
'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]);
}


Expand All @@ -103,12 +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 (`items`.`status` & ' . StatusFlag::UNREAD . ') = ' .
StatusFlag::UNREAD . ' ' .
'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);
}
Expand Down
55 changes: 11 additions & 44 deletions lib/Db/Item.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Expand All @@ -67,53 +67,28 @@ class Item extends Entity implements IAPI, \JsonSerializable {
protected $searchIndex;
protected $rtl;
protected $fingerprint;
protected $unread = false;
protected $starred = false;

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;
}

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 [
Expand Down Expand Up @@ -196,16 +171,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($import['unread']);
$item->setStarred($import['starred']);

return $item;
}
Expand Down
Loading

0 comments on commit a97dd58

Please sign in to comment.