QPID-6126: Improve validation and exception messages for BDB HA node

git-svn-id: https://svn.apache.org/repos/asf/qpid/trunk@1629010 13f79535-47bb-0310-9956-ffa450edef68

5 files changed, 44 insertions, 43 deletions

diff --git a/qpid/java/bdbstore/src/main/java/org/apache/qpid/server/store/berkeleydb/replication/ReplicatedEnvironmentFacade.java b/qpid/java/bdbstore/src/main/java/org/apache/qpid/server/store/berkeleydb/replication/ReplicatedEnvironmentFacade.java
index 20dbdc703a..090ca87536 100644
--- a/qpid/java/bdbstore/src/main/java/org/apache/qpid/server/store/berkeleydb/replication/ReplicatedEnvironmentFacade.java
+++ b/qpid/java/bdbstore/src/main/java/org/apache/qpid/server/store/berkeleydb/replication/ReplicatedEnvironmentFacade.java
@@ -1329,7 +1329,7 @@ public class ReplicatedEnvironmentFacade implements EnvironmentFacade, StateChan
         }
         catch (IOException e)
         {
-            throw new IllegalConfigurationException(String.format("Cannot connect to '%s'", helperHostPort), e);
+            throw new IllegalConfigurationException(String.format("Cannot connect to existing node '%s' at '%s'", helperNodeName, helperHostPort), e);
         }
         catch (ServiceConnectFailedException e)
         {
@@ -1337,8 +1337,8 @@ public class ReplicatedEnvironmentFacade implements EnvironmentFacade, StateChan
         }
         catch (Exception e)
         {
-            throw new RuntimeException(String.format("Unexpected exception on attempt to retrieve state from '%s' at '%s'",
-                    helperNodeName, helperHostPort), e);
+            throw new RuntimeException(String.format("Cannot retrieve state for node '%s' (%s) from group '%s'",
+                    helperNodeName, helperHostPort, groupName), e);
         }
 
         if (LOGGER.isDebugEnabled())
diff --git a/qpid/java/bdbstore/src/main/java/org/apache/qpid/server/virtualhostnode/berkeleydb/BDBHAVirtualHostNodeImpl.java b/qpid/java/bdbstore/src/main/java/org/apache/qpid/server/virtualhostnode/berkeleydb/BDBHAVirtualHostNodeImpl.java
index 4888965eec..25890029f9 100644
--- a/qpid/java/bdbstore/src/main/java/org/apache/qpid/server/virtualhostnode/berkeleydb/BDBHAVirtualHostNodeImpl.java
+++ b/qpid/java/bdbstore/src/main/java/org/apache/qpid/server/virtualhostnode/berkeleydb/BDBHAVirtualHostNodeImpl.java
@@ -22,6 +22,8 @@ package org.apache.qpid.server.virtualhostnode.berkeleydb;
 
 import java.io.File;
 import java.net.InetSocketAddress;
+import java.net.URI;
+import java.net.URISyntaxException;
 import java.nio.file.Files;
 import java.security.PrivilegedAction;
 import java.text.MessageFormat;
