From 9fd6c90f038a0488c0fab00e2ce461c8226a2ac8 Mon Sep 17 00:00:00 2001 From: Gerke Date: Wed, 24 Jul 2024 10:40:53 +0200 Subject: [PATCH 1/2] move error handling for duplicated urls to replce URLS method to prevent failing entire store URL regenration --- src/Service/RegenerateProductUrl.php | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/Service/RegenerateProductUrl.php b/src/Service/RegenerateProductUrl.php index d9b53f0..12a1209 100644 --- a/src/Service/RegenerateProductUrl.php +++ b/src/Service/RegenerateProductUrl.php @@ -215,7 +215,17 @@ private function log(string $message): void private function replaceUrls(array &$urls, bool $last = false): int { $this->log(sprintf('replaceUrls%s batch: %d', $last ? ' last' : '', count($urls))); - $this->urlPersist->replace($urls); + + foreach ($urls as $url) { + try { + $this->urlPersist->replace([$url]); + } catch (UrlAlreadyExistsException $e) { + $this->log(sprintf($e->getMessage(). ' Entity id: %d Request path: %s', + $url->getEntityId(), + $url->getRequestPath() + )); + } + } $count = count($urls); $urls = []; From b742038686a029658a55954f4d6d4ce1dbfbad41 Mon Sep 17 00:00:00 2001 From: Gerke Date: Wed, 24 Jul 2024 11:28:07 +0200 Subject: [PATCH 2/2] remove try/catch because error is no longer thrown by replaceUrls method --- src/Service/RegenerateProductUrl.php | 87 ++++++++++------------------ 1 file changed, 30 insertions(+), 57 deletions(-) diff --git a/src/Service/RegenerateProductUrl.php b/src/Service/RegenerateProductUrl.php index 12a1209..6259c43 100644 --- a/src/Service/RegenerateProductUrl.php +++ b/src/Service/RegenerateProductUrl.php @@ -109,56 +109,29 @@ public function execute(?array $productIds = null, ?int $storeId = null, bool $v } $newUrls = []; - try { - /** @var Product $product */ - foreach ($collection as $product) { - if ($verbose) { - $this->log( - sprintf( - 'Regenerating urls for %s (%s) in store (%s)', - $product->getSku(), - $product->getId(), - $store->getName() - ) - ); - } - - $product->setStoreId($store->getId()); - - $newUrls = array_merge($newUrls, $this->urlRewriteGenerator->generate($product)); - if (count($newUrls) >= self::BATCH_SIZE) { - $regeneratedForStore += $this->replaceUrls($newUrls); - } + /** @var Product $product */ + foreach ($collection as $product) { + if ($verbose) { + $this->log( + sprintf( + 'Regenerating urls for %s (%s) in store (%s)', + $product->getSku(), + $product->getId(), + $store->getName() + ) + ); } - if (count($newUrls)) { - $regeneratedForStore += $this->replaceUrls($newUrls, true); + $product->setStoreId($store->getId()); + + $newUrls = array_merge($newUrls, $this->urlRewriteGenerator->generate($product)); + if (count($newUrls) >= self::BATCH_SIZE) { + $regeneratedForStore += $this->replaceUrls($newUrls); } - } catch (\Magento\UrlRewrite\Model\Exception\UrlAlreadyExistsException $e) { - $this->log(sprintf( - 'Couldn\'t insert duplicate URL rewrites for the following ' . - 'products on store ID %d (current batch failed):' . PHP_EOL . '%s', - $store->getId(), - implode(PHP_EOL, array_map(function ($url) { - return sprintf( - '- Product ID: %d, request path: %s', - $url['entity_id'], - $url['request_path'] - ); - }, $e->getUrls())) - )); - } catch (Exception $e) { - $this->log( - sprintf( - 'Duplicated url for store ID %d, product %d (%s) - %s Generated URLs:' . - PHP_EOL . '%s' . PHP_EOL, - $store->getId(), - $product->getId(), - $product->getSku(), - $e->getMessage(), - implode(PHP_EOL, array_keys($newUrls)) - ) - ); + } + + if (count($newUrls)) { + $regeneratedForStore += $this->replaceUrls($newUrls, true); } $this->log( @@ -204,13 +177,10 @@ private function log(string $message): void } /** - * Add product ulrs - * * @param array $urls - * @param bool $last + * @param bool $last * * @return int - * @throws UrlAlreadyExistsException */ private function replaceUrls(array &$urls, bool $last = false): int { @@ -219,11 +189,14 @@ private function replaceUrls(array &$urls, bool $last = false): int foreach ($urls as $url) { try { $this->urlPersist->replace([$url]); - } catch (UrlAlreadyExistsException $e) { - $this->log(sprintf($e->getMessage(). ' Entity id: %d Request path: %s', - $url->getEntityId(), - $url->getRequestPath() - )); + } catch (\Exception $e) { + $this->log( + sprintf( + $e->getMessage() . ' Entity id: %d Request path: %s', + $url->getEntityId(), + $url->getRequestPath() + ) + ); } } $count = count($urls); @@ -243,7 +216,7 @@ private function replaceUrls(array &$urls, bool $last = false): int */ private function deleteUrls(array &$productIds, StoreInterface $store, bool $last = false): void { - $this->log(sprintf('deleteUrls%s batch: %d', $last ? 'last' : '', count($productIds))); + $this->log(sprintf('deleteUrls%s batch: %d', $last ? ' last' : '', count($productIds))); $this->urlPersist->deleteByData([ UrlRewrite::ENTITY_ID => $productIds, UrlRewrite::ENTITY_TYPE => ProductUrlRewriteGenerator::ENTITY_TYPE,