[SQS] add messageId to the sqsMessage#992
Conversation
pkg/sqs/SqsConsumer.php
Outdated
| { | ||
| $message = $this->context->createMessage(); | ||
|
|
||
| if (isset($sqsMessage['MessageId'])) { |
There was a problem hiding this comment.
No need for if. As far as I can see the field is there all the time.
There was a problem hiding this comment.
@makasim ok, i've update my code to remove the isset check and have also move the setMessage call to the end of the function, because the setHeader is overriding the message_id header value
There was a problem hiding this comment.
The if is still there, isn't it? Should be removed as the MessageId is always there.
There was a problem hiding this comment.
oups, my bad i was sure to have pushed the deletion on this if
BenoitLeveque
force-pushed
the
improve-sqs-message-content
branch
2 times, most recently
from
7d7b76b to
d6eb41a
Compare
|
@makasim any news about this pull request ? |
BenoitLeveque
force-pushed
the
improve-sqs-message-content
branch
from
d6559f6 to
f8a458d
Compare
| $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()); |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 reverts commit f2bd9db.
2 participants
Hi,
I saw that we are not allowing user to access the messageId of a sqs message.
following the issue #837 i have added the messageId in the convertMessage method.