QPID-6126: Invoke on-create validation after common validation, delete CO if creation fails on any stage

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

3 files changed, 39 insertions, 39 deletions

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 98b9cc3cf0..4888965eec 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
@@ -283,10 +283,20 @@ public class BDBHAVirtualHostNodeImpl extends AbstractVirtualHostNode<BDBHAVirtu
     public void onCreate()
     {
         super.onCreate();
-
+        if (!isFirstNodeInAGroup())
+        {
+            _permittedNodes = new ArrayList<>(getPermittedNodesFromHelper());
+        }
         getEventLogger().message(getVirtualHostNodeLogSubject(), HighAvailabilityMessages.CREATED());
     }
 
+    @Override
+    public void onOpen()
+    {
+        validatePermittedNodesFormat(_permittedNodes);
+        super.onOpen();
+    }
+
     protected ReplicatedEnvironmentFacade getReplicatedEnvironmentFacade()
     {
         return _environmentFacade.get();
@@ -421,13 +431,6 @@ public class BDBHAVirtualHostNodeImpl extends AbstractVirtualHostNode<BDBHAVirtu
     }
 
     @Override
-    public void onValidate()
-    {
-        super.onValidate();
-        validatePermittedNodes(_permittedNodes);
-    }
-
-    @Override
     protected void postResolve()
     {
         super.postResolve();
@@ -447,9 +450,7 @@ public class BDBHAVirtualHostNodeImpl extends AbstractVirtualHostNode<BDBHAVirtu
 
         if (!isFirstNodeInAGroup())
         {
-            // validate that helper address points to valid node
-            // we need _permittedNodes for the further validation in onValidate
-            _permittedNodes = new ArrayList<>(getPermittedNodesFromHelper());
+            getPermittedNodesFromHelper();
         }
     }
 
@@ -872,6 +873,11 @@ public class BDBHAVirtualHostNodeImpl extends AbstractVirtualHostNode<BDBHAVirtu
             }
         }
 
+        validatePermittedNodesFormat(proposedPermittedNodes);
+    }
+
+    private void validatePermittedNodesFormat(Collection<String> proposedPermittedNodes)
+    {
         for (String permittedNode: proposedPermittedNodes)
         {
             String[] tokens = permittedNode.split(":");
@@ -888,7 +894,6 @@ public class BDBHAVirtualHostNodeImpl extends AbstractVirtualHostNode<BDBHAVirtu
                 throw new IllegalArgumentException(String.format("Invalid port is specified in permitted node '%s'. ", permittedNode));
             }
         }
-
     }
 
     private class RemoteNodesDiscoverer implements ReplicationGroupListener
diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/AbstractConfiguredObject.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/AbstractConfiguredObject.java
index 9ff7e224cd..77810a4d69 100644
--- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/AbstractConfiguredObject.java
+++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/AbstractConfiguredObject.java
@@ -502,8 +502,8 @@ public abstract class AbstractConfiguredObject<X extends ConfiguredObject<X>> im
             try
             {
                 doResolution(true, createExceptionHandler);
-                validateOnCreate();
                 doValidation(true, createExceptionHandler);
+                validateOnCreate();
                 registerWithParents();
             }
             catch(RuntimeException e)
@@ -515,22 +515,13 @@ public abstract class AbstractConfiguredObject<X extends ConfiguredObject<X>> im
             try
             {
                 doCreation(true, unregisteringExceptionHandler);
+                doOpening(true, unregisteringExceptionHandler);
+                doAttainState(unregisteringExceptionHandler);
             }
             catch(RuntimeException e)
             {
                 unregisteringExceptionHandler.handleException(e, this);
             }
-
-            OpenExceptionHandler openExceptionHandler = new OpenExceptionHandler();
-            try
-            {
-                doOpening(true, openExceptionHandler);
-                doAttainState(openExceptionHandler);
-            }
-            catch(RuntimeException e)
-            {
-                openExceptionHandler.handleException(e, this);
-            }
         }
     }
 
@@ -1914,20 +1905,20 @@ public abstract class AbstractConfiguredObject<X extends ConfiguredObject<X>> im
 
         public void handleException(RuntimeException exception, AbstractConfiguredObject<?> source)
         {
-            if (source.getState() != State.DELETED)
+            try
             {
-                try
+                if (source.getState() != State.DELETED)
                 {
                     source.delete();
                 }
-                finally
+            }
+            finally
+            {
+                if (_unregister)
                 {
-                    if (_unregister)
-                    {
-                        source.unregister(false);
-                    }
-                    throw exception;
+                    source.unregister(false);
                 }
+                throw exception;
             }
         }
     }
diff --git a/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/AbstractConfiguredObjectTest.java b/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/AbstractConfiguredObjectTest.java
index a09910b7e2..4c6f0710c4 100644
--- a/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/AbstractConfiguredObjectTest.java
+++ b/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/AbstractConfiguredObjectTest.java
@@ -357,14 +357,18 @@ public class AbstractConfiguredObjectTest extends TestCase
     {
         TestConfiguredObject object = new TestConfiguredObject(getName());
         object.setThrowExceptionOnOpen(true);
-        object.create();
-        assertFalse("Unexpected opened", object.isOpened());
-        assertEquals("Unexpected state", State.ERRORED, object.getState());
+        try
+        {
+            object.create();
+            fail("Exception should have been re-thrown");
+        }
+        catch (RuntimeException re)
+        {
+            // pass
+        }
 
-        object.setThrowExceptionOnOpen(false);
-        object.start();
-        assertTrue("Unexpected opened", object.isOpened());
-        assertEquals("Unexpected state", State.ACTIVE, object.getState());
+        assertFalse("Unexpected opened", object.isOpened());
+        assertEquals("Unexpected state", State.DELETED, object.getState());
     }
 
     public void testCreationWithExceptionThrownFromOnCreate() throws Exception