Hi,
the d6_url_alias
migration fails when the src
path in the alias from D6 contains no slashes.
The failure message is this one:
Processed 527 items (526 created, 0 updated, 1 failed, 0 ignored) - done with 'ao2_it_d6_url_alias' [status]
$ drush migrate-messages ao2_it_d6_url_alias
source_ids_hash level message
93b99e18ed0ac290d6853182b67e4127f473d77e0fe07d1f7d2fbe5c69fec4f7 1 Array index missing, extraction failed.
This comes from core/modules/migrate/src/Plugin/migrate/process/Extract.php
.
The cause seems to be an alias which does not contain any slashes in the src
path:
SELECT * FROM `url_alias` WHERE `src` NOT LIKE '%/%'
pid src dst language
----------------------------------------
294 contact contatto it
I think the above is a valid alias but it's a case not covered by d6_url_alias
in its process part:
...
node_translation:
-
plugin: explode
source: src
delimiter: /
-
plugin: extract
index:
- 1
-
plugin: migration_lookup
migration:
- ao2_it_d6_node_translation_blog
- ao2_it_d6_node_translation_news
- ao2_it_d6_node_translation_page
...
AFAIU here the migration extracts the second element (index: 1) of the path to pass in node_translation
, knowing that core/modules/path/src/Plugin/migrate/destination/UrlAlias.php
will use this data only in case the path actually represents a node, having the guarantee that the extracted element is the nid.
This "blind" approach generally works (for instance it causes no problem with paths like taxonomy/term/xxx
because node_translation
will be ignored in the destination plugin for these paths), but it breaks if the src
path does not contain any slashes.
Thanks,
Antonio
Comment | File | Size | Author |
---|---|---|---|
#18 | 2927227-18.patch | 1.77 KB | jofitz |
#18 | interdiff-15-18.txt | 1.75 KB | jofitz |
#15 | 2927227-15-8.6.x.patch | 1.68 KB | Olarin |
#14 | 2927227-14-8.4.x.patch | 1.71 KB | Olarin |
#12 | 2927227-12.patch | 1.66 KB | jofitz |
Comments
Comment #2
ao2 CreditAttribution: ao2 as a volunteer commentedThe issue is also in 8.5.x and the attached patch should fix it.
Now I can import all my url aliases.
Consider applying the patch with
git am
if you like to preserve the detailed commit message.Ciao,
Antonio
Comment #4
ao2 CreditAttribution: ao2 as a volunteer commentedComment #5
quietone CreditAttribution: quietone as a volunteer commentedLet's start with a fail test. Added a url alias without a slash to the test fixture and updated the test.
Comment #7
ao2 CreditAttribution: ao2 as a volunteer commentedThanks quietone,
The second assertion in the patch from #5 seems wrong, the alias is set with
'language' => ''
but then the assertion checks for$this->assertSame('en', $path['langcode']);
I think the latter could be$this->assertSame('und', $path['langcode']);
.Anyways, I ran some local tests with:
../../vendor/bin/phpunit modules/path/tests/src/Kernel/Migrate/d6/MigrateUrlAliasTest.php
With the current vanilla core all the tests pass:
With your new test from #5 there is indeed a failure:
After applying the patch from #2, and fixing the second assert as per my comment above, the tests pass:
So the proposed solution should be OK.
BTW, the failure is not about translated nodes, so maybe the new assertions should not be in
testUrlAliasWithTranslatedNodes()
? And do we really need the one about$path['language']
?Ciao,
Antonio
Comment #8
ao2 CreditAttribution: ao2 as a volunteer commentedPing.
Comment #9
jofitz CreditAttribution: jofitz at ComputerMinds commentedCombine @quietone's test-only patch from #5 (with the langcode correction suggested by @ao2 in #7) with @ao2's fix patch from #2.
Comment #11
quietone CreditAttribution: quietone as a volunteer commentedRight, this should be changed and it looks like the last thing to do. I don't have time now.
Comment #12
jofitz CreditAttribution: jofitz at ComputerMinds commentedMoved the test into testUrlAlias().
Comment #14
Olarin CreditAttribution: Olarin at Kosada commentedFor anyone still using Drupal 8.4 here's a patch that applies against that (when the migrations directory was still the migration_templates directory). Meanwhile I can confirm that the fix works for me.
Comment #15
Olarin CreditAttribution: Olarin at Kosada commentedAnd just for the sake of argument let's demonstrate that it applies against 8.6.x, too.
Comment #16
quietone CreditAttribution: quietone as a volunteer commentedJust adding tags
Comment #17
heddnThis is a super nit. In general, I like the name noslash in the test case. However, its very confusing. No one will ever see the value without the slash in the fixture. All they will see is this reference in the test case. And it obviously has a slash in it here. Which made me review the code several times. And the only reason it made sense was because I could see the diff with the fixture changes. So in this case, picking another name would less confusing. Perhaps
source-noslash
or evenexample
would work just as well. Or a comment in the test and keep it noslash?What is this all about? Can we add a comment explaining?
Comment #18
jofitz CreditAttribution: jofitz at ComputerMinds commentedAddress @heddn's review in #17.
Comment #19
heddnThanks for pandering to my request. RTBC.
Comment #20
alexpottShouldn't we fix the D7 one too?
Comment #21
alexpottAs far as I can see core/modules/path/migrations/d7_url_alias.yml has the same problem.
Comment #22
alexpott@quietone wants a follow-up. Needs to be created before a commit.
Comment #23
quietone CreditAttribution: quietone as a volunteer commentedYea, I'm all into small incremental steps right now, just like the small steps I'm taking to clean out the spare room.
Followup made #2942388: Ensure d7_url_alias migration works for paths without a slashes
Comment #24
alexpottAdding issue credit.
Comment #27
alexpottCommitted 17a47b8 and pushed to 8.6.x. Thanks!
Committed bf47414 and pushed to 8.5.x. Thanks!
Backported to 8.5.x since this is a bugfix.