diff options
author | Bernhard Posselt <nukeawhale@gmail.com> | 2013-03-25 12:54:38 +0100 |
---|---|---|
committer | Bernhard Posselt <nukeawhale@gmail.com> | 2013-03-25 12:54:38 +0100 |
commit | 0f9e0e3de77de68cc75ccd69395665f9e1346951 (patch) | |
tree | be6398f7134c3192e4ee0f89a88cbbfadfffc808 | |
parent | 77ec6f08aa4fb223fba859ca0f6060e84006da43 (diff) |
implemented feed update
-rw-r--r-- | appinfo/database.xml | 1 | ||||
-rw-r--r-- | bl/feedbl.php | 48 | ||||
-rw-r--r-- | bl/itembl.php | 6 | ||||
-rw-r--r-- | db/itemmapper.php | 16 | ||||
-rw-r--r-- | dependencyinjection/dicontainer.php | 3 | ||||
-rw-r--r-- | tests/bl/FeedBlTest.php | 109 | ||||
-rw-r--r-- | tests/bl/ItemBlTest.php | 11 | ||||
-rw-r--r-- | tests/db/ItemMapperTest.php | 16 |
8 files changed, 171 insertions, 39 deletions
diff --git a/appinfo/database.xml b/appinfo/database.xml index 8fd44b76f..b8e8947c0 100644 --- a/appinfo/database.xml +++ b/appinfo/database.xml @@ -212,6 +212,7 @@ <index> <name>news_items_item_guid</name> + <unique>true</unique> <field> <name>guid_hash</name> </field> diff --git a/bl/feedbl.php b/bl/feedbl.php index 1340de632..105a43a05 100644 --- a/bl/feedbl.php +++ b/bl/feedbl.php @@ -26,21 +26,28 @@ namespace OCA\News\Bl; use \OCA\AppFramework\Db\DoesNotExistException; +use \OCA\AppFramework\Core\API; use \OCA\News\Db\Feed; use \OCA\News\Db\FeedMapper; +use \OCA\News\Db\ItemMapper; use \OCA\News\Utility\FeedFetcher; use \OCA\News\Utility\FetcherException; class FeedBl extends Bl { private $feedFetcher; + private $itemMapper; + private $api; public function __construct(FeedMapper $feedMapper, - FeedFetcher $feedFetcher, ItemBl $itemBl){ + FeedFetcher $feedFetcher, + ItemMapper $itemMapper, + API $api){ parent::__construct($feedMapper); $this->feedFetcher = $feedFetcher; - $this->itemBl = $itemBl; + $this->itemMapper = $itemMapper; + $this->api = $api; } @@ -66,7 +73,7 @@ class FeedBl extends Bl { // insert items foreach($items as $item){ $item->setFeedId($feed->getId()); - $this->itemBl->create($item); + $this->itemMapper->insert($item); } return $feed; @@ -75,17 +82,46 @@ class FeedBl extends Bl { } } + + // FIXME: this method is not covered by any tests public function updateAll(){ - // TODO: needs test $feeds = $this->mapper->findAll(); foreach($feeds as $feed){ - $this->update($feed->getId(), $feed->getUser()); + try { + $this->update($feed->getId(), $feed->getUser()); + } catch(BLException $ex){ + continue; + } } } public function update($feedId, $userId){ - // TODO: update given feed + $feed = $this->mapper->find($feedId, $userId); + try { + list($feed, $items) = $this->feedFetcher->fetch($feed->getUrl()); + + // update items + foreach($items as $item){ + + // if a database exception is being thrown the unique constraint + // on the item guid hash is being violated and we need to update + // the item + try { + $item->setFeedId($feed->getId()); + $this->itemMapper->insert($item); + } catch(\DatabaseException $ex){ + $existing = $this->itemMapper->findByGuidHash( + $item->getGuidHash(), $userId); + $item->setId($existing->getId()); + $this->itemMapper->update($item); + } + } + + } catch(FetcherException $ex){ + // failed updating is not really a problem, so only log it + $this->api->log('Can not update feed: Not found or bad source'); + } } diff --git a/bl/itembl.php b/bl/itembl.php index c45ee3a91..4a3f302c7 100644 --- a/bl/itembl.php +++ b/bl/itembl.php @@ -117,10 +117,4 @@ class ItemBl extends Bl { } - // ATTENTION: this does no validation and is only for creating - // items from the fetcher - public function create($item){ - $this->mapper->insert($item); - } - } diff --git a/db/itemmapper.php b/db/itemmapper.php index 0d9469c9d..8f200c39e 100644 --- a/db/itemmapper.php +++ b/db/itemmapper.php @@ -67,8 +67,8 @@ class ItemMapper extends Mapper implements IMapper { public function find($id, $userId){ - $sql = $this->makeSelectQuery('WHERE `*PREFIX*news_items`.`id` = ? '); - $row = $this->findOneQuery($sql, array($id, $userId)); + $sql = $this->makeSelectQuery('AND `*PREFIX*news_items`.`id` = ? '); + $row = $this->findOneQuery($sql, array($userId, $id)); $item = new Item(); $item->fromRow($row); @@ -168,4 +168,16 @@ class ItemMapper extends Mapper implements IMapper { return $this->findAllRows($sql, $params, $limit); } + + public function findByGuidHash($guidHash, $userId){ + $sql = $this->makeSelectQuery('AND `*PREFIX*news_items`.`guid_hash` = ? '); + $row = $this->findOneQuery($sql, array($userId, $guidHash)); + + $item = new Item(); + $item->fromRow($row); + + return $item; + } + + } diff --git a/dependencyinjection/dicontainer.php b/dependencyinjection/dicontainer.php index b162adeee..efbd28252 100644 --- a/dependencyinjection/dicontainer.php +++ b/dependencyinjection/dicontainer.php @@ -97,7 +97,8 @@ class DIContainer extends BaseContainer { }); $this['FeedBl'] = $this->share(function($c){ - return new FeedBl($c['FeedMapper'], $c['FeedFetcher'], $c['ItemBl']); + return new FeedBl($c['FeedMapper'], $c['FeedFetcher'], + $c['ItemMapper'], $c['API']); }); $this['ItemBl'] = $this->share(function($c){ diff --git a/tests/bl/FeedBlTest.php b/tests/bl/FeedBlTest.php index 15414a721..2ec3d3e47 100644 --- a/tests/bl/FeedBlTest.php +++ b/tests/bl/FeedBlTest.php @@ -23,7 +23,12 @@ * */ -namespace OCA\News\Bl; +namespace { + class DatabaseException extends Exception{}; +} + + +namespace OCA\News\Bl { require_once(__DIR__ . "/../classloader.php"); @@ -42,7 +47,7 @@ class FeedBlTest extends \OCA\AppFramework\Utility\TestUtility { protected $user; protected $response; protected $fetcher; - protected $itemBl; + protected $itemMapper; protected function setUp(){ $this->api = $this->getAPIMock(); @@ -52,10 +57,11 @@ class FeedBlTest extends \OCA\AppFramework\Utility\TestUtility { $this->fetcher = $this->getMockBuilder('\OCA\News\Utility\FeedFetcher') ->disableOriginalConstructor() ->getMock(); - $this->itemBl = $this->getMockBuilder('\OCA\News\Bl\ItemBl') + $this->itemMapper = $this->getMockBuilder('\OCA\News\Db\ItemMapper') ->disableOriginalConstructor() ->getMock(); - $this->bl = new FeedBl($this->mapper, $this->fetcher, $this->itemBl); + $this->bl = new FeedBl($this->mapper, + $this->fetcher, $this->itemMapper, $this->api); $this->user = 'jack'; $response = 'hi'; } @@ -110,11 +116,11 @@ class FeedBlTest extends \OCA\AppFramework\Utility\TestUtility { ->method('insert') ->with($this->equalTo($createdFeed)) ->will($this->returnValue($createdFeed)); - $this->itemBl->expects($this->at(0)) - ->method('create') + $this->itemMapper->expects($this->at(0)) + ->method('insert') ->with($this->equalTo($return[1][0])); - $this->itemBl->expects($this->at(1)) - ->method('create') + $this->itemMapper->expects($this->at(1)) + ->method('insert') ->with($this->equalTo($return[1][1])); $feed = $this->bl->create($url, $folderId, $this->user); @@ -130,14 +136,95 @@ class FeedBlTest extends \OCA\AppFramework\Utility\TestUtility { ->with($this->equalTo(md5($url)), $this->equalTo($this->user)); $this->setExpectedException('\OCA\News\Bl\BLException'); $this->bl->create($url, 1, $this->user); + } + + + public function testUpdateCreatesNewEntry(){ + $feed = new Feed(); + $feed->setId(3); + $feed->getUrl('test'); + + $item = new Item(); + $item->setGuidHash(md5('hi')); + $items = array( + $item + ); + + $fetchReturn = array($feed, $items); + $this->mapper->expects($this->once()) + ->method('find') + ->with($this->equalTo($feed->getId()), + $this->equalTo($this->user)) + ->will($this->returnValue($feed)); + $this->fetcher->expects($this->once()) + ->method('fetch') + ->will($this->returnValue($fetchReturn)); + $this->itemMapper->expects($this->once()) + ->method('insert') + ->with($this->equalTo($items[0])); + + $this->bl->update($feed->getId(), $this->user); } - public function testUpdate(){ - // TODO + public function testUpdateUpdatesEntry(){ + $feed = new Feed(); + $feed->setId(3); + $feed->getUrl('test'); + $ex = new \DatabaseException(''); + + $item = new Item(); + $item->setGuidHash(md5('hi')); + $items = array( + $item + ); + + $fetchReturn = array($feed, $items); + + $this->mapper->expects($this->once()) + ->method('find') + ->with($this->equalTo($feed->getId()), + $this->equalTo($this->user)) + ->will($this->returnValue($feed)); + $this->fetcher->expects($this->once()) + ->method('fetch') + ->will($this->returnValue($fetchReturn)); + $this->itemMapper->expects($this->once()) + ->method('insert') + ->with($this->equalTo($items[0])) + ->will($this->throwException($ex)); + $this->itemMapper->expects($this->once()) + ->method('findByGuidHash') + ->with($this->equalTo($item->getGuidHash()), + $this->equalTo($this->user)) + ->will($this->returnValue($item)); + $this->itemMapper->expects($this->once()) + ->method('update') + ->with($this->equalTo($item)); + + $this->bl->update($feed->getId(), $this->user); } + public function testCreateUpdateFails(){ + $feed = new Feed(); + $feed->setId(3); + $feed->getUrl('test'); + $ex = new FetcherException(''); + + $this->mapper->expects($this->once()) + ->method('find') + ->with($this->equalTo($feed->getId()), + $this->equalTo($this->user)) + ->will($this->returnValue($feed)); + $this->fetcher->expects($this->once()) + ->method('fetch') + ->will($this->throwException($ex)); + $this->api->expects($this->once()) + ->method('log'); + + $this->bl->update($feed->getId(), $this->user); + } public function testMove(){ $feedId = 3; @@ -161,4 +248,6 @@ class FeedBlTest extends \OCA\AppFramework\Utility\TestUtility { } +} + }
\ No newline at end of file diff --git a/tests/bl/ItemBlTest.php b/tests/bl/ItemBlTest.php index e8c4cc9d7..dcc208705 100644 --- a/tests/bl/ItemBlTest.php +++ b/tests/bl/ItemBlTest.php @@ -238,17 +238,6 @@ class ItemBlTest extends \OCA\AppFramework\Utility\TestUtility { $this->bl->readFeed($feedId, $this->user); } - - public function testCreate(){ - $item = new Item(); - - $this->mapper->expects($this->once()) - ->method('insert') - ->with($this->equalTo($item)); - - $this->bl->create($item, $this->user); - } - } diff --git a/tests/db/ItemMapperTest.php b/tests/db/ItemMapperTest.php index 626ac2cd0..433120a9f 100644 --- a/tests/db/ItemMapperTest.php +++ b/tests/db/ItemMapperTest.php @@ -88,13 +88,12 @@ class ItemMapperTest extends \OCA\AppFramework\Utility\MapperTestUtility { public function testFind(){ - $sql = $this->makeSelectQuery('WHERE `*PREFIX*news_items`.`id` = ? '); + $sql = $this->makeSelectQuery('AND `*PREFIX*news_items`.`id` = ? '); - $this->setMapperResult($sql, array($this->id, $this->userId), $this->row); + $this->setMapperResult($sql, array($this->userId, $this->id), $this->row); $result = $this->mapper->find($this->id, $this->userId); $this->assertEquals($this->items[0], $result); - } @@ -248,4 +247,15 @@ class ItemMapperTest extends \OCA\AppFramework\Utility\MapperTestUtility { $this->assertEquals($this->items, $result); } + + public function testFindByGuidHash(){ + $hash = md5('test'); + $sql = $this->makeSelectQuery('AND `*PREFIX*news_items`.`guid_hash` = ? '); + + $this->setMapperResult($sql, array($this->userId, $hash), $this->row); + + $result = $this->mapper->findByGuidHash($hash, $this->userId); + $this->assertEquals($this->items[0], $result); + } + }
\ No newline at end of file |