Skip to content

[SQS] add messageId to the sqsMessage #992

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.

Sign up for GitHub

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

Merged
merged 7 commits into from
Jan 18, 2020
Merged

[SQS] add messageId to the sqsMessage #992

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
e6aa278
inject messageId to the sqsMessage
Nov 29, 2019
ca12581
inject messageId to the sqsMessage
Nov 29, 2019
84d09e0
fix unit test
Dec 4, 2019
f8a458d
remove useless if
Jan 7, 2020
5736c26
fix phpunit test
Jan 7, 2020
f2bd9db
use a property for the messageId
Jan 14, 2020
b7a9877
Revert "use a property for the messageId"
Jan 14, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pkg/snsqs/Tests/SnsQsProducerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ public function testCouldBeConstructedWithRequiredArguments()
public function testShouldThrowIfMessageIsInvalidType()
{
$this->expectException(InvalidMessageException::class);
$this->expectExceptionMessage('The message must be an instance of Enqueue\SnsQs\SnsQsMessage but it is Double\Message\P1');
$this->expectExceptionMessage('The message must be an instance of Enqueue\SnsQs\SnsQsMessage but it is Double\Message\P4');

$producer = new SnsQsProducer($this->createSnsContextMock(), $this->createSqsContextMock());

Expand Down
2 changes: 2 additions & 0 deletions pkg/sqs/SqsConsumer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,8 @@ protected function convertMessage(array $sqsMessage): SqsMessage
$message->setProperties($headers[1]);
}

$message->setMessageId($sqsMessage['MessageId']);

return $message;
}
}
3 changes: 2 additions & 1 deletion pkg/sqs/Tests/Functional/SqsCommonUseCasesTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,8 @@ public function testProduceAndReceiveOneMessageSentDirectlyToQueue()

$this->assertEquals(__METHOD__, $message->getBody());
$this->assertEquals(['FooProperty' => 'FooVal'], $message->getProperties());
$this->assertEquals(['BarHeader' => 'BarVal'], $message->getHeaders());
$this->assertEquals('BarVal', $message->getHeaders()['BarHeader']);
$this->assertNotNull($message->getMessageId());
}

public function testProduceAndReceiveOneMessageSentDirectlyToTopic()
Expand Down
5 changes: 4 additions & 1 deletion pkg/sqs/Tests/SqsConsumerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,7 @@ public function testShouldReceiveMessage()
$expectedSqsMessage = [
'Body' => 'The Body',
'ReceiptHandle' => 'The Receipt',
'MessageId' => 'theMessageId',
'Attributes' => [
'SenderId' => 'AROAX5IAWYILCTYIS3OZ5:foo@bar.com',
'ApproximateFirstReceiveTimestamp' => '1560512269481',
Expand Down Expand Up @@ -337,7 +338,7 @@ public function testShouldReceiveMessage()

$this->assertInstanceOf(SqsMessage::class, $result);
$this->assertEquals('The Body', $result->getBody());
$this->assertEquals(['hkey' => 'hvalue'], $result->getHeaders());
$this->assertEquals(['hkey' => 'hvalue', 'message_id' => 'theMessageId'], $result->getHeaders());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

message_id should not be a part of headers. That was wrong to put it there. Could you please create and use a dedicated property for it in Message class.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I change this, we will need to change this in all the other message type as we are reusing the test between services

$this->assertEquals(['key' => 'value'], $result->getProperties());
$this->assertEquals([
'SenderId' => 'AROAX5IAWYILCTYIS3OZ5:foo@bar.com',
Expand All @@ -347,6 +348,7 @@ public function testShouldReceiveMessage()
], $result->getAttributes());
$this->assertTrue($result->isRedelivered());
$this->assertEquals('The Receipt', $result->getReceiptHandle());
$this->assertEquals('theMessageId', $result->getMessageId());
}

public function testShouldReceiveMessageWithCustomRegion()
Expand All @@ -368,6 +370,7 @@ public function testShouldReceiveMessageWithCustomRegion()
->willReturn(new Result(['Messages' => [[
'Body' => 'The Body',
'ReceiptHandle' => 'The Receipt',
'MessageId' => 'theMessageId',
'Attributes' => [
'ApproximateReceiveCount' => 3,
],
Expand Down