Use Drupal's filesystem wrappers when creating thumbnails

Had to fix this for a client. Pretty much nothing was using the Drupal file interface, and this is a solid step forward. When you clear the thumbnails the image styles are now updated as well. Some things like loading the files based on a uri don't appear to be part of the file interface so I wrote some queries, would love opinions on that. Looks like this touched a lot of other broken things.

Cheers!

aguilarm,

Thanks for the patch! A few things:

Please ensure all comments follow Drupal's API documentation and comment standards. This includes:

* Proper capitalization and punctuation.
* A space between // and the comment.
* Proper @return documentation for youtube_get_remote_image().

I admit the module doesn't always follow these standards as-is, but we might as well correct them moving forward.

Also, the module's current state and this patch both assume "public://" in multiple places. I believe both the s3fs (from the original post) and amazons3 modules use a wrapper of s3://, which would cause problems with this current implementation. To fix this, it might make sense to either add a file system setting at the module configuration or field configuration level, or perhaps to use whatever is set as the default in admin/config/media/file-system. It might be easiest to use the system's default (file_default_scheme_public) unless/until someone needed a more granular feature.

And as for the complex query in youtube_thumb_delete_all(), I wonder if it makes sense to connect the field value's row in the field's table to the file's row in the file_managed table by something other than the video's ID. This module has always constructed the path based on the video ID and assumed that's where the image would be, but I'm wondering if maybe saving the image's fid into the field value's table would make retrieving the thumbnail for a given field value easier. Thoughts?

@guschilds:

Comments cleaned up and refactored a little.

file_default_scheme() now used to ensure s3 compat.

I really like the concept of storing the managed file instead if youd be willing to commit it ill make a follow up issue and scaffold it out there. As this issue stands i think its still a massive improvement in file handling as a start.

Reroll to include file_save_data() somehow it was missing from my previous patch.

This patch is primarily code cleanup (see interdiff.txt) to hopefully get this to a reviewable state.

Nice work michaelfavia! And thanks for the cleanup, Tommy Cox.

There have been a number of recent commits to the 7.x-1.x branch so the patch in #7 no longer applied. I've attached a re-roll. It does work as advertised and I'd like to get this committed once I'm able to also commit it to the 8.x-1.x branch. I'll likely begin porting it today.

I've committed the patch to the 7.x-1.x branch and ported it to 8.x-1.x as well. I'll be a part of the next releases.

Thanks for the help on this one! It only took a year. :)