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]

Reply via email to