#filefield_value_callback [#1048054]

WRT: filefield_widget.inc:214:function filefield_widget_value

/**
 * The #value_callback for the filefield_widget type element.
 */
function filefield_widget_value($element, $edit = FALSE) {
  if (!$edit) {
    $file = field_file_load($element['#default_value']['fid']);
    $item = $element['#default_value'];
  }
  else {
    $item = $edit;
    $field = content_fields($element['#field_name'], $element['#type_name']);

    // Uploads take priority over value of fid text field.
    if ($fid = filefield_save_upload($element)) {
      $item['fid'] = $fid;
    }
    // Check for #filefield_value_callback values.
    // Because FAPI does not allow multiple #value_callback values like it does
    // for #element_validate and #process, this fills the missing functionality
    // to allow FileField to be extended purely through FAPI.
    elseif (isset($element['#filefield_value_callback'])) {
      foreach ($element['#filefield_value_callback'] as $callback) {
        $callback($element, $item);
      }
    }

    // Load file if the FID has changed so that it can be saved by CCK.
    $file = field_file_load($item['fid']);

    // If the file entry doesn't exist, don't save anything.
    if (empty($file)) {
      $item = array();
    }

    // Checkboxes loose their value when empty.
    // If the list field is present make sure its unchecked value is saved.
    if (!empty($field['list_field']) && empty($edit['list'])) {
      $item['list'] = 0;
    }
  }
  // Merge file and item data so it is available to all widgets.
  if (isset($item['data']) && isset($file['data'])) {
    $file['data'] = array_merge($item['data'], $file['data']);
  }
  $item = array_merge($item, $file);

  return $item;
}

The implementation of the is a very usefull feature that I am using. I'm curious why inthe following line:

231:    elseif (isset($element['#filefield_value_callback'])) {

You've used an elseif instead of an if. This means that if the file_upload process saves a file, then the hook is never called, so it's an either-or situation. That said, if there is more than one hook, all are called regardless of what they return (no the first that returns true.)
Is there a scenario that requires/justifies this? instead of calling the callbacks regardless?

In my particular situation, I'd like to perform some processing on uploaded files (unarchives, analyze a manifest.xml inside the archive, and validate that manifest.) I can't use the file.inc validate callbacks, as one doesn't have access to the actual files before validating (they haven't been 'move_upload_file'd yet.) I can jack into the hook_file_insert hook, but that would apply to all fields, so I'd have to filter out my cases.
If the above line was an 'if', instead of the 'elseif then I'd be able to rely on it to run a single hook to evaluate my case after a file has been uploaded.

Any ideas?

I was thinking that it might be a good idea to petition a change to the core file.inc, to have a validation step after the file is made available. A validation failure could then be followed by deleting the file still? I guess that it doesn't play as well as the 'temporary' file concept?

Comments

Comment #1

quicksketch

he/him

commented 2 February 2011 at 04:14

Hm, an interesting question. I think it's an elseif because that's what I needed for FileField Sources. In that module I don't have any "sources" that both do an upload and need to set the value.

The only intended purpose of this hook was to retrieve a File ID (fid). I think the assumption is that if you've uploaded the file and core has saved it, there's no other need to determine the FID in any other way, so the #filefield_value_callback is not called.

I can't use the file.inc validate callbacks, as one doesn't have access to the actual files before validating (they haven't been 'move_upload_file'd yet.)
...
I'd like to perform some processing on uploaded files (unarchives, analyze a manifest.xml inside the archive, and validate that manifest.)

What prevents you from modifying the file while it is in the tmp directory? If you're just looking to do validation, I think #upload_validators would still work just fine, you'd just have to work on a copy of the file (the file path which you can retrieve from $_FILES['files']['tmp_name'][$field_name]).

Comment #2

jaxxed commented 2 February 2011 at 15:59

Thanks for the fast response.

I actually tried briefly playing with file as a tmp file, but I couldn' get access to it in it's {tmp} folder. Maybe I gapped out too early. After some thought though, I realized that I need to extract the contents of an archive that may be up to 100Mbs, and analyze the contents, which is too much work for a validation process really.

The thing about the elseif is that you don't exit the loop of callbacks, when one callback has found an fid, then each callback would have to test if an fid has been found in a previous callback. If the order of callbacks can change (it is just an array) then all callbacks would have to test this. If callbacks have to be aware of this, then the else isn't necessary.
In my case I will want to use your FF_sources module (I doubt that I can rely on uploading 100mb arhcives through Drupal) so I don't want to make any modifications that break it on my own FF, but maybe you'd let me submit a patch to you after testing with FF_S.

I'd moved to the value callback, as I am pulling some values and concepts from the callback function, to use as column values, as defined in my module. I'm still just getting used to the filefield innards, and so I might be trying to get it's liver to do the job of a pancreas; life is hard enough for our livers to start off with, and nobody I asked even knows what a pancreas does.
I'm using the callback to unarchive the uploaded file (first decide on a path, then unzip to that path), and then parse the contents of an xml file located in the root (check for the file, then open with simplexml, then parse out elements) then try to return certain values to the element that match the columns defined.
I've got your filedfield value hook running after changing the elseif to an if. I haven't finished my custom module yet, so it isn't yet properly setting the additional field values - but I'm on that today. Right now my code piggie backs on yours so that my unarchiving process runs after upload (including on the ahah process) and can handle the cck delta concept.

I'll touch base later. Thanks for you help

Comment #3

quicksketch

he/him

commented 2 February 2011 at 22:41

Yeah I was wondering if doing this upon upload was a good idea also. Like you say you're probably dealing with an expensive process, this would cause a rather significant delay between when the file finished transferring, uploading or whatever, and when it was accepted into the field. Perhaps doing this entire process after the upload has been accepted would be a better way to go.

Comment #4

jaxxed commented 7 March 2011 at 15:15

FYI: Your advice was sound. It didn't make sense to try to add so much processing on upload. I ended up splitting up the operations, and using your filefield widget just for the upload.