From 16af57cf900639a57942c8904dd6ee531fed9986 Mon Sep 17 00:00:00 2001 From: "J. King" Date: Tue, 14 Mar 2023 19:58:33 -0400 Subject: [PATCH] Partially working cleanup tests --- lib/Database.php | 2 +- lib/Service.php | 4 +- tests/cases/Database/AbstractTest.php | 2 +- tests/cases/Database/SeriesCleanup.php | 75 ++++++++++----------- tests/cases/REST/NextcloudNews/TestV1_2.php | 8 +-- tests/cases/Service/TestService.php | 6 +- tests/phpunit.dist.xml | 10 +-- 7 files changed, 50 insertions(+), 57 deletions(-) diff --git a/lib/Database.php b/lib/Database.php index a2a132c6..6a06b90d 100644 --- a/lib/Database.php +++ b/lib/Database.php @@ -1461,7 +1461,7 @@ class Database { // first unmark any icons which are no longer orphaned; an icon is considered orphaned if it is not used or only used by feeds which are themselves orphaned $this->db->query("UPDATE arsse_icons set orphaned = null where id in (select distinct icon from arsse_subscriptions where icon is not null and deleted = 0)"); // next mark any newly orphaned icons with the current date and time - $this->db->query("UPDATE arsse_icons set orphaned = CURRENT_TIMESTAMP where orphaned is null and id not in (select distinct icon from arsse_subscriptions where icon is not null and delete = 0)"); + $this->db->query("UPDATE arsse_icons set orphaned = CURRENT_TIMESTAMP where orphaned is null and id not in (select distinct icon from arsse_subscriptions where icon is not null and deleted = 0)"); // finally delete icons that have been orphaned longer than the feed retention period, if a a purge threshold has been specified $out = 0; if (Arsse::$conf->purgeFeeds) { diff --git a/lib/Service.php b/lib/Service.php index 0bbbdd10..65317d61 100644 --- a/lib/Service.php +++ b/lib/Service.php @@ -89,8 +89,8 @@ class Service { } public static function cleanupPre(): bool { - // mark unsubscribed feeds as orphaned and delete orphaned feeds that are beyond their retention period - Arsse::$db->feedCleanup(); + // delete soft-deleted subscriptions that are beyond their retention period + Arsse::$db->subscriptionCleanup(); // do the same for icons Arsse::$db->iconCleanup(); // delete expired log-in sessions diff --git a/tests/cases/Database/AbstractTest.php b/tests/cases/Database/AbstractTest.php index 3a0f0f73..479ad9cd 100644 --- a/tests/cases/Database/AbstractTest.php +++ b/tests/cases/Database/AbstractTest.php @@ -23,7 +23,7 @@ abstract class AbstractTest extends \JKingWeb\Arsse\Test\AbstractTest { use SeriesLabel; use SeriesTag; use SeriesArticle; - //use SeriesCleanup; + use SeriesCleanup; /** @var \JKingWeb\Arsse\Db\Driver */ protected static $drv; diff --git a/tests/cases/Database/SeriesCleanup.php b/tests/cases/Database/SeriesCleanup.php index f8d7ae7d..d58a1053 100644 --- a/tests/cases/Database/SeriesCleanup.php +++ b/tests/cases/Database/SeriesCleanup.php @@ -62,15 +62,6 @@ trait SeriesCleanup { [3,'http://localhost:8000/Icon/SVG1',null], ], ], - 'arsse_feeds' => [ - 'columns' => ["id", "url", "title", "orphaned", "size", "icon"], - 'rows' => [ - [1,"http://example.com/1","",$daybefore,2,null], //latest two articles should be kept - [2,"http://example.com/2","",$yesterday,0,2], - [3,"http://example.com/3","",null,0,1], - [4,"http://example.com/4","",$nowish,0,null], - ], - ], 'arsse_subscriptions' => [ 'columns' => ["id", "owner", "url", "size", "icon", "deleted", "modified"], 'rows' => [ @@ -79,21 +70,31 @@ trait SeriesCleanup { [2,'john.doe@example.com',"http://example.com/1",2,null,0,$daybefore], // the other subscriptions are used for subscription cleanup [3,'jane.doe@example.com',"http://example.com/2",0, 2,1,$yesterday], + [4,'jane.doe@example.com',"http://example.com/4",0, 1,1,$nowish], ], ], 'arsse_articles' => [ - 'columns' => ["id", "subscription", "url_title_hash", "url_content_hash", "title_content_hash", "modified"], + 'columns' => ["id", "subscription", "url_title_hash", "url_content_hash", "title_content_hash", "modified", "read", "starred", "hidden", "marked"], 'rows' => [ - [1,1,"","","",$weeksago], // is the latest article, thus is kept - [2,1,"","","",$weeksago], // is the second latest article, thus is kept - [3,1,"","","",$weeksago], // is starred by one user, thus is kept - [4,1,"","","",$weeksago], // does not meet the unread threshold due to a recent mark, thus is kept - [5,1,"","","",$daysago], // does not meet the unread threshold due to age, thus is kept - [6,1,"","","",$weeksago], // does not meet the read threshold due to a recent mark, thus is kept - [7,1,"","","",$weeksago], // meets the unread threshold without marks, thus is deleted - [8,1,"","","",$weeksago], // meets the unread threshold even with marks, thus is deleted - [9,1,"","","",$weeksago], // meets the read threshold, thus is deleted + [ 1,1,"","","",$weeksago,0,0,0,null], // is the latest article, thus is kept + [ 2,1,"","","",$weeksago,0,0,0,null], // is the second latest article, thus is kept + [ 3,1,"","","",$weeksago,0,1,0,$weeksago], // is starred by the user, thus is kept + [ 4,1,"","","",$weeksago,1,0,0,$daysago], // does not meet the unread threshold due to a recent mark, thus is kept + [ 5,1,"","","",$daysago, 0,0,0,null], // does not meet the unread threshold due to age, thus is kept + [ 6,1,"","","",$weeksago,1,0,0,$nowish], // does not meet the read threshold due to a recent mark, thus is kept + [ 7,1,"","","",$weeksago,0,0,0,null], // meets the unread threshold without marks, thus is deleted + [ 8,1,"","","",$weeksago,1,0,0,$weeksago], // meets the unread threshold even with marks, thus is deleted + [ 9,1,"","","",$weeksago,1,0,0,$daysago], // meets the read threshold, thus is deleted + [1001,2,"","","",$weeksago,0,0,0,null], // is the latest article, thus is kept + [1002,2,"","","",$weeksago,0,0,0,null], // is the second latest article, thus is kept + [1003,2,"","","",$weeksago,0,0,0,null], // meets the unread threshold without marks, thus is deleted + [1004,2,"","","",$weeksago,0,0,0,null], // meets the unread threshold without marks, thus is deleted + [1005,2,"","","",$daysago, 0,0,0,null], // does not meet the unread threshold due to age, thus is kept + [1006,2,"","","",$weeksago,1,0,0,$weeksago], // meets the unread threshold even with marks, thus is deleted + [1007,2,"","","",$weeksago,0,1,1,$weeksago], // hidden overrides starred, thus is deleted + [1008,2,"","","",$weeksago,0,0,0,null], // meets the unread threshold without marks, thus is deleted + [1009,2,"","","",$weeksago,0,0,1,$daysago], // meets the read threshold because hidden is equivalent to read, thus is deleted ], ], 'arsse_editions' => [ @@ -110,19 +111,17 @@ trait SeriesCleanup { [9,9], [201,1], [102,2], - ], - ], - 'arsse_marks' => [ - 'columns' => ["article", "subscription", "read", "starred", "hidden", "modified"], - 'rows' => [ - [3,1,0,1,0,$weeksago], - [4,1,1,0,0,$daysago], - [6,1,1,0,0,$nowish], - [6,2,1,0,0,$weeksago], - [7,2,0,1,1,$weeksago], // hidden takes precedence over starred - [8,1,1,0,0,$weeksago], - [9,1,1,0,0,$daysago], - [9,2,0,0,1,$daysago], // hidden is the same as read for the purposes of cleanup + [1001,1001], + [1002,1002], + [1003,1003], + [1004,1004], + [1005,1005], + [1006,1006], + [1007,1007], + [1008,1008], + [1009,1009], + [1201,1001], + [1102,1002], ], ], ]; @@ -134,13 +133,10 @@ trait SeriesCleanup { public function testCleanUpDeletedSubscriptions(): void { Arsse::$db->subscriptionCleanup(); - $now = gmdate("Y-m-d H:i:s"); $state = $this->primeExpectations($this->data, [ - 'arsse_feeds' => ["id","orphaned"], + 'arsse_subscriptions' => ["id"], ]); - $state['arsse_feeds']['rows'][0][1] = null; - unset($state['arsse_feeds']['rows'][1]); - $state['arsse_feeds']['rows'][2][1] = $now; + unset($state['arsse_subscriptions']['rows'][2]); $this->compareExpectations(static::$drv, $state); } @@ -149,12 +145,9 @@ trait SeriesCleanup { 'purgeFeeds' => null, ]); Arsse::$db->subscriptionCleanup(); - $now = gmdate("Y-m-d H:i:s"); $state = $this->primeExpectations($this->data, [ - 'arsse_feeds' => ["id","orphaned"], + 'arsse_subscriptions' => ["id"], ]); - $state['arsse_feeds']['rows'][0][1] = null; - $state['arsse_feeds']['rows'][2][1] = $now; $this->compareExpectations(static::$drv, $state); } diff --git a/tests/cases/REST/NextcloudNews/TestV1_2.php b/tests/cases/REST/NextcloudNews/TestV1_2.php index 5a1e2e9a..74e71b05 100644 --- a/tests/cases/REST/NextcloudNews/TestV1_2.php +++ b/tests/cases/REST/NextcloudNews/TestV1_2.php @@ -858,17 +858,17 @@ class TestV1_2 extends \JKingWeb\Arsse\Test\AbstractTest { } public function testCleanUpBeforeUpdate(): void { - $this->dbMock->feedCleanup->with()->returns(true); + $this->dbMock->subscriptionCleanup->with()->returns(true); $exp = HTTP::respEmpty(204); $this->assertMessage($exp, $this->req("GET", "/cleanup/before-update")); - $this->dbMock->feedCleanup->calledWith(); + $this->dbMock->subscriptionCleanup->calledWith(); } public function testCleanUpBeforeUpdateWithoutAuthority(): void { $this->userMock->propertiesGet->returns(['admin' => false]); $exp = HTTP::respEmpty(403); $this->assertMessage($exp, $this->req("GET", "/cleanup/before-update")); - $this->dbMock->feedCleanup->never()->called(); + $this->dbMock->subscriptionCleanup->never()->called(); } public function testCleanUpAfterUpdate(): void { @@ -882,7 +882,7 @@ class TestV1_2 extends \JKingWeb\Arsse\Test\AbstractTest { $this->userMock->propertiesGet->returns(['admin' => false]); $exp = HTTP::respEmpty(403); $this->assertMessage($exp, $this->req("GET", "/cleanup/after-update")); - $this->dbMock->feedCleanup->never()->called(); + $this->dbMock->subscriptionCleanup->never()->called(); } public function testQueryTheUserStatus(): void { diff --git a/tests/cases/Service/TestService.php b/tests/cases/Service/TestService.php index 8ac4e4a1..874d00a5 100644 --- a/tests/cases/Service/TestService.php +++ b/tests/cases/Service/TestService.php @@ -45,7 +45,7 @@ class TestService extends \JKingWeb\Arsse\Test\AbstractTest { public function testPerformPreCleanup(): void { $this->assertTrue(Service::cleanupPre()); - $this->dbMock->feedCleanup->called(); + $this->dbMock->subscriptionCleanup->called(); $this->dbMock->iconCleanup->called(); $this->dbMock->sessionCleanup->called(); } @@ -69,7 +69,7 @@ class TestService extends \JKingWeb\Arsse\Test\AbstractTest { public function testRefreshFeeds(): void { // set up mock database actions $this->dbMock->metaSet->returns(true); - $this->dbMock->feedCleanup->returns(true); + $this->dbMock->subscriptionCleanup->returns(true); $this->dbMock->sessionCleanup->returns(true); $this->dbMock->articleCleanup->returns(0); $this->dbMock->feedListStale->returns([1,2,3]); @@ -82,7 +82,7 @@ class TestService extends \JKingWeb\Arsse\Test\AbstractTest { $d->queue->calledWith(1, 2, 3); $d->exec->called(); $d->clean->called(); - $this->dbMock->feedCleanup->called(); + $this->dbMock->subscriptionCleanup->called(); $this->dbMock->iconCleanup->called(); $this->dbMock->sessionCleanup->called(); $this->dbMock->articleCleanup->called(); diff --git a/tests/phpunit.dist.xml b/tests/phpunit.dist.xml index 1f2c4fef..f50257c5 100644 --- a/tests/phpunit.dist.xml +++ b/tests/phpunit.dist.xml @@ -75,7 +75,7 @@ cases/Db/SQLite3/TestCreation.php cases/Db/SQLite3/TestDriver.php cases/Db/SQLite3/TestUpdate.php - + cases/Db/SQLite3/TestDatabase.php cases/Db/SQLite3PDO/TestResult.php cases/Db/SQLite3PDO/TestStatement.php cases/Db/SQLite3PDO/TestCreation.php @@ -89,13 +89,13 @@ cases/Db/PostgreSQL/TestCreation.php cases/Db/PostgreSQL/TestDriver.php cases/Db/PostgreSQL/TestUpdate.php - + cases/Db/PostgreSQL/TestDatabase.php cases/Db/PostgreSQLPDO/TestResult.php cases/Db/PostgreSQLPDO/TestStatement.php cases/Db/PostgreSQLPDO/TestCreation.php cases/Db/PostgreSQLPDO/TestDriver.php cases/Db/PostgreSQLPDO/TestUpdate.php - + cases/Db/PostgreSQLPDO/TestDatabase.php cases/Db/MySQL/TestResult.php @@ -103,13 +103,13 @@ cases/Db/MySQL/TestCreation.php cases/Db/MySQL/TestDriver.php cases/Db/MySQL/TestUpdate.php - + cases/Db/MySQL/TestDatabase.php cases/Db/MySQLPDO/TestResult.php cases/Db/MySQLPDO/TestStatement.php cases/Db/MySQLPDO/TestCreation.php cases/Db/MySQLPDO/TestDriver.php cases/Db/MySQLPDO/TestUpdate.php - + cases/Db/MySQLPDO/TestDatabase.php cases/REST/TestREST.php