Magento\CatalogImportExport\Model\Import\Product\CategoryProcessor Failed categories aren't removed from the categories returned by upsertCategories

[simonworkhouse]

The function upsertCategories may return a category id that is no longer present in the DB as upsertCategory on line 138 of Magento\CatalogImportExport\Model\Import\Product\CategoryProcessor fetches the category id from the local class cache ($this->categories) of the available categories.
This causes the import to fail if the category was inserted, added to the cache and then removed from the DB due to either a transaction being rolled back or the category being automatically deleted.
Transactions in this instance are probably being rolled back due to url_key conflicts.

Preconditions

1. Magento 2.2.5
2. PHP 7.1.20
3. MySQL 5.7.23

Steps to reproduce

My best guess is that transactions are being rolled back due to url_key conflicts.
Refer to the issue #17586 for clarification on how to set up import data that will have unexpected URL rewrite conflicts.
It would take quite a bit of time to set up a full procedure, but let me know if it's absolutely required.
It should be obvious enough for anyone reviewing the code and referring to the previous bug report #17586

I have implemented a work-around for now, but this needs to be addressed and fixed in the core.

<?php

namespace {my_namespace}\Plugin\Magento\CatalogImportExport\Model\Import\Product;

use Magento\CatalogImportExport\Model\Import\Product\CategoryProcessor;

class CategoryProcessorPlugin
{
    /**
     * // NOTE: Must cache the failed categories here as Magento caches them even if they have failed and as 
     *          a result won't add them to the failed categories list
     * @var array
     */
    protected static $failedCategoriesCache = [];

    /**
     * [afterUpsertCategories description]
     * @param  CategoryProcessor $categoryProcessor [description]
     * @param  [type]            $categoryIds       [description]
     * @return [type]                               [description]
     */
    public function afterUpsertCategories(CategoryProcessor $categoryProcessor, $categoryIds)
    {
        // NOTE: Work-around for a Magento bug where it's still adding failed categories to the list
        if ($failedCategories = $categoryProcessor->getFailedCategories()) {
            foreach ($failedCategories as $failedCategory) {
                self::$failedCategoriesCache[] = $failedCategory['category']->getId();
            }
        }
        $categoryIds = array_diff($categoryIds, self::$failedCategoriesCache);
        return $categoryIds;
    }
}

Expected result

The function upsertCategories should only ever return categories that actually exist.

Actual result

The function upsertCategories returns categories from the local class cache that have since been removed from the DB during the same import run.

[magento-engcom-team]

Hi @simonworkhouse. Thank you for your report.
To help us process this issue please make sure that you provided the following information:

• Summary of the issue
• Information on your environment
• Steps to reproduce
• Expected and actual results

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:

@magento-engcom-team give me {$VERSION} instance

where {$VERSION} is version tags (starting from 2.2.0+) or develop branches (2.2-develop +).
For more details, please, review the Magento Contributor Assistant documentation.

@simonworkhouse do you confirm that you was able to reproduce the issue on vanilla Magento instance following steps to reproduce?

• yes
• no

[ghost]

@simonworkhouse, thank you for your report.
Please update Steps to Reproduce to be more obvious and self-sufficient, without any references to other existing issues.

[ghost]

@simonworkhouse, we are closing this issue due to inactivity. If you'd like to update it, please reopen the issue.

[simonworkhouse]

@engcom-backlog-pb are you able to please re-open this issue? I am still in the process of defining steps to reproduce. Unfortunately a lot of my time is currently being taken up implementing work-arounds for a number of other bugs I have reported, so it might take a little while longer.

[ghost]

@simonworkhouse feel free to open a new issue if you have any updates.