Better handle replacement of missing filters with filter_null

Since Drupal 8 core does not support the PHP filter, migration replaces it with filter_null so that PHP content is filtered out on the front-end. The filter_null plugin, unfortunately, unconditionally generates that message, which isn't particularly helpful. But, apart from the message, things are operating as they should - it's unlikely this would have anything to do with anything hanging.

Moving to the core queue - we should find a way to provide a more helpful message. Since filter_null is designed as a fallback for missing filters, maybe it would be as simple as replacing 'filter_null' with something like 'php_filter_not_supported'...

Discussed @xjm, @mikeryan, @penyaskito, and @mlhess. Rather than falling back to filter_null in the migration we should just migrate with the missing plugin ID so the filter will fallback naturally to the filter_null plugin. This will then tell the user what plugin is missing when they edit the filter format. Also we need to make it very clear to the user that does the migration that they need to fix the migrated filter format.

Applying the #7, finger crossed :D

Seems like valid failures.

+++ b/core/modules/filter/migration_templates/d6_filter_format.yml
@@ -23,7 +23,6 @@ process:
-        default_value: filter_null

D6 filter seems to be missing a flag so the static_map doesn't throw an exception. It might be the cause of the failures.
bypass: true

Expecting (for D6) that the error messaging is going to be any more helpful is probably going to let someone down. The source is a numeric filter ID, which is not very useful. But we can expect to see more useful language for the D7 migration.

Actually, if we did

    default_value: name

would that give us useful messaging?

Ignore that, of course that would make the default value the exact string 'name'...

Rather than falling back to filter_null in the migration we should just migrate with the missing plugin ID

What does this mean in the context of D6 filters? They don't have a "plugin ID", they're just (module, delta) pairs. I guess we'd just have to come up with a name that sounds reasonable to us?

For Drupal 6,
1) Should we try to get the UI label of the Drupal6 filter format from filter_formats table (ex: format id = 3 and name = PHP code)?
This could help the site migrator to find out what was the filter used for in Drupal 6.

2) Could be enhanced by adding the module that provided the filter from filters D6 table (ex: format 3 was orivide by module php).

What do you recommand to do?

Patch https://www.drupal.org/files/issues/2618332-11.patch successfully applied, but I have not a right D6 website copy to test it today. I will try later back home.

I am wondering if we should add php module to the map, which could lead to some thing like this in core/modules/filter/migration_templates/d6_filter_format.yml?
map:
filter:
- filter_html
- filter_autop
- filter_url
- filter_htmlcorrector
- filter_html_escape
php:
- filter_php

Convinced the D6 MigrateFilterFormatTest to pass. Let's see if that has a trickle-down effect on the other failing tests...

Hey...progress! This should address the D7 migration failure. Hopefully it will also fix the other broken tests.

The tests pass, now to address the issue - how to provide useful messaging for missing filters?

@mikeryan: I originally submitted a patch in #2630578-18: Formats duplicated in D6 upgrade. It was resubmitted in other two issues that were closed in favour of this one. At the risk to being repetitive, here's the patch again:

Here's a proposal: return an informative message for admins, instead of the empty string.

We don't have access to the format neither the entity from within the filter class, so the message can't be more friendly --although probably can have a better wording.

I'm not sure I agree with the approach in #23 -- I fear that we will potentially get mired in core politics if we try to change the filter system itself, and it's not necessary for us to do it anyway. IMHO, it's Migrate that needs to accommodate the filter system, not the other way around.

This patch takes another whack at it. It will almost certainly break the tests, but it adds a new process plugin that takes the time to validate the filter ID after it has been run through the static map. It validates it by actually trying to create an instance of the filter. If that fails, it logs the PluginNotFoundException to the migration, which should at least be a little more informative than the current message -- but it lets invalid values through anyway, since missing filters will automatically fall back to filter_null (at which point the user should deal with the problem).

Ugh. I hate how one little typo can eff your entire patch. Same interdiff as in #25.

Okay, here's an even better version, based on a correct understanding of how fallback-aware plugin managers work. I don't think this one will pass the tests, but it should behave as we intend.

This should fix the failing Drupal 6 tests.

I think this will fix the D7 tests, and also give us the result we want.

Behold, a unit test!

1. +++ b/core/modules/filter/migration_templates/d6_filter_format.yml
   @@ -16,8 +16,9 @@ process:
   +        # Because the bypass flag is not set, the row will be skipped
   +        # entirely if the filter ID cannot be determined.
   

   That's not true, is it? filter_id should apply the fallback filter in that case? And I don't think we want to throw out the whole format for want of a filter...

