Had a conversation on IRC about this with aaronwinborn, pwolanin, heyrocker and others. Right now we define our own interface for read-only stream wrappers, instead of implementing DrupalStreamWrapperInterface. This is because the PHP docs say that read-only stream wrappers shouldn't define methods for things like unlink. Instead, they should leave the methods undefined so that the right error message is emitted.

Unfortunately, when you don't implement DrupalStreamWrapperInterface, Drupal doesn't register your stream wrapper for you. That means you have to register it yourself, and make sure you're not registering it twice (Gardens ran into a situation where media_youtube was somehow registering its wrapper twice in one page request, which is why I started looking into it).

As far as I can tell, there's no actual benefit to leaving the methods undefined. If you try to unlink youtube://whatever, PHP emits an E_WARNING, and returns false. It seems okay to just return false. Surely anybody trying to unlink a youtube video knows it might just possibly not work. So I suggest we go ahead and implement DrupalStreamWrapperInterface. Patch forthcoming.

CommentFileSizeAuthor
#1 928104-1-implement-dswi.patch2.4 KBksenzee

Comments

ksenzee’s picture

ksenzee
she/her
commented
Status: Active » Needs review
StatusFileSize
new 928104-1-implement-dswi.patch2.4 KB
effulgentsia’s picture

effulgentsia commented

Looks good to me. Would be great to get some feedback here from chx, pwolanin, or someone else who worked on stream wrappers in D7 core to see if this is what they intended contrib authors to do.

effulgentsia’s picture

effulgentsia commented
Status: Needs review » Fixed

In IRC, got a thumbs up from heyrocker and chx, so committed to CVS. heyrocker said "i still wish we had DrupalReadOnlyStreamWrapperInterface", so #947456: Implementing read-only stream wrapper classes is harder than it needs to be.

Status: Fixed » Closed (fixed)

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