Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit fb17f3a

Browse filesBrowse files
nicolas-grekasnicolas-grekas
committed
bug #64128 [Messenger] Fix "--fetch-size" option rejecting valid values (xeno-suter)
This PR was merged into the 8.1 branch. Discussion ---------- [Messenger] Fix "--fetch-size" option rejecting valid values | Q | A | ------------- | --- | Branch? | 8.1 | Bug fix? | yes | New feature? | no | Deprecations? | no | Issues | - | License | MIT The `--fetch-size` option, added in 8.1 (#63662), is currently unusable for its intended purpose: any value greater than 1 throws an `InvalidArgumentException`. In `ConsumeMessagesCommand::execute()` the validation reads: ```php if (1 < $fetchSize = (int) $input->getOption('fetch-size')) { throw new \InvalidArgumentException(\sprintf('The "--fetch-size" option must be a positive integer, "%s" given.', ...)); } ``` The condition is inverted. It triggers on the valid range (`> 1`) and silently accepts `0` and negative values. The error message ("must be a positive integer") confirms the intent was the opposite: reject values `< 1`. Since `Worker::run()` caps `fetch_size` with `max(1, ...)`, the bug is purely in the command layer, but it makes `messenger:consume --fetch-size=8` unusable from the CLI. Before: ```console $ bin/console messenger:consume async --fetch-size=8 In ConsumeMessagesCommand.php line 317: The "--fetch-size" option must be a positive integer, "8" given. ``` After: the command runs and the receiver is asked for batches of 8. This PR flips the comparison to `1 > $fetchSize` and adds two tests in `ConsumeMessagesCommandTest`: - `testRunWithFetchSizeOption` runs the command with `--fetch-size=8` and asserts the receiver actually saw the fetch size (regression test; fails on 8.1 without this patch). - `testRunWithInvalidFetchSizeOption` keeps the existing rejection behaviour for `--fetch-size=0`. Commits ------- 69b03d0 [Messenger] Fix "--fetch-size" option rejecting valid values
2 parents d12ed2b + 69b03d0 commit fb17f3a
Copy full SHA for fb17f3a

2 files changed

+48-1Lines changed: 48 additions & 1 deletion

File tree

Expand file treeCollapse file tree
Open diff view settings
Filter options
Expand file treeCollapse file tree
Open diff view settings
Collapse file

‎src/Symfony/Component/Messenger/Command/ConsumeMessagesCommand.php‎

Copy file name to clipboardExpand all lines: src/Symfony/Component/Messenger/Command/ConsumeMessagesCommand.php
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -312,7 +312,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
312312
$options['queues'] = $queues;
313313
}
314314

315-
if (1 < $fetchSize = (int) $input->getOption('fetch-size')) {
315+
if (1 > $fetchSize = (int) $input->getOption('fetch-size')) {
316316
throw new \InvalidArgumentException(\sprintf('The "--fetch-size" option must be a positive integer, "%s" given.', $input->getOption('fetch-size')));
317317
}
318318

Collapse file

‎src/Symfony/Component/Messenger/Tests/Command/ConsumeMessagesCommandTest.php‎

Copy file name to clipboardExpand all lines: src/Symfony/Component/Messenger/Tests/Command/ConsumeMessagesCommandTest.php
+47Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
use Symfony\Component\Messenger\MessageBusInterface;
3131
use Symfony\Component\Messenger\RoutableMessageBus;
3232
use Symfony\Component\Messenger\Stamp\BusNameStamp;
33+
use Symfony\Component\Messenger\Tests\Fixtures\DummyReceiver;
3334
use Symfony\Component\Messenger\Tests\Fixtures\ResettableDummyReceiver;
3435
use Symfony\Component\Messenger\Transport\Receiver\ReceiverInterface;
3536

@@ -176,6 +177,52 @@ public static function getInvalidOptions()
176177
yield 'Negative reset interval' => ['--no-reset', '-1', 'Option "no-reset" must be a positive integer, "-1" passed.'];
177178
}
178179

180+
public function testRunWithFetchSizeOption()
181+
{
182+
$envelope = new Envelope(new \stdClass(), [new BusNameStamp('dummy-bus')]);
183+
184+
$receiver = new DummyReceiver([[$envelope]]);
185+
186+
$receiverLocator = new Container();
187+
$receiverLocator->set('dummy-receiver', $receiver);
188+
189+
$busLocator = new Container();
190+
$busLocator->set('dummy-bus', new MessageBus());
191+
192+
$command = new ConsumeMessagesCommand(new RoutableMessageBus($busLocator), $receiverLocator, new EventDispatcher());
193+
194+
$application = new Application();
195+
$application->addCommand($command);
196+
$tester = new CommandTester($application->get('messenger:consume'));
197+
$tester->execute([
198+
'receivers' => ['dummy-receiver'],
199+
'--fetch-size' => '8',
200+
'--limit' => 1,
201+
]);
202+
203+
$tester->assertCommandIsSuccessful();
204+
$this->assertSame([8], $receiver->getFetchSizes());
205+
}
206+
207+
public function testRunWithInvalidFetchSizeOption()
208+
{
209+
$receiverLocator = new Container();
210+
$receiverLocator->set('dummy-receiver', new \stdClass());
211+
212+
$command = new ConsumeMessagesCommand(new RoutableMessageBus(new Container()), $receiverLocator, new EventDispatcher());
213+
214+
$application = new Application();
215+
$application->addCommand($command);
216+
$tester = new CommandTester($application->get('messenger:consume'));
217+
218+
$this->expectException(\InvalidArgumentException::class);
219+
$this->expectExceptionMessage('The "--fetch-size" option must be a positive integer, "0" given.');
220+
$tester->execute([
221+
'receivers' => ['dummy-receiver'],
222+
'--fetch-size' => '0',
223+
]);
224+
}
225+
179226
public function testRunWithTimeLimit()
180227
{
181228
$envelope = new Envelope(new \stdClass(), [new BusNameStamp('dummy-bus')]);

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.