extra work for postgres, fix #338

10 files changed, 409 insertions, 8 deletions

diff --git a/CHANGELOG b/CHANGELOG
index f9c9b5c9b..aa7015dc6 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,3 +1,6 @@
+owncloud-news (1.404)
+* Fix bug on postgres databases that would not delete old articles
+
 owncloud-news (1.403)
 * Respect encoding in feed enhancers
 * Hotfix for update on posgresql databases
diff --git a/appinfo/info.xml b/appinfo/info.xml
index 714ec16ec..15ddf6ee5 100644
--- a/appinfo/info.xml
+++ b/appinfo/info.xml
@@ -5,6 +5,6 @@
 	<description>An RSS/Atom feed reader. Requires the App Framework app and backgroundjobs need to be enabled. See the README.rst in the apps top directory</description>
 	<licence>AGPL</licence>
 	<author>Alessandro Cosentino, Bernhard Posselt, Jan-Christoph Borchardt. Powered by SimplePie (Ryan Parman, Geoffrey Sneddon, Ryan McCue and contributors).</author>
-	<version>1.403</version>
+	<version>1.404</version>
 	<require>5.0.6</require>
 </info>
diff --git a/appinfo/version b/appinfo/version
index dd2037e0e..9539f398d 100644
--- a/appinfo/version
+++ b/appinfo/version
@@ -1 +1 @@
-1.403
\ No newline at end of file
+1.404
\ No newline at end of file
diff --git a/db/itemmapper.php b/db/itemmapper.php
index eaf24dc49..0a78b02df 100644
--- a/db/itemmapper.php
+++ b/db/itemmapper.php
@@ -257,7 +257,6 @@ class ItemMapper extends Mapper implements IMapper {
 		$params = array($status, $threshold);
 		$result = $this->execute($sql, $params);
 
-		/* FIXME: this is broken on posgres
 		while($row = $result->fetchRow()) {
 
 			$size = (int) $row['size'];
@@ -274,7 +273,6 @@ class ItemMapper extends Mapper implements IMapper {
 				$this->execute($sql, $params, $limit);
 			}
 		}
-		*/
 	}
 
 
