From 58a2c8fb01b79224e5b2b4eac9bb4247857b29bd Mon Sep 17 00:00:00 2001 From: Lachlan Turner Date: Fri, 15 Nov 2024 16:32:21 +1030 Subject: [PATCH 1/2] Use random suffix for temp table to avoid conflicts Also use table instead of temporary table --- src/index/Model/Changelog/InsertRecords.php | 42 ++++++++----- src/index/Model/Changelog/TempTable.php | 65 +++++++++++++++++++-- src/index/Model/ProductIndexer.php | 23 ++++---- 3 files changed, 100 insertions(+), 30 deletions(-) diff --git a/src/index/Model/Changelog/InsertRecords.php b/src/index/Model/Changelog/InsertRecords.php index feae4c6..f250287 100644 --- a/src/index/Model/Changelog/InsertRecords.php +++ b/src/index/Model/Changelog/InsertRecords.php @@ -6,16 +6,19 @@ use Aligent\FredhopperIndexer\Model\DataHandler; use Aligent\FredhopperIndexer\Model\ResourceModel\Changelog as ChangelogResource; use Magento\Framework\App\ResourceConnection; +use Magento\Framework\Exception\LocalizedException; class InsertRecords { /** * @param ResourceConnection $resourceConnection * @param ChangelogResource $changelogResource + * @param TempTable $tempTable */ public function __construct( private readonly ResourceConnection $resourceConnection, - private readonly ChangelogResource $changelogResource + private readonly ChangelogResource $changelogResource, + private readonly TempTable $tempTable ) { } @@ -24,30 +27,36 @@ public function __construct( * * @return void * @throws \Zend_Db_Select_Exception + * @throws LocalizedException */ public function execute(): void { + $tempTableName = $this->tempTable->getTempTableName(); $addedProductIds = $this->getAddedOrDeletedProductsByType( true, - DataHandler::TYPE_PRODUCT + DataHandler::TYPE_PRODUCT, + $tempTableName ); $addedVariantIds = $this->getAddedOrDeletedProductsByType( true, - DataHandler::TYPE_VARIANT + DataHandler::TYPE_VARIANT, + $tempTableName ); $this->changelogResource->insertAdditionOperations($addedProductIds, $addedVariantIds); - $updatedProductIds = $this->getUpdatedProductsByType(DataHandler::TYPE_PRODUCT); - $updatedVariantIds = $this->getUpdatedProductsByType(DataHandler::TYPE_VARIANT); + $updatedProductIds = $this->getUpdatedProductsByType(DataHandler::TYPE_PRODUCT, $tempTableName); + $updatedVariantIds = $this->getUpdatedProductsByType(DataHandler::TYPE_VARIANT, $tempTableName); $this->changelogResource->insertUpdateOperations($updatedProductIds, $updatedVariantIds); $deletedProductIds = $this->getAddedOrDeletedProductsByType( false, - DataHandler::TYPE_PRODUCT + DataHandler::TYPE_PRODUCT, + $tempTableName ); $deletedVariantIds = $this->getAddedOrDeletedProductsByType( false, - DataHandler::TYPE_VARIANT + DataHandler::TYPE_VARIANT, + $tempTableName ); $this->changelogResource->insertDeleteOperations($deletedProductIds, $deletedVariantIds); } @@ -57,21 +66,23 @@ public function execute(): void * * @param bool $isAddition * @param string $productType + * @param string $tempTableName * @return array */ private function getAddedOrDeletedProductsByType( bool $isAddition, - string $productType + string $productType, + string $tempTableName ): array { $connection = $this->resourceConnection->getConnection(); $select = $connection->select(); $select->from( - ['main_table' => ($isAddition ? DataHandler::INDEX_TABLE_NAME : TempTable::TEMP_TABLE_NAME)], + ['main_table' => ($isAddition ? DataHandler::INDEX_TABLE_NAME : $tempTableName)], ['product_id'] ); $select->joinLeft( - ['temp_table' => ($isAddition ? TempTable::TEMP_TABLE_NAME : DataHandler::INDEX_TABLE_NAME)], + ['temp_table' => ($isAddition ? $tempTableName : DataHandler::INDEX_TABLE_NAME)], 'temp_table.product_id = main_table.product_id AND '. 'temp_table.product_type = main_table.product_type', [] @@ -87,16 +98,17 @@ private function getAddedOrDeletedProductsByType( * Determine which products have been updated between the main and temporary table * * @param string $productType + * @param string $tempTableName * @return array * @throws \Zend_Db_Select_Exception */ - private function getUpdatedProductsByType(string $productType): array + private function getUpdatedProductsByType(string $productType, string $tempTableName): array { // get all product ids and variant ids that exist in both tables // we do not want to consider products that are being added or deleted completely $connection = $this->resourceConnection->getConnection(); $existingProductsSelect = $connection->select(); - $existingProductsSelect->from(['temp_table' => TempTable::TEMP_TABLE_NAME], ['product_id']); + $existingProductsSelect->from(['temp_table' => $tempTableName], ['product_id']); $existingProductsSelect->joinInner( ['main_table' => DataHandler::INDEX_TABLE_NAME], 'main_table.product_id = temp_table.product_id AND ' . @@ -114,7 +126,7 @@ private function getUpdatedProductsByType(string $productType): array ['product_id'] ); $existingProductsTempMissingSelect->joinLeft( - ['temp_table' => TempTable::TEMP_TABLE_NAME], + ['temp_table' => $tempTableName], 'main_table.product_id = temp_table.product_id AND ' . 'main_table.product_type = temp_table.product_type AND ' . 'main_table.store_id = temp_table.store_id', @@ -127,7 +139,7 @@ private function getUpdatedProductsByType(string $productType): array // records that are in the old table, but not in the new table $existingProductsMainMissingSelect = $connection->select(); $existingProductsMainMissingSelect->from( - ['temp_table' => TempTable::TEMP_TABLE_NAME], + ['temp_table' => $tempTableName], ['product_id'] ); $existingProductsMainMissingSelect->joinLeft( @@ -148,7 +160,7 @@ private function getUpdatedProductsByType(string $productType): array ['product_id'] ); $existingProductsDifferenceSelect->joinInner( - ['temp_table' => TempTable::TEMP_TABLE_NAME], + ['temp_table' => $tempTableName], 'main_table.product_id = temp_table.product_id AND ' . 'main_table.product_type = temp_table.product_type AND ' . 'main_table.store_id = temp_table.store_id AND '. diff --git a/src/index/Model/Changelog/TempTable.php b/src/index/Model/Changelog/TempTable.php index 05a8dfc..b4de087 100644 --- a/src/index/Model/Changelog/TempTable.php +++ b/src/index/Model/Changelog/TempTable.php @@ -5,30 +5,82 @@ use Aligent\FredhopperIndexer\Model\DataHandler; use Magento\Framework\App\ResourceConnection; +use Magento\Framework\Exception\LocalizedException; +use Random\RandomException; class TempTable { - public const string TEMP_TABLE_NAME = DataHandler::INDEX_TABLE_NAME . '_temp'; + private const string TEMP_TABLE_PREFIX = DataHandler::INDEX_TABLE_NAME . '_temp_'; + private string $tempTableName; + + /** + * @param ResourceConnection $resourceConnection + */ public function __construct( private readonly ResourceConnection $resourceConnection ) { } + /** + * Gets the current temporary table name + * + * @return string + * @throws LocalizedException + */ + public function getTempTableName(): string + { + if (!isset($this->tempTableName)) { + throw new LocalizedException(__(__METHOD__ . ': temp table name not set')); + } + return $this->tempTableName; + } + + /** + * Sets the current temporary table to a unique value + * + * @return void + * @throws LocalizedException + */ + public function generateTempTableName(): void + { + if (isset($this->tempTableName)) { + throw new LocalizedException(__(__METHOD__ . ': temp table name already set')); + } + try { + $tempTableName = self::TEMP_TABLE_PREFIX . bin2hex(random_bytes(4)); + } catch (RandomException) { + $tempTableName = uniqid(self::TEMP_TABLE_PREFIX, true); + } + $this->tempTableName = $tempTableName; + } + /** * Creates a temporary copy of the index table for use in generating changelog records * * @return void + * @throws LocalizedException */ public function create(): void { + if (!isset($this->tempTableName)) { + throw new LocalizedException(__(__METHOD__ . ': temp table name not set')); + } $connection = $this->resourceConnection->getConnection(); - $connection->createTemporaryTableLike(self::TEMP_TABLE_NAME, DataHandler::INDEX_TABLE_NAME); + $table = $connection->createTableByDdl(DataHandler::INDEX_TABLE_NAME, $this->tempTableName); + if ($connection->isTableExists($this->tempTableName)) { + throw new LocalizedException(__(__METHOD__ . ': temp table already exists')); + } + try { + $connection->createTable($table); + } catch (\Exception $e) { + throw new LocalizedException(__(__METHOD__ . ': ' . $e->getMessage()), $e); + } $copySelect = $connection->select(); $copySelect->from(DataHandler::INDEX_TABLE_NAME); $copyInsert = $connection->insertFromSelect( $copySelect, - self::TEMP_TABLE_NAME + $this->tempTableName ); $connection->query($copyInsert); } @@ -37,9 +89,14 @@ public function create(): void * Drops the temporary table if it exists * * @return void + * @throws LocalizedException */ public function drop(): void { - $this->resourceConnection->getConnection()->dropTemporaryTable(self::TEMP_TABLE_NAME); + if (!isset($this->tempTableName)) { + throw new LocalizedException(__(__METHOD__ . ': temp table name not set')); + } + $this->resourceConnection->getConnection()->dropTable($this->tempTableName); + unset($this->tempTableName); } } diff --git a/src/index/Model/ProductIndexer.php b/src/index/Model/ProductIndexer.php index 1fb6e17..304edea 100644 --- a/src/index/Model/ProductIndexer.php +++ b/src/index/Model/ProductIndexer.php @@ -70,25 +70,26 @@ public function executeFull(): void /** * @inheritDoc * - * @throws LocalizedException */ public function executeList(array $ids): void { - // create temporary table to handle changelogs - $this->tempTable->create(); - foreach ($this->dimensionProvider->getIterator() as $dimension) { - try { - $this->executeByDimensions($dimension, new \ArrayIterator($ids)); - } catch (FileSystemException|RuntimeException) { - continue; - } - } try { + // create temporary table to handle changelogs + $this->tempTable->generateTempTableName(); + $this->tempTable->create(); + foreach ($this->dimensionProvider->getIterator() as $dimension) { + try { + $this->executeByDimensions($dimension, new \ArrayIterator($ids)); + } catch (FileSystemException|RuntimeException) { + continue; + } + } $this->insertChangelogRecords->execute(); + $this->tempTable->drop(); } catch (\Exception $e) { $this->logger->critical($e->getMessage(), ['exception' => $e]); } - $this->tempTable->drop(); + } /** From 3cf2a537f4fcf206f177ec86dc1a830cfc014f46 Mon Sep 17 00:00:00 2001 From: Lachlan Turner Date: Mon, 18 Nov 2024 08:53:22 +1030 Subject: [PATCH 2/2] Ensure "temporary" table is always dropped after indexing --- src/index/Model/ProductIndexer.php | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/src/index/Model/ProductIndexer.php b/src/index/Model/ProductIndexer.php index 304edea..e4181db 100644 --- a/src/index/Model/ProductIndexer.php +++ b/src/index/Model/ProductIndexer.php @@ -77,15 +77,20 @@ public function executeList(array $ids): void // create temporary table to handle changelogs $this->tempTable->generateTempTableName(); $this->tempTable->create(); - foreach ($this->dimensionProvider->getIterator() as $dimension) { - try { - $this->executeByDimensions($dimension, new \ArrayIterator($ids)); - } catch (FileSystemException|RuntimeException) { - continue; + // try block here is nested to ensure that if the table was created, it gets dropped at the end + try { + foreach ($this->dimensionProvider->getIterator() as $dimension) { + try { + $this->executeByDimensions($dimension, new \ArrayIterator($ids)); + } catch (FileSystemException|RuntimeException) { + continue; + } } + $this->insertChangelogRecords->execute(); + } finally { + // we want to ensure that the "temporary" table is always dropped + $this->tempTable->drop(); } - $this->insertChangelogRecords->execute(); - $this->tempTable->drop(); } catch (\Exception $e) { $this->logger->critical($e->getMessage(), ['exception' => $e]); }