Doc/processor #141

Neirda24 · 2025-01-11T16:18:27Z

Closes #140

docs/processing.md

Jean-Beru · 2025-01-11T19:29:17Z

docs/processing.md

+
+## Using FileProcessor
+
+Useful if you want to store the file.


"the file in the filesystem"

docs/processing.md

Jean-Beru · 2025-01-11T20:19:14Z

docs/processing.md

+    $chunk = yield;
+    // do something with it
+} while (!$chunk->isLast());
+```


We should also explain that the returned value of the processor will be returned by the process method

DanielBadura · 2025-01-12T12:10:53Z

What would be the solution when I want to use it via cli & use e.g. flysystem to save the pdf to another source like a s3 bucket? If I understood the docs correctly, this is right now not possible unless I implement a own Processor, like e.g. StringProcessor which simply returns the file as a string (I know, this could be consuming alot of memory, but this could be used universally). Other solution would be to implement a Processor like the FileProcessor but coupled to Flysystem.

Neirda24 · 2025-01-12T14:09:57Z

What would be the solution when I want to use it via cli & use e.g. flysystem to save the pdf to another source like a s3 bucket? If I understood the docs correctly, this is right now not possible unless I implement a own Processor, like e.g. StringProcessor which simply returns the file as a string (I know, this could be consuming alot of memory, but this could be used universally). Other solution would be to implement a Processor like the FileProcessor but coupled to Flysystem.

A StringProcessor would eventually cancel all the memory efficiency that this bundle tries to provide. But yes a custom Processor to leverage https://github.com/async-aws/aws/blob/master/src/Integration/Aws/SimpleS3/src/SimpleS3Client.php#L69 for example to upload chunk by chunk. We are planning for a flysystem native integration with this bundle.

Jean-Beru · 2025-01-12T14:31:41Z

What would be the solution when I want to use it via cli & use e.g. flysystem to save the pdf to another source like a s3 bucket? If I understood the docs correctly, this is right now not possible unless I implement a own Processor, like e.g. StringProcessor which simply returns the file as a string (I know, this could be consuming alot of memory, but this could be used universally). Other solution would be to implement a Processor like the FileProcessor but coupled to Flysystem.

You're right, you will have to create your own Processor to stream to your bucket. You can use AsyncAws to make it easier.

There is no such class in the GotenbergBundle yet but it's planned as a Bridge.

Neirda24 · 2025-01-12T19:52:03Z

What would be the solution when I want to use it via cli & use e.g. flysystem to save the pdf to another source like a s3 bucket? If I understood the docs correctly, this is right now not possible unless I implement a own Processor, like e.g. StringProcessor which simply returns the file as a string (I know, this could be consuming alot of memory, but this could be used universally). Other solution would be to implement a Processor like the FileProcessor but coupled to Flysystem.

Here is a naive approach for async aws : https://gist.github.com/Neirda24/05e6a65a862ab34854be3b5f9e562420 As @Jean-Beru said we will try to integrate natively with Flysystem and so on.

Neirda24 · 2025-01-12T22:59:01Z

and here is a working example for flysystem. https://gist.github.com/Neirda24/f950f1aaa5343c67bcbaeb12ed0cb887 I'll have to investigate more to see if its possible without a tmpfile but I don't see how given the abstraction.

Neirda24 · 2025-01-13T00:30:33Z

composer.json

@@ -33,12 +33,16 @@
        "symfony/mime": "^6.4 || ^7.0"
    },
    "require-dev": {
+        "ext-mbstring": "*",
+        "async-aws/s3": "^2.6",


might be worth to lower the requirement

Neirda24 · 2025-01-13T00:30:38Z

composer.json

+        "league/flysystem": "^3.29",
+        "league/flysystem-bundle": "^3.3",


might be worth to lower the requirement

Neirda24 · 2025-01-13T00:30:57Z

composer.json

        "phpstan/extension-installer": "^1.3",
        "phpstan/phpstan": "^1.10",
        "phpstan/phpstan-symfony": "^1.3",
        "phpunit/phpunit": "^10.4",
-        "shipmonk/composer-dependency-analyser": "^1.7",
+        "shipmonk/composer-dependency-analyser": "^1.8",


needed to ignore ext-mbstring issues

smnandre · 2025-01-13T04:00:43Z

and here is a working example for flysystem. https://gist.github.com/Neirda24/f950f1aaa5343c67bcbaeb12ed0cb887 I'll have to investigate more to see if its possible without a tmpfile but I don't see how given the abstraction.

What about using kbond filesystem that does all the abstractions for you?

Neirda24 · 2025-01-13T07:29:09Z

and here is a working example for flysystem. https://gist.github.com/Neirda24/f950f1aaa5343c67bcbaeb12ed0cb887 I'll have to investigate more to see if its possible without a tmpfile but I don't see how given the abstraction.

What about using kbond filesystem that does all the abstractions for you?

Not sure hw that would help in this case. Flysystem does not expose a way to write by chunk. the version from kbonc suffers the same issues.

smnandre · 2025-01-13T09:42:09Z

src/Processor/AsyncAwsProcessor.php

+    public function __invoke(string|null $fileName): \Generator
+    {
+        if (null === $fileName) {
+            $fileName = uniqid('gotenberg_', true);


Maybe use Filesystem here ?

This will be droped soon I think. I don't see a reason filename should be null. I went along with it in this PR but it will change. Otherwise yes filesystem could be a lead.

yeah because of the getFileName() who's returned $this->response->getFileName(). But I think we can't receive a null or it willcrash with an exception before this part.

smnandre · 2025-01-13T09:42:29Z

src/Processor/AsyncAwsProcessor.php

+            $this->logger?->debug('{processor}: no filename given. Content will be dumped to "{file}".', ['processor' => self::class, 'file' => $fileName]);
+        }
+
+        $this->logger?->debug('{processor}: starting multi part upload of "{file}".', ['processor' => self::class, 'file' => $fileName]);


private readonly LoggerInterface $logger = new NullLogger()

?

Why ? This works well already. What kind of gain would we have ?

smnandre · 2025-01-13T09:44:27Z

src/Processor/FlysystemProcessor.php

+            $this->filesystemOperator->writeStream($fileName, $tmpfile);
+
+            $this->logger?->debug('{processor}: content dumped to "{file}".', ['processor' => self::class, 'file' => $fileName]);
+
+            return function () use ($fileName) {
+                return $this->filesystemOperator->read($fileName); // use readStream instead ?
+            };


Does not feel optimised

it does nothing unless you can that callable. I was thinking of returning an anonymous class that implements Stringable and returns this call instead to make it easier to use. What do you suggest ?

Jean-Beru · 2025-01-13T13:15:14Z

src/Processor/AsyncAwsProcessor.php

+ *
+ * @implements ProcessorInterface<CompleteMultipartUploadOutput>
+ */
+final class AsyncAwsProcessor implements ProcessorInterface


IMHO, specific processors like AWS or Flysystem must be in a separated bridge to avoid adding dev-dependencies. It could block the update of GotenbergBundle to Symfony x.y if, for example, FlysystemBundle does not support it yet.

but then they would have to require it just for one class potentially ? Seems a bit too much don't you think ?

IMO such classes must be fixed on package version instead of framework version. So maybe rename it to include more clearly package name and version of said package ?

OK for a Bridge folder (like the Notifier for example) for now.

To avoid developer to install both Flysystem and AsyncAWS on their local project, we can install these dev dependencies only in the CI (if tests need them).

they are dev dependencies already so they are not installed on developper projects. Could be move to suggest only if needed and manually required in test env for CI

Why not. We should also not rely on the bundle but just the FlySystem lib

StevenRenaux · 2025-01-13T13:19:40Z

docs/processing.md

+```php
+do {
+    $chunk = yield;
+    // do something with it
+} while (!$chunk->isLast());
+```


Suggested change

```php

