Skip to content

Commit e16324d

Browse files
norberttechnorberttech
committed
refactor: cover async aws s3 fs bridge with mago
1 parent acccbdf commit e16324d

12 files changed

Lines changed: 78 additions & 63 deletions

File tree

‎Justfile‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ lint-actions:
5151
set -uo pipefail
5252
rc=0
5353
actionlint || rc=$?
54-
zizmor .github/workflows || rc=$?
54+
zizmor --offline .github/workflows || rc=$?
5555
exit $rc
5656
5757
# Run static analysis (PHPStan).
@@ -75,7 +75,8 @@ analyze-mago *args:
7575
src/bridge/openapi/specification \
7676
src/bridge/psr3/telemetry \
7777
src/bridge/symfony/http-foundation-telemetry \
78-
src/bridge/telemetry/otlp
78+
src/bridge/telemetry/otlp \
79+
src/bridge/filesystem/async-aws
7980

8081
# Auto-fix code style (Mago format + lint --fix) and GitHub Actions findings (zizmor --fix).
8182
fix:

‎src/bridge/filesystem/async-aws/composer.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
"require": {
1717
"php": "~8.3.0 || ~8.4.0 || ~8.5.0",
1818
"flow-php/filesystem": "self.version",
19+
"flow-php/types": "self.version",
1920
"async-aws/s3": "^2.6 || ^3.0"
2021
},
2122
"config": {

‎src/bridge/filesystem/async-aws/src/Flow/Filesystem/Bridge/AsyncAWS/AsyncAWSS3DesintationStream/AsyncAWSS3BlockLifecycle.php‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313

1414
use function fclose;
1515
use function fopen;
16-
use function is_resource;
1716
use function unlink;
1817

1918
final readonly class AsyncAWSS3BlockLifecycle implements BlockLifecycle
@@ -47,9 +46,7 @@ public function filled(Block $block): void
4746
*/
4847
$etag = $uploadPartResponse->getETag();
4948

50-
if (is_resource($handle)) {
51-
fclose($handle);
52-
}
49+
fclose($handle);
5350

5451
unlink($block->path()->path());
5552

‎src/bridge/filesystem/async-aws/src/Flow/Filesystem/Bridge/AsyncAWS/AsyncAWSS3DestinationStream.php‎

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -50,10 +50,6 @@ public static function openAppend(
5050
BlockFactory $blockFactory = new NativeLocalFileBlocksFactory(),
5151
int $blockSize = 1024 * 1024 * 4,
5252
): self {
53-
if ($blockSize < 1) {
54-
throw new InvalidArgumentException('Block size must be greater than 0');
55-
}
56-
5753
try {
5854
$objectHead = $s3Client->headObject([
5955
'Bucket' => $bucket,
@@ -79,7 +75,9 @@ public static function openAppend(
7975
* and append to it. We need to read the file to memory and append it to the new file.
8076
* S3 allows only the last part to be smaller than 5Mb, all other parts needs to be 5Mb+.
8177
*/
82-
if ($objectHead->getContentLength() < SizeUnits::mbToBytes(5)) {
78+
$contentLength = $objectHead->getContentLength() ?? 0;
79+
80+
if ($contentLength < SizeUnits::mbToBytes(5)) {
8381
$blocks = new Blocks(
8482
$blockSize,
8583
$blockFactory,
@@ -140,10 +138,6 @@ public static function openBlank(
140138
BlockFactory $blockFactory = new NativeLocalFileBlocksFactory(),
141139
int $blockSize = 1024 * 1024 * 4,
142140
): self {
143-
if ($blockSize < 1) {
144-
throw new InvalidArgumentException('Block size must be greater than 0');
145-
}
146-
147141
$response = $s3Client->createMultipartUpload(new CreateMultipartUploadRequest([
148142
'Bucket' => $bucket,
149143
'Key' => ltrim($path->path(), '/'),

‎src/bridge/filesystem/async-aws/src/Flow/Filesystem/Bridge/AsyncAWS/AsyncAWSS3Filesystem.php‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,9 @@ public function getSystemTmpDir(): Path
5959
return $this->options->tmpDir();
6060
}
6161

62+
/**
63+
* @return \Generator<FileStatus>
64+
*/
6265
public function list(Path $path, Filter $pathFilter = new KeepAll()): Generator
6366
{
6467
$this->mount->supports($path) || throw new InvalidSchemeException($path->protocol(), $this->mount->protocol);

‎src/bridge/filesystem/async-aws/src/Flow/Filesystem/Bridge/AsyncAWS/AsyncAWSS3SourceStream.php‎

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,9 @@ public function isOpen(): bool
4949

5050
public function iterate(int $length = 1): Generator
5151
{
52-
for ($offset = 0; $offset < $this->size(); $offset += $length) {
52+
$size = $this->size() ?? 0;
53+
54+
for ($offset = 0; $offset < $size; $offset += $length) {
5355
yield $this->read($length, $offset);
5456
}
5557
}
@@ -74,8 +76,9 @@ public function readLines(string $separator = "\n", ?int $length = null): Genera
7476
{
7577
$offset = 0;
7678
$content = '';
79+
$size = $this->size() ?? 0;
7780

78-
while ($offset < $this->size()) {
81+
while ($offset < $size) {
7982
// Read a chunk of the file
8083
$chunk = $this->read($length ?? (1024 * 1024 * 9), $offset);
8184
$offset += strlen($chunk);
@@ -98,12 +101,12 @@ public function readLines(string $separator = "\n", ?int $length = null): Genera
98101

99102
$content = $lines[$lastIndex];
100103
} elseif (substr_count($content, $separator) === 1) {
101-
// Split the content by the separator
102-
/**
103-
* @phpstan-ignore-next-line
104-
*/
105-
yield substr($content, 0, strpos($content, $separator));
106-
$content = substr($content, strpos($content, $separator) + 1);
104+
$pos = strpos($content, $separator);
105+
106+
if ($pos !== false) {
107+
yield substr($content, 0, $pos);
108+
$content = substr($content, $pos + 1);
109+
}
107110
}
108111
}
109112

‎src/bridge/filesystem/async-aws/src/Flow/Filesystem/Bridge/AsyncAWS/DSL/functions.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
namespace Flow\Filesystem\Bridge\AsyncAWS\DSL;
66

7+
use AsyncAws\Core\Configuration;
78
use AsyncAws\S3\S3Client;
89
use Flow\ETL\Attribute\DocumentationDSL;
910
use Flow\ETL\Attribute\Module;
@@ -13,12 +14,11 @@
1314
use Flow\Filesystem\Mount;
1415

1516
/**
16-
* @param array<string, mixed> $configuration - for details please see https://async-aws.com/clients/s3.html
17+
* @param array<Configuration::OPTION_*, null|string> $configuration - for details please see https://async-aws.com/clients/s3.html
1718
*/
1819
#[DocumentationDSL(module: Module::S3_FILESYSTEM, type: Type::HELPER)]
1920
function aws_s3_client(array $configuration): S3Client
2021
{
21-
/** @phpstan-ignore-next-line */
2222
return new S3Client($configuration);
2323
}
2424

‎src/bridge/filesystem/async-aws/tests/Flow/Filesystem/Bridge/AsyncAWS/Tests/Integration/AsyncAWSS3DestinationStreamTest.php‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,10 @@ public function test_writing_content_from_resource(): void
3030
$stream->fromResource($resource);
3131
$stream->close();
3232

33-
static::assertTrue($fs->status(path('aws-s3://orders.csv'))?->isFile());
34-
static::assertFalse($fs->status(path('aws-s3://orders.csv'))->isDirectory());
33+
$status = $fs->status(path('aws-s3://orders.csv'));
34+
static::assertNotNull($status);
35+
static::assertTrue($status->isFile());
36+
static::assertFalse($status->isDirectory());
3537
static::assertSame(
3638
file_get_contents(__DIR__ . '/Fixtures/orders.csv'),
3739
$fs->readFrom(path('aws-s3://orders.csv'))->content(),
@@ -48,8 +50,10 @@ public function test_writing_content_smaller_than_block_size_to_s3(): void
4850
$stream->append('Hello, World!');
4951
$stream->close();
5052

51-
static::assertTrue($fs->status(path('aws-s3://file.txt'))?->isFile());
52-
static::assertFalse($fs->status(path('aws-s3://file.txt'))->isDirectory());
53+
$status = $fs->status(path('aws-s3://file.txt'));
54+
static::assertNotNull($status);
55+
static::assertTrue($status->isFile());
56+
static::assertFalse($status->isDirectory());
5357
static::assertSame('Hello, World!', $fs->readFrom(path('aws-s3://file.txt'))->content());
5458

5559
$fs->rm(path('aws-s3://file.txt'));

‎src/bridge/filesystem/async-aws/tests/Flow/Filesystem/Bridge/AsyncAWS/Tests/Integration/AsyncAWSS3FilesystemFileFastPathTest.php‎

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,13 @@
66

77
use Flow\Filesystem\Bridge\AsyncAWS\Options;
88
use Flow\Filesystem\Bridge\AsyncAWS\Tests\Double\RecordingS3Client;
9+
use Flow\Filesystem\FileStatus;
910
use Flow\Filesystem\Path\Filter\OnlyFiles;
1011
use Flow\Filesystem\Tests\Double\RejectingFilter;
1112

1213
use function Flow\Filesystem\Bridge\AsyncAWS\DSL\aws_s3_filesystem;
1314
use function Flow\Filesystem\DSL\path;
15+
use function Flow\Types\DSL\type_string;
1416
use function iterator_to_array;
1517

1618
final class AsyncAWSS3FilesystemFileFastPathTest extends AsyncAWSS3TestCase
@@ -123,6 +125,8 @@ public function test_list_single_file_does_not_yield_prefix_siblings(): void
123125
$statuses = iterator_to_array($fs->list(path('aws-s3://var/orders/file.txt')));
124126

125127
static::assertCount(1, $statuses);
128+
static::assertArrayHasKey(0, $statuses);
129+
static::assertInstanceOf(FileStatus::class, $statuses[0]);
126130
static::assertSame('aws-s3://var/orders/file.txt', $statuses[0]->path->uri());
127131
static::assertSame(1, $client->headObjectCount);
128132
static::assertSame(0, $client->listObjectsV2Count);
@@ -140,6 +144,8 @@ public function test_list_single_file_yields_via_head_only_when_fast_path_enable
140144
$statuses = iterator_to_array($fs->list(path('aws-s3://var/orders/orders.csv'), new OnlyFiles()));
141145

142146
static::assertCount(1, $statuses);
147+
static::assertArrayHasKey(0, $statuses);
148+
static::assertInstanceOf(FileStatus::class, $statuses[0]);
143149
static::assertSame('aws-s3://var/orders/orders.csv', $statuses[0]->path->uri());
144150
static::assertTrue($statuses[0]->isFile());
145151
static::assertSame(1, $client->headObjectCount, 'exactly one HEAD must be issued');
@@ -153,14 +159,13 @@ public function test_list_single_file_yields_via_head_only_when_fast_path_enable
153159
private function recordingClient(): RecordingS3Client
154160
{
155161
$configuration = [
156-
'pathStyleEndpoint' => true,
157-
'endpoint' => $_ENV['S3_ENDPOINT'],
158-
'region' => $_ENV['S3_REGION'],
159-
'accessKeyId' => $_ENV['S3_ACCESS_KEY_ID'],
160-
'accessKeySecret' => $_ENV['S3_SECRET_ACCESS_KEY'],
162+
'pathStyleEndpoint' => 'true',
163+
'endpoint' => type_string()->assert($_ENV['S3_ENDPOINT']),
164+
'region' => type_string()->assert($_ENV['S3_REGION']),
165+
'accessKeyId' => type_string()->assert($_ENV['S3_ACCESS_KEY_ID']),
166+
'accessKeySecret' => type_string()->assert($_ENV['S3_SECRET_ACCESS_KEY']),
161167
];
162168

163-
/** @phpstan-ignore-next-line */
164169
return new RecordingS3Client($configuration);
165170
}
166171
}

‎src/bridge/filesystem/async-aws/tests/Flow/Filesystem/Bridge/AsyncAWS/Tests/Integration/AsyncAWSS3FilesystemTest.php‎

Lines changed: 27 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -27,13 +27,12 @@ public function test_appending_to_existing_5mb_blob(): void
2727

2828
$fs->appendTo(path('aws-s3://var/file.txt'))->append("This is second line\n")->close();
2929

30-
static::assertTrue($fs->status(path('aws-s3://var/file.txt'))?->isFile());
31-
static::assertFalse($fs->status(path('aws-s3://var/file.txt'))->isDirectory());
30+
$status = $fs->status(path('aws-s3://var/file.txt'));
31+
static::assertNotNull($status);
32+
static::assertTrue($status->isFile());
33+
static::assertFalse($status->isDirectory());
3234

33-
static::assertStringStartsWith(
34-
str_repeat('a', 1024),
35-
$fs->readFrom(path('aws-s3://var/file.txt'))->read(1024, 0),
36-
);
35+
static::assertSame(str_repeat('a', 1024), $fs->readFrom(path('aws-s3://var/file.txt'))->read(1024, 0));
3736
static::assertStringEndsWith("This is second line\n", $fs->readFrom(path('aws-s3://var/file.txt'))->read(
3837
58,
3938
1024 * 1024 * 5,
@@ -50,8 +49,10 @@ public function test_appending_to_existing_blob(): void
5049

5150
$fs->appendTo(path('aws-s3://var/file.txt'))->append("This is second line\n")->close();
5251

53-
static::assertTrue($fs->status(path('aws-s3://var/file.txt'))?->isFile());
54-
static::assertFalse($fs->status(path('aws-s3://var/file.txt'))->isDirectory());
52+
$status = $fs->status(path('aws-s3://var/file.txt'));
53+
static::assertNotNull($status);
54+
static::assertTrue($status->isFile());
55+
static::assertFalse($status->isDirectory());
5556
static::assertSame(<<<'TXT'
5657
This is first line
5758
This is second line
@@ -127,11 +128,10 @@ public function test_file_status_on_pattern(): void
127128
$stream = $fs->writeTo(path('aws-s3://var/some_path_to/file.txt'))->fromResource($resource);
128129
$stream->close();
129130

130-
static::assertTrue($fs->status(path('aws-s3://var/some_path_to/*.txt'))?->isFile());
131-
static::assertSame(
132-
'aws-s3://var/some_path_to/file.txt',
133-
$fs->status(path('aws-s3://var/some_path_to/*.txt'))->path->uri(),
134-
);
131+
$status = $fs->status(path('aws-s3://var/some_path_to/*.txt'));
132+
static::assertNotNull($status);
133+
static::assertTrue($status->isFile());
134+
static::assertSame('aws-s3://var/some_path_to/file.txt', $status->path->uri());
135135
}
136136

137137
public function test_file_status_on_root_folder(): void
@@ -207,7 +207,8 @@ public function test_remove_pattern(): void
207207
$fs->status(path('aws-s3://var/flow-fs-test-directory/remove_file_when_exists.txt'))?->isFile(),
208208
);
209209
$fs->rm(path('aws-s3://var/flow-fs-test-directory/*.txt'));
210-
static::assertTrue($fs->status(path('aws-s3://var/flow-fs-test-directory/'))->isDirectory());
210+
/** @phpstan-ignore nullsafe.neverNull */
211+
static::assertTrue($fs->status(path('aws-s3://var/flow-fs-test-directory/'))?->isDirectory());
211212
static::assertNull($fs->status(path('aws-s3://var/flow-fs-test-directory/remove_file_when_exists.txt')));
212213
$fs->rm(path('aws-s3://var/flow-fs-test-directory/'));
213214
}
@@ -346,8 +347,10 @@ public function test_writing_to_aws_s3_storage(): void
346347
$stream->append('Hello, World!');
347348
$stream->close();
348349

349-
static::assertTrue($fs->status(path('aws-s3://file.txt'))?->isFile());
350-
static::assertFalse($fs->status(path('aws-s3://file.txt'))->isDirectory());
350+
$status = $fs->status(path('aws-s3://file.txt'));
351+
static::assertNotNull($status);
352+
static::assertTrue($status->isFile());
353+
static::assertFalse($status->isDirectory());
351354
static::assertSame('Hello, World!', $fs->readFrom(path('aws-s3://file.txt'))->content());
352355

353356
$fs->rm(path('aws-s3://file.txt'));
@@ -363,8 +366,10 @@ public function test_writing_to_to_aws_s3_from_resources(): void
363366
$stream->fromResource($resource);
364367
$stream->close();
365368

366-
static::assertTrue($fs->status(path('aws-s3://orders.csv'))?->isFile());
367-
static::assertFalse($fs->status(path('aws-s3://orders.csv'))->isDirectory());
369+
$status = $fs->status(path('aws-s3://orders.csv'));
370+
static::assertNotNull($status);
371+
static::assertTrue($status->isFile());
372+
static::assertFalse($status->isDirectory());
368373
static::assertSame(
369374
file_get_contents(__DIR__ . '/Fixtures/orders.csv'),
370375
$fs->readFrom(path('aws-s3://orders.csv'))->content(),
@@ -387,8 +392,10 @@ public function test_writing_to_to_s3_using_blocks(): void
387392

388393
$stream->close();
389394

390-
static::assertTrue($fs->status(path('aws-s3://block_blob.csv'))?->isFile());
391-
static::assertFalse($fs->status(path('aws-s3://block_blob.csv'))->isDirectory());
395+
$status = $fs->status(path('aws-s3://block_blob.csv'));
396+
static::assertNotNull($status);
397+
static::assertTrue($status->isFile());
398+
static::assertFalse($status->isDirectory());
392399

393400
$fs->rm(path('aws-s3://block_blob.csv'));
394401
}

0 commit comments

Comments
 (0)