diff --git a/db/mapperfactory.php b/db/mapperfactory.php
new file mode 100644
index 000000000..cf64f90d6
--- /dev/null
+++ b/db/mapperfactory.php
@@ -0,0 +1,52 @@
+<?php
+
+/**
+* ownCloud - News
+*
+* @author Alessandro Cosentino
+* @author Bernhard Posselt
+* @copyright 2012 Alessandro Cosentino cosenal@gmail.com
+* @copyright 2012 Bernhard Posselt dev@bernhard-posselt.com
+*
+* This library is free software; you can redistribute it and/or
+* modify it under the terms of the GNU AFFERO GENERAL PUBLIC LICENSE
+* License as published by the Free Software Foundation; either
+* version 3 of the License, or any later version.
+*
+* This library is distributed in the hope that it will be useful,
+* but WITHOUT ANY WARRANTY; without even the implied warranty of
+* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+* GNU AFFERO GENERAL PUBLIC LICENSE for more details.
+*
+* You should have received a copy of the GNU Affero General Public
+* License along with this library.  If not, see <http://www.gnu.org/licenses/>.
+*
+*/
+
+namespace OCA\News\Db;
+
+use \OCA\AppFramework\Core\API;
+
+
+class MapperFactory {
+
+	private $api;
+
+	public function __construct(API $api) {
+		$this->api = $api;
+	}
+
+
+	public function getItemMapper() {
+		switch($this->api->getSystemValue('dbtype')) {
+			case 'pgsql':
+				return new \OCA\News\Db\Postgres\ItemMapper($this->api);
+				break;
+			default:
+				return new ItemMapper($this->api);
+				break;
+		}
+	}
+
+
+}
\ No newline at end of file
diff --git a/db/postgres/itemmapper.php b/db/postgres/itemmapper.php
new file mode 100644
index 000000000..ddf83ae10
--- /dev/null
+++ b/db/postgres/itemmapper.php
@@ -0,0 +1,80 @@
+<?php
+/**
+* ownCloud - News app
+*
+* @author Alessandro Cosentino
+* @author Bernhard Posselt
+* @copyright 2012 Alessandro Cosentino cosenal@gmail.com
+* @copyright 2012 Bernhard Posselt dev@bernhard-posselt.com
+*
+* This library is free software; you can redistribute it and/or
+* modify it under the terms of the GNU AFFERO GENERAL PUBLIC LICENSE
+* License as published by the Free Software Foundation; either
+* version 3 of the License, or any later version.
+*
+* This library is distributed in the hope that it will be useful,
+* but WITHOUT ANY WARRANTY; without even the implied warranty of
+* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+* GNU AFFERO GENERAL PUBLIC LICENSE for more details.
+*
+* You should have received a copy of the GNU Affero General Public
+* License along with this library.  If not, see <http://www.gnu.org/licenses/>.
+*
+*/
+
+namespace OCA\News\Db\Postgres;
+
+use \OCA\AppFramework\Db\DoesNotExistException;
+use \OCA\AppFramework\Db\MultipleObjectsReturnedException;
+use \OCA\AppFramework\Db\Mapper;
+use \OCA\AppFramework\Core\API;
+
+use \OCA\News\Db\StatusFlag;
+
+
+class ItemMapper extends \OCA\News\Db\ItemMapper {
+
+	public function __construct(API $api){
+		parent::__construct($api);
+	}
+
+
+	/**
+	 * Delete all items for feeds that have over $threshold unread and not
+	 * starred items
+	 */
+	public function deleteReadOlderThanThreshold($threshold){
+		$status = StatusFlag::STARRED | StatusFlag::UNREAD;
+		$sql = 'SELECT COUNT(*) `size`, `feed_id` ' .
+			'FROM `*PREFIX*news_items` ' .
+			'WHERE NOT ((`status` & ?) > 0) ' .
+			'GROUP BY `feed_id` ' .
+			'HAVING COUNT(*) > ?';
+		$params = array($status, $threshold);
+		$result = $this->execute($sql, $params);
+
+		while($row = $result->fetchRow()) {
+
+			$size = (int) $row['size'];
+			$limit = $size - $threshold;
+
+			if($limit > 0) {
+				$params = array($status, $row['feed_id'], $limit);
+
+				$sql = 'DELETE FROM `*PREFIX*news_items` ' .
+				'WHERE `id` IN (' .
+					'SELECT `id` FROM `*PREFIX*news_items` ' .
+					'WHERE NOT ((`status` & ?) > 0) ' .
+					'AND `feed_id` = ? ' .
+					'ORDER BY `id` ASC ' .
+					'LIMIT ?' .
+				')';
+
+				$this->execute($sql, $params);
+			}
+		}
+
+	}
+
+
+}
\ No newline at end of file
diff --git a/dependencyinjection/dicontainer.php b/dependencyinjection/dicontainer.php
index 4d0d2d7ad..8ad816f8d 100644
--- a/dependencyinjection/dicontainer.php
+++ b/dependencyinjection/dicontainer.php
@@ -45,6 +45,7 @@ use \OCA\News\Db\FolderMapper;
 use \OCA\News\Db\FeedMapper;
 use \OCA\News\Db\ItemMapper;
 use \OCA\News\Db\StatusFlag;
+use \OCA\News\Db\MapperFactory;
 
 use \OCA\News\External\NewsAPI;
 use \OCA\News\External\FolderAPI;
@@ -208,6 +209,10 @@ class DIContainer extends BaseContainer {
 		/**
 		 * MAPPERS
 		 */
+		$this['MapperFactory'] = $this->share(function($c){
+			return new MapperFactory($c['API']);
+		});
+
 		$this['FolderMapper'] = $this->share(function($c){
 			return new FolderMapper($c['API']);
 		});
@@ -217,7 +222,7 @@ class DIContainer extends BaseContainer {
 		});
 
 		$this['ItemMapper'] = $this->share(function($c){
-			return new ItemMapper($c['API']);
+			return $c['MapperFactory']->getItemMapper($c['API']);
 		});
 
 