do {

$chunk = yield;

// do something with it

} while (!$chunk->isLast());

```

```php

public function __invoke(string|null $fileName): \Generator

{

do {

$chunk = yield;

// do something with it

} while (!$chunk->isLast());

// rest of your code

}

StevenRenaux · 2025-01-13T13:28:54Z

docs/processing.md

+
+## Other processors
+
+* `Sensiolabs\GotenbergBundle\Processor\AsyncAwsProcessor` : Upload using the `async-aws/s3` package. Uploads using the (multipart upload)[https://docs.aws.amazon.com/AmazonS3/latest/userguide/mpuoverview.html] feature of S3. Returns a `AsyncAws\S3\Result\CompleteMultipartUploadOutput` object.


Maybe add an example of it

StevenRenaux · 2025-01-13T13:29:06Z

src/Processor/AsyncAwsProcessor.php

+    public function __invoke(string|null $fileName): \Generator
+    {
+        if (null === $fileName) {
+            $fileName = uniqid('gotenberg_', true);


yeah because of the getFileName() who's returned $this->response->getFileName(). But I think we can't receive a null or it willcrash with an exception before this part.

Neirda24 added 3 commits January 11, 2025 17:08

Add processor documentation

29ed939

Add custom processor documentation

610aa18

Fix doc

9ff44e1

Neirda24 requested review from StevenRenaux and Jean-Beru January 11, 2025 18:13

Neirda24 added the documentation Improvements or additions to documentation label Jan 11, 2025

Jean-Beru reviewed Jan 11, 2025

View reviewed changes

Neirda24 added 5 commits January 13, 2025 00:49

Modify doc + Add new processors

ac079d2

Fix code style

e4e7845

Fix composer dependency scanning

63c2ce5

Fix composer dependency analyser for ext-*

5e74cfe

Fix composer dependency analyser for ext-*

03d6372

Neirda24 requested a review from Jean-Beru January 13, 2025 00:29

Neirda24 commented Jan 13, 2025

View reviewed changes

smnandre reviewed Jan 13, 2025

View reviewed changes

Jean-Beru reviewed Jan 13, 2025

View reviewed changes

StevenRenaux reviewed Jan 13, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Doc/processor #141

Doc/processor #141

Neirda24 commented Jan 11, 2025

Jean-Beru Jan 11, 2025

Jean-Beru Jan 11, 2025

DanielBadura commented Jan 12, 2025

Neirda24 commented Jan 12, 2025

Jean-Beru commented Jan 12, 2025

Neirda24 commented Jan 12, 2025

Neirda24 commented Jan 12, 2025

Neirda24 Jan 13, 2025

Neirda24 Jan 13, 2025

Neirda24 Jan 13, 2025

smnandre commented Jan 13, 2025

Neirda24 commented Jan 13, 2025

smnandre Jan 13, 2025

Neirda24 Jan 13, 2025

StevenRenaux Jan 13, 2025

smnandre Jan 13, 2025

Neirda24 Jan 13, 2025

smnandre Jan 13, 2025

Neirda24 Jan 13, 2025

Jean-Beru Jan 13, 2025

Neirda24 Jan 13, 2025

Neirda24 Jan 13, 2025

Jean-Beru Jan 13, 2025

Neirda24 Jan 13, 2025

Jean-Beru Jan 13, 2025

StevenRenaux Jan 13, 2025

StevenRenaux Jan 13, 2025

StevenRenaux Jan 13, 2025

		"league/flysystem": "^3.29",
		"league/flysystem-bundle": "^3.3",


		## Other processors

		* `Sensiolabs\GotenbergBundle\Processor\AsyncAwsProcessor` : Upload using the `async-aws/s3` package. Uploads using the (multipart upload)[https://docs.aws.amazon.com/AmazonS3/latest/userguide/mpuoverview.html] feature of S3. Returns a `AsyncAws\S3\Result\CompleteMultipartUploadOutput` object.

Doc/processor #141

Are you sure you want to change the base?

Doc/processor #141

Conversation

Neirda24 commented Jan 11, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DanielBadura commented Jan 12, 2025

Neirda24 commented Jan 12, 2025

Jean-Beru commented Jan 12, 2025

Neirda24 commented Jan 12, 2025

Neirda24 commented Jan 12, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

smnandre commented Jan 13, 2025

Neirda24 commented Jan 13, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment