Skip to content

Commit c6e170e

Browse files
#19345: Incorrect caching subscription status in newsletter plugin.
1 parent 6df702e commit c6e170e

File tree

3 files changed

+95
-19
lines changed

3 files changed

+95
-19
lines changed
Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
<?php
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
declare(strict_types=1);
7+
8+
namespace Magento\Newsletter\Model;
9+
10+
/**
11+
* This service provides caching Subscriber by Customer id.
12+
*/
13+
class CustomerSubscriberCache
14+
{
15+
/**
16+
* @var array
17+
*/
18+
private $customerSubscriber = [];
19+
20+
/**
21+
* Get Subscriber from cache by Customer id.
22+
*
23+
* @param int $customerId
24+
* @return Subscriber|null
25+
*/
26+
public function getCustomerSubscriber(int $customerId): ?Subscriber
27+
{
28+
$subscriber = null;
29+
if (isset($this->customerSubscriber[$customerId])) {
30+
$subscriber = $this->customerSubscriber[$customerId];
31+
}
32+
33+
return $subscriber;
34+
}
35+
36+
/**
37+
* Set Subscriber to cache by Customer id.
38+
*
39+
* @param int $customerId
40+
* @param Subscriber|null $subscriber
41+
*/
42+
public function setCustomerSubscriber(int $customerId, ?Subscriber $subscriber): void
43+
{
44+
$this->customerSubscriber[$customerId] = $subscriber;
45+
}
46+
}

app/code/Magento/Newsletter/Model/Plugin/CustomerPlugin.php

Lines changed: 35 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -3,21 +3,24 @@
33
* Copyright © Magento, Inc. All rights reserved.
44
* See COPYING.txt for license details.
55
*/
6+
67
namespace Magento\Newsletter\Model\Plugin;
78

89
use Magento\Customer\Api\CustomerRepositoryInterface;
10+
use Magento\Customer\Api\Data\CustomerExtensionInterface;
911
use Magento\Customer\Api\Data\CustomerInterface;
1012
use Magento\Customer\Model\Config\Share;
1113
use Magento\Framework\Api\ExtensionAttributesFactory;
14+
use Magento\Framework\Api\SearchResults;
15+
use Magento\Framework\App\ObjectManager;
1216
use Magento\Framework\Exception\NoSuchEntityException;
13-
use Magento\Newsletter\Model\Subscriber;
17+
use Magento\Newsletter\Model\CustomerSubscriberCache;
1418
use Magento\Newsletter\Model\ResourceModel\Subscriber\CollectionFactory;
15-
use Magento\Customer\Api\Data\CustomerExtensionInterface;
19+
use Magento\Newsletter\Model\Subscriber;
1620
use Magento\Newsletter\Model\SubscriberFactory;
1721
use Magento\Newsletter\Model\SubscriptionManagerInterface;
1822
use Magento\Store\Model\Store;
1923
use Magento\Store\Model\StoreManagerInterface;
20-
use Magento\Framework\Api\SearchResults;
2124
use Psr\Log\LoggerInterface;
2225

2326
/**
@@ -62,6 +65,11 @@ class CustomerPlugin
6265
*/
6366
private $logger;
6467

