Fix entry->lastSeen in case of feed errors (#8646)

* Fix entry->lastSeen in case of feed errors
Fix https://github.com/FreshRSS/FreshRSS/issues/8643
Fix lastSeen and feed's lastUpdate going out of sync

Previous related PRs:
* https://github.com/FreshRSS/FreshRSS/pull/5404
* https://github.com/FreshRSS/FreshRSS/pull/5382
* https://github.com/FreshRSS/FreshRSS/pull/5315

* Minor uneeded change

app/Controllers/entryController.php

@@ -268,6 +268,7 @@ class FreshRSS_entry_Controller extends FreshRSS_ActionController {
 		}
 
 		$databaseDAO = FreshRSS_Factory::createDatabaseDAO();
+		$databaseDAO->minorDbMaintenance();
 		$databaseDAO->optimize();
 
 		$feedDAO = FreshRSS_Factory::createFeedDao();
@@ -295,6 +296,9 @@ class FreshRSS_entry_Controller extends FreshRSS_ActionController {
 			@set_time_limit(300);
 		}
 
+		$databaseDAO = FreshRSS_Factory::createDatabaseDAO();
+		$databaseDAO->minorDbMaintenance();
+
 		$feedDAO = FreshRSS_Factory::createFeedDao();
 		$feeds = $feedDAO->listFeeds();
 		$nb_total = 0;
@@ -310,9 +314,6 @@ class FreshRSS_entry_Controller extends FreshRSS_ActionController {
 		$feedDAO->updateCachedValues();
 		$feedDAO->commit();
 
-		$databaseDAO = FreshRSS_Factory::createDatabaseDAO();
-		$databaseDAO->minorDbMaintenance();
-
 		invalidateHttpCache();
 		Minz_Request::good(
 			_t('feedback.sub.purge_completed', $nb_total),

app/Controllers/feedController.php

@@ -567,7 +567,7 @@ class FreshRSS_feed_Controller extends FreshRSS_ActionController {
 				$mtime = $feed->cacheModifiedTime() ?: time();
 			} catch (FreshRSS_Feed_Exception $e) {
 				Minz_Log::warning($e->getMessage());
-				$feedDAO->updateLastUpdate($feed->id(), true);
+				$feedDAO->updateLastError($feed->id());
 				if ($e->getCode() === 410) {
 					// HTTP 410 Gone
 					Minz_Log::warning('Muting gone feed: ' . $feed->url(false));
@@ -720,10 +720,13 @@ class FreshRSS_feed_Controller extends FreshRSS_ActionController {
 				}
 			}
 
-			$feedDAO->updateLastUpdate($feed->id(), false, $mtime);
-			if ($simplePiePush === null) {
+			if ($simplePiePush === null) {	// Not WebSub
+				$feedDAO->updateLastUpdate($feed->id(), $mtime);
 				// Do not call for WebSub events, as we do not know the list of articles still on the upstream feed.
 				$needFeedCacheRefresh |= ($feed->markAsReadUponGone($feedIsEmpty, $mtime) != false);
+			} elseif ($feed->inError()) {
+				// Reset feed error state in case of successful WebSub push
+				$feedDAO->updateLastError($feed->id(), 0);
 			}
 			if ($needFeedCacheRefresh) {
 				$feedsCacheToRefresh[] = $feed;
@@ -928,12 +931,13 @@ class FreshRSS_feed_Controller extends FreshRSS_ActionController {
 			$feedDAO = FreshRSS_Factory::createFeedDao();
 			$feedDAO->updateCachedValues();
 		} else {
-			if ($id === 0 && $url === '') {
-				// Case of a batch refresh (e.g. cron)
+			if (!$noCommit) {
 				$databaseDAO = FreshRSS_Factory::createDatabaseDAO();
 				$databaseDAO->minorDbMaintenance();
 				Minz_ExtensionManager::callHookVoid(Minz_HookType::FreshrssUserMaintenance);
-
+			}
+			if ($id === 0 && $url === '') {
+				// Case of a batch refresh (e.g. cron)
 				FreshRSS_feed_Controller::commitNewEntries();
 				$feedDAO = FreshRSS_Factory::createFeedDao();
 				$feedDAO->updateCachedValues();

app/Models/Category.php

@@ -267,7 +267,11 @@ class FreshRSS_Category extends Minz_Model {
 		}
 
 		$catDAO = FreshRSS_Factory::createCategoryDao();
-		$catDAO->updateLastUpdate($this->id(), !$ok);
+		if ($ok) {
+			$catDAO->updateLastUpdate($this->id());
+		} else {
+			$catDAO->updateLastError($this->id());
+		}
 
 		return (bool)$ok;
 	}

app/Models/CategoryDAO.php

@@ -30,7 +30,7 @@ class FreshRSS_CategoryDAO extends Minz_ModelPdo {
 			} elseif ($name === 'lastUpdate') {	//v1.20.0
 				return $this->pdo->exec('ALTER TABLE `_category` ADD COLUMN `lastUpdate` BIGINT DEFAULT 0') !== false;
 			} elseif ($name === 'error') {	//v1.20.0
-				return $this->pdo->exec('ALTER TABLE `_category` ADD COLUMN error SMALLINT DEFAULT 0') !== false;
+				return $this->pdo->exec('ALTER TABLE `_category` ADD COLUMN error BIGINT DEFAULT 0') !== false;
 			} elseif ('attributes' === $name) {	//v1.15.0
 				$ok = $this->pdo->exec('ALTER TABLE `_category` ADD COLUMN attributes TEXT') !== false;
 
@@ -212,14 +212,30 @@ class FreshRSS_CategoryDAO extends Minz_ModelPdo {
 		}
 	}
 
-	public function updateLastUpdate(int $id, bool $inError = false, int $mtime = 0): int|false {
+	public function updateLastUpdate(int $id, int $mtime = 0): int|false {
 		$sql = <<<'SQL'
-			UPDATE `_category` SET `lastUpdate`=:last_update, error=:error WHERE id=:id
+			UPDATE `_category` SET `lastUpdate`=:last_update, error=0 WHERE id=:id
 			SQL;
 		$stm = $this->pdo->prepare($sql);
 		if ($stm !== false &&
 			$stm->bindValue(':last_update', $mtime <= 0 ? time() : $mtime, PDO::PARAM_INT) &&
-			$stm->bindValue(':error', $inError ? 1 : 0, PDO::PARAM_INT) &&
+			$stm->bindValue(':id', $id, PDO::PARAM_INT) &&
+			$stm->execute()) {
+			return $stm->rowCount();
+		} else {
+			$info = $stm === false ? $this->pdo->errorInfo() : $stm->errorInfo();
+			Minz_Log::error('SQL error ' . __METHOD__ . json_encode($info));
+			return false;
+		}
+	}
+
+	public function updateLastError(int $id, ?int $mtime = null): int|false {
+		$sql = <<<'SQL'
+			UPDATE `_category` SET error=:last_update WHERE id=:id
+			SQL;
+		$stm = $this->pdo->prepare($sql);
+		if ($stm !== false &&
+			$stm->bindValue(':last_update', $mtime === null || $mtime < 0 ? time() : $mtime, PDO::PARAM_INT) &&
 			$stm->bindValue(':id', $id, PDO::PARAM_INT) &&
 			$stm->execute()) {
 			return $stm->rowCount();

app/Models/EntryDAO.php

@@ -1951,7 +1951,7 @@ class FreshRSS_EntryDAO extends Minz_ModelPdo {
 	public function updateLastSeenUnchanged(int $id_feed, int $mtime = 0): int|false {
 		$sql = <<<'SQL'
 			UPDATE `_entry` SET `lastSeen` = :mtime
-			WHERE id_feed = :id_feed1 AND `lastSeen` = (
+			WHERE id_feed = :id_feed1 AND `lastSeen` >= (
 				SELECT `lastUpdate` FROM `_feed` f
 				WHERE f.id = :id_feed2
 			)

app/Models/FeedDAO.php

@@ -227,14 +227,30 @@ class FreshRSS_FeedDAO extends Minz_ModelPdo {
 	/**
 	 * @see updateCachedValues()
 	 */
-	public function updateLastUpdate(int $id, bool $inError = false, int $mtime = 0): int|false {
+	public function updateLastUpdate(int $id, int $mtime = 0): int|false {
 		$sql = <<<'SQL'
-			UPDATE `_feed` SET `lastUpdate`=:last_update, error=:error WHERE id=:id
+			UPDATE `_feed` SET `lastUpdate`=:last_update, error=0 WHERE id=:id
 			SQL;
 		$stm = $this->pdo->prepare($sql);
 		if ($stm !== false &&
 			$stm->bindValue(':last_update', $mtime <= 0 ? time() : $mtime, PDO::PARAM_INT) &&
-			$stm->bindValue(':error', $inError ? 1 : 0, PDO::PARAM_INT) &&
+			$stm->bindValue(':id', $id, PDO::PARAM_INT) &&
+			$stm->execute()) {
+			return $stm->rowCount();
+		} else {
+			$info = $stm === false ? $this->pdo->errorInfo() : $stm->errorInfo();
+			Minz_Log::warning(__METHOD__ . ' error: ' . $sql . ' : ' . json_encode($info));
+			return false;
+		}
+	}
+
+	public function updateLastError(int $id, ?int $mtime = null): int|false {
+		$sql = <<<'SQL'
+			UPDATE `_feed` SET error=:last_update WHERE id=:id
+			SQL;
+		$stm = $this->pdo->prepare($sql);
+		if ($stm !== false &&
+			$stm->bindValue(':last_update', $mtime === null || $mtime < 0 ? time() : $mtime, PDO::PARAM_INT) &&
 			$stm->bindValue(':id', $id, PDO::PARAM_INT) &&
 			$stm->execute()) {
 			return $stm->rowCount();
@@ -445,6 +461,7 @@ class FreshRSS_FeedDAO extends Minz_ModelPdo {
 	public function listFeedsOrderUpdate(int $defaultCacheDuration = 3600, int $limit = 0): array {
 		$ttlDefault = FreshRSS_Feed::TTL_DEFAULT;
 		$refreshThreshold = time() + 60;
+		$lastAttemptExpression = '(CASE WHEN error > `lastUpdate` THEN error ELSE `lastUpdate` END)';
 
 		$sql = <<<SQL
 			SELECT * FROM `_feed`
@@ -452,11 +469,11 @@ class FreshRSS_FeedDAO extends Minz_ModelPdo {
 		if ($defaultCacheDuration >= 0) {
 			$sql .= "\n" . <<<SQL
 				WHERE ttl >= {$ttlDefault}
-				AND `lastUpdate` < ({$refreshThreshold}-(CASE WHEN ttl={$ttlDefault} THEN {$defaultCacheDuration} ELSE ttl END))
+				AND {$lastAttemptExpression} < ({$refreshThreshold}-(CASE WHEN ttl={$ttlDefault} THEN {$defaultCacheDuration} ELSE ttl END))
 				SQL;
 		}
 		$sql .= "\n" . <<<SQL
-			ORDER BY `lastUpdate`
+			ORDER BY {$lastAttemptExpression} ASC
 			SQL;
 		if ($limit > 0) {
 			$sql .= "\n" . <<<SQL

app/SQL/install.sql.mysql.php

@@ -9,7 +9,7 @@ CREATE TABLE IF NOT EXISTS `_category` (
 	`name` VARCHAR(191) NOT NULL,	-- Max index length for Unicode is 191 characters (767 bytes) FreshRSS_DatabaseDAO::LENGTH_INDEX_UNICODE
 	`kind` SMALLINT DEFAULT 0,	-- 1.20.0
 	`lastUpdate` BIGINT DEFAULT 0,	-- 1.20.0
-	`error` SMALLINT DEFAULT 0,	-- 1.20.0
+	`error` BIGINT DEFAULT 0,	-- Date, v1.29.0
 	`attributes` TEXT,	-- v1.15.0
 	PRIMARY KEY (`id`),
 	UNIQUE KEY (`name`)	-- v0.7
@@ -28,7 +28,7 @@ CREATE TABLE IF NOT EXISTS `_feed` (
 	`priority` TINYINT(2) NOT NULL DEFAULT 10,
 	`pathEntries` VARCHAR(4096) DEFAULT NULL,
 	`httpAuth` VARCHAR(1024) CHARACTER SET latin1 COLLATE latin1_bin DEFAULT NULL,
-	`error` BOOLEAN DEFAULT 0,
+	`error` BIGINT DEFAULT 0,	-- Date, v1.29.0
 	`ttl` INT NOT NULL DEFAULT 0,	-- v0.7.3
 	`attributes` TEXT,	-- v1.11.0
 	`cache_nbEntries` INT DEFAULT 0,	-- v0.7
@@ -137,14 +137,17 @@ BEGIN
 
 	SELECT COUNT(*) INTO up_to_date FROM information_schema.COLUMNS
 		WHERE TABLE_SCHEMA = DATABASE()
-		AND TABLE_NAME = REPLACE('`_tag`', '`', '')
-		AND COLUMN_NAME = 'name'
-		AND COLUMN_TYPE = 'VARCHAR(191)';
+		AND TABLE_NAME = REPLACE('`_feed`', '`', '')
+		AND COLUMN_NAME = 'error'
+		AND DATA_TYPE = 'bigint';
 
 	IF up_to_date = 0 THEN
+		ALTER TABLE `_category`
+			MODIFY COLUMN `error` BIGINT DEFAULT 0;	-- v1.29.0
 		ALTER TABLE `_feed`
 			MODIFY COLUMN `website` TEXT CHARACTER SET latin1 COLLATE latin1_bin,
 			MODIFY COLUMN `lastUpdate` BIGINT DEFAULT 0,
+			MODIFY COLUMN `error` BIGINT DEFAULT 0,	-- v1.29.0
 			MODIFY COLUMN `pathEntries` VARCHAR(4096),
 			MODIFY COLUMN `httpAuth` VARCHAR(1024) CHARACTER SET latin1 COLLATE latin1_bin DEFAULT NULL;
 		ALTER TABLE `_entry`

app/SQL/install.sql.pgsql.php

@@ -9,7 +9,7 @@ CREATE TABLE IF NOT EXISTS `_category` (
 	"name" VARCHAR(191) UNIQUE NOT NULL,
 	"kind" SMALLINT DEFAULT 0,	-- 1.20.0
 	"lastUpdate" BIGINT DEFAULT 0,	-- 1.20.0
-	"error" SMALLINT DEFAULT 0,	-- 1.20.0
+	"error" BIGINT DEFAULT 0,	-- Date, v1.29.0
 	"attributes" TEXT	-- v1.15.0
 );
 
@@ -25,7 +25,7 @@ CREATE TABLE IF NOT EXISTS `_feed` (
 	"priority" SMALLINT NOT NULL DEFAULT 10,
 	"pathEntries" VARCHAR(4096) DEFAULT NULL,
 	"httpAuth" VARCHAR(1024) DEFAULT NULL,
-	"error" SMALLINT DEFAULT 0,
+	"error" BIGINT DEFAULT 0,	-- Date, v1.29.0
 	"ttl" INT NOT NULL DEFAULT 0,
 	"attributes" TEXT,	-- v1.11.0
 	"cache_nbEntries" INT DEFAULT 0,
@@ -122,18 +122,20 @@ BEGIN
 	IF NOT EXISTS (
 		SELECT 1 FROM information_schema.columns
 		WHERE table_schema = 'public'
-			AND table_name = REPLACE('`_tag`', '"', '')
-			AND column_name = 'name'
-			AND character_maximum_length = 191
+			AND table_name = REPLACE('`_feed`', '"', '')
+			AND column_name = 'error'
+			AND data_type = 'bigint'
 	) THEN
 		ALTER TABLE `_category`
-			ALTER COLUMN "name" SET DATA TYPE VARCHAR(191);
+			ALTER COLUMN "name" SET DATA TYPE VARCHAR(191),
+			ALTER COLUMN "error" SET DATA TYPE BIGINT;
 		ALTER TABLE `_feed`
 			DROP CONSTRAINT IF EXISTS `_feed_url_key`,
 			ALTER COLUMN "url" SET DATA TYPE VARCHAR(32768),
 			ALTER COLUMN "name" SET DATA TYPE VARCHAR(191),
 			ALTER COLUMN "website" SET DATA TYPE VARCHAR(32768),
 			ALTER COLUMN "lastUpdate" SET DATA TYPE BIGINT,
+			ALTER COLUMN "error" SET DATA TYPE BIGINT,
 			ALTER COLUMN "pathEntries" SET DATA TYPE VARCHAR(4096),
 			ALTER COLUMN "httpAuth" SET DATA TYPE VARCHAR(1024);
 		ALTER TABLE `_entry`

app/SQL/install.sql.sqlite.php

@@ -9,7 +9,7 @@ CREATE TABLE IF NOT EXISTS `category` (
 	`name` VARCHAR(191) NOT NULL,
 	`kind` SMALLINT DEFAULT 0,	-- 1.20.0
 	`lastUpdate` BIGINT DEFAULT 0,	-- 1.20.0
-	`error` SMALLINT DEFAULT 0,	-- 1.20.0
+	`error` BIGINT DEFAULT 0,	-- Date, v1.29.0
 	`attributes` TEXT,	-- v1.15.0
 	UNIQUE (`name`)
 );
@@ -26,7 +26,7 @@ CREATE TABLE IF NOT EXISTS `feed` (
 	`priority` TINYINT(2) NOT NULL DEFAULT 10,
 	`pathEntries` VARCHAR(4096) DEFAULT NULL,
 	`httpAuth` VARCHAR(1024) DEFAULT NULL,
-	`error` BOOLEAN DEFAULT 0,
+	`error` BIGINT DEFAULT 0,	-- Date, v1.29.0
 	`ttl` INT NOT NULL DEFAULT 0,
 	`attributes` TEXT,	-- v1.11.0
 	`cache_nbEntries` INT DEFAULT 0,