migrate-upgrade keeps prefixing upgrade_ on subsequent runs [#3039544]

This is a follow up from #2850309-14: Regenerate config entities when using --config-only multiple times..

I found that once you have exported migration files once using the following command:

drush migrate:upgrade --legacy-db-key=ukids7 --legacy-root=sites/default/files  --configure-only

Future runs of the above command (which are needed when you have made changes to the migration files) will keep prefixing upgrade_ to migration files turning out like upgrade_upgrade_d7_dblog_settings.

I discovered that web/core/modules/migrate/src/Plugin/MigrationPluginManager.php::createInstancesByTag() returns not only the core and contrib migrations but also previously exported migrations which are prefixed with "upgrade_". See the following screenshot:

Which then, when exporting configuration, turns into these additional migration files with the upgrade_upgrade_ prefix:

I also found that there is another test in the same class that uses assertEquals(count(), count()) as well so if we change this here we should change it there too for consistency.

Work around

Export config, then remove the extra files from your exported config containing `upgrade_upgrade_ and then import config.

Example:

drush cex
rm *upgrade_upgrade_*.yml 
drush cim

Comment	File	Size	Author
#31	migrate_upgrade-n3039544-30.patch	3.17 KB	alejo d
#30	migrate_upgrade-n3039544-29.patch	988 bytes	alejo d
#28	migrate_upgrade-n3039544-28.patch	4 KB	damienmckenna
#28	migrate_upgrade-n3039544-28.interdiff.txt	737 bytes	damienmckenna
#27	migrate_upgrade-n3039544-27.patch	3.7 KB	damienmckenna
#27	migrate_upgrade-n3039544-27.interdiff.txt	538 bytes	damienmckenna
#26	migrate_upgrade-n3039544-26.patch	3.17 KB	damienmckenna
#13	migrate_upgrade-filter_previous_exported_migrations-3039544-13.patch	2.54 KB	juampynr
#13	interdiff.txt	649 bytes	juampynr
#9	migrate_upgrade-filter_previous_exported_migrations-3039544-9.patch	2.54 KB	juampynr
#9	migrate_upgrade-filter_previous_exported_migrations-3039544-9-test-only.patch	1.44 KB	juampynr
#2	migrate_upgrade-filter_previous_exported_migrations-3039544-2.patch	1.43 KB	juampynr

Comments

Comment #1

12 March 2019 at 18:37

juampynr created an issue. See original summary.

Comment #2

juampynr commented 12 March 2019 at 18:40

Status:

Active

» Needs review

Status	File	Size
new	migrate_upgrade-filter_previous_exported_migrations-3039544-2.patch	1.43 KB

Here is a patch that fixes it.

Comment #3

juampynr commented 12 March 2019 at 18:56

Title:

migrate-upgrade keep prefixing upgrade_ on subsequent runs

» migrate-upgrade keeps prefixing upgrade_ on subsequent runss

Comment #4

heddn

English

Illinois

commented 12 March 2019 at 19:13

Would be awful nice to have a test added?

Comment #5

heddn

English

Illinois

commented 20 March 2019 at 19:09

also:

src/MigrateUpgradeDrushRunner.php ✗ 1 more
line 525	Type hint "array" missing for $migrations

Comment #6

heddn

English

Illinois

commented 20 March 2019 at 19:09

Status:	Needs review	» Needs work
Issue tags:		+Needs tests

Comment #7

juampynr commented 20 March 2019 at 19:45

Title:

migrate-upgrade keeps prefixing upgrade_ on subsequent runss

» migrate-upgrade keeps prefixing upgrade_ on subsequent runs

Comment #8

aiphes

French

commented 12 April 2019 at 10:23

Maybe relative to my issue with D6 : https://www.drupal.org/project/migrate_upgrade/issues/3047436

Comment #9

juampynr commented 5 August 2019 at 20:25

Status:	Needs work	» Needs review
Issue tags:	-Needs tests

Status	File	Size
new	migrate_upgrade-filter_previous_exported_migrations-3039544-9-test-only.patch	1.44 KB
new	migrate_upgrade-filter_previous_exported_migrations-3039544-9.patch	2.54 KB

Here is an updated patch that includes a test. I am also including a test-only patch.

Comment #10

5 August 2019 at 20:28

The last submitted patch, 9: migrate_upgrade-filter_previous_exported_migrations-3039544-9-test-only.patch, failed testing. View results

Comment #11

heddn

English

Illinois

commented 5 August 2019 at 20:31

Status:

Needs review

» Needs work

Just a small nit:

+++ b/tests/src/Kernel/DrushTest.php
@@ -103,6 +103,31 @@ namespace Drupal\Tests\migrate_upgrade\Kernel {
+      $this->assertEquals(count($migrate_plus_migrations_first_run), count($migrate_plus_migrations_second_run));

I think there's an assertCount method in phpunit?

Comment #12

heddn

English

Illinois

commented 5 August 2019 at 20:32

Also, looks like phpcs found something:

src/MigrateUpgradeDrushRunner.php ✗
line 449	Expected 1 space after FUNCTION keyword; 0 found

Comment #13

juampynr commented 5 August 2019 at 20:45

Issue summary:	View changes
Status:	Needs work	» Needs review

Status	File	Size
new	interdiff.txt	649 bytes
new	migrate_upgrade-filter_previous_exported_migrations-3039544-13.patch	2.54 KB

Thanks for the feedback @heddn!

I fixed the coding standards issue. As for the assertCount(), I decided to leave it as it is as it seemed more clear to me. Here is what the patch does:

$this->assertEquals(count($migrate_plus_migrations_first_run), count($migrate_plus_migrations_second_run));

And here is how it should be written using assertCount:

$this->assertCount(count($migrate_plus_migrations_first_run), $migrate_plus_migrations_second_run);

Comment #14

5 August 2019 at 20:59

Status:

Needs review

» Needs work

The last submitted patch, 13: migrate_upgrade-filter_previous_exported_migrations-3039544-13.patch, failed testing. View results

Comment #15

juampynr commented 6 August 2019 at 18:39

Status:

Needs work

» Needs review

Triggered a re-test since I cannot reproduce the PHP warning.

Comment #16

6 August 2019 at 18:50

Status:

Needs review

» Needs work

The last submitted patch, 13: migrate_upgrade-filter_previous_exported_migrations-3039544-13.patch, failed testing. View results

Comment #17

juampynr commented 6 August 2019 at 19:56

Can anyone reproduce the warning? I cannot, not even using run_tests.sh.

Comment #18

juampynr commented 6 August 2019 at 19:56

Status:

Needs work

» Needs review

Comment #19

damienmckenna

TN, USA

commented 27 August 2019 at 19:42

The question is, what would cause $plugin['plugin'] to be empty? I was able to reproduce it locally and used print_r() to find out what the plugin was:

Array
(
    [map] => Array
        (
            [file] => Array
                (
                    [default] => file_default
                    [url_plain] => file_url_plain
                    [path_plain] => file_url_plain
                    [image_plain] => image
                    [image_nodelink] => image
                    [image_imagelink] => image
                )
            [entityreference] => Array
                (
                    [entityreference_label] => entity_reference_label
                    [entityreference_entity_id] =>entity_reference_entity_id
                    [entityreference_entity_view] =>entity_reference_entity_view
                )
        )
)

Comment #20

damienmckenna

TN, USA

commented 27 August 2019 at 19:45

FWIW I tested with Drush 10, maybe there's a bug that doesn't show in Drush 9?

Comment #21

heddn

English

Illinois

commented 9 September 2019 at 22:04

It isn't very clear here what is needed?

Comment #22

heddn

English

Illinois

commented 2 March 2020 at 16:50

  protected function modifyId($id) {
    if ((strpos($id, $this->options['migration-prefix']) === 0)) {
      return $id;
    }
    return $this->options['migration-prefix'] . str_replace(':', '_', $id);
  }

This seems like a better solution?