68+
/**
69+
* @var CustomerSubscriberCache
70+
*/
71+
private $customerSubscriberCache;
72+
6573
/**
6674
* @param SubscriberFactory $subscriberFactory
6775
* @param ExtensionAttributesFactory $extensionFactory
@@ -70,6 +78,7 @@ class CustomerPlugin
7078
* @param Share $shareConfig
7179
* @param StoreManagerInterface $storeManager
7280
* @param LoggerInterface $logger
81+
* @param CustomerSubscriberCache|null $customerSubscriberCache
7382
*/
7483
public function __construct(
7584
SubscriberFactory $subscriberFactory,
@@ -78,7 +87,8 @@ public function __construct(
7887
SubscriptionManagerInterface $subscriptionManager,
7988
Share $shareConfig,
8089
StoreManagerInterface $storeManager,
81-
LoggerInterface $logger
90+
LoggerInterface $logger,
91+
CustomerSubscriberCache $customerSubscriberCache = null
8292
) {
8393
$this->subscriberFactory = $subscriberFactory;
8494
$this->extensionFactory = $extensionFactory;
@@ -87,6 +97,8 @@ public function __construct(
8797
$this->shareConfig = $shareConfig;
8898
$this->storeManager = $storeManager;
8999
$this->logger = $logger;
100+
$this->customerSubscriberCache = $customerSubscriberCache
101+
?? ObjectManager::getInstance()->get(CustomerSubscriberCache::class);
90102
}
91103

92104
/**
@@ -124,9 +136,11 @@ public function afterSave(
124136
}
125137
if ($needToUpdate) {
126138
$storeId = $this->getCurrentStoreId($result);
139+
$customerId = (int)$result->getId();
127140
$subscriber = $subscribeStatus
128-
? $this->subscriptionManager->subscribeCustomer((int)$result->getId(), $storeId)
129-
: $this->subscriptionManager->unsubscribeCustomer((int)$result->getId(), $storeId);
141+
? $this->subscriptionManager->subscribeCustomer($customerId, $storeId)
142+
: $this->subscriptionManager->unsubscribeCustomer($customerId, $storeId);
143+
$this->customerSubscriberCache->setCustomerSubscriber($customerId, $subscriber);
130144
}
131145
$this->addIsSubscribedExtensionAttribute($result, $subscriber->isSubscribed());
132146

@@ -252,7 +266,7 @@ public function afterGetList(CustomerRepositoryInterface $subject, SearchResults
252266
$extensionAttributes = $customer->getExtensionAttributes();
253267
/** @var Subscriber $subscribe */
254268
$subscribe = $collection->getItemByColumnValue('subscriber_email', $customer->getEmail());
255-
$isSubscribed = $subscribe && (int) $subscribe->getStatus() === Subscriber::STATUS_SUBSCRIBED;
269+
$isSubscribed = $subscribe && (int)$subscribe->getStatus() === Subscriber::STATUS_SUBSCRIBED;
256270
$extensionAttributes->setIsSubscribed($isSubscribed);
257271
}
258272

@@ -308,16 +322,20 @@ private function deleteSubscriptionsAfterCustomerDelete(CustomerInterface $custo
308322
*/
309323
private function getSubscriber(CustomerInterface $customer): Subscriber
310324
{
311-
/** @var Subscriber $subscriber */
312-
$subscriber = $this->subscriberFactory->create();
313-
$websiteId = $this->getCurrentWebsiteId($customer);
314-
$subscriber->loadByCustomer((int)$customer->getId(), $websiteId);
315-
/**
316-
* If subscriber was't found by customer id then try to find subscriber by customer email.
317-
* It need when the customer is creating and he has already subscribed as guest by same email.
318-
*/
319-
if (!$subscriber->getId()) {
320-
$subscriber->loadBySubscriberEmail((string)$customer->getEmail(), $websiteId);
325+
$customerId = (int)$customer->getId();
326+
$subscriber = $this->customerSubscriberCache->getCustomerSubscriber($customerId);
327+
if ($subscriber === null) {
328+
$subscriber = $this->subscriberFactory->create();
329+
$websiteId = $this->getCurrentWebsiteId($customer);
330+
$subscriber->loadByCustomer((int)$customer->getId(), $websiteId);
331+
/**
332+
* If subscriber wasn't found by customer id then try to find subscriber by customer email.
333+
* It need when the customer is creating and he has already subscribed as guest by same email.
334+
*/
335+
if (!$subscriber->getId()) {
336+
$subscriber->loadBySubscriberEmail((string)$customer->getEmail(), $websiteId);
337+
}
338+
$this->customerSubscriberCache->setCustomerSubscriber($customerId, $subscriber);
321339
}
322340

323341
return $subscriber;

app/code/Magento/Newsletter/Model/SubscriptionManager.php

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
use Magento\Customer\Api\CustomerRepositoryInterface;
1212
use Magento\Customer\Api\Data\CustomerInterface;
1313
use Magento\Framework\App\Config\ScopeConfigInterface;
14+
use Magento\Framework\App\ObjectManager;
1415
use Magento\Framework\Exception\MailException;
1516
use Magento\Store\Model\ScopeInterface;
1617
use Magento\Store\Model\StoreManagerInterface;
@@ -51,28 +52,37 @@ class SubscriptionManager implements SubscriptionManagerInterface
5152
*/
5253
private $customerRepository;
5354

55+
/**
56+
* @var CustomerSubscriberCache
57+
*/
58+
private $customerSubscriberCache;
59+
5460
/**
5561
* @param SubscriberFactory $subscriberFactory
5662
* @param LoggerInterface $logger
5763
* @param StoreManagerInterface $storeManager
5864
* @param ScopeConfigInterface $scopeConfig
5965
* @param AccountManagementInterface $customerAccountManagement
6066
* @param CustomerRepositoryInterface $customerRepository
67+
* @param CustomerSubscriberCache|null $customerSubscriberCache
6168
*/
6269
public function __construct(
6370
SubscriberFactory $subscriberFactory,
6471
LoggerInterface $logger,
6572
StoreManagerInterface $storeManager,
6673
ScopeConfigInterface $scopeConfig,
6774
AccountManagementInterface $customerAccountManagement,
68-
CustomerRepositoryInterface $customerRepository
75+
CustomerRepositoryInterface $customerRepository,
76+
CustomerSubscriberCache $customerSubscriberCache = null
6977
) {
7078
$this->subscriberFactory = $subscriberFactory;
7179
$this->logger = $logger;
7280
$this->storeManager = $storeManager;
7381
$this->scopeConfig = $scopeConfig;
7482
$this->customerAccountManagement = $customerAccountManagement;
7583
$this->customerRepository = $customerRepository;
84+
$this->customerSubscriberCache = $customerSubscriberCache
85+
?? ObjectManager::getInstance()->get(CustomerSubscriberCache::class);
7686
}
7787

7888
/**
@@ -209,14 +219,16 @@ private function saveSubscriber(
209219
if (!$subscriber->getId()) {
210220
$subscriber->setSubscriberConfirmCode($subscriber->randomSequence());
211221
}
222+
$customerId = (int)$customer->getId();
212223
$subscriber->setStatus($status)
213224
->setStatusChanged($statusChanged)
214-
->setCustomerId($customer->getId())
225+
->setCustomerId($customerId)
215226
->setStoreId($storeId)
216227
->setEmail($customer->getEmail())
217228
->save();
218229

219230
if ($statusChanged) {
231+
$this->customerSubscriberCache->setCustomerSubscriber($customerId, null);
220232
return true;
221233
}
222234

0 commit comments

Comments
 (0)