Merge pull request #724 from Nosto/release/5.2.0

Release - 5.2.0

.github/workflows/action.yml

@@ -14,7 +14,7 @@ jobs:
     - name: Set up PHP
       uses: shivammathur/setup-php@v2
       with:
-        php-version: '7.2'
+        php-version: '7.3'
         tools: composer, prestissimo
         extensions: ast
         coverage: none
@@ -68,7 +68,7 @@ jobs:
     - name: Set up PHP
       uses: shivammathur/setup-php@v2
       with:
-        php-version: '7.2'
+        php-version: '7.3'
         tools: composer, prestissimo
         extensions: ast
         coverage: none
@@ -119,7 +119,7 @@ jobs:
     - name: Set up PHP
       uses: shivammathur/setup-php@v2
       with:
-        php-version: '7.2'
+        php-version: '7.3'
         tools: composer, prestissimo
         extensions: ast
         coverage: none

.github/workflows/ide.yml

@@ -15,8 +15,8 @@ jobs:
     - name: Set up PHP
       uses: shivammathur/setup-php@v2
       with:
-        php-version: '7.2'
-        tools: composer, prestissimo, pecl
+        php-version: '7.3'
+        tools: composer:v1, prestissimo, pecl
         coverage: none
 
     - name: Install AST extension
@@ -54,7 +54,7 @@ jobs:
     - name: Install Magento
       id: install-magento
       run: |
-        composer create-project magento/community-edition=2.3.2 magento
+        composer create-project magento/community-edition=2.4.1 magento
         cd magento
         composer config minimum-stability dev
         composer config prefer-stable true

.github/workflows/phan.yml

@@ -15,7 +15,7 @@ jobs:
     - name: Set up PHP
       uses: shivammathur/setup-php@v2
       with:
-        php-version: '7.2'
+        php-version: '7.3'
         tools: composer:v1, prestissimo, pecl
         coverage: none
         extensions: ast
@@ -51,7 +51,7 @@ jobs:
     - name: Install Magento
       id: install-magento
       run: |
-        composer create-project magento/community-edition=2.3.2 magento
+        composer create-project magento/community-edition=2.4.1 magento
         cd magento
         composer config minimum-stability dev
         composer config prefer-stable true

CHANGELOG.md

@@ -1,75 +1,79 @@
 All notable changes to this project will be documented in this file. This project adheres to Semantic Versioning.
 
-# 5.1.2
+### 5.2.0
+* Remove synchronous product indexing
+* Improve queue consumer message
+
+### 5.1.2
 * Update PHP-SDK dependency version for better http exception logging
 
-# 5.1.1
+### 5.1.1
 * Check that order payment is instance of payment interface
 
-# 5.1.0
+### 5.1.0
 * Fix addMultipleProductsToCart issue happening in M2 cloud
 
-# 5.0.8
+### 5.0.8
 * Bump dependencies to be compatible with Nosto CMP module  
 
-# 5.0.7
+### 5.0.7
 * User serializer provided by the PHP SDK to keep the module compatible with Magento EQP  
 
-# 5.0.6
+### 5.0.6
 * Add Content Security Policy (CSP) whitelist 
 
-# 5.0.5
+### 5.0.5
 * Fix an issue where if deleted user with ID 1, the indexer will throw foreign keys constraints errors
 
-# 5.0.4
+### 5.0.4
 * Fix an issue with incorrect prices when different base currencies are used in websites and taxes are included in display prices   
 
-# 5.0.3
+### 5.0.3
 * Bump the PHP SDK version to be compatible with Nosto CMP module (no functional changes)
 
-# 5.0.2
+### 5.0.2
 * Fix an issue where custom tags (tag1) were overridden by default tags
 
-# 5.0.1
+### 5.0.1
 * Fix an issue with configurable product prices being zero in Nosto product data when taxes are included in display prices
 
-# 5.0.0
+### 5.0.0
 * Refactor the indexing logic to use batched queues & decouple the caching logic from product updates
 * Use Magento's built-in caching logic for caching Nosto product data 
 * Add google category as customisable attribute
 * Change the namespaces to comply with PHP SDK 5.0.0
 * Add check for an empty array before trying to get min price for bundled product price
 * Remove mview subscription / trigger to catalog_product_entity_media_gallery
 
-# 4.0.9
+### 4.0.9
 * Fix an issue with configurable product prices not being set when using MSI  
 
-# 4.0.8
+### 4.0.8
 * Add null guard for caching product service in case the product data building fails for dirty product
 
-# 4.0.7
+### 4.0.7
 * Handle empty / invalid product cache entries and possible failures in product data building gracefully
 
-# 4.0.6
+### 4.0.6
 * Fix issue with non-generated proxy classes during di compilation 
 
-# 4.0.5
+### 4.0.5
 * Fix an issue where product cache table was not created during upgrade
 
