Browse Source

Adapt NCN to new type converter

This has the side-effect of removing the ability to reset a feed's title by passing null explicitly. As a non-standard behaviour it was simpler to just remove it.
microsub
J. King 7 years ago
parent
commit
d45401fb8b
  1. 49
      lib/REST/AbstractHandler.php
  2. 139
      lib/REST/NextCloudNews/V1_2.php
  3. 18
      tests/REST/NextCloudNews/TestNCNV1_2.php

49
lib/REST/AbstractHandler.php

@ -32,52 +32,13 @@ abstract class AbstractHandler implements Handler {
return $data;
}
protected function NormalizeInput(array $data, array $types, string $dateFormat = null): array {
protected function normalizeInput(array $data, array $types, string $dateFormat = null, int $mode = 0): array {
$out = [];
foreach ($data as $key => $value) {
if (!isset($types[$key])) {
$out[$key] = $value;
continue;
}
if (is_null($value)) {
foreach ($types as $key => $type) {
if (isset($data[$key])) {
$out[$key] = ValueInfo::normalize($data[$key], $type | $mode, $dateFormat);
} else {
$out[$key] = null;
continue;
}
switch ($types[$key]) {
case "int":
if (valueInfo::int($value) & ValueInfo::VALID) {
$out[$key] = (int) $value;
}
break;
case "string":
if (is_bool($value)) {
$out[$key] = var_export($value, true);
} elseif (!is_scalar($value)) {
break;
} else {
$out[$key] = (string) $value;
}
break;
case "bool":
$test = filter_var($value, \FILTER_VALIDATE_BOOLEAN, \FILTER_NULL_ON_FAILURE);
if (!is_null($test)) {
$out[$key] = $test;
}
break;
case "float":
$test = filter_var($value, \FILTER_VALIDATE_FLOAT);
if ($test !== false) {
$out[$key] = $test;
}
break;
case "datetime":
$t = Date::normalize($value, $dateFormat);
if ($t) {
$out[$key] = $t;
}
break;
default:
throw new Exception("typeUnknown", $types[$key]);
}
}
return $out;

139
lib/REST/NextCloudNews/V1_2.php

@ -21,21 +21,21 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler {
protected $dateFormat = "unix";
protected $validInput = [
'name' => "string",
'url' => "string",
'folderId' => "int",
'feedTitle' => "string",
'userId' => "string",
'feedId' => "int",
'newestItemId' => "int",
'batchSize' => "int",
'offset' => "int",
'type' => "int",
'id' => "int",
'getRead' => "bool",
'oldestFirst' => "bool",
'lastModified' => "datetime",
// 'items' => "array int", // just pass these through
'name' => ValueInfo::T_STRING,
'url' => ValueInfo::T_STRING,
'folderId' => ValueInfo::T_INT,
'feedTitle' => ValueInfo::T_STRING,
'userId' => ValueInfo::T_STRING,
'feedId' => ValueInfo::T_INT,
'newestItemId' => ValueInfo::T_INT,
'batchSize' => ValueInfo::T_INT,
'offset' => ValueInfo::T_INT,
'type' => ValueInfo::T_INT,
'id' => ValueInfo::T_INT,
'getRead' => ValueInfo::T_BOOL,
'oldestFirst' => ValueInfo::T_BOOL,
'lastModified' => ValueInfo::T_DATE,
'items' => ValueInfo::T_MIXED | ValueInfo::M_ARRAY,
];
public function __construct() {
@ -61,10 +61,7 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler {
$data = [];
}
// FIXME: Do query parameters take precedence in NextCloud? Is there a conflict error when values differ?
$data = $this->normalizeInput($data, $this->validInput, "U");
$query = $this->normalizeInput($req->query, $this->validInput, "U");
$data = array_merge($data, $query);
unset($query);
$data = $this->normalizeInput(array_merge($data, $req->query), $this->validInput, "unix");
// check to make sure the requested function is implemented
try {
$func = $this->chooseCall($req->paths, $req->method);
@ -233,7 +230,7 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler {
// create a folder
protected function folderAdd(array $url, array $data): Response {
try {
$folder = Arsse::$db->folderAdd(Arsse::$user->id, $data);
$folder = Arsse::$db->folderAdd(Arsse::$user->id, ['name' => $data['name']]);
} catch (ExceptionInput $e) {
switch ($e->getCode()) {
// folder already exists
@ -263,13 +260,8 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler {
// rename a folder (also supports moving nesting folders, but this is not a feature of the API)
protected function folderRename(array $url, array $data): Response {
// there must be some change to be made
if (!sizeof($data)) {
return new Response(422);
}
// perform the edit
try {
Arsse::$db->folderPropertiesSet(Arsse::$user->id, (int) $url[1], $data);
Arsse::$db->folderPropertiesSet(Arsse::$user->id, (int) $url[1], ['name' => $data['name']]);
} catch (ExceptionInput $e) {
switch ($e->getCode()) {
// folder does not exist
@ -288,15 +280,13 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler {
// mark all articles associated with a folder as read
protected function folderMarkRead(array $url, array $data): Response {
$c = new Context;
if (isset($data['newestItemId'])) {
// if the item ID is valid (i.e. an integer), add it to the context
$c->latestEdition($data['newestItemId']);
} else {
// otherwise return an error
if (!ValueInfo::id($data['newestItemId'])) {
// if the item ID is invalid (i.e. not a positive integer), this is an error
return new Response(422);
}
// add the folder ID to the context
// build the context
$c = new Context;
$c->latestEdition((int) $data['newestItemId']);
$c->folder((int) $url[1]);
// perform the operation
try {
@ -330,10 +320,6 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler {
if (Arsse::$user->rightsGet(Arsse::$user->id)==User::RIGHTS_NONE) {
return new Response(403);
}
// perform an update of a single feed
if (!isset($data['feedId'])) {
return new Response(422);
}
try {
Arsse::$db->feedUpdate($data['feedId']);
} catch (ExceptionInput $e) {
@ -351,16 +337,10 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler {
// add a new feed
protected function subscriptionAdd(array $url, array $data): Response {
// normalize the feed URL
if (!isset($data['url'])) {
return new Response(422);
}
// normalize the folder ID, if specified
$folder = isset($data['folderId']) ? $data['folderId'] : null;
// try to add the feed
$tr = Arsse::$db->begin();
try {
$id = Arsse::$db->subscriptionAdd(Arsse::$user->id, $data['url']);
$id = Arsse::$db->subscriptionAdd(Arsse::$user->id, (string) $data['url']);
} catch (ExceptionInput $e) {
// feed already exists
return new Response(409);
@ -369,9 +349,9 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler {
return new Response(422);
}
// if a folder was specified, move the feed to the correct folder; silently ignore errors
if ($folder) {
if ($data['folderId']) {
try {
Arsse::$db->subscriptionPropertiesSet(Arsse::$user->id, $id, ['folder' => $folder]);
Arsse::$db->subscriptionPropertiesSet(Arsse::$user->id, $id, ['folder' => $data['folderId']]);
} catch (ExceptionInput $e) {
}
}
@ -416,16 +396,8 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler {
// rename a feed
protected function subscriptionRename(array $url, array $data): Response {
// normalize input
$in = [];
if (array_key_exists('feedTitle', $data)) { // we use array_key_exists because null is a valid input
$in['title'] = $data['feedTitle'];
} else {
return new Response(422);
}
// perform the renaming
try {
Arsse::$db->subscriptionPropertiesSet(Arsse::$user->id, (int) $url[1], $in);
Arsse::$db->subscriptionPropertiesSet(Arsse::$user->id, (int) $url[1], ['title' => (string) $data['feedTitle']]);
} catch (ExceptionInput $e) {
switch ($e->getCode()) {
// subscription does not exist
@ -442,16 +414,13 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler {
// move a feed to a folder
protected function subscriptionMove(array $url, array $data): Response {
// normalize input
$in = [];
if (isset($data['folderId'])) {
$in['folder'] = $data['folderId'] ? $data['folderId'] : null;
} else {
// if no folder is specified this is an error
if (!isset($data['folderId'])) {
return new Response(422);
}
// perform the move
try {
Arsse::$db->subscriptionPropertiesSet(Arsse::$user->id, (int) $url[1], $in);
Arsse::$db->subscriptionPropertiesSet(Arsse::$user->id, (int) $url[1], ['folder' => $data['folderId']]);
} catch (ExceptionInput $e) {
switch ($e->getCode()) {
case 10239: // subscription does not exist
@ -468,14 +437,13 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler {
// mark all articles associated with a subscription as read
protected function subscriptionMarkRead(array $url, array $data): Response {
$c = new Context;
if (isset($data['newestItemId'])) {
$c->latestEdition($data['newestItemId']);
} else {
// otherwise return an error
if (!ValueInfo::id($data['newestItemId'])) {
// if the item ID is invalid (i.e. not a positive integer), this is an error
return new Response(422);
}
// add the subscription ID to the context
// build the context
$c = new Context;
$c->latestEdition((int) $data['newestItemId']);
$c->subscription((int) $url[1]);
// perform the operation
try {
@ -492,17 +460,17 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler {
// set the context options supplied by the client
$c = new Context;
// set the batch size
if (isset($data['batchSize']) && $data['batchSize'] > 0) {
if ($data['batchSize'] > 0) {
$c->limit($data['batchSize']);
}
// set the order of returned items
if (isset($data['oldestFirst']) && $data['oldestFirst']) {
if ($data['oldestFirst']) {
$c->reverse(false);
} else {
$c->reverse(true);
}
// set the edition mark-off; the database uses an or-equal comparison for internal consistency, but the protocol does not, so we must adjust by one
if (isset($data['offset']) && $data['offset'] > 0) {
if ($data['offset'] > 0) {
if ($c->reverse) {
$c->latestEdition($data['offset'] - 1);
} else {
@ -510,13 +478,11 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler {
}
}
// set whether to only return unread
if (isset($data['getRead']) && !$data['getRead']) {
if (!ValueInfo::bool($data['getRead'], true)) {
$c->unread(true);
}
// if no type is specified assume 3 (All)
if (!isset($data['type'])) {
$data['type'] = 3;
}
$data['type'] = $data['type'] ?? 3;
switch ($data['type']) {
case 0: // feed
if (isset($data['id'])) {
@ -535,7 +501,7 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler {
// return all items
}
// whether to return only updated items
if (isset($data['lastModified'])) {
if ($data['lastModified']) {
$c->modifiedSince($data['lastModified']);
}
// perform the fetch
@ -555,14 +521,13 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler {
// mark all articles as read
protected function articleMarkReadAll(array $url, array $data): Response {
$c = new Context;
if (isset($data['newestItemId'])) {
// set the newest item ID as specified
$c->latestEdition($data['newestItemId']);
} else {
// otherwise return an error
if (!ValueInfo::id($data['newestItemId'])) {
// if the item ID is invalid (i.e. not a positive integer), this is an error
return new Response(422);
}
// build the context
$c = new Context;
$c->latestEdition((int) $data['newestItemId']);
// perform the operation
Arsse::$db->articleMark(Arsse::$user->id, ['read' => true], $c);
return new Response(204);
@ -604,13 +569,9 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler {
protected function articleMarkReadMulti(array $url, array $data): Response {
// determine whether to mark read or unread
$set = ($url[1]=="read");
// if the input data is not at all valid, return an error
if (!isset($data['items']) || !is_array($data['items'])) {
return new Response(422);
}
// start a transaction and loop through the items
$t = Arsse::$db->begin();
$in = array_chunk($data['items'], 50);
$in = array_chunk($data['items'] ?? [], 50);
for ($a = 0; $a < sizeof($in); $a++) {
// initialize the matching context
$c = new Context;
@ -628,13 +589,9 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler {
protected function articleMarkStarredMulti(array $url, array $data): Response {
// determine whether to mark starred or unstarred
$set = ($url[1]=="star");
// if the input data is not at all valid, return an error
if (!isset($data['items']) || !is_array($data['items'])) {
return new Response(422);
}
// start a transaction and loop through the items
$t = Arsse::$db->begin();
$in = array_chunk(array_column($data['items'], "guidHash"), 50);
$in = array_chunk(array_column($data['items'] ?? [], "guidHash"), 50);
for ($a = 0; $a < sizeof($in); $a++) {
// initialize the matching context
$c = new Context;

18
tests/REST/NextCloudNews/TestNCNV1_2.php

@ -388,6 +388,7 @@ class TestNCNV1_2 extends Test\AbstractTest {
['id' => 2, 'name' => "Hardware", 'parent' => null],
];
// set of various mocks for testing
Phake::when(Arsse::$db)->folderAdd($this->anything(), $this->anything())->thenThrow(new \Exception);
Phake::when(Arsse::$db)->folderAdd(Arsse::$user->id, $in[0])->thenReturn(1)->thenThrow(new ExceptionInput("constraintViolation")); // error on the second call
Phake::when(Arsse::$db)->folderAdd(Arsse::$user->id, $in[1])->thenReturn(2)->thenThrow(new ExceptionInput("constraintViolation")); // error on the second call
Phake::when(Arsse::$db)->folderPropertiesGet(Arsse::$user->id, 1)->thenReturn($out[0]);
@ -499,6 +500,7 @@ class TestNCNV1_2 extends Test\AbstractTest {
// set up the necessary mocks
Phake::when(Arsse::$db)->subscriptionAdd(Arsse::$user->id, "http://example.com/news.atom")->thenReturn(2112)->thenThrow(new ExceptionInput("constraintViolation")); // error on the second call
Phake::when(Arsse::$db)->subscriptionAdd(Arsse::$user->id, "http://example.org/news.atom")->thenReturn(42)->thenThrow(new ExceptionInput("constraintViolation")); // error on the second call
Phake::when(Arsse::$db)->subscriptionAdd(Arsse::$user->id, "")->thenThrow(new \JKingWeb\Arsse\Feed\Exception("", new \PicoFeed\Reader\SubscriptionNotFoundException));
Phake::when(Arsse::$db)->subscriptionPropertiesGet(Arsse::$user->id, 2112)->thenReturn($this->feeds['db'][0]);
Phake::when(Arsse::$db)->subscriptionPropertiesGet(Arsse::$user->id, 42)->thenReturn($this->feeds['db'][1]);
Phake::when(Arsse::$db)->subscriptionPropertiesGet(Arsse::$user->id, 47)->thenReturn($this->feeds['db'][2]);
@ -584,7 +586,7 @@ class TestNCNV1_2 extends Test\AbstractTest {
Phake::when(Arsse::$db)->subscriptionPropertiesSet(Arsse::$user->id, 1, $this->identicalTo(['title' => ""]))->thenThrow(new ExceptionInput("missing"));
Phake::when(Arsse::$db)->subscriptionPropertiesSet(Arsse::$user->id, 1, $this->identicalTo(['title' => false]))->thenThrow(new ExceptionInput("missing"));
Phake::when(Arsse::$db)->subscriptionPropertiesSet(Arsse::$user->id, 42, $this->anything())->thenThrow(new ExceptionInput("subjectMissing"));
$exp = new Response(204);
$exp = new Response(422);
$this->assertEquals($exp, $this->h->dispatch(new Request("PUT", "/feeds/1/rename", json_encode($in[0]), 'application/json')));
$exp = new Response(204);
$this->assertEquals($exp, $this->h->dispatch(new Request("PUT", "/feeds/1/rename", json_encode($in[1]), 'application/json')));
@ -628,7 +630,7 @@ class TestNCNV1_2 extends Test\AbstractTest {
];
Phake::when(Arsse::$db)->feedUpdate(42)->thenReturn(true);
Phake::when(Arsse::$db)->feedUpdate(2112)->thenThrow(new ExceptionInput("subjectMissing"));
Phake::when(Arsse::$db)->feedUpdate(-1)->thenThrow(new ExceptionInput("typeViolation"));
Phake::when(Arsse::$db)->feedUpdate($this->lessThan(1))->thenThrow(new ExceptionInput("typeViolation"));
$exp = new Response(204);
$this->assertEquals($exp, $this->h->dispatch(new Request("GET", "/feeds/update", json_encode($in[0]), 'application/json')));
$exp = new Response(404);
@ -788,7 +790,7 @@ class TestNCNV1_2 extends Test\AbstractTest {
Phake::when(Arsse::$db)->articleMark(Arsse::$user->id, $this->anything(), (new Context)->editions($in[1]))->thenThrow(new ExceptionInput("tooLong")); // data model function limited to 50 items for multiples
Phake::when(Arsse::$db)->articleMark(Arsse::$user->id, $this->anything(), (new Context)->articles([]))->thenThrow(new ExceptionInput("tooShort")); // data model function requires one valid integer for multiples
Phake::when(Arsse::$db)->articleMark(Arsse::$user->id, $this->anything(), (new Context)->articles($in[1]))->thenThrow(new ExceptionInput("tooLong")); // data model function limited to 50 items for multiples
$exp = new Response(422);
$exp = new Response(204);
$this->assertEquals($exp, $this->h->dispatch(new Request("PUT", "/items/read/multiple")));
$this->assertEquals($exp, $this->h->dispatch(new Request("PUT", "/items/unread/multiple")));
$this->assertEquals($exp, $this->h->dispatch(new Request("PUT", "/items/star/multiple")));
@ -797,14 +799,12 @@ class TestNCNV1_2 extends Test\AbstractTest {
$this->assertEquals($exp, $this->h->dispatch(new Request("PUT", "/items/unread/multiple", json_encode(['items' => "ook"]), 'application/json')));
$this->assertEquals($exp, $this->h->dispatch(new Request("PUT", "/items/star/multiple", json_encode(['items' => "ook"]), 'application/json')));
$this->assertEquals($exp, $this->h->dispatch(new Request("PUT", "/items/unstar/multiple", json_encode(['items' => "ook"]), 'application/json')));
$exp = new Response(204);
$this->assertEquals($exp, $this->h->dispatch(new Request("PUT", "/items/read/multiple", json_encode(['items' => []]), 'application/json')));
$this->assertEquals($exp, $this->h->dispatch(new Request("PUT", "/items/unread/multiple", json_encode(['items' => []]), 'application/json')));
$this->assertEquals($exp, $this->h->dispatch(new Request("PUT", "/items/read/multiple", json_encode(['items' => $in[0]]), 'application/json')));
$this->assertEquals($exp, $this->h->dispatch(new Request("PUT", "/items/unread/multiple", json_encode(['items' => $in[0]]), 'application/json')));
$this->assertEquals($exp, $this->h->dispatch(new Request("PUT", "/items/read/multiple", json_encode(['items' => $in[1]]), 'application/json')));
$this->assertEquals($exp, $this->h->dispatch(new Request("PUT", "/items/unread/multiple", json_encode(['items' => $in[1]]), 'application/json')));
$exp = new Response(204);
$this->assertEquals($exp, $this->h->dispatch(new Request("PUT", "/items/star/multiple", json_encode(['items' => []]), 'application/json')));
$this->assertEquals($exp, $this->h->dispatch(new Request("PUT", "/items/unstar/multiple", json_encode(['items' => []]), 'application/json')));
$this->assertEquals($exp, $this->h->dispatch(new Request("PUT", "/items/star/multiple", json_encode(['items' => $inStar[0]]), 'application/json')));
@ -812,13 +812,13 @@ class TestNCNV1_2 extends Test\AbstractTest {
$this->assertEquals($exp, $this->h->dispatch(new Request("PUT", "/items/star/multiple", json_encode(['items' => $inStar[1]]), 'application/json')));
$this->assertEquals($exp, $this->h->dispatch(new Request("PUT", "/items/unstar/multiple", json_encode(['items' => $inStar[1]]), 'application/json')));
// ensure the data model was queried appropriately for read/unread
Phake::verify(Arsse::$db)->articleMark(Arsse::$user->id, $read, (new Context)->editions([]));
Phake::verify(Arsse::$db)->articleMark(Arsse::$user->id, $read, (new Context)->editions($in[0]));
Phake::verify(Arsse::$db, Phake::times(2))->articleMark(Arsse::$user->id, $read, (new Context)->editions([]));
Phake::verify(Arsse::$db, Phake::times(2))->articleMark(Arsse::$user->id, $read, (new Context)->editions($in[0]));
Phake::verify(Arsse::$db, Phake::times(0))->articleMark(Arsse::$user->id, $read, (new Context)->editions($in[1]));
Phake::verify(Arsse::$db)->articleMark(Arsse::$user->id, $read, (new Context)->editions($in[2]));
Phake::verify(Arsse::$db)->articleMark(Arsse::$user->id, $read, (new Context)->editions($in[3]));
Phake::verify(Arsse::$db)->articleMark(Arsse::$user->id, $unread, (new Context)->editions([]));
Phake::verify(Arsse::$db)->articleMark(Arsse::$user->id, $unread, (new Context)->editions($in[0]));
Phake::verify(Arsse::$db, Phake::times(2))->articleMark(Arsse::$user->id, $unread, (new Context)->editions([]));
Phake::verify(Arsse::$db, Phake::times(2))->articleMark(Arsse::$user->id, $unread, (new Context)->editions($in[0]));
Phake::verify(Arsse::$db, Phake::times(0))->articleMark(Arsse::$user->id, $unread, (new Context)->editions($in[1]));
Phake::verify(Arsse::$db)->articleMark(Arsse::$user->id, $unread, (new Context)->editions($in[2]));
Phake::verify(Arsse::$db)->articleMark(Arsse::$user->id, $unread, (new Context)->editions($in[3]));

Loading…
Cancel
Save