[Feature] Async generation #84

maelanleborgne · 2024-06-25T07:39:20Z

Closes #46

Gotenberg allows async generation of files by passing webhooks urls in dedicated headers. Doing this, the request to Gotenberg server returns a 204 immediately, and once the file is generated, a call will be made by Gotenberg server on the specified endpoint to upload the generated file. This PR aims at enabling easy integration of this feature with the bundle.

The base of the feature is a new interface AsyncBuilderInterface that defines a generateAsync method. The AbstractPdfBuilder and AbstractScreenshotBuilder now implement this interface.
Another important addition is the webhookUrls(string $successWebhook, string|null $errorWebhook) that allows directly setting the webhooks url to use with the builders.

To make things easier when not working directly with urls (if you'd like to generate the url from a route, or if using the Webhook component), a some new configuration options are available :

# packages/sensiolabs_gotenberg.yaml

sensiolabs_gotenberg:  
    # ...
    webhook:  
        # Here you can define any number of webhook configuration so that you can reuse them in builder configs or to switch configuration at runtime
        my_config_name:  
            success:  
                url: 'http://example.com/webhook'
                # route: ['my_route_name', ['param1', 'param2']]
            # This "error" entry is optional. If not set, the "success" configuration will be used for both success and error.
            error:  
                url: 'http://example.com/webhook-error'
                # route: ['my_route_name', ['param1', 'param2']]
    default_options:
        # Here you can define which webhook config to use by default
        webhook: 'my_config_name'
        pdf:
            html:
                webhook: 'my_config_name'
                # Or you can define a specific configuration directly from here
                # webhook:
                #    success:
                #	    url: 'http://example.com/custom-webhook'
    #...

TODO:

Write documentation
Add example config to flex recipe
Add extra-headers option in the webhook config ?

Neirda24 · 2024-06-27T14:05:46Z

After discussing this with @ConstantBqt we have another usecase to take into account : If we generate the PDF but the webhook is not setup in our own app but is external. We should be able to declare such case. WDYT @maelanleborgne ?

maelanleborgne · 2024-06-27T14:16:50Z

After discussing this with @ConstantBqt we have another usecase to take into account : If we generate the PDF but the webhook is not setup in our own app but is external. We should be able to declare such case. WDYT @maelanleborgne ?

This is already present in the PR :

// ...
$this->gotenberg->html()
  ->webhookUrls('https://external.service/webhook', 'https://external.service/webhook-error')
  ->webhookExtraHeaders(['X-ExternalService-Secret' => 'a_secret'])
  ->generateAsync()
;

or via the configuration :

sensiolabs_gotenberg:  
    # ...
    webhook:  
        external_service:  
            success:  
                url: 'https://external.service/webhook'
            error:  
                url: 'https://external.service/webhook-error'
    default_options:
        pdf:
            html:
                webhook: 'external_service'

Or maybe I'm missing the point.

Jean-Beru · 2024-06-27T14:20:47Z

This is already present in the PR :

Could this part be done in a dedicated PR ?

allow defining a webhook to add as a header
make "internal" webhook easier to developer using the component

composer.json

config/builder_pdf.php

src/Builder/Pdf/AbstractPdfBuilder.php

src/DependencyInjection/Configuration.php

Jean-Beru · 2024-06-27T20:47:40Z

src/Builder/Pdf/AbstractPdfBuilder.php


    public function __construct(
        protected readonly GotenbergClientInterface $gotenbergClient,
        protected readonly AssetBaseDirFormatter $asset,
+        protected readonly WebhookConfigurationRegistry|null $webhookConfigurationRegistry = null,


What is the purpose of the WebhookConfigurationRegistry ? Could it be handled like other parameters ?

It holds the different webhook configs available, so changes can be made at runtime using ->webhookConfiguration('custom_config'). I'm not sure we can make it work with setConfiguration because the config of a builder may be one of :

the name of a webhook config defined above in the config file (so it will still have to access the config registry)

a new definition : success and error with a route/url each (and then we'd have to make the router available in order to generate urls for the routes)

Jean-Beru · 2024-09-03T14:49:16Z

src/DependencyInjection/SensiolabsGotenbergExtension.php

+        $container->registerForAutoconfiguration(WebhookConfigurationRegistryInterface::class)
+            ->addTag('sensiolabs_gotenberg.webhook_configuration_registry')
+        ;


What is the purpose of this tag ?

Jean-Beru · 2024-09-03T14:50:36Z

config/builder_pdf.php

@@ -25,6 +25,7 @@
            service('twig')->nullOnInvalid(),
        ])
        ->call('setLogger', [service('logger')->nullOnInvalid()])
+        ->call('setWebhookConfigurationRegistry', [service('.sensiolabs_gotenberg.webhook_configuration_registry')->nullOnInvalid()])


is ->nullOnInvalid() needed since we always define this service in services.php ?

Neirda24 · 2024-09-07T13:42:53Z

config/builder_pdf.php

@@ -38,6 +39,7 @@
            service('router')->nullOnInvalid(),
        ])
        ->call('setLogger', [service('logger')->nullOnInvalid()])
+        ->call('setWebhookConfigurationRegistry', [service('.sensiolabs_gotenberg.webhook_configuration_registry')->nullOnInvalid()])


Why through a setter ? Why not the constructor ?

Neirda24 · 2024-09-07T13:46:03Z

src/Builder/AsyncBuilderTrait.php

