From 4b53c5e8b3569e1bb5ae0f2e0441e01a4b25b49e Mon Sep 17 00:00:00 2001 From: "J. King" Date: Sun, 7 Jan 2018 12:59:10 -0500 Subject: [PATCH] Tests and fixes for REST class; fixes #53; improves #66 --- lib/REST.php | 39 ++++++++++-------- tests/cases/REST/TestREST.php | 78 +++++++++++++++++++++++++++++++++++ 2 files changed, 100 insertions(+), 17 deletions(-) diff --git a/lib/REST.php b/lib/REST.php index 4ccb3a0..1d3fa0a 100644 --- a/lib/REST.php +++ b/lib/REST.php @@ -7,6 +7,7 @@ declare(strict_types=1); namespace JKingWeb\Arsse; use Psr\Http\Message\RequestInterface; +use Psr\Http\Message\ServerRequestInterface; use Psr\Http\Message\ResponseInterface; use Zend\Diactoros\ServerRequest; use Zend\Diactoros\ServerRequestFactory; @@ -60,25 +61,29 @@ class REST { // create a request object if not provided $req = $req ?? ServerRequestFactory::fromGlobals(); // find the API to handle - list ($api, $target, $class) = $this->apiMatch($req->getRequestTarget(), $this->apis); - // modify the request to have a stripped target - $req = $req->withRequestTarget($target); - // generate a response - $res = $this->handOffRequest($class, $req); + try { + list ($api, $target, $class) = $this->apiMatch($req->getRequestTarget(), $this->apis); + // modify the request to have a stripped target + $req = $req->withRequestTarget($target); + // fetch the correct handler + $drv = $this->getHandler($class); + // generate a response + if ($req->getMethod()=="HEAD") { + // if the request is a HEAD request, we act exactly as if it were a GET request, and simply remove the response body later + $res = $drv->dispatch($req->withMethod("GET")); + } else { + $res = $drv->dispatch($req); + } + } catch (REST\Exception501 $e) { + $res = new EmptyResponse(501); + } // modify the response so that it has all the required metadata - $res = $this->normalizeResponse($res, $req); + return $this->normalizeResponse($res, $req); } - protected function handOffRequest(string $className, ServerRequestInterface $req): ResponseInterface { + public function getHandler(string $className): REST\Handler { // instantiate the API handler - $drv = new $className(); - // perform the request and return the response - if ($req->getMethod()=="HEAD") { - // if the request is a HEAD request, we act exactly as if it were a GET request, and simply remove the response body later - return $drv->dispatch($req->withMethod("GET")); - } else { - return $drv->dispatch($req); - } + return new $className(); } public function apiMatch(string $url): array { @@ -121,12 +126,12 @@ class REST { $res = $res->withoutHeader("Content-Length"); } // if the response is to a HEAD request, the body should be omitted - if ($req->getMethod()=="HEAD") { + if ($req && $req->getMethod()=="HEAD") { $res = new EmptyResponse($res->getStatusCode(), $res->getHeaders()); } // if an Allow header field is present, normalize it if ($res->hasHeader("Allow")) { - $methods = preg_split("<\s+,\s+>", strtoupper($res->getHeaderLine())); + $methods = preg_split("<\s*,\s*>", strtoupper($res->getHeaderLine("Allow"))); // if GET is allowed, HEAD should be allowed as well if (in_array("GET", $methods) && !in_array("HEAD", $methods)) { $methods[] = "HEAD"; diff --git a/tests/cases/REST/TestREST.php b/tests/cases/REST/TestREST.php index aef736e..10aa59d 100644 --- a/tests/cases/REST/TestREST.php +++ b/tests/cases/REST/TestREST.php @@ -7,7 +7,18 @@ declare(strict_types=1); namespace JKingWeb\Arsse\TestCase\REST; use JKingWeb\Arsse\REST; +use JKingWeb\Arsse\REST\Handler; use JKingWeb\Arsse\REST\Exception501; +use JKingWeb\Arsse\REST\NextCloudNews\V1_2 as NCN; +use JKingWeb\Arsse\REST\TinyTinyRSS\API as TTRSS; +use Psr\Http\Message\RequestInterface; +use Psr\Http\Message\ResponseInterface; +use Zend\Diactoros\Request; +use Zend\Diactoros\Response; +use Zend\Diactoros\ServerRequest; +use Zend\Diactoros\Response\TextResponse; +use Zend\Diactoros\Response\EmptyResponse; +use Phake; /** @covers \JKingWeb\Arsse\REST */ class TestREST extends \JKingWeb\Arsse\Test\AbstractTest { @@ -47,4 +58,71 @@ class TestREST extends \JKingWeb\Arsse\Test\AbstractTest { [$fake, "/full/url-not", []], ]; } + + /** @dataProvider provideUnnormalizedResponses */ + public function testNormalizeHttpResponses(ResponseInterface $res, ResponseInterface $exp, RequestInterface $req = null) { + $r = new REST(); + $act = $r->normalizeResponse($res, $req); + $this->assertResponse($exp, $act); + } + + public function provideUnnormalizedResponses() { + $stream = fopen("php://memory", "w+b"); + fwrite($stream,"ook"); + return [ + [new EmptyResponse(204), new EmptyResponse(204)], + [new EmptyResponse(204, ['Allow' => "PUT"]), new EmptyResponse(204, ['Allow' => "PUT, OPTIONS"])], + [new EmptyResponse(204, ['Allow' => "PUT, OPTIONS"]), new EmptyResponse(204, ['Allow' => "PUT, OPTIONS"])], + [new EmptyResponse(204, ['Allow' => "PUT,OPTIONS"]), new EmptyResponse(204, ['Allow' => "PUT, OPTIONS"])], + [new EmptyResponse(204, ['Allow' => ["PUT", "OPTIONS"]]), new EmptyResponse(204, ['Allow' => "PUT, OPTIONS"])], + [new EmptyResponse(204, ['Allow' => ["PUT, DELETE", "OPTIONS"]]), new EmptyResponse(204, ['Allow' => "PUT, DELETE, OPTIONS"])], + [new EmptyResponse(204, ['Allow' => "HEAD,GET"]), new EmptyResponse(204, ['Allow' => "HEAD, GET, OPTIONS"])], + [new EmptyResponse(204, ['Allow' => "GET"]), new EmptyResponse(204, ['Allow' => "GET, HEAD, OPTIONS"])], + [new TextResponse("ook", 200), new TextResponse("ook", 200, ['Content-Length' => "3"])], + [new TextResponse("", 200), new TextResponse("", 200, ['Content-Length' => "0"])], + [new TextResponse("ook", 404), new TextResponse("ook", 404, ['Content-Length' => "3"])], + [new TextResponse("", 404), new TextResponse("", 404)], + [new Response($stream, 200), new Response($stream, 200, ['Content-Length' => "3"]), new Request("", "GET")], + [new Response($stream, 200), new EmptyResponse(200, ['Content-Length' => "3"]), new Request("", "HEAD")], + ]; + } + + public function testCreateHandlers() { + $r = new REST(); + foreach (REST::API_LIST as $api) { + $class = $api['class']; + $this->assertInstanceOf(Handler::class, $r->getHandler($class)); + } + } + + /** @dataProvider provideMockRequests */ + public function testDispatchRequests(ServerRequest $req, string $method, bool $called, string $class = "", string $target ="") { + $r = Phake::partialMock(REST::class); + if ($called) { + $h = Phake::mock($class); + Phake::when($r)->getHandler($class)->thenReturn($h); + Phake::when($h)->dispatch->thenReturn(new EmptyResponse(204)); + } + $out = $r->dispatch($req); + $this->assertInstanceOf(ResponseInterface::class, $out); + if ($called) { + Phake::verify($h)->dispatch(Phake::capture($in)); + $this->assertSame($method, $in->getMethod()); + $this->assertSame($target, $in->getRequestTarget()); + } else { + $this->assertSame(501, $out->getStatusCode()); + } + Phake::verify($r)->apiMatch; + Phake::verify($r)->normalizeResponse; + } + + public function provideMockRequests() { + return [ + [new ServerRequest([], [], "/index.php/apps/news/api/v1-2/feeds", "GET"), "GET", true, NCN::Class, "/feeds"], + [new ServerRequest([], [], "/index.php/apps/news/api/v1-2/feeds", "HEAD"), "GET", true, NCN::Class, "/feeds"], + [new ServerRequest([], [], "/tt-rss/api/", "POST"), "POST", true, TTRSS::Class, "/"], + [new ServerRequest([], [], "/no/such/api/", "HEAD"), "GET", false], + [new ServerRequest([], [], "/no/such/api/", "GET"), "GET", false], + ]; + } } \ No newline at end of file