New hook triggered on file download

Marked #346125: Add hook_file_downloaded() a duplicate of this.

Attached is a really simple and basic attempt to implement this hook.

I opted to call it in the file_transfer() function because, in my humble opinion, the call is only required when the file is actually, and probably correctly, transferred. At least that's what I would expect. I passed the $source and $headers variables which are also passed to the file_transfer() function although, by heart, I can't really find where I would use the $headers.

The downside of placing this in the file_transfer() function is that the $source variably might get cluttered with the path to the files directory so that would be up to the user to extract the correct name of the original file and handle his actions accordingly.

Forgot to change the status.

The PHPDoc should probably just omit the @return if we don't return anything. Also you should come up with an example no matter how trivial.

It might even be fun to have a hook before the file goes out that allows headers and source to be modified.... I think you could trigger the completion after the file push is completed, but should be able to do that in hook_exit... oh wait file_transfer never triggers hook_exit.... maybe it should call something like drupal_page_footer... or invoke hook_exit itself....

Very cool idea.

This patch looks very good. However, it probably needs a short test before it's ready to be committed.

I wanted to add a test to see whether the hook is called, but it seems that file_transfer is not called when calling file_download. Any pointers on how I should test this?

Actually, file_transfer() IS called from file_download(). The following code is taken from api.drupal.org for the function file_download:

<?php
  if (file_exists(file_create_path($filepath))) {
    // Let other modules provide headers and controls access to the file.
    $headers = module_invoke_all('file_download', $filepath);
    if (in_array(-1, $headers)) {
      return drupal_access_denied();
    }
    if (count($headers)) {
      file_transfer($filepath, $headers);
    }
  }
?>

So as long as the file exists, and headers are returned (though none of them are -1), file_transfer() will be called. Perhaps this logic is broken - I really don't know enough about it to tell you.

The problem lies within the following code segment of file_transfer():

// Transfer file in 1024 byte chunks to save memory usage.
if ($fd = fopen($source, 'rb')) {
  while (!feof($fd)) {
    print fread($fd, 1024);
  }
  fclose($fd);
}
else {
  drupal_not_found();
}

In my original patch I placed the call to module_invoke_all() behind the fclose() call. If I place the call before the while (!feof($fd)) { call it seems to work. My guess is that the call to the hook is lost somewhere when printing to a buffer starts.

I modified my original patch and created a hook_file_transferring() hook which is called before the while loop starts.

There are a couple of minor issues with this patch.

Issues with white space at the end of at least one line.

+ // Check whether required hooks were called.

This line

+ * String specifying the file path to transfer.

Should probably read something like

"String specifying the path of the file to transfer"

Also, why are we calling this hook_file_transferring? To me that implies that the hook is called while the file is actually being transferred when in reality it is called just before the file is transfered. Why not hook_file_transfer, and then maybe also add a hook_file_transferred that gets called immediately after a successful file transfer.

Subscribing.

needs to be reworked a bit to work w/ stream uri's. also an api change, so switching to d8.

Rerolled patch against 8.x

+ * Act on begin of file transfer.
+ *

Should be Act on begining of file transfer I assume

Fixed

#16: hook-file-transferring-233997-16.patch queued for re-testing.

Interesting hook and I think it could be very useful. I think it should probably be named hook_file_transfer() though.

Looks like #1561362: Change file_transfer() to use BinaryFileResponse removed file_transfer() in favor of Symfony\Component\HttpFoundation\BinaryFileResponse. I don't think a hook is possible anymore.

Actually, this could still be nice to have to Drupal 7.

Re-rolled for Drupal 7 and renamed to hook_file_transfer().

Forgot to re-add the test assertion.

Added 'load' to the expected hooks to have run.

I suggest to trigger hook just before sending file to the client.

I'd prefer to have this kind of issue as part of distribution or patch in the issue
Only sites that need to count any metrics for private file downloads needs this hooks
Otherwise hook_menu_alter will be enough for other 99%

+++ b/includes/file.inc
@@ -1952,6 +1952,8 @@ function file_unmanaged_save_data($data, $destination = NULL, $replace = FILE_EX
 function file_transfer($uri, $headers) {
+  module_invoke_all('file_transfer', $uri, $headers);

so when someone will need to count files that delivered per sent... core will add another hook?

Any chances this will be integrated to D7 ever?
Dave, what do you think about the comments by maximpodorov and andypost?

All I need is to count private downloads. I need rough figures of initiated transfers. I don't want to count successful transfers or detect power line breaks, or something else.
One of my contrib modules already implements hook_menu_alter() for 'system/files' path but still allows to take advantage of hook_file_download().

Could someone suggest, wouldn't this be a good idea to implement hook_file_download() together with hook_module_implements_alter() in my custom module.
- hook_file_download() will do the job, e.g. watchdog(...);
- hook_module_implements_alter() will take care of my hook_file_download() to be triggered in the last turn.

I'll also add some 'weight' to that module for more reliability.
I think it's almost the same that is provided in 233997-hook-file-transfer.patch, but without patching the core. Am I right?

@Chewits, that sounds like it would work in Drupal 7 (since https://api.drupal.org/api/drupal/includes!file.inc/function/file_downlo... stops invoking additional modules' hooks once it knows that a download won't be allowed). Probably wouldn't work in Drupal 8.

It seems to me like the hook requested here could certainly be implemented in https://api.drupal.org/api/drupal/core!modules!system!src!FileDownloadCo... or similar in Drupal 8 (which matches the original issue proposal here, to have it in file_download() in Drupal 7).

For Drupal 7, I'm not sure which is better - file_transfer() is more generic, but also lower-level and gets called randomly in other scenarios (for example, image_style_deliver() calls it for image derivatives, but only when a derivative is delivered by Drupal the first time it's generated) so implementations of the hook would have to be careful to filter those other cases out if they are e.g. just trying to count private file downloads.

Repeating of #28:
I suggest to trigger hook just before sending file to the client.

@maximpodorov, that's not a very specific suggestion - do you think it should be in file_download() or file_transfer()? If we invoke it from file_download() right before file_transfer() is called then the file transfer is essentially guaranteed to happen.

Here is my patch from #2500949: Add hook_file_transfer to allow modules to react on private file downloads (it's for D7):

diff --git a/includes/file.inc b/includes/file.inc
index d3ac87e..92b1469 100644
--- a/includes/file.inc
+++ b/includes/file.inc
@@ -1964,6 +1964,7 @@ function file_transfer($uri, $headers) {
   $scheme = file_uri_scheme($uri);
   // Transfer file in 1024 byte chunks to save memory usage.
   if ($scheme && file_stream_wrapper_valid_scheme($scheme) && $fd = fopen($uri, 'rb')) {
+    module_invoke_all('file_transfer', $uri, $headers, $scheme);
     while (!feof($fd)) {
       print fread($fd, 1024);
     }

This ticket has been around for a really long time. Wonder if its still valid? looks like it still needs a fair amount of work and issue summary update.

If still a valid request please reopen.

Thanks!