[GitHub] [activemq-artemis] jbertram commented on a change in pull request #3783: ARTEMIS-3513 Compacting exception invalidates deletes and updates

[GitBox <gi...@apache.org>]

jbertram commented on a change in pull request #3783:
URL: https://github.com/apache/activemq-artemis/pull/3783#discussion_r721836399



##########
File path: artemis-protocols/artemis-openwire-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/openwire/OpenWireMessageConverter.java
##########
@@ -710,7 +710,12 @@ private static ActiveMQMessage toAMQMessage(MessageReference reference,
    private static <T> T getObjectProperty(ICoreMessage message, Class<T> type, SimpleString property) {
       if (message.getPropertyNames().contains(property)) {
          try {
-            return type.cast(message.getObjectProperty(property));
+            Object value = message.getObjectProperty(property);
+            if (type == String.class) {
+               return (T)value.toString();

Review comment:
       My recent change was mainly to ensure that any `ClassCastException` would be isolated and logged rather than blowing up the whole message conversion. Your change doesn't alter that semantic so it's fine. Although I think the `else` block should have this instead:
   ```
   return type.cast(value);
   ```
   Since `value` already has the result of `message.getObjectProperty(property)`.




-- 
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: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org