From dd7390ec7337fcf517d3e928797f1465d7284aab Mon Sep 17 00:00:00 2001 From: Rostislav Wolny Date: Mon, 14 Sep 2020 14:00:52 +0200 Subject: [PATCH] Refactor mailpoet_get_subscribers_in_segment_finders hook to direct calls [MAILPOET-3077] --- lib/DI/ContainerConfigurator.php | 4 ++ lib/DynamicSegments/DynamicSegmentHooks.php | 12 ------ lib/Segments/SubscribersFinder.php | 35 ++++++++-------- .../Segments/SubscribersFinderTest.php | 41 +++++-------------- 4 files changed, 31 insertions(+), 61 deletions(-) diff --git a/lib/DI/ContainerConfigurator.php b/lib/DI/ContainerConfigurator.php index 9e98c39ebe..17d57f344d 100644 --- a/lib/DI/ContainerConfigurator.php +++ b/lib/DI/ContainerConfigurator.php @@ -132,8 +132,12 @@ class ContainerConfigurator implements IContainerConfigurator { // Dynamic segments $container->autowire(\MailPoet\DynamicSegments\DynamicSegmentHooks::class); $container->autowire(\MailPoet\DynamicSegments\FreePluginConnectors\AddToNewslettersSegments::class); + $container->autowire(\MailPoet\DynamicSegments\FreePluginConnectors\SendingNewslettersSubscribersFinder::class)->setPublic(true); + $container->autowire(\MailPoet\DynamicSegments\Mappers\DBMapper::class); $container->autowire(\MailPoet\DynamicSegments\Persistence\Loading\Loader::class); $container->autowire(\MailPoet\DynamicSegments\Persistence\Loading\SubscribersCount::class); + $container->autowire(\MailPoet\DynamicSegments\Persistence\Loading\SubscribersIds::class); + $container->autowire(\MailPoet\DynamicSegments\Persistence\Loading\SingleSegmentLoader::class); // Cron $container->autowire(\MailPoet\Cron\CronHelper::class)->setPublic(true); $container->autowire(\MailPoet\Cron\CronTrigger::class)->setPublic(true); diff --git a/lib/DynamicSegments/DynamicSegmentHooks.php b/lib/DynamicSegments/DynamicSegmentHooks.php index 494836a443..9aeba1cb6e 100644 --- a/lib/DynamicSegments/DynamicSegmentHooks.php +++ b/lib/DynamicSegments/DynamicSegmentHooks.php @@ -3,14 +3,12 @@ namespace MailPoet\DynamicSegments; use MailPoet\DynamicSegments\FreePluginConnectors\AddToSubscribersFilters; -use MailPoet\DynamicSegments\FreePluginConnectors\SendingNewslettersSubscribersFinder; use MailPoet\DynamicSegments\FreePluginConnectors\SubscribersBulkActionHandler; use MailPoet\DynamicSegments\FreePluginConnectors\SubscribersListingsHandlerFactory; use MailPoet\DynamicSegments\Mappers\DBMapper; use MailPoet\DynamicSegments\Persistence\Loading\Loader; use MailPoet\DynamicSegments\Persistence\Loading\SingleSegmentLoader; use MailPoet\DynamicSegments\Persistence\Loading\SubscribersCount; -use MailPoet\DynamicSegments\Persistence\Loading\SubscribersIds; use MailPoet\WP\Functions as WPFunctions; class DynamicSegmentHooks { @@ -22,11 +20,6 @@ class DynamicSegmentHooks { } public function init() { - $this->wp->addAction( - 'mailpoet_get_subscribers_in_segment_finders', - [$this, 'getSubscribersInSegmentsFinders'] - ); - $this->wp->addAction( 'mailpoet_get_subscribers_listings_in_segment_handlers', [$this, 'getSubscribersListingsInSegmentsHandlers'] @@ -48,11 +41,6 @@ class DynamicSegmentHooks { ); } - public function getSubscribersInSegmentsFinders(array $finders) { - $finders[] = new SendingNewslettersSubscribersFinder(new SingleSegmentLoader(new DBMapper()), new SubscribersIds()); - return $finders; - } - public function getSubscribersListingsInSegmentsHandlers(array $handlers) { $handlers[] = new SubscribersListingsHandlerFactory(); return $handlers; diff --git a/lib/Segments/SubscribersFinder.php b/lib/Segments/SubscribersFinder.php index c35608ce3c..6dd7c4d46a 100644 --- a/lib/Segments/SubscribersFinder.php +++ b/lib/Segments/SubscribersFinder.php @@ -2,25 +2,28 @@ namespace MailPoet\Segments; +use MailPoet\DI\ContainerWrapper; +use MailPoet\DynamicSegments\FreePluginConnectors\SendingNewslettersSubscribersFinder; use MailPoet\Models\ScheduledTask; use MailPoet\Models\ScheduledTaskSubscriber; use MailPoet\Models\Segment; use MailPoet\Models\Subscriber; -use MailPoet\WP\Functions as WPFunctions; use MailPoetVendor\Idiorm\ORM; use function MailPoetVendor\array_column; class SubscribersFinder { - /** @var WPFunctions */ - private $wp; + /** @var SendingNewslettersSubscribersFinder */ + private $dynamicSegmentSubscriberFinder; - public function __construct(WPFunctions $wp = null) { - if (!$wp) { - $wp = new WPFunctions; + public function __construct( + SendingNewslettersSubscribersFinder $dynamicSegmentSubscriberFinder = null + ) { + if ($dynamicSegmentSubscriberFinder === null) { + $dynamicSegmentSubscriberFinder = ContainerWrapper::getInstance()->get(SendingNewslettersSubscribersFinder::class); } - $this->wp = $wp; + $this->dynamicSegmentSubscriberFinder = $dynamicSegmentSubscriberFinder; } public function findSubscribersInSegments($subscribersToProcessIds, $newsletterSegmentsIds) { @@ -40,12 +43,9 @@ class SubscribersFinder { $subscribers = Subscriber::findSubscribersInSegments($subscribersToProcessIds, [$segment->id])->findMany(); return Subscriber::extractSubscribersIds($subscribers); } - $finders = $this->wp->applyFilters('mailpoet_get_subscribers_in_segment_finders', []); - foreach ($finders as $finder) { - $subscribers = $finder->findSubscribersInSegment($segment, $subscribersToProcessIds); - if ($subscribers) { - return Subscriber::extractSubscribersIds($subscribers); - } + $subscribers = $this->dynamicSegmentSubscriberFinder->findSubscribersInSegment($segment, $subscribersToProcessIds); + if ($subscribers) { + return Subscriber::extractSubscribersIds($subscribers); } return []; } @@ -108,13 +108,10 @@ class SubscribersFinder { } private function addSubscribersToTaskFromDynamicSegment(ScheduledTask $task, Segment $segment) { - $finders = $this->wp->applyFilters('mailpoet_get_subscribers_in_segment_finders', []); $count = 0; - foreach ($finders as $finder) { - $subscribers = $finder->getSubscriberIdsInSegment($segment); - if ($subscribers) { - $count += $this->addSubscribersToTaskByIds($task, $subscribers); - } + $subscribers = $this->dynamicSegmentSubscriberFinder->getSubscriberIdsInSegment($segment); + if ($subscribers) { + $count += $this->addSubscribersToTaskByIds($task, $subscribers); } return $count; } diff --git a/tests/integration/Segments/SubscribersFinderTest.php b/tests/integration/Segments/SubscribersFinderTest.php index c5a164e3c2..e5b3cc4f2f 100644 --- a/tests/integration/Segments/SubscribersFinderTest.php +++ b/tests/integration/Segments/SubscribersFinderTest.php @@ -5,13 +5,13 @@ namespace MailPoet\Segments; require_once('FinderMock.php'); use Codeception\Util\Stub; +use MailPoet\DynamicSegments\FreePluginConnectors\SendingNewslettersSubscribersFinder; use MailPoet\Models\ScheduledTask; use MailPoet\Models\ScheduledTaskSubscriber; use MailPoet\Models\Segment; use MailPoet\Models\Subscriber; use MailPoet\Models\SubscriberSegment; use MailPoet\Tasks\Sending as SendingTask; -use MailPoet\WP\Functions as WPFunctions; use MailPoetVendor\Idiorm\ORM; use PHPUnit\Framework\MockObject\MockObject; @@ -72,38 +72,28 @@ class SubscribersFinderTest extends \MailPoetTest { } public function testFindSubscribersInSegmentUsingFinder() { - /** @var MockObject $mock */ - $mock = Stub::makeEmpty('MailPoet\Segments\FinderMock', ['findSubscribersInSegment']); + /** @var SendingNewslettersSubscribersFinder & MockObject $mock */ + $mock = Stub::makeEmpty(SendingNewslettersSubscribersFinder::class, ['findSubscribersInSegment']); $mock ->expects($this->once()) ->method('findSubscribersInSegment') ->will($this->returnValue([$this->subscriber3])); - remove_all_filters('mailpoet_get_subscribers_in_segment_finders'); - (new WPFunctions)->addFilter('mailpoet_get_subscribers_in_segment_finders', function () use ($mock) { - return [$mock]; - }); - - $finder = new SubscribersFinder(); + $finder = new SubscribersFinder($mock); $subscribers = $finder->findSubscribersInSegments([$this->subscriber3->id], [$this->segment3->id]); expect($subscribers)->count(1); expect($subscribers)->contains($this->subscriber3->id); } public function testFindSubscribersInSegmentUsingFinderMakesResultUnique() { - /** @var MockObject $mock */ - $mock = Stub::makeEmpty('MailPoet\Segments\FinderMock', ['findSubscribersInSegment']); + /** @var SendingNewslettersSubscribersFinder & MockObject $mock */ + $mock = Stub::makeEmpty(SendingNewslettersSubscribersFinder::class, ['findSubscribersInSegment']); $mock ->expects($this->exactly(2)) ->method('findSubscribersInSegment') ->will($this->returnValue([$this->subscriber3])); - remove_all_filters('mailpoet_get_subscribers_in_segment_finders'); - (new WPFunctions)->addFilter('mailpoet_get_subscribers_in_segment_finders', function () use ($mock) { - return [$mock]; - }); - - $finder = new SubscribersFinder(); + $finder = new SubscribersFinder($mock); $subscribers = $finder->findSubscribersInSegments([$this->subscriber3->id], [$this->segment3->id, $this->segment3->id]); expect($subscribers)->count(1); } @@ -133,19 +123,14 @@ class SubscribersFinderTest extends \MailPoetTest { } public function testItAddsSubscribersToTaskFromDynamicSegments() { - /** @var MockObject $mock */ - $mock = Stub::makeEmpty('MailPoet\Segments\FinderMock', ['getSubscriberIdsInSegment']); + /** @var SendingNewslettersSubscribersFinder & MockObject $mock */ + $mock = Stub::makeEmpty(SendingNewslettersSubscribersFinder::class, ['getSubscriberIdsInSegment']); $mock ->expects($this->once()) ->method('getSubscriberIdsInSegment') ->will($this->returnValue([['id' => $this->subscriber1->id]])); - remove_all_filters('mailpoet_get_subscribers_in_segment_finders'); - (new WPFunctions)->addFilter('mailpoet_get_subscribers_in_segment_finders', function () use ($mock) { - return [$mock]; - }); - - $finder = new SubscribersFinder(); + $finder = new SubscribersFinder($mock); $subscribersCount = $finder->addSubscribersToTaskFromSegments( $this->sending->task(), [ @@ -160,15 +145,11 @@ class SubscribersFinderTest extends \MailPoetTest { $finder = new SubscribersFinder(); /** @var MockObject $mock */ - $mock = Stub::makeEmpty('MailPoet\Segments\FinderMock', ['getSubscriberIdsInSegment']); + $mock = Stub::makeEmpty(SendingNewslettersSubscribersFinder::class, ['getSubscriberIdsInSegment']); $mock ->expects($this->once()) ->method('getSubscriberIdsInSegment') ->will($this->returnValue([['id' => $this->subscriber2->id]])); - remove_all_filters('mailpoet_get_subscribers_in_segment_finders'); - (new WPFunctions)->addFilter('mailpoet_get_subscribers_in_segment_finders', function () use ($mock) { - return [$mock]; - }); $subscribersCount = $finder->addSubscribersToTaskFromSegments( $this->sending->task(),