From 57dc72ebc49cceb22f6a917efcd89239b5ff8eb9 Mon Sep 17 00:00:00 2001 From: xlecours Date: Fri, 24 Feb 2023 10:11:46 -0500 Subject: [PATCH 1/6] changes to Candidate class --- php/libraries/Candidate.class.inc | 20 ++++++ .../test/api/LorisApiCandidatesTest.php | 72 ++++++++++++++++++- 2 files changed, 91 insertions(+), 1 deletion(-) diff --git a/php/libraries/Candidate.class.inc b/php/libraries/Candidate.class.inc index cf987cf047b..92495eeb9d9 100644 --- a/php/libraries/Candidate.class.inc +++ b/php/libraries/Candidate.class.inc @@ -223,6 +223,7 @@ class Candidate implements \LORIS\StudyEntities\AccessibleResource, ProjectID $registrationProjectID = null ): CandID { $factory = NDB_Factory::singleton(); + $db = $factory->database(); $site = \Site::singleton($centerID); @@ -264,6 +265,25 @@ class Candidate implements \LORIS\StudyEntities\AccessibleResource, ); } + // check pscid uniqueness + $existing = $db->pselectOne( + 'SELECT + COUNT(*) + FROM candidate + WHERE PSCID = :v_pscid + GROUP BY + PSCID + ', + ['v_pscid' => $PSCID] + ); + + if ($existing >= 0 ) { + throw new \InvalidArgumentException( + "PSCID must be unique", + PSCID_NOT_UNIQUE + ); + } + // check pscid structure if (!Candidate::validatePSCID( $PSCID, diff --git a/raisinbread/test/api/LorisApiCandidatesTest.php b/raisinbread/test/api/LorisApiCandidatesTest.php index c75d4f6bd10..fa432443bcc 100644 --- a/raisinbread/test/api/LorisApiCandidatesTest.php +++ b/raisinbread/test/api/LorisApiCandidatesTest.php @@ -239,6 +239,7 @@ public function testPostCandidatesCandid(): void 'DELETE FROM user_psc_rel WHERE UserID=999990 AND CenterID <> 1' ); + // Second, try to create a valid new candidate in a site that the // user is not affiliated with. The test user is only afficilated to // Data Coordinating Center @@ -292,7 +293,8 @@ public function testPostCandidatesCandid(): void $body = $response_new->getBody(); $this->assertNotEmpty($body); - // Finally, try to create a new candidate with an invalid input + + // Try to create a new candidate with an invalid input $json_invalid = [ 'Candidate' => [ @@ -318,4 +320,72 @@ public function testPostCandidatesCandid(): void $body = $response_invalid->getBody(); $this->assertNotEmpty($body); } + + /** + * Tests the HTTP POST request for the endpoint /candidates/{candid} + * with user generated pscid checking for uniqueness. + * + * @return void + */ + public function testPostCandidatesUserGeneratedPSCID(): void + { + // First, change the PSCID config generation to "user". + $seq = [ + 'seq' => [ + 0 => [ + '#' => '', + '@' => ['type' => 'siteAbbrev'], + ], + 1 => [ + '#' => '', + '@' => [ + 'type' => 'alphanumeric', + 'length' => '4', + ], + ], + ], + ]; + $configMap = [ + [ + 'PSCID', + [ + 'generation' => 'user', + 'structure' => $seq, + ], + ], + ]; + + $configMock = $this->getMockBuilder('NDB_Config')->getMock(); + $configMock->method('getSetting') + ->will($this->returnValueMap($configMap)); + + \NDB_Factory::singleton()->setConfig($this->_configMock); + + // Then, create a valid new candidate + $json_new = [ + 'Candidate' => + [ + 'Project' => "Rye", + 'Site' => "Data Coordinating Center", + 'EDC' => "2020-01-03", + 'DoB' => "2020-01-03", + 'Sex' => "Male" + ] + ]; + $response_new = $this->client->request( + 'POST', + "candidates", + [ + 'headers' => $this->headers, + 'json' => $json_new + ] + ); + // Verify the status code + $this->assertEquals(201, $response_new->getStatusCode()); + // Verify the endpoint has a body + $body = $response_new->getBody(); + $this->assertNotEmpty($body); + + + } } From 07c04235e260239071472c9c43be28a68cc6e69e Mon Sep 17 00:00:00 2001 From: xlecours Date: Fri, 24 Feb 2023 13:47:01 -0500 Subject: [PATCH 2/6] catching missing pscid exception finishing tests --- .../api/php/endpoints/candidates.class.inc | 2 +- .../test/api/LorisApiCandidatesTest.php | 76 +++++++++++++++---- 2 files changed, 61 insertions(+), 17 deletions(-) diff --git a/modules/api/php/endpoints/candidates.class.inc b/modules/api/php/endpoints/candidates.class.inc index 64ae7d6c7e2..3482d50177a 100644 --- a/modules/api/php/endpoints/candidates.class.inc +++ b/modules/api/php/endpoints/candidates.class.inc @@ -241,7 +241,7 @@ class Candidates extends Endpoint implements \LORIS\Middleware\ETagCalculator $pscid, $project->getId() ); - } catch (\LorisException | \InvalidArgumentException $e) { + } catch (\Exception | \LorisException | \InvalidArgumentException $e) { return new \LORIS\Http\Response\JSON\BadRequest($e->getMessage()); } diff --git a/raisinbread/test/api/LorisApiCandidatesTest.php b/raisinbread/test/api/LorisApiCandidatesTest.php index fa432443bcc..87821375393 100644 --- a/raisinbread/test/api/LorisApiCandidatesTest.php +++ b/raisinbread/test/api/LorisApiCandidatesTest.php @@ -359,33 +359,77 @@ public function testPostCandidatesUserGeneratedPSCID(): void $configMock->method('getSetting') ->will($this->returnValueMap($configMap)); - \NDB_Factory::singleton()->setConfig($this->_configMock); + \NDB_Factory::singleton()->setConfig($configMock); // Then, create a valid new candidate - $json_new = [ - 'Candidate' => - [ - 'Project' => "Rye", - 'Site' => "Data Coordinating Center", - 'EDC' => "2020-01-03", - 'DoB' => "2020-01-03", - 'Sex' => "Male" - ] + $json = [ + 'Candidate' => [ + 'Project' => "Rye", + 'PSCID' => "DCC0707", + 'Site' => "Data Coordinating Center", + 'EDC' => "2020-01-03", + 'DoB' => "2020-01-03", + 'Sex' => "Male" + ] ]; - $response_new = $this->client->request( + $response = $this->client->request( 'POST', "candidates", [ 'headers' => $this->headers, - 'json' => $json_new + 'json' => $json ] ); // Verify the status code - $this->assertEquals(201, $response_new->getStatusCode()); - // Verify the endpoint has a body - $body = $response_new->getBody(); - $this->assertNotEmpty($body); + $this->assertEquals(201, $response->getStatusCode()); + // Then, check error when missing PSCID + $json = [ + 'Candidate' => [ + 'Project' => "Rye", + 'Site' => "Data Coordinating Center", + 'EDC' => "2020-01-03", + 'DoB' => "2020-01-03", + 'Sex' => "Male" + ] + ]; + $response = $this->client->request( + 'POST', + "candidates", + [ + 'headers' => $this->headers, + 'json' => $json + ] + ); + // Verify the status code + $this->assertEquals(400, $response->getStatusCode()); + // Verify the endpoint has a body + $body = json_decode((string) $response->getBody(), true); + $this->assertEquals($body['error'], 'PSCID must be specified'); + // Then, check error when existing PSCID + $json = [ + 'Candidate' => [ + 'Project' => "Rye", + 'PSCID' => "DCC0707", + 'Site' => "Data Coordinating Center", + 'EDC' => "2020-01-03", + 'DoB' => "2020-01-03", + 'Sex' => "Male" + ] + ]; + $response = $this->client->request( + 'POST', + "candidates", + [ + 'headers' => $this->headers, + 'json' => $json + ] + ); + // Verify the status code + $this->assertEquals(400, $response->getStatusCode()); + // Verify the endpoint has a body + $body = json_decode((string) $response->getBody(), true); + $this->assertEquals($body['error'], 'PSCID must be unique'); } } From a8029952dae8489c3de9cbd50f143234b47b9bf4 Mon Sep 17 00:00:00 2001 From: xlecours Date: Tue, 21 Mar 2023 12:10:04 -0400 Subject: [PATCH 3/6] removing test because of static calls --- php/libraries/Candidate.class.inc | 2 +- .../test/api/LorisApiCandidatesTest.php | 113 +----------------- 2 files changed, 2 insertions(+), 113 deletions(-) diff --git a/php/libraries/Candidate.class.inc b/php/libraries/Candidate.class.inc index 92495eeb9d9..c98c27bc9ef 100644 --- a/php/libraries/Candidate.class.inc +++ b/php/libraries/Candidate.class.inc @@ -277,7 +277,7 @@ class Candidate implements \LORIS\StudyEntities\AccessibleResource, ['v_pscid' => $PSCID] ); - if ($existing >= 0 ) { + if ($existing > 0) { throw new \InvalidArgumentException( "PSCID must be unique", PSCID_NOT_UNIQUE diff --git a/raisinbread/test/api/LorisApiCandidatesTest.php b/raisinbread/test/api/LorisApiCandidatesTest.php index 87821375393..cd4240864ea 100644 --- a/raisinbread/test/api/LorisApiCandidatesTest.php +++ b/raisinbread/test/api/LorisApiCandidatesTest.php @@ -320,116 +320,5 @@ public function testPostCandidatesCandid(): void $body = $response_invalid->getBody(); $this->assertNotEmpty($body); } - - /** - * Tests the HTTP POST request for the endpoint /candidates/{candid} - * with user generated pscid checking for uniqueness. - * - * @return void - */ - public function testPostCandidatesUserGeneratedPSCID(): void - { - // First, change the PSCID config generation to "user". - $seq = [ - 'seq' => [ - 0 => [ - '#' => '', - '@' => ['type' => 'siteAbbrev'], - ], - 1 => [ - '#' => '', - '@' => [ - 'type' => 'alphanumeric', - 'length' => '4', - ], - ], - ], - ]; - $configMap = [ - [ - 'PSCID', - [ - 'generation' => 'user', - 'structure' => $seq, - ], - ], - ]; - - $configMock = $this->getMockBuilder('NDB_Config')->getMock(); - $configMock->method('getSetting') - ->will($this->returnValueMap($configMap)); - - \NDB_Factory::singleton()->setConfig($configMock); - - // Then, create a valid new candidate - $json = [ - 'Candidate' => [ - 'Project' => "Rye", - 'PSCID' => "DCC0707", - 'Site' => "Data Coordinating Center", - 'EDC' => "2020-01-03", - 'DoB' => "2020-01-03", - 'Sex' => "Male" - ] - ]; - $response = $this->client->request( - 'POST', - "candidates", - [ - 'headers' => $this->headers, - 'json' => $json - ] - ); - // Verify the status code - $this->assertEquals(201, $response->getStatusCode()); - - // Then, check error when missing PSCID - $json = [ - 'Candidate' => [ - 'Project' => "Rye", - 'Site' => "Data Coordinating Center", - 'EDC' => "2020-01-03", - 'DoB' => "2020-01-03", - 'Sex' => "Male" - ] - ]; - $response = $this->client->request( - 'POST', - "candidates", - [ - 'headers' => $this->headers, - 'json' => $json - ] - ); - // Verify the status code - $this->assertEquals(400, $response->getStatusCode()); - // Verify the endpoint has a body - $body = json_decode((string) $response->getBody(), true); - $this->assertEquals($body['error'], 'PSCID must be specified'); - - // Then, check error when existing PSCID - $json = [ - 'Candidate' => [ - 'Project' => "Rye", - 'PSCID' => "DCC0707", - 'Site' => "Data Coordinating Center", - 'EDC' => "2020-01-03", - 'DoB' => "2020-01-03", - 'Sex' => "Male" - ] - ]; - $response = $this->client->request( - 'POST', - "candidates", - [ - 'headers' => $this->headers, - 'json' => $json - ] - ); - // Verify the status code - $this->assertEquals(400, $response->getStatusCode()); - // Verify the endpoint has a body - $body = json_decode((string) $response->getBody(), true); - $this->assertEquals($body['error'], 'PSCID must be unique'); - } } + From ec8c9753d03e31967ae90671f5e19aa4f5c117c6 Mon Sep 17 00:00:00 2001 From: xlecours Date: Tue, 21 Mar 2023 12:12:23 -0400 Subject: [PATCH 4/6] remove unwanted changes --- raisinbread/test/api/LorisApiCandidatesTest.php | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/raisinbread/test/api/LorisApiCandidatesTest.php b/raisinbread/test/api/LorisApiCandidatesTest.php index cd4240864ea..c75d4f6bd10 100644 --- a/raisinbread/test/api/LorisApiCandidatesTest.php +++ b/raisinbread/test/api/LorisApiCandidatesTest.php @@ -239,7 +239,6 @@ public function testPostCandidatesCandid(): void 'DELETE FROM user_psc_rel WHERE UserID=999990 AND CenterID <> 1' ); - // Second, try to create a valid new candidate in a site that the // user is not affiliated with. The test user is only afficilated to // Data Coordinating Center @@ -293,8 +292,7 @@ public function testPostCandidatesCandid(): void $body = $response_new->getBody(); $this->assertNotEmpty($body); - - // Try to create a new candidate with an invalid input + // Finally, try to create a new candidate with an invalid input $json_invalid = [ 'Candidate' => [ @@ -321,4 +319,3 @@ public function testPostCandidatesCandid(): void $this->assertNotEmpty($body); } } - From 00c8166c4d00097a800a0787b58d36edd6ab4038 Mon Sep 17 00:00:00 2001 From: xlecours Date: Tue, 20 Jun 2023 11:56:17 -0400 Subject: [PATCH 5/6] specificaly use Conflict Exception to return 409 --- modules/api/php/endpoints/candidates.class.inc | 8 +++++++- php/exceptions/ConflictException.class.inc | 11 +++++++++++ php/libraries/Candidate.class.inc | 2 +- 3 files changed, 19 insertions(+), 2 deletions(-) create mode 100644 php/exceptions/ConflictException.class.inc diff --git a/modules/api/php/endpoints/candidates.class.inc b/modules/api/php/endpoints/candidates.class.inc index 3482d50177a..ed56e9a7242 100644 --- a/modules/api/php/endpoints/candidates.class.inc +++ b/modules/api/php/endpoints/candidates.class.inc @@ -241,8 +241,14 @@ class Candidates extends Endpoint implements \LORIS\Middleware\ETagCalculator $pscid, $project->getId() ); + } catch (\ConflictException $e) { + return new \LORIS\Http\Response\JSON\Conflict( + $e->getMessage() + ); } catch (\Exception | \LorisException | \InvalidArgumentException $e) { - return new \LORIS\Http\Response\JSON\BadRequest($e->getMessage()); + return new \LORIS\Http\Response\JSON\BadRequest( + $e->getMessage() + ); } $candidate = \NDB_Factory::singleton()->candidate($candid); diff --git a/php/exceptions/ConflictException.class.inc b/php/exceptions/ConflictException.class.inc new file mode 100644 index 00000000000..6fd37f0fd57 --- /dev/null +++ b/php/exceptions/ConflictException.class.inc @@ -0,0 +1,11 @@ + 0) { - throw new \InvalidArgumentException( + throw new \ConflictException( "PSCID must be unique", PSCID_NOT_UNIQUE ); From d584813414e2b4041acde2c27be48717ff75df87 Mon Sep 17 00:00:00 2001 From: xlecours Date: Wed, 21 Jun 2023 08:52:41 -0400 Subject: [PATCH 6/6] phpcs --- php/exceptions/ConflictException.class.inc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/php/exceptions/ConflictException.class.inc b/php/exceptions/ConflictException.class.inc index 6fd37f0fd57..0c5ef1b0bfa 100644 --- a/php/exceptions/ConflictException.class.inc +++ b/php/exceptions/ConflictException.class.inc @@ -4,7 +4,7 @@ * * PHP Version 7 * - * @license http://www.gnu.org/licenses/gpl-3.0.txt GPLv3 + * @license http://www.gnu.org/licenses/gpl-3.0.txt GPLv3 */ class ConflictException extends LorisException {