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 2 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
36 changes: 18 additions & 18 deletions pom.xml
Original file line number Diff line number Diff line change
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
27 changes: 24 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 @@ -30,10 +30,12 @@
import javax.jms.MessageNotWriteableException;

import com.amazon.sqs.javamessaging.SQSMessageConsumerPrefetch;
import com.amazon.sqs.javamessaging.SQSMessageProducer;
onyxraven marked this conversation as resolved.
Show resolved Hide resolved
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;
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 @@ -158,10 +160,26 @@ private void mapSystemAttributeToJmsMessageProperty(Map<String,String> 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<String, MessageAttributeValue> entry : sqsMessage.getMessageAttributes().entrySet()) {
properties.put(entry.getKey(), new JMSMessagePropertyValue(
entry.getValue().getStringValue(), entry.getValue().getDataType()));
// 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(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
// 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());
}
}
}

Expand Down Expand Up @@ -1128,7 +1146,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 +1229,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
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,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<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());
}

}