-
Notifications
You must be signed in to change notification settings - Fork 9.4k
URL rewrites not being generated - calls to doReplace() actually insert instead. #17378
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
Hi @simonworkhouse. Thank you for your report.
Please make sure that the issue is reproducible on the vanilla Magento instance following Steps to reproduce. To deploy vanilla Magento instance on our environment, please, add a comment to the issue:
where @simonworkhouse do you confirm that you was able to reproduce the issue on vanilla Magento instance following steps to reproduce?
|
@simonworkhouse, thank you for your report. |
Has a human actually read the bug report? As I stated, this is an intermittent issue that happens when the catalog data is under certain conditions. Unfortunately I cannot simply provide an SQL dump for obvious reasons (privacy issues, etc...) and it could take quite some time for me to generate a CSV (or series of CSVs) that one could use to import into a clean installation that simulates these conditions. This insistence on everything needing to be immediately reproducible from a clean installation is nonsense, particularly when the details of the issue can be clearly seen if one just reads the bug report and reviews the code. Here are a number of bug reports related to this issue: Clearly there's a serious issue with the way that the URL rewrites are generated and updating the Magento\UrlRewrite\Model\Storage\DbStorage::doReplace(...) function to actually do a REPLACE fixes the issue in our instance. Also, the issues with Magento\Framework\DB\Adapter\Pdo\Mysql::insertMultiple(...) need to be noted and someone needs to manually review this. |
@simonworkhouse, we believe that this issue exists. But we cannot add issue to our work backlog and fix it until we have obvious and 100% clear way to reproduce it. |
There is nothing wrong with that, method names should not be mixed up with underlying SQL. Please provide an isolated test case which reproduces a problem on a clean Magento installation. |
I do realise that, but the problem is due to the fact it is actually trying to perform the action of a replace with an insert. There doesn't appear to be any protection to prevent duplicates in the batch update either, so batches will fail if there are ever any duplicates. Additionally, the exception that is supposed to be thrown on duplicates doesn't ever appear to be thrown. MySQL logs indicate that the transaction was rolled back so could it be possible that the exception wasn't thrown due to this? The query that Magento was trying to run generated the MySQL error "Error Code: 1062. Duplicate entry 'some-product-1' for key 'URL_REWRITE_REQUEST_PATH_STORE_ID'" and updating the code to perform a REPLACE fixed the issue for me. Whether or not that's a byproduct of another bug/issue I can't be sure without digging further, but from reviewing the code I would suggest that it was probably intended to perform a replace initially. I will have to spend a bit of time setting up a test case for you, as I can't be certain if it has anything to do with a multi store setup, CSV imports, the category structure, something else or a combination of these. You may have to wait a little while for that as I don't have too much time available at the moment. However; I would have thought that reviewing the code would highlight the issue clearly enough. Is there any reason as to why it's not performing a REPLACE? |
@simonworkhouse because old URL rewrites are removed prior to inserting:
Feel free to reopen issue whenever you find out exact steps. |
Yes, I had noticed that. However; it's not necessarily the conflict with the existing URLs that is the issue, it appears that it might be the data that it's trying to insert that contains the duplicates. Are we able to have this issue re-opened please? |
I believe it's not possible by construction. You can play with your buggy instance to find out if this is the case. To me it looks like you have some orphan records due to improper migration or some other sort of URL rewrite creation and thus they stay in database after |
@orlangur Well it certainly is an issue and isn't limited to our instance, just see the other bug reports. It may not be related to the data going in, it might be the call to $this->deleteOldUrls($urls); not working correctly or possibly something else. Like I said, I was only able to get so far with debugging, located the query that was failing and reported a general gist of the code path. I can assure you that this is definitely an issue, I'm just not certain on the conditions that are required to cause it. I will allocate some time to this, and try to determine the root cause. Please re-open the issue and I will see about providing the data over the weekend. |
@orlangur I have been able to determine the cause of the issues that we were experiencing, but updating the query to a REPLACE will not resolve the core issues with the way that the data in the url_rewrite table is managed. It is my opinion that the whole rewrite system is in need of a bit of an overhaul and needs to be thoroughly tested. Changing the configuration values of "catalog/seo/product_url_suffix", "catalog/seo/category_url_suffix" and "catalog/seo/product_use_categories" can cause serious issues with the URL rewrites and there doesn't appear to be any mechanism to regenerate them. I will report each variation of this issue as a separate bug and also the other bugs that I encountered during testing. |
Is this a Magento 2 instance which is migrated from magento 1? 100% sure this is breaking the magento 2 database in different ways. So if you install a clean magento 2 and import your products without migrating anything else you will see no problems. Best Regards. |
Sires... Do we have any temporary fix for this or band-aid fix? Any workarounds will do. |
What version of magento are you using? If you migrated your data from a magento 1 instance this is not good but normal behavior in magento 2. If these issues occur after export and importing data (not true data migration tool) on a newly fresh Magento 2 instance of the latest version this should be addressed on the other forums. The problem should not be fixed after data migration, no the data migration tool should be checked, repaired or much better rewritten. For the record: Please feel free to answer my questions So again: Best regards. |
Best,
Thank you very much for the information.
I am going to try to get this patch.
Do you have information about the patch, a patch number and file name?
Best regards Bram Slagboom
Adres/Address
Koopjesboom V.O.F.
Lorentzweg 26
2964LN Groot-Ammers
Tel/Phone : 0031184634329
Mobile : 0031651180371
E-Mail : [email protected]
<https://kbmodelcars.com/> https://kbmodelcars.com
<https://ownstock.eu/> https://ownstock.eu
KVK : 58220259
Van: eWolf62095 <[email protected]>
Verzonden: woensdag 24 oktober 2018 11:36
Aan: magento/magento2 <[email protected]>
CC: koopjesboom <[email protected]>; Mention <[email protected]>
Onderwerp: Re: [magento/magento2] URL rewrites not being generated - calls to doReplace() actually insert instead. (#17378)
@koopjesboom <https://github.com/koopjesboom> hi sire, just for an update. Magento Support provided a patch for us. The patch was about "URL Key not updated during product import". The issue is different from what's in this thread however the patch provided also fixed the issue that I experienced same on this thread. Sadly I cannot share you the patch because it is the owned by our company and only our infra team has accessed on the support desk of Magento 2.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub <#17378 (comment)> , or mute the thread <https://github.com/notifications/unsubscribe-auth/AOippDHfpxWYTAsmGP0B-Zc5ps8iaKD_ks5uoDR7gaJpZM4VvxTE> . <https://github.com/notifications/beacon/AOippG3scUJKU9Zx4YDKPk82V8o94Q3Mks5uoDR7gaJpZM4VvxTE.gif>
|
Hi, sir sorry for the late response, |
Came here for the same issue, problem is when you import a product with a sku that is not lowercase. That product will get a new product_id and the whole |
Uh oh!
There was an error while loading. Please reload this page.
We have been experiencing an intermittent issue where entries into the url_rewrite table are not being generated automatically. I have noticed that this issue has been mentioned in the past but I believe that we have isolated the cause.
Magento\UrlRewrite\Model\Storage\DbStorage::doReplace(...) doesn't do a REPLACE but an INSERT, and errors for duplicate entries aren't throwing an AlreadyExistsException as expected.
A general gist of the code path is as follows:
Magento\CatalogUrlRewrite\Model\UrlRewriteBunchReplacer::doBunchReplace(...) ->
Magento\UrlRewrite\Model\Storage\DbStorage::doReplace(...) ->
Magento\Framework\DB\Adapter\Pdo\Mysql::insertMultiple(...) ->
Magento\Framework\DB\Adapter\Pdo\Mysql::insertArray(...)
When we updated the doReplace(...) and insertMultiple(...) functions to pass the replace strategy through to insertArray(...) everything worked as expected. Would I be correct in assuming that this was the intended strategy and that is was just missing from doReplace and insertMultiple? Why would it not be a replace?
Also, any calls to Magento\Framework\DB\Adapter\Pdo\Mysql::insertMultiple(...) will fail if there are duplicates in the data based on unique constraints. This function doesn't take this into account and doesn't have the ability to pass the strategy through to insertArray(...). There also doesn't appear to be an equivalent replaceMultiple(...).
I would guess that this might be causing some other bugs we have noticed, particularly with importing catalog data from a CSV.
Preconditions
Steps to reproduce
Assuming that the category data will fail for a duplicate entry:
Actual result
Received the following MySQL error running the query that Magento genereted:
Error Code: 1062. Duplicate entry 'some-product-1' for key 'URL_REWRITE_REQUEST_PATH_STORE_ID'
An AlreadyExistsException didn't appear to have been thrown, or was silently handled.
Expected result
The correct entries in the url_rewrite table are created.
The text was updated successfully, but these errors were encountered: