Move onSettingsChange from Bridge service to SettingsChangeHandler

Bridge is a low level service that processes requests and responses to/from Bridge API.
This change is needed so that we can remove dependency on MailPoet\Util\License\Features\Subscribers
service from the Bridge. The dependecy is a higher level service and may easily cause a circular dependency issue.

The SettingsChangeHandler is service for handling side effects
when saving settings. This feels as a better place to put the functionality.
[MAILPOET-5191]

mailpoet/lib/API/JSON/v1/Settings.php

@@ -150,7 +150,7 @@ class Settings extends APIEndpoint {
 
       // when pending approval, leave this to cron / Key Activation tab logic
       if (!$this->servicesChecker->isMailPoetAPIKeyPendingApproval()) {
-        $this->bridge->onSettingsSave($settings);
+        $this->settingsChangeHandler->updateBridge($settings);
       }
 
       $meta = $this->authorizedEmailsController->onSettingsSave($settings);

mailpoet/lib/Services/Bridge.php

@@ -303,23 +303,4 @@ class Bridge {
       null)
     );
   }
-
-  public function onSettingsSave($settings) {
-    $apiKey = $settings[Mailer::MAILER_CONFIG_SETTING_NAME]['mailpoet_api_key'] ?? null;
-    $premiumKey = $settings['premium']['premium_key'] ?? null;
-    if (!empty($apiKey)) {
-      $apiKeyState = $this->checkMSSKey($apiKey);
-      $this->storeMSSKeyAndState($apiKey, $apiKeyState);
-    }
-    if (!empty($premiumKey)) {
-      $premiumState = $this->checkPremiumKey($premiumKey);
-      $this->storePremiumKeyAndState($premiumKey, $premiumState);
-    }
-    if ($apiKey && !empty($apiKeyState) && in_array($apiKeyState['state'], [self::KEY_VALID, self::KEY_VALID_UNDERPRIVILEGED], true)) {
-      return $this->updateSubscriberCount($apiKey);
-    }
-    if ($premiumKey && !empty($premiumState) && in_array($premiumState['state'], [self::KEY_VALID, self::KEY_VALID_UNDERPRIVILEGED], true)) {
-      return $this->updateSubscriberCount($apiKey);
-    }
-  }
 }

mailpoet/lib/Settings/SettingsChangeHandler.php

@@ -5,7 +5,10 @@ namespace MailPoet\Settings;
 use MailPoet\Cron\Workers\InactiveSubscribers;
 use MailPoet\Cron\Workers\WooCommerceSync;
 use MailPoet\Entities\ScheduledTaskEntity;
+use MailPoet\Mailer\Mailer;
 use MailPoet\Newsletter\Sending\ScheduledTasksRepository;
+use MailPoet\Services\Bridge;
+use MailPoet\Services\SubscribersCountReporter;
 use MailPoet\WP\Functions as WPFunctions;
 use MailPoetVendor\Carbon\Carbon;
 
