Optimised code.

Processed review comments.

Author: Graham Wharton

app/code/Magento/CatalogUrlRewrite/Observer/ProductProcessUrlRewriteSavingObserver.php

@@ -5,19 +5,19 @@
  */
 namespace Magento\CatalogUrlRewrite\Observer;
 
+use Magento\Catalog\Api\Data\ProductInterface;
 use Magento\Catalog\Model\Product;
-use Magento\Catalog\Model\ProductRepository;
+use Magento\Catalog\Model\Product\Visibility;
+use Magento\Catalog\Model\ResourceModel\Product\Collection;
 use Magento\CatalogUrlRewrite\Model\ProductScopeRewriteGenerator;
 use Magento\CatalogUrlRewrite\Model\ProductUrlPathGenerator;
 use Magento\CatalogUrlRewrite\Model\ProductUrlRewriteGenerator;
 use Magento\Framework\Event\Observer;
-use Magento\Framework\Exception\NoSuchEntityException;
 use Magento\UrlRewrite\Model\Exception\UrlAlreadyExistsException;
 use Magento\UrlRewrite\Model\Storage\DeleteEntitiesFromStores;
 use Magento\UrlRewrite\Model\UrlPersistInterface;
 use Magento\Framework\Event\ObserverInterface;
 use Magento\Store\Model\StoreManagerInterface;
-use Magento\Store\Api\StoreWebsiteRelationInterface;
 
 /**
  * Class ProductProcessUrlRewriteSavingObserver
@@ -29,73 +29,65 @@
 class ProductProcessUrlRewriteSavingObserver implements ObserverInterface
 {
     /**
-     * @var ProductUrlRewriteGenerator $productUrlRewriteGenerator
+     * @var ProductUrlRewriteGenerator
      */
     private $productUrlRewriteGenerator;
 
     /**
-     * @var UrlPersistInterface $urlPersist
+     * @var UrlPersistInterface
      */
     private $urlPersist;
 
     /**
-     * @var ProductUrlPathGenerator $productUrlPathGenerator
+     * @var ProductUrlPathGenerator
      */
     private $productUrlPathGenerator;
 
     /**
-     * @var StoreManagerInterface $storeManager
+     * @var StoreManagerInterface
      */
     private $storeManager;
 
     /**
-     * @var StoreWebsiteRelationInterface $storeWebsiteRelation
+     * @var ProductScopeRewriteGenerator
      */
-    private $storeWebsiteRelation;
-
-    /**
-     * @var ProductRepository $productRepository
-     */
-    private $productRepository;
+    private $productScopeRewriteGenerator;
 
     /**
-     * @var ProductScopeRewriteGenerator $productScopeRewriteGenerator
+     * @var DeleteEntitiesFromStores
      */
-    private $productScopeRewriteGenerator;
+    private $deleteEntitiesFromStores;
 
     /**
-     * @var DeleteEntitiesFromStores $deleteEntitiesFromStores
+     * @var Collection
      */
-    private $deleteEntitiesFromStores;
+    private $productCollection;
 
     /**
      * @param ProductUrlRewriteGenerator $productUrlRewriteGenerator
      * @param UrlPersistInterface $urlPersist
      * @param ProductUrlPathGenerator $productUrlPathGenerator
      * @param StoreManagerInterface $storeManager
-     * @param StoreWebsiteRelationInterface $storeWebsiteRelation
-     * @param ProductRepository $productRepository
      * @param ProductScopeRewriteGenerator $productScopeRewriteGenerator
      * @param DeleteEntitiesFromStores $deleteEntitiesFromStores
+     * @param Collection $productCollection
      */
     public function __construct(
         ProductUrlRewriteGenerator $productUrlRewriteGenerator,
         UrlPersistInterface $urlPersist,
         ProductUrlPathGenerator $productUrlPathGenerator,
         StoreManagerInterface $storeManager,
-        StoreWebsiteRelationInterface $storeWebsiteRelation,
-        ProductRepository $productRepository,
         ProductScopeRewriteGenerator $productScopeRewriteGenerator,
-        DeleteEntitiesFromStores $deleteEntitiesFromStores
+        DeleteEntitiesFromStores $deleteEntitiesFromStores,
+        Collection $productCollection
     ) {
         $this->productUrlRewriteGenerator = $productUrlRewriteGenerator;
         $this->urlPersist = $urlPersist;
         $this->productUrlPathGenerator = $productUrlPathGenerator;
         $this->storeManager = $storeManager;
-        $this->storeWebsiteRelation = $storeWebsiteRelation;
-        $this->productRepository = $productRepository;
         $this->productScopeRewriteGenerator = $productScopeRewriteGenerator;
         $this->deleteEntitiesFromStores = $deleteEntitiesFromStores;
+        $this->productCollection = $productCollection;
     }
 
     /**
@@ -104,8 +96,6 @@ public function __construct(
      * @param Observer $observer
      * @return void
      * @throws UrlAlreadyExistsException
-     * @throws NoSuchEntityException
-     * @SuppressWarnings(PHPMD.CyclomaticComplexity)
      */
     public function execute(Observer $observer)
     {
@@ -125,29 +115,31 @@ public function execute(Observer $observer)
             }
 
             $storeIdsToRemove = [];
