reactphp / filesystem

Evented filesystem access.
MIT License
135 stars 40 forks source link

Rework file stream API #46

Open WyriHaximus opened 5 years ago

WyriHaximus commented 5 years ago

Rework file stream API if it is a bit wonky.

clue commented 5 years ago

What are your plans for the API, do you have anything in mind?

I agree that the API surface is much more complex than everything I've ever needed for my (perhaps somewhat simple) use cases. I would welcome to have something like read(string $path): PromiseInterface<string> and readStream(string $path): ReadableStreamInterface<string> more prominently instead of exposing low-level file and directory objects.

WyriHaximus commented 5 years ago

I should have work this issue out better as this is really about streaming the contents of a directory. @CharlotteDunois is already working on improving the reading and writing to and from files.

To simplify that API I'd like to collapse both into read(string $path): Observable using RxPHP. To clarify the read() method is for use cases where you only get a small set of results where readStream() is to be used when the call will yield a lot of results and will cause increased memory usage. Now when using observables we can easily do both.

For small loads:

$directoryContents = Promise::fromObservable($filesystem->read(__DIR__)->toArray());

For large loads:

$filesystem->read(__DIR__)->subscribe(function (NodeInterface $node) {
   // Do something with $node
});

Using observables we can filter out files, directories, or symlinks:

$filesystem->read(__DIR__)->filter(function (NodeInterface $node) {
    // Filter out everything by files
    return $node instanceof FileInterface;
})->subscribe(function (NodeInterface $node) {
   // Do something with $node
});

Or to go even more advanced filter our all files above 1KB:

$filesystem->read(__DIR__)->filter(function (NodeInterface $node) {
    return $node instanceof FileInterface;
})->filter(function (FileInterface $file) {
    // This example is actually more complicated because we have to deal with a promise but this is the basic idea
    return $file->size() < 1024;
})->subscribe(function (FileInterface $file) {
   // Do something with $file
});
ghost commented 5 years ago

For the File API I want to add a new method (for writing also one), which is an optimization for the adapters child process and pthreads to the current behaviour/method. This new method will collapse the current calls open -> read/write (recursive) -> close into one method call, which is an useful optimization for small files.

Currently the File nodes invokes three methods (one maybe more than once), which leads to minimum three calls to the adapter implementations. The mentioned two adapters can use file_get_contents (respectively file_put_contents), which makes the engine handle opening and closing automatically - and it removes minimum two adapter calls.

This would be used for File::getContents (and File::putContents respectively for writing). For Eio we can not do this optimization on the adapter level, but instead the adapter will invoke the three methods.

Something along this (for Eio):

function readFile($path) {
    return $this->open($path)->then(function ($fd) {
        return $this->read($fd, 0, PHP_INT_MAX)->always(function () use ($fd) {
            $this->close($fd);
        });
    });
}

@WyriHaximus This would remove a method to check whether the adapter supports this - I think this makes more sense to do (the adapter handling this instead).

WyriHaximus commented 5 years ago

@CharlotteDunois sorry for the late response, but go for it, this makes a lot of sense :+1: