Problem/Motivation
While working on #2928904: Add a mapping target to media field and seeing several file related issues, I think it make sense to move the code for resolving a file to a separate service. This way the Media target can make use of the file resolving logic without needing to extend the File class.
The downside is that this does introduce a BC break. Modules that were extending the File class to resolve file related issues would now need to decorate the FileResolver service.
Proposed resolution
Add a FileResolver service and adjust the File target to make use of this service. Deprecate methods in the File class that are related to resolving files.
Remaining tasks
- Review
- Merge
User interface changes
None.
API changes
Several methods in \Drupal\feeds\Feeds\Target\File will no longer be used and get deprecated:
- getFileName()
- getContent()
- writeData()
Modules that overrode these methods could break.
Data model changes
None.
Issue fork feeds-3565186
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:
Comments
Comment #2
megachrizComment #4
megachrizI've done some testing and reviewing the code this week.
Latest changes:
getFileName(),getContent()orwriteData()a fallback to the old implementation will be followed. If we would leave out the fallback, then these methods would no longer get called if subclasses overrode them. And that possible would break workflows. The methods are still deprecated, though. So the fallback won't be there forever.Findings so far on reviewing/testing the code:
I did not manage to get a successful import with a file specified with either file:// or ftp://, so I'm going to assume it is a far fetched edge case. We could re-add support for other protocols once users really need it.
Comment #5
megachrizI scheduled merging the code!