Separating the query parameter and request body methods of interactio…

…n with the sql endpoint into more appropriate verbs. (#251)
GetDKAN · Nov 15, 2019 · dd24b0e · dd24b0e
1 parent b9c1bc6
commit dd24b0e
Show file tree

Hide file tree

Showing 7 changed files with 71 additions and 38 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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)