-
Notifications
You must be signed in to change notification settings - Fork 7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Feature] Async generation #84
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -25,6 +25,7 @@ | |
service('twig')->nullOnInvalid(), | ||
]) | ||
->call('setLogger', [service('logger')->nullOnInvalid()]) | ||
->call('setWebhookConfigurationRegistry', [service('.sensiolabs_gotenberg.webhook_configuration_registry')->nullOnInvalid()]) | ||
->tag('sensiolabs_gotenberg.pdf_builder') | ||
; | ||
|
||
|
@@ -38,6 +39,7 @@ | |
service('router')->nullOnInvalid(), | ||
]) | ||
->call('setLogger', [service('logger')->nullOnInvalid()]) | ||
->call('setWebhookConfigurationRegistry', [service('.sensiolabs_gotenberg.webhook_configuration_registry')->nullOnInvalid()]) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why through a setter ? Why not the constructor ? |
||
->call('setRequestContext', [service('.sensiolabs_gotenberg.request_context')->nullOnInvalid()]) | ||
->tag('sensiolabs_gotenberg.pdf_builder') | ||
; | ||
|
@@ -50,6 +52,7 @@ | |
service('request_stack'), | ||
service('twig')->nullOnInvalid(), | ||
]) | ||
->call('setWebhookConfigurationRegistry', [service('.sensiolabs_gotenberg.webhook_configuration_registry')->nullOnInvalid()]) | ||
->call('setLogger', [service('logger')->nullOnInvalid()]) | ||
->tag('sensiolabs_gotenberg.pdf_builder') | ||
; | ||
|
@@ -61,6 +64,7 @@ | |
service('sensiolabs_gotenberg.asset.base_dir_formatter'), | ||
]) | ||
->call('setLogger', [service('logger')->nullOnInvalid()]) | ||
->call('setWebhookConfigurationRegistry', [service('.sensiolabs_gotenberg.webhook_configuration_registry')->nullOnInvalid()]) | ||
->tag('sensiolabs_gotenberg.pdf_builder') | ||
; | ||
|
||
|
@@ -71,6 +75,7 @@ | |
service('sensiolabs_gotenberg.asset.base_dir_formatter'), | ||
]) | ||
->call('setLogger', [service('logger')->nullOnInvalid()]) | ||
->call('setWebhookConfigurationRegistry', [service('.sensiolabs_gotenberg.webhook_configuration_registry')->nullOnInvalid()]) | ||
->tag('sensiolabs_gotenberg.pdf_builder') | ||
; | ||
|
||
|
@@ -81,6 +86,7 @@ | |
service('sensiolabs_gotenberg.asset.base_dir_formatter'), | ||
]) | ||
->call('setLogger', [service('logger')->nullOnInvalid()]) | ||
->call('setWebhookConfigurationRegistry', [service('.sensiolabs_gotenberg.webhook_configuration_registry')->nullOnInvalid()]) | ||
->tag('sensiolabs_gotenberg.pdf_builder') | ||
; | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
<?php | ||
|
||
namespace Sensiolabs\GotenbergBundle\Builder; | ||
|
||
interface AsyncBuilderInterface | ||
{ | ||
/** | ||
* Generates a file asynchronously. | ||
*/ | ||
public function generateAsync(): string; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,88 @@ | ||
<?php | ||
|
||
namespace Sensiolabs\GotenbergBundle\Builder; | ||
|
||
use Sensiolabs\GotenbergBundle\DependencyInjection\WebhookConfiguration\WebhookConfigurationRegistry; | ||
use Sensiolabs\GotenbergBundle\Exception\WebhookConfigurationException; | ||
|
||
trait AsyncBuilderTrait | ||
{ | ||
use DefaultBuilderTrait; | ||
private string $webhookUrl; | ||
private string $errorWebhookUrl; | ||
/** | ||
* @var array<string, mixed> | ||
*/ | ||
private array $webhookExtraHeaders = []; | ||
private \Closure $operationIdGenerator; | ||
private WebhookConfigurationRegistry|null $webhookConfigurationRegistry = null; | ||
|
||
public function generateAsync(): string | ||
{ | ||
$operationId = ($this->operationIdGenerator ?? self::defaultOperationIdGenerator(...))(); | ||
$this->logger?->debug('Generating a file asynchronously with operation id {sensiolabs_gotenberg.operation_id} using {sensiolabs_gotenberg.builder} builder.', [ | ||
'sensiolabs_gotenberg.operation_id' => $operationId, | ||
'sensiolabs_gotenberg.builder' => $this::class, | ||
]); | ||
|
||
$this->webhookExtraHeaders['X-Gotenberg-Operation-Id'] = $operationId; | ||
$headers = [ | ||
'Gotenberg-Webhook-Url' => $this->webhookUrl, | ||
'Gotenberg-Webhook-Error-Url' => $this->errorWebhookUrl, | ||
'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'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. remove the basename call for now. it will be handled correctly in another PR. Plus : the async is not only for PDFs. |
||
} | ||
$this->client->call($this->getEndpoint(), $this->getMultipartFormData(), $headers); | ||
|
||
return $operationId; | ||
} | ||
|
||
public function setWebhookConfigurationRegistry(WebhookConfigurationRegistry $registry): static | ||
{ | ||
$this->webhookConfigurationRegistry = $registry; | ||
|
||
return $this; | ||
} | ||
|
||
public function webhookConfiguration(string $webhook): static | ||
{ | ||
if (null === $this->webhookConfigurationRegistry) { | ||
throw new WebhookConfigurationException('The WebhookConfigurationRegistry is not available.'); | ||
} | ||
$webhookConfiguration = $this->webhookConfigurationRegistry->get($webhook); | ||
|
||
return $this->webhookUrls($webhookConfiguration['success'], $webhookConfiguration['error']); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. maybe it can also contain extra headers ? Would that be relevant ? Or authent or anything else ? |
||
} | ||
|
||
public function webhookUrls(string $successWebhook, string|null $errorWebhook = null): static | ||
{ | ||
$this->webhookUrl = $successWebhook; | ||
$this->errorWebhookUrl = $errorWebhook ?? $successWebhook; | ||
|
||
return $this; | ||
} | ||
|
||
/** | ||
* @param array<string, mixed> $extraHeaders | ||
*/ | ||
public function webhookExtraHeaders(array $extraHeaders): static | ||
{ | ||
$this->webhookExtraHeaders = array_merge($this->webhookExtraHeaders, $extraHeaders); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. so how do we reset this ? |
||
|
||
return $this; | ||
} | ||
|
||
public function operationIdGenerator(\Closure $operationIdGenerator): static | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. phpstan annotation for closure please. |
||
{ | ||
$this->operationIdGenerator = $operationIdGenerator; | ||
|
||
return $this; | ||
} | ||
|
||
protected static function defaultOperationIdGenerator(): string | ||
{ | ||
return 'gotenberg_'.bin2hex(random_bytes(16)).microtime(true); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 ? |
||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why ? |
||
default => throw new InvalidBuilderConfiguration(sprintf('Invalid option "%s": no method does not exist in class "%s" to configured it.', $configurationName, static::class)), | ||
}; | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,14 +2,15 @@ | |
|
||
namespace Sensiolabs\GotenbergBundle\Debug\Builder; | ||
|
||
use Sensiolabs\GotenbergBundle\Builder\AsyncBuilderInterface; | ||
use Sensiolabs\GotenbergBundle\Builder\GotenbergFileResult; | ||
use Sensiolabs\GotenbergBundle\Builder\Screenshot\ScreenshotBuilderInterface; | ||
use Symfony\Component\Stopwatch\Stopwatch; | ||
|
||
final class TraceableScreenshotBuilder implements ScreenshotBuilderInterface | ||
{ | ||
/** | ||
* @var list<array{'time': float, 'memory': int, 'size': int<0, max>|null, 'fileName': string, 'calls': list<array{'method': string, 'class': class-string<ScreenshotBuilderInterface>, 'arguments': array<mixed>}>}> | ||
* @var list<array{'time': float|null, 'memory': int|null, 'size': int<0, max>|null, 'fileName': string|null, 'calls': list<array{'method': string, 'class': class-string<ScreenshotBuilderInterface>, 'arguments': array<mixed>}>}> | ||
*/ | ||
private array $screenshots = []; | ||
|
||
|
@@ -24,7 +25,7 @@ | |
|
||
public function __construct( | ||
private readonly ScreenshotBuilderInterface $inner, | ||
private readonly Stopwatch $stopwatch, | ||
private readonly Stopwatch|null $stopwatch, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why this change ? |
||
) { | ||
} | ||
|
||
|
@@ -33,9 +34,9 @@ | |
$name = self::$count.'.'.$this->inner::class.'::'.__FUNCTION__; | ||
++self::$count; | ||
|
||
$swEvent = $this->stopwatch->start($name, 'gotenberg.generate_screenshot'); | ||
$swEvent = $this->stopwatch?->start($name, 'gotenberg.generate_screenshot'); | ||
$response = $this->inner->generate(); | ||
$swEvent->stop(); | ||
$swEvent?->stop(); | ||
|
||
$fileName = 'Unknown'; | ||
if ($response->getHeaders()->has('Content-Disposition')) { | ||
|
@@ -43,7 +44,7 @@ | |
|
||
/* @see https://onlinephp.io/c/c2606 */ | ||
preg_match('#[^;]*;\sfilename="?(?P<fileName>[^"]*)"?#', $response->getHeaders()->get('Content-Disposition', ''), $matches); | ||
$fileName = $matches['fileName']; | ||
} | ||
|
||
$lengthInBytes = null; | ||
|
@@ -53,8 +54,8 @@ | |
|
||
$this->screenshots[] = [ | ||
'calls' => $this->calls, | ||
'time' => $swEvent->getDuration(), | ||
'memory' => $swEvent->getMemory(), | ||
'time' => $swEvent?->getDuration(), | ||
'memory' => $swEvent?->getMemory(), | ||
'status' => $response->getStatusCode(), | ||
'size' => $lengthInBytes, | ||
'fileName' => $fileName, | ||
|
@@ -65,6 +66,32 @@ | |
return $response; | ||
} | ||
|
||
public function generateAsync(): string | ||
{ | ||
if (!$this->inner instanceof AsyncBuilderInterface) { | ||
throw new \LogicException(sprintf('The inner builder of %s must implement %s.', self::class, AsyncBuilderInterface::class)); | ||
} | ||
|
||
$name = self::$count.'.'.$this->inner::class.'::'.__FUNCTION__; | ||
++self::$count; | ||
|
||
$swEvent = $this->stopwatch?->start($name, 'gotenberg.generate_screenshot'); | ||
$operationId = $this->inner->generateAsync(); | ||
$swEvent?->stop(); | ||
|
||
$this->screenshots[] = [ | ||
'calls' => $this->calls, | ||
'time' => $swEvent?->getDuration(), | ||
'memory' => $swEvent?->getMemory(), | ||
'size' => null, | ||
'fileName' => null, | ||
]; | ||
|
||
++$this->totalGenerated; | ||
|
||
return $operationId; | ||
} | ||
|
||
/** | ||
* @param array<mixed> $arguments | ||
*/ | ||
|
@@ -86,7 +113,7 @@ | |
} | ||
|
||
/** | ||
* @return list<array{'time': float, 'memory': int, 'size': int<0, max>|null, 'fileName': string, 'calls': list<array{'class': class-string<ScreenshotBuilderInterface>, 'method': string, 'arguments': array<mixed>}>}> | ||
* @return list<array{'time': float|null, 'memory': int|null, 'size': int<0, max>|null, 'fileName': string|null, 'calls': list<array{'class': class-string<ScreenshotBuilderInterface>, 'method': string, 'arguments': array<mixed>}>}> | ||
*/ | ||
public function getFiles(): array | ||
{ | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is
->nullOnInvalid()
needed since we always define this service inservices.php
?