Merge pull request #1185 from magento-troll/MAGETWO-67087

[Troll] MAGETWO-67087: Saving category deletes url-rewrites for products in anchor categories

app/code/Magento/CatalogUrlRewrite/Model/Product/AnchorUrlRewriteGenerator.php

@@ -41,7 +41,7 @@ public function __construct(
     }
 
     /**
-     * Generate list based on categories
+     * Generate product rewrites for anchor categories
      *
      * @param int $storeId
      * @param Product $product

app/code/Magento/CatalogUrlRewrite/Model/Product/CanonicalUrlRewriteGenerator.php

@@ -30,7 +30,7 @@ public function __construct(ProductUrlPathGenerator $productUrlPathGenerator, Ur
     }
 
     /**
-     * Generate list based on store view
+     * Generate product rewrites without categories
      *
      * @param int $storeId
      * @param Product $product

app/code/Magento/CatalogUrlRewrite/Model/Product/CategoriesUrlRewriteGenerator.php

@@ -31,7 +31,7 @@ public function __construct(ProductUrlPathGenerator $productUrlPathGenerator, Ur
     }
 
     /**
-     * Generate list based on categories
+     * Generate product rewrites with categories
      *
      * @param int $storeId
      * @param Product $product

app/code/Magento/CatalogUrlRewrite/Model/Product/CurrentUrlRewritesRegenerator.php

@@ -6,6 +6,7 @@
 namespace Magento\CatalogUrlRewrite\Model\Product;
 
 use Magento\Catalog\Model\Category;
+use Magento\Catalog\Model\CategoryRepository;
 use Magento\Catalog\Model\Product;
 use Magento\UrlRewrite\Service\V1\Data\UrlRewrite;
 use Magento\UrlRewrite\Model\OptionProvider;
@@ -56,19 +57,24 @@ class CurrentUrlRewritesRegenerator
     /** @var \Magento\UrlRewrite\Model\MergeDataProvider */
     private $mergeDataProviderPrototype;
 
