From 19abce85c34dc72185f51d51f2794e136088e56a Mon Sep 17 00:00:00 2001 From: "J. King" Date: Sun, 2 Apr 2017 12:14:15 -0400 Subject: [PATCH] Implement NCN API v1-2 folder creation - Fixes #4 - Tests for failure modes still required --- lib/REST/NextCloudNews/V1_2.php | 65 +++++++++++++++++++----- lib/REST/Request.php | 18 ++++--- locale/en.php | 1 + tests/REST/NextCloudNews/TestNCNV1_2.php | 23 +++++++++ tests/lib/Result.php | 2 +- 5 files changed, 89 insertions(+), 20 deletions(-) diff --git a/lib/REST/NextCloudNews/V1_2.php b/lib/REST/NextCloudNews/V1_2.php index 76675bad..d50c1e05 100644 --- a/lib/REST/NextCloudNews/V1_2.php +++ b/lib/REST/NextCloudNews/V1_2.php @@ -3,6 +3,7 @@ declare(strict_types=1); namespace JKingWeb\Arsse\REST\NextCloudNews; use JKingWeb\Arsse\Data; use JKingWeb\Arsse\REST\Response; +use JKingWeb\Arsse\AbstractException; class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler { function __construct() { @@ -10,22 +11,44 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler { function dispatch(\JKingWeb\Arsse\REST\Request $req): \JKingWeb\Arsse\REST\Response { // try to authenticate - if(!Data::$user->authHTTP()) return new Response(401, "", null, ['WWW-Authenticate: Basic realm="NextCloud News API"']); + 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); + 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 - switch($match[1]) { - case "folders": - switch($req->method) { - case "GET": return $this->folderList(); - case "POST": return $this->folderAdd($this->normalizeInput($req)); - case "PUT": - list($path, $scope, $id, $action) = $match; - return $this->folderEdit($id, $action, $this->normalizeInput($req)); - } - default: return new Response(404); + 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); @@ -36,4 +59,22 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler { $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]]); + } } \ No newline at end of file diff --git a/lib/REST/Request.php b/lib/REST/Request.php index 12e6dd79..a8848cdb 100644 --- a/lib/REST/Request.php +++ b/lib/REST/Request.php @@ -8,18 +8,22 @@ class Request { public $path =""; public $query = ""; public $type =""; - public $stream = "php://input"; + public $body = ""; - function __construct(string $method = null, string $url = null, string $bodyStream = null, string $contentType = null) { - if(is_null($method)) $method = $_SERVER['REQUEST_METHOD']; - if(is_null($url)) $url = $_SERVER['REQUEST_URI']; - if(is_null($bodyStream)) $bodyStream = "php://input"; + function __construct(string $method = null, string $url = null, string $body = null, string $contentType = null) { + if(is_null($method)) $method = $_SERVER['REQUEST_METHOD']; + if(is_null($url)) $url = $_SERVER['REQUEST_URI']; + if(is_null($body)) $body = file_get_contents("php://input"); if(is_null($contentType)) { - if(isset($_SERVER['HTTP_CONTENT_TYPE'])) $contentType = $_SERVER['HTTP_CONTENT_TYPE']; + if(isset($_SERVER['HTTP_CONTENT_TYPE'])) { + $contentType = $_SERVER['HTTP_CONTENT_TYPE']; + } else { + $contentType = ""; + } } $this->method = strtoupper($method); $this->url = $url; - $this->stream = $bodyStream; + $this->body = $body; $this->type = $contentType; $this->refreshURL(); } diff --git a/locale/en.php b/locale/en.php index 4f905dae..f1779086 100644 --- a/locale/en.php +++ b/locale/en.php @@ -9,6 +9,7 @@ return [ 'HTTP.Status.401' => 'Unauthorized', 'HTTP.Status.404' => 'Not Found', 'HTTP.Status.405' => 'Method Not Allowed', + 'HTTP.Status.415' => 'Unsupported Media Type', // this should only be encountered in testing (because tests should cover all exceptions!) 'Exception.JKingWeb/Arsse/Exception.uncoded' => 'The specified exception symbol {0} has no code specified in AbstractException.php', diff --git a/tests/REST/NextCloudNews/TestNCNV1_2.php b/tests/REST/NextCloudNews/TestNCNV1_2.php index 180dfefb..5c667102 100644 --- a/tests/REST/NextCloudNews/TestNCNV1_2.php +++ b/tests/REST/NextCloudNews/TestNCNV1_2.php @@ -36,4 +36,27 @@ class TestNCNV1_2 extends \PHPUnit\Framework\TestCase { $exp = new Response(200, ['folders' => $list]); $this->assertEquals($exp, $this->h->dispatch(new Request("GET", "/folders"))); } + + function testAddAFolder() { + $in = [ + ["name" => "Software"], + ["name" => "Hardware"], + ]; + $out = [ + ['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); + Phake::when(Data::$db)->folderPropertiesGet(Data::$user->id, 1)->thenReturn($out[0]); + Phake::when(Data::$db)->folderPropertiesGet(Data::$user->id, 2)->thenReturn($out[1]); + $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]]]); + $this->assertEquals($exp, $this->h->dispatch(new Request("POST", "/folders?name=Hardware"))); + Phake::verify(Data::$db)->folderAdd(Data::$user->id, $in[0]); + 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); + } } \ No newline at end of file diff --git a/tests/lib/Result.php b/tests/lib/Result.php index e7d43e82..fb857015 100644 --- a/tests/lib/Result.php +++ b/tests/lib/Result.php @@ -68,6 +68,6 @@ class Result implements \JKingWeb\Arsse\Db\Result { } public function rewind() { - rewind($this->set); + reset($this->set); } } \ No newline at end of file