barryvdh / elfinder-flysystem-driver

elFinder driver for Flysystem
183 stars 41 forks source link

[BUG] Can't open folders #44

Closed adridev closed 8 years ago

adridev commented 8 years ago

Hi @barryvdh I'm having troubles with the Driver. When I try to open a folder "Folder not found." is returned.

ENV: barryvdh/elfinder-flysystem-driver v0.2.0
barryvdh/laravel-elfinder dev-master d9df120

league/flysystem 1.0.24
league/flysystem-aws-s3-v3 1.0.12
league/flysystem-cached-adapter 1.0.3

laravel/framework v5.2.39

I found the solution for my problem. If I remove the following snipped:

// return empty on failure
if (!$meta) {
    return array();
}

from vendor/barryvdh/elfinder-flysystem-driver/src/Driver.php:284 it works perfectly.

I just start using your library so I am not sure if this change will break something. Let me know about before I submit a pull request.

EDIT: Mention that I'm using an S3 bucket as Disk.

Thanks.

barryvdh commented 8 years ago

Not really sure, if the $meta is empty, I assume the path doesn't exist. Can you test what the actual output is when just using the flysystem adapter, to load that folder?

adridev commented 8 years ago

Hey @barryvdh thanks for the response.

I'm pretty sure the response would be false, I tested It with the following code:

    $client = S3Client::factory([
        'credentials' => [
            'key'    => env('AWS_ACCESS_KEY_ID'),
            'secret' => env('AWS_SECRET_ACCESS_KEY'),
        ],
        'region' => env('AWS_REGION'),
        'version' => 'latest',
    ]);
    $adapter = new \League\Flysystem\AwsS3v3\AwsS3Adapter($client, env('AWS_CLOUD_BUCKET'), '', []);

    $s3 = new League\Flysystem\Filesystem($adapter);

    dd($s3->getMetadata('path'));

Let me make the PR for you can test the changes.

Cheers.

barryvdh commented 8 years ago

But the path exists? Isn't that an issue with Flysystem? Or is that accepted behaviour for folders?

adridev commented 8 years ago

Ah, ok, truth.

When the path doesn't exist an exception of type FileNotFoundException is thrown and then catch block returns an empty array.

barryvdh commented 8 years ago

But the path does exists?

adridev commented 8 years ago

Yes, when it doesn't, the exception is thrown.

barryvdh commented 8 years ago

And what does 'has()' say? Cause we have a fallback for when the directory isn't returned:

// If not exists, return empty
        if (!$this->fs->has($path)) {
            // Check if the parent doesn't have this path
            if ($this->_dirExists($path)) {
                return $stat;
            }
            // Neither a file or directory exist, return empty
            return array();
        }

So perhaps we should add the check when $meta is empty to.

adridev commented 8 years ago

'has()' returns true if the folder exists and false if it doesn't.

barryvdh commented 8 years ago

Hmm, but I would assume that if has() returns true, it would also return something in $meta..

adridev commented 8 years ago

Seems like not for dirs. Here is a discussion about it.

https://github.com/thephpleague/flysystem-aws-s3-v3/issues/46

adridev commented 8 years ago

we can remove this fallback too, and return an empty array instead.

// Check if the parent doesn't have this path
if ($this->_dirExists($path)) {
    return $stat;
}

I'm right?

barryvdh commented 8 years ago

No don't think so, because not every adapter works correctly with has()

adridev commented 8 years ago

ahh, Ok.

adridev commented 8 years ago

I will close the issue, thanks @barryvdh.

Fuitad commented 8 years ago

Based on https://github.com/Studio-42/elFinder/issues/1589, is there any chance this issue could be reopened and a fix found? I've been plagued by this issue for months and this simple fix solved it for me 0_o

bart commented 7 years ago

Same question here. I would like to use elfinder with S3 in Laravel 5.3 but due to this issue it's not possible. I can't quick fix Driver.php:284 because when deploying via envoyer all dependencies get updated. Any solution for this?

barryvdh commented 7 years ago

Can you submit a PR that checks the following: Works for local Works for AWS Prevents double making of directory (eg. make dir with same name) Can access folders.

bart commented 7 years ago

I don't know why, but when I use the roots config part in elfinder.php it works well without commenting the 3 lines of $meta check. Can anyone confirm this?