Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Yesterday I did an update to the new beta7-version. I did a new import of my previously defined CSV-Mapping in which I have filefields with a path. After running the import with beta7 all sourcefiles on the server where deleted.
Comment | File | Size | Author |
---|---|---|---|
#10 | 919172-10_dont_delete.patch | 2.03 KB | alex_b |
#2 | 919172-2_dont_delete.patch | 1.46 KB | alex_b |
#1 | 919172-1_dont_delete.patch | 807 bytes | alex_b |
Comments
Comment #1
alex_b CreditAttribution: alex_b commentedI can confirm that.
These files are local to the site, right? You have placed them in your sites directory, referenced them from your CSV file, then mapped them to a filefield. After the import they are gone?
This bug is an interaction between two "improvements" to Feeds, one deleting temporary files, the next one allowing local files to be references. The latter one actually referencing files that should be permanent.
#863494: Delete temporary enclosures file
#755556: Support saving local files in filefields
This patch reverses #863494.
Comment #2
alex_b CreditAttribution: alex_b commentedThis approach still removes temporary files, but keeps files living in site's files directory alive.
Comment #3
xurubo93 CreditAttribution: xurubo93 commentedYour are right. The sourcefiles are stored in my sites directory and after import they have been vanished.
I have downgraded to beta5 and everything works as expected.
Comment #4
alex_b CreditAttribution: alex_b commented#3: Care to give me a quick review on #2?
Comment #5
xurubo93 CreditAttribution: xurubo93 commentedSorry, but I do not find the time for it right now.
Comment #6
andrewlevine CreditAttribution: andrewlevine commentedIMHO, getFile and getContent are looking quite messy right now (actually partly my fault from #755556: Support saving local files in filefields). They should be rewritten...though that is probably a separate less urgent issue.
After disagreeing with the use of __destruct here initially and thinking about every way we could avoid it, I finally came around to realizing it should be OK and it logically makes sense if you think of FeedsEnclosure as the only exposed access to the file for feeds. Ideally getFile should be named getTempFilename if that filename is going to be deleted as soon as the FeedsEnclosure is unset. But that's for another issue.
I haven't gotten around to actually testing the patch but it looks good to me.
Comment #7
alex_b CreditAttribution: alex_b commented#6
Please take a look at the D7 version of FeedsEnclosure: http://github.com/lxbarth/Feeds/blob/master/plugins/FeedsParser.inc
D7's FeedsEnclosure:: getFile() takes $destination parameter and returns a permanent file object. This has the following implications:
1. Caller of getFile() can specify where it expects the file to be placed. In many cases, this avoids an additional copy operation on the file system.
2. The file returned by the enclosure is permanent. Due to the fact that we have the $directory is fine for all use cases right now, we MAY want to change that.
3. If the original resource is a file (not a URL like in most cases) and the file is NOT in the location requested via $destination, it will be copied, but not deleted. I think this behavior is fine, we shouldn't delete these files.
Comment #8
andrewlevine CreditAttribution: andrewlevine commentedAlex, that does indeed seem like a nicer model, although I'm confused about this line in the D7 getFile:
I don't see $directory set before this line...
Anyway, for this D6 issue, what do we need to do to get this patch in? Have one person test it?
Comment #9
alex_b CreditAttribution: alex_b commented#8: you got it: http://github.com/lxbarth/Feeds/commit/d570ee01b6c902c08db1eaf88a214fa62...
Updated #2 with a more concise comment for getFile():
Will commit shortly.
Comment #10
alex_b CreditAttribution: alex_b commentedComment #11
alex_b CreditAttribution: alex_b commentedCommitted, thank you for reporting and reviewing.
http://drupal.org/cvs?commit=427320