diff --git a/lib/API/JSON/v1/Services.php b/lib/API/JSON/v1/Services.php index d2078c0ba1..7e61cbbdb2 100644 --- a/lib/API/JSON/v1/Services.php +++ b/lib/API/JSON/v1/Services.php @@ -30,6 +30,7 @@ class Services extends APIEndpoint { try { $result = $this->bridge->checkMSSKey($key); + $this->bridge->storeMSSKeyAndState($key, $result); } catch(\Exception $e) { return $this->errorResponse(array( $e->getCode() => $e->getMessage() @@ -79,6 +80,7 @@ class Services extends APIEndpoint { try { $result = $this->bridge->checkPremiumKey($key); + $this->bridge->storePremiumKeyAndState($key, $result); } catch(\Exception $e) { return $this->errorResponse(array( $e->getCode() => $e->getMessage() diff --git a/lib/Cron/Workers/KeyCheck/PremiumKeyCheck.php b/lib/Cron/Workers/KeyCheck/PremiumKeyCheck.php index aa3e741b76..edd0ed8db1 100644 --- a/lib/Cron/Workers/KeyCheck/PremiumKeyCheck.php +++ b/lib/Cron/Workers/KeyCheck/PremiumKeyCheck.php @@ -16,6 +16,7 @@ class PremiumKeyCheck extends KeyCheckWorker { function checkKey() { $premium_key = Setting::getValue(Bridge::PREMIUM_KEY_SETTING_NAME); $result = $this->bridge->checkPremiumKey($premium_key); + $this->bridge->storePremiumKeyAndState($premium_key, $result); return $result; } } diff --git a/lib/Cron/Workers/KeyCheck/SendingServiceKeyCheck.php b/lib/Cron/Workers/KeyCheck/SendingServiceKeyCheck.php index a275cdf76c..6f3189ce54 100644 --- a/lib/Cron/Workers/KeyCheck/SendingServiceKeyCheck.php +++ b/lib/Cron/Workers/KeyCheck/SendingServiceKeyCheck.php @@ -15,7 +15,9 @@ class SendingServiceKeyCheck extends KeyCheckWorker { function checkKey() { $mailer_config = Mailer::getMailerConfig(); - $result = $this->bridge->checkMSSKey($mailer_config['mailpoet_api_key']); + $mss_key = $mailer_config['mailpoet_api_key']; + $result = $this->bridge->checkMSSKey($mss_key); + $this->bridge->storeMSSKeyAndState($mss_key, $result); $this->bridge->updateSubscriberCount($result); return $result; } diff --git a/lib/Services/Bridge.php b/lib/Services/Bridge.php index 503218d953..0e343ea892 100644 --- a/lib/Services/Bridge.php +++ b/lib/Services/Bridge.php @@ -63,38 +63,45 @@ class Bridge { function checkMSSKey($api_key) { $this->initApi($api_key); $result = $this->api->checkMSSKey(); - return $this->processAPIKeyCheckResult($result); + return $this->processMSSKeyCheckResult($result); } - private function processAPIKeyCheckResult(array $result) { + private function processMSSKeyCheckResult(array $result) { $state_map = array( 200 => self::MAILPOET_KEY_VALID, 401 => self::MAILPOET_KEY_INVALID, 402 => self::MAILPOET_KEY_EXPIRING ); - $update_settings = false; - if(!empty($result['code']) && isset($state_map[$result['code']])) { $key_state = $state_map[$result['code']]; - $update_settings = true; } else { $key_state = self::MAILPOET_KEY_CHECK_ERROR; } - // store the key itself - if($update_settings) { - Setting::setValue( - self::API_KEY_SETTING_NAME, - $this->api->getKey() - ); - } - return $this->buildKeyState( $key_state, - $result, + $result + ); + } + + function storeMSSKeyAndState($key, $state) { + if(empty($state['state']) + || $state['state'] === self::MAILPOET_KEY_CHECK_ERROR + ) { + return false; + } + + // store the key itself + Setting::setValue( + self::API_KEY_SETTING_NAME, + $key + ); + + // store the key state + Setting::setValue( self::API_KEY_STATE_SETTING_NAME, - $update_settings + $state ); } @@ -111,8 +118,6 @@ class Bridge { 402 => self::PREMIUM_KEY_ALREADY_USED ); - $update_settings = false; - if(!empty($result['code']) && isset($state_map[$result['code']])) { if($state_map[$result['code']] == self::PREMIUM_KEY_VALID && !empty($result['data']['expire_at']) @@ -121,41 +126,43 @@ class Bridge { } else { $key_state = $state_map[$result['code']]; } - $update_settings = true; } else { $key_state = self::PREMIUM_KEY_CHECK_ERROR; } - // store the key itself - if($update_settings) { - Setting::setValue( - self::PREMIUM_KEY_SETTING_NAME, - $this->api->getKey() - ); - } - return $this->buildKeyState( $key_state, - $result, - self::PREMIUM_KEY_STATE_SETTING_NAME, - $update_settings + $result ); } - private function buildKeyState($key_state, $result, $setting_name, $update_settings = false) { + function storePremiumKeyAndState($key, $state) { + if(empty($state['state']) + || $state['state'] === self::PREMIUM_KEY_CHECK_ERROR + ) { + return false; + } + + // store the key itself + Setting::setValue( + self::PREMIUM_KEY_SETTING_NAME, + $key + ); + + // store the key state + Setting::setValue( + self::PREMIUM_KEY_STATE_SETTING_NAME, + $state + ); + } + + private function buildKeyState($key_state, $result) { $state = array( 'state' => $key_state, 'data' => !empty($result['data']) ? $result['data'] : null, 'code' => !empty($result['code']) ? $result['code'] : self::CHECK_ERROR_UNKNOWN ); - if($update_settings) { - Setting::setValue( - $setting_name, - $state - ); - } - return $state; } diff --git a/tests/unit/API/JSON/v1/ServicesTest.php b/tests/unit/API/JSON/v1/ServicesTest.php index f0ee1a580f..1e06538e04 100644 --- a/tests/unit/API/JSON/v1/ServicesTest.php +++ b/tests/unit/API/JSON/v1/ServicesTest.php @@ -21,7 +21,10 @@ class ServicesTest extends MailPoetTest { function testItRespondsWithSuccessIfMSSKeyIsValid() { $this->services_endpoint->bridge = Stub::make( new Bridge(), - array('checkMSSKey' => array('state' => Bridge::MAILPOET_KEY_VALID)), + array( + 'checkMSSKey' => array('state' => Bridge::MAILPOET_KEY_VALID), + 'storeMSSKeyAndState' => Stub::once() + ), $this ); $response = $this->services_endpoint->checkMSSKey($this->data); @@ -31,7 +34,10 @@ class ServicesTest extends MailPoetTest { function testItRespondsWithErrorIfMSSKeyIsInvalid() { $this->services_endpoint->bridge = Stub::make( new Bridge(), - array('checkMSSKey' => array('state' => Bridge::MAILPOET_KEY_INVALID)), + array( + 'checkMSSKey' => array('state' => Bridge::MAILPOET_KEY_INVALID), + 'storeMSSKeyAndState' => Stub::once() + ), $this ); $response = $this->services_endpoint->checkMSSKey($this->data); @@ -42,10 +48,13 @@ class ServicesTest extends MailPoetTest { $date = new DateTime; $this->services_endpoint->bridge = Stub::make( new Bridge(), - array('checkMSSKey' => array( - 'state' => Bridge::MAILPOET_KEY_EXPIRING, - 'data' => array('expire_at' => $date->format('c')) - )), + array( + 'checkMSSKey' => array( + 'state' => Bridge::MAILPOET_KEY_EXPIRING, + 'data' => array('expire_at' => $date->format('c')) + ), + 'storeMSSKeyAndState' => Stub::once() + ), $this ); $response = $this->services_endpoint->checkMSSKey($this->data); @@ -57,7 +66,10 @@ class ServicesTest extends MailPoetTest { function testItRespondsWithErrorIfServiceIsUnavailableDuringMSSCheck() { $this->services_endpoint->bridge = Stub::make( new Bridge(), - array('checkMSSKey' => array('code' => Bridge::CHECK_ERROR_UNAVAILABLE)), + array( + 'checkMSSKey' => array('code' => Bridge::CHECK_ERROR_UNAVAILABLE), + 'storeMSSKeyAndState' => Stub::once() + ), $this ); $response = $this->services_endpoint->checkMSSKey($this->data); @@ -68,7 +80,10 @@ class ServicesTest extends MailPoetTest { function testItRespondsWithErrorIfMSSCheckThrowsAnException() { $this->services_endpoint->bridge = Stub::make( new Bridge(), - array('checkMSSKey' => function() { throw new \Exception('test'); }), + array( + 'checkMSSKey' => function() { throw new \Exception('test'); }, + 'storeMSSKeyAndState' => Stub::never() + ), $this ); $response = $this->services_endpoint->checkMSSKey($this->data); @@ -85,7 +100,10 @@ class ServicesTest extends MailPoetTest { function testItRespondsWithSuccessIfPremiumKeyIsValid() { $this->services_endpoint->bridge = Stub::make( new Bridge(), - array('checkPremiumKey' => array('state' => Bridge::PREMIUM_KEY_VALID)), + array( + 'checkPremiumKey' => array('state' => Bridge::PREMIUM_KEY_VALID), + 'storePremiumKeyAndState' => Stub::once() + ), $this ); $response = $this->services_endpoint->checkPremiumKey($this->data); @@ -98,7 +116,10 @@ class ServicesTest extends MailPoetTest { function testItRespondsWithErrorIfPremiumKeyIsInvalid() { $this->services_endpoint->bridge = Stub::make( new Bridge(), - array('checkPremiumKey' => array('state' => Bridge::PREMIUM_KEY_INVALID)), + array( + 'checkPremiumKey' => array('state' => Bridge::PREMIUM_KEY_INVALID), + 'storePremiumKeyAndState' => Stub::once() + ), $this ); $response = $this->services_endpoint->checkPremiumKey($this->data); @@ -108,7 +129,10 @@ class ServicesTest extends MailPoetTest { function testItRespondsWithErrorIfPremiumKeyIsUsed() { $this->services_endpoint->bridge = Stub::make( new Bridge(), - array('checkPremiumKey' => array('state' => Bridge::PREMIUM_KEY_ALREADY_USED)), + array( + 'checkPremiumKey' => array('state' => Bridge::PREMIUM_KEY_ALREADY_USED), + 'storePremiumKeyAndState' => Stub::once() + ), $this ); $response = $this->services_endpoint->checkPremiumKey($this->data); @@ -119,10 +143,13 @@ class ServicesTest extends MailPoetTest { $date = new DateTime; $this->services_endpoint->bridge = Stub::make( new Bridge(), - array('checkPremiumKey' => array( - 'state' => Bridge::PREMIUM_KEY_EXPIRING, - 'data' => array('expire_at' => $date->format('c')) - )), + array( + 'checkPremiumKey' => array( + 'state' => Bridge::PREMIUM_KEY_EXPIRING, + 'data' => array('expire_at' => $date->format('c')) + ), + 'storePremiumKeyAndState' => Stub::once() + ), $this ); $response = $this->services_endpoint->checkPremiumKey($this->data); @@ -134,7 +161,10 @@ class ServicesTest extends MailPoetTest { function testItRespondsWithErrorIfServiceIsUnavailableDuringPremiumCheck() { $this->services_endpoint->bridge = Stub::make( new Bridge(), - array('checkPremiumKey' => array('code' => Bridge::CHECK_ERROR_UNAVAILABLE)), + array( + 'checkPremiumKey' => array('code' => Bridge::CHECK_ERROR_UNAVAILABLE), + 'storePremiumKeyAndState' => Stub::once() + ), $this ); $response = $this->services_endpoint->checkPremiumKey($this->data); @@ -145,11 +175,14 @@ class ServicesTest extends MailPoetTest { function testItRespondsWithErrorIfPremiumCheckThrowsAnException() { $this->services_endpoint->bridge = Stub::make( new Bridge(), - array('checkPremiumKey' => function() { throw new \Exception('test'); }), + array( + 'checkPremiumKey' => function() { throw new \Exception('test'); }, + 'storePremiumKeyAndState' => Stub::never() + ), $this ); $response = $this->services_endpoint->checkPremiumKey($this->data); expect($response->status)->equals(APIResponse::STATUS_NOT_FOUND); expect($response->errors[0]['message'])->equals('test'); } -} \ No newline at end of file +} diff --git a/tests/unit/Cron/Workers/KeyCheck/PremiumKeyCheckTest.php b/tests/unit/Cron/Workers/KeyCheck/PremiumKeyCheckTest.php index 0ea260d2ba..c9bff945e9 100644 --- a/tests/unit/Cron/Workers/KeyCheck/PremiumKeyCheckTest.php +++ b/tests/unit/Cron/Workers/KeyCheck/PremiumKeyCheckTest.php @@ -21,12 +21,21 @@ class PremiumKeyCheckTest extends MailPoetTest { $response = array('code' => Bridge::PREMIUM_KEY_VALID); $this->worker->bridge = Stub::make( new Bridge, - array('checkPremiumKey' => $response), + array( + 'checkPremiumKey' => $response, + 'storePremiumKeyAndState' => null + ), $this ); $this->worker->bridge->expects($this->once()) ->method('checkPremiumKey') ->with($this->equalTo($this->premium_key)); + $this->worker->bridge->expects($this->once()) + ->method('storePremiumKeyAndState') + ->with( + $this->equalTo($this->premium_key), + $this->equalTo($response) + ); $this->fillPremiumKey(); expect($this->worker->checkKey())->equals($response); } diff --git a/tests/unit/Cron/Workers/KeyCheck/SendingServiceKeyCheckTest.php b/tests/unit/Cron/Workers/KeyCheck/SendingServiceKeyCheckTest.php index ebb75d6fac..c82e71e235 100644 --- a/tests/unit/Cron/Workers/KeyCheck/SendingServiceKeyCheckTest.php +++ b/tests/unit/Cron/Workers/KeyCheck/SendingServiceKeyCheckTest.php @@ -8,6 +8,7 @@ use MailPoet\Services\Bridge; class SendingServiceKeyCheckTest extends MailPoetTest { function _before() { + $this->mss_key = 'some_key'; $this->worker = new SendingServiceKeyCheck(microtime(true)); } @@ -23,10 +24,20 @@ class SendingServiceKeyCheckTest extends MailPoetTest { new Bridge, array( 'checkMSSKey' => $response, + 'storeMSSKeyAndState' => null, 'updateSubscriberCount' => Stub::once() ), $this ); + $this->worker->bridge->expects($this->once()) + ->method('checkMSSKey') + ->with($this->equalTo($this->mss_key)); + $this->worker->bridge->expects($this->once()) + ->method('storeMSSKeyAndState') + ->with( + $this->equalTo($this->mss_key), + $this->equalTo($response) + ); $this->setMailPoetSendingMethod(); expect($this->worker->checkKey())->equals($response); } @@ -36,7 +47,7 @@ class SendingServiceKeyCheckTest extends MailPoetTest { Mailer::MAILER_CONFIG_SETTING_NAME, array( 'method' => 'MailPoet', - 'mailpoet_api_key' => 'some_key', + 'mailpoet_api_key' => $this->mss_key, ) ); } diff --git a/tests/unit/Services/BridgeTest.php b/tests/unit/Services/BridgeTest.php index f3f3d65e4c..502057dfa6 100644 --- a/tests/unit/Services/BridgeTest.php +++ b/tests/unit/Services/BridgeTest.php @@ -50,14 +50,12 @@ class BridgeTest extends MailPoetTest { $result = $this->bridge->checkMSSKey($this->valid_key); expect($result)->notEmpty(); expect($result['state'])->equals(Bridge::MAILPOET_KEY_VALID); - expect($this->getMSSKey())->equals($this->valid_key); } function testItChecksInvalidMSSKey() { $result = $this->bridge->checkMSSKey($this->invalid_key); expect($result)->notEmpty(); expect($result['state'])->equals(Bridge::MAILPOET_KEY_INVALID); - expect($this->getMSSKey())->equals($this->invalid_key); } function testItChecksExpiringMSSKey() { @@ -65,7 +63,6 @@ class BridgeTest extends MailPoetTest { expect($result)->notEmpty(); expect($result['state'])->equals(Bridge::MAILPOET_KEY_EXPIRING); expect($result['data']['expire_at'])->notEmpty(); - expect($this->getMSSKey())->equals($this->expiring_key); } function testItReturnsErrorStateOnEmptyAPIResponseCodeDuringMSSCheck() { @@ -74,28 +71,50 @@ class BridgeTest extends MailPoetTest { $result = $this->bridge->checkMSSKey($this->valid_key); expect($result)->notEmpty(); expect($result['state'])->equals(Bridge::MAILPOET_KEY_CHECK_ERROR); - expect($this->getMSSKey())->notEquals($this->valid_key); + } + + function testItStoresExpectedMSSKeyStates() { + $states = array( + Bridge::MAILPOET_KEY_VALID => $this->valid_key, + Bridge::MAILPOET_KEY_INVALID => $this->invalid_key, + Bridge::MAILPOET_KEY_EXPIRING => $this->expiring_key + ); + foreach($states as $state => $key) { + $state = array('state' => $state); + $this->bridge->storeMSSKeyAndState($key, $state); + expect($this->getMSSKey())->equals($key); + expect($this->getMSSKeyState())->equals($state); + } + } + + function testItDoesNotStoreErroneousOrUnexpectedMSSKeyStates() { + $states = array( + array('state' => Bridge::MAILPOET_KEY_CHECK_ERROR), + array() + ); + foreach($states as $state) { + $this->bridge->storeMSSKeyAndState($this->valid_key, $state); + expect($this->getMSSKey())->notEquals($this->valid_key); + expect($this->getMSSKeyState())->notEquals($state); + } } function testItChecksValidPremiumKey() { $result = $this->bridge->checkPremiumKey($this->valid_key); expect($result)->notEmpty(); expect($result['state'])->equals(Bridge::PREMIUM_KEY_VALID); - expect($this->getPremiumKey())->equals($this->valid_key); } function testItChecksInvalidPremiumKey() { $result = $this->bridge->checkPremiumKey($this->invalid_key); expect($result)->notEmpty(); expect($result['state'])->equals(Bridge::PREMIUM_KEY_INVALID); - expect($this->getPremiumKey())->equals($this->invalid_key); } function testItChecksAlreadyUsedPremiumKey() { $result = $this->bridge->checkPremiumKey($this->used_premium_key); expect($result)->notEmpty(); expect($result['state'])->equals(Bridge::PREMIUM_KEY_ALREADY_USED); - expect($this->getPremiumKey())->equals($this->used_premium_key); } function testItChecksExpiringPremiumKey() { @@ -103,7 +122,6 @@ class BridgeTest extends MailPoetTest { expect($result)->notEmpty(); expect($result['state'])->equals(Bridge::PREMIUM_KEY_EXPIRING); expect($result['data']['expire_at'])->notEmpty(); - expect($this->getPremiumKey())->equals($this->expiring_premium_key); } function testItReturnsErrorStateOnEmptyAPIResponseCodeDuringPremiumCheck() { @@ -112,7 +130,33 @@ class BridgeTest extends MailPoetTest { $result = $this->bridge->checkPremiumKey($this->valid_key); expect($result)->notEmpty(); expect($result['state'])->equals(Bridge::PREMIUM_KEY_CHECK_ERROR); - expect($this->getPremiumKey())->notEquals($this->valid_key); + } + + function testItStoresExpectedPremiumKeyStates() { + $states = array( + Bridge::PREMIUM_KEY_VALID => $this->valid_key, + Bridge::PREMIUM_KEY_INVALID => $this->invalid_key, + Bridge::PREMIUM_KEY_ALREADY_USED => $this->used_premium_key, + Bridge::PREMIUM_KEY_EXPIRING => $this->expiring_key + ); + foreach($states as $state => $key) { + $state = array('state' => $state); + $this->bridge->storePremiumKeyAndState($key, $state); + expect($this->getPremiumKey())->equals($key); + expect($this->getPremiumKeyState())->equals($state); + } + } + + function testItDoesNotStoreErroneousOrUnexpectedPremiumKeyStates() { + $states = array( + array('state' => Bridge::PREMIUM_KEY_CHECK_ERROR), + array() + ); + foreach($states as $state) { + $this->bridge->storePremiumKeyAndState($this->valid_key, $state); + expect($this->getPremiumKey())->notEquals($this->valid_key); + expect($this->getPremiumKeyState())->notEquals($state); + } } function testItUpdatesSubscriberCount() { @@ -136,8 +180,7 @@ class BridgeTest extends MailPoetTest { array('state' => Bridge::MAILPOET_KEY_VALID) ); Bridge::invalidateKey(); - $value = Setting::getValue(Bridge::API_KEY_STATE_SETTING_NAME); - expect($value)->equals(array('state' => Bridge::MAILPOET_KEY_INVALID)); + expect($this->getMSSKeyState())->equals(array('state' => Bridge::MAILPOET_KEY_INVALID)); } function testItChecksKeysOnSettingsSave() { @@ -173,6 +216,10 @@ class BridgeTest extends MailPoetTest { return Setting::getValue(Bridge::API_KEY_SETTING_NAME); } + private function getMSSKeyState() { + return Setting::getValue(Bridge::API_KEY_STATE_SETTING_NAME); + } + private function fillPremiumKey() { Setting::setValue( Bridge::PREMIUM_KEY_SETTING_NAME, @@ -184,6 +231,10 @@ class BridgeTest extends MailPoetTest { return Setting::getValue(Bridge::PREMIUM_KEY_SETTING_NAME); } + private function getPremiumKeyState() { + return Setting::getValue(Bridge::PREMIUM_KEY_STATE_SETTING_NAME); + } + function _after() { ORM::raw_execute('TRUNCATE ' . Setting::$_table); }