Description
Preconditions (*)
- Magento 2.2.7 but I think it's on all version Magento 2.3-develop branch too
- MySQL 5.7.26
Steps to reproduce (*)
- Not much to test unless you make a code construct like this:
/* @var \Magento\Catalog\Model\ResourceModel\Url $this->catalogUrl */
$this->catalogUrl->getRewriteByProductStore([$productId => $storeId])
Expected result (*)
- It should return an array with product slug like this:
[4256] => Array
(
[store_id] => 9999
[visibility] => 4
[url_rewrite] => lorem-ipsum.html
)
Actual result (*)
- It returns product category URL in some cases:
[4256] => Array
(
[store_id] => 9999
[visibility] => 4
[url_rewrite] => test-category/lorem-ipsum.html
)
It happens because the SQL query from
\Magento\Catalog\Model\ResourceModel\Url::getRewriteByProductStore
doesn't look good.
This is wrong:
)->joinLeft(
['r' => $this->getTable('catalog_url_rewrite_product_category')],
'u.url_rewrite_id = r.url_rewrite_id AND r.category_id is NULL',
[]
);
To get rows from table url_rewrite
not in left joined table catalog_url_rewrite_product_category
we should remove AND r.category_id is NULL
from left join and add it in where as:
AND r.url_rewrite_id IS NULL
where r
is the alias of table url_rewrite
.
Leaving AND r.category_id is NULL'
in join condition will create a result with more than one row. I get all rows for that specific product including rows with product category request_path
.
Further down in function getRewriteByProductStore
it adds to return variable the last row for a product:
$rowSet = $connection->fetchAll($select, $bind);
foreach ($rowSet as $row) {
$result[$row['product_id']] = [
'store_id' => $row['store_id'],
'visibility' => $row['visibility'],
'url_rewrite' => $row['request_path'],
];
}
In my case the last row is product category request_path
and is not product request_path
.
The product request_path
is in the first row from the result set ($rowSet
).
Query done in getRewriteByProductStore
SELECT `i`.`product_id`,
`i`.`store_id`,
`i`.`visibility`,
`u`.`request_path`
FROM `catalog_category_product_index_store22` AS `i`
LEFT JOIN `url_rewrite` AS `u`
ON i.product_id = u.entity_id
AND i.store_id = u.store_id
AND u.entity_type = "product"
LEFT JOIN `catalog_url_rewrite_product_category` AS `r`
ON u.url_rewrite_id = r.url_rewrite_id
AND r.category_id IS NULL
WHERE (( i.product_id = 4256
AND i.store_id = 22
AND i.category_id = 2 ));
It returns 3 rows.
Query modied:
SELECT `i`.`product_id`,
`i`.`store_id`,
`i`.`visibility`,
`u`.`request_path`
FROM `catalog_category_product_index_store22` AS `i`
LEFT JOIN `url_rewrite` AS `u`
ON i.product_id = u.entity_id
AND i.store_id = u.store_id
AND u.entity_type = "product"
LEFT JOIN `catalog_url_rewrite_product_category` AS `r`
ON u.url_rewrite_id = r.url_rewrite_id
WHERE (( i.product_id = 4256
AND i.store_id = 22
AND i.category_id = 2 )
AND r.url_rewrite_id IS NULL);
It returns 1 row with product request_path
.
UPDATE
The SQL query should include u.redirect_type=0
also because otherwise it can take row with redirect_type=301
(permanent redirects).
This code
$select = $connection->select()->from(
['i' => $this->tableMaintainer->getMainTable($storeId)],
['product_id', 'store_id', 'visibility']
)->joinLeft(
['u' => $this->getMainTable()],
'i.product_id = u.entity_id AND i.store_id = u.store_id'
. ' AND u.entity_type = "' . ProductUrlRewriteGenerator::ENTITY_TYPE . '"',
['request_path']
)->joinLeft(
['r' => $this->getTable('catalog_url_rewrite_product_category')],
'u.url_rewrite_id = r.url_rewrite_id AND r.category_id is NULL',
[]
);
$bind = [];
foreach ($productIds as $productId) {
$catId = $this->_storeManager->getStore($storeId)->getRootCategoryId();
$productBind = 'product_id' . $productId;
$storeBind = 'store_id' . $storeId;
$catBind = 'category_id' . $catId;
$bindArray = [
'i.product_id = :' . $productBind,
'i.store_id = :' . $storeBind,
'i.category_id = :' . $catBind
];
$cond = '(' . implode(' AND ', $bindArray) . ')';
$bind[$productBind] = $productId;
$bind[$storeBind] = $storeId;
$bind[$catBind] = $catId;
$select->orWhere($cond);
}
$rowSet = $connection->fetchAll($select, $bind);
foreach ($rowSet as $row) {
$result[$row['product_id']] = [
'store_id' => $row['store_id'],
'visibility' => $row['visibility'],
'url_rewrite' => $row['request_path'],
];
}
should be something like:
$select = $connection->select()->from(
['i' => $this->tableMaintainer->getMainTable($storeId)],
['product_id', 'store_id', 'visibility']
)->joinLeft(
['u' => $this->getMainTable()],
'i.product_id = u.entity_id AND i.store_id = u.store_id'
. ' AND u.entity_type = "' . ProductUrlRewriteGenerator::ENTITY_TYPE . '"',
['request_path']
)->joinLeft(
['r' => $this->getTable('catalog_url_rewrite_product_category')],
'u.url_rewrite_id = r.url_rewrite_id',
[]
);
$bind = [];
foreach ($productIds as $productId) {
$catId = $this->_storeManager->getStore($storeId)->getRootCategoryId();
$productBind = 'product_id' . $productId;
$storeBind = 'store_id' . $storeId;
$catBind = 'category_id' . $catId;
$bindArray = [
'i.product_id = :' . $productBind,
'i.store_id = :' . $storeBind,
'i.category_id = :' . $catBind
];
$cond = '(' . implode(' AND ', $bindArray) . ' AND (r.url_rewrite_id IS NULL)) AND u.redirect_type = 0';
$bind[$productBind] = $productId;
$bind[$storeBind] = $storeId;
$bind[$catBind] = $catId;
$select->orWhere($cond);
}
$rowSet = $connection->fetchAll($select, $bind);
foreach ($rowSet as $row) {
$result[$row['product_id']] = [
'store_id' => $row['store_id'],
'visibility' => $row['visibility'],
'url_rewrite' => $row['request_path'],
];
}
Diff (don't copy paste, white-spaces will mess the diff):
Index: ../vendor/magento/module-catalog/Model/ResourceModel/Url.php
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- ../vendor/magento/module-catalog/Model/ResourceModel/Url.php (date 1567697855000)
+++ ../vendor/magento/module-catalog/Model/ResourceModel/Url.php (date 1567697855000)
@@ -680,7 +680,7 @@
['request_path']
)->joinLeft(
['r' => $this->getTable('catalog_url_rewrite_product_category')],
- 'u.url_rewrite_id = r.url_rewrite_id AND r.category_id is NULL',
+ 'u.url_rewrite_id = r.url_rewrite_id',
[]
);
@@ -695,7 +695,7 @@
'i.store_id = :' . $storeBind,
'i.category_id = :' . $catBind
];
- $cond = '(' . implode(' AND ', $bindArray) . ')';
+ $cond = '(' . implode(' AND ', $bindArray) . ' AND (r.url_rewrite_id IS NULL)) AND u.redirect_type = 0';
$bind[$productBind] = $productId;
$bind[$storeBind] = $storeId;
$bind[$catBind] = $catId;