ARTEMIS-3363 Fix TransportConfiguration extraProps encoding

artemis-core-client/src/main/java/org/apache/activemq/artemis/api/core/TransportConfiguration.java

@@ -20,6 +20,7 @@ import javax.json.JsonObject;
 import java.io.Serializable;
 import java.util.HashMap;
 import java.util.Map;
+import java.util.Objects;
 
 import org.apache.activemq.artemis.core.client.ActiveMQClientMessageBundle;
 import org.apache.activemq.artemis.core.remoting.impl.TransportConfigurationUtil;
@@ -46,6 +47,8 @@ public class TransportConfiguration implements Serializable {
 
    private static final long serialVersionUID = -3994528421527392679L;
 
+   public static final String EXTRA_PROPERTY_PREFIX = "$.EP.";
+
    private String name;
 
    private String factoryClassName;
@@ -215,8 +218,15 @@ public class TransportConfiguration implements Serializable {
          return false;
       }
 
-      if (name != null ? !name.equals(that.name) : that.name != null)
+      if (!Objects.equals(name, that.name)) {
          return false;
+      }
+
+      // Empty and null extraProps maps are equivalent so the condition to check if two extraProps maps are equal is:
+      // (extraProps == that.extraProps) || (extraProps != null && ((extraProps.isEmpty() && that.extraProps == null) || extraProps.equals(that.extraProps)))
+      if ((extraProps != that.extraProps) && (extraProps == null || ((!extraProps.isEmpty() || that.extraProps != null) && !extraProps.equals(that.extraProps)))) {
+         return false;
+      }
 
       return true;
    }
@@ -224,7 +234,7 @@ public class TransportConfiguration implements Serializable {
    public boolean isSameParams(TransportConfiguration that) {
       if (!factoryClassName.equals(that.factoryClassName))
          return false;
-      if (params != null ? !params.equals(that.params) : that.params != null)
+      if (!Objects.equals(params, that.params))
          return false;
 
       return true;
@@ -306,9 +316,9 @@ public class TransportConfiguration implements Serializable {
       return str.toString();
    }
 
-   private void encodeMap(final ActiveMQBuffer buffer, final Map<String, Object> map) {
+   private void encodeMap(final ActiveMQBuffer buffer, final Map<String, Object> map, final String prefix) {
       for (Map.Entry<String, Object> entry : map.entrySet()) {
-         buffer.writeString(entry.getKey());
+         buffer.writeString(prefix != null ? prefix + entry.getKey() : entry.getKey());
 
          Object val = entry.getValue();
 
@@ -341,13 +351,13 @@ public class TransportConfiguration implements Serializable {
       buffer.writeString(name);
       buffer.writeString(factoryClassName);
 
-      buffer.writeInt(params == null ? 0 : params.size());
+      buffer.writeInt((params == null ? 0 : params.size()) + (extraProps == null ? 0 : extraProps.size()));
 
       if (params != null) {
-         encodeMap(buffer, params);
+         encodeMap(buffer, params, null);
       }
       if (extraProps != null) {
-         encodeMap(buffer, extraProps);
+         encodeMap(buffer, extraProps, EXTRA_PROPERTY_PREFIX);
       }
    }
 
@@ -405,7 +415,14 @@ public class TransportConfiguration implements Serializable {
             }
          }
 
-         params.put(key, val);
+         if (key.startsWith(EXTRA_PROPERTY_PREFIX)) {
+            if (extraProps == null) {
+               extraProps = new HashMap<>();
+            }
+            extraProps.put(key.substring(EXTRA_PROPERTY_PREFIX.length()), val);
+         } else {
+            params.put(key, val);
+         }
       }
    }
 

artemis-core-client/src/test/java/org/apache/activemq/artemis/api/core/TransportConfigurationTest.java

@@ -17,8 +17,12 @@
 
 package org.apache.activemq.artemis.api.core;
 
+import java.util.Collections;
 import java.util.HashMap;
+import java.util.Map;
 
+import io.netty.buffer.Unpooled;
+import org.apache.activemq.artemis.core.buffers.impl.ChannelBufferWrapper;
 import org.apache.activemq.artemis.core.remoting.impl.netty.TransportConstants;
 import org.junit.Assert;
 import org.junit.Test;
@@ -74,4 +78,49 @@ public class TransportConfigurationTest {
       Assert.assertThat(configuration.toString(), not(containsString("secret_password")));
    }
 
+   @Test
+   public void testEncodingDecoding() {
+      Map<String, Object> params = new HashMap<>();
+      params.put("BOOLEAN_PARAM", true);
+      params.put("INT_PARAM", 0);
+      params.put("LONG_PARAM", 1);
+      params.put("STRING_PARAM", "A");
+
+      Map<String, Object> extraProps = new HashMap<>();
+      extraProps.put("EXTRA_BOOLEAN_PROP", false);
+      extraProps.put("EXTRA_INT_PROP", 1);
+      extraProps.put("EXTRA_LONG_PROP", 0);
+      extraProps.put("EXTRA_STRING_PROP", "Z");
+
+      testEncodingDecoding(new TransportConfiguration("SomeClass", params, "TEST", extraProps));
+   }
+
+   @Test
+   public void testEncodingDecodingWithEmptyMaps() {
+      testEncodingDecoding(new TransportConfiguration("SomeClass", Collections.emptyMap(), "TEST", Collections.emptyMap()));
+   }
+
+   @Test
+   public void testEncodingDecodingWithNullMaps() {
+      testEncodingDecoding(new TransportConfiguration("SomeClass", null, "TEST", null));
+   }
+
+   private void testEncodingDecoding(TransportConfiguration transportConfiguration) {
+      ActiveMQBuffer buffer = new ChannelBufferWrapper(Unpooled.buffer(1024));
+
+      transportConfiguration.encode(buffer);
+
+      TransportConfiguration decodedTransportConfiguration = new TransportConfiguration();
+      decodedTransportConfiguration.decode(buffer);
+
+      Assert.assertFalse(buffer.readable());
+
+      Assert.assertEquals(transportConfiguration.getParams(), decodedTransportConfiguration.getParams());
+
+      Assert.assertTrue((transportConfiguration.getExtraParams() == null && (decodedTransportConfiguration.getExtraParams() == null || decodedTransportConfiguration.getExtraParams().isEmpty())) ||
+            (decodedTransportConfiguration.getExtraParams() == null && (transportConfiguration.getExtraParams() == null || transportConfiguration.getExtraParams().isEmpty())) ||
+            (transportConfiguration.getExtraParams() != null && decodedTransportConfiguration.getExtraParams() != null && transportConfiguration.getExtraParams().equals(decodedTransportConfiguration.getExtraParams())));
+
+      Assert.assertEquals(transportConfiguration, decodedTransportConfiguration);
+   }
 }