reactphp / filesystem

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

putContents doesn't truncate the file like file_put_contents #63

Closed choval closed 4 years ago

choval commented 5 years ago

React\Filesystem\Node\File::putContents @ line 158 opens without truncating option t. Hence, it writes the length of the content and leaves previous existing bytes.

It kinda goes against the regular behaviour of file_put_contents, is this intended?

$file = 'tmp.txt';
file_put_contents( $file, '000');
$promise = $fs->file( $file )->putContents( '11' );
$res = Block\await($promise, $loop);
var_dump( file_get_contents( $file ));
// returns 110 instead of 11
choval commented 5 years ago

The open('cwt') described in the README does not work as expected, but instead open('w') behaves the way documented (creates when does not exist & truncates).

WyriHaximus commented 5 years ago

That's not the intention, have you tried out the changes in #62?

choval commented 5 years ago

Yes, tried with c98de2fc4a85a46737e3c00488e7bb9e21a75b0e and still the same.

WyriHaximus commented 5 years ago

Will look into it, have some PR planned for filesystem and will fix this with those as well

ghost commented 5 years ago

c describes that if the file exists, it will not be truncated. w will automatically attempt to create the file if it does not exist, and otherwise truncate it.

'w'
Open for writing only; place the file pointer at the
beginning of the file and truncate the file to zero length.
If the file does not exist, attempt to create it. 
'c'
Open the file for writing only. If the file does not exist,
it is created. If it exists, it is neither truncated (as opposed to 'w'),
nor the call to this function fails (as is the case with 'x').

https://secure.php.net/manual/en/function.fopen.php (The behaviour of PHP open flags are effectively not the same to the POSIX ones.)

If you're using Eio, this might look a bit differently.

But this unit test on a separate branch (splitted from the PR with added truncate unit test) shows the file is truncated before writing: https://github.com/CharlotteDunoisLabs/filesystem/compare/file-put-get-rocket...CharlotteDunoisLabs:file-test-truncate https://travis-ci.org/CharlotteDunoisLabs/filesystem/builds/503252599

(The only build passing at the time of writing this comment was PHP 5.6, the others were restarted due to typical Eio timeouts and are unrelated to the described issue.)

developernaren commented 4 years ago

@WyriHaximus @choval @CharlotteDunois any update on this?

ghost commented 4 years ago

@developernaren I'm not sure where we left off, can you reproduce the issue? Maybe the File::putContents method should be updated to use AdapterInterface::putContents instead of a normal open-write cycle?

developernaren commented 4 years ago

@CharlotteDunois it has the exact behavior as the original code in the first comment.

$file = 'tmp.txt';
file_put_contents( $file, '000');
$promise = $fs->file( $file )->putContents( '11' );
$res = Block\await($promise, $loop);
var_dump( file_get_contents( $file ));
// returns 110 instead of 11
ghost commented 4 years ago

@developernaren Do you have the same behaviour with

$promise = $fs->getAdapter()->putContents($file, '11');

instead?

developernaren commented 4 years ago

@CharlotteDunois there is no putContents in AdapterInterface. I tried open method with cwt flag to truncate and write but that is also not working. I am having to delete the file and write again as a work around.

ghost commented 4 years ago

@developernaren There is in the master branch. https://github.com/reactphp/filesystem/commit/00ed432974c7191094fbdda5b2ef697fb2baa58d

developernaren commented 4 years ago

I am trying to use it in this framework https://driftphp.io/. I am not exactly sure how to fix use master as this is the dependency of a dependency.

choval commented 4 years ago

@developernaren I'll try the master branch tonite and get back to you.

choval commented 4 years ago

Still the same using the latest master 3133bd75dbab5116560ed6dc9c03f12d362224d8, which includes 00ed432974c7191094fbdda5b2ef697fb2baa58d.

If you use open('w') instead of open('cw') on the src/Node/File.php@158 of the current master, this behaves as expected.

https://github.com/reactphp/filesystem/blob/master/src/Node/File.php

From:

        return $this->open('cw')->then(function (WritableStreamInterface $stream) use ($contents) {

To:

        return $this->open('w')->then(function (WritableStreamInterface $stream) use ($contents) {

I no longer use this library. Good luck.

Test:

#!/usr/bin/env php
<?php
require('vendor/autoload.php');

$loop = \React\EventLoop\Factory::create();
$fs = \React\Filesystem\Filesystem::create($loop);

$file = 'tmp.txt';
file_put_contents( $file, 'xx');
$promise = $fs->file( $file )->putContents( 'o' );
$promise->done(
    function($r) use ($file) {
        $data = file_get_contents($file);
        if ($data === 'o') {
            echo "GOOD!\n";
        } else {
            echo "Expected 'o', but got 'xx' \n";
        }
    },
    function($e) {
        echo $e->getMessage()."\n";
    });

$loop->run();
developernaren commented 4 years ago

@choval thank you so much for trying this out. really appreciate the effort.