From 9cbfa378bc6409903748663a985688c2a64984df Mon Sep 17 00:00:00 2001 From: "J. King" Date: Sun, 2 Apr 2017 21:34:30 -0400 Subject: [PATCH] Implement NCN API v1-2 folder deleting/renaming - Fixes #5 - Fixes #6 - Rewrote the NCNv1 dispatcher to better handle URL edge cases --- lib/REST/NextCloudNews/V1_2.php | 192 +++++++++++++++-------- tests/REST/NextCloudNews/TestNCNV1_2.php | 98 +++++++++++- 2 files changed, 219 insertions(+), 71 deletions(-) diff --git a/lib/REST/NextCloudNews/V1_2.php b/lib/REST/NextCloudNews/V1_2.php index d50c1e05..f4f06f91 100644 --- a/lib/REST/NextCloudNews/V1_2.php +++ b/lib/REST/NextCloudNews/V1_2.php @@ -2,79 +2,133 @@ declare(strict_types=1); namespace JKingWeb\Arsse\REST\NextCloudNews; use JKingWeb\Arsse\Data; -use JKingWeb\Arsse\REST\Response; use JKingWeb\Arsse\AbstractException; +use JKingWeb\Arsse\Db\ExceptionInput; +use JKingWeb\Arsse\REST\Response; class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler { - function __construct() { - } + function __construct() { + } - function dispatch(\JKingWeb\Arsse\REST\Request $req): \JKingWeb\Arsse\REST\Response { - // try to authenticate - if(!Data::$user->authHTTP()) return new Response(401, "", "", ['WWW-Authenticate: Basic realm="NextCloud News API"']); - // only accept GET, POST, or PUT - if(!in_array($req->method, ["GET", "POST", "PUT"])) return new Response(405, "", "", ['Allow: GET, POST, PUT']); - // normalize the input - if($req->body) { - // if the entity body is not JSON according to content type, return "415 Unsupported Media Type" - if(!preg_match("<^application/json\b|^$>", $req->type)) return new Response(415, "", "", ['Accept: application/json']); - try { - $data = json_decode($req->body, true); - } catch(\Throwable $e) { - // if the body could not be parsed as JSON, return "400 Bad Request" - return new Response(400); - } - } else { - $data = []; - } - // FIXME: Do query parameters take precedence in NextCloud? Is there a conflict error when values differ? - $data = array_merge($data, $req->query); - // match the path - if(preg_match("<^/(items|folders|feeds|cleanup|version|status|user)(?:/([^/]+))?(?:/([^/]+))?(?:/([^/]+))?/?$>", $req->path, $match)) { - // dispatch - try { - switch($match[1]) { - case "folders": - switch($req->method) { - case "GET": return $this->folderList(); - case "POST": return $this->folderAdd($data); - case "PUT": - return $this->folderEdit($match[2], $data); - } - default: return new Response(404); - } - } catch(Exception $e) { - // if there was a REST exception return 400 - return new Response(400); - } catch(AbstractException $e) { - // if there was any other Arsse exception return 500 - return new Response(500); - } - } else { - return new Response(404); - } - } + function dispatch(\JKingWeb\Arsse\REST\Request $req): Response { + // try to authenticate + if(!Data::$user->authHTTP()) return new Response(401, "", "", ['WWW-Authenticate: Basic realm="NextCloud News API v1-2"']); + // only accept GET, POST, PUT, or DELETE + if(!in_array($req->method, ["GET", "POST", "PUT", "DELETE"])) return new Response(405, "", "", ['Allow: GET, POST, PUT, DELETE']); + // normalize the input + if($req->body) { + // if the entity body is not JSON according to content type, return "415 Unsupported Media Type" + if(!preg_match("<^application/json\b|^$>", $req->type)) return new Response(415, "", "", ['Accept: application/json']); + try { + $data = json_decode($req->body, true); + } catch(\Throwable $e) { + // if the body could not be parsed as JSON, return "400 Bad Request" + return new Response(400); + } + } else { + $data = []; + } + // FIXME: Do query parameters take precedence in NextCloud? Is there a conflict error when values differ? + $data = array_merge($data, $req->query); + // match the path + if(preg_match("<^/(items|folders|feeds|cleanup|version|status|user)(?:/([^/]+))?(?:/([^/]+))?(?:/([^/]+))?/?$>", $req->path, $url)) { + // clean up the path + $scope = $url[1]; + unset($url[0]); + unset($url[1]); + $url = array_filter($url); + $url = array_values($url); + // check to make sure the requested function is implemented + $func = $scope.$req->method; + if(!method_exists($this, $func)) return new Response(501); + // dispatch + try { + return $this->$func($url, $data); + } catch(Exception $e) { + // if there was a REST exception return 400 + return new Response(400); + } catch(AbstractException $e) { + // if there was any other Arsse exception return 500 + return new Response(500); + } + } else { + return new Response(404); + } + } - protected function folderList(): Response { - $folders = Data::$db->folderList(Data::$user->id, null, false)->getAll(); - return new Response(200, ['folders' => $folders]); - } + // list folders + protected function foldersGET(array $url, array $data): Response { + // if URL is more than '/folders' this is an error + if(sizeof($url)==1) return new Response(405, "", "", ['Allow: PUT, DELETE']); + if(sizeof($url) > 1) return new Response(404); + $folders = Data::$db->folderList(Data::$user->id, null, false)->getAll(); + return new Response(200, ['folders' => $folders]); + } - protected function folderAdd($data): Response { - try { - $folder = Data::$db->folderAdd(Data::$user->id, $data); - } catch(\JKingWeb\Arsse\Db\ExceptionInput $e) { - switch($e->getCode()) { - // folder already exists - case 10236: return new Response(409); - // folder name not acceptable - case 10231: - case 10232: return new Response(422); - // other errors related to input - default: return new Response(400); - } - } - $folder = Data::$db->folderPropertiesGet(Data::$user->id, $folder); - return new Response(200, ['folders' => [$folder]]); - } + // create a folder + protected function foldersPOST(array $url, array $data): Response { + // if URL is more than '/folders' this is an error + if(sizeof($url)==1) return new Response(405, "", "", ['Allow: PUT, DELETE']); + if(sizeof($url) > 1) return new Response(404); + try { + $folder = Data::$db->folderAdd(Data::$user->id, $data); + } catch(ExceptionInput $e) { + switch($e->getCode()) { + // folder already exists + case 10236: return new Response(409); + // folder name not acceptable + case 10231: + case 10232: return new Response(422); + // other errors related to input + default: return new Response(400); + } + } + $folder = Data::$db->folderPropertiesGet(Data::$user->id, $folder); + return new Response(200, ['folders' => [$folder]]); + } + + // delete a folder + protected function foldersDELETE(array $url, array $data): Response { + // if URL is more or less than '/folders/$id' this is an error + if(sizeof($url) < 1) return new Response(405, "", "", ['Allow: GET, POST']); + if(sizeof($url) > 1) return new Response(404); + // folder ID must be integer + if(strval(intval($url[0])) !== $url[0]) return new Response(404); + // perform the deletion + try { + Data::$db->folderRemove(Data::$user->id, (int) $url[0]); + } catch(ExceptionInput $e) { + // folder does not exist + return new Response(404); + } + return new Response(204); + } + + // rename a folder (also supports moving nesting folders, but this is not a feature of the API) + protected function foldersPUT(array $url, array $data): Response { + // if URL is more or less than '/folders/$id' this is an error + if(sizeof($url) < 1) return new Response(405, "", "", ['Allow: GET, POST']); + if(sizeof($url) > 1) return new Response(404); + // folder ID must be integer + if(strval(intval($url[0])) !== $url[0]) return new Response(404); + // there must be some change to be made + if(!sizeof($data)) return new Response(422); + // perform the edit + try { + Data::$db->folderPropertiesSet(Data::$user->id, (int) $url[0], $data); + } catch(ExceptionInput $e) { + switch($e->getCode()) { + // folder does not exist + case 10235: return new Response(404); + // folder already exists + case 10236: return new Response(409); + // folder name not acceptable + case 10231: + case 10232: return new Response(422); + // other errors related to input + default: return new Response(400); + } + } + return new Response(204); + } } \ No newline at end of file diff --git a/tests/REST/NextCloudNews/TestNCNV1_2.php b/tests/REST/NextCloudNews/TestNCNV1_2.php index 5c667102..e88224e1 100644 --- a/tests/REST/NextCloudNews/TestNCNV1_2.php +++ b/tests/REST/NextCloudNews/TestNCNV1_2.php @@ -27,6 +27,43 @@ class TestNCNV1_2 extends \PHPUnit\Framework\TestCase { $this->clearData(); } + function testRespondToInvalidPaths() { + $errs = [ + 404 => [ + ['GET', "/"], + ['PUT', "/"], + ['POST', "/"], + ['DELETE', "/"], + ['GET', "/folders/1/invalid"], + ['PUT', "/folders/1/invalid"], + ['POST', "/folders/1/invalid"], + ['DELETE', "/folders/1/invalid"], + ], + 405 => [ + 'GET, POST' => [ + ['PUT', "/folders"], + ['DELETE', "/folders"], + ], + 'PUT, DELETE' => [ + ['GET', "/folders/1"], + ['POST', "/folders/1"], + ], + ], + ]; + foreach($errs[404] as $req) { + $exp = new Response(404); + list($method, $path) = $req; + $this->assertEquals($exp, $this->h->dispatch(new Request($method, $path)), "$method call to $path did not return 404."); + } + foreach($errs[405] as $allow => $cases) { + $exp = new Response(405, "", "", ['Allow: '.$allow]); + foreach($cases as $req) { + list($method, $path) = $req; + $this->assertEquals($exp, $this->h->dispatch(new Request($method, $path)), "$method call to $path did not return 405."); + } + } + } + function testListFolders() { $list = [ ['id' => 1, 'name' => "Software", 'parent' => null], @@ -46,10 +83,16 @@ class TestNCNV1_2 extends \PHPUnit\Framework\TestCase { ['id' => 1, 'name' => "Software", 'parent' => null], ['id' => 2, 'name' => "Hardware", 'parent' => null], ]; - Phake::when(Data::$db)->folderAdd(Data::$user->id, $in[0])->thenReturn(1); - Phake::when(Data::$db)->folderAdd(Data::$user->id, $in[1])->thenReturn(2); + // set of various mocks for testing + Phake::when(Data::$db)->folderAdd(Data::$user->id, $in[0])->thenReturn(1)->thenThrow(new \JKingWeb\Arsse\Db\ExceptionInput("constraintViolation")); // error on the second call + Phake::when(Data::$db)->folderAdd(Data::$user->id, $in[1])->thenReturn(2)->thenThrow(new \JKingWeb\Arsse\Db\ExceptionInput("constraintViolation")); // error on the second call Phake::when(Data::$db)->folderPropertiesGet(Data::$user->id, 1)->thenReturn($out[0]); Phake::when(Data::$db)->folderPropertiesGet(Data::$user->id, 2)->thenReturn($out[1]); + // set up mocks that produce errors + Phake::when(Data::$db)->folderAdd(Data::$user->id, [])->thenThrow(new \JKingWeb\Arsse\Db\ExceptionInput("missing")); + Phake::when(Data::$db)->folderAdd(Data::$user->id, ['name' => ""])->thenThrow(new \JKingWeb\Arsse\Db\ExceptionInput("missing")); + Phake::when(Data::$db)->folderAdd(Data::$user->id, ['name' => " "])->thenThrow(new \JKingWeb\Arsse\Db\ExceptionInput("whitespace")); + // correctly add two folders, using different means $exp = new Response(200, ['folders' => [$out[0]]]); $this->assertEquals($exp, $this->h->dispatch(new Request("POST", "/folders", json_encode($in[0]), 'application/json'))); $exp = new Response(200, ['folders' => [$out[1]]]); @@ -58,5 +101,56 @@ class TestNCNV1_2 extends \PHPUnit\Framework\TestCase { Phake::verify(Data::$db)->folderAdd(Data::$user->id, $in[1]); Phake::verify(Data::$db)->folderPropertiesGet(Data::$user->id, 1); Phake::verify(Data::$db)->folderPropertiesGet(Data::$user->id, 2); + // test bad folder names + $exp = new Response(422); + $this->assertEquals($exp, $this->h->dispatch(new Request("POST", "/folders"))); + $this->assertEquals($exp, $this->h->dispatch(new Request("POST", "/folders", '{"name":""}', 'application/json'))); + $this->assertEquals($exp, $this->h->dispatch(new Request("POST", "/folders", '{"name":" "}', 'application/json'))); + // try adding the same two folders again + $exp = new Response(409); + $this->assertEquals($exp, $this->h->dispatch(new Request("POST", "/folders?name=Software"))); + $exp = new Response(409); + $this->assertEquals($exp, $this->h->dispatch(new Request("POST", "/folders", json_encode($in[1]), 'application/json'))); + } + + function testRemoveAFolder() { + Phake::when(Data::$db)->folderRemove(Data::$user->id, 1)->thenReturn(true)->thenThrow(new \JKingWeb\Arsse\Db\ExceptionInput("idMissing")); + $exp = new Response(204); + $this->assertEquals($exp, $this->h->dispatch(new Request("DELETE", "/folders/1"))); + // fail on the second invocation because it no longer exists + $exp = new Response(404); + $this->assertEquals($exp, $this->h->dispatch(new Request("DELETE", "/folders/1"))); + Phake::verify(Data::$db, Phake::times(2))->folderRemove(Data::$user->id, 1); + // use a non-integer folder ID + $exp = new Response(404); + $this->assertEquals($exp, $this->h->dispatch(new Request("DELETE", "/folders/invalid"))); + } + + function testRenameAFolder() { + $in = [ + ["name" => "Software"], + ["name" => "Software"], + ["name" => ""], + ["name" => " "], + [], + ]; + Phake::when(Data::$db)->folderPropertiesSet(Data::$user->id, 1, $in[0])->thenReturn(true); + Phake::when(Data::$db)->folderPropertiesSet(Data::$user->id, 2, $in[1])->thenThrow(new \JKingWeb\Arsse\Db\ExceptionInput("constraintViolation")); + Phake::when(Data::$db)->folderPropertiesSet(Data::$user->id, 1, $in[2])->thenThrow(new \JKingWeb\Arsse\Db\ExceptionInput("missing")); + Phake::when(Data::$db)->folderPropertiesSet(Data::$user->id, 1, $in[3])->thenThrow(new \JKingWeb\Arsse\Db\ExceptionInput("whitespace")); + Phake::when(Data::$db)->folderPropertiesSet(Data::$user->id, 1, $in[4])->thenReturn(true); // this should be stopped by the handler before the request gets to the database + Phake::when(Data::$db)->folderPropertiesSet(Data::$user->id, 3, $this->anything())->thenThrow(new \JKingWeb\Arsse\Db\ExceptionInput("idMissing")); // folder ID 3 does not exist + $exp = new Response(204); + $this->assertEquals($exp, $this->h->dispatch(new Request("PUT", "/folders/1", json_encode($in[0]), 'application/json'))); + $exp = new Response(409); + $this->assertEquals($exp, $this->h->dispatch(new Request("PUT", "/folders/2", json_encode($in[1]), 'application/json'))); + $exp = new Response(422); + $this->assertEquals($exp, $this->h->dispatch(new Request("PUT", "/folders/1", json_encode($in[2]), 'application/json'))); + $exp = new Response(422); + $this->assertEquals($exp, $this->h->dispatch(new Request("PUT", "/folders/1", json_encode($in[3]), 'application/json'))); + $exp = new Response(422); + $this->assertEquals($exp, $this->h->dispatch(new Request("PUT", "/folders/1", json_encode($in[4]), 'application/json'))); + $exp = new Response(404); + $this->assertEquals($exp, $this->h->dispatch(new Request("PUT", "/folders/3", json_encode($in[0]), 'application/json'))); } } \ No newline at end of file