8000 feature #58248 [Webhook] Allow request parsers to return multiple `Re… · symfony/symfony@0ff1704 · GitHub
[go: up one dir, main page]

Skip to content

Commit 0ff1704

Browse files
committed
feature #58248 [Webhook] Allow request parsers to return multiple RemoteEvent's (kbond)
This PR was merged into the 7.2 branch. Discussion ---------- [Webhook] Allow request parsers to return multiple `RemoteEvent`'s | Q | A | ------------- | --- | Branch? | 7.2 | Bug fix? | no | New feature? | yes | Deprecations? | no | Issues | Unblocks #50324 | License | MIT I've run into services that send their webhook events in bulk - a single request containing multiple events. This is one idea to allow this. I'd like to get some input before going further. TODO: - [x] Update changelog - [x] Update/add tests Commits ------- a1fcfae [Webhook] Allow request parsers to return multiple `RemoteEvent`'s
2 parents 9bc8007 + a1fcfae commit 0ff1704

File tree

7 files changed

+79
-11
lines changed

7 files changed

+79
-11
lines changed

UPGRADE-7.2.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,15 @@ TwigBridge
93 8000 93

9494
* Deprecate passing a tag to the constructor of `FormThemeNode`
9595

96+
Webhook
97+
-------
98+
99+
* [BC BREAK] `RequestParserInterface::parse()` return type changed from
100+
`?RemoteEvent` to `RemoteEvent|array<RemoteEvent>|null`. Classes already
101+
implementing this interface are unaffected but consumers of this method
102+
will need to be updated to handle the new return type. Projects relying on
103+
the `WebhookController` of the component are not affected by the BC break
104+
96105
Yaml
97106
----
98107

src/Symfony/Component/Webhook/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ CHANGELOG
77
* Make `AbstractRequestParserTestCase` compatible with PHPUnit 10+
88
* Add `PayloadSerializerInterface` with implementations to decouple the remote event handling from the Serializer component
99
* Add optional `$request` argument to `RequestParserInterface::createSuccessfulResponse()` and `RequestParserInterface::createRejectedResponse()`
10+
* [BC BREAK] Change return type of `RequestParserInterface::parse()` to `RemoteEvent|array<RemoteEvent>|null` (from `?RemoteEvent`)
1011

1112
6.4
1213
---

