From ef32a22b1eb47be83a73ac4e5c941fa8f455a18f Mon Sep 17 00:00:00 2001 From: Christian Flothmann Date: Sat, 29 Jun 2024 20:46:00 +0200 Subject: [PATCH] decouple the Webhook component from the Serializer component --- .../FrameworkExtension.php | 15 ++--- .../Resources/config/webhook.php | 9 +++ .../DependencyInjection/ConfigurationTest.php | 6 +- .../FrameworkExtensionTestCase.php | 8 +-- .../Bundle/FrameworkBundle/composer.json | 2 + src/Symfony/Component/Webhook/CHANGELOG.md | 5 ++ .../Webhook/Server/JsonBodyConfigurator.php | 10 +-- .../Server/NativeJsonPayloadSerializer.php | 20 ++++++ .../Server/PayloadSerializerInterface.php | 17 +++++ .../Server/SerializerPayloadSerializer.php | 27 ++++++++ .../Tests/Server/JsonBodyConfiguratorTest.php | 67 +++++++++++++++++++ src/Symfony/Component/Webhook/composer.json | 4 ++ 12 files changed, 168 insertions(+), 22 deletions(-) create mode 100644 src/Symfony/Component/Webhook/Server/NativeJsonPayloadSerializer.php create mode 100644 src/Symfony/Component/Webhook/Server/PayloadSerializerInterface.php create mode 100644 src/Symfony/Component/Webhook/Server/SerializerPayloadSerializer.php create mode 100644 src/Symfony/Component/Webhook/Tests/Server/JsonBodyConfiguratorTest.php diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php index 49821a043de57..e784329dd3efb 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php @@ -548,7 +548,7 @@ public function load(array $configs, ContainerBuilder $container): void $this->registerProfilerConfiguration($config['profiler'], $container, $loader); if ($this->readConfigEnabled('webhook', $container, $config['webhook'])) { - $this->registerWebhookConfiguration($config['webhook'], $container, $loader); + $this->registerWebhookConfiguration($config['webhook'], $container, $loader, $this->readConfigEnabled('serializer', $container, $config['serializer'])); // If Webhook is installed but the HttpClient or Serializer components are not available, we should throw an error if (!$this->readConfigEnabled('http_client', $container, $config['http_client'])) { @@ -559,14 +559,6 @@ public function load(array $configs, ContainerBuilder $container): void ) ->addTag('container.error'); } - if (!$this->readConfigEnabled('serializer', $container, $config['serializer'])) { - $container->getDefinition('webhook.body_configurator.json') - ->setArguments([]) - ->addError('You cannot use the "webhook transport" service since the Serializer component is not ' - .(class_exists(Serializer::class) ? 'enabled. Try setting "framework.serializer.enabled" to true.' : 'installed. Try running "composer require symfony/serializer-pack".') - ) - ->addTag('container.error'); - } } if ($this->readConfigEnabled('remote-event', $container, $config['remote-event'])) { @@ -2919,7 +2911,7 @@ private function registerNotifierConfiguration(array $config, ContainerBuilder $ } } - private function registerWebhookConfiguration(array $config, ContainerBuilder $container, PhpFileLoader $loader): void + private function registerWebhookConfiguration(array $config, ContainerBuilder $container, PhpFileLoader $loader, bool $serializerEnabled): void { if (!class_exists(WebhookController::class)) { throw new LogicException('Webhook support cannot be enabled as the component is not installed. Try running "composer require symfony/webhook".'); @@ -2938,6 +2930,9 @@ private function registerWebhookConfiguration(array $config, ContainerBuilder $c $controller = $container->getDefinition('webhook.controller'); $controller->replaceArgument(0, $parsers); $controller->replaceArgument(1, new Reference($config['message_bus'])); + + $jsonBodyConfigurator = $container->getDefinition('webhook.body_configurator.json'); + $jsonBodyConfigurator->replaceArgument(0, new Reference($serializerEnabled ? 'webhook.payload_serializer.serializer' : 'webhook.payload_serializer.json')); } private function registerRemoteEventConfiguration(PhpFileLoader $loader): void diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/config/webhook.php b/src/Symfony/Bundle/FrameworkBundle/Resources/config/webhook.php index a7e9d58ce9a65..85cf9bb40607a 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Resources/config/webhook.php +++ b/src/Symfony/Bundle/FrameworkBundle/Resources/config/webhook.php @@ -17,6 +17,8 @@ use Symfony\Component\Webhook\Server\HeadersConfigurator; use Symfony\Component\Webhook\Server\HeaderSignatureConfigurator; use Symfony\Component\Webhook\Server\JsonBodyConfigurator; +use Symfony\Component\Webhook\Server\NativeJsonPayloadSerializer; +use Symfony\Component\Webhook\Server\SerializerPayloadSerializer; use Symfony\Component\Webhook\Server\Transport; return static function (ContainerConfigurator $container) { @@ -32,6 +34,13 @@ ->set('webhook.headers_configurator', HeadersConfigurator::class) ->set('webhook.body_configurator.json', JsonBodyConfigurator::class) + ->args([ + abstract_arg('payload serializer'), + ]) + + ->set('webhook.payload_serializer.json', NativeJsonPayloadSerializer::class) + + ->set('webhook.payload_serializer.serializer', SerializerPayloadSerializer::class) ->args([ service('serializer'), ]) diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/ConfigurationTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/ConfigurationTest.php index 793be02836b3c..1aee0564626b9 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/ConfigurationTest.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/ConfigurationTest.php @@ -27,10 +27,12 @@ use Symfony\Component\Messenger\MessageBusInterface; use Symfony\Component\Notifier\Notifier; use Symfony\Component\RateLimiter\Policy\TokenBucketLimiter; +use Symfony\Component\RemoteEvent\RemoteEvent; use Symfony\Component\Scheduler\Messenger\SchedulerTransportFactory; use Symfony\Component\Serializer\Encoder\JsonDecode; use Symfony\Component\TypeInfo\Type; use Symfony\Component\Uid\Factory\UuidFactory; +use Symfony\Component\Webhook\Controller\WebhookController; class ConfigurationTest extends TestCase { @@ -925,12 +927,12 @@ class_exists(SemaphoreStore::class) && SemaphoreStore::isSupported() ? 'semaphor ], 'exceptions' => [], 'webhook' => [ - 'enabled' => false, + 'enabled' => !class_exists(FullStack::class) && class_exists(WebhookController::class), 'routing' => [], 'message_bus' => 'messenger.default_bus', ], 'remote-event' => [ - 'enabled' => false, + 'enabled' => !class_exists(FullStack::class) && class_exists(RemoteEvent::class), ], ]; } diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTestCase.php b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTestCase.php index b37d2e910ec45..dc35d0f863018 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTestCase.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTestCase.php @@ -2357,7 +2357,7 @@ public function testWebhook() $this->assertSame(RequestParser::class, $container->getDefinition('webhook.request_parser')->getClass()); $this->assertFalse($container->getDefinition('webhook.transport')->hasErrors()); - $this->assertFalse($container->getDefinition('webhook.body_configurator.json')->hasErrors()); + $this->assertEquals('webhook.payload_serializer.serializer', $container->getDefinition('webhook.body_configurator.json')->getArgument(0)); } public function testWebhookWithoutSerializer() @@ -2369,11 +2369,7 @@ public function testWebhookWithoutSerializer() $container = $this->createContainerFromFile('webhook_without_serializer'); $this->assertFalse($container->getDefinition('webhook.transport')->hasErrors()); - $this->assertTrue($container->getDefinition('webhook.body_configurator.json')->hasErrors()); - $this->assertSame( - ['You cannot use the "webhook transport" service since the Serializer component is not enabled. Try setting "framework.serializer.enabled" to true.'], - $container->getDefinition('webhook.body_configurator.json')->getErrors() - ); + $this->assertEquals('webhook.payload_serializer.json', $container->getDefinition('webhook.body_configurator.json')->getArgument(0)); } public function testAssetMapperWithoutAssets() diff --git a/src/Symfony/Bundle/FrameworkBundle/composer.json b/src/Symfony/Bundle/FrameworkBundle/composer.json index af934f35df91f..5e27ec62cf482 100644 --- a/src/Symfony/Bundle/FrameworkBundle/composer.json +++ b/src/Symfony/Bundle/FrameworkBundle/composer.json @@ -71,6 +71,7 @@ "symfony/property-info": "^6.4|^7.0", "symfony/uid": "^6.4|^7.0", "symfony/web-link": "^6.4|^7.0", + "symfony/webhook": "^7.2", "phpdocumentor/reflection-docblock": "^3.0|^4.0|^5.0", "twig/twig": "^3.0.4" }, @@ -102,6 +103,7 @@ "symfony/twig-bundle": "<6.4", "symfony/validator": "<6.4", "symfony/web-profiler-bundle": "<6.4", + "symfony/webhook": "<7.2", "symfony/workflow": "<6.4" }, "autoload": { diff --git a/src/Symfony/Component/Webhook/CHANGELOG.md b/src/Symfony/Component/Webhook/CHANGELOG.md index 903b9bece8105..00964c8b8550e 100644 --- a/src/Symfony/Component/Webhook/CHANGELOG.md +++ b/src/Symfony/Component/Webhook/CHANGELOG.md @@ -1,6 +1,11 @@ CHANGELOG ========= +7.2 +--- + + * Add `PayloadSerializerInterface` with implementations to decouple the remote event handling from the Serializer component + 6.4 --- diff --git a/src/Symfony/Component/Webhook/Server/JsonBodyConfigurator.php b/src/Symfony/Component/Webhook/Server/JsonBodyConfigurator.php index b67b0ab01d42e..380d615195fe5 100644 --- a/src/Symfony/Component/Webhook/Server/JsonBodyConfigurator.php +++ b/src/Symfony/Component/Webhook/Server/JsonBodyConfigurator.php @@ -20,14 +20,16 @@ */ final class JsonBodyConfigurator implements RequestConfiguratorInterface { - public function __construct( - private readonly SerializerInterface $serializer, - ) { + private PayloadSerializerInterface $payloadSerializer; + + public function __construct(SerializerInterface|PayloadSerializerInterface $payloadSerializer) + { + $this->payloadSerializer = $payloadSerializer instanceof SerializerInterface ? new SerializerPayloadSerializer($payloadSerializer) : $payloadSerializer; } public function configure(RemoteEvent $event, #[\SensitiveParameter] string $secret, HttpOptions $options): void { - $body = $this->serializer->serialize($event->getPayload(), 'json'); + $body = $this->payloadSerializer->serialize($event->getPayload()); $options->setBody($body); $headers = $options->toArray()['headers']; $headers['Content-Type'] = 'application/json'; diff --git a/src/Symfony/Component/Webhook/Server/NativeJsonPayloadSerializer.php b/src/Symfony/Component/Webhook/Server/NativeJsonPayloadSerializer.php new file mode 100644 index 0000000000000..1d4e26670efae --- /dev/null +++ b/src/Symfony/Component/Webhook/Server/NativeJsonPayloadSerializer.php @@ -0,0 +1,20 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Webhook\Server; + +final class NativeJsonPayloadSerializer implements PayloadSerializerInterface +{ + public function serialize(array $payload): string + { + return json_encode($payload, \JSON_THROW_ON_ERROR); + } +} diff --git a/src/Symfony/Component/Webhook/Server/PayloadSerializerInterface.php b/src/Symfony/Component/Webhook/Server/PayloadSerializerInterface.php new file mode 100644 index 0000000000000..95430a7c541b5 --- /dev/null +++ b/src/Symfony/Component/Webhook/Server/PayloadSerializerInterface.php @@ -0,0 +1,17 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Webhook\Server; + +interface PayloadSerializerInterface +{ + public function serialize(array $payload): string; +} diff --git a/src/Symfony/Component/Webhook/Server/SerializerPayloadSerializer.php b/src/Symfony/Component/Webhook/Server/SerializerPayloadSerializer.php new file mode 100644 index 0000000000000..ea1e9390e1baf --- /dev/null +++ b/src/Symfony/Component/Webhook/Server/SerializerPayloadSerializer.php @@ -0,0 +1,27 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Webhook\Server; + +use Symfony\Component\Serializer\SerializerInterface; + +final class SerializerPayloadSerializer implements PayloadSerializerInterface +{ + public function __construct( + private SerializerInterface $serializer, + ) { + } + + public function serialize(array $payload): string + { + return $this->serializer->serialize($payload, 'json'); + } +} diff --git a/src/Symfony/Component/Webhook/Tests/Server/JsonBodyConfiguratorTest.php b/src/Symfony/Component/Webhook/Tests/Server/JsonBodyConfiguratorTest.php new file mode 100644 index 0000000000000..a0fbfdbe226ca --- /dev/null +++ b/src/Symfony/Component/Webhook/Tests/Server/JsonBodyConfiguratorTest.php @@ -0,0 +1,67 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Webhook\Tests\Server; + +use PHPUnit\Framework\TestCase; +use Symfony\Component\HttpClient\HttpOptions; +use Symfony\Component\RemoteEvent\RemoteEvent; +use Symfony\Component\Serializer\SerializerInterface; +use Symfony\Component\Webhook\Server\JsonBodyConfigurator; +use Symfony\Component\Webhook\Server\PayloadSerializerInterface; + +class JsonBodyConfiguratorTest extends TestCase +{ + public function testPayloadWithPayloadSerializer() + { + $payload = ['foo' => 'bar']; + + $payloadSerializer = $this->createMock(PayloadSerializerInterface::class); + $payloadSerializer + ->expects($this->once()) + ->method('serialize') + ->with($payload) + ; + + $httpOptions = new HttpOptions(); + $httpOptions->setHeaders([ + 'Webhook-Event' => 'event-name', + 'Webhook-Id' => 'event-id', + ]); + + $configurator = new JsonBodyConfigurator($payloadSerializer); + $configurator->configure(new RemoteEvent('event-name', 'event-id', $payload), 's3cr3t', $httpOptions); + } + + public function testPayloadWithSerializer() + { + $payload = ['foo' => 'bar']; + + $payloadEncoder = $this->createMock(SerializerInterface::class); + $payloadEncoder + ->expects($this->once()) + ->method('serialize') + ->with($payload, 'json') + ->willReturn('{"foo": "bar"}') + ; + + $httpOptions = new HttpOptions(); + $httpOptions->setHeaders([ + 'Webhook-Event' => 'event-name', + 'Webhook-Id' => 'event-id', + ]); + + $configurator = new JsonBodyConfigurator($payloadEncoder); + $configurator->configure(new RemoteEvent('event-name', 'event-id', $payload), 's3cr3t', $httpOptions); + + $this->assertJsonStringEqualsJsonString('{"foo": "bar"}', $httpOptions->toArray()['body']); + } +} diff --git a/src/Symfony/Component/Webhook/composer.json b/src/Symfony/Component/Webhook/composer.json index af3074e56844a..46ce35b5d90cb 100644 --- a/src/Symfony/Component/Webhook/composer.json +++ b/src/Symfony/Component/Webhook/composer.json @@ -22,6 +22,10 @@ "symfony/messenger": "^6.4|^7.0", "symfony/remote-event": "^6.4|^7.0" }, + "require-dev": { + "symfony/http-client": "^6.4|^7.0", + "symfony/serializer": "^6.4|^7.0" + }, "autoload": { "psr-4": { "Symfony\\Component\\Webhook\\": "" }, "exclude-from-classmap": [