[ticket/13697] Moving filesystem related functions to filesystem service #3487
Conversation
|
I am not sure adding an interface for a helper class is all that useful. |
|
Furthermore, I don't think it is particularly useful to have methods that just call into symfony and then do exception conversion. Why not just call into symfony in the first place? |
|
Exception wraping is because symfony throws hardcoded exceptions so language independency. Also we only meed to add one dependency for filesystem. The interface exists because IRC discussion with @Nicofuma |
CHItA
force-pushed
the
ticket/13697
branch
3 times, most recently
from
4ba6942
to
a23449d
Compare
| /** | ||
| * A class with various functions that are related to paths, files and the filesystem | ||
| */ | ||
| class filesystem |
There was a problem hiding this comment.
please keep an empty and deprecated class extending \phpbb\filesystem\filesystem (to be removed: 3.3) for BC purpose
|
Well, as @MateBartus said we need to wrap the methods if we want to make the exception translatable. And by the way adding an interface could be a good idea because it allows us to change the real filesystem transparently (to store the avatars on AWS per example) |
|
@bantu is the interface okay with you then? Also please double check phpbb_own_realpath and resolve_path functions as they were rewritten from scratch, and basically impossible to properly unit test them, I did my best to check if they work correctly. |
| } | ||
|
|
||
| // Use directory permissions on symlinks as well | ||
| if (is_dir($file) || is_link($file)) |
There was a problem hiding this comment.
Not sure if chmoding is a good idea, it works on macs for sure, but most linux systems will result in error as links only allowed with 0777.
CHItA
force-pushed
the
ticket/13697
branch
5 times, most recently
from
98d6799
to
a027ae9
Compare
|
|
||
| if ($is_absolute_path) | ||
| { | ||
| if (strpos(strtolower(PHP_OS), 'win') !== false) |
There was a problem hiding this comment.
Someone knows what is the best way to replace this? defined('PHP_WINDOWS_VERSION_MAJOR')?
CHItA
force-pushed
the
ticket/13697
branch
4 times, most recently
from
1478432
to
dfd1257
Compare
|
please rebase |
| @@ -184,7 +184,7 @@ static public function methods() | |||
| } | |||
|
|
|||
| /** | |||
| * Zip creation class from phpMyAdmin 2.3.0 (c) Tobias Ratschiller, Olivier M�ller, Lo�c Chapeaux, | |||
| * Zip creation class from phpMyAdmin 2.3.0 (c) Tobias Ratschiller, Olivier M�ller, Lo�c Chapeaux, | |||
There was a problem hiding this comment.
please revert and make sure to save as UTF8 without bom
CHItA
force-pushed
the
ticket/13697
branch
2 times, most recently
from
e6821fb
to
c2186e7
Compare
| @@ -184,7 +184,7 @@ static public function methods() | |||
| } | |||
|
|
|||
| /** | |||
| * Zip creation class from phpMyAdmin 2.3.0 (c) Tobias Ratschiller, Olivier M�ller, Lo�c Chapeaux, | |||
| * Zip creation class from phpMyAdmin 2.3.0 (c) Tobias Ratschiller, Olivier Müller, Loïc Chapeaux, | |||
There was a problem hiding this comment.
actually it seems to be a github issue
There was a problem hiding this comment.
probably yes, the file is UTF-8 encoded for sure
* Moving filesystem service to \phpbb\filesystem namespace
* Wraping Symfony's Filesystem component
* Moving filesystem related functions from includes/functions.php
into \phpbb\filesystem\filesystem
Functions moved (and deprecated):
- phpbb_chmod
- phpbb_is_writable
- phpbb_is_absolute
- phpbb_own_realpath
- phpbb_realpath
* Adding interface for filesystem service
PHPBB3-13697
|
As far as I'm concerned every above mentioned issue is resolved now. |
[ticket/13697] Moving filesystem related functions to filesystem service
https://tracker.phpbb.com/browse/PHPBB3-13697
into \phpbb\filesystem\filesystem
Functions moved (and deprecated):
PHPBB3-13697