Merge pull request #777 from Nosto/release/6.0.2

Release 6.0.2

CHANGELOG.md

@@ -1,5 +1,11 @@
 All notable changes to this project will be documented in this file. This project adheres to Semantic Versioning.
 
+### 6.0.2
+* Indexer - Reduces completed queue cleanup time to 1 hour instead of 8
+* Indexer Sync Service - Uses Magento Product repository to fetch product data in bulk, reducing amount of database queries
+* Indexer - Prevent insertion of duplicated rows to be upsert
+* Move currencies building process to a cronjob, reducing admin loading time in setups with a large amount of currencies and storeviews
+
 ### 6.0.1
 * Add support for both version 2 and 3 of phpseclib/phpseclib library (for compatibility with Magento versions 2.3.x and 2.4.x)
 * Remove customer visitor checksum generation when 2c.cid cookie does not exist

Cron/CurrenciesCron.php

@@ -0,0 +1,108 @@
+<?php
+
+/**
+ * Copyright (c) 2020, Nosto Solutions Ltd
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without modification,
+ * are permitted provided that the following conditions are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright notice,
+ * this list of conditions and the following disclaimer.
+ *
+ * 2. Redistributions in binary form must reproduce the above copyright notice,
+ * this list of conditions and the following disclaimer in the documentation
+ * and/or other materials provided with the distribution.
+ *
+ * 3. Neither the name of the copyright holder nor the names of its contributors
+ * may be used to endorse or promote products derived from this software without
+ * specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
+ * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+ * DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR
+ * ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
+ * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
+ * ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ *
+ * @author Nosto Solutions Ltd <[email protected]>
+ * @copyright 2020 Nosto Solutions Ltd
+ * @license http://opensource.org/licenses/BSD-3-Clause BSD 3-Clause
+ *
+ */
+
+namespace Nosto\Tagging\Cron;
+
+use Exception;
+use Nosto\Operation\UpdateSettings;
+use Nosto\Tagging\Helper\Account as NostoHelperAccount;
+use Nosto\Tagging\Helper\Scope as NostoHelperScope;
+use Nosto\Tagging\Logger\Logger as NostoLogger;
+use Nosto\Tagging\Model\Meta\Account\Settings\Builder as NostoSettingsBuilder;
+use Nosto\Tagging\Model\Meta\Account\Settings\Currencies\Builder as NostoCurrenciesBuilder;
+
+/**
+ * Cronjob class that periodically updates currencies used in the store
+ */
+class CurrenciesCron
+{
+    protected NostoLogger $logger;
+    private NostoCurrenciesBuilder $nostoCurrenciesBuilder;
+    private NostoHelperScope $nostoHelperScope;
+    private NostoHelperAccount $nostoHelperAccount;
+    private NostoSettingsBuilder $nostoSettingsBuilder;
+
+    /**
+     * CurrenciesCron constructor.
+     *
+     * @param NostoLogger $logger
+     * @param NostoHelperScope $nostoHelperScope
+     * @param NostoCurrenciesBuilder $nostoCurrenciesBuilder
+     * @param NostoHelperAccount $nostoHelperAccount
+     * @param NostoSettingsBuilder $nostoSettingsBuilder
+     */
+    public function __construct(
+        NostoLogger $logger,
+        NostoHelperScope $nostoHelperScope,
+        NostoCurrenciesBuilder $nostoCurrenciesBuilder,
+        NostoHelperAccount $nostoHelperAccount,
+        NostoSettingsBuilder $nostoSettingsBuilder
+    ) {
+        $this->logger = $logger;
+        $this->nostoHelperScope = $nostoHelperScope;
+        $this->nostoCurrenciesBuilder = $nostoCurrenciesBuilder;
+        $this->nostoHelperAccount = $nostoHelperAccount;
+        $this->nostoSettingsBuilder = $nostoSettingsBuilder;
+    }
+
+    public function execute(): void
+    {
+        $this->logger->info('Updating currencies to Nosto for all store views');
+        foreach ($this->nostoHelperScope->getStores(false) as $store) {
+            $this->logger->info('Updating currencies for ' . $store->getName());
+            if ($account = $this->nostoHelperAccount->findAccount($store)) {
+                try {
+                    $settings = $this->nostoSettingsBuilder->build($store);
+                    $settings->setCurrencies($this->nostoCurrenciesBuilder->build($store));
+                    $service = new UpdateSettings($account);
+                    $service->update($settings);
+                } catch (Exception $e) {
+                    $this->logger->error(sprintf(
+                        'Unable to update the currencies for the store view %s. Message was: %s',
+                        $store->getName(),
+                        $e->getMessage()
+                    ));
+                }
+            } else {
+                $this->logger->info(sprintf(
+                    'Skipping update; an account doesn\'t exist for %s',
+                    $store->getName()
+                ));
+            }
+        }
+    }
+}

