Allow replacing based on a xpath expression

The attached patch makes several changes compared to the one in #3:

1. Add test coverage for the new DomStrReplace class (dom_str_replace plugin).
2. Add a base class that can be used by DomStrReplace and other classes that work with DOMDocument objects.
3. Add a new property $xpath to the base class.
4. Replace the method getNodeList() with $this->xpath->query(...)
5. Coding standards: replace null with NULL.
6. Add an init() method to the base class to set the two properties.
7. Instead of an explicit test $value instanceof \DOMDocument in the validate() method, rely on the type hint of the init() method.
8. Minor change to the logic when selecting str_replace(), str_ireplace(), or preg_replace().

Oops, I used .patch instead of .txt on the interdiff.

The attached patch fixes coding standards found by the testbot.

I am attaching interdiffs comparing it to the the two previous patches (from #3 and #6).

I think this should fix the failing tests.

Again, I am attaching an interdiff comparing to the patch in #3 as well as one for the previous patch.

Try again. Once again, I am attaching an interdiff for the patch in #3 as well as one for my previous attempt.

You cannot catch a TypeError, so let's go back to the $value instanceof \DOMDocument test.

1. +++ b/src/Plugin/migrate/process/DomProcessBase.php
   @@ -0,0 +1,47 @@
   +      throw new \InvalidArgumentException('Dom plugins only work with \DOMDocument objects.');
   
   +++ b/src/Plugin/migrate/process/DomStrReplace.php
   @@ -0,0 +1,201 @@
   +      // @todo Move out once another mode is supported.
   

   Add an issue for the follow-up.

2. +++ b/src/Plugin/migrate/process/DomProcessBase.php
   @@ -0,0 +1,47 @@
   +  protected function init($value) {
   +    if (!($value instanceof \DOMDocument)) {
   +      throw new \InvalidArgumentException('Dom plugins only work with \DOMDocument objects.');
   
   +++ b/src/Plugin/migrate/process/DomStrReplace.php
   @@ -0,0 +1,201 @@
   +    try {
   +      $this->init($value);
   +    }
   +    catch (\InvalidArgumentException $e) {
   

   Pass in as much context for the exception as possible. So we can print the migration and destination field, etc.

3. +++ b/src/Plugin/migrate/process/DomStrReplace.php
   @@ -0,0 +1,201 @@
   +      // @todo Move out once another mode is supported.
   

   Add an issue for the follow-up.

4. +++ b/src/Plugin/migrate/process/DomStrReplace.php
   @@ -0,0 +1,201 @@
   +        throw new MigrateException("Configuration option '$option_name' is required.");
   

   InvalidPluginDefinitionException?

5. +++ b/src/Plugin/migrate/process/DomStrReplace.php
   @@ -0,0 +1,201 @@
   +    foreach ($this->xpath->query($this->configuration['expression']) as $html_node) {
   

   Would it make sense to use array_map and iterator_to_array?

@heddn:

Thanks for the review, and for discussing this issue in person at MidCamp. I have implemented most of your recommendations.

1. I added #3042833: Process non-attribute strings in the dom_str_replace process plugin, and I added an @see comment after the @todo comment.
2. I did not see any way to get the current migration. Maybe using getPluginDefinition()? But maybe that is not really needed, since the Migrate system (or the drush commands from the Migrate Tools module) will make it clear which migration is active. I did pass $destination_property to the init() method, and simply used $this->getPluginId() in the base class: once the object is instantiated, it "knows" the data of the derived class.
3. I think this is a repeat of (1).
4. Done. I had to look at the constructor for the InvalidArgumentException class to find that it requires two arguments.
5. I do not think that array_map() would be useful here. I think of that as a way tp generate a new array (with the same keys as the input) by transforming the values. Here, for each $html_node, we modify $this->document, the DOMDocument object. Also, I think there could be performance problems if we used iterator_to_array(). We do not need to collect all the $html_node elements in a single array.

For the record: as we discussed in person, the big change in error handling is that we do not catch the exceptions in (1) and (4). The reason is that these exceptions will be thrown when the migration that uses this plugin is mis-configured. In this case, it is better to stop the execution than to continue throwing errors for each row.

I also took the opportunity to expand the test coverage. We now test all of the validation checks in the constructor. I did this by removing one test method and adding a data provider, so we get more tests with only a few net lines added and less duplicated code.

Since the base class is not a plugin, I guess it should be moved to src/ from src/Plugin/migrate/process/. If I do that, then would you also like me to rename the class? Perhaps DomProcessPluginBase instead of DomProcessBase?

This should fix the coding standards problems.

This looks good. Thanks for all your contributions to migrate!

On commit I will switched from throwing an invalid argument exception to throwing a skip row exception in init(). After thinking about this, we don't want to kill the entire migration if a single row or two isn't DOM parsable.

-      throw new \InvalidArgumentException($message);
+      throw new MigrateSkipRowException($message);