Re: [development] files owned by uid, patch review request.
On Tue, 08 May 2007 09:40:05 -0400, Earnie Boyd <earnie@users.sourceforge.net> wrote:
Quoting Darrel O'Pry <dopry@thing.net>:
What if I want the files to get deleted when the node is deleted? Would they still be deleted with the proposed patch?
No. The file remains, but is disassociated from the node. There is no way to guarantee the file is not in use elsewhere on your site, or linked from an external site, so the file remains.
It can be modified to do so, but being able to do it sanely will require the addition on drewish's modules column, or a hook delete for reference counting to make sure files are not in use elsewhere.
I haven't looked at the patch so forgive me if this is addressed. Would it be possible to make this a feature within the system settings? Would it be possible to have a count of nodes using the file and if the node being deleted is the only one prompt for the user for file deletion?
Earnie
I would concur here. Leaving untracked orphan files around should not be typical behavior. Some sort of simple reference counting would need to be included so that (a) we can auto-delete files that are no longer in use (give or take a setting to do so or leave them) and/or (b) provide an admin area to manage said files. If files become a first-class core data type (along with nodes, users, vocabularies, and comments), then they need some sort of direct management. It doesn't have to be complicated; something akin to admin/content/node is probably all that would be needed (with a "number of uses" column). But a first-class data type needs administrative control within core. Contribs can add alternate admin screens if desired, but core needs to have something. That said, I'm strongly in favor of this move. --Larry Garfield
On 08 May 2007, at 17:05, Larry Garfield wrote:
That said, I'm strongly in favor of this move.
As most people are in favor, I think we should move forward with this patch. I think that some of the concerns could (and probably should) be addressed with a follow-up patch or two. Not sure if Darrel aspires to work on that, but I'd love to see us pave the path towards better file/document/image management. I'd love to see us make more baby steps towards that direction. -- Dries Buytaert :: http://www.buytaert.net/
On 5/8/07, Dries Buytaert <dries.buytaert@gmail.com> wrote:
As most people are in favor, I think we should move forward with this patch. I think that some of the concerns could (and probably should) be addressed with a follow-up patch or two. Not sure if Darrel aspires to work on that, but I'd love to see us pave the path towards better file/document/image management. I'd love to see us make more baby steps towards that direction.
With the code freeze coming up I wanted to follow up on this and urge people to take a moment to review the patch that dopry and I came up with: http://drupal.org/node/142995 There have been quite a few changes from what was last discussed here. I'll try to summarize the changes as they now stand: * The existing behavior of the upload module is preserved, deleting a node deletes the files. * The file_revisions table is renamed to upload. The reasoning being, the new name makes it clear that contrib modules shouldn't put their files in it unless they want upload.module monkeying around with them. They can create their own table to link files to nodes. * file_check_upload() has been merged into file_save_upload() which now takes an array of callback functions to validate the upload before it's saved as a temporary file. This cleans up a lot of duplicative code in core's upload processing. file_validate_extensions() checks that the file extension in in the given list file_validate_size() checks for maximum file sizes and against a user's quota file_validate_is_image() checks that the upload is an image file_validate_image_resolution() checks that images meets maximum and minimum resolutions requirements. * It refactors the way that upload.module determines the types and sizes of files a user is allowed to upload and displays it on the form. This gives the user an idea of what will be permitted before beginning the upload. There's still a great deal of files related work I'd like to do before the code freeze but since this is really just a first step, it all depends on getting this in. andrew
On 5/8/07, Larry Garfield <larry@garfieldtech.com> wrote:
I would concur here. Leaving untracked orphan files around should not be typical behavior. Some sort of simple reference counting would need to be included so that (a) we can auto-delete files that are no longer in use (give or take a setting to do so or leave them) and/or (b) provide an admin area to manage said files.
If files become a first-class core data type (along with nodes, users, vocabularies, and comments), then they need some sort of direct management. It doesn't have to be complicated; something akin to admin/content/node is probably all that would be needed (with a "number of uses" column). But a first-class data type needs administrative control within core. Contribs can add alternate admin screens if desired, but core needs to have something.
I don't want to sound like a broken record here but I think we should just provide a framework in core and leave this up to the various modules. I've done a bit of work with the audio and image modules. Each of those has a clear relation between a node and files. Audio has the primary audio file as well as album artwork that may have been embedded in it. Image has the image and the resized derivative images. When you delete either one of these nodes, you want all the file deleted as well. But I can envision a module that acts like lets Joomla's Media Manager. You upload all your images via one screen and use it's UI for managing the images. When you create a node you select the image from the library and the module has some JavaScript to insert the images into nodes. The point is, these are very different methods of managing files and I don't think core should proscribe one. I'd love to be able to use core's upload and file handling functionality but it's currently (even in dopry's patch) too tied to the upload module. andrew
participants (3)
-
andrew morton -
Dries Buytaert -
Larry Garfield