From 89b1e519b94e823984d5e4739ab59e1183f241f5 Mon Sep 17 00:00:00 2001 From: Pavlo Shulha Date: Tue, 17 Sep 2024 11:36:29 +1000 Subject: [PATCH] refactor: data sync performance optimisation Add category resource to minimise a need to request category information for each category item, instead relying on full category tree names snapshot. Prevent data sync from triggering in the context of `setup:upgrade` command, instead being conducted only in the context of dedicated `dotdigital:migrate` command. Prevent pending automation processing within `setup:upgrade` command context. To be handled instead within normal automation processing context. Minor code refactoring better adhering to methods signatures (e.g. in Consent.php try{}catch block). Add logger in Order.php since no explicit dependency injected but used in the business logic implementation. --- Model/Connector/Category.php | 94 +++++++++++++++++++ Model/Connector/ContactData.php | 24 ++--- Model/ResourceModel/Order.php | 16 ++++ Model/Sync/Consent.php | 2 +- Setup/Install/Type/AbstractDataMigration.php | 4 +- .../InsertEmailContactTableCustomerSales.php | 4 +- .../UpdateEmailContactTableCustomerSales.php | 4 +- .../Patch/Data/ProcessPendingAutomations.php | 2 +- etc/db_schema.xml | 11 +++ 9 files changed, 145 insertions(+), 16 deletions(-) create mode 100644 Model/Connector/Category.php diff --git a/Model/Connector/Category.php b/Model/Connector/Category.php new file mode 100644 index 00000000..7b52d5c3 --- /dev/null +++ b/Model/Connector/Category.php @@ -0,0 +1,94 @@ +storeManager = $storeManager; + $this->categoryCollection = $categoryCollection; + $this->categoryResource = $categoryResource; + $this->init(); + } + + /** + * @return Category + */ + public function init() + { + foreach ($this->categoryCollection->setPageSize(10000)->load() as $category) { + /** @var Category $category */ + foreach ($this->storeManager->getStores() as $store) { + $category->setStoreId($store->getId()); + $this->categoryResource->load($category, $category->getId()); + if (!array_key_exists($category->getId(), $this->categoryNames)) { + $this->categoryNames[$category->getId()] = []; + } + if (!array_key_exists($store->getId(), $this->categoryNames[$category->getId()])) { + $this->categoryNames[$category->getId()][$store->getId()] = null; + } + $this->categoryNames[$category->getId()][$store->getId()] = $category->getName(); + } + } + + return $this; + } + + /** + * + * Get category names. + * + * @param array $categoryIds + * @return string + */ + public function getCategoryNames($categoryIds, $storeId): string + { + $names = []; + foreach ($categoryIds as $id) { + if (array_key_exists($id, $this->categoryNames) + && array_key_exists($storeId, $this->categoryNames[$id])) { + $names[$id] = $this->categoryNames[$id][$storeId]; + } + } + + //comma separated category names + if (count($names)) { + return implode(',', $names); + } + + return ''; + } +} diff --git a/Model/Connector/ContactData.php b/Model/Connector/ContactData.php index 6180c3be..c55ecefb 100644 --- a/Model/Connector/ContactData.php +++ b/Model/Connector/ContactData.php @@ -3,7 +3,9 @@ namespace Dotdigitalgroup\Email\Model\Connector; use Dotdigitalgroup\Email\Logger\Logger; +use Dotdigitalgroup\Email\Model\Connector\Category as ConnectorCategoryModel; use Dotdigitalgroup\Email\Model\Customer\DataField\Date; +use Magento\Framework\App\ObjectManager; use Magento\Framework\Model\AbstractModel; /** @@ -58,6 +60,11 @@ class ContactData */ private $categoryFactory; + /** + * @var ConnectorCategoryModel + */ + protected ConnectorCategoryModel $connectorCategoryModel; + /** * @var \Magento\Catalog\Model\ResourceModel\Category */ @@ -118,6 +125,7 @@ class ContactData * @param \Magento\Sales\Model\ResourceModel\Order $orderResource * @param \Magento\Catalog\Api\Data\CategoryInterfaceFactory $categoryFactory * @param \Magento\Catalog\Model\ResourceModel\Category $categoryResource + * @param Category $connectorCategoryModel * @param \Dotdigitalgroup\Email\Helper\Config $configHelper * @param Logger $logger * @param Date $dateField @@ -131,6 +139,7 @@ public function __construct( \Magento\Sales\Model\ResourceModel\Order $orderResource, \Magento\Catalog\Api\Data\CategoryInterfaceFactory $categoryFactory, \Magento\Catalog\Model\ResourceModel\Category $categoryResource, + ConnectorCategoryModel $connectorCategoryModel, \Dotdigitalgroup\Email\Helper\Config $configHelper, Logger $logger, Date $dateField, @@ -142,6 +151,7 @@ public function __construct( $this->orderResource = $orderResource; $this->productFactory = $productFactory; $this->categoryFactory = $categoryFactory; + $this->connectorCategoryModel = $connectorCategoryModel; $this->productResource = $productResource; $this->categoryResource = $categoryResource; $this->logger = $logger; @@ -441,19 +451,11 @@ private function getCategoryValue($categoryId) */ public function getCategoryNames($categoryIds) { - $names = []; - foreach ($categoryIds as $id) { - if (! isset($this->categoryNames[$id])) { - $this->categoryNames[$id] = $this->getCategoryValue($id); - } - $names[$this->categoryNames[$id]] = $this->categoryNames[$id]; - } - //comma separated category names - if (count($names)) { - return implode(',', $names); + if (!$this->connectorCategoryModel) { + $this->connectorCategoryModel = ObjectManager::getInstance()->get(ConnectorCategoryModel::class); } - return ''; + return $this->connectorCategoryModel->getCategoryNames($categoryIds, $this->model->getStoreId()); } /** diff --git a/Model/ResourceModel/Order.php b/Model/ResourceModel/Order.php index b14643f0..3862a75e 100755 --- a/Model/ResourceModel/Order.php +++ b/Model/ResourceModel/Order.php @@ -3,9 +3,25 @@ namespace Dotdigitalgroup\Email\Model\ResourceModel; use Dotdigitalgroup\Email\Setup\SchemaInterface as Schema; +use Magento\Framework\Model\ResourceModel\Db\Context; +use Psr\Log\LoggerInterface; class Order extends \Magento\Framework\Model\ResourceModel\Db\AbstractDb { + /** + * @param Context $context + * @param LoggerInterface $_logger + * @param $connectionName + */ + public function __construct( + Context $context, + LoggerInterface $_logger, + $connectionName = null + ) { + $this->_logger = $_logger; + parent::__construct($context, $connectionName); + } + /** * Initialize resource. * diff --git a/Model/Sync/Consent.php b/Model/Sync/Consent.php index 19424a37..01599240 100644 --- a/Model/Sync/Consent.php +++ b/Model/Sync/Consent.php @@ -117,7 +117,7 @@ public function sync(\DateTime $from = null) 'Error in consent sync for website id: %d', $websiteId ), - [(string)$e] + ['message' => $e->getMessage()] ); } } diff --git a/Setup/Install/Type/AbstractDataMigration.php b/Setup/Install/Type/AbstractDataMigration.php index fe679043..275f7b48 100755 --- a/Setup/Install/Type/AbstractDataMigration.php +++ b/Setup/Install/Type/AbstractDataMigration.php @@ -98,6 +98,8 @@ public function getTableName() */ public function isEnabled(): bool { - return true; + //change applied to ensure no setup:upgrade induced data migration and sync + global $argv; + return (isset($argv) && is_array($argv) && in_array('dotdigital:migrate', $argv)); } } diff --git a/Setup/Install/Type/InsertEmailContactTableCustomerSales.php b/Setup/Install/Type/InsertEmailContactTableCustomerSales.php index 2a8bdec2..b5c267cb 100644 --- a/Setup/Install/Type/InsertEmailContactTableCustomerSales.php +++ b/Setup/Install/Type/InsertEmailContactTableCustomerSales.php @@ -124,6 +124,8 @@ protected function insertData(Select $selectStatement) */ public function isEnabled(): bool { - return $this->config->isAccountSharingGlobal(); + global $argv; + $status = (isset($argv) && is_array($argv) && in_array('dotdigital:migrate', $argv)); + return $this->config->isAccountSharingGlobal() && $status; } } diff --git a/Setup/Install/Type/UpdateEmailContactTableCustomerSales.php b/Setup/Install/Type/UpdateEmailContactTableCustomerSales.php index 5771dc3e..085abd33 100644 --- a/Setup/Install/Type/UpdateEmailContactTableCustomerSales.php +++ b/Setup/Install/Type/UpdateEmailContactTableCustomerSales.php @@ -88,7 +88,9 @@ public function getUpdateWhereClause($row) */ public function isEnabled(): bool { - return $this->config->isAccountSharingGlobal(); + global $argv; + $status = (isset($argv) && is_array($argv) && in_array('dotdigital:migrate', $argv)); + return $this->config->isAccountSharingGlobal() && $status; } /** diff --git a/Setup/Patch/Data/ProcessPendingAutomations.php b/Setup/Patch/Data/ProcessPendingAutomations.php index 379b260f..452cc615 100644 --- a/Setup/Patch/Data/ProcessPendingAutomations.php +++ b/Setup/Patch/Data/ProcessPendingAutomations.php @@ -39,7 +39,7 @@ public function __construct( */ public function apply() { - $this->automationFactory->create()->sync(); + //$this->automationFactory->create()->sync(); } /** diff --git a/etc/db_schema.xml b/etc/db_schema.xml index 3805eb57..58b3979e 100644 --- a/etc/db_schema.xml +++ b/etc/db_schema.xml @@ -59,6 +59,11 @@ + + + + + @@ -443,4 +448,10 @@
+ + + + + +