Is there an existing str_replace plugin for Migrate Plus? I've found myself needing to do simple string replacements. Happy to add that if it's useful.

Comments

brooke_heaton created an issue. See original summary.

mikeryan’s picture

Nope, not yet.

mikeryan’s picture

Title: Basic Find/Replace Plugin » String replacement plugins
Assigned: brooke_heaton » Unassigned

Bump - this would be useful. And so would regular expression replacements.

process:
  uri:
    # Convert file schemes from private to public.
    plugin: str_replace
    source: uri
    search: 'private://'
    replacement: 'public://'
  relative_path:
    # Remove scheme/host from URL
    plugin: preg_replace
    source: uri
    pattern: '|^[a-z]+://[^/]*?|i'
    replacement: ''
mikeryan’s picture

Issue tags: +Novice

Setting to Novice - this is actually pretty simple, given http://cgit.drupalcode.org/drupal/tree/core/modules/migrate/src/Plugin/m... as a model.

Lowell’s picture

Version: 8.x-2.x-dev » 8.x-3.x-dev
Status: Active » Needs review
StatusFileSize
new1.11 KB

initial code for string_replace plugin

Lowell’s picture

StatusFileSize
new1.29 KB

initial code for the preg_replace plugin

heddn’s picture

Status: Needs review » Needs work

