There are times when you need to display media objects that have not yet been saved to the database, for example when building a browser searching through youtube videos or when displaying module files not intended to be saved to the database. Patch forthcoming.

Comments

aaron’s picture

Status: Active » Needs review
StatusFileSize
new1.22 KB
aaron’s picture

Priority: Normal » Major

Without this function in a release, I suspect we'll begin to see some otherwise unnecessary code duplication in all the provider files (youtube, vimeo, flickr, brightcove, etc), as we begin to pull in remote streams that haven't yet been saved to the db. Thus, I'm bumping it up to major.

However, as it's not used in the core of Media, it's a little tricky to review. I suggest using the devel module, with something like the following at /devel/php (assuming the Media: YouTube module is installed). Test it both before and after adding the media to the database.

  dpm(media_uri_to_object('youtube://v/lZ-s3DRZJKY'));
aaron’s picture

While we're at it, perhaps we need to follow the core model for entities (at least nodes) and add ->is_new or some other flag if the object doesn't currently exist in the db...

aaron’s picture

though that likely belongs in file_uri_to_object().

for that matter, file_uri_to_object() should probably take care of all this anyway. it's been over a year, and i brought that up at http://drupal.org/node/685818. too bad we didn't push that more in core...

aaron’s picture

thinking about it more, i think we need to push all of this into file_uri_to_object(). then that will be the callback of choice, and as it already exists in the module, it won't cause as many issues going forward. new patch forthcoming.

aaron’s picture

StatusFileSize
new1.76 KB

this patch moves it to file_uri_to_object() and adds ->is_new when needed. i'll update the core issue as well, though it's obviously too late to include that there for d7.

aaron’s picture

The cool thing is that this change should also make it easier to get rid of that annoying "You have entered a URL for a video that is already in your library." message.

aaron’s picture

Issue tags: +Needs tests

this should be testable as well. not necessary for a release obviously. updating the tags.

effulgentsia’s picture

StatusFileSize
new2.31 KB

I like it, but I'm nervous about breaking BC of code that calls file_uri_to_object() expecting a new $file (with no 'fid'), especially since file_save() invokes different logic based on whether an fid is set. Would this approach work for you, or does it fail to achieve what you're intending to make possible?

aaron’s picture

yes, that would be acceptable. thanks, @effulgentsia. should be safe in my use-case, as the file objects shouldn't be saved at that point in the game.

effulgentsia’s picture

Title: Load a media object from a given URI » Need tests for file_uri_to_object()
Category: feature » task
Priority: Major » Normal
Status: Needs review » Needs work
StatusFileSize
new2.32 KB

I committed this to HEAD (changed #9 to use isset() where needed), but I agree with #8 that tests are still needed.

effulgentsia’s picture

Title: Need tests for file_uri_to_object() » Write tests for file_uri_to_object()
Component: Code » Tests
effulgentsia’s picture

Title: Write tests for file_uri_to_object() » Write unit tests for file_uri_to_object()
Issue tags: -Needs tests

Ignore the spam. Just trying to organize the issue queue a bit.

effulgentsia’s picture

Status: Needs work » Active
effulgentsia’s picture

Title: Write unit tests for file_uri_to_object() » Load a media object from a given URI
Component: Tests » Code
Category: task » feature
Priority: Normal » Major
Status: Active » Fixed

Resetting issue attributes to match original issue. Since we're so low on test coverage, I'm tracking tests that are needed as their own tasks. The one for this is #1026538: Write unit tests for file_uri_to_object(). Eventually, with any luck, we'll have good baseline test coverage, and can then require patches to come with their own tests, as is the standard policy for Drupal core.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.