On the file/add form, we need some way to alter the upload location for the uploaded file. For example, the AWS S3 SDK requires a bucket in all S3 URLs. When creating a new file, if the field is passed to us by media modules, then other modules can alter the URI as needed.

Obviously this would be best as a core API, but for now an alter hook seems the least invasive and most likely to make it in for D7 sites.

CommentFileSizeAuthor
#4 2479483-4.patch481 bytesbenjy
#2 2479483.2-uri-alter.patch1.58 KBdeviantintegral

Comments

deviantintegral’s picture

deviantintegral commented
Issue summary: View changes
deviantintegral’s picture

deviantintegral commented
StatusFileSize
new 2479483.2-uri-alter.patch1.58 KB
dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented

Would it help if we called file_stream_wrapper_uri_normalize() on this URI string?

benjy’s picture

benjy commented
StatusFileSize
new 2479483-4.patch481 bytes

The solution @Dave mentions in #3 is what I came up with when debugging this issue, patch attached.

muhaha03’s picture

muhaha03 commented

I don't have file_entity.api.php.
Where is it?

deviantintegral’s picture

deviantintegral commented

The S3 module supports per-field bucket destinations for S3 buckets, so we need $params['field'] and $params['file_directory']. I don't think just calling file_stream_wrapper_uri_normalize() will do it unless you're always using the site-wide default bucket.

quicksketch’s picture

quicksketch
he/him
commented
Status: Needs review » Needs work

I don't think just calling file_stream_wrapper_uri_normalize() will do it unless you're always using the site-wide default bucket.

+1 for @deviantintegral's comment here. file_stream_wrapper_uri_normalize() helps for single-bucket installations (e.g. for the 1.x version of AmazonS3 module) but for sites with multiple buckets you need the field information as well, so the alter hook is still needed. For users of the 2.x branch of AmazonS3, the current #2 patch is sufficient (but #4 is not). Seems like the ideal patch would include both changes.