Great start here. Much appreciated you stepping up and rolling some patches.

  1. +++ b/src/Plugin/migrate/process/StrRepace.php
    @@ -0,0 +1,37 @@
    +    $search = isset($this->configuration['search']) ? $this->configuration['search'] : null;
    +    $replacement = isset($this->configuration['replacement']) ? $this->configuration['replacement'] : null;
    

    Nit: normally, we use NULL.

  2. +++ b/src/Plugin/migrate/process/StrRepace.php
    @@ -0,0 +1,37 @@
    +    if (!is_string($search)) {
    

    Can you just throw a MigrateException if it is missing?

  3. +++ b/src/Plugin/migrate/process/StrRepace.php
    @@ -0,0 +1,37 @@
    +    if ($search && is_string($value)) {
    

    If it isn't a string, should we fail more loudly? Or provide that as an option?

  1. +++ b/src/Plugin/migrate/process/PregReplace.php
    @@ -0,0 +1,41 @@
    +    $limit = isset($this->configuration['limit']) ? $this->configuration['limit'] : -1;
    

    I think isset on 0 returns FALSE? Might try adding a phpunit test to confirm.

  2. +++ b/src/Plugin/migrate/process/PregReplace.php
    @@ -0,0 +1,41 @@
    +    if ($pattern && is_string($value) && is_int($limit)) {
    

    Should we fail more loudly if the required parameters aren't available?

#2820490: FormatDate process plugin has some examples on how to throw exceptions and write unit tests, if you are up for the challenge. On your next patches, can you either combine them together or (preferably) split out the regex into a separate issue.

Lowell’s picture

sure, thanks for the feedback.
My thought was to let them fail gracefully and return the input $value.
If we want exceptions thrown, I did see how those were done.
I will make regex a separate issue

Lowell’s picture

StatusFileSize
new1.29 KB

updated str_replace plugin
I looked at the skip_on_value test as a reference, but haven't made the test for this plugin yet

Lowell’s picture

shawngo’s picture

This looks like it may be a duplicate of Similar things are happening here #2830512: String Replace Process plugin

Edit: I didn't see the PregReplace plugin at first.

mikeryan’s picture

Title: String replacement plugins » Regular expression replace process plugin
Issue tags: -Novice +Needs tests

Given we've got a patch in the other issue for str_replace, let's narrow the scope of this to preg_replace. #6 is the current patch along those lines.

osopolar’s picture

Would be nice if we can also manipulate vallues captured by subpatterns in the regular expression. Imagine for example a link to an old path like <a href="/node/1234"> should be rewritten to use the destination nid instead of the source nid. Using preg_replace_callback() instead preg_replace may allow manipulating the node-id captured with the subpattern in the regex #<a href=['"]/node/(\d+)['"]>#i.

The following patch allows to use process plugins to alter the captured subpattern matches. That way the source node-id could be replaced with the destination node-id using the migrate_lookup process plugin. Or for example Wordpress Next Gen Gallery tags could be replaced with entity embed tags and the correct destination entity ids:

process:
  'body/value':
    -
      plugin: preg_replace
      source: post_content
      pattern: '#\[nggallery id="?(\d+)"?\]#i'
      replacement_pattern: '<drupal-entity data-entity-type="node" data-entity-id="preg_replace_placeholder_1" data-entity-embed-display="view_mode:node.full"></drupal-entity>'
      process:
        preg_replace_placeholder_1:
          plugin: migration_lookup
          migration: drv_gallery
          no_stub: true
          source: 'preg_replace_match_1'
    -
      plugin: preg_replace
      pattern: '#\[ngg_images source="galleries" container_ids="(\d+)"(?:".*?"|.*?)*?\]#i'
      replacement_pattern: '<drupal-entity data-entity-type="node" data-entity-id="preg_replace_placeholder_1" data-entity-embed-display="view_mode:node.full"></drupal-entity>'
      process:
        preg_replace_placeholder_1:
          plugin: migration_lookup
          migration: drv_gallery
          no_stub: true
          source: 'preg_replace_match_1'

If there are no subpatterns the function will just replace the pattern with the replacement_pattern, as in preg_replace.

Tests are still missing, sorry.

pingwin4eg’s picture

Version: 8.x-3.x-dev » 8.x-4.x-dev
Issue tags: +Needs issue summary update

Thanks @osopolar ! Your solution is perfect!
Though one notice: the $self variable is undefined.

I suppose the patch from #9 can be hidden as it only contains the str_replace plugin.

osopolar’s picture

StatusFileSize
new3.51 KB

Thanks @pingwin4eg for bringing this up again. I did som changes, to make the process plugins (inside preg_replace) optional, to be able to do simple replacementes with no additional processing. This version also allow source placeholder (like preg_replace_match_1) in replace patterns.

Yes, this $self variable is a leftover. Before I used $self = &$this; to access $this inside the anonymous function. In newer php versions (> 5.3 I think) $this can be accessed from inside the anonymous function. I removed it from the patch (but didn't test again).

Still no tests, sorry.

heddn’s picture

Issue tags: +Novice

Adding tests to this is a fairly trivial phpunit task. Tagging for that. See #2923891: Transliterate file names as a recent example on how to add this type of test.

heddn’s picture

This also seems very similar to #2830512: String Replace Process plugin, which has more progress right now. So I might suggest joining forces and closing this one as duplicate.

osopolar’s picture

Not sure how you came to the conclusion that #2830512: String Replace Process plugin has more progress, as both issues do not have tests, but if it is about popularity you are right, the other issue has attracted more attention. So do you think it would be better to merge both solutions into one plugin or to have two separate plugins?

I'd like to provide tests and want to add them, but currently I have zero resources for that.

The functionality could be helpful for a lot of migrations, for example the following modules:
* Node Embed
* Entity Embed
* Media or Media (WYSIWYG) Embed
* Insert Views
* Many Wordpress shortcodes etc.

k_a_l’s picture

Version: 8.x-4.x-dev » 8.x-4.0-beta1

Just confirming that patch #15 worked for me. Thanks!

k_a_l’s picture

Version: 8.x-4.0-beta1 » 8.x-4.x-dev

re-assigning back to 4.x-dev. Sorry

heddn’s picture

Status: Needs work » Closed (duplicate)

Let's close this as duplicate.

osopolar’s picture

@heddn: You didn't answer my question in #18, so I guess there is no interest in getting this code in. Otherwise let me know.

heddn’s picture

re #22:sorry I missed responding. The issue that this is closed dup to has regex capabilities.

osopolar’s picture

Yes, I have see that, but is it able to process the regex matches? For example Replace a source id with a destination id (i.e. node id), can't see that. That seemed important to me and worth implementing, because it has so much power. Lets say you have a migration where sorce nid (i.e. 123) != destination nid (i.e. 987) and content with Links like node/123. Is there any plugin which can replace node/123 with node/987, this one can, see #13 (more examples for use-cases in #18). This works, while for each matched value $migrate_executable->processRow($new_row, $this->configuration['process']); gets called, so it's no only about replacing IDs, it exposed the matched value to all process plugins.

Maybe I have overlooked it in the other issue, if so we won't need this functionality.

See also #37 in the other issue.

heddn’s picture

There's regex capabilities that uses preg_replace in that other issue. I'm fine with re-opening this, but it seems like the features exist?

nathan.simmonds’s picture

I agree with @osopolar, the ability to expose and manipulate matched patterns as described would be really useful, and is in fact exactly what I need to migrate embedded entities. Basic regex search & replace can't look up the migrated ids, and there's no other contrib module that I can see which enables embedded media to be migrated correctly. So unless it's is already covered elsewhere definitely I support inclusion of this patch.

fls-pcate’s picture

I agree with @osopolar, the ability to expose and manipulate matched patterns as described would be really useful, and is in fact exactly what I need to migrate embedded entities. Basic regex search & replace can't look up the migrated ids, and there's no other contrib module that I can see which enables embedded media to be migrated correctly. So unless it's is already covered elsewhere definitely I support inclusion of this patch.

I'm in the same position. This feature would make it easy to map migrated id's in body field urls. Has anyone found a solution which does this, or is it possible with the referenced str_replace plugin?

bcweaver’s picture

I've got to agree, the ability to sub-process matched subpatterns through other process plugins is a killer feature, and looks like exactly what I need for handling embedded media references.

The killer feature in @osopolar's patch isn't the use of regular expressions, it's the ability to pass what's matched through other process plugins, most notably migration_lookup in my case. Match an embedded reference and then pass the ID portion of that match through migration_lookup to convert it to the destination ID of the previously-migrated content.

The example YAML in #13 illustrates this usage.

If other replacement plugins already provide the ability to pass matches through sub-process-plugins, I'll certainly use those, but it seems like this functionality isn't provided elsewhere.

fls-pcate’s picture

Just a followup.

After finding: https://www.drupal.org/project/migrate_plus/issues/2958642 and https://www.drupal.org/project/migrate_plus/issues/2958672 I applied the patches and used it to replace entity id links in HTML body fields with relative ease.

I still link @osopolar additions would be useful, but the above solved my immediate use case, and may for others.

bcweaver’s picture

@fls-pcate thanks so much for those links. I had not already come across these from my searching.

andyg5000’s picture

Status: Closed (duplicate) » Active

Several people agree with @osopolar, including myself. I'm likely going to try the patches mentioned by @fls-pcate for my immediate needs, but I see a ton of value in the #15 patch. Especially, because the other issue only supports DOM objects.

andyg5000’s picture

Status: Active » Needs work

After further review, the patch isn't working as expected and throws "Undefined variable: self" error for me. If someone can provide an example of using the `str_replace` plugin and iterating/processing the result, I think we could close this out again. Otherwise, still feel it would be useful.

osopolar’s picture

@andyg5000 the patch was working at time of publishing. Currently I don't have a need for this functionality and maintainers aren't interested to implement this functionality, therefore I do not maintain the plugin until I have a project requiring it. Be free to re-roll the patch to get it work with current state of migration API.

alison’s picture

Looooooove this functionality, and thank you for all the work on it.

And thanks for sharing those links, @fls-pcate -- in particular, I hadn't noticed #2958672: Use migration lookup on text fields before, which does seem to provide comparable functionality to @osopolar's work here, it just happens to be more current, because, #time.

If others agree, maybe close this out as outdated or duplicate (even though this one originally came first)?

andyg5000’s picture

Status: Needs work » Closed (duplicate)

Hey @alisonjo2786,

Thanks for finding and posting the related issue. I reopened this a few weeks ago, so I'm closing it back out based on your feedback ;)