From b73e61ac528443b7978a9c70e5b76a1a0b8aeefc Mon Sep 17 00:00:00 2001 From: Andy Taylor Date: Tue, 3 Nov 2015 10:53:07 +0000 Subject: [PATCH] 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 --- .../api/core/TransportConfiguration.java | 4 +- .../client/impl/ClientSessionFactoryImpl.java | 3 +- .../core/client/impl/TopologyMemberImpl.java | 2 +- .../impl/TransportConfigurationUtil.java | 19 +++++ .../impl/TransportConfigurationTest.java | 75 +++++++++++++++++++ 5 files changed, 99 insertions(+), 4 deletions(-) diff --git a/artemis-core-client/src/main/java/org/apache/activemq/artemis/api/core/TransportConfiguration.java b/artemis-core-client/src/main/java/org/apache/activemq/artemis/api/core/TransportConfiguration.java index 666c5c8369..3ad7a735ba 100644 --- a/artemis-core-client/src/main/java/org/apache/activemq/artemis/api/core/TransportConfiguration.java +++ b/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) diff --git a/artemis-core-client/src/main/java/org/apache/activemq/artemis/core/client/impl/ClientSessionFactoryImpl.java b/artemis-core-client/src/main/java/org/apache/activemq/artemis/core/client/impl/ClientSessionFactoryImpl.java index 07a51e4c99..c69ef4d923 100644 --- a/artemis-core-client/src/main/java/org/apache/activemq/artemis/core/client/impl/ClientSessionFactoryImpl.java +++ b/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 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); diff --git a/artemis-core-client/src/main/java/org/apache/activemq/artemis/core/client/impl/TopologyMemberImpl.java b/artemis-core-client/src/main/java/org/apache/activemq/artemis/core/client/impl/TopologyMemberImpl.java index 7c11cd0a52..10704c6793 100644 --- a/artemis-core-client/src/main/java/org/apache/activemq/artemis/core/client/impl/TopologyMemberImpl.java +++ b/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 { diff --git a/artemis-core-client/src/main/java/org/apache/activemq/artemis/core/remoting/impl/TransportConfigurationUtil.java b/artemis-core-client/src/main/java/org/apache/activemq/artemis/core/remoting/impl/TransportConfigurationUtil.java index 721290a64c..244e138de7 100644 --- a/artemis-core-client/src/main/java/org/apache/activemq/artemis/core/remoting/impl/TransportConfigurationUtil.java +++ b/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; + } } diff --git a/tests/unit-tests/src/test/java/org/apache/activemq/artemis/tests/unit/core/config/impl/TransportConfigurationTest.java b/tests/unit-tests/src/test/java/org/apache/activemq/artemis/tests/unit/core/config/impl/TransportConfigurationTest.java index dccd5c2b47..e670bbc4c4 100644 --- a/tests/unit-tests/src/test/java/org/apache/activemq/artemis/tests/unit/core/config/impl/TransportConfigurationTest.java +++ b/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 params1 = new HashMap<>(); + params1.put("host", "blah"); + params1.put("port", "5467"); + TransportConfiguration tc1 = new TransportConfiguration(NettyConnectorFactory.class.getName(), params1); + Map 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 params1 = new HashMap<>(); + params1.put("host", "blah"); + params1.put("port", "5467"); + TransportConfiguration tc1 = new TransportConfiguration(NettyConnectorFactory.class.getName(), params1); + Map 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 params1 = new HashMap<>(); + params1.put("host", TransportConstants.DEFAULT_HOST); + params1.put("port", TransportConstants.DEFAULT_PORT); + TransportConfiguration tc1 = new TransportConfiguration(NettyConnectorFactory.class.getName(), params1); + Map params2 = new HashMap<>(); + TransportConfiguration tc2 = new TransportConfiguration(NettyConnectorFactory.class.getName(), params2); + Assert.assertTrue(TransportConfigurationUtil.isSameHost(tc1, tc2)); + } + + @Test + public void testSameHostInVMTrue() { + Map 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 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 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 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 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 params2 = new HashMap<>(); + TransportConfiguration tc2 = new TransportConfiguration(NettyConnectorFactory.class.getName(), params2); + Assert.assertTrue(TransportConfigurationUtil.isSameHost(tc1, tc2)); + } // Package protected --------------------------------------------- // Protected -----------------------------------------------------