Logger/Logger.php

@@ -62,7 +62,7 @@ public function exception(Throwable $exception)
      * @param $message
      * @param Store|null $store
      * @param null $sourceClass
-     * @return bool
+     * @return bool|void
      */
     public function logWithMemoryConsumption($message, Store $store = null, $sourceClass = null)
     {
@@ -78,7 +78,7 @@ public function logWithMemoryConsumption($message, Store $store = null, $sourceC
             $context['storeId'] = $store->getId();
         }
         if (is_object($sourceClass)) {
-            return $this->debugWithSource($message, $context, $sourceClass);
+            return $this->debugWithSource($msg, $context, $sourceClass);
         }
         return $this->debug($msg, $context);
     }

Model/Indexer/IndexerUtil.php

@@ -60,7 +60,7 @@ public function isCalledFromSetupUpgrade(InputInterface $input): bool
             if (count($parts) !== 2) {
                 return false;
             }
-            list($commandScope, $commandAction) = $parts;
+            [$commandScope, $commandAction] = $parts;
             $currentCommandScope = substr($commandScope, 0, strlen(self::SETUP_UPGRADE_SCOPE));
             $currentCommandAction = substr($commandAction, 0, strlen(self::SETUP_UPGRADE_ACTION));
             return (

Model/Meta/Account/Settings/Builder.php

@@ -47,13 +47,11 @@
 use Nosto\Tagging\Helper\Data as NostoDataHelper;
 use Nosto\Tagging\Helper\Variation as NostoVariationHelper;
 use Nosto\Tagging\Logger\Logger as NostoLogger;
-use Nosto\Tagging\Model\Meta\Account\Settings\Currencies\Builder as NostoCurrenciesBuilder;
 
 class Builder
 {
     private NostoLogger $logger;
     private ManagerInterface $eventManager;
-    private NostoCurrenciesBuilder $nostoCurrenciesBuilder;
     private NostoHelperCurrency $nostoHelperCurrency;
     private NostoDataHelper $nostoDataHelper;
     private NostoVariationHelper $nostoVariationHelper;
@@ -63,21 +61,18 @@ class Builder
      * @param NostoLogger $logger
      * @param ManagerInterface $eventManager
      * @param NostoHelperCurrency $nostoHelperCurrency
-     * @param NostoCurrenciesBuilder $nostoCurrenciesBuilder
      * @param NostoDataHelper $nostoDataHelper
      * @param NostoVariationHelper $nostoVariationHelper
      */
     public function __construct(
         NostoLogger $logger,
         ManagerInterface $eventManager,
         NostoHelperCurrency $nostoHelperCurrency,
-        NostoCurrenciesBuilder $nostoCurrenciesBuilder,
         NostoDataHelper $nostoDataHelper,
         NostoVariationHelper $nostoVariationHelper
     ) {
         $this->logger = $logger;
         $this->eventManager = $eventManager;
-        $this->nostoCurrenciesBuilder = $nostoCurrenciesBuilder;
         $this->nostoHelperCurrency = $nostoHelperCurrency;
         $this->nostoDataHelper = $nostoDataHelper;
         $this->nostoVariationHelper = $nostoVariationHelper;
@@ -102,7 +97,7 @@ public function build(Store $store)
             } elseif ($this->nostoDataHelper->isPricingVariationEnabled($store)) {
                 $settings->setDefaultVariantId($this->nostoVariationHelper->getDefaultVariationCode());
             }
-            $settings->setCurrencies($this->nostoCurrenciesBuilder->build($store));
+            // Currencies are build in CurrenciesCron to avoid increased loading times when logging into the admin
         } catch (Exception $e) {
             $this->logger->exception($e);
         }

Model/Product/Queue/QueueRepository.php

@@ -110,4 +110,18 @@ public function delete(ProductUpdateQueueInterface $entry)
         /** @phan-suppress-next-line PhanTypeMismatchArgument */
         $this->queueResource->delete($entry);
     }