@@ -17,12 +20,22 @@ class SettingsChangeHandler {
   /** @var SettingsController */
   private $settingsController;
 
+  /** @var Bridge */
+  private $bridge;
+
+  /** @var SubscribersCountReporter */
+  private $subscribersCountReporter;
+
   public function __construct(
     ScheduledTasksRepository $scheduledTasksRepository,
-    SettingsController $settingsController
+    SettingsController $settingsController,
+    Bridge $bridge,
+    SubscribersCountReporter $subscribersCountReporter
   ) {
     $this->scheduledTasksRepository = $scheduledTasksRepository;
     $this->settingsController = $settingsController;
+    $this->bridge = $bridge;
+    $this->subscribersCountReporter = $subscribersCountReporter;
   }
 
   public function onSubscribeOldWoocommerceCustomersChange(): void {
@@ -77,4 +90,23 @@ class SettingsChangeHandler {
     $task->setStatus(ScheduledTaskEntity::STATUS_SCHEDULED);
     return $task;
   }
+
+  public function updateBridge($settings) {
+    $apiKey = $settings[Mailer::MAILER_CONFIG_SETTING_NAME]['mailpoet_api_key'] ?? null;
+    $premiumKey = $settings['premium']['premium_key'] ?? null;
+    if (!empty($apiKey)) {
+      $apiKeyState = $this->bridge->checkMSSKey($apiKey);
+      $this->bridge->storeMSSKeyAndState($apiKey, $apiKeyState);
+    }
+    if (!empty($premiumKey)) {
+      $premiumState = $this->bridge->checkPremiumKey($premiumKey);
+      $this->bridge->storePremiumKeyAndState($premiumKey, $premiumState);
+    }
+    if ($apiKey && !empty($apiKeyState) && in_array($apiKeyState['state'], [Bridge::KEY_VALID, Bridge::KEY_VALID_UNDERPRIVILEGED], true)) {
+      return $this->subscribersCountReporter->report($apiKey);
+    }
+    if ($premiumKey && !empty($premiumState) && in_array($premiumState['state'], [Bridge::KEY_VALID, Bridge::KEY_VALID_UNDERPRIVILEGED], true)) {
+      return $this->subscribersCountReporter->report($apiKey);
+    }
+  }
 }

mailpoet/tests/integration/API/JSON/v1/SettingsTest.php

@@ -101,7 +101,7 @@ class SettingsTest extends \MailPoetTest {
 
     $this->endpoint = new Settings(
       $this->settings,
-      $this->make(Bridge::class, ['onSettingsSave' => Expected::once()]),
+      $this->diContainer->get(Bridge::class),
       $this->make(AuthorizedEmailsController::class, ['onSettingsSave' => Expected::once()]),
       $this->diContainer->get(AuthorizedSenderDomainController::class),
       $this->make(TransactionalEmails::class),
@@ -113,7 +113,7 @@ class SettingsTest extends \MailPoetTest {
       $this->diContainer->get(FormMessageController::class),
       $this->make(ServicesChecker::class, ['isMailPoetAPIKeyPendingApproval' => false]),
       $this->diContainer->get(SegmentsRepository::class),
-      $this->diContainer->get(SettingsChangeHandler::class),
+      $this->make(SettingsChangeHandler::class, ['updateBridge' => Expected::once()]),
       $this->diContainer->get(SubscribersCountsController::class),
       $this->diContainer->get(TrackingConfig::class),
       $this->diContainer->get(ConfirmationEmailCustomizer::class)

mailpoet/tests/integration/Services/BridgeTest.php

@@ -9,7 +9,6 @@ use MailPoet\Services\Bridge\API;
 use MailPoet\Services\Bridge\BridgeTestMockAPI as MockAPI;
 use MailPoet\Settings\SettingsController;
 use MailPoet\WP\Functions as WPFunctions;
-use PHPUnit\Framework\MockObject\MockObject;
 
 require_once('BridgeTestMockAPI.php');
 
@@ -211,49 +210,6 @@ class BridgeTest extends \MailPoetTest {
     expect($storedState['state'])->equals(Bridge::KEY_INVALID);
   }
 
-  public function testItChecksAndStoresKeysOnSettingsSave() {
-    $response = ['state' => Bridge::KEY_VALID];
-    /** @var Bridge&MockObject $bridge */
-    $bridge = Stub::makeEmptyExcept(
-      Bridge::class,
-      'onSettingsSave',
-      [
-        'checkMSSKey' => $response,
-        'checkPremiumKey' => $response,
-      ],
-      $this
-    );
-    $bridge->expects($this->once())
-      ->method('checkMSSKey')
-      ->with($this->equalTo($this->validKey));
-    $bridge->expects($this->once())
-      ->method('storeMSSKeyAndState')
-      ->with(
-        $this->equalTo($this->validKey),
-        $this->equalTo($response)
-      );
-
-    $bridge->expects($this->once())
-      ->method('checkPremiumKey')
-      ->with($this->equalTo($this->validKey));
-    $bridge->expects($this->once())
-      ->method('storePremiumKeyAndState')
-      ->with(
-        $this->equalTo($this->validKey),
-        $this->equalTo($response)
-      );
-    $bridge->expects($this->once())
-      ->method('updateSubscriberCount')
-      ->with($this->equalTo($this->validKey));
-
-    $settings = [];
-    $settings[Mailer::MAILER_CONFIG_SETTING_NAME]['mailpoet_api_key'] = $this->validKey;
-    $settings['premium']['premium_key'] = $this->validKey;
-
-    $this->setMailPoetSendingMethod();
-    $bridge->onSettingsSave($settings);
-  }
-
   public function testItPingsBridge() {
     if (getenv('WP_TEST_ENABLE_NETWORK_TESTS') !== 'true') $this->markTestSkipped();
     expect(Bridge::pingBridge())->true();

mailpoet/tests/integration/Settings/SettingsChangeHandlerTest.php

@@ -5,7 +5,10 @@ namespace MailPoet\Settings;
 use MailPoet\Cron\Workers\InactiveSubscribers;
 use MailPoet\Cron\Workers\WooCommerceSync;
 use MailPoet\Entities\ScheduledTaskEntity;
+use MailPoet\Mailer\Mailer;
 use MailPoet\Newsletter\Sending\ScheduledTasksRepository;
+use MailPoet\Services\Bridge;
+use MailPoet\Services\SubscribersCountReporter;
 use MailPoet\WP\Functions as WPFunctions;
 use MailPoetVendor\Carbon\Carbon;
 
@@ -79,6 +82,49 @@ class SettingsChangeHandlerTest extends \MailPoetTest {
     return $task;
   }
 
+  public function testItChecksAndStoresKeysWhenUpdatingBridge() {
+    $key = 'valid-key';
+    $settings = [];
+    $settings[Mailer::MAILER_CONFIG_SETTING_NAME]['mailpoet_api_key'] = $key;
+    $settings['premium']['premium_key'] = $key;
+    $response = ['state' => Bridge::KEY_VALID];
+
+    $bridge = $this->createMock(Bridge::class);
+    $bridge->expects($this->once())
+      ->method('checkMSSKey')
+      ->with($this->equalTo($key))
+      ->willReturn($response);
+    $bridge->expects($this->once())
+      ->method('storeMSSKeyAndState')
+      ->with(
+        $this->equalTo($key),
+        $this->equalTo($response)
+      );
+
+    $bridge->expects($this->once())
+      ->method('checkPremiumKey')
+      ->with($this->equalTo($key))
+      ->willReturn($response);
+    $bridge->expects($this->once())
+      ->method('storePremiumKeyAndState')
+      ->with(
+        $this->equalTo($key),
+        $this->equalTo($response)
+      );
+
+    $countReporterMock = $this->createMock(SubscribersCountReporter::class);
+    $countReporterMock->expects($this->once())
+      ->method('report')
+      ->with($this->equalTo($key));
+
+    $changeHandler = $this->getServiceWithOverrides(SettingsChangeHandler::class, [
+      'bridge' => $bridge,
+      'subscribersCountReporter' => $countReporterMock,
+    ]);
+
+    $changeHandler->updateBridge($settings);
+  }
+
   private function getScheduledTaskByType(string $type): ?ScheduledTaskEntity {
     return $this->tasksRepository->findOneBy([
       'type' => $type,