Rework subscriber count reporting after a code review [MAILPOET-804]

Move updateSubscriberCount() out of checkKey()
Move wp_remote_post() to request()
Simplify a response check

lib/API/Endpoints/Settings.php

@@ -26,7 +26,8 @@ class Settings extends APIEndpoint {
           && Bridge::isMPSendingServiceEnabled()
       ) {
         $bridge = new Bridge();
-        $bridge->checkKey($settings['mta']['mailpoet_api_key']);
+        $result = $bridge->checkKey($settings['mta']['mailpoet_api_key']);
+        $bridge->updateSubscriberCount($result);
       }
       return $this->successResponse(Setting::getAll());
     }

lib/Cron/Workers/SendingServiceKeyCheck.php

@@ -88,6 +88,7 @@ class SendingServiceKeyCheck {
     try {
       $mailer_config = Mailer::getMailerConfig();
       $result = $this->bridge->checkKey($mailer_config['mailpoet_api_key']);
+      $this->bridge->updateSubscriberCount($result);
     } catch (\Exception $e) {
       $result = false;
     }

lib/Services/Bridge.php

@@ -42,9 +42,7 @@ class Bridge {
   function checkKey($api_key) {
     $this->initApi($api_key);
     $result = $this->api->checkKey();
-    $result = $this->processResult($result);
+    return $this->processResult($result);
-    $this->updateSubscriberCount($result);
-    return $result;
   }
 
   function processResult(array $result) {

lib/Services/Bridge/API.php

@@ -10,6 +10,8 @@ class API {
 
   const RESPONSE_CODE_KEY_INVALID = 401;
 
+  const RESPONSE_CODE_STATS_SAVED = 204;
+
   private $api_key;
 
   public $url_me = 'https://bridge.mailpoet.com/api/v0/me';
@@ -22,9 +24,9 @@ class API {
   }
 
   function checkKey() {
-    $result = wp_remote_post(
+    $result = $this->request(
       $this->url_me,
-      $this->request(array('site' => home_url()))
+      array('site' => home_url())
     );
 
     $code = wp_remote_retrieve_response_code($result);
@@ -45,9 +47,9 @@ class API {
   }
 
   function sendMessages($message_body) {
-    $result = wp_remote_post(
+    $result = $this->request(
       $this->url_messages,
-      $this->request($message_body)
+      $message_body
     );
     if(is_wp_error($result)) {
       return array(
@@ -70,9 +72,9 @@ class API {
   }
 
   function checkBounces(array $emails) {
-    $result = wp_remote_post(
+    $result = $this->request(
       $this->url_bounces,
-      $this->request($emails)
+      $emails
     );
     if(wp_remote_retrieve_response_code($result) === 200) {
       return json_decode(wp_remote_retrieve_body($result), true);
@@ -81,14 +83,12 @@ class API {
   }
 
   function updateSubscriberCount($count) {
-    $result = wp_remote_post(
+    $result = $this->request(
       $this->url_stats,
-      $this->request(array('subscriber_count' => (int)$count), 'PUT')
+      array('subscriber_count' => (int)$count),
+      'PUT'
     );
-    if(wp_remote_retrieve_response_code($result) === 204) {
+    return wp_remote_retrieve_response_code($result) === self::RESPONSE_CODE_STATS_SAVED;
-      return true;
-    }
-    return false;
   }
 
   function setKey($api_key) {
@@ -103,8 +103,8 @@ class API {
     return 'Basic ' . base64_encode('api:' . $this->api_key);
   }
 
-  private function request($body, $method = 'POST') {
+  private function request($url, $body, $method = 'POST') {
-    return array(
+    $params = array(
       'timeout' => 10,
       'httpversion' => '1.0',
       'method' => $method,
@@ -114,5 +114,6 @@ class API {
       ),
       'body' => json_encode($body)
     );
+    return wp_remote_post($url, $params);
   }
 }

tests/unit/Services/BridgeTest.php

@@ -60,6 +60,21 @@ class BridgeTest extends MailPoetTest {
     expect($result['state'])->equals(Bridge::MAILPOET_KEY_CHECK_ERROR);
   }
 
+  function testItUpdatesSubscriberCount() {
+    // it performs update if the key is valid or expiring
+    $result = array();
+    $result['state'] = Bridge::MAILPOET_KEY_VALID;
+    $updated = $this->bridge->updateSubscriberCount($result);
+    expect($updated)->true();
+    $result['state'] = Bridge::MAILPOET_KEY_EXPIRING;
+    $updated = $this->bridge->updateSubscriberCount($result);
+    expect($updated)->true();
+    // it does not perform update if the key is invalid
+    $result['state'] = Bridge::MAILPOET_KEY_INVALID;
+    $updated = $this->bridge->updateSubscriberCount($result);
+    expect($updated)->null();
+  }
+
   function testItInvalidatesKey() {
     Setting::setValue(
       Bridge::API_KEY_STATE_SETTING_NAME,