thephpleague / flysystem

Abstraction for local and remote filesystems
https://flysystem.thephpleague.com
MIT License
13.35k stars 826 forks source link

Download/Upload-Progress #23

Closed marcj closed 10 years ago

marcj commented 10 years ago

When working with mid-size or large files it's actual always required to have a kind of progress information. Especially, when the server has not a big bandwidth or the connection is pretty busy and you want to inform your client/user with a progress-bar while uploading/downloading.

What do you think about providing such a API? I don't know whether all adapters are able to support that or if we have to emulate it somehow.

I guess a event-based system would be a good method to achieve that.

Example:

$fs = new Filesystem();
$fs->on('upload', function($bytes, $totalBytes, $path){
    echo sprintf("%s: %d of %d\n", $path, $bytes, $totalBytes);
});

$fs->write('bla', 'hossa');

What do you think?

frankdejonge commented 10 years ago

@marcj this would be awesome, if you had a socket connection and all the drivers would support upload progress callbacks. Unfortunately this is not the case, none of the drivers do, so it's impossible to implement.

marcj commented 10 years ago

Well, I don't know why you've immediately closed this issue, but I did some research.

ftp: http://stackoverflow.com/questions/2885468/getting-ftp-put-progress s3: uses curl, so we could overwrite the request class and use CURLOPT_PROGRESSFUNCTION. dropbox: progress is already supported in OAuth/Curl (Dropbox_OAuth_Curl) class (property ProgressFunction) webdav: possible with curl progress as well by overwriting Sabre\HTTP\Client::curlRequest

Zip and Local doesn't require that progress I guess, so all adapters are able to support it.

frankdejonge commented 10 years ago

@marcj I was a little quick on the draw, so re-opened the issue. I think your example confused me a little by using write instead of writeStream?

frankdejonge commented 10 years ago

We're using the official Dropbox API package, which doesn't support this I think

frankdejonge commented 10 years ago

@marcj ping! Any more idea's about this?

marcj commented 10 years ago

Actually all what I've mentioned is included in the packages of the adapter libs. Would be a bit of work to get all done tho', but should work then.

frankdejonge commented 10 years ago

@marcj I'm closing this issue since this is not something I need myself. If you want to implement it yourself a PR is always welcome. Also I think this is something that can easily be injected by creating a stream wrapper which would comply to the streamWrapper interface and using that to emit the progress events.

frankdejonge commented 10 years ago

@XApp-Studio I'd suggest looking into this: http://php.net/manual/en/function.stream-notification-callback.php which you can use to open a stream locally and track the progress as the stream is consumed by flysystem.

frankdejonge commented 10 years ago

@XApp-Studio The last release features an EventableFilesystem class which exposes before and after events for every call.

frankdejonge commented 10 years ago

@XApp-Studio it's not only for the event stuff, also for adapters like WebDAV moving to 5.4 as a minimum. AWS will up their version in the near future, as has Guzzle.

GrahamCampbell commented 10 years ago

I disagree. Many people are upgrading to php 5.5 straight from php 5.3. PHP 5.3 is eol anyway.

GrahamCampbell commented 10 years ago

I don't know how representative of our audience those stats are. The number of php servers out there is very large. Hosting companies still limited to 5.2/5.3 are not aimed at high end users.

GrahamCampbell commented 10 years ago

As far as I see it, the only reason these hosting companies are getting away with it is because nobody does anything about it. If we all force php 5.4, then their users will complain, and we will all move forward. This discussion has diverged considerably though, so if you want to support php 5.3, that's fine, but I don't the main flysystem repo is planning on changing any time soon.

frankdejonge commented 10 years ago

There are definitely two sides to this story. As @GrahamCampbell mentioned, our users are indeed far less in the "shared hosting" side of the spectrum. It's a given that when you stay behind on PHP versions you won't be able to benefit from the new possibilities that are made available for package maintainers when raising the minimum PHP version. Also, Flysystem is still usable for people on 5.3, they just have to use the 0.4.x version, which already was pretty stable. Frameworks like Laravel, Aura and FuelPHP have already upped their PHP version to 5.4+, as have many other popular packages. I see no reason to stay behind.

frankdejonge commented 10 years ago

