Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] Can't open folders #44

Closed
adridev opened this issue Jun 20, 2016 · 18 comments
Closed

[BUG] Can't open folders #44

adridev opened this issue Jun 20, 2016 · 18 comments

Comments

@adridev
Copy link
Contributor

adridev commented Jun 20, 2016

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.

@adridev adridev changed the title Can't open folders [BUG] Can't open folders Jun 21, 2016
@barryvdh
Copy link
Owner

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
Copy link
Contributor Author

adridev commented Jun 22, 2016

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
Copy link
Owner

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

@adridev
Copy link
Contributor Author

adridev commented Jun 22, 2016

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
Copy link
Owner

But the path does exists?

@adridev
Copy link
Contributor Author

adridev commented Jun 22, 2016

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

@barryvdh
Copy link
Owner

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
Copy link
Contributor Author

adridev commented Jun 22, 2016

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

@barryvdh
Copy link
Owner

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

@adridev
Copy link
Contributor Author

adridev commented Jun 22, 2016

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

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

@adridev
Copy link
Contributor Author

adridev commented Jun 22, 2016

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
Copy link
Owner

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

@adridev
Copy link
Contributor Author

adridev commented Jun 22, 2016

ahh, Ok.

@adridev
Copy link
Contributor Author

adridev commented Jun 22, 2016

I will close the issue, thanks @barryvdh.

@Fuitad
Copy link

Fuitad commented Aug 12, 2016

Based on Studio-42/elFinder#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
Copy link

bart commented Sep 21, 2016

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
Copy link
Owner

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
Copy link

bart commented Sep 21, 2016

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants