If no custom entity attributes exist then return without processing

[thisisandrew]

Description

When there are no CustomEntityAttributes to migrate then an invalid SQL statement is produced and the migration dies. Adds a defensive if statement to avoid this error and return early

Fixed Issues (if relevant)

1. n/a

Manual testing scenarios

1. Migrate from a Magento 1.9.2.1 which has no custom entity attributes (aside from those in the ignore list)
2. Run the data migration step of the migration tool

Contribution checklist

• [x] Pull request has a meaningful description of its purpose
• [x] All commits are accompanied by meaningful commit messages
• [x] All new or changed code is covered with unit/integration tests (if applicable)

[victor-v-rad]

Thank you @thisisandrew for your contribution to the project. Internal ticket MC-35382 to process it.

[ghost]

Hi @thisisandrew, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.

[HeiderSati]

Ok,... Debugged the damn thing,... now understood what's going on:

1) Download the latest "Data Migration Tool" (I'm on Magento CLI 2.3.5 at the time I wrote this response) 2) Goto vendor/magento/data-migration-tool/src/Migration/Step/Eav/Data.php 3) Go to line 528, you should see the following function:

private function migrateCustomEntityAttributes()
{
    $this->progress->advance();
    $sourceDocName = 'eav_entity_attribute';
    $destinationDocument = $this->destination->getDocument(
        $this->map->getDocumentMap($sourceDocName, MapInterface::TYPE_SOURCE)
    );
    $recordsToSave = $destinationDocument->getRecords();
    $customAttributeIds = $this->modelData->getCustomAttributeIds();
    $customEntityAttributes = $this->source->getRecords(
        $sourceDocName,
        0,
        $this->source->getRecordsCount($sourceDocName),
        new \Zend_Db_Expr(sprintf('attribute_id IN (%s)', implode(',', $customAttributeIds)))
    );
    foreach ($customEntityAttributes as $record) {
        $record['sort_order'] = $this->getCustomAttributeSortOrder($record);
        $record['attribute_group_id'] = $this->mapAttributeGroupIdsSourceDest[$record['attribute_group_id']];
        $record['entity_attribute_id'] = null;
        $destinationRecord = $this->factory->create(['document' => $destinationDocument, 'data' => $record]);
        $recordsToSave->addRecord($destinationRecord);
    }
    $this->saveRecords($destinationDocument, $recordsToSave);
}

4) The above code builds the CustomAttributeIDs by calling the getCustomAttributes() function which takes the AttributeIDs from the (Desintation) database "assuming" it's a vanilla/new database, and then compare these with the source database attributeIDs, if anything is found on the destination that is not in source then it's taken into the array. This would work if the destination database is brand-new. However, if the Data Migration Tool was run previously then the results would be NULL, with that the getRecords() line would fail causing this nasty error because the system is trying to select with the condition of "attribute_id IN ())"

This clearly have never been tested on a database that has already been migrated previously, or in other words, this would NEVER work for a system that was already imported.

5) Before the fix: IMPORTANT NOTE: I am always totally against editing release/firmware files of Magento Original Releases, however, unfortunately on this occasion, it's a must as the out-of-the-box does not work with an existing set of data unless you are migrating into a brand-new/fresh M2 install, which is impractical since you would build things and then re-migrate and then go live.

IMPORTANT: From a high level perspective; a gentle note: I take no responsibility for any damage you may cause to your data, use this procedure on "your own risk":

6a) Now the Fix: The new logic will be: A) Read the DEST attributes in the same way. B) Do a simple check (Do I have a NULL array?) C) If NULL then nohing needs to be imported, and the function should succeed. D) If we have something in the array, then proceed normally.

6b) Another Fix:

Just go into the source database (i.e. your production site), and add any dummy attribute there, this would cause the scrip to import nicely since the array won't be NULL anymore.

7) Now the fix is just to add the following logic:

  $customAttributeIds = $this->modelData->getCustomAttributeIds();
    if ($customAttributeIds)
    {
  .... do the stuf...             
    }

8) Now the fix in detail: Replace the above function with:

private function migrateCustomEntityAttributes()
{
    $this->progress->advance();
    $sourceDocName = 'eav_entity_attribute';
    $destinationDocument = $this->destination->getDocument(
        $this->map->getDocumentMap($sourceDocName, MapInterface::TYPE_SOURCE)
    );
    $recordsToSave = $destinationDocument->getRecords();
    $customAttributeIds = $this->modelData->getCustomAttributeIds();

    if ($customAttributeIds)        // Added by Heider to ensure that we process ONLY if there are new ATTRs
    {
        $customEntityAttributes = $this->source->getRecords(
            $sourceDocName,
            0,
            $this->source->getRecordsCount($sourceDocName),
            new \Zend_Db_Expr(sprintf('attribute_id IN (%s)', implode(',', $customAttributeIds)))
        );
        foreach ($customEntityAttributes as $record) {
            $record['sort_order'] = $this->getCustomAttributeSortOrder($record);
            $record['attribute_group_id'] = $this->mapAttributeGroupIdsSourceDest[$record['attribute_group_id']];
            $record['entity_attribute_id'] = null;
            $destinationRecord = $this->factory->create(['document' => $destinationDocument, 'data' => $record]);
            $recordsToSave->addRecord($destinationRecord);
        }
        $this->saveRecords($destinationDocument, $recordsToSave);
    }

}

I hope this helps.

Regards Heider

[kanevbg]

@HeiderSati

This clearly have never been tested on a database that has already been migrated previously, or in other words, this would NEVER work for a system that was already imported.

Are you running bin/magento migrate:data more than once? I believe this command is meant to be run only once, than should be used bin/magento migrate:delta. There is option --reset for bin/magento migrate:data however I am not sure if it will work right if the EAV step is active (in the XML config). I am using multiple times migrate:data during development, however I am commenting steps which I know are already done. And do a Magento 2 backup of the database after the successful migration (before you execute installed modules) in order to come back later for complete re-test of the migration process. Docs: https://devdocs.magento.com/guides/v2.3/migration/migration-migrate-settings.html

[HeiderSati]

Hi Kostadin

The command I am typing is:

php -d memory_limit=-1 bin/magento migrate:data -r vendor/magento/data-migration-tool/etc/opensource-to-opensource/1.9.3.8/config.xml

The “-r” option is the same as “—reset” that you mentioned.

The problem I am having now is; what’s the point of having the “-r” option when it does not reset the EAV info? This is clearly a bug, either it does it what it says “resetting” for re-import or remove it altogether.

To explain our setup, about a year ago we took on a project to migrate from v1.9.3.8 into v2, a lot of testing has gone since then, this involves creating orders, adding test products etc, therefore, it makes sense to re-import with the (reset) option to wipe out these new items.

Using the “incremental” option that you suggested wouldn’t help in this case because it would not replace the test-orders in M2 with real-ones.

Therefore, the viable method would be to “reset” the work and re-import the DB altogether without having to re-config as I already have a lot of stuff put in all over the place, including the core_config_table changes, and other items etc.

Happy to discuss this further if you like, right now, the import is not working as it should.

Kind Regards Heider

[kanevbg]

@HeiderSati Hi,

I have inspected the algorithm of the data-migration tool, the only thing it is doing when -r option is provided is to empty the data in file var/migration-tool-progress.lock. If you manually delete this file, the result will be the same even if you not provide -r. This in result will make that the data to be copied from the M1 instance will be all of it (as you have not run migrate:data before). But the data in the M2 instance is not cleared at all.

The only solution I see in order to do complete re-migration is to install new M2 instance for the specific version you are migration to and the run data:migrate over it. Your current configurations can be dumped with bin/magento app:config:dump than it will be re-imported in your new-version. You can make backup of the database before and after the migration and use them as checkpoints for later re-migrations. That is what I am doing.

The process of migration seems to be very far from straight-forward process, so think about your specific setup and history of the migration and in every case make backups of current sources (server-side) and databases, before performing re-installation migrations. If you find something which eases the process please let me know.