src/Symfony/Component/Webhook/Client/AbstractRequestParser.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222
*/
2323
abstract class AbstractRequestParser implements RequestParserInterface
2424
{
25-
public function parse(Request $request, #[\SensitiveParameter] string $secret): ?RemoteEvent
25+
public function parse(Request $request, #[\SensitiveParameter] string $secret): RemoteEvent|array|null
2626
{
2727
$this->validate($request);
2828

@@ -47,7 +47,7 @@ public function createRejectedResponse(string $reason/* , ?Request $request = nu
4747

4848
abstract protected function getRequestMatcher(): RequestMatcherInterface;
4949

50-
abstract protected function doParse(Request $request, #[\SensitiveParameter] string $secret): ?RemoteEvent;
50+
abstract protected function doParse(Request $request, #[\SensitiveParameter] string $secret): RemoteEvent|array|null;
5151

5252
protected function validate(Request $request): void
5353
{

src/Symfony/Component/Webhook/Client/RequestParserInterface.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,11 +24,11 @@ interface RequestParserInterface
2424
/**
2525
* Parses an HTTP Request and converts it into a RemoteEvent.
2626
*
27-
* @return ?RemoteEvent Returns null if the webhook must be ignored
27+
* @return RemoteEvent|RemoteEvent[]|null Returns null if the webhook must be ignored
2828
*
2929
* @throws RejectWebhookException When the payload is rejected (signature issue, parse issue, ...)
3030
*/
31-
public function parse(Request $request, #[\SensitiveParameter] string $secret): ?RemoteEvent;
31+
public function parse(Request $request, #[\SensitiveParameter] string $secret): RemoteEvent|array|null;
3232

3333
/**
3434
* @param Request|null $request The original request that was received by the webhook controller

src/Symfony/Component/Webhook/Controller/WebhookController.php

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,12 +40,17 @@ public function handle(string $type, Request $request): Response
4040
}
4141
/** @var RequestParserInterface $parser */
4242
$parser = $this->parsers[$type]['parser'];
43+
$events = $parser->parse($request, $this->parsers[$type]['secret']);
4344

44-
if (!$event = $parser->parse($request, $this->parsers[$type]['secret'])) {
45+
if (!$events) {
4546
return $parser->createRejectedResponse('Unable to parse the webhook payload.', $request);
4647
}
4748

48-
$this->bus->dispatch(new ConsumeRemoteEventMessage($type, $event));
49+
$events = \is_array($events) ? $events : [$events];
50+
51+
foreach ($events as $event) {
52+
$this->bus->dispatch(new ConsumeRemoteEventMessage($type, $event));
53+
}
4954

5055
return $parser->createSuccessfulResponse($request);
5156
}

src/Symfony/Component/Webhook/Test/AbstractRequestParserTestCase.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ abstract class AbstractRequestParserTestCase extends TestCase
2626
* @dataProvider getPayloads
2727
*/
2828
#[DataProvider('getPayloads')]
29-
public function testParse(string $payload, RemoteEvent $expected)
29+
public function testParse(string $payload, RemoteEvent|array $expected)
3030
{
3131
$request = $this->createRequest($payload);
3232
$parser = $this->createRequestParser();
@@ -35,7 +35,7 @@ public function testParse(string $payload, RemoteEvent $expected)
3535
}
3636

3737
/**
38-
* @return iterable<array{string, RemoteEvent}>
38+
* @return iterable<array{string, RemoteEvent|RemoteEvent[]}>
3939
*/
4040
public static function getPayloads(): iterable
4141
{

src/Symfony/Component/Webhook/Tests/Controller/WebhookControllerTest.php

Lines changed: 56 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,10 @@ public function testNoParserAvailable()
3232
$this->assertSame(404, $response->getStatusCode());
3333
}
3434

35-
public function testParserRejectsPayload()
35+
/**
36+
* @dataProvider rejectedParseProvider
37+
*/
38+
public function testParserRejectsPayload($return)
3639
{
3740
$secret = '1234';
3841
$request = new Request();
@@ -41,7 +44,7 @@ public function testParserRejectsPayload()
4144
->expects($this->once())
4245
->method('parse')
4346
->with($request, $secret)
44-
->willReturn(null);
47+
->willReturn($return);
4548
$parser
4649
->expects($this->once())
4750
->method('createRejectedResponse')
@@ -59,7 +62,13 @@ public function testParserRejectsPayload()
5962
$this->assertSame('Unable to parse the webhook payload.', $response->getContent());
6063
}
6164

62-
public function testParserAcceptsPayload()
65+
public static function rejectedParseProvider(): iterable
66+
{
67+
yield 'null' => [null];
68+
yield 'empty array' => [[]];
69+
}
70+
71+
public function testParserAcceptsPayloadAndReturnsSingleEvent()
6372
{
6473
$secret = '1234';
6574
$request = new Request();
@@ -97,4 +106,48 @@ public function dispatch(object $message, array $stamps = []): Envelope
97106
$this->assertSame('foo', $bus->message->getType());
98107
$this->assertEquals($event, $bus->message->getEvent());
99108
}
109+
110+
public function testParserAcceptsPayloadAndReturnsMultipleEvents()
111+
{
112+
$secret = '1234';
113+
$request = new Request();
114+
$event1 = new RemoteEvent('name1', 'id1', ['payload1']);
115+
$event2 = new RemoteEvent('name2', 'id2', ['payload2']);
116+
$parser = $this->createMock(RequestParserInterface::class);
117+
$parser
118+
->expects($this->once())
119+
->method('parse')
120+
->with($request, $secret)
121+
->willReturn([$event1, $event2]);
122+
$parser
123+
->expects($this->once())
124+
->method('createSuccessfulResponse')
125+
->with($request)
126+
->willReturn(new Response('', 202));
127+
$bus = new class implements MessageBusInterface {
128+
public array $messages = [];
129+
130+
public function dispatch(object $message, array $stamps = []): Envelope
131+
{
132+
return new Envelope($this->messages[] = $message, $stamps);
133+
}
134+
};
135+
136+
$controller = new WebhookController(
137+
['foo' => ['parser' => $parser, 'secret' => $secret]],
138+
$bus,
139+
);
140+
141+
$response = $controller->handle('foo', $request);
142+
143+
$this->assertSame(202, $response->getStatusCode());
144+
$this->assertSame('', $response->getContent());
145+
$this->assertCount(2, $bus->messages);
146+
$this->assertInstanceOf(ConsumeRemoteEventMessage::class, $bus->messages[0]);
147+
$this->assertSame('foo', $bus->messages[0]->getType());
148+
$this->assertEquals($event1, $bus->messages[0]->getEvent());
149+
$this->assertInstanceOf(ConsumeRemoteEventMessage::class, $bus->messages[1]);
150+
$this->assertSame('foo', $bus->messages[1]->getType());
151+
$this->assertEquals($event2, $bus->messages[1]->getEvent());
152+
}
100153
}

0 commit comments

Comments
 (0)
0