+    /** @var CategoryRepository */
+    private $categoryRepository;
+
     /**
      * @param UrlFinderInterface $urlFinder
      * @param ProductUrlPathGenerator $productUrlPathGenerator
      * @param UrlRewriteFactory $urlRewriteFactory
      * @param UrlRewriteFinder|null $urlRewriteFinder
      * @param \Magento\UrlRewrite\Model\MergeDataProviderFactory|null $mergeDataProviderFactory
+     * @param CategoryRepository|null $categoryRepository
      */
     public function __construct(
         UrlFinderInterface $urlFinder,
         ProductUrlPathGenerator $productUrlPathGenerator,
         UrlRewriteFactory $urlRewriteFactory,
         UrlRewriteFinder $urlRewriteFinder = null,
-        MergeDataProviderFactory $mergeDataProviderFactory = null
+        MergeDataProviderFactory $mergeDataProviderFactory = null,
+        CategoryRepository $categoryRepository = null
     ) {
         $this->urlFinder = $urlFinder;
         $this->productUrlPathGenerator = $productUrlPathGenerator;
@@ -78,11 +84,12 @@ public function __construct(
         if (!isset($mergeDataProviderFactory)) {
             $mergeDataProviderFactory = ObjectManager::getInstance()->get(MergeDataProviderFactory::class);
         }
+        $this->categoryRepository = $categoryRepository ?: ObjectManager::getInstance()->get(CategoryRepository::class);
         $this->mergeDataProviderPrototype = $mergeDataProviderFactory->create();
     }
 
     /**
-     * Generate list based on current rewrites
+     * Generate product rewrites based on current rewrites without anchor categories
      *
      * @param int $storeId
      * @param Product $product
@@ -115,6 +122,52 @@ public function generate($storeId, Product $product, ObjectRegistry $productCate
         return $mergeDataProvider->getData();
     }
 
+    /**
+     * Generate product rewrites for anchor categories based on current rewrites
+     *
+     * @param int $storeId
+     * @param Product $product
+     * @param ObjectRegistry $productCategories
+     * @param int|null $rootCategoryId
+     * @return UrlRewrite[]
+     */
+    public function generateAnchor(
+        $storeId,
+        Product $product,
+        ObjectRegistry $productCategories,
+        $rootCategoryId = null
+    ) {
+        $anchorCategoryIds = [];
+        $mergeDataProvider = clone $this->mergeDataProviderPrototype;
+
+        $currentUrlRewrites = $this->urlRewriteFinder->findAllByData(
+            $product->getEntityId(),
+            $storeId,
+            ProductUrlRewriteGenerator::ENTITY_TYPE,
+            $rootCategoryId
+        );
+
+        foreach ($productCategories->getList() as $productCategory) {
+            $anchorCategoryIds = array_merge($productCategory->getAnchorsAbove(), $anchorCategoryIds);
+        }
+
+        foreach ($currentUrlRewrites as $currentUrlRewrite) {
+            $metadata = $currentUrlRewrite->getMetadata();
+            if (isset($metadata['category_id']) && $metadata['category_id'] > 0) {
+                $category = $this->categoryRepository->get($metadata['category_id'], $storeId);
+                if (in_array($category->getId(), $anchorCategoryIds)) {
+                    $mergeDataProvider->merge(
+                        $currentUrlRewrite->getIsAutogenerated()
+                        ? $this->generateForAutogenerated($currentUrlRewrite, $storeId, $category, $product)
+                        : $this->generateForCustom($currentUrlRewrite, $storeId, $category, $product)
+                    );
+                }
+            }
+        }
+
+        return $mergeDataProvider->getData();
+    }
+
     /**
      * @param UrlRewrite $url
      * @param int $storeId

app/code/Magento/CatalogUrlRewrite/Model/ProductScopeRewriteGenerator.php

@@ -171,7 +171,14 @@ public function generateForSpecificStoreView($storeId, $productCategories, Produ
         $mergeDataProvider->merge(
             $this->anchorUrlRewriteGenerator->generate($storeId, $product, $productCategories)
         );
-
+        $mergeDataProvider->merge(
+            $this->currentUrlRewritesRegenerator->generateAnchor(
+                $storeId,
+                $product,
+                $productCategories,
+                $rootCategoryId
+            )
+        );
         return $mergeDataProvider->getData();
     }
 

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

@@ -80,11 +80,11 @@ public function __construct(
     public function generateProductUrlRewrites(\Magento\Catalog\Model\Category $category)
     {
         $mergeDataProvider = clone $this->mergeDataProviderPrototype;
-        $this->isSkippedProduct = [];
+        $this->isSkippedProduct[$category->getEntityId()] = [];
         $saveRewriteHistory = $category->getData('save_rewrites_history');
         $storeId = $category->getStoreId();
         if ($category->getAffectedProductIds()) {
-            $this->isSkippedProduct = $category->getAffectedProductIds();
+            $this->isSkippedProduct[$category->getEntityId()] = $category->getAffectedProductIds();
             $collection = $this->productCollectionFactory->create()
                 ->setStoreId($storeId)
                 ->addIdFilter($category->getAffectedProductIds())
@@ -137,17 +137,25 @@ public function getCategoryProductsUrlRewrites(
         $rootCategoryId = null
     ) {
         $mergeDataProvider = clone $this->mergeDataProviderPrototype;
+
         /** @var \Magento\Catalog\Model\ResourceModel\Product\Collection $productCollection */
-        $productCollection = $category->getProductCollection()
+        $productCollection = $this->productCollectionFactory->create();
+
+        $productCollection->addCategoriesFilter(['eq' => [$category->getEntityId()]])
+            ->setStoreId($storeId)
             ->addAttributeToSelect('name')
             ->addAttributeToSelect('visibility')
             ->addAttributeToSelect('url_key')
             ->addAttributeToSelect('url_path');
+
         foreach ($productCollection as $product) {
-            if (in_array($product->getId(), $this->isSkippedProduct)) {
+            if (
+                isset($this->isSkippedProduct[$category->getEntityId()]) &&
+                in_array($product->getId(), $this->isSkippedProduct[$category->getEntityId()])
+            ) {
                 continue;
             }
-            $this->isSkippedProduct[] = $product->getId();
+            $this->isSkippedProduct[$category->getEntityId()][] = $product->getId();
             $product->setStoreId($storeId);
             $product->setData('save_rewrites_history', $saveRewriteHistory);
             $mergeDataProvider->merge(

app/code/Magento/CatalogUrlRewrite/Test/Unit/Model/ProductScopeRewriteGeneratorTest.php

@@ -137,6 +137,8 @@ public function testGenerationForGlobalScope()
             ->setStoreId(3);
         $this->currentUrlRewritesRegenerator->expects($this->any())->method('generate')
             ->will($this->returnValue([$current]));
+        $this->currentUrlRewritesRegenerator->expects($this->any())->method('generateAnchor')
+            ->will($this->returnValue([$current]));
         $anchorCategories = new \Magento\UrlRewrite\Service\V1\Data\UrlRewrite([], $this->serializer);
         $anchorCategories->setRequestPath('category-4')
             ->setStoreId(4);
@@ -178,6 +180,8 @@ public function testGenerationForSpecificStore()
             ->will($this->returnValue([]));
         $this->currentUrlRewritesRegenerator->expects($this->any())->method('generate')
             ->will($this->returnValue([]));
+        $this->currentUrlRewritesRegenerator->expects($this->any())->method('generateAnchor')
+            ->will($this->returnValue([]));
         $this->anchorUrlRewriteGenerator->expects($this->any())->method('generate')
             ->will($this->returnValue([]));
 

dev/tests/integration/testsuite/Magento/CatalogUrlRewrite/Model/CategoryUrlRewriteGeneratorTest.php

@@ -10,6 +10,7 @@
 
 use Magento\UrlRewrite\Model\OptionProvider;
 use Magento\UrlRewrite\Service\V1\Data\UrlRewrite;
+use Magento\CatalogUrlRewrite\Model\ProductUrlRewriteGenerator;
 
 /**
  * @magentoAppArea adminhtml
@@ -25,7 +26,7 @@ protected function setUp()
     }
 
     /**
-     * @magentoDataFixture Magento/CatalogUrlRewrite/_files/categories.php
+     * @magentoDataFixture Magento/CatalogUrlRewrite/_files/categories_with_products.php
      * @magentoDbIsolation enabled
      * @magentoAppIsolation enabled
      */
@@ -50,11 +51,56 @@ public function testGenerateUrlRewritesWithoutSaveHistory()
         ];
 
         $this->assertResults($categoryExpectedResult, $actualResults);
+
+        /** @var \Magento\Catalog\Model\ProductRepository $productRepository */
+        $productRepository = $this->objectManager->create(\Magento\Catalog\Model\ProductRepository::class);
+        $product = $productRepository->get('12345');
+        $productForTest = $product->getId();
+
+        $productFilter = [
+            UrlRewrite::ENTITY_TYPE => ProductUrlRewriteGenerator::ENTITY_TYPE,
+            UrlRewrite::ENTITY_ID => [$productForTest]
+        ];
+        $actualResults = $this->getActualResults($productFilter);
+        $productExpectedResult = [
+            [
+                'simple-product-two.html',
+                'catalog/product/view/id/' . $productForTest,
+                1,
+                0
+            ],
+            [
+                'new-url/category-1-1/category-1-1-1/simple-product-two.html',
+                'catalog/product/view/id/' . $productForTest . '/category/5',
+                1,
+                0
+            ],
+            [
+                'new-url/simple-product-two.html',
+                'catalog/product/view/id/' . $productForTest . '/category/3',
+                1,
+                0
+            ],
+            [
+                'new-url/category-1-1/simple-product-two.html',
+                'catalog/product/view/id/' . $productForTest . '/category/4',
+                1,
+                0
+            ],
+            [
+                '/simple-product-two.html',
+                'catalog/product/view/id/' . $productForTest . '/category/2',
+                1,
+                0
+            ]
+        ];
+
+        $this->assertResults($productExpectedResult, $actualResults);
     }
 
     /**
      * @magentoDbIsolation enabled
-     * @magentoDataFixture Magento/CatalogUrlRewrite/_files/categories.php
+     * @magentoDataFixture Magento/CatalogUrlRewrite/_files/categories_with_products.php
      * @magentoAppIsolation enabled
      */
     public function testGenerateUrlRewritesWithSaveHistory()
@@ -86,6 +132,69 @@ public function testGenerateUrlRewritesWithSaveHistory()
         ];
 
         $this->assertResults($categoryExpectedResult, $actualResults);
+
+        /** @var \Magento\Catalog\Model\ProductRepository $productRepository */
+        $productRepository = $this->objectManager->create(\Magento\Catalog\Model\ProductRepository::class);
+        $product = $productRepository->get('12345');
+        $productForTest = $product->getId();
+
+        $productFilter = [
+            UrlRewrite::ENTITY_TYPE => ProductUrlRewriteGenerator::ENTITY_TYPE,
+            UrlRewrite::ENTITY_ID => [$productForTest]
+        ];
+        $actualResults = $this->getActualResults($productFilter);
+        $productExpectedResult = [
+            [
+                'simple-product-two.html',
+                'catalog/product/view/id/' . $productForTest,
+                1,
+                0
+            ],
+            [
+                'new-url/category-1-1/category-1-1-1/simple-product-two.html',
+                'catalog/product/view/id/' . $productForTest . '/category/5',
+                1,
+                0
+            ],
+            [
+                'category-1/category-1-1/category-1-1-1/simple-product-two.html',
+                'new-url/category-1-1/category-1-1-1/simple-product-two.html',
+                0,
+                OptionProvider::PERMANENT
+            ],
+            [
+                'new-url/simple-product-two.html',
+                'catalog/product/view/id/' . $productForTest . '/category/3',
+                1,
+                0
+            ],
+            [
+                'new-url/category-1-1/simple-product-two.html',
+                'catalog/product/view/id/' . $productForTest . '/category/4',
+                1,
+                0
+            ],
+            [
+                '/simple-product-two.html',
+                'catalog/product/view/id/' . $productForTest . '/category/2',
+                1,
+                0
+            ],
+            [
+                'category-1/simple-product-two.html',
+                'new-url/simple-product-two.html',
+                0,
+                OptionProvider::PERMANENT
+            ],
+            [
+                'category-1/category-1-1/simple-product-two.html',
+                'new-url/category-1-1/simple-product-two.html',
+                0,
+                OptionProvider::PERMANENT
+            ],
+        ];
+
+        $this->assertResults($productExpectedResult, $actualResults);
     }
 
     /**
@@ -145,6 +254,7 @@ protected function getActualResults(array $filter)
      */
     protected function assertResults($expected, $actual)
     {
+        $this->assertEquals(count($expected), count($actual), 'Number of rewrites does not match');
         foreach ($expected as $row) {
             $this->assertContains(
                 $row,