@@ -371,9 +373,15 @@ public class BDBHAVirtualHostNodeImpl extends AbstractVirtualHostNode<BDBHAVirtu
     @StateTransition( currentState = { State.ACTIVE, State.STOPPED, State.ERRORED}, desiredState = State.DELETED )
     protected void doDelete()
     {
+        // get helpers before close. on close all children are closed and not available anymore
         Set<InetSocketAddress> helpers = getRemoteNodeAddresses();
         super.doDelete();
-        getEventLogger().message(getVirtualHostNodeLogSubject(), HighAvailabilityMessages.DELETED());
+
+        if (getConfigurationStore() != null)
+        {
+            getEventLogger().message(getVirtualHostNodeLogSubject(), HighAvailabilityMessages.DELETED());
+        }
+
         if (getState() == State.DELETED && !helpers.isEmpty())
         {
             try
@@ -487,40 +495,33 @@ public class BDBHAVirtualHostNodeImpl extends AbstractVirtualHostNode<BDBHAVirtu
     {
         String address = getAddress();
 
-        if (address == null || "".equals(address))
-        {
-            throw new IllegalConfigurationException("Node address is not set");
-        }
+        URI uri = addressToURI(address);
 
-        String[] tokens = address.split(":");
-        if (tokens.length != 2)
+        if (!PortUtil.isPortAvailable(uri.getHost(), uri.getPort()))
         {
-            throw new IllegalConfigurationException(String.format("Invalid address specified '%s'. ", address));
+            throw new IllegalConfigurationException(String.format("Cannot bind to address '%s'. Address is already in use.", address));
         }
+    }
 
-        String hostName = tokens[0];
-        if ("".equals(hostName.trim()))
+    private URI addressToURI(String address)
+    {
+        if (address == null || "".equals(address))
         {
-            throw new IllegalConfigurationException(String.format("Invalid address specified '%s'. ", address));
+            throw new IllegalConfigurationException("Node address is not set");
         }
 
-        int port = -1;
+        URI uri = null;
         try
         {
-            port = Integer.parseInt(tokens[1]);
+            uri = new URI( "tcp://" + address);
         }
-        catch(Exception e)
+        catch (URISyntaxException e)
         {
-            throw new IllegalConfigurationException(String.format("Invalid port is specified in address '%s'. ", address));
-        }
-        if (!PortUtil.isPortAvailable(hostName, port))
-        {
-            throw new IllegalConfigurationException(String.format("Cannot bind to address '%s'. Address is already in use.", address));
+            throw new IllegalConfigurationException(String.format("Invalid address specified '%s'. ", address));
         }
+        return uri;
     }
 
-
-
     private void onMaster()
     {
         try
@@ -801,7 +802,7 @@ public class BDBHAVirtualHostNodeImpl extends AbstractVirtualHostNode<BDBHAVirtu
 
     private boolean isFirstNodeInAGroup()
     {
-        return getAddress().equals(getHelperAddress());
+        return getHelperNodeName() == null;
     }
 
     BDBHAVirtualHostNodeLogSubject getVirtualHostNodeLogSubject()
@@ -876,23 +877,16 @@ public class BDBHAVirtualHostNodeImpl extends AbstractVirtualHostNode<BDBHAVirtu
         validatePermittedNodesFormat(proposedPermittedNodes);
     }
 
-    private void validatePermittedNodesFormat(Collection<String> proposedPermittedNodes)
+    private void validatePermittedNodesFormat(Collection<String> permittedNodes)
     {
-        for (String permittedNode: proposedPermittedNodes)
+        if (permittedNodes == null || permittedNodes.isEmpty())
         {
-            String[] tokens = permittedNode.split(":");
-            if (tokens.length != 2)
-            {
-                throw new IllegalArgumentException(String.format("Invalid permitted node specified '%s'. ", permittedNode));
-            }
-            try
-            {
-                Integer.parseInt(tokens[1]);
-            }
-            catch(Exception e)
-            {
-                throw new IllegalArgumentException(String.format("Invalid port is specified in permitted node '%s'. ", permittedNode));
-            }
+            throw new IllegalConfigurationException("Permitted nodes are not set");
+        }
+
+        for (String permittedNode: permittedNodes)
+        {
+            addressToURI(permittedNode);
         }
     }
 
diff --git a/qpid/java/bdbstore/src/test/java/org/apache/qpid/server/store/berkeleydb/BDBHAVirtualHostNodeTest.java b/qpid/java/bdbstore/src/test/java/org/apache/qpid/server/store/berkeleydb/BDBHAVirtualHostNodeTest.java
index 16486e3564..19365a6478 100644
--- a/qpid/java/bdbstore/src/test/java/org/apache/qpid/server/store/berkeleydb/BDBHAVirtualHostNodeTest.java
+++ b/qpid/java/bdbstore/src/test/java/org/apache/qpid/server/store/berkeleydb/BDBHAVirtualHostNodeTest.java
@@ -293,6 +293,7 @@ public class BDBHAVirtualHostNodeTest extends QpidTestCase
         _helper.awaitRemoteNodes(master, 2);
 
         BDBHAVirtualHostNode<?> replica = _helper.awaitAndFindNodeInRole(NodeRole.REPLICA);
+        _helper.awaitRemoteNodes(replica, 2);
 
         assertNotNull("Remote node " + replica.getName() + " is not found", _helper.findRemoteNode(master, replica.getName()));
         replica.delete();
@@ -607,7 +608,7 @@ public class BDBHAVirtualHostNodeTest extends QpidTestCase
         catch(IllegalConfigurationException e)
         {
             assertEquals("Unexpected exception on connection to non-existing helper address",
-                    String.format("Cannot connect to '%s'", "localhost:" + node2PortNumber), e.getMessage());
+                    String.format("Cannot connect to existing node '%s' at '%s'", "node2", "localhost:" + node2PortNumber), e.getMessage());
         }
     }
 
