[Bug]: to_csv() inside write do not set up ContentType for S3

[sdieguez-wotc]

What happened?

Looks like then the S3 stream opens the blank set of chunks to upload is not providing the content-type wo the s3 object.

How to reproduce?

<?php

use function Flow\ETL\Adapter\CSV\to_csv;
use function Flow\ETL\DSL\config_builder;
use function Flow\ETL\DSL\data_frame;
use function Flow\ETL\DSL\from_rows;
use function Flow\Filesystem\Bridge\AsyncAWS\DSL\aws_s3_client;
use function Flow\Filesystem\Bridge\AsyncAWS\DSL\aws_s3_filesystem;
use function Flow\Filesystem\DSL\path;

$config = config_builder()
            ->mount(
                aws_s3_filesystem(
                    $_ENV['AWS_BUCKET'],
                    aws_s3_client([
                        'region' => $_ENV['AWS_DEFAULT_REGION'],
                        'accessKeyId' => $_ENV['AWS_ACCESS_KEY_ID'],
                        'accessKeySecret' => $_ENV['AWS_SECRET_ACCESS_KEY'],
                    ])
                )
            );

// 3. Create a DataFrame with your data
$dataFrame = data_frame([
    ['id' => 1, 'name' => 'Alice'],
    ['id' => 2, 'name' => 'Bob'],
    ['id' => 3, 'name' => 'Charlie'],
], config: $config);

// 4. Format the DataFrame as a CSV and write to the S3 stream
$dataFrame
    ->write(to_csv(path("aws-s3://{$path}"), true, ',')))
    ->run();

echo "CSV file successfully written to S3 but Content-Type: application/xml\n";

Data required to reproduce bug locally

any data

Version

Latest

Relevant error output

No error just the file uploaded is assumed to be `application/xml` even if is a csv file

[sdieguez-wotc]

See

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

Line 48 in 5af4345

$response = $s3Client->createMultipartUpload(new CreateMultipartUploadRequest([

where it should already be setting ContentType

[stloyd]

Fixing this is "not that easy" as the filesystem and path are not aware of what they get:

• the filesystem is aware of what it will receive, as its job is to "store" something that is given;
• and the path is, as the name says, just a path holder with helpers.

Without big refactoring like the introduction of a new concept (i.e., File holding more details), the hack over Path::options() could be used, i.e.:

Subject: [PATCH] Content type as part of path options for S3 stream
---
Index: src/bridge/filesystem/async-aws/src/Flow/Filesystem/Bridge/AsyncAWS/AsyncAWSS3DestinationStream.php
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/bridge/filesystem/async-aws/src/Flow/Filesystem/Bridge/AsyncAWS/AsyncAWSS3DestinationStream.php b/src/bridge/filesystem/async-aws/src/Flow/Filesystem/Bridge/AsyncAWS/AsyncAWSS3DestinationStream.php
--- a/src/bridge/filesystem/async-aws/src/Flow/Filesystem/Bridge/AsyncAWS/AsyncAWSS3DestinationStream.php	(revision 5af43457e152ecdd3ee70dd64de605e1cc471b94)
+++ b/src/bridge/filesystem/async-aws/src/Flow/Filesystem/Bridge/AsyncAWS/AsyncAWSS3DestinationStream.php	(date 1761756444060)
@@ -48,6 +48,7 @@
             $response = $s3Client->createMultipartUpload(new CreateMultipartUploadRequest([
                 'Bucket' => $bucket,
                 'Key' => \ltrim($appendPath->path(), '/'),
+                'Content-Type' => $path->options()->getAsString('content-type', 'application/xml'),
             ]));
 
             /**
@@ -122,6 +123,7 @@
         $response = $s3Client->createMultipartUpload(new CreateMultipartUploadRequest([
             'Bucket' => $bucket,
             'Key' => \ltrim($path->path(), '/'),
+            'Content-Type' => $path->options()->getAsString('content-type', 'application/xml'),
         ]));
 
         /** @var string $uploadId */

But for now it's just a dirty proposal :)

[norberttech]

No error just the file uploaded is assumed to be application/xml even if is a csv file

Hah that's interesting, I thought that S3 is smarter than that assuming that files without content-type are XML's 😅
Sorry for inconvenience @sdieguez-wotc

As for the fix, the Options are actually made for that. When designing filesystem I expected that some destinations/sources will require additional data to be carried with the Path as I didn't want to introduce any other state.

One solution would be to introduce an enum ContentType in filesystem library that can be set on the Path.

Something like:

Path::setOption(ContentType::CSV)

Then we can make all Loaders to set that on the path in the constructor (so even before filesystem even gets the path).

Finally the AWSDestinationStream (and azure potentially also) can check if ContentType option is set and use proper Content-Type header.

I'm going to take a look at this tomorrow

[norberttech]

So in #1942 I ended up doing making it possible to set content type explicitly but when not set, each loader will do it for the user.

I was remembering correctly, AWS when you don't set ContentType will still show in the bucket UI correct types:

The problem is with Metadata, more specifically System-Metadata - when we don't pass ContentType for some weird reason it will set it automatically to xml/application 🤦‍♂️