We all agree on that then :dancer:

justinmccombs commented 10 years ago

Any further direction on how to implement this, or at least how to extract the stream context from the putStream method so the progress can be monitored? I feel like I'm close but just missing a crucial step. Here is the class I'm writing:

class AwsFileUploader
{
    // Dependency Injected AWS S3 Connection:
    // @var League\Flysystem\EventableFilesystem
    protected $connection;

    public function putFile($localPath, $storagePath)
    {
        $stream = fopen($localPath, 'r+');

        $this->connection->addListener('before.putstream', function(Before $event) {
            // How do I allow stream_context_set_params to access the putstream context?
        });

        return $this->connection->putStream($storagePath, $stream);
    }
}
frankdejonge commented 10 years ago

@justinmccombs you'd have to create a stream wrapper for this. You can use fstat to get the size of the stream and see track the process internally, emitting events along the way. This is the way to go since it doesn't require additional hooks or settings.

frankdejonge commented 10 years ago

@XApp-Studio I hope to have a proof of concept up for this by tomorrow. Progress doesn't have anything to do with the internals. It's got everything to do with the stream, which can be wrapped. I'll ping this issue when it's done.

frankdejonge commented 10 years ago

Btw, this is only for uploads, not downloads.

dinamic commented 9 years ago

ping @FrenkyNet

frankdejonge commented 9 years ago

haha, sorry guys, this totally got snowed under... I'll see what I can do about that. My schedule seems to have cleared up and I might have a full day next week (happy happy joy joy).

dinamic commented 9 years ago

If you can describe the steps we can take to do that you may postpone it for a bit more :)

I will be using it for FTP connection.

frankdejonge commented 9 years ago

@dinamic it's a bit difficult to do that, it's mostly because I'll be spiking it out (research + coding, then refactoring).

frankdejonge commented 9 years ago

I spiked something out over the weekend:

<?php

class StreamReadProgressWrapper
{
    /** @var resource */
    public $context;
    /** @var StreamInterface */
    private $stream;

    const PROTOCOL_NAME = 'flysystemreadprogress';

    public static function ensureWrapperIsRegisered()
    {
        if ( ! in_array(self::PROTOCOL_NAME, stream_get_wrappers())) {
            stream_register_wrapper(self::PROTOCOL_NAME, __CLASS__);
        }
    }

    public static function wrapStream($stream, callable $listener)
    {
        static::ensureWrapperIsRegisered();

        $context = stream_context_create([
            self::PROTOCOL_NAME => [
                'stream' => $stream,
                'listener' => $listener,
            ],
        ]);

        return fopen(self::PROTOCOL_NAME . '://stream', fstat($stream)['mode'], null, $context);
    }

    public function stream_open($path, $mode, $flags, &$opened_path)
    {
        $options = stream_context_get_options($this->context);

        if ( ! isset($options[self::PROTOCOL_NAME])) {
            return false;
        }

        $this->listener = $options[self::PROTOCOL_NAME]['listener'];
        $this->stream = $options[self::PROTOCOL_NAME]['stream'];
        $this->size = fstat($this->stream)['size'];

        return true;
    }

    public function stream_close()
    {
        return fclose($this->stream);
    }

    public function stream_eof()
    {
        return feof($this->stream);
    }

    public function  stream_read($count)
    {
        $response =  fread($this->stream, $count);
        call_user_func($this->listener, $this->size, ftell($this->stream), $count);

        return $response;
    }

    public function stream_seek($offset , $whence = SEEK_SET)
    {
        return fseek($this->stream, $offset, $whence);
    }

    public function stream_stat()
    {
        return fstat($this->stream);
    }

    public function stream_tell()
    {
        return ftell($this->stream);
    }

    public function stream_truncate($size)
    {
        return ftruncate($this->stream, $size);
    }

    public function stream_write($data)
    {
        return fwrite($this->stream, $data);
    }
}

You use it by wrapping the stream. But after spiking this out I realised this is mostly useless because it only tracks the stream reads, not the actual curl stuff. I think we're kind of tied to what the adapters expose (which is limited). The thing where it gets really tricky is to define the lowest common denominator. The aws sdk v3 is the only one that seems to emit anything related to what we're requesting.