amazon-sqs-java-messaging-lib icon indicating copy to clipboard operation
amazon-sqs-java-messaging-lib copied to clipboard

Support SQS messages with Type Binary and Number

Open candrews opened this issue 6 years ago • 8 comments

The SQS documentation indicates that messages with "Type" of "Number" and "Binary" are valid, see https://docs.aws.amazon.com/AWSSimpleQueueService/latest/SQSDeveloperGuide/sqs-message-attributes.html

This PR adds support for these "Type" values. According to https://docs.aws.amazon.com/AWSSimpleQueueService/latest/SQSDeveloperGuide/sqs-message-attributes.html

Number – Number attributes can store positive or negative numerical values. A number can have up to 38 digits of precision, and it can be between 10^-128 and 10^+126.

That means that this library cannot internally handle values as the largest JMS allowed numeric type (which is Double see https://docs.oracle.com/javaee/6/api/javax/jms/Message.html ) is not big enough to handle such values. Therefore, this PR treats "Number" values internally as strings, converting them upon request when methods such as Message.getDouble(String) are invoked.

The same approach is used for "Binary" - the value is held in a String. ByteBuffer or byte[], for example, as those are not JMS permitted data types, from https://docs.oracle.com/javaee/6/api/javax/jms/Message.html :

The getObjectProperty method only returns values of class Boolean, Byte, Short, Integer, Long, Float, Double, and String.

See #60

candrews avatar May 17 '18 21:05 candrews

@robin-aws pointed out that the suggested change for type "Binary" is incorrect - it gets the Binary data from the "StringValue" when binary data is actually stored in the "BinaryValue" field.

candrews avatar May 22 '18 18:05 candrews

Is this project still being maintained? The Number support is really a SEVERE issue, because even amazon console allows to add Number without a custom data type string after it.

Please merge.

ghost avatar Dec 14 '18 18:12 ghost

Hi @storebot - it is absolutely still maintained! I'm almost finished addressing some internal build issues that have made accepting pull requests difficult in the past, at which point I'll tackle this issue as it's clearly blocking several people.

@candrews and I have discussed this offline and as per his comment above the PR it still needs some tweaking. I will review the related issues/PRs and propose a solution next week.

robin-aws avatar Dec 14 '18 18:12 robin-aws

@candrews - could you separate this into two PRs? After reviewing the JMS specs I'm happy with the "Number" fix, given the allowances for converting to and from String values, and the fact that attributes are not really strongly typed (i.e. there's no way to ask for "the" type of an attribute). The "Binary" one needs more thought though.

robin-aws avatar Dec 21 '18 19:12 robin-aws

could you separate this into two PRs?

The binary commit builds on the work done in the number commit - if I separate the 2 commits into 2 PRs, there will be conflicts. So I pulled out the number commit into a new PR, https://github.com/awslabs/amazon-sqs-java-messaging-lib/pull/71, and I left the number commit here in this PR, so when you merge the number commit, this PR will still work for the binary change and not have conflicts.

candrews avatar Dec 23 '18 03:12 candrews

I've merged https://github.com/awslabs/amazon-sqs-java-messaging-lib/pull/71, so this PR will just be for the binary commit now.

robin-aws avatar Feb 08 '19 00:02 robin-aws

I've rebased this PR fixing the conflict. @robin-aws what do you, is this merge-able?

candrews avatar Oct 10 '19 16:10 candrews

Hi Craig,

No I'm afraid not, it won't actually work - as per your comment above (May 22), "it gets the Binary data from the "StringValue" when binary data is actually stored in the "BinaryValue" field." I had to re-read the history to recall that myself. :)

To support this through JMS, there will have to be some kind of configuration within this client to specify how to read the ByteBuffer from getBinaryValue() as a String. Perhaps it could default to Base64? I'm open to suggestion here.

robin-aws avatar Oct 11 '19 17:10 robin-aws