2. +++ b/core/modules/filter/src/Plugin/migrate/process/FilterID.php
   @@ -0,0 +1,81 @@
   +      $message = $this->t('Filter @id could not be mapped to an existing filter plugin; defaulting to @fallback.', [
   

   I don't see any test validating the message.

So I ran this on a big migration. The migration log was pretty and everything looked like it was going to be great but wait, what is this:

Running d6_filter_format                                                    [ok]
Processed 3 items (0 created, 0 updated, 0 failed, 3 ignored) - done    [status]

1. +++ b/core/modules/filter/migration_templates/d6_filter_format.yml
   @@ -16,8 +16,9 @@ process:
   -        plugin: static_map
   -        default_value: filter_null
   +        # Because the bypass flag is not set, the row will be skipped
   +        # entirely if the filter ID cannot be determined.
   +        plugin: filter_id
   

   So... this skipped the entire row as documented. I've got a filter that will never exist in d8 applied to all my filter_formats though so now I've got no filters on any of my content and everything is messed up and the entire filter mapping is lost. All I needed was for it to be passed through so I could remove the dead filter manually.

2. +++ b/core/modules/filter/src/Plugin/migrate/process/FilterID.php
   @@ -0,0 +1,81 @@
   +      $message = $this->t('Filter @id could not be mapped to an existing filter plugin; defaulting to @fallback.', [
   +        '@id' => $value,
   +        '@fallback' => $fallback,
   +      ]);
   +      $migrate_executable->saveMessage($message, MigrationInterface::MESSAGE_WARNING);
   

   Just a notice deep in the message logs? Ouch...

That's pretty rough, I'm not sure I'd say its better.

Just a notice deep in the message logs? Ouch...

Yeah, it's not optimal...I'm open to other ideas about how to output the message. I am loath to use drupal_set_message(), that's clunky and intended only for the UI. If only Migrate was using PSR-3...

Okay, this should address everything except #38.2. Not sure what to do about that.

How 'bout now?

I'm happy with what I see here - @neclimdul, can you run your migration against the latest patch here? If that appears to work fine, it's RTBC as far as I'm concerned.

Thanks!

Running d6_filter_format                                                    [ok]
Missing filter plugin: filter_null.                                      [error]
Missing filter plugin: filter_null.                                      [error]
Missing filter plugin: filter_null.                                      [error]
Processed 3 items (3 created, 0 updated, 0 failed, 0 ignored) - done    [status]

Feedback is still a bit weak but as discussed that's a bigger issue.

Migration ran smooth, after that. I've got my filters and content seems to be correctly using them. The ones that have missing filters are initially broken and render nothing but when you open the filter settings you get "The filter_null filter is missing, and will be removed once this format is saved." Saving it has them working again.

This is a definite improvement so this seems good to go.

The ones that have missing filters are initially broken and render nothing

#23 address this.

@jonhattan: The trouble with #23, at least in the context of this issue, is that it changes a filter plugin, which drags the issue out of Migrate's scope. It also doesn't address the problem of Migrate trying to save formats with non-existent filters -- which, as I discovered during a debugging session the other day, is impossible due to a quirk of the way FilterPluginCollection works.

Don't get me wrong -- I think #23 is definitely worth pursuing. But in a separate, non-Migrate issue.

+++ b/core/modules/filter/src/Plugin/migrate/process/FilterID.php
@@ -0,0 +1,90 @@
+      return $fallback;

Should we return the untransformed value and rely on the filter system here?

This is what happens in \Drupal\filter\Element\ProcessedText::preRenderText()

    // If the requested text format doesn't exist or its disabled, the text
    // cannot be filtered.
    if (!$format || !$format->status()) {
      $message = !$format ? 'Missing text format: %format.' : 'Disabled text format: %format.';
      static::logger('filter')->alert($message, array('%format' => $format_id));
      $element['#markup'] = '';
      return $element;
    }

So the system handles missing formats...

We cannot return the unprocessed value, because filter formats cannot be saved with invalid plugin IDs under any circumstances. A quirk of the plugin system, and the way FilterPluginCollection uses it, will not allow it.

Let's say we return the unprocessed value, and it happens to be an invalid plugin ID. Once the row reaches the destination plugin, it will try to save a FilterFormat entity. Here, then, is what happens:

1. FilterFormat::preSave() will load the FilterPluginCollection in an attempt to sort the filters. (\Drupal\filter\Entity\FilterFormat, line 198)
2. FilterPluginCollection::sort() calls $this->getAll().
3. getAll() calls $this->initializePlugin().
4. initializePlugin() calls getDefinition() on the filter plugin manager -- without setting the $exception_on_invalid flag to FALSE.
5. getDefinition() throws a PluginNotFoundException which is not caught by initializePlugin() -- although it freaking should be, since the filter plugin manager implements FallbackPluginManagerInterface, but it isn't. The exception bubbles up and halts execution, and the filter format is not saved.

So in summary...because of flaws in the filter system, Migrate has to play defense. That's what's going on here. Back to RTBC!

Yeah, it's not optimal...I'm open to other ideas about how to output the message. I am loath to use drupal_set_message(), that's clunky and intended only for the UI.

trigger_error() with E_USER_WARNING would work. You get dsm() via the error handler if you run by the UI, drush will report it via cli in red.

@catch: Nice idea. My only problem with it is an issue of scope; process plugins should have no business determining how errors are to be displayed. They should just pass things along to a PSR-3 logger class. But since Migrate's error reporting in general is pretty crappy, I figured that the next-best thing was to save a message in the MigrateExecutable.

The idmap saveMessage() is the API for recording messages during migration - what else is needed? Note that drush migrate-import, when run with -d, will also display them at the console.

Committed and pushed ae25829 to 8.3.x and 0a3ce8b to 8.2.x and 806d956 to 8.1.x. Thanks!

I think we should log in the place the where all migrate info is logged which this patch does.

I ran into this issue today .. what is the best approach to resolve it?

I also have this issue on 8.2.6 with the following error messages:

Missing filter plugin: filter_null.

and

Source ID full_html: Filter media_filter could not be mapped to an existing filter plugin; defaulting to filter_null.

I verified that 8.2.6 has the patches applied by alexpott.

Hi,
Upgrading from a D6, I get this error on filter_null.
The D6 website use module's filter like embed media, core's filter like PHP.
So do I need to do something before upgrading according filters ?
Actually, changing the default format profile doesn't change anything, data are in DB but the front or back office show empty fields for body field...

Thanks

I have migrated a D6 website with several 1000 nodes to D8. Is there a way to do a bulk a bulk text format change, instead of editing and saving each of the nodes ?

@rnexussix, your best bet is probably to edit the database fields directly. for example:

$ drush sqlc

? update node set filter_format = 'filtered_html' where filter_format = 'full_html' ;

... or similar.

Do make sure there's a backup!

Missing filter plugin: filter_null. - still I'm facing this issue while migration from drupal- 6 to 8 . I've installed php filter module also in drupal8. Formats are mapped in both drupal6 and drupal8.

This should be fixed in 8.3 already but I am seeing it in 8.4 when migrating from D6.

also see it in d7 to d8 - testing 8.5x

In a case here, at some point in time the php module was enabled. It is now disabled, but the php filter entries remain in the database.

You might need to run the following in your D7 system before the migration AND after making sure that you don't use the PHP text filter anywhere:

delete from filter where format = 'php_code';
delete from filter_format where format = 'php_code';

Issue is in 8.5.4 as well. The solution was to click Configure on the text formats that I used in Drupal 7, and then just save them again.

• example.com/admin/config/content/formats
• Configure
• Save Configuration

This issue just happened to me from a Drupal 7 install to a Drupal 8.6 migration. I had to re-save the "full_html" text format to restore body fields that had their data migrated.

I'm proposing that this issue gets reopened for investigation and I can help with any testing steps.

I chatted with heddn on Slack. I will be filing a new issue with some greater details over the weekend. I'll link to this issue.

@nerdstein There's always #2946889: Missing migration filters that are replaced with filter_null may have invalid settings applied which is already open on having to resave filters to again be able to view migrated content.

I found this page because my body fields were disappearing. After pulling out some hair, I found this error in the log:

Missing filter plugin: filter_null.

After stumbling around, I found that if I went to Home>Administration>Configuration>Content authoring and started editing the content types, I would start getting messages that "filter_null" couldn't be found and so it would be deleted when I saved the text format. After saving the text format for Full HTML, the body text started appearing.

@bogdog400
i can confirm this and that is how i fixed it too. after resave of the filters, the body will appear.

any update on this?
WHat I have exprienced is that I have no comment body....it is empty....comment subject is there and user and date....so only body is empty...

I'm unable to do the solution proposed in #69 and #70. When I click on configure for the text format "Full HTML", for example, I get a notice that says " The filter_null filter is missing, and will be removed once this format is saved." When I click the Save button, as suggested above, I get the notice "Forbidden: You don't have permission to access this resource. (Apache/2.4.41 (Ubuntu) Server at 137.184.2.119 Port 80)".

This happens only on text formats that have CKEditor selected in the Text Editor dropdown menu.

Sorry. I didn't have that issue so I can't offer advice. Maybe it's something to do with permissions?