Skip to content

Commit

Permalink
#19345: Incorrect caching subscription status in newsletter plugin.
Browse files Browse the repository at this point in the history
  • Loading branch information
engcom-Foxtrot committed Mar 22, 2021
1 parent 6df702e commit c6e170e
Show file tree
Hide file tree
Showing 3 changed files with 95 additions and 19 deletions.
46 changes: 46 additions & 0 deletions app/code/Magento/Newsletter/Model/CustomerSubscriberCache.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
<?php
/**
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/
declare(strict_types=1);

namespace Magento\Newsletter\Model;

/**
* This service provides caching Subscriber by Customer id.
*/
class CustomerSubscriberCache
{
/**
* @var array
*/
private $customerSubscriber = [];

/**
* Get Subscriber from cache by Customer id.
*
* @param int $customerId
* @return Subscriber|null
*/
public function getCustomerSubscriber(int $customerId): ?Subscriber
{
$subscriber = null;
if (isset($this->customerSubscriber[$customerId])) {
$subscriber = $this->customerSubscriber[$customerId];
}

return $subscriber;
}

/**
* Set Subscriber to cache by Customer id.
*
* @param int $customerId
* @param Subscriber|null $subscriber
*/
public function setCustomerSubscriber(int $customerId, ?Subscriber $subscriber): void
{
$this->customerSubscriber[$customerId] = $subscriber;
}
}
52 changes: 35 additions & 17 deletions app/code/Magento/Newsletter/Model/Plugin/CustomerPlugin.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,21 +3,24 @@
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/

namespace Magento\Newsletter\Model\Plugin;

use Magento\Customer\Api\CustomerRepositoryInterface;
use Magento\Customer\Api\Data\CustomerExtensionInterface;
use Magento\Customer\Api\Data\CustomerInterface;
use Magento\Customer\Model\Config\Share;
use Magento\Framework\Api\ExtensionAttributesFactory;
use Magento\Framework\Api\SearchResults;
use Magento\Framework\App\ObjectManager;
use Magento\Framework\Exception\NoSuchEntityException;
use Magento\Newsletter\Model\Subscriber;
use Magento\Newsletter\Model\CustomerSubscriberCache;
use Magento\Newsletter\Model\ResourceModel\Subscriber\CollectionFactory;
use Magento\Customer\Api\Data\CustomerExtensionInterface;
use Magento\Newsletter\Model\Subscriber;
use Magento\Newsletter\Model\SubscriberFactory;
use Magento\Newsletter\Model\SubscriptionManagerInterface;
use Magento\Store\Model\Store;
use Magento\Store\Model\StoreManagerInterface;
use Magento\Framework\Api\SearchResults;
use Psr\Log\LoggerInterface;

/**
Expand Down Expand Up @@ -62,6 +65,11 @@ class CustomerPlugin
*/
private $logger;

/**
* @var CustomerSubscriberCache
*/
private $customerSubscriberCache;