+            'Gotenberg-Webhook-Extra-Http-Headers' => json_encode($this->webhookExtraHeaders, \JSON_THROW_ON_ERROR),
+        ];
+        if (null !== $this->fileName) {
+            $headers['Gotenberg-Output-Filename'] = basename($this->fileName, '.pdf');


remove the basename call for now. it will be handled correctly in another PR. Plus : the async is not only for PDFs.

Neirda24 · 2024-09-07T13:47:27Z

src/Builder/AsyncBuilderTrait.php

+        return $this;
+    }
+
+    public function operationIdGenerator(\Closure $operationIdGenerator): static


phpstan annotation for closure please.

Neirda24 · 2024-09-07T13:47:42Z

src/Builder/AsyncBuilderTrait.php

+     */
+    public function webhookExtraHeaders(array $extraHeaders): static
+    {
+        $this->webhookExtraHeaders = array_merge($this->webhookExtraHeaders, $extraHeaders);


so how do we reset this ?

Neirda24 · 2024-09-07T13:50:35Z

src/Builder/AsyncBuilderTrait.php

+
+    protected static function defaultOperationIdGenerator(): string
+    {
+        return 'gotenberg_'.bin2hex(random_bytes(16)).microtime(true);


Maybe we should keep the id stateful per builder ? Reset it only when generating the PDF. Also the ID should be part of any operation I guess. Not only the async one. WDYT ?

Neirda24 · 2024-09-07T13:50:58Z

src/Builder/Pdf/AbstractChromiumPdfBuilder.php

@@ -586,6 +586,7 @@ protected function addConfiguration(string $configurationName, mixed $value): vo
            'fail_on_console_exceptions' => $this->failOnConsoleExceptions($value),
            'skip_network_idle_event' => $this->skipNetworkIdleEvent($value),
            'metadata' => $this->metadata($value),
+            'webhook' => null,


Neirda24 · 2024-09-07T13:52:38Z

src/Builder/AsyncBuilderTrait.php

+        }
+        $webhookConfiguration = $this->webhookConfigurationRegistry->get($webhook);
+
+        return $this->webhookUrls($webhookConfiguration['success'], $webhookConfiguration['error']);


maybe it can also contain extra headers ? Would that be relevant ? Or authent or anything else ?

Neirda24 · 2024-09-07T13:53:55Z

src/Debug/Builder/TraceableScreenshotBuilder.php

@@ -24,7 +25,7 @@ final class TraceableScreenshotBuilder implements ScreenshotBuilderInterface

    public function __construct(
        private readonly ScreenshotBuilderInterface $inner,
-        private readonly Stopwatch $stopwatch,
+        private readonly Stopwatch|null $stopwatch,


why this change ?

Neirda24 · 2024-09-07T13:55:00Z

src/DependencyInjection/Configuration.php

+                    ->children()
+                        ->scalarNode('name')
+                            ->validate()
+                                ->ifTrue(static function ($option) {


return type + arg type

Neirda24 · 2024-09-07T13:55:22Z

src/DependencyInjection/Configuration.php

+                        ->append($this->addWebhookConfigurationNode('error'))
+                    ->end()
+                    ->validate()
+                        ->ifTrue(static function ($option): bool {


maelanleborgne changed the title ~~Feature/webhook integration~~ [Feature] Async generation Jun 25, 2024

maelanleborgne marked this pull request as draft June 25, 2024 07:40

Jean-Beru reviewed Jun 27, 2024

View reviewed changes

maelanleborgne force-pushed the feature/webhook-integration branch from f7e3db6 to a2ad358 Compare July 12, 2024 12:01

maelanleborgne force-pushed the feature/webhook-integration branch from a2ad358 to 13eb438 Compare July 22, 2024 10:17

maelanleborgne force-pushed the feature/webhook-integration branch from 13eb438 to 075dedd Compare September 3, 2024 12:12

Add ability to generate asynchronously using webhooks

9915a29

maelanleborgne force-pushed the feature/webhook-integration branch from 075dedd to 9915a29 Compare September 3, 2024 12:50

Jean-Beru reviewed Sep 3, 2024

View reviewed changes

Neirda24 reviewed Sep 7, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Feature] Async generation #84

[Feature] Async generation #84

maelanleborgne commented Jun 25, 2024 •

edited

Loading

Neirda24 commented Jun 27, 2024

maelanleborgne commented Jun 27, 2024

Jean-Beru commented Jun 27, 2024

Jean-Beru Jun 27, 2024

maelanleborgne Jun 28, 2024

Jean-Beru Sep 3, 2024

Jean-Beru Sep 3, 2024

Neirda24 Sep 7, 2024

Neirda24 Sep 7, 2024

Neirda24 Sep 7, 2024

Neirda24 Sep 7, 2024

Neirda24 Sep 7, 2024

Neirda24 Sep 7, 2024

Neirda24 Sep 7, 2024

Neirda24 Sep 7, 2024

Neirda24 Sep 7, 2024

Neirda24 Sep 7, 2024

[Feature] Async generation #84

Are you sure you want to change the base?

[Feature] Async generation #84

Conversation

maelanleborgne commented Jun 25, 2024 • edited Loading

Neirda24 commented Jun 27, 2024

maelanleborgne commented Jun 27, 2024

Jean-Beru commented Jun 27, 2024

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

maelanleborgne commented Jun 25, 2024 •

edited

Loading