D6 MySQL to D8.6.x PgSQL Migration

When using source plugin d6_taxonomy_vocabulary_per_type, if the Internationalization module was not enabled in D6 and there is no variable set 'i18ntaxonomy_vocabulary', the Migrate source outputs a harmless but noisy error.

In the Migrate source, prepareRow defaults $i17ntaxonomy_vocab to NULL if the variable is unset, then passes it to in_array.

We could either default it to an empty array (which handles the case of it being unset), or check the input with is_array() (which also handles cases where it's set to an unexpected value such as an integer).

CommentFileSizeAuthor
#16 interdiff-5-8.txt688 bytesquietone
#16 3025865-8.patch6.04 KBquietone
#8 3025865-8.patch6.04 KBquietone
#8 interdiff-5-8.patch688 bytesquietone
#5 interdiff.txt722 bytesquietone
#5 3025865-5.patch6.04 KBquietone
#5 3025865-5-fail.patch5.16 KBquietone
#3 drupal-3025865-3-i18n_vocab_empty_array_default-8.6.x.patch1.07 KBxurizaemon
#2 drupal-3025865-2-i18n_vocab_input_check-8.6.x.patch1.07 KBxurizaemon
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

xurizaemon created an issue. See original summary.

xurizaemon’s picture

xurizaemon
he/him
Location Ōtepoti, Aotearoa 🏝
CreditAttribution: xurizaemon at Catalyst IT commented
Status: Active » Needs review
Issue tags: +migrate, +migration
FileSize
drupal-3025865-2-i18n_vocab_input_check-8.6.x.patch1.07 KB

Input checking (relaxed)

xurizaemon’s picture

xurizaemon
he/him
Location Ōtepoti, Aotearoa 🏝
CreditAttribution: xurizaemon at Catalyst IT commented
FileSize
drupal-3025865-3-i18n_vocab_empty_array_default-8.6.x.patch1.07 KB

Defaulting to empty array (stricter, so my proposed fix).

quietone’s picture

quietone CreditAttribution: quietone as a volunteer commented
Assigned: Unassigned » quietone

@xurizaemon, thanks for the patch. I took a brief look and this looks good. Hope to do a proper review in the next couple of days.

quietone’s picture

quietone CreditAttribution: quietone as a volunteer commented
FileSize
3025865-5-fail.patch5.16 KB
3025865-5.patch6.04 KB
interdiff.txt722 bytes

I was curious to know how d6 handles the i18ntaxonomy_vocabulary variable. The search found these

  i18ntaxonomy.install:  $options = variable_get('i18ntaxonomy_vocabulary', array());
  i18ntaxonomy.install:  $translate = variable_get('i18ntaxonomy_vocabularies', array());

which clearly indicate that i18ntaxonomy is used in the d6 code as an array.

So, how is it used in D8 migration? Yes, it is used as an array and right in the source plugin, at line 40.
if (array_key_exists($vid, $i18ntaxonomy_vocab)) {

OK, time to update the source plugin test and prove the error. That's when I found there is no source plugin test. This patch add a source plugin for VocabularyPerType (added back in 2014) and the fix in #3. And a fail patch which is just the new test.

quietone’s picture

quietone CreditAttribution: quietone as a volunteer commented
Title: D6 Internationalization / Migration: " [warning] array_key_exists() expects parameter 2 to be array, null given VocabularyPerType.php:40" » i18ntaxonomy should be an array in d6 VocabularyPerType source plugin
Issue tags: +migrate-d6-d8

A shorter, easier to read title.

The last submitted patch, 5: 3025865-5-fail.patch, failed testing. View results

quietone’s picture

quietone CreditAttribution: quietone as a volunteer commented
FileSize
interdiff-5-8.patch688 bytes
3025865-8.patch6.04 KB

The error in #5 the expected one,
array_key_exists() expects parameter 2 to be array, null given

This just corrects the summary line of the class.

The last submitted patch, 8: interdiff-5-8.patch, failed testing. View results

quietone’s picture

quietone CreditAttribution: quietone as a volunteer commented
Assigned: quietone » Unassigned
mikelutz’s picture

mikelutz
He/Him
Primary language English
Location Michigan, USA
CreditAttribution: mikelutz as a volunteer commented
Status: Needs review » Reviewed & tested by the community

Looks good.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 8: 3025865-8.patch, failed testing. View results

quietone’s picture

quietone CreditAttribution: quietone as a volunteer commented
Status: Needs work » Reviewed & tested by the community

The last patch, 3025865-8.patch, is passing tests, setting back to RTBC

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 8: 3025865-8.patch, failed testing. View results

mikelutz’s picture

mikelutz
He/Him
Primary language English
Location Michigan, USA
CreditAttribution: mikelutz as a volunteer commented
Status: Needs work » Reviewed & tested by the community
quietone’s picture

quietone CreditAttribution: quietone as a volunteer commented
Status: Reviewed & tested by the community » Needs review
FileSize
3025865-8.patch6.04 KB
interdiff-5-8.txt688 bytes

Re-uploading the patch from #8 to correct the misnaming of the interdiff and subsequent reports of test failures.

quietone’s picture

quietone CreditAttribution: quietone as a volunteer commented
Status: Needs review » Reviewed & tested by the community

Ok, back to RTBC as set by mikelutz in #11. The report of failures since then were because of the wrongly named interdiff.

mikelutz’s picture

mikelutz
He/Him
Primary language English
Location Michigan, USA
CreditAttribution: mikelutz as a volunteer commented

Adding a test against 8.7. +1 RTBC on green.

quietone’s picture

quietone CreditAttribution: quietone as a volunteer commented
Version: 8.6.x-dev » 8.7.x-dev

Ah, right, let's make this 8.7.x then.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 16: 3025865-8.patch, failed testing. View results

quietone’s picture

quietone CreditAttribution: quietone as a volunteer commented
Status: Needs work » Reviewed & tested by the community

Tests are passing for 8.7.x, back to RTBC

knyshuk.vova’s picture

knyshuk.vova CreditAttribution: knyshuk.vova at Internetdevels, Drupal Ukraine Community commented

+1 to RTBC

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy at Acquia commented

  • Gábor Hojtsy committed 6689d45 on 8.7.x 
    Issue #3025865 by quietone, xurizaemon, mikelutz: i18ntaxonomy should be...

  • Gábor Hojtsy committed acc7d7a on 8.6.x 
    Issue #3025865 by quietone, xurizaemon, mikelutz: i18ntaxonomy should be...
Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy at Acquia commented
Status: Reviewed & tested by the community » Fixed

Good find, thanks!

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy at Acquia commented
Version: 8.7.x-dev » 8.6.x-dev

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.