awslabs / amazon-sqs-java-extended-client-lib

An extension to the Amazon SQS client that enables sending and receiving messages up to 2GB via Amazon S3.
Apache License 2.0
211 stars 109 forks source link

Fixes #25: Only add message attribute name if not already there #26

Closed dsw2127 closed 6 years ago

dsw2127 commented 6 years ago

If someone re-uses the ReceiveMessageRequest object for multiple calls to AmazonSQSExtendedClient, the latter will add the RESERVED_ATTRIBUTE_NAME String as a message attribute name every time. This can cause 413 / Request Entity Too Large errors from AWS SQS over time.

dsw2127 commented 6 years ago

Without the fix, the new test fails with:

junit.framework.AssertionFailedError: expected:<{AttributeNames: [],MessageAttributeNames: [SQSLargePayloadSize],}> but was:<{AttributeNames: [],MessageAttributeNames: [SQSLargePayloadSize, SQSLargePayloadSize],}>
troy-aws commented 6 years ago

It's already been fixed with recent release.

dsw2127 commented 6 years ago

@troy-aws it looks like you included the exact diff of my fix in the 1.0.2 commit, but not the commit itself, and did not credit me in any way. Even your comment "It's already been fixed" seems to imply someone else did the work, not just copied my code. I realize this is open source and you are within your right to do this and it was only a small amount of work so I'm not super upset about it, but for the future it would be nice to merge the pull request instead (or cherry-pick the commit to a branch if it cannot be merged into master directly).

That said, thank you for maintaining this project!

troy-aws commented 6 years ago

Hi @dsw2127,

Thank you for the feedback, and your contributions to make this project better! Due to constraints in our current release process that need to keep both Github source and Maven release in sync, we were not able to directly merge pull requests into the master. I have updated the release notes to explicitly mention your commit as the source of fix. We will be work on improving this release process for future releases.

Thanks again,

dsw2127 commented 6 years ago

That makes sense, thank you for the additional context!