#2983197: Catch TamperException to prevent import from crashing tries to prevent an import from crashing if an unapplicable tamper plugin gets applied to a source. A tamper plugin is for example not applicable to a source if the tamper plugin expects the source to be in certain format when the source isn't. For example, the tamper plugin "implode" expects the source to be an array and throws an exception if it doesn't.
The patch from the issue noted above now catches an exception and records an error message for it. The problem is that the import of the item goes through, only just without the tampers applied. This can possible lead to unintended values being imported.
Removing the item from the parser result is also not an option, because then it might be removed from the site if the feed type is configured to delete previously imported items.
A possible solution is to be able to mark an item as invalid, by adding new methods to \Drupal\feeds\Feeds\Item\ItemInterface.
Then, during processing, Feeds needs to detect that the item is marked invalid. And if so, don't import it. And report which item is invalid.
None.
\Drupal\feeds\Feeds\Item\ItemInterface gets new methods.
None.
New methods are added to \Drupal\feeds\Feeds\Item\ItemInterface: items can now be marked as invalid.
| Comment | File | Size | Author |
|---|---|---|---|
| #9 | feeds-item-validation-3063055-9.patch | 6.64 KB | webdrips |
| #6 | feeds-item-validation-3063055-6.patch | 7.87 KB | florianmuellerch |
| #3 | interdiff-3063055-2-3.txt | 1.09 KB | megachriz |
| #3 | feeds-item-validation-3063055-3.patch | 7.15 KB | megachriz |
Show commandsStart 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:
changes
Comments
Comment #2
megachrizThis is a possible implementation. I'm not sure about the approach yet. Maybe it should be implemented using a constraint instead? It feels a bit dirty to have two places where an error message for the failing entity is composed.
Comment #3
megachrizFixing PHP syntax errors.
Comment #5
florianmuellerch@MegaChriz thank you very much for that valuable patch!
I applied it and also added an issue with patch to feeds_tamper in #3109509: Mark item as invalid when tampering data does not meet expectations so we can use setInvalid() in a tamper.
Comment #6
florianmuellerchI extended the #3 patch with a message which can be optionally set on
markInvalid(), so we can see in the error message what lead to the invalidity of the element. @MegaChriz maybe you want to check if that still meets coding guidelines.Comment #7
gaele commentedComment #9
webdrips commentedRe-rolling #6 with latest dev
Comment #10
gaele commentedComment #13
megachrizFor backwards compatibility I have moved the new methods to a new interface called ValidatableItemInterface. Feeds Tamper can then check if the item implements that interface before trying to call the new methods. This way, Feeds can still be compatible with older Feeds versions.
Let's see if the test passes now.
Comment #14
megachrizI see that the new properties are also exported when calling
BaseItem::toArray(), which I think we don't want. So these properties should not be exported. So that should get fixed, but perhaps before we do that it makes sense to finalize #3452563: Creation of dynamic property Drupal\feeds\Feeds\Item\SyndicationItem::$parent:fid is deprecated first, because in that issueBaseItem::toArray()gets also changed. And I think that issue is good to go, just waiting a few more days for possible feedback on that one.Comment #15
megachrizThis is now ready to be tested in combination with #2983197: Catch TamperException to prevent import from crashing for Feeds Tamper.
Comment #17
megachrizI merged the changes!
I will merge #2983197: Catch TamperException to prevent import from crashing shortly.