gemmellr commented on code in PR #979:
URL: https://github.com/apache/activemq/pull/979#discussion_r1108708010
##########
activemq-client/src/main/java/org/apache/activemq/command/ActiveMQTextMessage.java:
##########
@@ -210,4 +210,22 @@ public String toString() {
}
return super.toString();
}
+
+ @SuppressWarnings("unchecked")
+ public boolean isBodyAssignableTo(Class c) throws JMSException {
+ /*
+ *If the message is a TextMessage, ObjectMessage, MapMessage or
BytesMessage
+ * and the message has no body, then the above does not apply and this
method
+ * will return true irrespective of the value of this parameter.
+ */
+ if (getText() == null) {
+ return true;
+ }
+ return c.isAssignableFrom(java.lang.String.class);
+ }
+
+ @SuppressWarnings("unchecked")
+ protected <T> T doGetBody(Class<T> asType) throws JMSException {
+ return getText() != null ? (T) getText() : null;
Review Comment:
Is the simpler "return (T) getText();" not equivalent?
##########
activemq-client/src/main/java/org/apache/activemq/command/ActiveMQMapMessage.java:
##########
@@ -827,4 +839,26 @@ public Map<String, Object> getContentMap() throws
JMSException {
initializeReading();
return map;
}
+
+ @Override
+ @SuppressWarnings("unchecked")
+ public boolean isBodyAssignableTo(Class c) throws JMSException {
+ final Map<String, Object> map = getContentMap();
+ if (map == null || map.isEmpty()) {
Review Comment:
doGetBody doesnt seem to special case the empty map as this does...should
it? (since it special casing saying true while being given any class here)
##########
activemq-client/src/main/java/org/apache/activemq/command/ActiveMQMessage.java:
##########
@@ -795,14 +795,22 @@ public void setJMSDeliveryTime(long deliveryTime) throws
JMSException {
this.deliveryTime = deliveryTime;
}
- @Override
- public <T> T getBody(Class<T> c) throws JMSException {
- throw new UnsupportedOperationException("getBody(Class<T>) is not
supported");
- }
+ @Override
+ public final <T> T getBody(Class<T> asType) throws JMSException {
+ if (isBodyAssignableTo(asType)) {
+ return doGetBody(asType);
+ }
+
+ throw new MessageFormatException("Message body cannot be read as type:
" + asType);
+ }
@Override
public boolean isBodyAssignableTo(Class c) throws JMSException {
- throw new UnsupportedOperationException("isBodyAssignableTo(Class) is
not supported");
+ return true;
Review Comment:
indentation is off (on the method itself...and some others above)
##########
activemq-client/src/main/java/org/apache/activemq/command/ActiveMQTextMessage.java:
##########
@@ -210,4 +210,22 @@ public String toString() {
}
return super.toString();
}
+
+ @SuppressWarnings("unchecked")
+ public boolean isBodyAssignableTo(Class c) throws JMSException {
+ /*
+ *If the message is a TextMessage, ObjectMessage, MapMessage or
BytesMessage
+ * and the message has no body, then the above does not apply and this
method
+ * will return true irrespective of the value of this parameter.
+ */
Review Comment:
Could probably make the comment make more sense by not using the full spec
sentence :)
##########
activemq-client/src/main/java/org/apache/activemq/command/ActiveMQBytesMessage.java:
##########
@@ -956,4 +956,22 @@ protected void doCompress() throws IOException {
}
}
}
+
+ @Override
+ @SuppressWarnings("unchecked")
+ public boolean isBodyAssignableTo(Class c) {
+ return getContent() == null || c.isAssignableFrom(byte[].class);
+ }
+
+ @SuppressWarnings("unchecked")
+ protected <T> T doGetBody(Class<T> asType) {
+ //Make sure the bytes are stored before trying to copy and return
+ if (dataOut != null && getContent() == null) {
+ storeContent();
+ }
+
+ final ByteSequence content = getContent();
+ return content != null ? (T) new ByteSequence(content.getData(),
content.getOffset(),
+ content.getLength()).getData() : null;
Review Comment:
(also if there is somehow any offset/length diff then the returned bytes
would actually be invalid as they dont account for those)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]