diff --git a/tests/unit/db/ItemMapperTest.php b/tests/unit/db/ItemMapperTest.php
index 23896727e..eb04b1514 100644
--- a/tests/unit/db/ItemMapperTest.php
+++ b/tests/unit/db/ItemMapperTest.php
@@ -331,9 +331,6 @@ class ItemMapperTest extends \OCA\AppFramework\Utility\MapperTestUtility {
 
 
 	public function testDeleteReadOlderThanThreshold(){
-		$this->markTestIncomplete(
-			'Fix on postgres first'
-		);
 		$threshold = 10;
 		$status = StatusFlag::STARRED | StatusFlag::UNREAD;
 
diff --git a/tests/unit/db/MapperFactoryTest.php b/tests/unit/db/MapperFactoryTest.php
new file mode 100644
index 000000000..c1be2095b
--- /dev/null
+++ b/tests/unit/db/MapperFactoryTest.php
@@ -0,0 +1,75 @@
+<?php
+
+/**
+* ownCloud - News
+*
+* @author Alessandro Cosentino
+* @author Bernhard Posselt
+* @copyright 2012 Alessandro Cosentino cosenal@gmail.com
+* @copyright 2012 Bernhard Posselt dev@bernhard-posselt.com
+*
+* This library is free software; you can redistribute it and/or
+* modify it under the terms of the GNU AFFERO GENERAL PUBLIC LICENSE
+* License as published by the Free Software Foundation; either
+* version 3 of the License, or any later version.
+*
+* This library is distributed in the hope that it will be useful,
+* but WITHOUT ANY WARRANTY; without even the implied warranty of
+* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+* GNU AFFERO GENERAL PUBLIC LICENSE for more details.
+*
+* You should have received a copy of the GNU Affero General Public
+* License along with this library.  If not, see <http://www.gnu.org/licenses/>.
+*
+*/
+
+namespace OCA\News\Db;
+
+require_once(__DIR__ . "/../../classloader.php");
+
+
+
+class MapperTest extends \PHPUnit_Framework_TestCase {
+
+
+	public function setUp() {
+		$this->api = $this->getMockBuilder('\OCA\AppFramework\Core\API')
+			->disableOriginalConstructor()
+			->getMock();
+	}
+
+
+	public function testGetItemMapperSqlite() {
+		$this->api->expects($this->once())
+			->method('getSystemValue')
+			->with($this->equalTo('dbtype'))
+			->will($this->returnValue('sqlite'));
+		$factory = new MapperFactory($this->api);
+
+		$this->assertTrue($factory->getItemMapper() instanceof ItemMapper);
+	}
+
+
+	public function testGetItemMapperMysql() {
+		$this->api->expects($this->once())
+			->method('getSystemValue')
+			->with($this->equalTo('dbtype'))
+			->will($this->returnValue('mysql'));
+		$factory = new MapperFactory($this->api);
+
+		$this->assertTrue($factory->getItemMapper() instanceof ItemMapper);
+	}
+
+
+	public function testGetItemMapperPostgres() {
+		$this->api->expects($this->once())
+			->method('getSystemValue')
+			->with($this->equalTo('dbtype'))
+			->will($this->returnValue('pgsql'));
+		$factory = new MapperFactory($this->api);
+
+		$this->assertTrue($factory->getItemMapper() instanceof \OCA\News\Db\Postgres\ItemMapper);
+	}
+
+
+}
\ No newline at end of file
diff --git a/tests/unit/db/postgres/ItemMapperTest.php b/tests/unit/db/postgres/ItemMapperTest.php
new file mode 100644
index 000000000..3e9fb8867
--- /dev/null
+++ b/tests/unit/db/postgres/ItemMapperTest.php
@@ -0,0 +1,191 @@
+<?php
+
+/**
+* ownCloud - News
+*
+* @author Alessandro Cosentino
+* @author Bernhard Posselt
+* @copyright 2012 Alessandro Cosentino cosenal@gmail.com
+* @copyright 2012 Bernhard Posselt dev@bernhard-posselt.com
+*
+* This library is free software; you can redistribute it and/or
+* modify it under the terms of the GNU AFFERO GENERAL PUBLIC LICENSE
+* License as published by the Free Software Foundation; either
+* version 3 of the License, or any later version.
+*
+* This library is distributed in the hope that it will be useful,
+* but WITHOUT ANY WARRANTY; without even the implied warranty of
+* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+* GNU AFFERO GENERAL PUBLIC LICENSE for more details.
+*
+* You should have received a copy of the GNU Affero General Public
+* License along with this library.  If not, see <http://www.gnu.org/licenses/>.
+*
+*/
+
+namespace OCA\News\Db\Postgres;
+
+use \OCA\News\Db\Item;
+use \OCA\News\Db\StatusFlag;
+
+
+require_once(__DIR__ . "/../../../classloader.php");
+
+
+class ItemMapperTest extends \OCA\AppFramework\Utility\MapperTestUtility {
+
+	private $mapper;
+	private $items;
+	private $newestItemId;
+	private $limit;
+	private $user;
+	private $offset;
+	private $updatedSince;
+	private $status;
+
+
+	public function setUp()
+	{
+		$this->beforeEach();
+
+		$this->mapper = new ItemMapper($this->api);
+
+		// create mock items
+		$item1 = new Item();
+		$item2 = new Item();
+
+		$this->items = array(
+			$item1,
+			$item2
+		);
+
+		$this->userId = 'john';
+		$this->id = 3;
+		$this->folderId = 2;
+
+		$this->row = array(
+			array('id' => $this->items[0]->getId()),
+		);
+
+		$this->rows = array(
+			array('id' => $this->items[0]->getId()),
+			array('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){
+		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` DESC';
+	}
+
+	private function makeSelectQueryStatus($prependTo, $status) {
+		$status = (int) $status;
+
+		return $this->makeSelectQuery(
+			'AND ((`items`.`status` & ' . $status . ') = ' . $status . ') ' .
+			$prependTo
+		);
+	}
+
+
+	public function testDeleteReadOlderThanThresholdDoesNotDeleteBelowThreshold(){
+		$status = StatusFlag::STARRED | StatusFlag::UNREAD;
+		$sql = 'SELECT COUNT(*) `size`, `feed_id` ' .
+			'FROM `*PREFIX*news_items` ' .
+			'WHERE NOT ((`status` & ?) > 0) ' .
+			'GROUP BY `feed_id` ' .
+			'HAVING COUNT(*) > ?';
+
+		$threshold = 10;
+		$rows = array(array('feed_id' => 30, 'size' => 9));
+		$params = array($status, $threshold);
+
+		$this->setMapperResult($sql, $params, $rows);
+		$this->mapper->deleteReadOlderThanThreshold($threshold);
+
+
+	}
+
+
+	public function testDeleteReadOlderThanThreshold(){
+		$threshold = 10;
+		$status = StatusFlag::STARRED | StatusFlag::UNREAD;
+
+		$sql1 = 'SELECT COUNT(*) `size`, `feed_id` ' .
+			'FROM `*PREFIX*news_items` ' .
+			'WHERE NOT ((`status` & ?) > 0) ' .
+			'GROUP BY `feed_id` ' .
+			'HAVING COUNT(*) > ?';
+		$params1 = array($status, $threshold);
+
+
+		$row = array('feed_id' => 30, 'size' => 11);
+
+		$sql2 = 'DELETE FROM `*PREFIX*news_items` ' .
+				'WHERE `id` IN (' .
+					'SELECT `id` FROM `*PREFIX*news_items` ' .
+					'WHERE NOT ((`status` & ?) > 0) ' .
+					'AND `feed_id` = ? ' .
+					'ORDER BY `id` ASC ' .
+					'LIMIT ?' .
+				')';
+		$params2 = array($status, 30, 1);
+
+
+		$pdoResult = $this->getMock('Result',
+			array('fetchRow'));
+
+		$pdoResult->expects($this->at(0))
+			->method('fetchRow')
+			->will($this->returnValue($row));
+		$pdoResult->expects($this->at(1))
+			->method('fetchRow')
+			->will($this->returnValue(false));
+
+		$query = $this->getMock('Query',
+			array('execute'));
+		$query->expects($this->at(0))
+			->method('execute')
+			->with($this->equalTo($params1))
+			->will($this->returnValue($pdoResult));
+
+		$this->api->expects($this->at(0))
+			->method('prepareQuery')
+			->with($this->equalTo($sql1))
+			->will(($this->returnValue($query)));
+
+		$query2 = $this->getMock('Query',
+			array('execute'));
+		$query2->expects($this->at(0))
+			->method('execute')
+			->with($this->equalTo($params2));
+
+		$this->api->expects($this->at(1))
+			->method('prepareQuery')
+			->with($this->equalTo($sql2))
+			->will($this->returnValue($query2));
+
+		$result = $this->mapper->deleteReadOlderThanThreshold($threshold);
+	}
+
+
+}