Skip to content
This repository has been archived by the owner on Oct 1, 2023. It is now read-only.

[IO] add handle copy function #120

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions src/io/copy.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
<?hh
/*
* Copyright (c) 2004-present, Facebook, Inc.
* All rights reserved.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
*/

namespace HH\Lib\IO;

use namespace HH\Lib\Str;

/**
* Copies content from the given $source read handle to the $target write handle
* until the eol of $source is reached.
*
* If `$chunk_size` is `null`, there is no limit on the chunk size - this function will
* copy the content of $source all at once.
*/
async function copy(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To match the naming conventions, copyAsync() or is this handled by the gen rewriter?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure about this. other functions in File\ don't use the _async suffix.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it should be copy_async

Other functions in File\ do not return Awaitables, so do not have the suffix (if that's not the case, it's a bug that needs fixing)

<<__AcceptDisposable>> ReadHandle $source,
<<__AcceptDisposable>> WriteHandle $target,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personal preference, but isn't $source $destination a more natural pair?

?int $chunk_size = null,
azjezz marked this conversation as resolved.
Show resolved Hide resolved
?float $timeout_seconds = null,
): Awaitable<void> {
if (!$source->isEndOfFile()) {
$content = await $source->readAsync($chunk_size, $timeout_seconds);
await $target->writeAsync($content, $timeout_seconds);
}

if (!$source->isEndOfFile()) {
azjezz marked this conversation as resolved.
Show resolved Hide resolved
await copy($source, $target, $chunk_size, $timeout_seconds);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess you implemented this via recursive call because of the DontAwaitInALoopLinter?
I guess we could violate this here, since this has the exact same performance characteristics.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess you implemented this via recursive call because of the DontAwaitInALoopLinter?

yes

I guess we could violate this here, since this has the exact same performance characteristics.

why violate the linter rule if we can avoid it? 😛

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suppress the linter.

why violate the linter rule if we can avoid it?

Because violating the linter rule doesn't consume stack frames, leading to fatals on large files ;)

}
}
45 changes: 45 additions & 0 deletions tests/io/CopyTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
<?hh // strict
/*
* Copyright (c) 2004-present, Facebook, Inc.
* All rights reserved.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
*/

use namespace HH\Lib\IO;
use namespace HH\Lib\File;

use function Facebook\FBExpect\expect; // @oss-enable
use type Facebook\HackTest\HackTest; // @oss-enable
// @oss-disable: use type HackTest;

// @oss-disable: <<Oncalls('hack')>>
final class CopyTest extends HackTest {
public async function testCopy(): Awaitable<void> {
await using $source = File\temporary_file();
await using $target = File\temporary_file();

await $source->writeAsync('Hello, World!');
await $source->seekAsync(0);

expect($source->isEndOfFile())->toBeFalse();
expect($target->isEndOfFile())->toBeFalse();
azjezz marked this conversation as resolved.
Show resolved Hide resolved

expect(await $target->readAsync())->toBeSame('');

await IO\copy($source, $target);

expect($source->isEndOfFile())->toBeTrue();
expect($target->isEndOfFile())->toBeTrue();

// Assert that the content has been copied.
await $target->seekAsync(0);
expect(await $target->readAsync())->toBeSame('Hello, World!');

// Assert that the original file contains the same content. not modifications are made.
await $source->seekAsync(0);
expect(await $source->readAsync())->toBeSame('Hello, World!');
}
}