Sfoglia il codice sorgente

Fix “mark as read older than…” widening the active search (#9173)

* Fix “mark as read older than…” widening the active search

Sibling searches of a `FreshRSS_BooleanSearch` are combined by OR, so
`prepend()` added the maximum publication date used by the “mark as read →
articles older than one day / one week” action as one more OR term instead of
an extra restriction.

With an active search and the stream sorted by publication date, that action
therefore marked as read every article older than the cut-off (ignoring the
search) plus every article matching the search at any date.

`prepend()` now wraps the existing expression first, reusing the logic that
`enforce()` already used for the same purpose.

Co-Authored-By: Claude <noreply@anthropic.com>

* Fix SQL space

* Slight reduction of comments

---------

Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Alexandre Alapetite <alexandre@alapetite.fr>
TowyTowy 22 ore fa
parent
commit
fa8e8a4400

+ 22 - 11
app/Models/BooleanSearch.php

@@ -432,8 +432,28 @@ class FreshRSS_BooleanSearch implements \Stringable {
 		return $this->operator;
 		return $this->operator;
 	}
 	}
 
 
-	/** @param FreshRSS_BooleanSearch|FreshRSS_Search $search */
+	/**
+	 * Wrap the existing searches in a single BooleanSearch if needed,
+	 * so that another search can be added as an additional restriction (AND).
+	 */
+	private function wrapSearches(): void {
+		if (count($this->searches) > 1 || (count($this->searches) > 0 && $this->searches[0] instanceof FreshRSS_Search)) {
+			$wrap = new FreshRSS_BooleanSearch('');
+			foreach ($this->searches as $existingSearch) {
+				$wrap->add($existingSearch);
+			}
+			if (count($wrap->searches) > 0) {
+				$this->searches = [$wrap];
+			}
+		}
+	}
+
+	/**
+	 * Add a search at the beginning of the Boolean expression, as an additional restriction (AND).
+	 * @param FreshRSS_BooleanSearch|FreshRSS_Search $search
+	 */
 	public function prepend(FreshRSS_BooleanSearch|FreshRSS_Search $search): void {
 	public function prepend(FreshRSS_BooleanSearch|FreshRSS_Search $search): void {
+		$this->wrapSearches();
 		array_unshift($this->searches, $search);
 		array_unshift($this->searches, $search);
 	}
 	}
 
 
@@ -472,16 +492,7 @@ class FreshRSS_BooleanSearch implements \Stringable {
 			}
 			}
 		}
 		}
 
 
-		if (count($result->searches) > 1 || (count($result->searches) > 0 && $result->searches[0] instanceof FreshRSS_Search)) {
-			// Wrap the existing searches in a new BooleanSearch if needed
-			$wrap = new FreshRSS_BooleanSearch('');
-			foreach ($result->searches as $existingSearch) {
-				$wrap->add($existingSearch);
-			}
-			if (count($wrap->searches) > 0) {
-				$result->searches = [$wrap];
-			}
-		}
+		$result->wrapSearches();
 		array_unshift($result->searches, $search);
 		array_unshift($result->searches, $search);
 		return $result;
 		return $result;
 	}
 	}

+ 1 - 1
app/Models/EntryDAO.php

@@ -970,7 +970,7 @@ class FreshRSS_EntryDAO extends Minz_ModelPdo {
 
 
 				if ($filterSearch !== '') {
 				if ($filterSearch !== '') {
 					if ($search !== '') {
 					if ($search !== '') {
-						$search .= $filter->operator();
+						$search = rtrim($search) . ' ' . $filter->operator();
 					} elseif (in_array($filter->operator(), ['AND NOT', 'OR NOT'], true)) {
 					} elseif (in_array($filter->operator(), ['AND NOT', 'OR NOT'], true)) {
 						// Special case if we start with a negation (there is already the default AND before)
 						// Special case if we start with a negation (there is already the default AND before)
 						$search .= ' NOT';
 						$search .= ' NOT';

+ 33 - 0
tests/app/Models/BooleanSearchTest.php

@@ -0,0 +1,33 @@
+<?php
+declare(strict_types=1);
+
+use PHPUnit\Framework\Attributes\DataProvider;
+
+final class BooleanSearchTest extends \PHPUnit\Framework\TestCase {
+
+	/**
+	 * `FreshRSS_BooleanSearch::prepend()` is used to restrict an existing search with an extra condition,
+	 * such as the maximum publication date of the “mark as read → articles older than one day/week” action.
+	 * @return list<array{string,string,list<string|int>}>
+	 */
+	public static function providePrependMaxPubdate(): array {
+		return [
+			['', '(e.date <= ?)', [1700000000]],
+			['intitle:sale', '(e.date <= ?) AND ((e.title LIKE ?))', [1700000000, '%sale%']],
+			['intitle:a OR intitle:b', '(e.date <= ?) AND ((e.title LIKE ?) OR (e.title LIKE ?))', [1700000000, '%a%', '%b%']],
+		];
+	}
+
+	/** @param list<string|int> $expectedValues */
+	#[DataProvider('providePrependMaxPubdate')]
+	public function test_prepend_restrictsTheSearchInsteadOfWideningIt(string $input, string $expectedSql, array $expectedValues): void {
+		$booleanSearch = new FreshRSS_BooleanSearch($input);
+		$maxPubdate = new FreshRSS_Search('');
+		$maxPubdate->setMaxPubdate(1700000000);
+		$booleanSearch->prepend($maxPubdate);
+
+		[$values, $sql] = FreshRSS_EntryDAO::sqlBooleanSearch('e.', $booleanSearch);
+		self::assertSame($expectedSql, trim($sql));
+		self::assertSame($expectedValues, $values);
+	}
+}