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

2 files changed, 22 insertions, 27 deletions

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