Problem/Motivation

Currently there is no chance to work with the currently processed/parsed URL from outside of the data parser plugin. This could be really useful if you'd like to use this value as source (e.g. the current filename as source ID, or the data files path for migrating files besides it).

Proposed resolution

  • Create a getter method for data parser plugins returning the currently processed/parsed URL

Remaining tasks

  • Create a patch

User interface changes

n/a

API changes

  • New getter method in DataParserPluginInterface

Data model changes

n/a

Release notes snippet

Allow currently processed/parsed URL to be read from outside of data parser plugin

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

hctom created an issue. See original summary.

hctom’s picture

Status: Active » Needs review
FileSize
1.02 KB

Here is the proposed patch introducing the new setter method

hctom’s picture

Title: Allow current URL to be read from data parser plugin » Allow currently processed/parsed URL to be read from data parser plugin
Issue summary: View changes

Minor changes to issue title and body

hctom’s picture

FileSize
1.05 KB
487 bytes

Here is an improved patch that also gets the URL for the first item.

heddn’s picture

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

Can we get some tests of this?

hctom’s picture

FileSize
1.05 KB
461 bytes

Here is a slightly updated patch that fixes Only variables should be passed by reference warnings due to the array_shift(array_keys($this->urls)) part.

Tests will be added soon (if I can get them to run, hehe).

hctom’s picture

Status: Needs work » Needs review
FileSize
3.25 KB
1.84 KB

...and here comes a new patch with a current URL test added to the simple_xml parser kernel test. I chose this one, because it has a better setup compared to json parser kernel test and allows reusing its basic configuration.

hctom’s picture

Issue tags: -Needs tests

Removing the "Needs tests" tag, as there is a test contained in the last submitted patch

roborn’s picture

Version: 8.x-4.x-dev » 8.x-5.x-dev
Status: Needs review » Reviewed & tested by the community

Tested also with 8.x-5.x and it is working as expected.

hctom’s picture

Any news on this? Would really like to see this getting into the module.

hctom’s picture

Anybody?! ;)

timlie’s picture

Hi hctom, how do you actually use this?
Is it possible to get the current url as a source value in your migration?

hctom’s picture

@timilie: Sorry for the late reply. In my case, I needed this for a custom data parser plugin that is used when reading multiple XML files from a directory. In order to e.g. be able to use the basename of the currently parsed/read file as an identifier, I needed this new functionality. In my custom data parser I do the following:


protected function fetchNextRow() {
  parent::fetchNextRow();

  // Inject file metadata.
  if ($this->valid()) {
    $current_file_url = $this->currentUrl();
    $this->currentItem['my_custom_source_prop'] = basename($current_file_url);
  }
}

With this, I can access the basename value via my_custom_source_prop source property in each row.

Hope this helps?

timlie’s picture

Hi hctom,

Clear example of how to use this.
I tested this and this does indeed work. Tested against migrate_plus:8.x-5.1

Thanks a lot!

hctom’s picture

Any news on this or is there still something to do to get it in?! I'd appreciate your feedback

Chalk’s picture

Patch makes sense and works for me too. Thanks you, @hctom.

Matroskeen made their first commit to this issue’s fork.

Matroskeen’s picture

Status: Reviewed & tested by the community » Needs work

Thank for you the patch and especially for the unit test and sorry it took so long to have some feedback :)

I think it looks good, apart from some minor items I mentioned in the pull request (I find it easier to comment on MRs so I opened one based on patch #7).

@hctom, hopefully, you're still interested to get it done - it looks like a nice API addition.
Thanks!

hctom’s picture

@Matroskeen No worries, but of course I am still interested in getting this in ;) I had a quick look at your comments and I'll take some time to fix all the stuff later today. Thanks for the feedback

hctom’s picture

Status: Needs work » Needs review

So all issues should be resolved. I guess I used the wrong buttons when commenting in the MR, but I hope you can get through my changes ;)

Matroskeen’s picture

We're down to one open thread: https://git.drupalcode.org/project/migrate_plus/-/merge_requests/14#note...
Can you take a look, please?

hctom’s picture

Status: Needs review » Reviewed & tested by the community

The last open thread should be resolved... but I don't find any button how to resolve this from my end ;) But setting this to RTBC for now.

Matroskeen’s picture

I was wondering what are the circumstances of NULL value in currentUrl. It seems the only case is when urls array is empty, which is fine I think.

I resolved the last active thread and I agree it is RTBC.
Thanks!

heddn’s picture

Status: Reviewed & tested by the community » Fixed

Thanks for your contributions.

heddn’s picture

Status: Fixed » Closed (fixed)

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