From 421e6cf3c2ad854e99a37662d1d7a1c0fd5a432b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Niedzielski?= Date: Mon, 1 Apr 2019 23:17:24 +0200 Subject: [PATCH 1/5] Fix DBAL Consumer duplicating messages when rejecting with requeue --- pkg/dbal/DbalConsumer.php | 6 ++--- .../Tests/Functional/DbalConsumerTest.php | 25 +++++++++++++++++++ 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/pkg/dbal/DbalConsumer.php b/pkg/dbal/DbalConsumer.php index e95a8f513..9d99d5a58 100644 --- a/pkg/dbal/DbalConsumer.php +++ b/pkg/dbal/DbalConsumer.php @@ -100,16 +100,14 @@ public function reject(Message $message, bool $requeue = false): void { InvalidMessageException::assertMessageInstanceOf($message, DbalMessage::class); + $this->acknowledge($message); + if ($requeue) { $message = clone $message; $message->setRedelivered(false); $this->getContext()->createProducer()->send($this->queue, $message); - - return; } - - $this->deleteMessage($message->getDeliveryId()); } protected function getContext(): DbalContext diff --git a/pkg/dbal/Tests/Functional/DbalConsumerTest.php b/pkg/dbal/Tests/Functional/DbalConsumerTest.php index 1f8757e15..fa74ae781 100644 --- a/pkg/dbal/Tests/Functional/DbalConsumerTest.php +++ b/pkg/dbal/Tests/Functional/DbalConsumerTest.php @@ -144,6 +144,31 @@ public function testShouldDeleteExpiredMessage() $this->assertSame(0, $this->getQuerySize()); } + public function testShouldRemoveOriginalMessageThatHaveBeenRejectedWithRequeu() + { + $context = $this->context; + $queue = $context->createQueue(__METHOD__); + + $consumer = $context->createConsumer($queue); + + // guard + $this->assertSame(0, $this->getQuerySize()); + + $producer = $context->createProducer(); + + /** @var DbalMessage $message */ + $message = $context->createMessage(__CLASS__); + $producer->send($queue, $message); + + $this->assertCount(1, $this->getQuerySize()); + + $message = $consumer->receive(100); // 100ms + + $this->assertInstanceOf(DbalMessage::class, $message); + $consumer->reject($message, true); + $this->assertCount(1, $this->getQuerySize()); + } + private function getQuerySize(): int { return (int) $this->context->getDbalConnection() From a3d741503bb38a66a77f8592b7ae2c584bfc1d21 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Niedzielski?= Date: Mon, 1 Apr 2019 23:25:22 +0200 Subject: [PATCH 2/5] Fix DBAL Consumer duplicating messages when rejecting with requeue --- pkg/dbal/Tests/Functional/DbalConsumerTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/dbal/Tests/Functional/DbalConsumerTest.php b/pkg/dbal/Tests/Functional/DbalConsumerTest.php index fa74ae781..fc6f54927 100644 --- a/pkg/dbal/Tests/Functional/DbalConsumerTest.php +++ b/pkg/dbal/Tests/Functional/DbalConsumerTest.php @@ -144,7 +144,7 @@ public function testShouldDeleteExpiredMessage() $this->assertSame(0, $this->getQuerySize()); } - public function testShouldRemoveOriginalMessageThatHaveBeenRejectedWithRequeu() + public function testShouldRemoveOriginalMessageThatHaveBeenRejectedWithRequeue() { $context = $this->context; $queue = $context->createQueue(__METHOD__); From 6967a04c32a253595b69d5f1b91df7b1c8e5dbe2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Niedzielski?= Date: Mon, 1 Apr 2019 23:38:54 +0200 Subject: [PATCH 3/5] Fix DbalConsumer message expecting to have delivery ID when rejecting --- pkg/dbal/Tests/DbalConsumerTest.php | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/dbal/Tests/DbalConsumerTest.php b/pkg/dbal/Tests/DbalConsumerTest.php index 85130893d..c042c5c86 100644 --- a/pkg/dbal/Tests/DbalConsumerTest.php +++ b/pkg/dbal/Tests/DbalConsumerTest.php @@ -169,6 +169,7 @@ public function testRejectShouldReSendMessageToSameQueueOnRequeue() $message = new DbalMessage(); $message->setBody('theBody'); + $message->setDeliveryId(__METHOD__); $producerMock = $this->createProducerMock(); $producerMock From 4e42a9cc42f6854659373bc5318cf5638b7b7389 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Niedzielski?= Date: Tue, 2 Apr 2019 00:01:55 +0200 Subject: [PATCH 4/5] Fix DBAL Consumer duplicating messages when rejecting with requeue Fix test because I'm dumb. --- pkg/dbal/Tests/Functional/DbalConsumerTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/dbal/Tests/Functional/DbalConsumerTest.php b/pkg/dbal/Tests/Functional/DbalConsumerTest.php index fc6f54927..0cca7c42e 100644 --- a/pkg/dbal/Tests/Functional/DbalConsumerTest.php +++ b/pkg/dbal/Tests/Functional/DbalConsumerTest.php @@ -166,7 +166,7 @@ public function testShouldRemoveOriginalMessageThatHaveBeenRejectedWithRequeue() $this->assertInstanceOf(DbalMessage::class, $message); $consumer->reject($message, true); - $this->assertCount(1, $this->getQuerySize()); + $this->assertSame(1, $this->getQuerySize()); } private function getQuerySize(): int From 270aa2879b218744abefd5028113548c1ca4b0f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Niedzielski?= Date: Tue, 2 Apr 2019 00:06:45 +0200 Subject: [PATCH 5/5] Fix DBAL Consumer duplicating messages when rejecting with requeue Fix test because I'm dumb. Now it's good. Seriously... --- pkg/dbal/Tests/Functional/DbalConsumerTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/dbal/Tests/Functional/DbalConsumerTest.php b/pkg/dbal/Tests/Functional/DbalConsumerTest.php index 0cca7c42e..686f87d16 100644 --- a/pkg/dbal/Tests/Functional/DbalConsumerTest.php +++ b/pkg/dbal/Tests/Functional/DbalConsumerTest.php @@ -160,7 +160,7 @@ public function testShouldRemoveOriginalMessageThatHaveBeenRejectedWithRequeue() $message = $context->createMessage(__CLASS__); $producer->send($queue, $message); - $this->assertCount(1, $this->getQuerySize()); + $this->assertSame(1, $this->getQuerySize()); $message = $consumer->receive(100); // 100ms