Modify FTP adapter to manage long paths #430
Conversation
| @@ -1,7 +1,7 @@ | |||
| { | |||
| "name": "knplabs/gaufrette", | |||
| "name": "FaustineLarmagna/gaufrette", | |||
There was a problem hiding this comment.
This must not be part of the branch you use to submit the pull -request. Open the PR using a feature branch containing the wanted changes but not the package renaming
| @@ -14,6 +14,10 @@ | |||
| { | |||
| "name": "The contributors", | |||
| "homepage": "http://github.com/knplabs/Gaufrette/contributors" | |||
| }, | |||
| { | |||
| "name": "Faustine Larmagna", | |||
There was a problem hiding this comment.
This should also be removed. Contributors are not all listed individually here (it would be unmaintainable). This is why the previous author is linking to the list of contributors on github
There was a problem hiding this comment.
Yes. I wanted to see your comments on the code before bringing changes to this.
| @@ -54,93 +55,122 @@ public function __construct($directory, $host, $options = array()) | |||
| } | |||
|
|
|||
| /** | |||
| * {@inheritDoc} | |||
| * @param string $key | |||
| * @return bool|string | |||
|
|
||
| $temp = fopen('php://temp', 'r+'); | ||
|
|
||
| if (!ftp_fget($this->getConnection(), $temp, $this->computePath($key), $this->mode)) { |
There was a problem hiding this comment.
not using $this->computePath($key) anywhere anymore looks weird to me
There was a problem hiding this comment.
Indeed, this code is from early this year. I wanted to know if you would change the feature completely before updating it. I will make the changes on another branch.
Thanks for the review.
| return false; | ||
| } | ||
|
|
||
| rewind($temp); | ||
| $contents = stream_get_contents($temp); | ||
| //change back to ftp root directory | ||
| $this->changeDirectory($this->directory); |
There was a problem hiding this comment.
I would not put this in the middle of the code retrieving the content from $temp, for readability
|
|
||
| if (false === $lines) { | ||
| return false; | ||
| } | ||
|
|
||
| $pattern = '{(?<!->) '.preg_quote(basename($file)).'( -> |$)}m'; | ||
| $pattern = '{(?<!->) '.preg_quote(basename($key)).'( -> |$)}m'; | ||
| foreach ($lines as $line) { | ||
| if (preg_match($pattern, $line)) { |
There was a problem hiding this comment.
this matching is broken now, as $line will not contain the full path anymore
| * @throws \RuntimeException | ||
| * @return bool | ||
| */ | ||
| protected function changeDirectory($directory) |
| * | ||
| * @param string $key | ||
| */ | ||
| protected function moveToTargetDirectory($key) |
|
@FaustineLarmagna Can you update your PR as requested please ? |
In this pull request you will find the solution I used to fix an issue ( #431 ) when dealing with too long file/directory paths: move the pointer to the directory containing the target file (or directory) before doing any FTP actions. And put the pointer back to where it was before the action afterwards.
This change is