From 8b50297e6df106655916bd661b71d5562618e4ef Mon Sep 17 00:00:00 2001
From: "J. King" <jking@jkingweb.ca>
Date: Fri, 24 Mar 2017 13:16:34 -0400
Subject: [PATCH] Complete tests for NCN version list

- Fixes #47
- Implemented an AbstractHandler class with a generic URL parser
---
 lib/REST/AbstractHandler.php                  | 33 +++++++++++++++++++
 lib/REST/NextCloudNews/Versions.php           | 15 +++++----
 .../NextCloudNews/TestNCNVersionDiscovery.php | 26 +++++++++++++--
 3 files changed, 65 insertions(+), 9 deletions(-)
 create mode 100644 lib/REST/AbstractHandler.php

diff --git a/lib/REST/AbstractHandler.php b/lib/REST/AbstractHandler.php
new file mode 100644
index 00000000..00649942
--- /dev/null
+++ b/lib/REST/AbstractHandler.php
@@ -0,0 +1,33 @@
+<?php
+declare(strict_types=1);
+namespace JKingWeb\NewsSync\REST;
+
+abstract class AbstractHandler implements Handler {
+	abstract function __construct(\JKingWeb\NewsSync\RuntimeData $data);
+	abstract function dispatch(Request $req): Response;
+
+	protected function parseURL(string $url): array {
+		// split the query string from the path
+		$parts = explode("?", $url);
+		$out = ['path' => $parts[0], 'query' => []];
+		// if there is a query string, parse it
+		if(isset($parts[1])) {
+			// split along & to get key-value pairs
+			$query = explode("&", $parts[1]);
+			for($a = 0; $a < sizeof($query); $a++) {
+				// split each pair, into no more than two parts
+				$data = explode("=", $query[$a], 2);
+				// decode the key
+				$key = rawurldecode($data[0]);
+				// decode the value if there is one
+				$value = "";
+				if(isset($data[1])) {
+					$value = rawurldecode($data[1]);
+				}
+				// add the pair to the query output, overwriting earlier values for the same key, is present
+				$out['query'][$key] = $value;
+			}
+		}
+		return $out;
+	}
+}
\ No newline at end of file
diff --git a/lib/REST/NextCloudNews/Versions.php b/lib/REST/NextCloudNews/Versions.php
index 24794161..090febc5 100644
--- a/lib/REST/NextCloudNews/Versions.php
+++ b/lib/REST/NextCloudNews/Versions.php
@@ -3,27 +3,28 @@ declare(strict_types=1);
 namespace JKingWeb\NewsSync\REST\NextCloudNews;
 use JKingWeb\NewsSync\REST\Response;
 
-class Versions implements \JKingWeb\NewsSync\REST\Handler {
+class Versions extends \JKingWeb\NewsSync\REST\AbstractHandler {
 	function __construct(\JKingWeb\NewsSync\RuntimeData $data) {
+		// runtime data is not needed; this method is deliberately empty
 	}
 
 	function dispatch(\JKingWeb\NewsSync\REST\Request $req): \JKingWeb\NewsSync\REST\Response {
-		$path = $req->url;
-		$query = "";
-		if(strpos($path, "?") !== false) {
-			list($path, $query) = explode("?", $path);
-		}
+		// parse the URL and populate $path and $query
+		extract($this->parseURL($req->url));
+		// if a method other than GET was used, this is an error
 		if($req->method != "GET") {
 			return new Response(405);
 		}
 		if(preg_match("<^/?$>",$path)) {
+			// if the request path is an empty string or just a slash, return the supported versions
 			$out = [
 				'apiLevels' => [
-					'v1-2'
+					'v1-2',
 				]
 			];
 			return new Response(200, $out);
 		} else {
+			// if the URL path was anything else, the client is probably trying a version we don't support
 			return new Response(404);
 		}
 	}
diff --git a/tests/REST/NextCloudNews/TestNCNVersionDiscovery.php b/tests/REST/NextCloudNews/TestNCNVersionDiscovery.php
index d58f5b9e..94522d98 100644
--- a/tests/REST/NextCloudNews/TestNCNVersionDiscovery.php
+++ b/tests/REST/NextCloudNews/TestNCNVersionDiscovery.php
@@ -13,10 +13,32 @@ class TestNCNVersionDiscovery extends \PHPUnit\Framework\TestCase {
 		$this->data = new Test\RuntimeData($conf);
 	}
 
-	function testVersionList() {
+	function testFetchVersionList() {
 		$exp = new Response(200, ['apiLevels' => ['v1-2']]);
-		$req = new Request("GET", "/");
 		$h = new Rest\NextCloudNews\Versions($this->data);
+		$req = new Request("GET", "/");
+		$res = $h->dispatch($req);
+		$this->assertEquals($exp, $res);
+		$req = new Request("GET", "");
+		$res = $h->dispatch($req);
+		$this->assertEquals($exp, $res);
+		$req = new Request("GET", "/?id=1827");
+		$res = $h->dispatch($req);
+		$this->assertEquals($exp, $res);
+	}
+
+	function testUseIncorrectMethod() {
+		$exp = new Response(405);
+		$h = new Rest\NextCloudNews\Versions($this->data);
+		$req = new Request("POST", "/");
+		$res = $h->dispatch($req);
+		$this->assertEquals($exp, $res);
+	}
+
+	function testUseIncorrectPath() {
+		$exp = new Response(404);
+		$h = new Rest\NextCloudNews\Versions($this->data);
+		$req = new Request("GET", "/ook");
 		$res = $h->dispatch($req);
 		$this->assertEquals($exp, $res);
 	}