Skip to content

Commit

Permalink
Refactor lastSeen and markReadAsGone (#5470)
Browse files Browse the repository at this point in the history 
* Refactor lastSeen and markReadAsGone
Make the logic a bit more robust and explicit

* Remove forgotten SQL param

* Add test inTransaction

* More robust transaction

* Add a debug log

* Add max timestamp to markAsReadUponGone

* Reduce number of debug lines

* typing

* Better detection of when feed is empty

* More explicit case for push
  • Loading branch information
@Alkarex
Alkarex committed Jun 16, 2023
1 parent 228d7ad commit 723f757
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 37 deletions.
52 changes: 29 additions & 23 deletions app/Controllers/feedController.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -368,42 +368,40 @@ public static function actualizeFeed(int $feed_id, string $feed_url, bool $force
$url = $feed->url(); //For detection of HTTP 301

$pubSubHubbubEnabled = $pubsubhubbubEnabledGeneral && $feed->pubSubHubbubEnabled();
if ((!$simplePiePush) && (!$feed_id) && $pubSubHubbubEnabled && ($feed->lastUpdate() > $pshbMinAge)) {
if ($simplePiePush === null && $feed_id === 0 && $pubSubHubbubEnabled && ($feed->lastUpdate() > $pshbMinAge)) {
//$text = 'Skip pull of feed using PubSubHubbub: ' . $url;
//Minz_Log::debug($text);
//Minz_Log::debug($text, PSHB_LOG);
continue; //When PubSubHubbub is used, do not pull refresh so often
}

$mtime = 0;
if ($feed->mute()) {
continue; //Feed refresh is disabled
}
$mtime = $feed->cacheModifiedTime() ?: 0;
$ttl = $feed->ttl();
if ((!$simplePiePush) && (!$feed_id) &&
($feed->lastUpdate() + 10 >= time() - (
$ttl === FreshRSS_Feed::TTL_DEFAULT ? FreshRSS_Context::$user_conf->ttl_default : $ttl))) {
if ($ttl === FreshRSS_Feed::TTL_DEFAULT) {
$ttl = FreshRSS_Context::$user_conf->ttl_default;
}
if ($simplePiePush === null && $feed_id === 0 && (time() <= $feed->lastUpdate() + $ttl)) {
//Too early to refresh from source, but check whether the feed was updated by another user
$mtime = $feed->cacheModifiedTime() ?: 0;
if ($feed->lastUpdate() + 10 >= $mtime) {
if ($mtime <= 0 || $feed->lastUpdate() >= $mtime) {
continue; //Nothing newer from other users
}
//Minz_Log::debug($feed->url(false) . ' was updated at ' . date('c', $mtime) . ' by another user');
//Will take advantage of the newer cache
} else {
$mtime = time();
Minz_Log::debug('Feed ' . $feed->url(false) . ' was updated at ' . date('c', $mtime) . ' by another user; will take advantage of the newer cache.');
}

if (!$feed->lock()) {
Minz_Log::notice('Feed already being actualized: ' . $feed->url(false));
continue;
}

$isNewFeed = $feed->lastUpdate() <= 0;
$feedIsNew = $feed->lastUpdate() <= 0;
$feedIsEmpty = false;
$feedIsUnchanged = false;

try {
if ($simplePiePush) {
if ($simplePiePush !== null) {
$simplePie = $simplePiePush; //Used by WebSub
} elseif ($feed->kind() === FreshRSS_Feed::KIND_HTML_XPATH) {
$simplePie = $feed->loadHtmlXpath();
Expand All @@ -416,14 +414,22 @@ public static function actualizeFeed(int $feed_id, string $feed_url, bool $force
throw new FreshRSS_Feed_Exception('XML+XPath parsing failed for [' . $feed->url(false) . ']');
}
} else {
$simplePie = $feed->load(false, $isNewFeed);
if ($simplePie === null) {
// Feed is cached and unchanged
$feedIsUnchanged = true;
}
$simplePie = $feed->load(false, $feedIsNew);
}

if ($simplePie === null) {
// Feed is cached and unchanged
$newGuids = [];
$entries = [];
$feedIsEmpty = false; // We do not know
$feedIsUnchanged = true;
} else {
$newGuids = $feed->loadGuids($simplePie);
$entries = $feed->loadEntries($simplePie);
$feedIsEmpty = $simplePiePush !== null && empty($newGuids);
$feedIsUnchanged = false;
}
$newGuids = $simplePie === null ? [] : $feed->loadGuids($simplePie);
$entries = $simplePie === null ? [] : $feed->loadEntries($simplePie);
$mtime = $feed->cacheModifiedTime() ?: time();
} catch (FreshRSS_Feed_Exception $e) {
Minz_Log::warning($e->getMessage());
$feedDAO->updateLastUpdate($feed->id(), true);
Expand Down Expand Up @@ -553,9 +559,9 @@ public static function actualizeFeed(int $feed_id, string $feed_url, bool $force

$feedDAO->updateLastUpdate($feed->id(), false, $mtime);
$needFeedCacheRefresh |= ($feed->keepMaxUnread() != false);
if (!$simplePiePush) {
if ($simplePiePush === null) {
// Do not call for WebSub events, as we do not know the list of articles still on the upstream feed.
$needFeedCacheRefresh |= ($feed->markAsReadUponGone() != false);
$needFeedCacheRefresh |= ($feed->markAsReadUponGone($feedIsEmpty, $mtime) != false);
}
if ($needFeedCacheRefresh) {
$feedDAO->updateCachedValues($feed->id());
Expand Down Expand Up @@ -608,7 +614,7 @@ public static function actualizeFeed(int $feed_id, string $feed_url, bool $force
}
if (!empty($feedProperties)) {
$ok = $feedDAO->updateFeed($feed->id(), $feedProperties);
if (!$ok && $isNewFeed) {
if (!$ok && $feedIsNew) {
//Cancel adding new feed in case of database error at first actualize
$feedDAO->deleteFeed($feed->id());
$feed->unlock();
Expand Down
13 changes: 9 additions & 4 deletions app/Models/EntryDAO.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ protected function autoUpdateDb(array $errorInfo): bool {
*/
private $addEntryPrepared = false;

/** @param array{'id':string,'guid':string,'title':string,'author':string,'content':string,'link':string,'date':int,'hash':string,
/** @param array{'id':string,'guid':string,'title':string,'author':string,'content':string,'link':string,'date':int,'lastSeen':int,'hash':string,
* 'is_read':bool|int|null,'is_favorite':bool|int|null,'id_feed':int,'tags':string,'attributes':array<string,mixed>} $valuesTmp */
public function addEntry(array $valuesTmp, bool $useTmpTable = true): bool {
if ($this->addEntryPrepared == null) {
Expand Down Expand Up @@ -556,7 +556,10 @@ public function markReadFeed(int $id_feed, string $idMax = '0', ?FreshRSS_Boolea
$idMax = time() . '000000';
Minz_Log::debug('Calling markReadFeed(0) is deprecated!');
}
$this->pdo->beginTransaction();
$hadTransaction = $this->pdo->inTransaction();
if (!$hadTransaction) {
$this->pdo->beginTransaction();
}

$sql = 'UPDATE `_entry` '
. 'SET is_read=? '
Expand Down Expand Up @@ -589,7 +592,9 @@ public function markReadFeed(int $id_feed, string $idMax = '0', ?FreshRSS_Boolea
}
}

$this->pdo->commit();
if (!$hadTransaction) {
$this->pdo->commit();
}
return $affected;
}

Expand Down Expand Up @@ -698,7 +703,7 @@ public function cleanOldEntries(int $id_feed, array $options = []) {
}
}

/** @return Traversable<array{'id':string,'guid':string,'title':string,'author':string,'content':string,'link':string,'date':int,
/** @return Traversable<array{'id':string,'guid':string,'title':string,'author':string,'content':string,'link':string,'date':int,'lastSeen':int,
* 'hash':string,'is_read':?bool,'is_favorite':?bool,'id_feed':int,'tags':string,'attributes':array<string,mixed>}> */
public function selectAll(): Traversable {
$content = static::isCompressed() ? 'UNCOMPRESS(content_bin) AS content' : 'content';
Expand Down
24 changes: 18 additions & 6 deletions app/Models/Feed.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -764,23 +764,35 @@ public function keepMaxUnread() {

/**
* Applies the *mark as read upon gone* policy, if enabled.
* Remember to call updateCachedValue($id_feed) or updateCachedValues() just after.
* Remember to call `updateCachedValue($id_feed)` or `updateCachedValues()` just after.
* @return int|false the number of lines affected, or false if not applicable
*/
public function markAsReadUponGone() {
public function markAsReadUponGone(bool $upstreamIsEmpty, int $maxTimestamp = 0) {
$readUponGone = $this->attributes('read_upon_gone');
if ($readUponGone === null) {
$readUponGone = FreshRSS_Context::$user_conf->mark_when['gone'];
}
if ($readUponGone) {
if (!$readUponGone) {
return false;
}
if ($upstreamIsEmpty) {
if ($maxTimestamp <= 0) {
$maxTimestamp = time();
}
$entryDAO = FreshRSS_Factory::createEntryDao();
$affected = $entryDAO->markReadFeed($this->id(), $maxTimestamp . '000000');
} else {
$feedDAO = FreshRSS_Factory::createFeedDao();
return $feedDAO->markAsReadUponGone($this->id());
$affected = $feedDAO->markAsReadUponGone($this->id());
}
return false;
if ($affected > 0) {
Minz_Log::debug(__METHOD__ . " $affected items" . ($upstreamIsEmpty ? ' (all)' : '') . ' [' . $this->url(false) . ']');
}
return $affected;
}

/**
* Remember to call updateCachedValue($id_feed) or updateCachedValues() just after
* Remember to call `updateCachedValue($id_feed)` or `updateCachedValues()` just after
* @return int|false
*/
public function cleanOldEntries() {
Expand Down
4 changes: 0 additions & 4 deletions app/Models/FeedDAO.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -503,16 +503,12 @@ public function markAsReadUponGone(int $id) {
`lastSeen` + 60 < (SELECT s1.maxlastseen FROM (
SELECT MAX(e2.`lastSeen`) AS maxlastseen FROM `_entry` e2 WHERE e2.id_feed = :id_feed2
) s1)
OR `lastSeen` + 60 < (SELECT s2.lastcorrectupdate FROM (
SELECT f2.`lastUpdate` AS lastcorrectupdate FROM `_feed` f2 WHERE f2.id = :id_feed3 AND f2.error = 0
) s2)
)
SQL;

if (($stm = $this->pdo->prepare($sql)) &&
$stm->bindParam(':id_feed1', $id, PDO::PARAM_INT) &&
$stm->bindParam(':id_feed2', $id, PDO::PARAM_INT) &&
$stm->bindParam(':id_feed3', $id, PDO::PARAM_INT) &&
$stm->execute()) {
return $stm->rowCount();
} else {
Expand Down

0 comments on commit 723f757

Please sign in to comment.