Active
Project:
Drupal core
Version:
main
Component:
file system
Priority:
Minor
Category:
Task
Assigned:
Unassigned
Reporter:
Created:
6 Jun 2015 at 17:43 UTC
Updated:
19 Apr 2024 at 19:40 UTC
Jump to comment: Most recent
Problem/Motivation
PHP Stream wrappers don't have to implement an interface. There's a reason for this. PHP determines what features a stream wrapper supports by what methods it implements:
http://php.net/manual/en/streamwrapper.mkdir.php
http://php.net/manual/en/streamwrapper.rename.php
http://php.net/manual/en/streamwrapper.rmdir.php
Drupal has an extended Drupal\Core\StreamWrapper\StreamWrapperInterface which implements some extra methods for doing Drupally things. This is odd, but fine. The problem is that StreamWrapperInterface extends PhpStreamWrapperInterface making it painful to register stream wrappers with Drupal that only support a subset of functionality.
I should be able to register a stream wrapper that only allows iterating directories, dir_closedir(), dir_opendir(), dir_readdir(), dir_rewinddir(), and register it with Drupal. Granted, this will be mostly useless, but the only thing stopping me is that I have to implement a bunch of other methods that return FALSE. That will work, but as the links above show, is incorrect.
This also makes things easier in the future when new stream wrapper methods get added for newer versions of PHP. See #2107287: PHP 5.4 calls a new stream_metadata() method on stream wrappers not implemented by Drupal
We should be able to just add support for the newer features and not have to create a sub-interface for them.
Proposed resolution
- Remove PhpStreamWrapperInterface.
- If we really decide that we need an interface, create a LocalStreamWrapperInterface that requires the functionality necessary to implement local stream wrappers.
Remaining tasks
Write patch if agreed.
User interface changes
None.
API changes
API is less restricted. All existing code works fine, unless a Drupal module directly implements PhpStreamWrapperInterface. (Not likely)
Comments
Comment #1
twistor commentedComment #2
dawehnerWon't it be enough to have the additional methods in the interface, but not extend the other interface?
Comment #11
catchRemoving the inheritance would break typehints on PhpStreamWrapperInterface, but if no-one does this it might be OK.
However we'd need to move all the methods of PhpStreamWrapperInterface over to StreamWrapperInterface then deprecate anything we don't want to be on there for Drupal 10.
Not really sure what the use case for this is, and moving to a task.
Comment #17
lisa.rae commented@catch The use case would be to open up the use of the StreamWrapperManager for managing stream wrappers that may not necessarily need to be 100% compliant with PhpStreamWrapperInterface.
Right now StreamWrapperManagerInterface defines certain parameters for methods that explicitely expect entities that comply with StreamWrapperInterface. By removing PhpStreamWrapperInterface from the dependency string this opens up the ability to do so.
There are multiple places in Drupal Core where multiple interfaces are implemented. The current entities that implement StreamWrapperInterface as it is defined today would simply need to be modified to also implement PhpStreamWrapperInterface.
All of the methods on StreamWrapperInterface except for two (which IMO should not be there, see the discussion on the related issue) are designed explicitely for registering a StreamWrapper entity with a StreamWrapperManager implementing StreamWrapperManagerInterface. By decoupling StreamWrapperInterface from PhpStreamWrapperInterface you make the StreamWrapper Management component in Drupal Core much more flexible, and would allow for implementing alternative File systems (Like a NoSQL implementation) for Drupal while still taking advantage of the Stream Wrapper Management component.
It's all about decoupling.