ARTEMIS-292 - fix isSameHost on TransportConfiguration

Ive renamed the current isSameHost method to isSameparams as thats what it checked and added a new method for isSameHost that checks the appropriate params for the connector. Ive changed ClientSessionFactoryImp to use this to correct the behaviour.

https://issues.apache.org/jira/browse/ARTEMIS-292

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

@@ -174,7 +174,7 @@ public class TransportConfiguration implements Serializable {
 
       TransportConfiguration that = (TransportConfiguration) o;
 
-      if (!isSameHost(that)) {
+      if (!isSameParams(that)) {
          return false;
       }
 
@@ -184,7 +184,7 @@ public class TransportConfiguration implements Serializable {
       return true;
    }
 
-   public boolean isSameHost(TransportConfiguration that) {
+   public boolean isSameParams(TransportConfiguration that) {
       if (!factoryClassName.equals(that.factoryClassName))
          return false;
       if (params != null ? !params.equals(that.params) : that.params != null)

artemis-core-client/src/main/java/org/apache/activemq/artemis/core/client/impl/ClientSessionFactoryImpl.java

@@ -48,6 +48,7 @@ import org.apache.activemq.artemis.core.client.ActiveMQClientLogger;
 import org.apache.activemq.artemis.core.client.ActiveMQClientMessageBundle;
 import org.apache.activemq.artemis.core.protocol.core.CoreRemotingConnection;
 import org.apache.activemq.artemis.core.remoting.FailureListener;
+import org.apache.activemq.artemis.core.remoting.impl.TransportConfigurationUtil;
 import org.apache.activemq.artemis.core.server.ActiveMQComponent;
 import org.apache.activemq.artemis.spi.core.protocol.RemotingConnection;
 import org.apache.activemq.artemis.spi.core.remoting.BufferHandler;
@@ -1295,7 +1296,7 @@ public class ClientSessionFactoryImpl implements ClientSessionFactoryInternal, C
                                Pair<TransportConfiguration, TransportConfiguration> connectorPair,
                                boolean isLast) {
          // if it is our connector then set the live id used for failover
-         if (connectorPair.getA() != null && connectorPair.getA().isSameHost(connectorConfig)) {
+         if (connectorPair.getA() != null && TransportConfigurationUtil.isSameHost(connectorPair.getA(), connectorConfig)) {
             liveNodeID = nodeID;
          }
          serverLocator.notifyNodeUp(uniqueEventID, nodeID, backupGroupName, scaleDownGroupName, connectorPair, isLast);

artemis-core-client/src/main/java/org/apache/activemq/artemis/core/client/impl/TopologyMemberImpl.java

@@ -107,7 +107,7 @@ public final class TopologyMemberImpl implements TopologyMember {
    }
 
    public boolean isMember(TransportConfiguration configuration) {
-      if (getConnector().getA() != null && getConnector().getA().isSameHost(configuration) || getConnector().getB() != null && getConnector().getB().isSameHost(configuration)) {
+      if (getConnector().getA() != null && getConnector().getA().isSameParams(configuration) || getConnector().getB() != null && getConnector().getB().isSameParams(configuration)) {
          return true;
       }
       else {

artemis-core-client/src/main/java/org/apache/activemq/artemis/core/remoting/impl/TransportConfigurationUtil.java

@@ -21,7 +21,10 @@ import java.security.PrivilegedAction;
 import java.util.HashMap;
 import java.util.Map;
 
+import org.apache.activemq.artemis.api.core.TransportConfiguration;
 import org.apache.activemq.artemis.api.core.TransportConfigurationHelper;
+import org.apache.activemq.artemis.core.remoting.impl.netty.NettyConnectorFactory;
+import org.apache.activemq.artemis.core.remoting.impl.netty.TransportConstants;
 import org.apache.activemq.artemis.utils.ClassloadingUtil;
 
 /**
@@ -78,4 +81,20 @@ public class TransportConfigurationUtil {
       }
       return cloned;
    }
+
+   public static boolean isSameHost(TransportConfiguration tc1, TransportConfiguration tc2) {
+      if (NettyConnectorFactory.class.getName().equals(tc1.getFactoryClassName())) {
+         String host1 = tc1.getParams().get("host") != null ? tc1.getParams().get("host").toString() : TransportConstants.DEFAULT_HOST;
+         String host2 = tc2.getParams().get("host") != null ? tc2.getParams().get("host").toString() : TransportConstants.DEFAULT_HOST;
+         String port1 = String.valueOf(tc1.getParams().get("port") != null ? tc1.getParams().get("port") : TransportConstants.DEFAULT_PORT);
+         String port2 = String.valueOf(tc2.getParams().get("port") != null ? tc2.getParams().get("port") : TransportConstants.DEFAULT_PORT);
+         return host1.equals(host2) && port1.equals(port2);
+      }
+      else if ("org.apache.activemq.artemis.core.remoting.impl.invm.InVMConnectorFactory".equals(tc1.getFactoryClassName())) {
+         String serverId1 = tc1.getParams().get("serverId") != null ? tc1.getParams().get("serverId").toString() : "0";
+         String serverId2 = tc2.getParams().get("serverId") != null ? tc2.getParams().get("serverId").toString() : "0";
+         return serverId1.equals(serverId2);
+      }
+      return false;
+   }
 }

tests/unit-tests/src/test/java/org/apache/activemq/artemis/tests/unit/core/config/impl/TransportConfigurationTest.java

@@ -16,6 +16,9 @@
  */
 package org.apache.activemq.artemis.tests.unit.core.config.impl;
 
+import org.apache.activemq.artemis.core.remoting.impl.TransportConfigurationUtil;
+import org.apache.activemq.artemis.core.remoting.impl.netty.NettyConnectorFactory;
+import org.apache.activemq.artemis.core.remoting.impl.netty.TransportConstants;
 import org.apache.activemq.artemis.tests.util.ActiveMQTestBase;
 import org.junit.Test;
 
@@ -23,6 +26,9 @@ import org.junit.Assert;
 
 import org.apache.activemq.artemis.api.core.TransportConfiguration;
 
+import java.util.HashMap;
+import java.util.Map;
+
 public class TransportConfigurationTest extends ActiveMQTestBase {
 
    // Constants -----------------------------------------------------
@@ -63,6 +69,75 @@ public class TransportConfigurationTest extends ActiveMQTestBase {
       Assert.assertEquals("192.168.0.10", addresses[2]);
    }
 
+   @Test
+   public void testSameHostNettyTrue() {
+      Map<String, Object> params1 = new HashMap<>();
+      params1.put("host", "blah");
+      params1.put("port", "5467");
+      TransportConfiguration tc1 = new TransportConfiguration(NettyConnectorFactory.class.getName(), params1);
+      Map<String, Object> params2 = new HashMap<>();
+      params2.put("host", "blah");
+      params2.put("port", "5467");
+      TransportConfiguration tc2 = new TransportConfiguration(NettyConnectorFactory.class.getName(), params2);
+      Assert.assertTrue(TransportConfigurationUtil.isSameHost(tc1, tc2));
+   }
+
+   @Test
+   public void testSameHostNettyFalse() {
+      Map<String, Object> params1 = new HashMap<>();
+      params1.put("host", "blah");
+      params1.put("port", "5467");
+      TransportConfiguration tc1 = new TransportConfiguration(NettyConnectorFactory.class.getName(), params1);
+      Map<String, Object> params2 = new HashMap<>();
+      params2.put("host", "blah2");
+      params2.put("port", "5467");
+      TransportConfiguration tc2 = new TransportConfiguration(NettyConnectorFactory.class.getName(), params2);
+      Assert.assertFalse(TransportConfigurationUtil.isSameHost(tc1, tc2));
+   }
+
+   @Test
+   public void testSameHostNettyTrueDefault() {
+      Map<String, Object> params1 = new HashMap<>();
+      params1.put("host", TransportConstants.DEFAULT_HOST);
+      params1.put("port", TransportConstants.DEFAULT_PORT);
+      TransportConfiguration tc1 = new TransportConfiguration(NettyConnectorFactory.class.getName(), params1);
+      Map<String, Object> params2 = new HashMap<>();
+      TransportConfiguration tc2 = new TransportConfiguration(NettyConnectorFactory.class.getName(), params2);
+      Assert.assertTrue(TransportConfigurationUtil.isSameHost(tc1, tc2));
+   }
+
+   @Test
+   public void testSameHostInVMTrue() {
+      Map<String, Object> params1 = new HashMap<>();
+      params1.put(org.apache.activemq.artemis.core.remoting.impl.invm.TransportConstants.SERVER_ID_PROP_NAME, "blah");
+      TransportConfiguration tc1 = new TransportConfiguration(INVM_CONNECTOR_FACTORY, params1);
+      Map<String, Object> params2 = new HashMap<>();
+      params2.put(org.apache.activemq.artemis.core.remoting.impl.invm.TransportConstants.SERVER_ID_PROP_NAME, "blah");
+      TransportConfiguration tc2 = new TransportConfiguration(NettyConnectorFactory.class.getName(), params2);
+      Assert.assertTrue(TransportConfigurationUtil.isSameHost(tc1, tc2));
+   }
+
+   @Test
+   public void testSameHostInVMFalse() {
+      Map<String, Object> params1 = new HashMap<>();
+      params1.put(org.apache.activemq.artemis.core.remoting.impl.invm.TransportConstants.SERVER_ID_PROP_NAME, "blah");
+      TransportConfiguration tc1 = new TransportConfiguration(INVM_CONNECTOR_FACTORY, params1);
+      Map<String, Object> params2 = new HashMap<>();
+      params2.put(org.apache.activemq.artemis.core.remoting.impl.invm.TransportConstants.SERVER_ID_PROP_NAME, "blah3");
+      TransportConfiguration tc2 = new TransportConfiguration(NettyConnectorFactory.class.getName(), params2);
+      Assert.assertFalse(TransportConfigurationUtil.isSameHost(tc1, tc2));
+   }
+
+
+   @Test
+   public void testSameHostInVMTrueDefault() {
+      Map<String, Object> params1 = new HashMap<>();
+      params1.put(org.apache.activemq.artemis.core.remoting.impl.invm.TransportConstants.SERVER_ID_PROP_NAME, "0");
+      TransportConfiguration tc1 = new TransportConfiguration(INVM_CONNECTOR_FACTORY, params1);
+      Map<String, Object> params2 = new HashMap<>();
+      TransportConfiguration tc2 = new TransportConfiguration(NettyConnectorFactory.class.getName(), params2);
+      Assert.assertTrue(TransportConfigurationUtil.isSameHost(tc1, tc2));
+   }
    // Package protected ---------------------------------------------
 
    // Protected -----------------------------------------------------