Global user used to set the $file->uid

We can change the $filepath parameter to accept a $file array of properties, or add an $account parameter.

I think the reason this isn't a $file object is because it's sometimes used to move files from the temporary directory to a permanent location, in which case there isn't a file object available, because it hasn't yet been saved to the database.

I think adding an $account parameter is the best approach, though it's unfortunate that we used the $uid in #419616: filefield_widget_file_path is hardcoded to global $user, because in this function we actually need the full account object to get the list of roles. Either that or we load the user, but I don't like this because user_load() isn't cached at all. Then again, it's not too likely that you'll be doing multiple moves unless you're doing an import script, in which case you probably don't need to worry so much because you're usually running it with beefy hardware.

#419616: filefield_widget_file_path is hardcoded to global $user takes the whole $account, not just uid, doesn't it?

It does! I don't know what I was thinking. That makes $account seem like the obvious choice.

I want to be able to set uid, description, title, alt and timestamp programatically.

You can always call field_file_load($fid), make any changes, then call field_file_save(), however this doesn't affect description, alt, or title, since these aren't actually properties of the file, they're just associated with the file by CCK. If you want to affect all the properties at the same time, changing them in hook_nodeapi($op = 'presave') is probably the best place. There you have access to all these properties, associated with node and the uploaded files.

If performance is an issue, use the Batch API to do the importing. It works quite well.

#419616: filefield_widget_file_path is hardcoded to global $user does take the whole account, so as well if performance is an issue I suppose your import script could cache user_load's locally as well.

In the custom module for one of my sites where I need to set the UID, I do the following:

      foreach ($filefield->fids as $fid) {
        $file = db_fetch_object(db_query("SELECT * FROM {files} WHERE fid = %d", $fid));
        $destination = filefield_widget_file_path($content_type['fields'][$filefield->field], $user);
        file_check_directory($destination, TRUE);
        file_copy($file->filepath, $destination, FILE_EXISTS_RENAME);
        $file->uid = $user->uid;
        unset($file->fid);
        drupal_write_record('files', $file);
        $node->{$filefield->field}[$field_index++] = (array)$file;
        // The user still hasn't made any changes, so keep this node linked to
        // it's associated template.
        $node->skeleton_template->keep_connected = TRUE;
        node_save($node);
      }

I'm not sure I like this code (which is why ATM it's in the site custom module and not as a feature in the Skeleton module yet), but it seems to work fine. Operations like this should be much better in D7 with files being true objects, but for D6 I think the above is the closest you'll get.

It still wouldn't hurt to make this suggested change, it doesn't feel right hard-coding the global $user in there.

Maybe we can add a $file object instead?

My response in #1:

I think the reason this isn't a $file object is because it's sometimes used to move files from the temporary directory to a permanent location, in which case there isn't a file object available, because it hasn't yet been saved to the database.

Besides, we're not making drastic API changes, we're trying to keep changes as minimal as possible (we're "frozen" now, new APIs are for FileField 4.x, which probably won't ever happen since we'll just put FileField into core). So no, we're not going to make sweeping changes to the APIs. We'll add a simple $account parameter which simply swaps out the global $user if passed in.

field_file_save_file has $user->roles as well, so either a full $account, or having to to use user_load() is needed.

Yep, a good reason for using the $account parameter IMO. I think this makes for the best scenario when we'll want to avoid unnecessary user_load() calls, since like I mentioned, user_load() isn't cached, so inserting 100 files by the same user would load the user object 100 times.

Yes, good APIs are good. However, this should have been addressed during the 2 years the 3.0 version was in development. It's too late to make large changes now. If you'd like to see better APIs in the future, open an issue against Drupal 7 HEAD and request for the changes to be made there. I think adding $account is simply accounting for a bug in the API, not actually making a change.

I've committed the attached patch to overcome this limitation. I also removed the variable_get() lines dealing with upload extensions, since it turns out after this data was collected, nothing was ever done with it. It's been like that since the function was originally added in #289335: Add field_file_save_file() to create files from the filesystem. Since this function is intended to be used as an API, I don't think it important that we change it's functionality to make it match core's file_save_upload() and munge the filename when moving it. Added a few additional docs while we're in here.