Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Refactor lastSeen and markReadAsGone #5470

Merged
merged 10 commits into from
Jun 16, 2023
Merged

Refactor lastSeen and markReadAsGone #5470

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
f7d36da
Refactor lastSeen and markReadAsGone
Alkarex Jun 15, 2023
1e8d342
Remove forgotten SQL param
Alkarex Jun 16, 2023
53b9fcf
Add test inTransaction
Alkarex Jun 16, 2023
86a82fb
More robust transaction
Alkarex Jun 16, 2023
90101c5
Add a debug log
Alkarex Jun 16, 2023
8b4738c
Add max timestamp to markAsReadUponGone
Alkarex Jun 16, 2023
7cf15dd
Reduce number of debug lines
Alkarex Jun 16, 2023
71f4b68
typing
Alkarex Jun 16, 2023
aecb8e1
Better detection of when feed is empty
Alkarex Jun 16, 2023
3e2a3c2
More explicit case for push
Alkarex Jun 16, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
52 changes: 29 additions & 23 deletions app/Controllers/feedController.php
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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