Introduce file API hook

+1 for this idea (even though I don't actually see a patch) .

My big concern here is trying to manage the order of operations. particularly when you get into resizing / re-encoding things. i.e. how could we handle multiple modules that want to do generate thumbnails? or multiple bitrates for a media file?

overall though, i like the idea alot!

(was there actually a patch somewhere?)

James,

how hard would it be to react to these lifecycle events in your image.module so that any image file that gets uploaded via upload.module gets handled as an image automatically?

As to the issue of "competing" interests from various modules on the same events (what you described above), don't we already have that danger in nodeapi? Have we run into any problems there? The things you want to do with files are rather discreet, I can't imagine two modules wanting to read the bitrate of a movie separately.

Anyway, thanks Bèr for submitting this patch, I'm looking forward to having people evaluate the idea and help think of possible uses and problems.

I would suggest adding a "validate" hook in there as well. This would allow for file cleaning modules to be developed (such as virus scanners). This could be placed along with the other validation checks in upload_nodeapi().

Added validate hook (2 lines). Please verify if this looks like the best choice of location.

Changed version to cvs

+1 We need this for musicforamerica. We need to allow artists to enter meta data about themselves and their files. Also, we want to be able to allow for encoding based on the metadata as selected by artists.

So on upload we encode a low bitstream for radio. We will also encode at a bitrate the artist allows us to distribute their music.

Kieran

Could we call this hook_file() instead? IIRC, I agree with Dries on this matter; the similar hook_user() and hook_taxonomy() constructs are more nicely named than hook_nodeapi() is.

Sure! It is now hook_file.

This will be useful for a whole lot of things going forward. Another example this would help drive is bittorrent integration.

i'll add to the chorus that this is a useful, minimally invasive patch. very drupalish. would be nice to run media.module without needing to patch core.

another +1

I think, in general, the more hooks, the better. Most time is consumed by SQL queries and file loading after all, so a few php function calls are not big deal.

I also like this patch. It will make the revisioning of files easier. Currently, I have to keep the nid column in some tables. I can remove that after the patch has landed and rely on vid only.

still applies cleanly ... seems like a a handy enhancement to me as well.

Shouldn't the validate hook allow the file to be actually validated? Now its return value seems to be unused.

It also seems weird to me to tie this hook into upload.module... shouldn't it go into file.inc somewhere, so it can apply to all uploaded files?

+1 from me. Drupal is really lacking in the file-handling department at the moment, which is quite a pain for us at Remix Reading since our web site is supposed to support a file-sharing remix community! From what I can see this at least starts to point Drupal in the right direction...

ok, let me qualify my previous +1 , I agree with Steven, this should *absolutely* be part of file.inc *not* upload.module - as there are some other important modules that utilise the file api besides upload.module.

it will also likely require refactoring file.inc a bit - since you need to accomodate for several "file saving" senarios - file_save_upload, file_save_data (and perhaps file_move and file_copy?)

Like nodeapi('validate'), this validate hook has no return value. Modules are encouraged to form_set_error() if they want to. This will stop the node validation automatically. I think this is a non issue.

As for hooking into the file API directly, I think I agree. I asked walkah and robert to comment here on this since they are more familiar with file.inc.

-moshe

I would be happy to see file.inc refactored to support this hook, but I remember that at the time I created it, it wasn't possible to add it there (details long ago forgotten). File.inc is the obvious appropriate place, and everyone seems to like the functionality. I can't volunteer to look into this at the moment, however. Unassigning myself.

anyone up for this file.inc job. if i could assign it to walkah, i would :)

I am somewhat new to Drupal. I am wanting to use the media module and am told I need to patch upload.module with this patch. How do you install the patch?

Big and high priority feature!
See also http://drupal.org/node/25685

Dupe of #142995