From d66cf32c1f12f4ebb9f0a986d207fa229f7befdc Mon Sep 17 00:00:00 2001 From: "J. King" Date: Tue, 22 Dec 2020 16:13:12 -0500 Subject: [PATCH] Style fixes --- lib/Database.php | 149 ++++++++------------ lib/REST/Fever/API.php | 2 +- lib/REST/Miniflux/V1.php | 8 +- tests/cases/Database/SeriesArticle.php | 2 +- tests/cases/Database/SeriesLabel.php | 2 +- tests/cases/Database/SeriesSubscription.php | 2 +- tests/cases/Database/SeriesUser.php | 2 +- tests/cases/REST/Miniflux/TestV1.php | 6 +- tests/cases/REST/TinyTinyRSS/TestAPI.php | 2 +- tests/cases/User/TestUser.php | 3 +- 10 files changed, 73 insertions(+), 105 deletions(-) diff --git a/lib/Database.php b/lib/Database.php index d193b57..6663e0f 100644 --- a/lib/Database.php +++ b/lib/Database.php @@ -327,7 +327,7 @@ class Database { settype($meta['num'], "integer"); settype($meta['admin'], "integer"); return $meta; - } + } public function userPropertiesSet(string $user, array $data): bool { if (!$this->userExists($user)) { @@ -660,20 +660,27 @@ class Database { // make sure both that the prospective parent exists, and that the it is not one of its children (a circular dependence); // also make sure that a folder with the same prospective name and parent does not already exist: if the parent is null, // SQL will happily accept duplicates (null is not unique), so we must do this check ourselves - $p = $this->db->prepare( + $p = $this->db->prepareArray( "WITH RECURSIVE - target as (select ? as userid, ? as source, ? as dest, ? as new_name), - folders as (SELECT id from arsse_folders join target on owner = userid and coalesce(parent,0) = source union all select arsse_folders.id as id from arsse_folders join folders on arsse_folders.parent=folders.id) - ". - "SELECT - case when ((select dest from target) is null or exists(select id from arsse_folders join target on owner = userid and coalesce(id,0) = coalesce(dest,0))) then 1 else 0 end as extant, - case when not exists(select id from folders where id = coalesce((select dest from target),0)) then 1 else 0 end as valid, - case when not exists(select id from arsse_folders join target on coalesce(parent,0) = coalesce(dest,0) and name = coalesce((select new_name from target),(select name from arsse_folders join target on id = source))) then 1 else 0 end as available - ", - "str", - "strict int", - "int", - "str" + target as ( + SELECT ? as userid, ? as source, ? as dest, ? as new_name + ), + folders as ( + SELECT id from arsse_folders join target on owner = userid and coalesce(parent,0) = source + union all + select arsse_folders.id as id from arsse_folders join folders on arsse_folders.parent=folders.id + ) + SELECT + case when + ((select dest from target) is null or exists(select id from arsse_folders join target on owner = userid and coalesce(id,0) = coalesce(dest,0))) + then 1 else 0 end as extant, + case when + not exists(select id from folders where id = coalesce((select dest from target),0)) + then 1 else 0 end as valid, + case when + not exists(select id from arsse_folders join target on coalesce(parent,0) = coalesce(dest,0) and name = coalesce((select new_name from target),(select name from arsse_folders join target on id = source))) + then 1 else 0 end as available", + ["str", "strict int", "int", "str"] )->run($user, $id, $parent, $name)->getRow(); if (!$p['extant']) { // if the parent doesn't exist or doesn't below to the user, throw an exception @@ -757,7 +764,13 @@ class Database { left join topmost as t on t.f_id = s.folder join arsse_feeds as f on f.id = s.feed left join arsse_icons as i on i.id = f.icon - left join (select feed, count(*) as articles from arsse_articles group by feed) as article_stats on article_stats.feed = s.feed + left join ( + select + feed, + count(*) as articles + from arsse_articles + group by feed + ) as article_stats on article_stats.feed = s.feed left join ( select subscription, @@ -1042,11 +1055,9 @@ class Database { } } catch (Feed\Exception $e) { // update the database with the resultant error and the next fetch time, incrementing the error count - $this->db->prepare( + $this->db->prepareArray( "UPDATE arsse_feeds SET updated = CURRENT_TIMESTAMP, next_fetch = ?, err_count = err_count + 1, err_msg = ? WHERE id = ?", - 'datetime', - 'str', - 'int' + ['datetime', 'str', 'int'] )->run(Feed::nextFetchOnError($f['err_count']), $e->getMessage(), $feedID); if ($throwError) { throw $e; @@ -1060,38 +1071,18 @@ class Database { $qInsertEdition = $this->db->prepare("INSERT INTO arsse_editions(article) values(?)", 'int'); } if (sizeof($feed->newItems)) { - $qInsertArticle = $this->db->prepare( + $qInsertArticle = $this->db->prepareArray( "INSERT INTO arsse_articles(url,title,author,published,edited,guid,content,url_title_hash,url_content_hash,title_content_hash,feed) values(?,?,?,?,?,?,?,?,?,?,?)", - 'str', - 'str', - 'str', - 'datetime', - 'datetime', - 'str', - 'str', - 'str', - 'str', - 'str', - 'int' + ['str', 'str', 'str', 'datetime', 'datetime', 'str', 'str', 'str', 'str', 'str', 'int'] ); } if (sizeof($feed->changedItems)) { $qDeleteEnclosures = $this->db->prepare("DELETE FROM arsse_enclosures WHERE article = ?", 'int'); $qDeleteCategories = $this->db->prepare("DELETE FROM arsse_categories WHERE article = ?", 'int'); $qClearReadMarks = $this->db->prepare("UPDATE arsse_marks SET \"read\" = 0, modified = CURRENT_TIMESTAMP WHERE article = ? and \"read\" = 1", 'int'); - $qUpdateArticle = $this->db->prepare( + $qUpdateArticle = $this->db->prepareArray( "UPDATE arsse_articles SET url = ?, title = ?, author = ?, published = ?, edited = ?, modified = CURRENT_TIMESTAMP, guid = ?, content = ?, url_title_hash = ?, url_content_hash = ?, title_content_hash = ? WHERE id = ?", - 'str', - 'str', - 'str', - 'datetime', - 'datetime', - 'str', - 'str', - 'str', - 'str', - 'str', - 'int' + ['str', 'str', 'str', 'datetime', 'datetime', 'str', 'str', 'str', 'str', 'str', 'int'] ); } // determine if the feed icon needs to be updated, and update it if appropriate @@ -1159,16 +1150,9 @@ class Database { $qClearReadMarks->run($articleID); } // lastly update the feed database itself with updated information. - $this->db->prepare( + $this->db->prepareArray( "UPDATE arsse_feeds SET title = ?, source = ?, updated = CURRENT_TIMESTAMP, modified = ?, etag = ?, err_count = 0, err_msg = '', next_fetch = ?, size = ?, icon = ? WHERE id = ?", - 'str', - 'str', - 'datetime', - 'strict str', - 'datetime', - 'int', - 'int', - 'int' + ['str', 'str', 'datetime', 'strict str', 'datetime', 'int', 'int', 'int'] )->run( $feed->data->title, $feed->data->siteUrl, @@ -1205,9 +1189,9 @@ class Database { } /** Retrieves the set of filters users have applied to a given feed - * + * * Each record includes the following keys: - * + * * - "owner": The user for whom to apply the filters * - "keep": The "keep" rule; any articles which fail to match this rule are hidden * - "block": The block rule; any article which matches this rule are hidden @@ -1258,13 +1242,9 @@ class Database { [$cHashUC, $tHashUC, $vHashUC] = $this->generateIn($hashesUC, "str"); [$cHashTC, $tHashTC, $vHashTC] = $this->generateIn($hashesTC, "str"); // perform the query - return $this->db->prepare( + return $this->db->prepareArray( "SELECT id, edited, guid, url_title_hash, url_content_hash, title_content_hash FROM arsse_articles WHERE feed = ? and (guid in($cId) or url_title_hash in($cHashUT) or url_content_hash in($cHashUC) or title_content_hash in($cHashTC))", - 'int', - $tId, - $tHashUT, - $tHashUC, - $tHashTC + ['int', $tId, $tHashUT, $tHashUC, $tHashTC] )->run($feedID, $vId, $vHashUT, $vHashUC, $vHashTC); } @@ -1880,15 +1860,14 @@ class Database { if (!V::id($id)) { throw new Db\ExceptionInput("typeViolation", ["action" => $this->caller(), "field" => "article", 'type' => "int > 0"]); // @codeCoverageIgnore } - $out = $this->db->prepare( + $out = $this->db->prepareArray( "SELECT articles.article as article, max(arsse_editions.id) as edition from ( select arsse_articles.id as article FROM arsse_articles join arsse_subscriptions on arsse_subscriptions.feed = arsse_articles.feed WHERE arsse_articles.id = ? and arsse_subscriptions.owner = ? ) as articles join arsse_editions on arsse_editions.article = articles.article group by articles.article", - "int", - "str" + ["int", "str"] )->run($id, $user)->getRow(); if (!$out) { throw new Db\ExceptionInput("subjectMissing", ["action" => $this->caller(), "field" => "article", 'id' => $id]); @@ -1907,7 +1886,7 @@ class Database { if (!V::id($id)) { throw new Db\ExceptionInput("typeViolation", ["action" => $this->caller(), "field" => "edition", 'type' => "int > 0"]); // @codeCoverageIgnore } - $out = $this->db->prepare( + $out = $this->db->prepareArray( "SELECT arsse_editions.id, arsse_editions.article, edition_stats.edition as current from arsse_editions @@ -1915,8 +1894,7 @@ class Database { join arsse_subscriptions on arsse_subscriptions.feed = arsse_articles.feed join (select article, max(id) as edition from arsse_editions group by article) as edition_stats on edition_stats.article = arsse_editions.article where arsse_editions.id = ? and arsse_subscriptions.owner = ?", - "int", - "str" + ["int", "str"] )->run($id, $user)->getRow(); if (!$out) { throw new Db\ExceptionInput("subjectMissing", ["action" => $this->caller(), "field" => "edition", 'id' => $id]); @@ -1968,7 +1946,7 @@ class Database { * @param boolean $includeEmpty Whether to include (true) or supress (false) labels which have no articles assigned to them */ public function labelList(string $user, bool $includeEmpty = true): Db\Result { - return $this->db->prepare( + return $this->db->prepareArray( "SELECT * FROM ( SELECT id, @@ -1992,11 +1970,8 @@ class Database { ) as mark_stats on mark_stats.label = arsse_labels.id WHERE owner = ? ) as label_data - where articles >= ? order by name - ", - "str", - "str", - "int" + where articles >= ? order by name", + ["str", "str", "int"] )->run($user, $user, !$includeEmpty); } @@ -2036,7 +2011,7 @@ class Database { $this->labelValidateId($user, $id, $byName, false); $field = $byName ? "name" : "id"; $type = $byName ? "str" : "int"; - $out = $this->db->prepare( + $out = $this->db->prepareArray( "SELECT id, name, @@ -2057,11 +2032,8 @@ class Database { where arsse_subscriptions.owner = ? group by label ) as mark_stats on mark_stats.label = arsse_labels.id - WHERE $field = ? and owner = ? - ", - "str", - $type, - "str" + WHERE $field = ? and owner = ?", + ["str", $type, "str"] )->run($user, $id, $user)->getRow(); if (!$out) { throw new Db\ExceptionInput("subjectMissing", ["action" => __FUNCTION__, "field" => "label", 'id' => $id]); @@ -2113,6 +2085,7 @@ class Database { } try { $q = $this->articleQuery($user, $c); + $q->setOrder("id"); $out = $this->db->prepare((string) $q, $q->getTypes())->run($q->getValues())->getAll(); } catch (Db\ExceptionInput $e) { if ($e->getCode() === 10235) { @@ -2261,7 +2234,7 @@ class Database { * @param boolean $includeEmpty Whether to include (true) or supress (false) tags which have no subscriptions assigned to them */ public function tagList(string $user, bool $includeEmpty = true): Db\Result { - return $this->db->prepare( + return $this->db->prepareArray( "SELECT * FROM ( SELECT id,name,coalesce(subscriptions,0) as subscriptions @@ -2269,10 +2242,8 @@ class Database { left join (SELECT tag, sum(assigned) as subscriptions from arsse_tag_members group by tag) as tag_stats on tag_stats.tag = arsse_tags.id WHERE owner = ? ) as tag_data - where subscriptions >= ? order by name - ", - "str", - "int" + where subscriptions >= ? order by name", + ["str", "int"] )->run($user, !$includeEmpty); } @@ -2288,7 +2259,7 @@ class Database { * @param string $user The user whose tags are to be listed */ public function tagSummarize(string $user): Db\Result { - return $this->db->prepare( + return $this->db->prepareArray( "SELECT arsse_tags.id as id, arsse_tags.name as name, @@ -2296,7 +2267,7 @@ class Database { FROM arsse_tag_members join arsse_tags on arsse_tags.id = arsse_tag_members.tag WHERE arsse_tags.owner = ? and assigned = 1", - "str" + ["str"] )->run($user); } @@ -2335,15 +2306,13 @@ class Database { $this->tagValidateId($user, $id, $byName, false); $field = $byName ? "name" : "id"; $type = $byName ? "str" : "int"; - $out = $this->db->prepare( + $out = $this->db->prepareArray( "SELECT id,name,coalesce(subscriptions,0) as subscriptions FROM arsse_tags left join (SELECT tag, sum(assigned) as subscriptions from arsse_tag_members group by tag) as tag_stats on tag_stats.tag = arsse_tags.id - WHERE $field = ? and owner = ? - ", - $type, - "str" + WHERE $field = ? and owner = ?", + [$type, "str"] )->run($id, $user)->getRow(); if (!$out) { throw new Db\ExceptionInput("subjectMissing", ["action" => __FUNCTION__, "field" => "tag", 'id' => $id]); diff --git a/lib/REST/Fever/API.php b/lib/REST/Fever/API.php index 2d5fcc1..8c94a8d 100644 --- a/lib/REST/Fever/API.php +++ b/lib/REST/Fever/API.php @@ -270,7 +270,7 @@ class API extends \JKingWeb\Arsse\REST\AbstractHandler { } else { // group zero is the "Kindling" supergroup i.e. all feeds // only exclude hidden articles - $c->hidden(false); + $c->hidden(false); } break; case "feed": diff --git a/lib/REST/Miniflux/V1.php b/lib/REST/Miniflux/V1.php index 5474dc0..bf3da2e 100644 --- a/lib/REST/Miniflux/V1.php +++ b/lib/REST/Miniflux/V1.php @@ -130,7 +130,7 @@ class V1 extends \JKingWeb\Arsse\REST\AbstractHandler { return true; } } - // next check HTTP auth + // next check HTTP auth if ($req->getAttribute("authenticated", false)) { Arsse::$user->id = $req->getAttribute("authenticatedUser"); return true; @@ -318,7 +318,7 @@ class V1 extends \JKingWeb\Arsse\REST\AbstractHandler { return new ErrorResponse($msg, 500); } $out = []; - foreach($list as $url) { + foreach ($list as $url) { // TODO: This needs to be refined once PicoFeed is replaced $out[] = ['title' => "Feed", 'type' => "rss", 'url' => $url]; } @@ -345,7 +345,7 @@ class V1 extends \JKingWeb\Arsse\REST\AbstractHandler { return new ErrorResponse("404", 404); } } - + protected function getCurrentUser(): ResponseInterface { return new Response($this->listUsers([Arsse::$user->id], false)[0] ?? new \stdClass); } @@ -411,7 +411,7 @@ class V1 extends \JKingWeb\Arsse\REST\AbstractHandler { Arsse::$db->folderRemove(Arsse::$user->id, $folder); } else { // if we're deleting from the root folder, delete each child subscription individually - // otherwise we'd be deleting the entire tree + // otherwise we'd be deleting the entire tree $tr = Arsse::$db->begin(); foreach (Arsse::$db->subscriptionList(Arsse::$user->id, null, false) as $sub) { Arsse::$db->subscriptionRemove(Arsse::$user->id, $sub['id']); diff --git a/tests/cases/Database/SeriesArticle.php b/tests/cases/Database/SeriesArticle.php index 033bbfd..c444977 100644 --- a/tests/cases/Database/SeriesArticle.php +++ b/tests/cases/Database/SeriesArticle.php @@ -1147,4 +1147,4 @@ trait SeriesArticle { $state = $this->primeExpectations($this->data, $this->checkTables); $this->compareExpectations(static::$drv, $state); } -} \ No newline at end of file +} diff --git a/tests/cases/Database/SeriesLabel.php b/tests/cases/Database/SeriesLabel.php index ec82613..4a4fac6 100644 --- a/tests/cases/Database/SeriesLabel.php +++ b/tests/cases/Database/SeriesLabel.php @@ -209,7 +209,7 @@ trait SeriesLabel { [11, 20,1,0,'2017-01-01 00:00:00',0], [12, 3,0,1,'2017-01-01 00:00:00',0], [12, 4,1,1,'2017-01-01 00:00:00',0], - [4, 8,0,0,'2000-01-02 02:00:00',1] + [4, 8,0,0,'2000-01-02 02:00:00',1], ], ], 'arsse_labels' => [ diff --git a/tests/cases/Database/SeriesSubscription.php b/tests/cases/Database/SeriesSubscription.php index 0e14a7b..7fe700a 100644 --- a/tests/cases/Database/SeriesSubscription.php +++ b/tests/cases/Database/SeriesSubscription.php @@ -23,7 +23,7 @@ trait SeriesSubscription { 'rows' => [ ["jane.doe@example.com", "", 1], ["john.doe@example.com", "", 2], - ["jill.doe@example.com", "", 3] + ["jill.doe@example.com", "", 3], ], ], 'arsse_folders' => [ diff --git a/tests/cases/Database/SeriesUser.php b/tests/cases/Database/SeriesUser.php index 7ed0182..af591d4 100644 --- a/tests/cases/Database/SeriesUser.php +++ b/tests/cases/Database/SeriesUser.php @@ -157,7 +157,7 @@ trait SeriesUser { public function testSetNoMetadata(): void { $in = [ - 'num' => 2112, + 'num' => 2112, 'stylesheet' => "body {background:lightgray}", ]; $this->assertTrue(Arsse::$db->userPropertiesSet("john.doe@example.com", $in)); diff --git a/tests/cases/REST/Miniflux/TestV1.php b/tests/cases/REST/Miniflux/TestV1.php index 3778a54..918a3da 100644 --- a/tests/cases/REST/Miniflux/TestV1.php +++ b/tests/cases/REST/Miniflux/TestV1.php @@ -62,7 +62,7 @@ class TestV1 extends \JKingWeb\Arsse\Test\AbstractTest { 'extra' => [ 'custom_css' => "", ], - ] + ], ]; protected function req(string $method, string $target, $data = "", array $headers = [], ?string $user = "john.doe@example.com", bool $body = true): ResponseInterface { @@ -185,8 +185,8 @@ class TestV1 extends \JKingWeb\Arsse\Test\AbstractTest { /** @dataProvider provideUserQueries */ public function testQueryUsers(bool $admin, string $route, ResponseInterface $exp): void { $u = [ - ['num'=> 1, 'admin' => true, 'theme' => "custom", 'lang' => "fr_CA", 'tz' => "Asia/Gaza", 'sort_asc' => true, 'page_size' => 200, 'shortcuts' => false, 'reading_time' => false, 'swipe' => false, 'stylesheet' => "p {}"], - ['num'=> 2, 'admin' => false, 'theme' => null, 'lang' => null, 'tz' => null, 'sort_asc' => null, 'page_size' => null, 'shortcuts' => null, 'reading_time' => null, 'swipe' => null, 'stylesheet' => null], + ['num' => 1, 'admin' => true, 'theme' => "custom", 'lang' => "fr_CA", 'tz' => "Asia/Gaza", 'sort_asc' => true, 'page_size' => 200, 'shortcuts' => false, 'reading_time' => false, 'swipe' => false, 'stylesheet' => "p {}"], + ['num' => 2, 'admin' => false, 'theme' => null, 'lang' => null, 'tz' => null, 'sort_asc' => null, 'page_size' => null, 'shortcuts' => null, 'reading_time' => null, 'swipe' => null, 'stylesheet' => null], new ExceptionConflict("doesNotExist"), ]; $user = $admin ? "john.doe@example.com" : "jane.doe@example.com"; diff --git a/tests/cases/REST/TinyTinyRSS/TestAPI.php b/tests/cases/REST/TinyTinyRSS/TestAPI.php index abcbdd9..923380d 100644 --- a/tests/cases/REST/TinyTinyRSS/TestAPI.php +++ b/tests/cases/REST/TinyTinyRSS/TestAPI.php @@ -1716,7 +1716,7 @@ LONG_STRING; } $this->assertMessage($exp, $this->req($in)); if ($out) { - \Phake::verify(Arsse::$db)->articleList(Arsse::$user->id, $c, $fields, $order); + \Phake::verify(Arsse::$db)->articleList(Arsse::$user->id, $c, $fields, $order); } else { \Phake::verify(Arsse::$db, \Phake::times(0))->articleList; } diff --git a/tests/cases/User/TestUser.php b/tests/cases/User/TestUser.php index b7d1266..597a158 100644 --- a/tests/cases/User/TestUser.php +++ b/tests/cases/User/TestUser.php @@ -98,7 +98,6 @@ class TestUser extends \JKingWeb\Arsse\Test\AbstractTest { \Phake::verify(Arsse::$db)->userLookup(2112); } - public function testAddAUser(): void { $user = "john.doe@example.com"; $pass = "secret"; @@ -439,7 +438,7 @@ class TestUser extends \JKingWeb\Arsse\Test\AbstractTest { [['tz' => false], new ExceptionInput("invalidValue")], [['lang' => "en-ca"], ['lang' => "en-CA"]], [['lang' => null], ['lang' => null]], - [['page_size' => 0], new ExceptionInput("invalidNonZeroInteger")] + [['page_size' => 0], new ExceptionInput("invalidNonZeroInteger")], ]; }