From f9a99a0b2b794ef51c9f3db8f4de522f504379e4 Mon Sep 17 00:00:00 2001 From: "Bruno P. Kinoshita" Date: Tue, 4 Jan 2022 19:31:52 +1300 Subject: [PATCH 1/3] Add test that fails only for zero (0) as reported in the issue --- tests/RestControllerTest.php | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/tests/RestControllerTest.php b/tests/RestControllerTest.php index 44f63b300..057ee0cc4 100644 --- a/tests/RestControllerTest.php +++ b/tests/RestControllerTest.php @@ -78,6 +78,41 @@ public function testSearchJsonLdWithAdditionalFields() { $this->assertJsonStringEqualsJsonString('{"@context":{"skos":"http:\/\/www.w3.org\/2004\/02\/skos\/core#","isothes":"http:\/\/purl.org\/iso25964\/skos-thes#","onki":"http:\/\/schema.onki.fi\/onki#","uri":"@id","type":"@type","results":{"@id":"onki:results","@container":"@list"},"prefLabel":"skos:prefLabel","altLabel":"skos:altLabel","hiddenLabel":"skos:hiddenLabel","broader":"skos:broader","relatedMatch":"skos:relatedMatch"},"uri":"","results":[{"uri":"http:\/\/www.skosmos.skos\/test\/ta117","type":["skos:Concept","meta:TestClass"],"broader":[{"uri":"http:\/\/www.skosmos.skos\/test\/ta1"}],"relatedMatch":[{"uri":"http:\/\/www.skosmos.skos\/test\/ta115"}],"prefLabel":"3D Bass","lang":"en","vocab":"test"},{"uri":"http:\/\/www.skosmos.skos\/test\/ta116","type":["skos:Concept","meta:TestClass"],"broader":[{"uri":"http:\/\/www.skosmos.skos\/test\/ta1"}],"prefLabel":"Bass","lang":"en","vocab":"test"},{"uri":"http:\/\/www.skosmos.skos\/test\/ta122","type":["skos:Concept","meta:TestClass"],"broader":[{"uri":"http:\/\/www.skosmos.skos\/test\/ta116"}],"prefLabel":"Black sea bass","lang":"en","vocab":"test"}]}', $out); } + /** + * Data provider for testSearchWithZero test. + */ + public function testSearchWithZeroData(): array { + return [ + ['0', true], + ['1', true], + ['-1', true], + ['skosmos', true], + ['', false], + [' ', false] + ]; + } + + /** + * @covers RestController::search + * @dataProvider testSearchWithZeroData + * @param $query string the search query + * @param $isSuccess bool whether the request must succeed or fail + */ + public function testSearchWithZero(string $query, bool $isSuccess) { + $request = new Request($this->model); + $request->setQueryParam('format', 'application/json'); + $request->setQueryParam('query', $query); + $request->setQueryParam('fields', 'broader relatedMatch'); + $this->controller->search($request); + + $out = $this->getActualOutput(); + if ($isSuccess) { + $this->assertStringNotContainsString("400 Bad Request", $out, "The REST search call returned an unexpected 400 bad request error!"); + } else { + $this->assertStringContainsString("400 Bad Request", $out, "The REST search call DID NOT returned an expected 400 bad request error!"); + } + } + /** * @covers RestController::indexLetters */ From e3ad47e8c8d7b2edf97efd1e6a70ff1dd6e2623e Mon Sep 17 00:00:00 2001 From: "Bruno P. Kinoshita" Date: Tue, 4 Jan 2022 19:38:05 +1300 Subject: [PATCH 2/3] Fix REST search call, preventing that zero string is treated as truthy value --- controller/RestController.php | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/controller/RestController.php b/controller/RestController.php index 31b1ac114..562a3d2d6 100644 --- a/controller/RestController.php +++ b/controller/RestController.php @@ -169,27 +169,30 @@ private function transformSearchResults($request, $results, $parameters) * Performs the search function calls. And wraps the result in a json-ld object. * @param Request $request */ - public function search($request) + public function search(Request $request): void { $maxhits = $request->getQueryParam('maxhits'); $offset = $request->getQueryParam('offset'); $term = $request->getQueryParamRaw('query'); - if (!$term && (!$request->getQueryParam('group') && !$request->getQueryParam('parent'))) { - return $this->returnError(400, "Bad Request", "query parameter missing"); + if ((!isset($term) || strlen(trim($term)) === 0) && (!$request->getQueryParam('group') && !$request->getQueryParam('parent'))) { + $this->returnError(400, "Bad Request", "query parameter missing"); + return; } if ($maxhits && (!is_numeric($maxhits) || $maxhits <= 0)) { - return $this->returnError(400, "Bad Request", "maxhits parameter is invalid"); + $this->returnError(400, "Bad Request", "maxhits parameter is invalid"); + return; } if ($offset && (!is_numeric($offset) || $offset < 0)) { - return $this->returnError(400, "Bad Request", "offset parameter is invalid"); + $this->returnError(400, "Bad Request", "offset parameter is invalid"); + return; } $parameters = $this->constructSearchParameters($request); $results = $this->model->searchConcepts($parameters); $ret = $this->transformSearchResults($request, $results, $parameters); - return $this->returnJson($ret); + $this->returnJson($ret); } /** Vocabulary-specific methods **/ From aff32dd99e578895ae03bc545de9d69443623196 Mon Sep 17 00:00:00 2001 From: Osma Suominen Date: Thu, 20 Jan 2022 14:14:23 +0200 Subject: [PATCH 3/3] fix minor typo --- tests/RestControllerTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/RestControllerTest.php b/tests/RestControllerTest.php index 057ee0cc4..2bc12d4d8 100644 --- a/tests/RestControllerTest.php +++ b/tests/RestControllerTest.php @@ -109,7 +109,7 @@ public function testSearchWithZero(string $query, bool $isSuccess) { if ($isSuccess) { $this->assertStringNotContainsString("400 Bad Request", $out, "The REST search call returned an unexpected 400 bad request error!"); } else { - $this->assertStringContainsString("400 Bad Request", $out, "The REST search call DID NOT returned an expected 400 bad request error!"); + $this->assertStringContainsString("400 Bad Request", $out, "The REST search call DID NOT return an expected 400 bad request error!"); } }