From 5124f76b70e9ea9323f046b1d2c3b1a64ebe750c Mon Sep 17 00:00:00 2001 From: "J. King" Date: Sun, 13 Dec 2020 22:10:34 -0500 Subject: [PATCH] Implementcategory deletion --- lib/REST/Miniflux/V1.php | 22 ++++++++- tests/cases/REST/Miniflux/TestV1.php | 73 +++++++++++++++++----------- tests/lib/AbstractTest.php | 6 ++- 3 files changed, 70 insertions(+), 31 deletions(-) diff --git a/lib/REST/Miniflux/V1.php b/lib/REST/Miniflux/V1.php index d84a7e9b..19f0d0b9 100644 --- a/lib/REST/Miniflux/V1.php +++ b/lib/REST/Miniflux/V1.php @@ -347,7 +347,7 @@ class V1 extends \JKingWeb\Arsse\REST\AbstractHandler { } catch (ExceptionInput $e) { if ($e->getCode() === 10236) { return new ErrorResponse(["DuplicateCategory", 'title' => $title], 500); - } elseif ($e->getCode() === 10239) { + } elseif (in_array($e->getCode(), [10237, 10239])) { return new ErrorResponse("404", 404); } else { return new ErrorResponse(["InvalidCategory", 'title' => $title], 500); @@ -357,6 +357,26 @@ class V1 extends \JKingWeb\Arsse\REST\AbstractHandler { return new Response(['id' => (int) $path[1], 'title' => $title, 'user_id' => $meta['num']]); } + protected function deleteCategory(array $path, array $query, array $data): ResponseInterface { + try { + $folder = $path[1] - 1; + if ($folder !== 0) { + 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 + $tr = Arsse::$db->begin(); + foreach (Arsse::$db->subscriptionList(Arsse::$user->id, null, false) as $sub) { + Arsse::$db->subscriptionRemove(Arsse::$user->id, $sub['id']); + } + $tr->commit(); + } + } catch (ExceptionInput $e) { + return new ErrorResponse("404", 404); + } + return new EmptyResponse(204); + } + public static function tokenGenerate(string $user, string $label): string { // Miniflux produces tokens in base64url alphabet $t = str_replace(["+", "/"], ["-", "_"], base64_encode(random_bytes(self::TOKEN_LENGTH))); diff --git a/tests/cases/REST/Miniflux/TestV1.php b/tests/cases/REST/Miniflux/TestV1.php index f9bb423c..e2f1033f 100644 --- a/tests/cases/REST/Miniflux/TestV1.php +++ b/tests/cases/REST/Miniflux/TestV1.php @@ -285,42 +285,59 @@ class TestV1 extends \JKingWeb\Arsse\Test\AbstractTest { } /** @dataProvider provideCategoryUpdates */ - public function testRenameACategory(int $id, $title, ResponseInterface $exp): void { + public function testRenameACategory(int $id, $title, $out, ResponseInterface $exp): void { Arsse::$user->method("propertiesSet")->willReturn(['root_folder_name' => $title]); - if (!in_array($id, [1,2])) { - \Phake::when(Arsse::$db)->folderPropertiesSet->thenThrow(new ExceptionInput("subjectMissing")); - } elseif (!strlen((string) $title)) { - \Phake::when(Arsse::$db)->folderPropertiesSet->thenThrow(new ExceptionInput("missing")); - } elseif (!strlen(trim((string) $title))) { - \Phake::when(Arsse::$db)->folderPropertiesSet->thenThrow(new ExceptionInput("whitespace")); - } elseif ($title === "Duplicate") { - \Phake::when(Arsse::$db)->folderPropertiesSet->thenThrow(new ExceptionInput("constraintViolation")); + if (is_string($out)) { + \Phake::when(Arsse::$db)->folderPropertiesSet->thenThrow(new ExceptionInput($out)); } else { - \Phake::when(Arsse::$db)->folderPropertiesSet->thenReturn(true); - } - if ($id === 1) { - $times = (int) (is_string($title) && strlen(trim($title))); - Arsse::$user->expects($this->exactly($times))->method("propertiesSet")->with("john.doe@example.com", ['root_folder_name' => $title]); + \Phake::when(Arsse::$db)->folderPropertiesSet->thenReturn($out); } + $times = (int) ($id === 1 && is_string($title) && strlen(trim($title))); + Arsse::$user->expects($this->exactly($times))->method("propertiesSet")->with("john.doe@example.com", ['root_folder_name' => $title]); $this->assertMessage($exp, $this->req("PUT", "/categories/$id", ['title' => $title])); - if ($id !== 1 && is_string($title)) { - \Phake::verify(Arsse::$db)->folderPropertiesSet("john.doe@example.com", $id - 1, ['name' => $title]); - } + $times = (int) ($id !== 1 && is_string($title)); + \Phake::verify(Arsse::$db, \Phake::times($times))->folderPropertiesSet("john.doe@example.com", $id - 1, ['name' => $title]); } public function provideCategoryUpdates(): iterable { return [ - [3, "New", new ErrorResponse("404", 404)], - [2, "New", new Response(['id' => 2, 'title' => "New", 'user_id' => 42])], - [2, "Duplicate", new ErrorResponse(["DuplicateCategory", 'title' => "Duplicate"], 500)], - [2, "", new ErrorResponse(["InvalidCategory", 'title' => ""], 500)], - [2, " ", new ErrorResponse(["InvalidCategory", 'title' => " "], 500)], - [2, false, new ErrorResponse(["InvalidInputType", 'field' => "title", 'actual' => "boolean", 'expected' => "string"], 400)], - [1, "New", new Response(['id' => 1, 'title' => "New", 'user_id' => 42])], - [1, "Duplicate", new Response(['id' => 1, 'title' => "Duplicate", 'user_id' => 42])], // This is allowed because the name of the root folder is only a duplicate in circumstances where it is used - [1, "", new ErrorResponse(["InvalidCategory", 'title' => ""], 500)], - [1, " ", new ErrorResponse(["InvalidCategory", 'title' => " "], 500)], - [1, false, new ErrorResponse(["InvalidInputType", 'field' => "title", 'actual' => "boolean", 'expected' => "string"], 400)], + [3, "New", "subjectMissing", new ErrorResponse("404", 404)], + [2, "New", true, new Response(['id' => 2, 'title' => "New", 'user_id' => 42])], + [2, "Duplicate", "constraintViolation", new ErrorResponse(["DuplicateCategory", 'title' => "Duplicate"], 500)], + [2, "", "missing", new ErrorResponse(["InvalidCategory", 'title' => ""], 500)], + [2, " ", "whitespace", new ErrorResponse(["InvalidCategory", 'title' => " "], 500)], + [2, false, "subjectMissing", new ErrorResponse(["InvalidInputType", 'field' => "title", 'actual' => "boolean", 'expected' => "string"], 400)], + [1, "New", true, new Response(['id' => 1, 'title' => "New", 'user_id' => 42])], + [1, "Duplicate", "constraintViolation", new Response(['id' => 1, 'title' => "Duplicate", 'user_id' => 42])], // This is allowed because the name of the root folder is only a duplicate in circumstances where it is used + [1, "", "missing", new ErrorResponse(["InvalidCategory", 'title' => ""], 500)], + [1, " ", "whitespace", new ErrorResponse(["InvalidCategory", 'title' => " "], 500)], + [1, false, false, new ErrorResponse(["InvalidInputType", 'field' => "title", 'actual' => "boolean", 'expected' => "string"], 400)], ]; } + + public function testDeleteARealCategory(): void { + \Phake::when(Arsse::$db)->folderRemove->thenReturn(true)->thenThrow(new ExceptionInput("subjectMissing")); + $this->assertMessage(new EmptyResponse(204), $this->req("DELETE", "/categories/2112")); + \Phake::verify(Arsse::$db)->folderRemove("john.doe@example.com", 2111); + $this->assertMessage(new ErrorResponse("404", 404), $this->req("DELETE", "/categories/47")); + \Phake::verify(Arsse::$db)->folderRemove("john.doe@example.com", 46); + } + + public function testDeleteTheSpecialCategory(): void { + \Phake::when(Arsse::$db)->subscriptionList->thenReturn(new Result($this->v([ + ['id' => 1], + ['id' => 47], + ['id' => 2112], + ]))); + \Phake::when(Arsse::$db)->subscriptionRemove->thenReturn(true); + $this->assertMessage(new EmptyResponse(204), $this->req("DELETE", "/categories/1")); + \Phake::inOrder( + \Phake::verify(Arsse::$db)->begin(), + \Phake::verify(Arsse::$db)->subscriptionList("john.doe@example.com", null, false), + \Phake::verify(Arsse::$db)->subscriptionRemove("john.doe@example.com", 1), + \Phake::verify(Arsse::$db)->subscriptionRemove("john.doe@example.com", 47), + \Phake::verify(Arsse::$db)->subscriptionRemove("john.doe@example.com", 2112), + \Phake::verify($this->transaction)->commit() + ); + } } diff --git a/tests/lib/AbstractTest.php b/tests/lib/AbstractTest.php index ed172430..54cde189 100644 --- a/tests/lib/AbstractTest.php +++ b/tests/lib/AbstractTest.php @@ -154,7 +154,7 @@ abstract class AbstractTest extends \PHPUnit\Framework\TestCase { protected function assertMessage(MessageInterface $exp, MessageInterface $act, string $text = ''): void { if ($exp instanceof ResponseInterface) { $this->assertInstanceOf(ResponseInterface::class, $act, $text); - $this->assertEquals($exp->getStatusCode(), $act->getStatusCode(), $text); + $this->assertSame($exp->getStatusCode(), $act->getStatusCode(), $text); } elseif ($exp instanceof RequestInterface) { if ($exp instanceof ServerRequestInterface) { $this->assertInstanceOf(ServerRequestInterface::class, $act, $text); @@ -165,12 +165,14 @@ abstract class AbstractTest extends \PHPUnit\Framework\TestCase { $this->assertSame($exp->getRequestTarget(), $act->getRequestTarget(), $text); } if ($exp instanceof JsonResponse) { + $this->assertInstanceOf(JsonResponse::class, $act, $text); $this->assertEquals($exp->getPayload(), $act->getPayload(), $text); $this->assertSame($exp->getPayload(), $act->getPayload(), $text); } elseif ($exp instanceof XmlResponse) { + $this->assertInstanceOf(XmlResponse::class, $act, $text); $this->assertXmlStringEqualsXmlString((string) $exp->getBody(), (string) $act->getBody(), $text); } else { - $this->assertEquals((string) $exp->getBody(), (string) $act->getBody(), $text); + $this->assertSame((string) $exp->getBody(), (string) $act->getBody(), $text); } $this->assertEquals($exp->getHeaders(), $act->getHeaders(), $text); }