Browse Source

Fix up the aadding of subscription

Tests to come.

Because a feed could be added without adding a subscription previously,
it was possible to check feeds for validity before adding subscriptions,
yielding visibly atomic operations. These new functions replicate this
functionality by adding soft-deleted subscriptions and undeleting them
once the fetch succeeds.
redup
J. King 2 years ago
parent
commit
9f784251e8
  1. 128
      lib/Database.php
  2. 137
      tests/cases/Database/SeriesSubscription.php

128
lib/Database.php

@ -769,19 +769,95 @@ class Database {
}
/** Adds a subscription to a newsfeed, and returns the numeric identifier of the added subscription
*
* This is an all-in-one operation which reserves an ID, sets the subscription's
* properties, and based on whether the feed is fetched successfully, either makes
* the subscription available to the user, or deletes the reservation.
*
* @param string $user The user which will own the subscription
* @param string $user The user who will own the subscription
* @param string $url The URL of the newsfeed or discovery source
* @param string $fetchUser The user name required to access the newsfeed, if applicable
* @param string $fetchPassword The password required to fetch the newsfeed, if applicable; this will be stored in cleartext
* @param boolean $discover Whether to perform newsfeed discovery if $url points to an HTML document
* @param boolean $scrape Whether the initial synchronization should scrape full-article content
* @param array $properties An associative array of properties accepted by the `subscriptionPropertiesSet` function
*/
public function subscriptionAdd(string $user, string $url, string $fetchUser = "", string $fetchPassword = "", bool $discover = true, bool $scrape = false): int {
// get the ID of the underlying feed, or add it if it's not yet in the database
$feedID = $this->feedAdd($url, $fetchUser, $fetchPassword, $discover, $scrape);
// Add the feed to the user's subscriptions and return the new subscription's ID.
return $this->db->prepare('INSERT INTO arsse_subscriptions(owner,feed) values(?,?)', 'str', 'int')->run($user, $feedID)->lastId();
public function subscriptionAdd(string $user, string $url, string $fetchUser = "", string $fetchPassword = "", bool $discover = true, array $properties = []): int {
$id = $this->subscriptionReserve($user, $url, $fetchUser, $fetchPassword, $discover);
try {
if ($properties) {
$this->subscriptionPropertiesSet($user, $id, $properties);
}
$this->subscriptionUpdate($user, $id, true);
$this->subscriptionReveal($user, $id);
} catch (\Throwable $e) {
// if the process failed, the subscription should be deleted immediately rather than cluttering up the database
$this->db->prepare("DELETE from arsse_subscriptions where id = ?", "int")->run($id);
throw $e;
}
return $id;
}
/** Adds a subscription to the database without exposing it to the user, returning its ID
*
* If the subscription already exists in the database an exception is thrown, unless the
* subscription was soft-deleted; in this case the the existing ID is returned without
* clearing the delete flag
*
* This function can used with `subscriptionUpdate` and `subscriptionReveal` to simulate
* atomic addition (or rollback) of multiple newsfeeds, something which is not normally
* practical due to network retrieval and processing times
*
* @param string $user The user who will own the subscription
* @param string $url The URL of the newsfeed or discovery source
* @param string $fetchUser The user name required to access the newsfeed, if applicable
* @param string $fetchPassword The password required to fetch the newsfeed, if applicable; this will be stored in cleartext
* @param boolean $discover Whether to perform newsfeed discovery if $url points to an HTML document
*/
public function subscriptionReserve(string $user, string $url, string $fetchUser = "", string $fetchPassword = "", bool $discover = true): int {
// normalize the input URL
$url = URL::normalize($url, $fetchUser, $fetchPassword);
// if discovery is enabled, check to see if the feed already exists; this will save us some network latency if it does
if ($discover) {
$id = $this->db->prepare("SELECT id from arsse_subscriptions where ownerr = ? and url = ?", "str", "str")->run($user, $url)->getValue();
if (!$id) {
// if it doesn't exist, perform discovery
$url = Feed::discover($url);
}
}
try {
return (int) $this->db->prepare('INSERT INTO arsse_feeds(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")->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;
}
}
/** Attempts to refresh a subscribed newsfeed, returning an indication of success
*
* @param string|null $user The user whose subscribed newsfeed is to be updated; this may be null to facilitate refreshing feeds from the CLI
* @param integer $id The numerical identifier of the subscription to refresh
* @param boolean $throwError Whether to throw an exception on failure in addition to storing error information in the database
*/
public function subscriptionUpdate(?string $user, int $id, bool $throwError = false): bool {
// TODO: stub
return true;
}
/** Clears the soft-delete flag from one or more subscriptions, making them visible to the user
*
* @param string $user The user whose subscriptions to reveal
* @param int $id The numerical identifier(s) of the subscription(s) to reveal
*/
public function subscriptionReveal(string $user, int ...$id): void {
[$inClause, $inTypes, $inValues] = $this->generateIn($id, "int");
$this->db->prepare("UPDATE arsse_subscriptions set deleted = 0, modified = CURRENT_TIMESTAMP where deleted = 1 and user = ? and id in ($inClause)", "str", $inTypes)->run($user, $inValues);
}
/** Lists a user's subscriptions, returning various data
@ -997,7 +1073,7 @@ class Database {
'pinned' => "strict bool",
'keep_rule' => "str",
'block_rule' => "str",
'scrape' => "bool",
'scrape' => "strict bool",
];
[$setClause, $setTypes, $setValues] = $this->generateSet($data, $valid);
if (!$setClause) {
@ -1132,42 +1208,6 @@ class Database {
return $out;
}
/** Adds a newsfeed to the database without adding any subscriptions, and returns the numeric identifier of the added feed
*
* If the feed already exists in the database, the existing ID is returned
*
* @param string $url The URL of the newsfeed or discovery source
* @param string $fetchUser The user name required to access the newsfeed, if applicable
* @param string $fetchPassword The password required to fetch the newsfeed, if applicable; this will be stored in cleartext
* @param boolean $discover Whether to perform newsfeed discovery if $url points to an HTML document
* @param boolean $scrape Whether the initial synchronization should scrape full-article content
*/
public function feedAdd(string $url, string $fetchUser = "", string $fetchPassword = "", bool $discover = true, bool $scrape = false): int {
// normalize the input URL
$url = URL::normalize($url);
// check to see if the feed already exists
$check = $this->db->prepare("SELECT id from arsse_feeds where url = ? and username = ? and password = ?", "str", "str", "str");
$feedID = $check->run($url, $fetchUser, $fetchPassword)->getValue();
if ($discover && is_null($feedID)) {
// if the feed doesn't exist, first perform discovery if requested and check for the existence of that URL
$url = Feed::discover($url, $fetchUser, $fetchPassword);
$feedID = $check->run($url, $fetchUser, $fetchPassword)->getValue();
}
if (is_null($feedID)) {
// if the feed still doesn't exist in the database, add it to the database; we do this unconditionally so as to lock SQLite databases for as little time as possible
$feedID = $this->db->prepare('INSERT INTO arsse_feeds(url,username,password) values(?,?,?)', 'str', 'str', 'str')->run($url, $fetchUser, $fetchPassword)->lastId();
try {
// perform an initial update on the newly added feed
$this->feedUpdate($feedID, true, $scrape);
} catch (\Throwable $e) {
// if the update fails, delete the feed we just added
$this->db->prepare('DELETE from arsse_feeds where id = ?', 'int')->run($feedID);
throw $e;
}
}
return (int) $feedID;
}
/** Returns an indexed array of numeric identifiers for newsfeeds which should be refreshed */
public function feedListStale(): array {
$feeds = $this->db->query("SELECT id from arsse_feeds where next_fetch <= CURRENT_TIMESTAMP")->getAll();

137
tests/cases/Database/SeriesSubscription.php

@ -41,24 +41,15 @@ trait SeriesSubscription {
[2,"http://example.net/favicon.ico", null],
],
],
'arsse_feeds' => [
'columns' => ["id", "url", "title", "username", "password", "updated", "next_fetch", "icon"],
'rows' => [
[1,"http://example.com/feed1", "Ook", "", "",strtotime("now"),strtotime("now"),null],
[2,"http://example.com/feed2", "eek", "", "",strtotime("now - 1 hour"),strtotime("now - 1 hour"),1],
[3,"http://example.com/feed3", "Ack", "", "",strtotime("now + 1 hour"),strtotime("now + 1 hour"),2],
[4,"http://example.com/feed4", "Foo", "", "",strtotime("now + 1 hour"),strtotime("now + 1 hour"),null],
],
],
'arsse_subscriptions' => [
'columns' => ["id", "owner", "feed", "title", "folder", "pinned", "order_type", "keep_rule", "block_rule", "scrape"],
'columns' => ["id", "owner", "url", "feed_title", "updated", "next_fetch", "icon", "title", "folder", "pinned", "order_type", "keep_rule", "block_rule", "scrape"],
'rows' => [
[1,"john.doe@example.com",2,null,null,1,2,null,null,0],
[2,"jane.doe@example.com",2,null,null,0,0,null,null,0],
[3,"john.doe@example.com",3,"Ook",2,0,1,null,null,0],
[4,"jill.doe@example.com",2,null,null,0,0,null,null,0],
[5,"jack.doe@example.com",2,null,null,1,2,"","3|E",0],
[6,"john.doe@example.com",4,"Bar",3,0,0,null,null,0],
[1, "john.doe@example.com", "http://example.com/feed2", "eek", strtotime("now - 1 hour"), strtotime("now - 1 hour"), 1, null, null, 1, 2, null, null, 0],
[2, "jane.doe@example.com", "http://example.com/feed2", "eek", strtotime("now - 1 hour"), strtotime("now - 1 hour"), 1, null, null, 0, 0, null, null, 0],
[3, "john.doe@example.com", "http://example.com/feed3", "Ack", strtotime("now + 1 hour"), strtotime("now + 1 hour"), 2, "Ook", 2, 0, 1, null, null, 0],
[4, "jill.doe@example.com", "http://example.com/feed2", "eek", strtotime("now - 1 hour"), strtotime("now - 1 hour"), 1, null, null, 0, 0, null, null, 0],
[5, "jack.doe@example.com", "http://example.com/feed2", "eek", strtotime("now - 1 hour"), strtotime("now - 1 hour"), 1, null, null, 1, 2, "", "3|E", 0],
[6, "john.doe@example.com", "http://example.com/feed4", "Foo", strtotime("now + 1 hour"), strtotime("now + 1 hour"), null, "Bar", 3, 0, 0, null, null, 0],
],
],
'arsse_tags' => [
@ -81,58 +72,83 @@ trait SeriesSubscription {
],
],
'arsse_articles' => [
'columns' => ["id", "feed", "url_title_hash", "url_content_hash", "title_content_hash", "title"],
'columns' => ["id", "subscription", "url_title_hash", "url_content_hash", "title_content_hash", "title", "read", "starred", "hidden"],
'rows' => [
[1,2,"","","","Title 1"],
[2,2,"","","","Title 2"],
[3,2,"","","","Title 3"],
[4,2,"","","","Title 4"],
[5,2,"","","","Title 5"],
[6,3,"","","","Title 6"],
[7,3,"","","","Title 7"],
[8,3,"","","","Title 8"],
[1, 1, "", "", "", "Title 1", 1, 0, 0],
[2, 1, "", "", "", "Title 2", 0, 0, 0],
[3, 1, "", "", "", "Title 3", 0, 0, 0],
[4, 1, "", "", "", "Title 4", 0, 0, 0],
[5, 1, "", "", "", "Title 5", 0, 0, 0],
[6, 2, "", "", "", "Title 1", 1, 0, 0],
[7, 2, "", "", "", "Title 2", 1, 0, 0],
[8, 2, "", "", "", "Title 3", 1, 0, 0],
[9, 2, "", "", "", "Title 4", 1, 0, 0],
[10, 2, "", "", "", "Title 5", 1, 0, 0],
[11, 4, "", "", "", "Title 1", 0, 0, 0],
[12, 4, "", "", "", "Title 2", 0, 0, 0],
[13, 4, "", "", "", "Title 3", 0, 0, 0],
[14, 4, "", "", "", "Title 4", 0, 0, 0],
[15, 4, "", "", "", "Title 5", 0, 0, 0],
[16, 5, "", "", "", "Title 1", 1, 0, 0],
[17, 5, "", "", "", "Title 2", 0, 0, 0],
[18, 5, "", "", "", "Title 3", 1, 0, 1],
[19, 5, "", "", "", "Title 4", 0, 0, 0],
[20, 5, "", "", "", "Title 5", 0, 0, 1],
[21, 3, "", "", "", "Title 6", 0, 0, 0],
[22, 3, "", "", "", "Title 7", 1, 0, 0],
[23, 3, "", "", "", "Title 8", 0, 0, 0],
],
],
'arsse_editions' => [
'columns' => ["id", "article"],
'rows' => [
[1,1],
[2,2],
[3,3],
[4,4],
[5,5],
[6,6],
[7,7],
[8,8],
[1, 1],
[2, 2],
[3, 3],
[4, 4],
[5, 5],
[6, 6],
[7, 7],
[8, 8],
[9, 9],
[10, 10],
[11, 11],
[12, 12],
[13, 13],
[14, 14],
[15, 15],
[16, 16],
[17, 17],
[18, 18],
[19, 19],
[20, 20],
[21, 21],
[22, 22],
[23, 23],
],
],
'arsse_categories' => [
'columns' => ["article", "name"],
'rows' => [
[1,"A"],
[2,"B"],
[4,"D"],
[5,"E"],
[6,"F"],
[7,"G"],
[8,"H"],
],
],
'arsse_marks' => [
'columns' => ["article", "subscription", "read", "starred", "hidden"],
'rows' => [
[1,2,1,0,0],
[2,2,1,0,0],
[3,2,1,0,0],
[4,2,1,0,0],
[5,2,1,0,0],
[1,1,1,0,0],
[7,3,1,0,0],
[8,3,0,0,0],
[1,5,1,0,0],
[3,5,1,0,1],
[4,5,0,0,0],
[5,5,0,0,1],
[1, "A"],
[2, "B"],
[4, "D"],
[5, "E"],
[6, "A"],
[7, "B"],
[9, "D"],
[10, "E"],
[11, "A"],
[12, "B"],
[14, "D"],
[15, "E"],
[16, "A"],
[17, "B"],
[19, "D"],
[20, "E"],
[21, "F"],
[22, "G"],
[23, "H"],
],
],
];
@ -151,11 +167,8 @@ trait SeriesSubscription {
Arsse::$db = $db->get();
$this->assertSame($subID, Arsse::$db->subscriptionAdd($this->user, $url));
$db->feedUpdate->never()->called();
$state = $this->primeExpectations($this->data, [
'arsse_feeds' => ['id','url','username','password'],
'arsse_subscriptions' => ['id','owner','feed'],
]);
$state['arsse_subscriptions']['rows'][] = [$subID,$this->user,1];
$state = $this->primeExpectations($this->data, ['arsse_subscriptions' => ['id', 'owner', 'feed', 'url']]);
$state['arsse_subscriptions']['rows'][] = [$subID, $this->user, "http://example.com/feed1"];
$this->compareExpectations(static::$drv, $state);
}

Loading…
Cancel
Save