diff --git a/qpid/java/bdbstore/src/test/java/org/apache/qpid/server/virtualhostnode/berkeleydb/BDBHAVirtualHostNodeTestHelper.java b/qpid/java/bdbstore/src/test/java/org/apache/qpid/server/virtualhostnode/berkeleydb/BDBHAVirtualHostNodeTestHelper.java
index c12fecece3..0dc72acdb9 100644
--- a/qpid/java/bdbstore/src/test/java/org/apache/qpid/server/virtualhostnode/berkeleydb/BDBHAVirtualHostNodeTestHelper.java
+++ b/qpid/java/bdbstore/src/test/java/org/apache/qpid/server/virtualhostnode/berkeleydb/BDBHAVirtualHostNodeTestHelper.java
@@ -279,11 +279,14 @@ public class BDBHAVirtualHostNodeTestHelper
         node1Attributes.put(BDBHAVirtualHostNode.ADDRESS, address);
         node1Attributes.put(BDBHAVirtualHostNode.HELPER_ADDRESS, helperAddress);
         node1Attributes.put(BDBHAVirtualHostNode.STORE_PATH, getMessageStorePath() + File.separator + nodeName);
-        node1Attributes.put(BDBHAVirtualHostNode.HELPER_NODE_NAME, helperNodeNode);
         if (address.equals(helperAddress))
         {
             node1Attributes.put(BDBHAVirtualHostNode.PERMITTED_NODES, getPermittedNodes(ports));
         }
+        else
+        {
+            node1Attributes.put(BDBHAVirtualHostNode.HELPER_NODE_NAME, helperNodeNode);
+        }
 
         Map<String, String> context = new HashMap<String, String>();
         context.put(ReplicationConfig.REPLICA_ACK_TIMEOUT, "2 s");
diff --git a/qpid/java/bdbstore/systests/src/test/java/org/apache/qpid/server/store/berkeleydb/replication/BDBHAVirtualHostNodeRestTest.java b/qpid/java/bdbstore/systests/src/test/java/org/apache/qpid/server/store/berkeleydb/replication/BDBHAVirtualHostNodeRestTest.java
index c0c1b23a34..cfbd030474 100644
--- a/qpid/java/bdbstore/systests/src/test/java/org/apache/qpid/server/store/berkeleydb/replication/BDBHAVirtualHostNodeRestTest.java
+++ b/qpid/java/bdbstore/systests/src/test/java/org/apache/qpid/server/store/berkeleydb/replication/BDBHAVirtualHostNodeRestTest.java
@@ -326,7 +326,10 @@ public class BDBHAVirtualHostNodeRestTest extends QpidRestTestCase
         nodeData.put(BDBHAVirtualHostNode.GROUP_NAME, _hostName);
         nodeData.put(BDBHAVirtualHostNode.ADDRESS, "localhost:" + nodePort);
         nodeData.put(BDBHAVirtualHostNode.HELPER_ADDRESS, "localhost:" + helperPort);
-        nodeData.put(BDBHAVirtualHostNode.HELPER_NODE_NAME, NODE1);
+        if (nodePort != helperPort)
+        {
+            nodeData.put(BDBHAVirtualHostNode.HELPER_NODE_NAME, NODE1);
+        }
 
         Map<String,String> context = new HashMap<>();
         nodeData.put(BDBHAVirtualHostNode.CONTEXT, context);