You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@activemq.apache.org by GitBox <gi...@apache.org> on 2021/10/17 00:42:49 UTC

[GitHub] [activemq-artemis] erwindon commented on a change in pull request #3796: ARTEMIS-3461 intercept an AMQP message before it is converted to CORE and try specialized converter

erwindon commented on a change in pull request #3796:
URL: https://github.com/apache/activemq-artemis/pull/3796#discussion_r729850641



##########
File path: artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/broker/AMQPMessage.java
##########
@@ -1726,4 +1732,47 @@ public Object getOwner() {
    public void setOwner(Object object) {
       this.owner = object;
    }
+
+
+   // *******************************************************************************************************************************
+   // Composite Data implementation
+
+   private static MessageOpenTypeFactory AMQP_FACTORY = new AmqpMessageOpenTypeFactory();
+
+   static class AmqpMessageOpenTypeFactory extends MessageOpenTypeFactory<AMQPMessage> {
+      @Override
+      protected void init() throws OpenDataException {
+         super.init();
+         addItem(CompositeDataConstants.TEXT_BODY, CompositeDataConstants.TEXT_BODY, SimpleType.STRING);
+      }
+
+      @Override
+      public Map<String, Object> getFields(AMQPMessage m, int valueSizeLimit, int delivery) throws OpenDataException {
+         Map<String, Object> rc = super.getFields(m, valueSizeLimit, delivery);
+
+         if (m.getBody() != null && m.getBody() instanceof AmqpValue) {

Review comment:
       the null-check is redundant as (null instanceof XXX) is always false

##########
File path: artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/broker/AMQPMessage.java
##########
@@ -1726,4 +1732,47 @@ public Object getOwner() {
    public void setOwner(Object object) {
       this.owner = object;
    }
+
+
+   // *******************************************************************************************************************************
+   // Composite Data implementation
+
+   private static MessageOpenTypeFactory AMQP_FACTORY = new AmqpMessageOpenTypeFactory();
+
+   static class AmqpMessageOpenTypeFactory extends MessageOpenTypeFactory<AMQPMessage> {
+      @Override
+      protected void init() throws OpenDataException {
+         super.init();
+         addItem(CompositeDataConstants.TEXT_BODY, CompositeDataConstants.TEXT_BODY, SimpleType.STRING);
+      }
+
+      @Override
+      public Map<String, Object> getFields(AMQPMessage m, int valueSizeLimit, int delivery) throws OpenDataException {
+         Map<String, Object> rc = super.getFields(m, valueSizeLimit, delivery);
+
+         if (m.getBody() != null && m.getBody() instanceof AmqpValue) {
+            // this is the QPID internal representation
+            // looks like e.g.: "[msg#2430, 4, {a=5, b=5}, 6, x, y]"

Review comment:
       (for better understanding, not a suggestion to change it)
   using the representation function from qpid has a few disadvantages:
   1) strings are not quoted, in the above example: "4" could have been a number or a string; also the representation suggests JSON, but due to the missing quotes it is not actually json
   2) the detailed types are not visible, a number may have been INT8, INT32, UINT8, etc.
   this representation is already a huge improvement over the previous implementation, which either showed a binary dump or it just crashed.

##########
File path: artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/broker/AMQPMessage.java
##########
@@ -1726,4 +1732,47 @@ public Object getOwner() {
    public void setOwner(Object object) {
       this.owner = object;
    }
+
+
+   // *******************************************************************************************************************************
+   // Composite Data implementation
+
+   private static MessageOpenTypeFactory AMQP_FACTORY = new AmqpMessageOpenTypeFactory();
+
+   static class AmqpMessageOpenTypeFactory extends MessageOpenTypeFactory<AMQPMessage> {
+      @Override
+      protected void init() throws OpenDataException {
+         super.init();
+         addItem(CompositeDataConstants.TEXT_BODY, CompositeDataConstants.TEXT_BODY, SimpleType.STRING);
+      }
+
+      @Override
+      public Map<String, Object> getFields(AMQPMessage m, int valueSizeLimit, int delivery) throws OpenDataException {
+         Map<String, Object> rc = super.getFields(m, valueSizeLimit, delivery);
+
+         if (m.getBody() != null && m.getBody() instanceof AmqpValue) {
+            // this is the QPID internal representation
+            // looks like e.g.: "[msg#2430, 4, {a=5, b=5}, 6, x, y]"

Review comment:
       (for better understanding, not a suggestion to change it)
   using the representation function from qpid has a few disadvantages:
   1) strings are not quoted, in the above example: "4" could have been a number or a string; also the representation suggests JSON, but due to the missing quotes it is not actually json
   2) the detailed types are not visible, a number may have been INT8, INT32, UINT8, etc.
   
   this representation is already a huge improvement over the previous implementation, which either showed a binary dump or it just crashed.




-- 
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