barryvdh / elfinder-flysystem-driver

elFinder driver for Flysystem
183 stars 41 forks source link

Serialization of Closure is not allowed #10

Closed phazei closed 9 years ago

phazei commented 9 years ago

Local file system roots seem to work just fine, but I created a s3 root with this config:

    'roots' => [
        [
            'driver' => 'Flysystem', // driver for accessing file system (REQUIRED)
            'alias' => 's3bucket',
            'URL' => url('upload/media'), // URL to files (REQUIRED)
            'adapter' => new AwsS3Adapter(new S3Client(['credentials'=>['key'=>'xxx','secret'=>'xxx'],'region'=>'','version'=>'latest']), 'mybucket'),
        ]
    ],

And in: /var/www/html/vendor/barryvdh/elfinder-builds/php/elFinder.class.php line 248 it throws the error "Serialization of 'Closure' is not allowed" I'm using Laravel 4.2 as well as "league/flysystem-aws-s3-v3"

It seems the AWSS3 adapter class has closures in it. Is it not compatible, or should the adapter not be in the place it is?

barryvdh commented 9 years ago

Pushed a new tag for elfinder-builds, using json_encode instead of serialize. Does that fix it? https://github.com/barryvdh/elfinder-builds/commit/f11eb97d55ad16cdb96b6e41c2d9ffc4e697303c

phazei commented 9 years ago

Yes, now I don't get that error anymore. Although, now, when elFinder loads, it does show the local root, but there's nothing else, doesn't show anything for S3 root.

phazei commented 9 years ago

I stepped through it and it doesn't appear to ever attempt to load Flysystem from the adapter provided...? Maybe I missed it?

Edit: Seems like the init should check if adapter exists, and if it does, build a FilesystemInterface from it...

phazei commented 9 years ago

I got S3 working when I switched it to using a Filesystem rather than an adapter.

Also important note: Although "US Standard" is seemingly regionless, it does have to be "us-east-1". It seems Flysystem eats some exceptions.

barryvdh commented 9 years ago

That's not something I can fix, right?

phazei commented 9 years ago

Nope, just a note, I think it's something the aws s3 would need to just clarify better.