Skip to content
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.

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

Handle binary and non-jms header names #1

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1 +1,3 @@
target/
.tool-versions
.idea
38 changes: 19 additions & 19 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

<groupId>com.amazonaws</groupId>
<artifactId>amazon-sqs-java-messaging-lib</artifactId>
<version>1.1.0</version>
<version>1.2.0</version>
<packaging>jar</packaging>
<name>Amazon SQS Java Messaging Library</name>
<description>The Amazon SQS Java Messaging Library holds the Java Message Service compatible classes, that are used
Expand Down Expand Up @@ -127,24 +127,24 @@
<autoReleaseAfterClose>true</autoReleaseAfterClose>
</configuration>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-gpg-plugin</artifactId>
<executions>
<execution>
<id>sign-artifacts</id>
<phase>verify</phase>
<goals>
<goal>sign</goal>
</goals>
<configuration>
<keyname>${gpg.sqs.keyname}</keyname>
<passphraseServerId>gpg.sqs.passphrase</passphraseServerId>
<skip>true</skip>
</configuration>
</execution>
</executions>
</plugin>
<!-- <plugin>-->
Copy link
Member Author

Choose a reason for hiding this comment

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

couldn't get this plugin to behave, so commented for now

<!-- <groupId>org.apache.maven.plugins</groupId>-->
<!-- <artifactId>maven-gpg-plugin</artifactId>-->
<!-- <executions>-->
<!-- <execution>-->
<!-- <id>sign-artifacts</id>-->
<!-- <phase>verify</phase>-->
<!-- <goals>-->
<!-- <goal>sign</goal>-->
<!-- </goals>-->
<!-- <configuration>-->
<!-- <keyname>${gpg.sqs.keyname}</keyname>-->
<!-- <passphraseServerId>gpg.sqs.passphrase</passphraseServerId>-->
<!-- <skip>true</skip>-->
<!-- </configuration>-->
<!-- </execution>-->
<!-- </executions>-->
<!-- </plugin>-->
</plugins>
</build>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down
29 changes: 26 additions & 3 deletions src/main/java/com/amazon/sqs/javamessaging/message/SQSMessage.java
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
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;
Copy link
Member Author

@onyxraven onyxraven Jul 7, 2022

Choose a reason for hiding this comment

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

temporary for now?


import static com.amazon.sqs.javamessaging.SQSMessagingClientConstants.*;

Expand Down Expand Up @@ -160,8 +161,27 @@ private void mapSystemAttributeToJmsMessageProperty(Map<String,String> systemAtt

private void addMessageAttributes(com.amazonaws.services.sqs.model.Message sqsMessage) throws JMSException {
for (Entry<String, MessageAttributeValue> entry : sqsMessage.getMessageAttributes().entrySet()) {
properties.put(entry.getKey(), new JMSMessagePropertyValue(
entry.getValue().getStringValue(), entry.getValue().getDataType()));
// 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 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 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
Copy link
Member Author

Choose a reason for hiding this comment

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

see comments

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);
}
}
}

Expand Down Expand Up @@ -1128,7 +1148,7 @@ static public <T> T convert(Object value, Class<T> clazz) {
* attribute type and message attribute string value.
*/
public static class JMSMessagePropertyValue {

private final Object value;

private final String type;
Expand Down Expand Up @@ -1211,6 +1231,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");
}
Expand Down
112 changes: 112 additions & 0 deletions src/test/java/com/amazon/sqs/javamessaging/message/SQSMessageTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@

import junit.framework.Assert;

import java.nio.ByteBuffer;
import java.nio.charset.StandardCharsets;
import java.util.*;

/**
Expand All @@ -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);
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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<String> propertyNamesSet = new HashSet<String>(Arrays.asList(
Expand Down Expand Up @@ -426,8 +438,108 @@ 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 testSQSUnknownType() throws JMSException {

Acknowledger ack = mock(Acknowledger.class);

Map<String,String> systemAttributes = new HashMap<String, String>();
systemAttributes.put(APPROXIMATE_RECEIVE_COUNT, "100");

Map<String, MessageAttributeValue> messageAttributes = new HashMap<String, MessageAttributeValue>();

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
*/
@Test
public void testSQSMessageAttributeRenaming() throws JMSException {

Acknowledger ack = mock(Acknowledger.class);

Map<String,String> systemAttributes = new HashMap<String, String>();
systemAttributes.put(APPROXIMATE_RECEIVE_COUNT, "100");

Map<String, MessageAttributeValue> messageAttributes = new HashMap<String, MessageAttributeValue>();

// 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<String> propertyNamesSet = new HashSet<String>(Arrays.asList(
"foo_bar_baz",
"this_has_no_chill",
JMSX_DELIVERY_COUNT));

Enumeration<String > 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());
}

}