summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--bl/feedbl.php30
-rw-r--r--bl/itembl.php4
-rw-r--r--controller/itemcontroller.php3
-rw-r--r--db/itemmapper.php15
-rw-r--r--js/app/controllers/feedcontroller.coffee4
-rw-r--r--js/app/services/models/itemmodel.coffee38
-rw-r--r--js/public/app.js39
-rw-r--r--js/tests/controllers/feedcontrollerSpec.coffee8
-rw-r--r--js/tests/services/models/itemmodelSpec.coffee29
-rw-r--r--tests/bl/FeedBlTest.php53
-rw-r--r--tests/bl/ItemBlTest.php7
-rw-r--r--tests/controller/ItemControllerTest.php7
-rw-r--r--tests/db/ItemMapperTest.php16
13 files changed, 201 insertions, 52 deletions
diff --git a/bl/feedbl.php b/bl/feedbl.php
index a77f9bdd2..3f9708234 100644
--- a/bl/feedbl.php
+++ b/bl/feedbl.php
@@ -71,8 +71,10 @@ class FeedBl extends Bl {
$feed->setUserId($userId);
$feed = $this->mapper->insert($feed);
- // insert items
- foreach($items as $item){
+ // insert items in reverse order because the first one is usually the
+ // newest item
+ for($i=count($items)-1; $i>=0; $i--){
+ $item = $items[$i];
$item->setFeedId($feed->getId());
$this->itemMapper->insert($item);
}
@@ -106,8 +108,10 @@ class FeedBl extends Bl {
try {
list($feed, $items) = $this->feedFetcher->fetch($feed->getUrl());
- // update items
- foreach($items as $item){
+ // insert items in reverse order because the first one is usually the
+ // newest item
+ for($i=count($items)-1; $i>=0; $i--){
+ $item = $items[$i];
// if a database exception is being thrown the unique constraint
// on the item guid hash is being violated and we need to update
@@ -116,10 +120,24 @@ class FeedBl extends Bl {
$item->setFeedId($feed->getId());
$this->itemMapper->insert($item);
} catch(\DatabaseException $ex){
+
+ // in case of an update the existing item has to be deleted
+ // if the pub_date changed because we sort by id on the
+ // client side since this is the only reliable way to do it
+ // to not get weird behaviour
$existing = $this->itemMapper->findByGuidHash(
$item->getGuidHash(), $feedId, $userId);
- $item->setId($existing->getId());
- $this->itemMapper->update($item);
+
+ if($existing->getPubDate() !== $item->getPubDate()){
+
+ // because the item is being replaced we need to keep
+ // status flags
+ $item->setStatus($existing->getStatus());
+ $item->setUnread();
+
+ $this->itemMapper->insert($item);
+ }
+
}
}
diff --git a/bl/itembl.php b/bl/itembl.php
index 332a51774..3c8649201 100644
--- a/bl/itembl.php
+++ b/bl/itembl.php
@@ -112,8 +112,8 @@ class ItemBl extends Bl {
}
- public function readFeed($feedId, $userId){
- $this->mapper->readFeed($feedId, $userId);
+ public function readFeed($feedId, $highestItemId, $userId){
+ $this->mapper->readFeed($feedId, $highestItemId, $userId);
}
diff --git a/controller/itemcontroller.php b/controller/itemcontroller.php
index 47061a14c..2c00fb1bd 100644
--- a/controller/itemcontroller.php
+++ b/controller/itemcontroller.php
@@ -163,8 +163,9 @@ class ItemController extends Controller {
public function readFeed(){
$userId = $this->api->getUserId();
$feedId = (int) $this->params('feedId');
+ $highestItemId = (int) $this->params('highestItemId');
- $this->itemBl->readFeed($feedId, $userId);
+ $this->itemBl->readFeed($feedId, $highestItemId, $userId);
}
diff --git a/db/itemmapper.php b/db/itemmapper.php
index 0208472b3..f4702a626 100644
--- a/db/itemmapper.php
+++ b/db/itemmapper.php
@@ -92,14 +92,15 @@ class ItemMapper extends Mapper implements IMapper {
}
- public function readFeed($feedId, $userId){
+ public function readFeed($feedId, $highestItemId, $userId){
$sql = 'UPDATE `*PREFIX*news_feeds` `feeds` ' .
'JOIN `*PREFIX*news_items` `items` ' .
'ON `items`.`feed_id` = `feeds`.`id` ' .
'AND `feeds`.`user_id` = ? ' .
- 'SET `items`.`status` = (`items`.`status` & ?) ' .
- 'WHERE `items`.`id` = ?';
- $params = array(~StatusFlag::UNREAD, $userId, $feedId);
+ 'AND `feeds`.`id` = ? ' .
+ 'AND `items`.`id` <= ? ' .
+ 'SET `items`.`status` = (`items`.`status` & ?) ';
+ $params = array($userId, $feedId, $highestItemId, ~StatusFlag::UNREAD);
$this->execute($sql, $params);
}
@@ -107,7 +108,7 @@ class ItemMapper extends Mapper implements IMapper {
public function findAllNewFeed($id, $updatedSince, $status, $userId){
$sql = 'AND `items`.`feed_id` = ? ' .
- 'AND `items`.`last_modified` >= ?';
+ 'AND `items`.`id` >= ?';
$sql = $this->makeSelectQueryStatus($sql);
$params = array($userId, $status, $id, $updatedSince);
return $this->findAllRows($sql, $params);
@@ -116,7 +117,7 @@ class ItemMapper extends Mapper implements IMapper {
public function findAllNewFolder($id, $updatedSince, $status, $userId){
$sql = 'AND `feeds`.`folder_id` = ? ' .
- 'AND `items`.`last_modified` >= ?';
+ 'AND `items`.`id` >= ?';
$sql = $this->makeSelectQueryStatus($sql);
$params = array($userId, $status, $id, $updatedSince);
return $this->findAllRows($sql, $params);
@@ -124,7 +125,7 @@ class ItemMapper extends Mapper implements IMapper {
public function findAllNew($updatedSince, $status, $userId){
- $sql = $this->makeSelectQueryStatus('AND `items`.`last_modified` >= ?');
+ $sql = $this->makeSelectQueryStatus('AND `items`.`id` >= ?');
$params = array($userId, $status, $updatedSince);
return $this->findAllRows($sql, $params);
}
diff --git a/js/app/controllers/feedcontroller.coffee b/js/app/controllers/feedcontroller.coffee
index 1c04cd92e..14e1aea5b 100644
--- a/js/app/controllers/feedcontroller.coffee
+++ b/js/app/controllers/feedcontroller.coffee
@@ -179,7 +179,7 @@ angular.module('News').factory '_FeedController', ->
@_persistence.getItems(type, id, 0)
@_active.handle({id: id, type: type})
else
- lastModified = @_itemModel.getLastModified()
+ lastModified = @_itemModel.getHighestId()
@_persistence.getItems(type, id, 0, null, lastModified)
@@ -208,6 +208,8 @@ angular.module('News').factory '_FeedController', ->
feed = @_feedModel.getById(id)
if angular.isDefined(feed)
feed.unreadCount = 0
+ # TODO: also update items in the right field if id is the
+ # the same
highestItemId = @_itemModel.getHighestId()
@_persistence.setFeedRead(id, highestItemId)
when @_feedType.Folder
diff --git a/js/app/services/models/itemmodel.coffee b/js/app/services/models/itemmodel.coffee
index 18af373d9..16250ecf7 100644
--- a/js/app/services/models/itemmodel.coffee
+++ b/js/app/services/models/itemmodel.coffee
@@ -27,14 +27,40 @@ angular.module('News').factory '_ItemModel',
class ItemModel extends _Model
- getLastModified: ->
- query = new _MaximumQuery('lastModified')
- lastModified = @get(query)
+ constructor: ->
+ @_guidFeedIdHash = {}
+ super()
+
+
+ clear: ->
+ @_guidFeedIdHash = {}
+ super()
+
+
+ # items have two unique fields: feed_id and guidhash
+ # in case we get updated items with the same two fields we
+ # also need to update the field
+ add: (data, clearCache=true) ->
+ hash = data.feedId + '_' + data.guidHash
+ entry = @_guidFeedIdHash[hash]
+
+ # update entry if exists with same feedid and guidhash
+ if angular.isDefined(entry)
+
+ # first update id that could have changed
+ delete @_dataMap[entry.id]
+ @_dataMap[data.id] = entry
+
+ # now copy over the elements data attrs
+ for key, value of data
+ if key == 'feedId' or key == 'guidHash'
+ continue
+ else
+ entry[key] = value
- if angular.isDefined(lastModified)
- return lastModified.lastModified
else
- return null
+ @_guidFeedIdHash[hash] = data
+ super(data, clearCache)
getHighestId: ->
diff --git a/js/public/app.js b/js/public/app.js
index 9efc9aece..d2ff687ae 100644
--- a/js/public/app.js
+++ b/js/public/app.js
@@ -372,7 +372,7 @@ License along with this library. If not, see <http://www.gnu.org/licenses/>.
type: type
});
} else {
- lastModified = this._itemModel.getLastModified();
+ lastModified = this._itemModel.getHighestId();
return this._persistence.getItems(type, id, 0, null, lastModified);
}
};
@@ -824,17 +824,38 @@ License along with this library. If not, see <http://www.gnu.org/licenses/>.
__extends(ItemModel, _super);
function ItemModel() {
- return ItemModel.__super__.constructor.apply(this, arguments);
+ this._guidFeedIdHash = {};
+ ItemModel.__super__.constructor.call(this);
}
- ItemModel.prototype.getLastModified = function() {
- var lastModified, query;
- query = new _MaximumQuery('lastModified');
- lastModified = this.get(query);
- if (angular.isDefined(lastModified)) {
- return lastModified.lastModified;
+ ItemModel.prototype.clear = function() {
+ this._guidFeedIdHash = {};
+ return ItemModel.__super__.clear.call(this);
+ };
+
+ ItemModel.prototype.add = function(data, clearCache) {
+ var entry, hash, key, value, _results;
+ if (clearCache == null) {
+ clearCache = true;
+ }
+ hash = data.feedId + '_' + data.guidHash;
+ entry = this._guidFeedIdHash[hash];
+ if (angular.isDefined(entry)) {
+ delete this._dataMap[entry.id];
+ this._dataMap[data.id] = entry;
+ _results = [];
+ for (key in data) {
+ value = data[key];
+ if (key === 'feedId' || key === 'guidHash') {
+ continue;
+ } else {
+ _results.push(entry[key] = value);
+ }
+ }
+ return _results;
} else {
- return null;
+ this._guidFeedIdHash[hash] = data;
+ return ItemModel.__super__.add.call(this, data, clearCache);
}
};
diff --git a/js/tests/controllers/feedcontrollerSpec.coffee b/js/tests/controllers/feedcontrollerSpec.coffee
index 8ef621030..6ec75e920 100644
--- a/js/tests/controllers/feedcontrollerSpec.coffee
+++ b/js/tests/controllers/feedcontrollerSpec.coffee
@@ -152,7 +152,7 @@ describe '_FeedController', ->
@ActiveFeed.handle({id: 3, type: 3})
@scope.loadFeed(3, 3)
- expect(@persistence.getItems).toHaveBeenCalledWith(3, 3, 0, null, 323)
+ expect(@persistence.getItems).toHaveBeenCalledWith(3, 3, 0, null, 6)
it 'should send a get all items query when feed changed', =>
@@ -232,9 +232,9 @@ describe '_FeedController', ->
it 'should mark feed as read', =>
@persistence.setFeedRead = jasmine.createSpy('setFeedRead')
@FeedModel.add({id: 5, unreadCount:2, folderId: 2})
- @ItemModel.add({id: 6})
- @ItemModel.add({id: 3})
- @ItemModel.add({id: 2})
+ @ItemModel.add({id: 6, feedId: 5, guidHash: 'a'})
+ @ItemModel.add({id: 3, feedId: 5, guidHash: 'a1'})
+ @ItemModel.add({id: 2, feedId: 5, guidHash: 'a2'})
@scope.markAllRead(@FeedType.Feed, 5)
expect(@persistence.setFeedRead).toHaveBeenCalledWith(5, 6)
diff --git a/js/tests/services/models/itemmodelSpec.coffee b/js/tests/services/models/itemmodelSpec.coffee
index 061f84f01..7d929b223 100644
--- a/js/tests/services/models/itemmodelSpec.coffee
+++ b/js/tests/services/models/itemmodelSpec.coffee
@@ -29,4 +29,31 @@ describe '_ItemModel', ->
it 'should extend model', =>
- expect(new @_ItemModel instanceof @_Model).toBeTruthy() \ No newline at end of file
+ expect(new @_ItemModel instanceof @_Model).toBeTruthy()
+
+
+ it 'should also update items with the same feed id and guidhash', =>
+ model = new @_ItemModel()
+ item1 = {id: 4, guidHash: 'abc', feedId: 3}
+ model.add(item1)
+
+ expect(model.getById(4)).toBe(item1)
+
+ # normal id update
+ item2 = {id: 4, guidHash: 'abc', feedId: 4}
+ model.add(item2)
+ expect(model.size()).toBe(1)
+
+ # new feeds should be added normally if different
+ item3 = {id: 5, guidHash: 'abc', feedId: 6}
+ model.add(item3)
+ expect(model.size()).toBe(2)
+
+ # feed should be updated when guidhash and feedid the same
+ item4 = {id: 3, guidHash: 'abc', feedId: 6}
+ model.add(item4)
+ expect(model.getById(3).guidHash).toBe(item4.guidHash)
+ expect(model.getById(3).feedId).toBe(item4.feedId)
+ expect(model.getById(3).id).toBe(item4.id)
+ expect(model.getById(5)).toBe(undefined)
+ expect(model.size()).toBe(2) \ No newline at end of file
diff --git a/tests/bl/FeedBlTest.php b/tests/bl/FeedBlTest.php
index b43060112..c970ac837 100644
--- a/tests/bl/FeedBlTest.php
+++ b/tests/bl/FeedBlTest.php
@@ -118,10 +118,10 @@ class FeedBlTest extends \OCA\AppFramework\Utility\TestUtility {
->will($this->returnValue($createdFeed));
$this->itemMapper->expects($this->at(0))
->method('insert')
- ->with($this->equalTo($return[1][0]));
+ ->with($this->equalTo($return[1][1]));
$this->itemMapper->expects($this->at(1))
->method('insert')
- ->with($this->equalTo($return[1][1]));
+ ->with($this->equalTo($return[1][0]));
$feed = $this->bl->create($url, $folderId, $this->user);
@@ -168,7 +168,7 @@ class FeedBlTest extends \OCA\AppFramework\Utility\TestUtility {
}
- public function testUpdateUpdatesEntry(){
+ public function testUpdateUpdatesEntryNotWhenPubDateSame(){
$feed = new Feed();
$feed->setId(3);
$feed->getUrl('test');
@@ -176,6 +176,7 @@ class FeedBlTest extends \OCA\AppFramework\Utility\TestUtility {
$item = new Item();
$item->setGuidHash(md5('hi'));
+ $item->setPubDate(3333);
$items = array(
$item
);
@@ -200,13 +201,57 @@ class FeedBlTest extends \OCA\AppFramework\Utility\TestUtility {
$this->equalTo($feed->getId()),
$this->equalTo($this->user))
->will($this->returnValue($item));
+
+ $this->bl->update($feed->getId(), $this->user);
+ }
+
+
+
+ public function testUpdateUpdatesEntry(){
+ $feed = new Feed();
+ $feed->setId(3);
+ $feed->getUrl('test');
+ $ex = new \DatabaseException('');
+
+ $item = new Item();
+ $item->setGuidHash(md5('hi'));
+ $item->setPubDate(3333);
+ $items = array(
+ $item
+ );
+
+ $item2 = new Item();
+ $item2->setPubDate(111);
+
+ $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->at(0))
+ ->method('insert')
+ ->with($this->equalTo($items[0]))
+ ->will($this->throwException($ex));
$this->itemMapper->expects($this->once())
- ->method('update')
+ ->method('findByGuidHash')
+ ->with($this->equalTo($item->getGuidHash()),
+ $this->equalTo($feed->getId()),
+ $this->equalTo($this->user))
+ ->will($this->returnValue($item2));
+ $this->itemMapper->expects($this->at(2))
+ ->method('insert')
->with($this->equalTo($item));
$this->bl->update($feed->getId(), $this->user);
+ $this->assertTrue($item->isUnread());
}
+
public function testCreateUpdateFails(){
$feed = new Feed();
$feed->setId(3);
diff --git a/tests/bl/ItemBlTest.php b/tests/bl/ItemBlTest.php
index dcc208705..225435b7d 100644
--- a/tests/bl/ItemBlTest.php
+++ b/tests/bl/ItemBlTest.php
@@ -230,12 +230,15 @@ class ItemBlTest extends \OCA\AppFramework\Utility\TestUtility {
public function testReadFeed(){
$feedId = 3;
+ $highestItemId = 6;
$this->mapper->expects($this->once())
->method('readFeed')
- ->with($this->equalTo($feedId), $this->equalTo($this->user));
+ ->with($this->equalTo($feedId),
+ $this->equalTo($highestItemId),
+ $this->equalTo($this->user));
- $this->bl->readFeed($feedId, $this->user);
+ $this->bl->readFeed($feedId, $highestItemId, $this->user);
}
}
diff --git a/tests/controller/ItemControllerTest.php b/tests/controller/ItemControllerTest.php
index 0c9899d4f..3ab3de145 100644
--- a/tests/controller/ItemControllerTest.php
+++ b/tests/controller/ItemControllerTest.php
@@ -181,14 +181,17 @@ class ItemControllerTest extends ControllerTestUtility {
$url = array(
'feedId' => 4
);
- $this->controller = $this->getPostController(array(), $url);
+ $post = array(
+ 'highestItemId' => 5
+ );
+ $this->controller = $this->getPostController($post, $url);
$this->api->expects($this->once())
->method('getUserId')
->will($this->returnValue($this->user));
$this->bl->expects($this->once())
->method('readFeed')
- ->with($url['feedId'], $this->user);
+ ->with($url['feedId'], $post['highestItemId'], $this->user);
$this->controller->readFeed();
}
diff --git a/tests/db/ItemMapperTest.php b/tests/db/ItemMapperTest.php
index d8b76722b..ea0feadd8 100644
--- a/tests/db/ItemMapperTest.php
+++ b/tests/db/ItemMapperTest.php
@@ -119,15 +119,17 @@ class ItemMapperTest extends \OCA\AppFramework\Utility\MapperTestUtility {
'JOIN `*PREFIX*news_items` `items` ' .
'ON `items`.`feed_id` = `feeds`.`id` ' .
'AND `feeds`.`user_id` = ? ' .
- 'SET `items`.`status` = (`items`.`status` & ?) ' .
- 'WHERE `items`.`id` = ?';
- $this->setMapperResult($sql, array(~StatusFlag::UNREAD, $this->user, 3));
- $this->mapper->readFeed(3, $this->user);
+ 'AND `feeds`.`id` = ? ' .
+ 'AND `items`.`id` <= ? ' .
+ 'SET `items`.`status` = (`items`.`status` & ?) ';
+ $params = array($this->user, 3, 6, ~StatusFlag::UNREAD);
+ $this->setMapperResult($sql, $params);
+ $this->mapper->readFeed(3, 6, $this->user);
}
public function testFindAllNew(){
- $sql = 'AND `items`.`last_modified` >= ?';
+ $sql = 'AND `items`.`id` >= ?';
$sql = $this->makeSelectQueryStatus($sql);
$params = array($this->user, $this->status, $this->updatedSince);
@@ -141,7 +143,7 @@ class ItemMapperTest extends \OCA\AppFramework\Utility\MapperTestUtility {
public function testFindAllNewFeed(){
$sql = 'AND `items`.`feed_id` = ? ' .
- 'AND `items`.`last_modified` >= ?';
+ 'AND `items`.`id` >= ?';
$sql = $this->makeSelectQueryStatus($sql);
$params = array($this->user, $this->status, $this->id, $this->updatedSince);
@@ -155,7 +157,7 @@ class ItemMapperTest extends \OCA\AppFramework\Utility\MapperTestUtility {
public function testFindAllNewFolder(){
$sql = 'AND `feeds`.`folder_id` = ? ' .
- 'AND `items`.`last_modified` >= ?';
+ 'AND `items`.`id` >= ?';
$sql = $this->makeSelectQueryStatus($sql);
$params = array($this->user, $this->status, $this->id,
generated by cgit v1.2.3 (git 2.25.1) at 2024-05-25 01:01:23 +0000