[drupal-devel] [feature] Introduce file API hook

Fri Sep 2 15:27:51 UTC 2005

Issue status update for 
http://drupal.org/node/18934
Post a follow up: 
http://drupal.org/project/comments/add/18934

 Project:      Drupal
 Version:      cvs
 Component:    file system
 Category:     feature requests
 Priority:     normal
 Assigned to:  robertDouglass
 Reported by:  Bèr Kessels
 Updated by:   walkah
 Status:       patch (code needs review)

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?)

walkah

Previous comments:
------------------------------------------------------------------------

Tue, 15 Mar 2005 16:15:30 +0000 : Bèr Kessels

This simple patch by Robert Douglas introduces a fileapi hook. Nothing
fancy, nothing difficult and most of all harldy any performance
overhead.

It simply allows modules to hook into the data of the file on:
insert
delete
update
load

The reason why this should be here, and not in nodeapi, is because, for
example media.module wants to "do stuff" with the file, like get id3
data and put that into a table on upload of a file.
Another valid use case would be to be able to track and count
downloads. Yet another case would be some file renaming, or organising
in directories.

Bèr

------------------------------------------------------------------------

Tue, 15 Mar 2005 17:01:40 +0000 : walkah

+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?)

------------------------------------------------------------------------

Tue, 15 Mar 2005 18:00:33 +0000 : Bèr Kessels

Attachment: http://drupal.org/files/issues/upload.module.patch.txt (1.92 KB)

forgot the attachement. *blush*

------------------------------------------------------------------------

Tue, 15 Mar 2005 18:34:30 +0000 : robertDouglass

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.

------------------------------------------------------------------------

Tue, 15 Mar 2005 21:12:56 +0000 : javanaut

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().

------------------------------------------------------------------------

Wed, 23 Mar 2005 06:44:06 +0000 : robertDouglass

Attachment: http://drupal.org/files/issues/upload.module.patch_0.txt (2.46 KB)

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

------------------------------------------------------------------------

Wed, 23 Mar 2005 06:44:56 +0000 : robertDouglass

Changed version to cvs

------------------------------------------------------------------------

Mon, 28 Mar 2005 22:39:56 +0000 : Amazon

+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

------------------------------------------------------------------------

Mon, 28 Mar 2005 22:52:24 +0000 : JonBob

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.

------------------------------------------------------------------------

Tue, 29 Mar 2005 08:41:08 +0000 : robertDouglass

Attachment: http://drupal.org/files/issues/upload.module.patch_1.txt (2.44 KB)

Sure! It is now hook_file.

------------------------------------------------------------------------

Tue, 29 Mar 2005 16:54:26 +0000 : joshk

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

------------------------------------------------------------------------

Tue, 05 Apr 2005 15:41:21 +0000 : moshe weitzman

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.

------------------------------------------------------------------------

Tue, 05 Apr 2005 16:41:19 +0000 : chx

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.

------------------------------------------------------------------------

Tue, 05 Apr 2005 16:45:46 +0000 : killes at www.drop.org

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.

------------------------------------------------------------------------

Thu, 26 May 2005 00:04:52 +0000 : moshe weitzman

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

------------------------------------------------------------------------

Tue, 02 Aug 2005 18:41:41 +0000 : Bèr Kessels

Patch still applies. with a little fuzzyness (1 or 2 lines).

------------------------------------------------------------------------

Wed, 17 Aug 2005 01:01:49 +0000 : Steven

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?

------------------------------------------------------------------------

Fri, 02 Sep 2005 15:19:57 +0000 : telex4

+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...