/**
* @param SubscriberFactory $subscriberFactory
* @param ExtensionAttributesFactory $extensionFactory
Expand All @@ -70,6 +78,7 @@ class CustomerPlugin
* @param Share $shareConfig
* @param StoreManagerInterface $storeManager
* @param LoggerInterface $logger
* @param CustomerSubscriberCache|null $customerSubscriberCache
*/
public function __construct(
SubscriberFactory $subscriberFactory,
Expand All @@ -78,7 +87,8 @@ public function __construct(
SubscriptionManagerInterface $subscriptionManager,
Share $shareConfig,
StoreManagerInterface $storeManager,
LoggerInterface $logger
LoggerInterface $logger,
CustomerSubscriberCache $customerSubscriberCache = null
) {
$this->subscriberFactory = $subscriberFactory;
$this->extensionFactory = $extensionFactory;
Expand All @@ -87,6 +97,8 @@ public function __construct(
$this->shareConfig = $shareConfig;
$this->storeManager = $storeManager;
$this->logger = $logger;
$this->customerSubscriberCache = $customerSubscriberCache
?? ObjectManager::getInstance()->get(CustomerSubscriberCache::class);
}

/**
Expand Down Expand Up @@ -124,9 +136,11 @@ public function afterSave(
}
if ($needToUpdate) {
$storeId = $this->getCurrentStoreId($result);
$customerId = (int)$result->getId();
$subscriber = $subscribeStatus
? $this->subscriptionManager->subscribeCustomer((int)$result->getId(), $storeId)
: $this->subscriptionManager->unsubscribeCustomer((int)$result->getId(), $storeId);
? $this->subscriptionManager->subscribeCustomer($customerId, $storeId)
: $this->subscriptionManager->unsubscribeCustomer($customerId, $storeId);
$this->customerSubscriberCache->setCustomerSubscriber($customerId, $subscriber);
}
$this->addIsSubscribedExtensionAttribute($result, $subscriber->isSubscribed());

Expand Down Expand Up @@ -252,7 +266,7 @@ public function afterGetList(CustomerRepositoryInterface $subject, SearchResults
$extensionAttributes = $customer->getExtensionAttributes();
/** @var Subscriber $subscribe */
$subscribe = $collection->getItemByColumnValue('subscriber_email', $customer->getEmail());
$isSubscribed = $subscribe && (int) $subscribe->getStatus() === Subscriber::STATUS_SUBSCRIBED;
$isSubscribed = $subscribe && (int)$subscribe->getStatus() === Subscriber::STATUS_SUBSCRIBED;
$extensionAttributes->setIsSubscribed($isSubscribed);
}

Expand Down Expand Up @@ -308,16 +322,20 @@ private function deleteSubscriptionsAfterCustomerDelete(CustomerInterface $custo
*/
private function getSubscriber(CustomerInterface $customer): Subscriber
{
/** @var Subscriber $subscriber */
$subscriber = $this->subscriberFactory->create();
$websiteId = $this->getCurrentWebsiteId($customer);
$subscriber->loadByCustomer((int)$customer->getId(), $websiteId);
/**
* If subscriber was't found by customer id then try to find subscriber by customer email.
* It need when the customer is creating and he has already subscribed as guest by same email.
*/
if (!$subscriber->getId()) {
$subscriber->loadBySubscriberEmail((string)$customer->getEmail(), $websiteId);
$customerId = (int)$customer->getId();
$subscriber = $this->customerSubscriberCache->getCustomerSubscriber($customerId);
if ($subscriber === null) {
$subscriber = $this->subscriberFactory->create();
$websiteId = $this->getCurrentWebsiteId($customer);
$subscriber->loadByCustomer((int)$customer->getId(), $websiteId);
/**
* If subscriber wasn't found by customer id then try to find subscriber by customer email.
* It need when the customer is creating and he has already subscribed as guest by same email.
*/
if (!$subscriber->getId()) {
$subscriber->loadBySubscriberEmail((string)$customer->getEmail(), $websiteId);
}
$this->customerSubscriberCache->setCustomerSubscriber($customerId, $subscriber);
}

return $subscriber;
Expand Down
16 changes: 14 additions & 2 deletions app/code/Magento/Newsletter/Model/SubscriptionManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
use Magento\Customer\Api\CustomerRepositoryInterface;
use Magento\Customer\Api\Data\CustomerInterface;
use Magento\Framework\App\Config\ScopeConfigInterface;
use Magento\Framework\App\ObjectManager;
use Magento\Framework\Exception\MailException;
use Magento\Store\Model\ScopeInterface;
use Magento\Store\Model\StoreManagerInterface;
Expand Down Expand Up @@ -51,28 +52,37 @@ class SubscriptionManager implements SubscriptionManagerInterface
*/
private $customerRepository;

/**
* @var CustomerSubscriberCache
*/
private $customerSubscriberCache;

/**
* @param SubscriberFactory $subscriberFactory
* @param LoggerInterface $logger
* @param StoreManagerInterface $storeManager
* @param ScopeConfigInterface $scopeConfig
* @param AccountManagementInterface $customerAccountManagement
* @param CustomerRepositoryInterface $customerRepository
* @param CustomerSubscriberCache|null $customerSubscriberCache
*/
public function __construct(
SubscriberFactory $subscriberFactory,
LoggerInterface $logger,
StoreManagerInterface $storeManager,
ScopeConfigInterface $scopeConfig,
AccountManagementInterface $customerAccountManagement,
CustomerRepositoryInterface $customerRepository
CustomerRepositoryInterface $customerRepository,
CustomerSubscriberCache $customerSubscriberCache = null
) {
$this->subscriberFactory = $subscriberFactory;
$this->logger = $logger;
$this->storeManager = $storeManager;
$this->scopeConfig = $scopeConfig;
$this->customerAccountManagement = $customerAccountManagement;
$this->customerRepository = $customerRepository;
$this->customerSubscriberCache = $customerSubscriberCache
?? ObjectManager::getInstance()->get(CustomerSubscriberCache::class);
}

/**
Expand Down Expand Up @@ -209,14 +219,16 @@ private function saveSubscriber(
if (!$subscriber->getId()) {
$subscriber->setSubscriberConfirmCode($subscriber->randomSequence());
}
$customerId = (int)$customer->getId();
$subscriber->setStatus($status)
->setStatusChanged($statusChanged)
->setCustomerId($customer->getId())
->setCustomerId($customerId)
->setStoreId($storeId)
->setEmail($customer->getEmail())
->save();

if ($statusChanged) {
$this->customerSubscriberCache->setCustomerSubscriber($customerId, null);
return true;
}

Expand Down

0 comments on commit c6e170e

Please sign in to comment.