+
+    /**
+     * @param ProductUpdateQueueInterface $entry
+     * @return bool
+     */
+    public function isEntryDuplicated(ProductUpdateQueueInterface $entry): bool
+    {
+        $collection = $this->queueCollectionFactory->create()
+            ->addStoreIdFilter($entry->getStoreId())
+            ->addStatusFilter($entry->getStatus())
+            ->addActionFilter($entry->getAction())
+            ->limitResults(1);
+        return (bool)$collection->getItems();
+    }
 }

Model/ResourceModel/Product/Update/Queue/QueueCollection.php

@@ -62,7 +62,7 @@ public function _construct()
      * @param StoreInterface $store
      * @return QueueCollection
      */
-    public function addStoreFilter(StoreInterface $store)
+    public function addStoreFilter(StoreInterface $store): QueueCollection
     {
         return $this->addStoreIdFilter($store->getId());
     }
@@ -71,7 +71,7 @@ public function addStoreFilter(StoreInterface $store)
      * @param array $ids
      * @return QueueCollection
      */
-    public function addIdsFilter(array $ids)
+    public function addIdsFilter(array $ids): QueueCollection
     {
         return $this->addFieldToFilter(
             ProductUpdateQueueInterface::ID,
@@ -85,7 +85,7 @@ public function addIdsFilter(array $ids)
      * @param int $storeId
      * @return QueueCollection
      */
-    public function addStoreIdFilter(int $storeId)
+    public function addStoreIdFilter(int $storeId): QueueCollection
     {
         return $this->addFieldToFilter(
             ProductUpdateQueueInterface::STORE_ID,
@@ -99,7 +99,7 @@ public function addStoreIdFilter(int $storeId)
      * @param string $status
      * @return QueueCollection
      */
-    public function addStatusFilter(string $status)
+    public function addStatusFilter(string $status): QueueCollection
     {
         return $this->addFieldToFilter(
             ProductUpdateQueueInterface::STATUS,
@@ -113,21 +113,35 @@ public function addStatusFilter(string $status)
      * @param DateTimeInterface $dateTime
      * @return QueueCollection
      */
-    public function addCompletedBeforeFilter(DateTimeInterface $dateTime)
+    public function addCompletedBeforeFilter(DateTimeInterface $dateTime): QueueCollection
     {
         return $this->addFieldToFilter(
             ProductUpdateQueueInterface::COMPLETED_AT,
             ['lteq' => $dateTime->format('Y-m-d H:i:s')]
         );
     }
 
+    /**
+     * Filters collection by action
+     *
+     * @param string $action
+     * @return QueueCollection
+     */
+    public function addActionFilter(string $action): QueueCollection
+    {
+        return $this->addFieldToFilter(
+            ProductUpdateQueueInterface::ACTION,
+            ['eq' => $action]
+        );
+    }
+
     /**
      * Filters collection by id (primary key)
      *
      * @param int $indexId
      * @return QueueCollection
      */
-    public function addIdFilter(int $indexId)
+    public function addIdFilter(int $indexId): QueueCollection
     {
         return $this->addFieldToFilter(
             ProductUpdateQueueInterface::ID,

Model/Service/Sync/AbstractBulkConsumer.php

@@ -38,7 +38,6 @@
 namespace Nosto\Tagging\Model\Service\Sync;
 
 use Exception;
-use InvalidArgumentException;
 use Magento\AsynchronousOperations\Api\Data\OperationInterface;
 use Magento\Framework\EntityManager\EntityManager;
 use Magento\Framework\Json\Helper\Data as JsonHelper;
@@ -88,15 +87,7 @@ public function __construct(
      */
     public function processOperation(OperationInterface $operation)
     {
-        $errorCode = null;
-        if ($operation instanceof OperationInterface) {
-            $serializedData = $operation->getSerializedData();
-        } else {
-            throw new InvalidArgumentException(
-                'Wrong type passed to AsyncBulkConsumer::processOperation.
-                Expected \Magento\AsynchronousOperations\Api\Data\OperationInterface.'
-            );
-        }
+        $serializedData = $operation->getSerializedData();
         $unserializedData = $this->jsonHelper->jsonDecode($serializedData);
         $productIds = $unserializedData['product_ids'];
         $storeId = $unserializedData['store_id'];

Model/Service/Sync/Upsert/SyncService.php

@@ -52,6 +52,7 @@
 use Nosto\Tagging\Model\Service\Cache\CacheService;
 use Nosto\Tagging\Model\Service\Product\ProductServiceInterface;
 use Nosto\Tagging\Util\PagingIterator;
+use Nosto\Tagging\Model\Product\Repository as ProductRepository;
 
 class SyncService extends AbstractService
 {
@@ -79,6 +80,9 @@ class SyncService extends AbstractService
     /** @var int */
     private int $apiTimeout;
 
+    /** @var ProductRepository */
+    private ProductRepository $productRepository;
+
     /**
      * Sync constructor.
      * @param NostoHelperAccount $nostoHelperAccount
@@ -87,6 +91,7 @@ class SyncService extends AbstractService
      * @param NostoDataHelper $nostoDataHelper
      * @param ProductServiceInterface $productService
      * @param CacheService $cacheService
+     * @param ProductRepository $productRepository
      * @param $apiBatchSize
      * @param $apiTimeout
      */
@@ -97,6 +102,7 @@ public function __construct(
         NostoDataHelper $nostoDataHelper,
         ProductServiceInterface $productService,
         CacheService $cacheService,
+        ProductRepository $productRepository,
         $apiBatchSize,
         $apiTimeout
     ) {
@@ -106,6 +112,7 @@ public function __construct(
         $this->nostoHelperUrl = $nostoHelperUrl;
         $this->nostoDataHelper = $nostoDataHelper;
         $this->cacheService = $cacheService;
+        $this->productRepository = $productRepository;
         $this->apiBatchSize = $apiBatchSize;
         $this->apiTimeout = $apiTimeout;
     }
@@ -139,8 +146,9 @@ public function syncProducts(ProductCollection $collection, Store $store)
             $this->checkMemoryConsumption('product sync');
             $op = new UpsertProduct($account, $this->nostoHelperUrl->getActiveDomain($store));
             $op->setResponseTimeout($this->apiTimeout);
+            $products = $this->productRepository->getByIds($page->getAllIds());
             /** @var Product $product */
-            foreach ($page as $product) {
+            foreach ($products->getItems() as $product) {
                 $productIdsInBatch[] = $product->getId();
                 $nostoProduct = $this->productService->getProduct($product, $store);
                 if ($nostoProduct === null) {

Model/Service/Update/QueueService.php

@@ -125,7 +125,7 @@ public function addCollectionToUpsertQueue(ProductCollection $collection, Store
                 $store,
                 $this->toParentProductIds($page)
             );
-            if (!empty($queueEntry->getProductIds())) {
+            if (!empty($queueEntry->getProductIds()) && !$this->queueRepository->isEntryDuplicated($queueEntry)) {
                 $this->queueRepository->save($queueEntry); // @codingStandardsIgnoreLine
             }
         }

Setup/Patch/Data/AddCustomerReference.php

@@ -157,6 +157,6 @@ public function addCustomerReference()
      */
     public static function getVersion(): string
     {
-        return '6.0.0';
+        return '6.0.2';
     }
 }

Setup/Patch/Data/AlterCustomerReferenceNonEditable.php

@@ -120,6 +120,6 @@ public function alterCustomerReferenceNonEditable()
      */
     public static function getVersion(): string
     {
-        return '6.0.0';
+        return '6.0.2';
     }
 }

Setup/Patch/Data/PopulateCustomerReference.php

@@ -144,6 +144,6 @@ public function populateCustomerReference()
      */
     public static function getVersion(): string
     {
-        return '6.0.0';
+        return '6.0.2';
     }
 }