-# 4.0.4
+### 4.0.4
 * Store cached Nosto product data as a base64 encoded string in database to avoid problems with character sets and collations
 * Alter the type of cached product to be longtext to allow saving large product data sets
 
-# 4.0.3
+### 4.0.3
 * `setup:upgrade` for customer now saves only customer reference instead of entire customer object
 
-# 4.0.2
+### 4.0.2
 * Fix an issue where setup:upgrade could crash if customer migration is faulty
 
-# 4.0.1
+### 4.0.1
 * Make the new order detection more fault tolerant by comparing also updated at and created at timestamps
 
-# 4.0.0
+### 4.0.0
 **New features (performance improvements)**
 * Introduce cache for Nosto product data to speedup the product tagging added to the product pages
 * Introduce Nosto product data change detection to avoid redundant API calls to Nosto

Model/Service/Sync/AbstractBulkConsumer.php

@@ -39,6 +39,7 @@
 
 use Exception;
 use InvalidArgumentException;
+use Magento\AsynchronousOperations\Api\Data\OperationInterface;
 use Magento\Framework\EntityManager\EntityManager;
 use Magento\Framework\Json\Helper\Data as JsonHelper;
 use Nosto\Tagging\Logger\Logger;
@@ -73,48 +74,40 @@ public function __construct(
     /**
      * Processing operation for product sync
      *
-     * @param array|\Magento\AsynchronousOperations\Api\Data\OperationInterface $operation
+     * @param OperationInterface $operation
      * @return void
      * @throws Exception
      * @suppress PhanUndeclaredClassConstant
-     * @noinspection PhpFullyQualifiedNameUsageInspection
      */
     public function processOperation($operation)
     {
         $errorCode = null;
-        if (is_array($operation)) {
-            $serializedData = $operation['data']['serialized_data'];
-        } elseif ($operation instanceof \Magento\AsynchronousOperations\Api\Data\OperationInterface) {
+        if ($operation instanceof OperationInterface) {
             $serializedData = $operation->getSerializedData();
         } else {
             throw new InvalidArgumentException(
-                'Wrong type passed to AsyncBulkConsumer::processOperation. 
-                Expected array|\Magento\AsynchronousOperations\Api\Data\OperationInterface.'
+                'Wrong type passed to AsyncBulkConsumer::processOperation.
+                Expected \Magento\AsynchronousOperations\Api\Data\OperationInterface.'
             );
         }
         $unserializedData = $this->jsonHelper->jsonDecode($serializedData);
         $productIds = $unserializedData['product_ids'];
         $storeId = $unserializedData['store_id'];
         try {
             $this->doOperation($productIds, $storeId);
-            if (!is_array($operation)) {
-                /** @phan-suppress-next-line PhanTypeMismatchArgument */
-                $message = __('Something went wrong when syncing products to Nosto. Check log for details.');
-                $operation->setStatus(
-                    \Magento\AsynchronousOperations\Api\Data\OperationInterface::STATUS_TYPE_COMPLETE
-                )->setResultMessage($message);
-                $this->entityManager->save($operation);
-            }
+            /** @phan-suppress-next-line PhanTypeMismatchArgument */
+            $message = __('Success.');
+            $operation->setStatus(OperationInterface::STATUS_TYPE_COMPLETE)
+                ->setResultMessage($message);
+            $this->entityManager->save($operation);
         } catch (Exception $e) {
-            $this->logger->critical($e->getMessage());
+            $this->logger->critical(sprintf('Bulk uuid: %s. %s', $operation->getBulkUuid(), $e->getMessage()));
             /** @phan-suppress-next-line PhanTypeMismatchArgument */
             $message = __('Something went wrong when syncing products to Nosto. Check log for details.');
-            if (!is_array($operation)) {
-                $operation->setStatus(
-                    \Magento\AsynchronousOperations\Api\Data\OperationInterface::STATUS_TYPE_NOT_RETRIABLY_FAILED
-                )->setErrorCode($e->getCode())
-                    ->setResultMessage($message);
-            }
+            $operation->setStatus(OperationInterface::STATUS_TYPE_NOT_RETRIABLY_FAILED)
+                ->setErrorCode($e->getCode())
+                ->setResultMessage($message);
+            $this->entityManager->save($operation);
         }
     }
 

Model/Service/Sync/AbstractBulkPublisher.php

@@ -67,6 +67,9 @@ abstract class AbstractBulkPublisher implements BulkPublisherInterface
     /** @var Manager */
     private $manager;
 
+    /** @var Logger */
+    private $logger;
+
     /**
      * AbstractBulkPublisher constructor.
      * @param IdentityGeneratorInterface $identityService
@@ -86,6 +89,7 @@ public function __construct(// @codingStandardsIgnoreLine
         $this->serializer = $serializer;
         $this->asyncBulkConsumer = $asyncBulkConsumer;
         $this->manager = $manager;
+        $this->logger = $logger;
         try {
             $this->bulkManagement = ObjectManager::getInstance()
                 ->get(\Magento\Framework\Bulk\BulkManagementInterface::class);
@@ -117,6 +121,13 @@ private function publishCollectionToQueue(
         $storeId,
         $productIds
     ) {
+
+        if (!$this->canUseAsyncOperations()) {
+            $this->logger->critical(
+                "Module Magento_AsynchronousOperations not available. Aborting bulk publish operation"
+            );
+            return;
+        }
         $productIdsChunks = array_chunk($productIds, $this->getBulkSize());
         $bulkUuid = $this->identityService->generateId();
         /** @phan-suppress-next-line PhanTypeMismatchArgument */
@@ -129,29 +140,23 @@ private function publishCollectionToQueue(
                 $bulkUuid
             );
         }
-        if ($this->canUseAsyncOperations()) {
-            $operations = [];
-            foreach ($operationsData as $operationData) {
-                $operations[] = $this->operationFactory->create($operationData);
-            }
-            if (empty($operations)) {
-                return;
-            }
-            $result = $this->bulkManagement->scheduleBulk(
-                $bulkUuid,
-                $operations,
-                $bulkDescription
-            );
-            if (!$result) {
-                /** @phan-suppress-next-line PhanTypeMismatchArgument */
-                throw new LocalizedException(__('Something went wrong while processing the request.'));
-            }
-        } else {
-            foreach ($operationsData as $operationData) {
-                $this->asyncBulkConsumer->processOperation($operationData);
-            }
+
+        $operations = [];
+        foreach ($operationsData as $operationData) {
+            $operations[] = $this->operationFactory->create($operationData);
+        }
+        if (empty($operations)) {
             return;
         }
+        $result = $this->bulkManagement->scheduleBulk(
+            $bulkUuid,
+            $operations,
+            $bulkDescription
+        );
+        if (!$result) {
+            /** @phan-suppress-next-line PhanTypeMismatchArgument */
+            throw new LocalizedException(__('Something went wrong while processing the request.'));
+        }
     }
 
     /**

Model/Service/Sync/BulkConsumerInterface.php

@@ -44,7 +44,7 @@ interface BulkConsumerInterface
     /**
      * Processing operation for product sync
      *
-     * @param array|OperationInterface $operation
+     * @param OperationInterface $operation
      * @return void
      * @throws Exception
      */

Model/Service/Sync/Upsert/SyncService.php

@@ -42,6 +42,7 @@
 use Nosto\Exception\MemoryOutOfBoundsException;
 use Nosto\NostoException;
 use Nosto\Operation\UpsertProduct;
+use Nosto\Request\Http\Exception\AbstractHttpException;
 use Nosto\Tagging\Helper\Account as NostoHelperAccount;
 use Nosto\Tagging\Helper\Data as NostoDataHelper;
 use Nosto\Tagging\Helper\Url as NostoHelperUrl;
@@ -114,6 +115,8 @@ public function __construct(
      * @param Store $store
      * @throws MemoryOutOfBoundsException
      * @throws NostoException
+     * @throws AbstractHttpException
+     * @throws Exception
      */
     public function syncProducts(ProductCollection $collection, Store $store)
     {
@@ -143,29 +146,22 @@ public function syncProducts(ProductCollection $collection, Store $store)
                 if ($nostoProduct === null) {
                     throw new NostoException('Could not get product from the product service.');
                 }
-                try {
-                    $op->addProduct($nostoProduct);
-                    // phpcs:ignore
-                    $this->cacheService->save($nostoProduct, $store);
-                    $this->tickBenchmark(self::BENCHMARK_SYNC_NAME);
-                } catch (Exception $e) {
-                    $this->getLogger()->exception($e);
-                }
-            }
-            try {
-                $this->logDebugWithStore(
-                    sprintf(
-                        'Upserting batch of %d (%s) - API timeout is set to %d seconds',
-                        $this->apiBatchSize,
-                        implode(',', $productIdsInBatch),
-                        $this->apiTimeout
-                    ),
-                    $store
-                );
-                $op->upsert();
-            } catch (Exception $upsertException) {
-                $this->getLogger()->exception($upsertException);
+                $op->addProduct($nostoProduct);
+                // phpcs:ignore
+                $this->cacheService->save($nostoProduct, $store);
+                $this->tickBenchmark(self::BENCHMARK_SYNC_NAME);
             }
+
+            $this->logDebugWithStore(
+                sprintf(
+                    'Upserting batch of %d (%s) - API timeout is set to %d seconds',
+                    $this->apiBatchSize,
+                    implode(',', $productIdsInBatch),
+                    $this->apiTimeout
+                ),
+                $store
+            );
+            $op->upsert();
         }
         $this->logBenchmarkSummary(self::BENCHMARK_SYNC_NAME, $store, $this);
     }