Redefines how endpoint namespaces are set

Updates error response to terminate connection only on AJAX requests
Optimizes and cleans up code based on code revew comments

lib/API/API.php

@@ -16,7 +16,6 @@ class API {
   private $_request_endpoint_class;
   private $_request_data = array();
   private $_endpoint_namespaces = array();
-  private $_ajax_request = false;
   private $_available_api_versions = array(
       'v1'
   );
@@ -24,12 +23,10 @@ class API {
   const ENDPOINTS_LOCATION = 'Endpoints';
 
   function __construct() {
-    foreach($this->_available_api_versions as $version) {
+    foreach($this->_available_api_versions as $available_api_version) {
       $this->addEndpointNamespace(
-        array(
+        sprintf('%s\%s', __NAMESPACE__, self::ENDPOINTS_LOCATION),
-          'name' => sprintf('%s\%s', __NAMESPACE__, self::ENDPOINTS_LOCATION),
+        $available_api_version
-          'version' => $version
-        )
       );
     }
   }
@@ -55,21 +52,20 @@ class API {
   }
 
   function setupAjax() {
-    $this->_ajax_request = true;
     Hooks::doAction('mailpoet_api_setup', array($this));
-    $this->getRequestData($_POST);
+    $this->setRequestData($_POST);
 
     if($this->checkToken() === false) {
       $error_message = __('Invalid API request.', 'mailpoet');
       $error_response = $this->createErrorResponse(Error::UNAUTHORIZED, $error_message, Response::STATUS_UNAUTHORIZED);
-      return $error_response;
+      return $error_response->send();
     }
 
     $response = $this->processRoute();
     $response->send();
   }
 
-  function getRequestData($data) {
+  function setRequestData($data) {
     $this->_request_api_version = !empty($data['api_version']) ? $data['api_version']: false;
 
     $this->_request_endpoint = isset($data['endpoint'])
@@ -90,13 +86,12 @@ class API {
       $error_message = __('Invalid API request.', 'mailpoet');
       $error_response = $this->createErrorResponse(Error::BAD_REQUEST, $error_message, Response::STATUS_BAD_REQUEST);
       return $error_response;
-    } else {
+    } else if(!empty($this->_endpoint_namespaces[$this->_request_api_version])) {
-      foreach($this->_endpoint_namespaces as $namespace) {
+      foreach($this->_endpoint_namespaces[$this->_request_api_version] as $namespace) {
-        if($namespace['version'] !== $this->_request_api_version) continue;
         $endpoint_class = sprintf(
           '%s\%s\%s',
-          $namespace['name'],
+          $namespace,
-          $namespace['version'],
+          $this->_request_api_version,
           ucfirst($this->_request_endpoint)
         );
         if(class_exists($endpoint_class)) {
@@ -176,11 +171,9 @@ class API {
     );
   }
 
-  function addEndpointNamespace(array $namespace) {
+  function addEndpointNamespace($namespace, $version) {
-    if(empty($namespace['version'])) {
+    if(!empty($this->_endpoint_namespaces[$version][$namespace])) return;
-      throw new \Exception(__('Namespace version is required.', 'mailpoet'));
+    $this->_endpoint_namespaces[$version][] = $namespace;
-    }
-    $this->_endpoint_namespaces[] = $namespace;
   }
 
   function getEndpointNamespaces() {
@@ -203,6 +196,6 @@ class API {
       array(),
       $response_status
     );
-    return ($this->_ajax_request === true) ? $error_response->send() : $error_response;
+    return $error_response;
   }
 }

lib/Subscription/Form.php

@@ -9,14 +9,16 @@ use MailPoet\Util\Url;
 class Form {
   static function onSubmit() {
     $api = new API();
-    $api->getRequestData($_REQUEST);
+    $api->setRequestData($_REQUEST);
-    $form_id = (!empty($_REQUEST['data']['form_id'])) ? $_REQUEST['data']['form_id']: false;
+    $form_id = (!empty($_REQUEST['data']['form_id'])) ? (int)$_REQUEST['data']['form_id'] : false;
     $response = $api->processRoute();
     if($response->status !== APIResponse::STATUS_OK) {
-      Url::redirectBack(array(
+      Url::redirectBack(
-        'mailpoet_error' => ($form_id) ? $form_id : true,
+        array(
-        'mailpoet_success' => null
+          'mailpoet_error' => ($form_id) ? $form_id : true,
-      ));
+          'mailpoet_success' => null
+        )
+      );
     } else {
       (isset($response->meta['redirect_url'])) ?
         Url::redirectTo($response->meta['redirect_url']) :

tests/unit/API/APITest.php

@@ -65,24 +65,11 @@ class APITest extends MailPoetTest {
       'name' => 'MailPoet\\Dummy\\Name\\Space',
       'version' => 'v2'
     );
-    $this->api->addEndpointNamespace($namespace);
+    $this->api->addEndpointNamespace($namespace['name'], $namespace['version']);
     $namespaces = $this->api->getEndpointNamespaces();
 
     expect($namespaces)->count(2);
-    expect($namespaces[1])->equals($namespace);
+    expect($namespaces[$namespace['version']][0])->equals($namespace['name']);
-  }
-
-  function testItRequiresNamespaceVersionToBeSpecified() {
-    $namespace = array(
-      'name' => 'MailPoet\\Dummy\\Name\\Space'
-    );
-
-    try {
-      $this->api->addEndpointNamespace($namespace);
-      $this->fail('Exception was not thrown');
-    } catch(Exception $e) {
-      expect($e->getMessage())->equals('Namespace version is required.');
-    }
   }
 
   function testItReturns400ErrorWhenAPIVersionIsNotSpecified() {
@@ -91,7 +78,7 @@ class APITest extends MailPoetTest {
       'method' => 'test'
     );
 
-    $response = $this->api->getRequestData($data);
+    $response = $this->api->setRequestData($data);
     expect($response->status)->equals(APIResponse::STATUS_BAD_REQUEST);
   }
 
@@ -100,14 +87,14 @@ class APITest extends MailPoetTest {
       'name' => 'MailPoet\API\Endpoints',
       'version' => 'v2'
     );
-    $this->api->addEndpointNamespace($namespace);
+    $this->api->addEndpointNamespace($namespace['name'], $namespace['version']);
 
     $data = array(
       'endpoint' => 'namespaced_endpoint_stub',
       'api_version' => 'v2',
       'method' => 'test'
     );
-    $this->api->getRequestData($data);
+    $this->api->setRequestData($data);
 
     expect($this->api->getRequestedAPIVersion())->equals('v2');
     expect($this->api->getRequestedEndpointClass())->equals(
@@ -120,7 +107,7 @@ class APITest extends MailPoetTest {
       'name' => 'MailPoet\API\Endpoints',
       'version' => 'v1'
     );
-    $this->api->addEndpointNamespace($namespace);
+    $this->api->addEndpointNamespace($namespace['name'], $namespace['version']);
 
     $data = array(
       'endpoint' => 'namespaced_endpoint_stub',
@@ -128,7 +115,7 @@ class APITest extends MailPoetTest {
       'api_version' => 'v1',
       'data' => array('test' => 'data')
     );
-    $this->api->getRequestData($data);
+    $this->api->setRequestData($data);
     $response = $this->api->processRoute();
 
     expect($response->getData()['data'])->equals($data['data']);
@@ -139,14 +126,14 @@ class APITest extends MailPoetTest {
       'name' => 'MailPoet\API\Endpoints',
       'version' => 'v2'
     );
-    $this->api->addEndpointNamespace($namespace);
+    $this->api->addEndpointNamespace($namespace['name'], $namespace['version']);
 
     $data = array(
       'endpoint' => 'namespaced_endpoint_stub',
       'api_version' => 'v2',
       'method' => 'testVersion'
     );
-    $this->api->getRequestData($data);
+    $this->api->setRequestData($data);
     $response = $this->api->processRoute();
 
     expect($response->getData()['data'])->equals($data['api_version']);