+            $productWebsiteMap = array_flip($product->getWebsiteIds());
+            $storeVisibilities = $this->productCollection->getAllAttributeValues(ProductInterface::VISIBILITY);
             if ($this->productScopeRewriteGenerator->isGlobalScope($product->getStoreId())) {
                 //Remove any rewrite URLs for websites the product is not in, or is not visible in. Global Scope.
-                foreach ($this->storeManager->getWebsites() as $website) {
-                    $websiteId = $website->getWebsiteId();
-                    foreach ($this->storeWebsiteRelation->getStoreByWebsiteId($websiteId) as $storeid) {
-                        //Load the product for the store we are processing so we can see if it is visible
-                        $storeProduct = $this->productRepository->getById(
-                            $product->getId(),
-                            false,
-                            $storeid,
-                            true
-                        );
-                        if (!$storeProduct->isVisibleInSiteVisibility() ||
-                            !in_array($websiteId, $product->getWebsiteIds())) {
-                            $storeIdsToRemove[] = $storeid;
-                        };
+                foreach ($this->storeManager->getStores() as $store) {
+                    $websiteId = $store->getWebsiteId();
+                    $storeId = $store->getStoreId();
+                    if (!isset($productWebsiteMap[$websiteId])) {
+                        $storeIdsToRemove[] = $storeId;
+                        continue;
+                    }
+                    //Check the visibility of the product in each store.
+                    if (isset($storeVisibilities[$product->getId()][$storeId])
+                        && ($storeVisibilities[$product->getId()][$storeId] === Visibility::VISIBILITY_NOT_VISIBLE)) {
+                        $storeIdsToRemove[] = $storeId;
                     }
                 }
             } else {
                 //Only remove rewrite for current scope
-                if (!$product->isVisibleInSiteVisibility() ||
-                    !in_array($product->getStoreId(), $product->getStoreIds())) {
-                    $storeIdsToRemove[] = $product->getStoreId();
+                $websiteId = $product->getStore()->getWebsiteId();
+                $storeId = $product->getStoreId();
+                if (!isset($productWebsiteMap[$websiteId]) ||
+                    (isset($storeVisibilities[$product->getId()][$storeId])
+                        && ($storeVisibilities[$product->getId()][$storeId] === Visibility::VISIBILITY_NOT_VISIBLE))) {
+                    $storeIdsToRemove[] = $storeId;
                 }
             }
             if (count($storeIdsToRemove)) {

app/code/Magento/CatalogUrlRewrite/Plugin/ProductProcessUrlRewriteRemovingPlugin.php

@@ -26,27 +26,27 @@
 class ProductProcessUrlRewriteRemovingPlugin
 {
     /**
-     * @var ProductRepositoryInterface $productRepository
+     * @var ProductRepositoryInterface
      */
     private $productRepository;
 
     /**
-     * @var StoreWebsiteRelationInterface $storeWebsiteRelation
+     * @var StoreWebsiteRelationInterface
      */
     private $storeWebsiteRelation;
 
     /**
-     * @var UrlPersistInterface $urlPersist
+     * @var UrlPersistInterface
      */
     private $urlPersist;
 
     /**
-     * @var ProductUrlRewriteGenerator $productUrlRewriteGenerator
+     * @var ProductUrlRewriteGenerator
      */
     private $productUrlRewriteGenerator;
 
     /**
-     * @var DeleteEntitiesFromStores $deleteEntitiesFromStores
+     * @var DeleteEntitiesFromStores
      */
     private $deleteEntitiesFromStores;
 
@@ -108,7 +108,8 @@ public function afterUpdateWebsites(
         }
 
         $storeIdsToRemove = [];
-        // Remove the URLs from websites this product no longer belongs to
+        // Remove the URLs for products in $productIds array
+        // from all stores that belong to websites in $websiteIds array
         if ($type === "remove" && $websiteIds && $productIds) {
             foreach ($websiteIds as $webId) {
                 foreach ($this->storeWebsiteRelation->getStoreByWebsiteId($webId) as $storeid) {