diff --git a/lib/Database.php b/lib/Database.php index ad6e76d3..472da39d 100644 --- a/lib/Database.php +++ b/lib/Database.php @@ -828,14 +828,14 @@ class Database { return (int) $this->db->prepare('INSERT INTO arsse_subscriptions(owner, url, deleted) values(?,?,?)', 'str', 'str', 'bool')->run($user, $url, 1)->lastId(); } catch (Db\ExceptionInput $e) { // if the insertion fails, throw if the delete flag is not set, otherwise return the existing ID - $id = (int) $this->db->prepare("SELECT id from arsse_subscriptions where owner = ? and url = ? and deleted = 1", "str", "str")->run($user, $url)->getValue(); + $id = $this->db->prepare("SELECT id from arsse_subscriptions where owner = ? and url = ? and deleted = 1", "str", "str")->run($user, $url)->getValue(); if (!$id) { throw $e; } else { // set the modification timestamp to the current time so it doesn't get cleaned up too soon $this->db->prepare("UPDATE arsse_subscriptions set modified = CURRENT_TIMESTAMP where id = ?", "int")->run($id); } - return $id; + return (int) $id; } } diff --git a/lib/Feed.php b/lib/Feed.php index 168a4427..3129a75d 100644 --- a/lib/Feed.php +++ b/lib/Feed.php @@ -29,7 +29,6 @@ class Feed { public $items = []; public $newItems = []; public $changedItems = []; - public $filteredItems = []; public static function discover(string $url, string $username = '', string $password = ''): string { // fetch the candidate feed @@ -83,9 +82,6 @@ class Feed { if (!sizeof($this->newItems) && !sizeof($this->changedItems)) { $this->modified = false; } else { - if ($feedID) { - $this->computeFilterRules($feedID); - } // if requested, scrape full content for any new and changed items if ($scrape) { $this->scrape(); diff --git a/lib/ImportExport/AbstractImportExport.php b/lib/ImportExport/AbstractImportExport.php index 6f0496fd..2a2df8d0 100644 --- a/lib/ImportExport/AbstractImportExport.php +++ b/lib/ImportExport/AbstractImportExport.php @@ -34,9 +34,18 @@ abstract class AbstractImportExport { $folderMap[$f['parent']][$f['name']] = true; } } - // get feed IDs for each URL, adding feeds where necessary + // add any new feeds, and try an initial fetch on them + $feedMap = []; foreach ($feeds as $k => $f) { - $feeds[$k]['id'] = Arsse::$db->feedAdd(($f['url'])); + try { + $feedMap[$k] = Arsse::$db->subscriptionReserve($user, $f['url']); + } catch (InputException $e) { + // duplication is not an error in this case + } + } + foreach ($feedMap as $f) { + // this may fail with an exception, halting the process before visible modifications are made to the database + Arsse::$db->subscriptionUpdate($user, $f, true); } // start a transaction for atomic rollback $tr = Arsse::$db->begin(); @@ -61,27 +70,26 @@ abstract class AbstractImportExport { } } // process newsfeed subscriptions - $feedMap = []; $tagMap = []; - foreach ($feeds as $f) { + foreach ($feeds as $k => $f) { $folder = $folderMap[$f['folder']]; $title = strlen(trim($f['title'])) ? $f['title'] : null; - $found = false; - // find a match for the import feed is existing subscriptions - foreach ($feedsDb as $db) { - if ((int) $db['feed'] == $f['id']) { - $found = true; - $feedMap[$f['id']] = (int) $db['id']; - break; + $new = false; + // find a match for the import feed in existing subscriptions, if necessary; reveal the subscription if it's just been added + if (!isset($feedMap[$k])) { + foreach ($feedsDb as $db) { + if ($db['url'] === $f['url']) { + $feedMap[$k] = (int) $db['id']; + break; + } } + } else { + $new = true; + Arsse::$db->subscriptionReveal($user, $feedMap[$k]); } - if (!$found) { - // if no subscription exists, add one - $feedMap[$f['id']] = Arsse::$db->subscriptionAdd($user, $f['url']); - } - if (!$found || $replace) { + if (!$new || $replace) { // set the subscription's properties, if this is a new feed or we're doing a full replacement - Arsse::$db->subscriptionPropertiesSet($user, $feedMap[$f['id']], ['title' => $title, 'folder' => $folder]); + Arsse::$db->subscriptionPropertiesSet($user, $feedMap[$k], ['title' => $title, 'folder' => $folder]); // compile the set of used tags, if this is a new feed or we're doing a full replacement foreach ($f['tags'] as $t) { if (!strlen(trim($t))) { @@ -92,7 +100,7 @@ abstract class AbstractImportExport { // populate the tag map $tagMap[$t] = []; } - $tagMap[$t][] = $f['id']; + $tagMap[$t][] = $feedMap[$k]; } } } diff --git a/lib/REST/Miniflux/V1.php b/lib/REST/Miniflux/V1.php index 08061446..c0e39cb1 100644 --- a/lib/REST/Miniflux/V1.php +++ b/lib/REST/Miniflux/V1.php @@ -350,6 +350,7 @@ class V1 extends \JKingWeb\Arsse\REST\AbstractHandler { // Miniflux does not attempt to coerce values into different types foreach (self::VALID_JSON as $k => $t) { if (!isset($body[$k])) { + // if a valid key is missing set it to null so that any key may be accessed safely $body[$k] = null; } elseif (gettype($body[$k]) !== $t) { return self::respError(["InvalidInputType", 'field' => $k, 'expected' => $t, 'actual' => gettype($body[$k])], 422); diff --git a/tests/cases/Feed/TestFeed.php b/tests/cases/Feed/TestFeed.php index 8630f51b..518df4e2 100644 --- a/tests/cases/Feed/TestFeed.php +++ b/tests/cases/Feed/TestFeed.php @@ -100,7 +100,6 @@ class TestFeed extends \JKingWeb\Arsse\Test\AbstractTest { $this->dbMock->feedMatchLatest->with(1, Phony::any())->returns(new Result($this->latest)); $this->dbMock->feedMatchIds->with(Phony::wildcard())->returns(new Result([])); $this->dbMock->feedMatchIds->with(1, Phony::wildcard())->returns(new Result($this->others)); - $this->dbMock->feedRulesGet->returns([]); Arsse::$db = $this->dbMock->get(); } @@ -386,26 +385,4 @@ class TestFeed extends \JKingWeb\Arsse\Test\AbstractTest { $this->assertSame("image/gif", $f->iconType); $this->assertSame($d, $f->iconData); } - - public function testApplyFilterRules(): void { - $exp = [ - 'jack' => ['new' => [false, true, true, false, true], 'changed' => [7 => true, 47 => true, 2112 => false, 1 => true, 42 => false]], - 'sam' => ['new' => [false, true, false, false, false], 'changed' => [7 => false, 47 => true, 2112 => false, 1 => false, 42 => false]], - ]; - $this->dbMock->feedMatchIds->returns(new Result([ - // these are the sixth through tenth entries in the feed; the title hashes have been omitted for brevity - ['id' => 7, 'guid' => '0f2a218c311e3d8105f1b075142a5d26dabf056ffc61abe77e96c8f071bbf4a7', 'edited' => null, 'url_title_hash' => "", 'url_content_hash' => '', 'title_content_hash' => ''], - ['id' => 47, 'guid' => '1c19e3b9018bc246b7414ae919ddebc88d0c575129e8c4a57b84b826c00f6db5', 'edited' => null, 'url_title_hash' => "", 'url_content_hash' => '', 'title_content_hash' => ''], - ['id' => 2112, 'guid' => '964db0b9292ad0c7a6c225f2e0966f3bda53486fae65db0310c97409974e65b8', 'edited' => null, 'url_title_hash' => "", 'url_content_hash' => '', 'title_content_hash' => ''], - ['id' => 1, 'guid' => '436070cda5713a0d9a8fdc8652c7ab142f0550697acfd5206a16c18aee355039', 'edited' => null, 'url_title_hash' => "", 'url_content_hash' => '', 'title_content_hash' => ''], - ['id' => 42, 'guid' => '1a731433a1904220ef26e731ada7262e1d5bcecae53e7b5df9e1f5713af6e5d3', 'edited' => null, 'url_title_hash' => "", 'url_content_hash' => '', 'title_content_hash' => ''], - ])); - $this->dbMock->feedRulesGet->returns([ - 'jack' => ['keep' => "", 'block' => '`A|W|J|S`u'], - 'sam' => ['keep' => "`B|T|X`u", 'block' => '`C`u'], - ]); - Arsse::$db = $this->dbMock->get(); - $f = new Feed(5, $this->base."Filtering/1"); - $this->assertSame($exp, $f->filteredItems); - } } diff --git a/tests/cases/REST/Miniflux/TestV1.php b/tests/cases/REST/Miniflux/TestV1.php index 759f7835..ced20dc9 100644 --- a/tests/cases/REST/Miniflux/TestV1.php +++ b/tests/cases/REST/Miniflux/TestV1.php @@ -553,93 +553,56 @@ class TestV1 extends \JKingWeb\Arsse\Test\AbstractTest { } /** @dataProvider provideFeedCreations */ - public function testCreateAFeed(array $in, $out1, $out2, $out3, $out4, ResponseInterface $exp): void { - if ($out1 instanceof \Exception) { - $this->dbMock->feedAdd->throws($out1); + public function testCreateAFeed(array $in, $out, ResponseInterface $exp): void { + if ($out instanceof \Exception) { + $this->dbMock->subscriptionAdd->throws($out); } else { - $this->dbMock->feedAdd->returns($out1); - } - if ($out2 instanceof \Exception) { - $this->dbMock->subscriptionAdd->throws($out2); - } else { - $this->dbMock->subscriptionAdd->returns($out2); - } - if ($out3 instanceof \Exception) { - $this->dbMock->subscriptionPropertiesSet->throws($out3); - } elseif ($out4 instanceof \Exception) { - $this->dbMock->subscriptionPropertiesSet->returns($out3)->throws($out4); - } else { - $this->dbMock->subscriptionPropertiesSet->returns($out3)->returns($out4); + $this->dbMock->subscriptionAdd->returns($out); } $this->assertMessage($exp, $this->req("POST", "/feeds", $in)); - $in1 = $out1 !== null; - $in2 = $out2 !== null; - $in3 = $out3 !== null; - $in4 = $out4 !== null; - if ($in1) { - $this->dbMock->feedAdd->calledWith($in['feed_url'], $in['username'] ?? "", $in['password'] ?? "", false, $in['crawler'] ?? false); - } else { - $this->dbMock->feedAdd->never()->called(); - } - if ($in2) { - $this->dbMock->begin->calledWith(); - $this->dbMock->subscriptionAdd->calledWith("john.doe@example.com", $in['feed_url'], $in['username'] ?? "", $in['password'] ?? "", false, $in['crawler'] ?? false); - } else { - $this->dbMock->begin->never()->called(); - $this->dbMock->subscriptionAdd->never()->called(); - } - if ($in3) { + if ($out) { $props = [ 'folder' => $in['category_id'] - 1, 'scrape' => $in['crawler'] ?? false, - ]; - $this->dbMock->subscriptionPropertiesSet->calledWith("john.doe@example.com", $out2, $props); - if (!$out3 instanceof \Exception) { - $this->transaction->commit->called(); - } - } else { - $this->dbMock->subscriptionPropertiesSet->never()->called(); - } - if ($in4) { - $rules = [ 'keep_rule' => $in['keeplist_rules'] ?? null, 'block_rule' => $in['blocklist_rules'] ?? null, ]; - $this->dbMock->subscriptionPropertiesSet->calledWith("john.doe@example.com", $out2, $rules); + $this->dbMock->subscriptionAdd->calledWith("john.doe@example.com", $in['feed_url'], $in['username'] ?? "", $in['password'] ?? "", false, $props); } else { - $this->dbMock->subscriptionPropertiesSet->atMost(1)->called(); + $this->dbMock->subscriptionAdd->never()->called(); } } public function provideFeedCreations(): iterable { self::clearData(); return [ - [['category_id' => 1], null, null, null, null, V1::respError(["MissingInputValue", 'field' => "feed_url"], 422)], - [['feed_url' => "http://example.com/"], null, null, null, null, V1::respError(["MissingInputValue", 'field' => "category_id"], 422)], - [['feed_url' => "http://example.com/", 'category_id' => "1"], null, null, null, null, V1::respError(["InvalidInputType", 'field' => "category_id", 'expected' => "integer", 'actual' => "string"], 422)], - [['feed_url' => "Not a URL", 'category_id' => 1], null, null, null, null, V1::respError(["InvalidInputValue", 'field' => "feed_url"], 422)], - [['feed_url' => "http://example.com/", 'category_id' => 0], null, null, null, null, V1::respError(["InvalidInputValue", 'field' => "category_id"], 422)], - [['feed_url' => "http://example.com/", 'category_id' => 1, 'keeplist_rules' => "["], null, null, null, null, V1::respError(["InvalidInputValue", 'field' => "keeplist_rules"], 422)], - [['feed_url' => "http://example.com/", 'category_id' => 1, 'blocklist_rules' => "["], null, null, null, null, V1::respError(["InvalidInputValue", 'field' => "blocklist_rules"], 422)], - [['feed_url' => "http://example.com/", 'category_id' => 1], new FeedException("internalError"), null, null, null, V1::respError("FetchOther", 502)], - [['feed_url' => "http://example.com/", 'category_id' => 1], new FeedException("invalidCertificate"), null, null, null, V1::respError("FetchOther", 502)], - [['feed_url' => "http://example.com/", 'category_id' => 1], new FeedException("invalidUrl"), null, null, null, V1::respError("Fetch404", 502)], - [['feed_url' => "http://example.com/", 'category_id' => 1], new FeedException("maxRedirect"), null, null, null, V1::respError("FetchOther", 502)], - [['feed_url' => "http://example.com/", 'category_id' => 1], new FeedException("maxSize"), null, null, null, V1::respError("FetchOther", 502)], - [['feed_url' => "http://example.com/", 'category_id' => 1], new FeedException("timeout"), null, null, null, V1::respError("FetchOther", 502)], - [['feed_url' => "http://example.com/", 'category_id' => 1], new FeedException("forbidden"), null, null, null, V1::respError("Fetch403", 502)], - [['feed_url' => "http://example.com/", 'category_id' => 1], new FeedException("unauthorized"), null, null, null, V1::respError("Fetch401", 502)], - [['feed_url' => "http://example.com/", 'category_id' => 1], new FeedException("transmissionError"), null, null, null, V1::respError("FetchOther", 502)], - [['feed_url' => "http://example.com/", 'category_id' => 1], new FeedException("connectionFailed"), null, null, null, V1::respError("FetchOther", 502)], - [['feed_url' => "http://example.com/", 'category_id' => 1], new FeedException("malformedXml"), null, null, null, V1::respError("FetchOther", 502)], - [['feed_url' => "http://example.com/", 'category_id' => 1], new FeedException("xmlEntity"), null, null, null, V1::respError("FetchOther", 502)], - [['feed_url' => "http://example.com/", 'category_id' => 1], new FeedException("subscriptionNotFound"), null, null, null, V1::respError("Fetch404", 502)], - [['feed_url' => "http://example.com/", 'category_id' => 1], new FeedException("unsupportedFeedFormat"), null, null, null, V1::respError("FetchFormat", 502)], - [['feed_url' => "http://example.com/", 'category_id' => 1], 2112, new ExceptionInput("constraintViolation"), null, null, V1::respError("DuplicateFeed", 409)], - [['feed_url' => "http://example.com/", 'category_id' => 1], 2112, 44, new ExceptionInput("idMissing"), null, V1::respError("MissingCategory", 422)], - [['feed_url' => "http://example.com/", 'category_id' => 1], 2112, 44, true, null, HTTP::respJson(['feed_id' => 44], 201)], - [['feed_url' => "http://example.com/", 'category_id' => 1, 'keeplist_rules' => "^A"], 2112, 44, true, true, HTTP::respJson(['feed_id' => 44], 201)], - [['feed_url' => "http://example.com/", 'category_id' => 1, 'blocklist_rules' => "A"], 2112, 44, true, true, HTTP::respJson(['feed_id' => 44], 201)], + [['category_id' => 1], null, V1::respError(["MissingInputValue", 'field' => "feed_url"], 422)], + [['feed_url' => "http://example.com/"], null, V1::respError(["MissingInputValue", 'field' => "category_id"], 422)], + [['feed_url' => "http://example.com/", 'category_id' => "1"], null, V1::respError(["InvalidInputType", 'field' => "category_id", 'expected' => "integer", 'actual' => "string"], 422)], + [['feed_url' => "Not a URL", 'category_id' => 1], null, V1::respError(["InvalidInputValue", 'field' => "feed_url"], 422)], + [['feed_url' => "http://example.com/", 'category_id' => 0], null, V1::respError(["InvalidInputValue", 'field' => "category_id"], 422)], + [['feed_url' => "http://example.com/", 'category_id' => 1, 'keeplist_rules' => "["], null, V1::respError(["InvalidInputValue", 'field' => "keeplist_rules"], 422)], + [['feed_url' => "http://example.com/", 'category_id' => 1, 'blocklist_rules' => "["], null, V1::respError(["InvalidInputValue", 'field' => "blocklist_rules"], 422)], + [['feed_url' => "http://example.com/", 'category_id' => 1], new FeedException("internalError"), V1::respError("FetchOther", 502)], + [['feed_url' => "http://example.com/", 'category_id' => 1], new FeedException("invalidCertificate"), V1::respError("FetchOther", 502)], + [['feed_url' => "http://example.com/", 'category_id' => 1], new FeedException("invalidUrl"), V1::respError("Fetch404", 502)], + [['feed_url' => "http://example.com/", 'category_id' => 1], new FeedException("maxRedirect"), V1::respError("FetchOther", 502)], + [['feed_url' => "http://example.com/", 'category_id' => 1], new FeedException("maxSize"), V1::respError("FetchOther", 502)], + [['feed_url' => "http://example.com/", 'category_id' => 1], new FeedException("timeout"), V1::respError("FetchOther", 502)], + [['feed_url' => "http://example.com/", 'category_id' => 1], new FeedException("forbidden"), V1::respError("Fetch403", 502)], + [['feed_url' => "http://example.com/", 'category_id' => 1], new FeedException("unauthorized"), V1::respError("Fetch401", 502)], + [['feed_url' => "http://example.com/", 'category_id' => 1], new FeedException("transmissionError"), V1::respError("FetchOther", 502)], + [['feed_url' => "http://example.com/", 'category_id' => 1], new FeedException("connectionFailed"), V1::respError("FetchOther", 502)], + [['feed_url' => "http://example.com/", 'category_id' => 1], new FeedException("malformedXml"), V1::respError("FetchOther", 502)], + [['feed_url' => "http://example.com/", 'category_id' => 1], new FeedException("xmlEntity"), V1::respError("FetchOther", 502)], + [['feed_url' => "http://example.com/", 'category_id' => 1], new FeedException("subscriptionNotFound"), V1::respError("Fetch404", 502)], + [['feed_url' => "http://example.com/", 'category_id' => 1], new FeedException("unsupportedFeedFormat"), V1::respError("FetchFormat", 502)], + [['feed_url' => "http://example.com/", 'category_id' => 1], new ExceptionInput("constraintViolation"), V1::respError("DuplicateFeed", 409)], + [['feed_url' => "http://example.com/", 'category_id' => 1], new ExceptionInput("idMissing"), V1::respError("MissingCategory", 422)], + [['feed_url' => "http://example.com/", 'category_id' => 1], 44, HTTP::respJson(['feed_id' => 44], 201)], + [['feed_url' => "http://example.com/", 'category_id' => 1, 'crawler' => true], 44, HTTP::respJson(['feed_id' => 44], 201)], + [['feed_url' => "http://example.com/", 'category_id' => 1, 'keeplist_rules' => "^A"], 44, HTTP::respJson(['feed_id' => 44], 201)], + [['feed_url' => "http://example.com/", 'category_id' => 1, 'blocklist_rules' => "A"], 44, HTTP::respJson(['feed_id' => 44], 201)], ]; }