refactor: data sync performance optimisation #634
Conversation
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.
| @@ -39,7 +39,7 @@ public function __construct( | |||
| */ | |||
| public function apply() | |||
| { | |||
| $this->automationFactory->create()->sync(); | |||
| //$this->automationFactory->create()->sync(); | |||
There was a problem hiding this comment.
The assumption is that this model is not used by CLI/cron job and this change won't prevent automation processing.
There was a problem hiding this comment.
This patch was added when we introduced queues for automation processing. It was possible at point of upgrade that a merchant still has some residual automations in the table (up to 15 mins' worth) which would not be queued and therefore left to become stale. Without this merchants would need to remember / be reminded to do this to process remaining old-style automations.
| if (count($names)) { | ||
| return implode(',', $names); | ||
| if (!$this->connectorCategoryModel) { | ||
| $this->connectorCategoryModel = ObjectManager::getInstance()->get(ConnectorCategoryModel::class); |
There was a problem hiding this comment.
Curious why this is necessary if the class was injected in the constructor in the usual way?
There was a problem hiding this comment.
Hi @sta1r
That snippet was maintained there since the original solution was applied as a patch. You are right, DI should suffice and the use of objectManager to be deleted/discouraged.
|
@pavelshulga plenty of good improvements here which we will aim to include in the coming sprints. 👍 |
setup:upgradecommand, instead being conducted only in the context of dedicateddotdigital:migratecommand.setup:upgradecommand context. To be handled instead within normal automation processing context.