From dd24b0e9f363e5686306280bd071c301440b69aa Mon Sep 17 00:00:00 2001 From: fmizzell Date: Fri, 15 Nov 2019 08:38:03 -0600 Subject: [PATCH] Separating the query parameter and request body methods of interaction with the sql endpoint into more appropriate verbs. (#251) --- cypress/integration/05_sql_endpoint.spec.js | 16 +++--- .../test/src/Unit/HarvesterTest.php | 2 +- .../Unit/Storage/DatabaseTableFactoryTest.php | 5 +- .../src/Unit/Storage/DatabaseTableTest.php | 3 + .../dkan_sql_endpoint.routing.yml | 11 +++- .../dkan_sql_endpoint/src/Controller/Api.php | 55 +++++++++++-------- .../tests/src/Unit/Controller/ApiTest.php | 17 +++++- 7 files changed, 71 insertions(+), 38 deletions(-) diff --git a/cypress/integration/05_sql_endpoint.spec.js b/cypress/integration/05_sql_endpoint.spec.js index 57926c516..baabc94f3 100644 --- a/cypress/integration/05_sql_endpoint.spec.js +++ b/cypress/integration/05_sql_endpoint.spec.js @@ -108,7 +108,7 @@ context('SQL Endpoint', () => { it('All', () => { let query = `[SELECT * FROM ${resource_identifier}];` cy.request({ - method: 'GET', + method: 'POST', url: apiUri + '/datastore/sql', body: { "query": query @@ -127,7 +127,7 @@ context('SQL Endpoint', () => { it('Specific fields', () => { let query = `[SELECT lon,lat FROM ${resource_identifier}];` cy.request({ - method: 'GET', + method: 'POST', url: apiUri + '/datastore/sql', body: { "query": query @@ -152,7 +152,7 @@ context('SQL Endpoint', () => { it('Single condition', () => { let query = `[SELECT * FROM ${resource_identifier}][WHERE dist_name = 'Pusht Rod'];` cy.request({ - method: 'GET', + method: 'POST', url: apiUri + '/datastore/sql', body: { "query": query @@ -166,7 +166,7 @@ context('SQL Endpoint', () => { it('Multiple conditions', () => { let query = `[SELECT * FROM ${resource_identifier}][WHERE prov_name = 'Farah' AND dist_name = 'Pusht Rod'];` cy.request({ - method: 'GET', + method: 'POST', url: apiUri + '/datastore/sql', body: { "query": query @@ -184,7 +184,7 @@ context('SQL Endpoint', () => { it('Ascending explicit', () => { let query = `[SELECT * FROM ${resource_identifier}][ORDER BY dist_name ASC];` cy.request({ - method: 'GET', + method: 'POST', url: apiUri + '/datastore/sql', body: { "query": query @@ -199,7 +199,7 @@ context('SQL Endpoint', () => { it('Descending explicit', () => { let query = `[SELECT * FROM ${resource_identifier}][ORDER BY dist_name DESC];` cy.request({ - method: 'GET', + method: 'POST', url: apiUri + '/datastore/sql', body: { "query": query @@ -217,7 +217,7 @@ context('SQL Endpoint', () => { it('Limit only', () => { let query = `[SELECT * FROM ${resource_identifier}][ORDER BY dist_name ASC][LIMIT 1];` cy.request({ - method: 'GET', + method: 'POST', url: apiUri + '/datastore/sql', body: { "query": query @@ -232,7 +232,7 @@ context('SQL Endpoint', () => { it('Limit and offset', () => { let query = `[SELECT * FROM ${resource_identifier}][ORDER BY dist_name ASC][LIMIT 1 OFFSET 1];` cy.request({ - method: 'GET', + method: 'POST', url: apiUri + '/datastore/sql', body: { "query": query diff --git a/modules/custom/dkan_harvest/test/src/Unit/HarvesterTest.php b/modules/custom/dkan_harvest/test/src/Unit/HarvesterTest.php index 260b07602..fd9dd8593 100644 --- a/modules/custom/dkan_harvest/test/src/Unit/HarvesterTest.php +++ b/modules/custom/dkan_harvest/test/src/Unit/HarvesterTest.php @@ -33,7 +33,7 @@ public function testDeregisterHarvest() { $storeFactory = (new Chain($this)) ->add(DatabaseTableFactory::class, "getInstance", DatabaseTable::class) ->add(DatabaseTable::class, "retrieve", "Hello") - ->add(DatabaseTable::class, "destroy", null) + ->add(DatabaseTable::class, "destroy", NULL) ->add(DatabaseTable::class, "remove", "Hello") ->getMock(); diff --git a/modules/custom/dkan_harvest/test/src/Unit/Storage/DatabaseTableFactoryTest.php b/modules/custom/dkan_harvest/test/src/Unit/Storage/DatabaseTableFactoryTest.php index 157af49bc..63b3627c8 100644 --- a/modules/custom/dkan_harvest/test/src/Unit/Storage/DatabaseTableFactoryTest.php +++ b/modules/custom/dkan_harvest/test/src/Unit/Storage/DatabaseTableFactoryTest.php @@ -3,7 +3,6 @@ namespace Drupal\Tests\dkan_harvest\Unit\Storage; use Drupal\Core\Database\Connection; -use Drupal\Core\File\FileSystem; use Drupal\dkan_common\Tests\Mock\Chain; use Drupal\dkan_harvest\Storage\DatabaseTable; use Drupal\dkan_harvest\Storage\DatabaseTableFactory; @@ -20,11 +19,11 @@ class DatabaseTableFactoryTest extends TestCase { public function test() { $connection = (new Chain($this)) - ->add(Connection::class, "blah", null) + ->add(Connection::class, "blah", NULL) ->getMock(); $databaseTable = (new Chain($this)) - ->add(DatabaseTable::class, "blah", null) + ->add(DatabaseTable::class, "blah", NULL) ->getMock(); $factory = $this->getMockBuilder(DatabaseTableFactory::class) diff --git a/modules/custom/dkan_harvest/test/src/Unit/Storage/DatabaseTableTest.php b/modules/custom/dkan_harvest/test/src/Unit/Storage/DatabaseTableTest.php index fdb316cfc..b014b15ea 100644 --- a/modules/custom/dkan_harvest/test/src/Unit/Storage/DatabaseTableTest.php +++ b/modules/custom/dkan_harvest/test/src/Unit/Storage/DatabaseTableTest.php @@ -13,6 +13,9 @@ */ class DatabaseTableTest extends TestCase { + /** + * + */ public function testConstruction() { $connection = (new Chain($this)) ->add(Connection::class, "schema", Schema::class) diff --git a/modules/custom/dkan_sql_endpoint/dkan_sql_endpoint.routing.yml b/modules/custom/dkan_sql_endpoint/dkan_sql_endpoint.routing.yml index 22143ae0e..dccf28981 100644 --- a/modules/custom/dkan_sql_endpoint/dkan_sql_endpoint.routing.yml +++ b/modules/custom/dkan_sql_endpoint/dkan_sql_endpoint.routing.yml @@ -1,8 +1,15 @@ -dkan_sql_endpoint.api: +dkan_sql_endpoint.get.api: path: '/api/1/datastore/sql' methods: [GET] defaults: - { _controller: '\Drupal\dkan_sql_endpoint\Controller\Api::runQuery'} + { _controller: '\Drupal\dkan_sql_endpoint\Controller\Api::runQueryGet'} + requirements: + _access: 'TRUE' +dkan_sql_endpoint.post.api: + path: '/api/1/datastore/sql' + methods: [POST] + defaults: + { _controller: '\Drupal\dkan_sql_endpoint\Controller\Api::runQueryPost'} requirements: _access: 'TRUE' dkan_sql_endpoint.settings: diff --git a/modules/custom/dkan_sql_endpoint/src/Controller/Api.php b/modules/custom/dkan_sql_endpoint/src/Controller/Api.php index 34e5a9e52..59f9bf568 100644 --- a/modules/custom/dkan_sql_endpoint/src/Controller/Api.php +++ b/modules/custom/dkan_sql_endpoint/src/Controller/Api.php @@ -63,17 +63,44 @@ public function __construct( /** * Method called by the router. */ - public function runQuery() { + public function runQueryGet() { - $query_string = $this->getQueryString(); + $query = NULL; + $query = $this->requestStack->getCurrentRequest()->get('query'); - if (empty($query_string)) { - return $this->response("Missing 'query' query parameter or value", 400); + if (empty($query)) { + return $this->response("Missing 'query' query parameter", 400); } + return $this->runQuery($query); + } + + /** + * Method called by the router. + */ + public function runQueryPost() { + + $query = NULL; + $payloadJson = $this->requestStack->getCurrentRequest()->getContent(); + $payload = json_decode($payloadJson); + if (isset($payload->query)) { + $query = $payload->query; + } + + if (empty($query)) { + return $this->response("Missing 'query' property in the request's body.", 400); + } + + return $this->runQuery($query); + } + + /** + * Private. + */ + private function runQuery($query) { $parser = new SqlParser(); - if ($parser->validate($query_string) === FALSE) { + if ($parser->validate($query) === FALSE) { return $this->response("Invalid query string.", 500); } @@ -92,7 +119,7 @@ public function runQuery() { $result = $databaseTable->query($query_object); } catch (\Exception $e) { - $this->response("Querying a datastore that does not exist.", 500); + return $this->response("Querying a datastore that does not exist.", 500); } return $this->response($result, 200); @@ -106,22 +133,6 @@ private function getDatabaseTable($stateMachine) { return $this->databaseTableFactory->getInstance($resource->getId(), ['resource' => $resource]); } - /** - * Private. - */ - private function getQueryString() { - $queryString = NULL; - $queryString = $this->requestStack->getCurrentRequest()->get('query'); - if (empty($queryString)) { - $payloadJson = $this->requestStack->getCurrentRequest()->getContent(); - $payload = json_decode($payloadJson); - if (isset($payload->query)) { - $queryString = $payload->query; - } - } - return $queryString; - } - /** * Private. */ diff --git a/modules/custom/dkan_sql_endpoint/tests/src/Unit/Controller/ApiTest.php b/modules/custom/dkan_sql_endpoint/tests/src/Unit/Controller/ApiTest.php index 7fcd29a97..1c46add78 100644 --- a/modules/custom/dkan_sql_endpoint/tests/src/Unit/Controller/ApiTest.php +++ b/modules/custom/dkan_sql_endpoint/tests/src/Unit/Controller/ApiTest.php @@ -29,7 +29,16 @@ class ApiTest extends TestCase { */ public function test() { $controller = Api::create($this->getContainer()); - $response = $controller->runQuery(); + $response = $controller->runQueryGet(); + $this->assertEquals("[]", $response->getContent()); + } + + /** + * + */ + public function test2() { + $controller = Api::create($this->getContainer()); + $response = $controller->runQueryPost(); $this->assertEquals("[]", $response->getContent()); } @@ -44,10 +53,14 @@ private function getContainer() { ->add('request_stack', RequestStack::class) ->add('dkan_datastore.database_table_factory', DatabaseTableFactory::class); + $query = '[SELECT * FROM abc][WHERE abc = \'blah\'][ORDER BY abc DESC][LIMIT 1 OFFSET 3];'; + $body = json_encode(["query" => $query]); + $container = (new Chain($this)) ->add(Container::class, "get", $options) ->add(RequestStack::class, 'getCurrentRequest', Request::class) - ->add(Request::class, 'get', '[SELECT * FROM abc][WHERE abc = \'blah\'][ORDER BY abc DESC][LIMIT 1 OFFSET 3];') + ->add(Request::class, 'get', $query) + ->add(Request::class, 'getContent', $body) ->add(ConfigFactory::class, 'get', Config::class) ->add(Config::class, 'get', 1000) ->add(ResourceServiceFactory::class, 'getInstance', ResourceService::class)