From f6ad0936b3f6e6f5fde31fa6bfe2be51d88bc795 Mon Sep 17 00:00:00 2001 From: Justin Hart Date: Wed, 6 Jul 2022 15:13:35 -0700 Subject: [PATCH 1/3] initial pass at gracefully handling binary --- .gitignore | 2 ++ pom.xml | 36 +++++++++---------- .../SQSMessagingClientConstants.java | 2 ++ .../sqs/javamessaging/message/SQSMessage.java | 21 +++++++++-- 4 files changed, 40 insertions(+), 21 deletions(-) diff --git a/.gitignore b/.gitignore index 2f7896d..49b5003 100644 --- a/.gitignore +++ b/.gitignore @@ -1 +1,3 @@ target/ +.tool-versions +.idea diff --git a/pom.xml b/pom.xml index 9e0e980..eba783f 100644 --- a/pom.xml +++ b/pom.xml @@ -127,24 +127,24 @@ true - - org.apache.maven.plugins - maven-gpg-plugin - - - sign-artifacts - verify - - sign - - - ${gpg.sqs.keyname} - gpg.sqs.passphrase - true - - - - + + + + + + + + + + + + + + + + + + diff --git a/src/main/java/com/amazon/sqs/javamessaging/SQSMessagingClientConstants.java b/src/main/java/com/amazon/sqs/javamessaging/SQSMessagingClientConstants.java index 099c8b4..5253873 100644 --- a/src/main/java/com/amazon/sqs/javamessaging/SQSMessagingClientConstants.java +++ b/src/main/java/com/amazon/sqs/javamessaging/SQSMessagingClientConstants.java @@ -52,6 +52,8 @@ public class SQSMessagingClientConstants { public static final String SHORT = "Number.short"; + public static final String BINARY = "Binary"; + public static final String INT_FALSE = "0"; public static final String INT_TRUE = "1"; diff --git a/src/main/java/com/amazon/sqs/javamessaging/message/SQSMessage.java b/src/main/java/com/amazon/sqs/javamessaging/message/SQSMessage.java index 04dbf3e..19f1035 100644 --- a/src/main/java/com/amazon/sqs/javamessaging/message/SQSMessage.java +++ b/src/main/java/com/amazon/sqs/javamessaging/message/SQSMessage.java @@ -30,10 +30,12 @@ import javax.jms.MessageNotWriteableException; import com.amazon.sqs.javamessaging.SQSMessageConsumerPrefetch; +import com.amazon.sqs.javamessaging.SQSMessageProducer; import com.amazon.sqs.javamessaging.SQSMessagingClientConstants; import com.amazon.sqs.javamessaging.SQSQueueDestination; import com.amazon.sqs.javamessaging.acknowledge.Acknowledger; import com.amazonaws.services.sqs.model.MessageAttributeValue; +import org.apache.commons.logging.LogFactory; import static com.amazon.sqs.javamessaging.SQSMessagingClientConstants.*; @@ -158,10 +160,20 @@ private void mapSystemAttributeToJmsMessageProperty(Map systemAtt writePermissionsForProperties = true; } + // TODO: do something to handle unsupported DataTypes instead of Exception + // TODO: Default: log a message, otherwise use a registered handler (future) private void addMessageAttributes(com.amazonaws.services.sqs.model.Message sqsMessage) throws JMSException { for (Entry entry : sqsMessage.getMessageAttributes().entrySet()) { - properties.put(entry.getKey(), new JMSMessagePropertyValue( - entry.getValue().getStringValue(), entry.getValue().getDataType())); + // getDataType: one of String, Number, and Binary. + String type = entry.getValue().getDataType(); + if (type != null && (type.startsWith(STRING) || type.startsWith(NUMBER))) { + properties.put(entry.getKey(), new JMSMessagePropertyValue( + entry.getValue().getStringValue(), entry.getValue().getDataType())); + } else if (BINARY.equals(type)) { + // if Binary, getBinaryValue() should be used but should require an userland mapper + // TODO but for now we're just going to log and skip it + LogFactory.getLog(SQSMessage.class).warn("MessageAttribute with BINARY key: " + entry.getKey()); + } } } @@ -1128,7 +1140,7 @@ static public T convert(Object value, Class clazz) { * attribute type and message attribute string value. */ public static class JMSMessagePropertyValue { - + private final Object value; private final String type; @@ -1211,6 +1223,9 @@ private static Object getObjectValue(String value, String type) throws JMSExcept return Short.valueOf(value); } else if (type != null && (type.startsWith(STRING) || type.startsWith(NUMBER))) { return value; + } else if (BINARY.equals(type)) { + // This is a safety catch for binary type + return null; } else { throw new JMSException(type + " is not a supported JMS property type"); } From 1b4f7aa9226b309740f2582f50d4321919c6b36c Mon Sep 17 00:00:00 2001 From: Justin Hart Date: Thu, 7 Jul 2022 08:03:06 -0700 Subject: [PATCH 2/3] wacky key handling --- .../sqs/javamessaging/message/SQSMessage.java | 10 ++- .../javamessaging/message/SQSMessageTest.java | 75 +++++++++++++++++++ 2 files changed, 83 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/amazon/sqs/javamessaging/message/SQSMessage.java b/src/main/java/com/amazon/sqs/javamessaging/message/SQSMessage.java index 19f1035..b9b4098 100644 --- a/src/main/java/com/amazon/sqs/javamessaging/message/SQSMessage.java +++ b/src/main/java/com/amazon/sqs/javamessaging/message/SQSMessage.java @@ -164,14 +164,20 @@ private void mapSystemAttributeToJmsMessageProperty(Map systemAtt // TODO: Default: log a message, otherwise use a registered handler (future) private void addMessageAttributes(com.amazonaws.services.sqs.model.Message sqsMessage) throws JMSException { for (Entry entry : sqsMessage.getMessageAttributes().entrySet()) { + // transform Key to conform to `\w` (alphanum_) only. + // TODO make this a userland transformer, or use a default + String key = entry.getKey(); + String sanitizedKey = key.replaceAll("[\\W$]", "_"); + // getDataType: one of String, Number, and Binary. String type = entry.getValue().getDataType(); if (type != null && (type.startsWith(STRING) || type.startsWith(NUMBER))) { - properties.put(entry.getKey(), new JMSMessagePropertyValue( + properties.put(sanitizedKey, new JMSMessagePropertyValue( entry.getValue().getStringValue(), entry.getValue().getDataType())); } else if (BINARY.equals(type)) { // if Binary, getBinaryValue() should be used but should require an userland mapper - // TODO but for now we're just going to log and skip it + // it must map to one of Boolean, Byte, Short, Integer, Long, Float, Double, and String. + // TODO userland mapper, but for now we're just going to log and skip it. The key won't be added LogFactory.getLog(SQSMessage.class).warn("MessageAttribute with BINARY key: " + entry.getKey()); } } diff --git a/src/test/java/com/amazon/sqs/javamessaging/message/SQSMessageTest.java b/src/test/java/com/amazon/sqs/javamessaging/message/SQSMessageTest.java index dab640c..9e30eb9 100644 --- a/src/test/java/com/amazon/sqs/javamessaging/message/SQSMessageTest.java +++ b/src/test/java/com/amazon/sqs/javamessaging/message/SQSMessageTest.java @@ -37,6 +37,8 @@ import junit.framework.Assert; +import java.nio.ByteBuffer; +import java.nio.charset.StandardCharsets; import java.util.*; /** @@ -56,6 +58,9 @@ public class SQSMessageTest { final String myCustomString = "myCustomString"; final String myNumber = "myNumber"; + final String binaryString = "myBinaryString"; + final ByteBuffer myBinaryString = ByteBuffer.wrap(binaryString.getBytes(StandardCharsets.UTF_8)); + @Before public void setup() { mockSQSSession = mock(SQSSession.class); @@ -335,6 +340,10 @@ public void testSQSMessageAttributeToProperty() throws JMSException { .withDataType(SQSMessagingClientConstants.NUMBER) .withStringValue("500")); + messageAttributes.put(binaryString, new MessageAttributeValue() + .withDataType(SQSMessagingClientConstants.BINARY) + .withBinaryValue(myBinaryString)); + com.amazonaws.services.sqs.model.Message sqsMessage = new com.amazonaws.services.sqs.model.Message() .withMessageAttributes(messageAttributes) .withAttributes(systemAttributes) @@ -392,6 +401,9 @@ public void testSQSMessageAttributeToProperty() throws JMSException { Assert.assertEquals(message.getFloatProperty(myNumber), 500f); Assert.assertEquals(message.getDoubleProperty(myNumber), 500d); + // Assert that the binary doesn't get set + Assert.assertFalse(message.propertyExists(binaryString)); + Assert.assertNull(message.getObjectProperty(binaryString)); // Validate property names Set propertyNamesSet = new HashSet(Arrays.asList( @@ -426,8 +438,71 @@ public void testSQSMessageAttributeToProperty() throws JMSException { Assert.assertFalse(message.propertyExists("myByteProperty")); Assert.assertFalse(message.propertyExists("myString")); Assert.assertFalse(message.propertyExists("myNumber")); + Assert.assertFalse(message.propertyExists("myBinaryString")); propertyNames = message.getPropertyNames(); assertFalse(propertyNames.hasMoreElements()); } + + /** + * Test using SQS message attribute during SQS Message constructing + */ + @Test + public void testSQSMessageAttributeRenaming() throws JMSException { + + Acknowledger ack = mock(Acknowledger.class); + + Map systemAttributes = new HashMap(); + systemAttributes.put(APPROXIMATE_RECEIVE_COUNT, "100"); + + Map messageAttributes = new HashMap(); + + // TODO key string that would normally break + messageAttributes.put("foo-bar-baz", new MessageAttributeValue() + .withDataType(SQSMessagingClientConstants.STRING) + .withStringValue("StringValue")); + + // TODO string that would really badly break + messageAttributes.put("this!has+no^chill", new MessageAttributeValue() + .withDataType(SQSMessagingClientConstants.NUMBER + ".custom") + .withStringValue("['one', 'two']")); + + com.amazonaws.services.sqs.model.Message sqsMessage = new com.amazonaws.services.sqs.model.Message() + .withMessageAttributes(messageAttributes) + .withAttributes(systemAttributes) + .withMessageId("messageId") + .withReceiptHandle("ReceiptHandle"); + + SQSMessage message = new SQSMessage(ack, "QueueUrl", sqsMessage); + + Assert.assertTrue(message.propertyExists("foo_bar_baz")); + Assert.assertEquals(message.getObjectProperty("foo_bar_baz"), "StringValue"); + Assert.assertEquals(message.getStringProperty("foo_bar_baz"), "StringValue"); + + Assert.assertTrue(message.propertyExists("this_has_no_chill")); + Assert.assertEquals(message.getObjectProperty("this_has_no_chill"), "['one', 'two']"); + Assert.assertEquals(message.getStringProperty("this_has_no_chill"), "['one', 'two']"); + + // Validate property names + Set propertyNamesSet = new HashSet(Arrays.asList( + "foo_bar_baz", + "this_has_no_chill", + JMSX_DELIVERY_COUNT)); + + Enumeration propertyNames = message.getPropertyNames(); + int counter = 0; + while (propertyNames.hasMoreElements()) { + assertTrue(propertyNamesSet.contains(propertyNames.nextElement())); + counter++; + } + assertEquals(propertyNamesSet.size(), counter); + + message.clearProperties(); + Assert.assertFalse(message.propertyExists("foo_bar_baz")); + Assert.assertFalse(message.propertyExists("this_has_no_chill")); + + propertyNames = message.getPropertyNames(); + assertFalse(propertyNames.hasMoreElements()); + } + } From 55c2df44792c795dded85f0537293fe96016193b Mon Sep 17 00:00:00 2001 From: Justin Hart Date: Tue, 26 Jul 2022 15:25:01 -0700 Subject: [PATCH 3/3] some cleanup, add a test for truely unknown type --- pom.xml | 2 +- .../sqs/javamessaging/message/SQSMessage.java | 18 +++++---- .../javamessaging/message/SQSMessageTest.java | 37 +++++++++++++++++++ 3 files changed, 48 insertions(+), 9 deletions(-) diff --git a/pom.xml b/pom.xml index eba783f..77c413b 100644 --- a/pom.xml +++ b/pom.xml @@ -6,7 +6,7 @@ com.amazonaws amazon-sqs-java-messaging-lib - 1.1.0 + 1.2.0 jar Amazon SQS Java Messaging Library The Amazon SQS Java Messaging Library holds the Java Message Service compatible classes, that are used diff --git a/src/main/java/com/amazon/sqs/javamessaging/message/SQSMessage.java b/src/main/java/com/amazon/sqs/javamessaging/message/SQSMessage.java index b9b4098..de039e7 100644 --- a/src/main/java/com/amazon/sqs/javamessaging/message/SQSMessage.java +++ b/src/main/java/com/amazon/sqs/javamessaging/message/SQSMessage.java @@ -30,7 +30,6 @@ import javax.jms.MessageNotWriteableException; import com.amazon.sqs.javamessaging.SQSMessageConsumerPrefetch; -import com.amazon.sqs.javamessaging.SQSMessageProducer; import com.amazon.sqs.javamessaging.SQSMessagingClientConstants; import com.amazon.sqs.javamessaging.SQSQueueDestination; import com.amazon.sqs.javamessaging.acknowledge.Acknowledger; @@ -160,25 +159,28 @@ private void mapSystemAttributeToJmsMessageProperty(Map systemAtt writePermissionsForProperties = true; } - // TODO: do something to handle unsupported DataTypes instead of Exception - // TODO: Default: log a message, otherwise use a registered handler (future) private void addMessageAttributes(com.amazonaws.services.sqs.model.Message sqsMessage) throws JMSException { for (Entry entry : sqsMessage.getMessageAttributes().entrySet()) { - // transform Key to conform to `\w` (alphanum_) only. - // TODO make this a userland transformer, or use a default + // transform Key to conform to java-identifier (alphanum_) mostly. also allows `.$` in the spec. + // TODO make this a userland transformer, with this default + // other libraries use like `_$dash$_ etc String key = entry.getKey(); - String sanitizedKey = key.replaceAll("[\\W$]", "_"); + String sanitizedKey = key.replaceAll("[^\\w$.]", "_"); - // getDataType: one of String, Number, and Binary. String type = entry.getValue().getDataType(); if (type != null && (type.startsWith(STRING) || type.startsWith(NUMBER))) { + //Supported directly by JMSMessagePropertyValue as a String properties.put(sanitizedKey, new JMSMessagePropertyValue( entry.getValue().getStringValue(), entry.getValue().getDataType())); } else if (BINARY.equals(type)) { - // if Binary, getBinaryValue() should be used but should require an userland mapper + // if Binary, getBinaryValue() should be used but requires an userland mapper // it must map to one of Boolean, Byte, Short, Integer, Long, Float, Double, and String. // TODO userland mapper, but for now we're just going to log and skip it. The key won't be added LogFactory.getLog(SQSMessage.class).warn("MessageAttribute with BINARY key: " + entry.getKey()); + } else { + // this is an unknown type. By default, throw an exception + // TODO allow a userland mapper? + throw new JMSException(type + " is not a supported JMS property type for key " + key); } } } diff --git a/src/test/java/com/amazon/sqs/javamessaging/message/SQSMessageTest.java b/src/test/java/com/amazon/sqs/javamessaging/message/SQSMessageTest.java index 9e30eb9..eeb48dc 100644 --- a/src/test/java/com/amazon/sqs/javamessaging/message/SQSMessageTest.java +++ b/src/test/java/com/amazon/sqs/javamessaging/message/SQSMessageTest.java @@ -444,6 +444,43 @@ public void testSQSMessageAttributeToProperty() throws JMSException { assertFalse(propertyNames.hasMoreElements()); } + /** + * Test using SQS message attribute during SQS Message constructing + */ + @Test + public void testSQSUnknownType() throws JMSException { + + Acknowledger ack = mock(Acknowledger.class); + + Map systemAttributes = new HashMap(); + systemAttributes.put(APPROXIMATE_RECEIVE_COUNT, "100"); + + Map messageAttributes = new HashMap(); + + messageAttributes.put(myString, new MessageAttributeValue() + .withDataType(SQSMessagingClientConstants.STRING) + .withStringValue("StringValue")); + + messageAttributes.put("arbitrary", new MessageAttributeValue() + .withDataType("Arbitrary") + .withStringValue("ArbitraryValue")); + + com.amazonaws.services.sqs.model.Message sqsMessage = new com.amazonaws.services.sqs.model.Message() + .withMessageAttributes(messageAttributes) + .withAttributes(systemAttributes) + .withMessageId("messageId") + .withReceiptHandle("ReceiptHandle"); + + boolean caught = false; + try { + SQSMessage message = new SQSMessage(ack, "QueueUrl", sqsMessage); + } catch (JMSException e) { + caught = true; + } + + assertTrue(caught); + } + /** * Test using SQS message attribute during SQS Message constructing */