node_clone fix
| Project: | Node clone |
| Version: | 5.x-2.x-dev |
| Component: | Code |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | postponed (maintainer needs more info) |
Jump to:
I've had some issue with the node_clone module. Specifically, when I clone a node that has an filefield, new entries are created in the files table, which point to the same file name as the original node. This works fine, but then when I delete either the original or the clone, the file is deleted from the files/ directory too. This leaves the remaining clone missing it's files.
I've added a few lines of code to _filefield_file_delete() that check if there are other nodes that still reference the file, and if there are, we skip calling file_delete() for the file, and leave it for the others. If there are no remaining references, the file is deleted as normal.
This is one of my first patches. Hope I'm doing this right, and it's useful to someone!
| Attachment | Size |
|---|---|
| filefield-clone-fix.patch | 1.12 KB |
#1
There should never be the same file with two different fids in the database. Your patch is appreciated, but I believe that the fix must rather involve preventing the creation of that second file entry. When that is working, we can try to find a way around deleting these entries (I hope).
Also, have a look at some other db_query() calls in Drupal - you'll instantly notice that they use upper-case operators, e.g. "SELECT * FROM {table}". That example also demonstrates the use of braces for table names, which is required in order to keep tables working that use custom prefixes. And lastly, SQL's official "not equal" operator is "<>", not "!=" (the latter might just work on MySQL but not on other databases).
Nevermind these points it's a good start, please keep investigating.
#2
No in this case the node_clone module should be duplicating files in D5. It should be creating a copy of the files as well instead of just duplicating the DB entries. D5 does not support multi linked files.
#3
Interesting -- users (me among them) of the node_clone module have been begging for some time now to get a file-copy-and-rename function in D5 node_clone to make it work with attachments managed by CCK. I think the recent patch to jettison file attachments to prevent this issue is ill-conceived.
In contrast, this approach is a back door solution to the same issue: node_clone duplicates the db entry, oh well! Just lock the back door and who cares whether node_clone is not copying files and setting new db entries.
Shouldn't need to do this, but if it solves a problem with node_clone, a lot of users will be happy to have this. See:
http://drupal.org/node/249049
http://drupal.org/node/257316
http://drupal.org/node/234372
I suppose the extra processing could add up if a site was doing tons of deletions. What other downsides do you guys see to this approach? I'd love to know before I experiment with this patch!
#4
The problem with filefields being incorrectly duplicated should be fixed in both 5.x -dev versions.
See: http://drupal.org/node/200092 and please test it. I will make a release soon. With this change, you can enable an add-on module to copy imagefield files. Without the add-on, both fieldfield and imagefiled information is stripped when the clone is made (as it already was with upload module).
The solution there for imagefield could probably be easily replicated for filefield (as an add-on module).
#5
Doh! I did forget the Drupal standard {}'s around table names.
I hadn't been aware that no duplicates should exist in the files table. I thought it seemed weird to find them, but figured if we were serious about that there would be a unique constraint on the filepath field or something.
I'll be anxiously awaiting a good fix for this sometime in the future. Thanks for the feedback on the patch.
#6
@psuda - please look at the 5.x - devtarballs and test the code there.
#7
Subscribing.