You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@qpid.apache.org by or...@apache.org on 2014/10/03 17:23:50 UTC
svn commit: r1629226 - in /qpid/trunk/qpid/java/broker-core/src:
main/java/org/apache/qpid/server/model/
test/java/org/apache/qpid/server/model/
test/java/org/apache/qpid/server/model/testmodel/
Author: orudyy
Date: Fri Oct 3 15:23:49 2014
New Revision: 1629226
URL: http://svn.apache.org/r1629226
Log:
QPID-6126: Normalize exception handling on open and prevent ERRORED children from being validated or opened
Modified:
qpid/trunk/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/AbstractConfiguredObject.java
qpid/trunk/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/AbstractConfiguredObjectTest.java
qpid/trunk/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/testmodel/TestConfiguredObject.java
Modified: qpid/trunk/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/AbstractConfiguredObject.java
URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/AbstractConfiguredObject.java?rev=1629226&r1=1629225&r2=1629226&view=diff
==============================================================================
--- qpid/trunk/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/AbstractConfiguredObject.java (original)
+++ qpid/trunk/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/AbstractConfiguredObject.java Fri Oct 3 15:23:49 2014
@@ -529,31 +529,32 @@ public abstract class AbstractConfigured
{
}
- protected final void handleExceptionOnOpen(RuntimeException e, AbstractConfiguredObject<?> configuredObject)
+ protected final void handleExceptionOnOpen(RuntimeException e)
{
if (e instanceof ServerScopedRuntimeException)
{
throw e;
}
- LOGGER.error("Exception occurred on open for " + configuredObject.getName(), e);
+ LOGGER.error("Exception occurred on open for " + getName(), e);
try
{
- configuredObject.onExceptionInOpen(e);
+ onExceptionInOpen(e);
}
catch (RuntimeException re)
{
- LOGGER.error("Unexpected exception while handling exception on open for " + configuredObject.getName(), e);
+ LOGGER.error("Unexpected exception while handling exception on open for " + getName(), e);
}
- if (!configuredObject._openComplete)
+ if (!_openComplete)
{
- configuredObject._openFailed = true;
- configuredObject._dynamicState.compareAndSet(DynamicState.OPENED, DynamicState.UNINIT);
+ _openFailed = true;
+ _dynamicState.compareAndSet(DynamicState.OPENED, DynamicState.UNINIT);
}
- configuredObject.closeChildren();
- configuredObject.setState(State.ERRORED);
+
+ //TODO: children of ERRORED CO will continue to remain in ACTIVE state
+ setState(State.ERRORED);
}
/**
@@ -604,7 +605,7 @@ public abstract class AbstractConfigured
@Override
public void performAction(final ConfiguredObject<?> child)
{
- if (child instanceof AbstractConfiguredObject)
+ if (child.getState() != State.ERRORED && child instanceof AbstractConfiguredObject)
{
AbstractConfiguredObject configuredObject = (AbstractConfiguredObject) child;
try
@@ -631,7 +632,7 @@ public abstract class AbstractConfigured
@Override
public void performAction(final ConfiguredObject<?> child)
{
- if (child instanceof AbstractConfiguredObject)
+ if (child.getState() != State.ERRORED && child instanceof AbstractConfiguredObject)
{
AbstractConfiguredObject configuredObject = (AbstractConfiguredObject) child;
try
@@ -1878,18 +1879,18 @@ public abstract class AbstractConfigured
void handleException(RuntimeException exception, AbstractConfiguredObject<?> source);
}
- private class OpenExceptionHandler implements AbstractConfiguredObjectExceptionHandler
+ private static class OpenExceptionHandler implements AbstractConfiguredObjectExceptionHandler
{
@Override
public void handleException(RuntimeException exception, AbstractConfiguredObject<?> source)
{
- handleExceptionOnOpen(exception, source);
+ source.handleExceptionOnOpen(exception);
}
}
- private class CreateExceptionHandler implements AbstractConfiguredObjectExceptionHandler
+ private static class CreateExceptionHandler implements AbstractConfiguredObjectExceptionHandler
{
- private boolean _unregister;
+ private final boolean _unregister;
private CreateExceptionHandler()
{
@@ -1898,7 +1899,7 @@ public abstract class AbstractConfigured
private CreateExceptionHandler(boolean unregister)
{
- this._unregister = unregister;
+ _unregister = unregister;
}
@Override
Modified: qpid/trunk/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/AbstractConfiguredObjectTest.java
URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/AbstractConfiguredObjectTest.java?rev=1629226&r1=1629225&r2=1629226&view=diff
==============================================================================
--- qpid/trunk/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/AbstractConfiguredObjectTest.java (original)
+++ qpid/trunk/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/AbstractConfiguredObjectTest.java Fri Oct 3 15:23:49 2014
@@ -388,4 +388,60 @@ public class AbstractConfiguredObjectTes
assertFalse("Unexpected opened", object.isOpened());
assertEquals("Unexpected state", State.DELETED, object.getState());
}
+
+ public void testUnresolvedChildInERROREDStateIsNotValidatedOrOpenedOrAttainedDesiredStateOnParentOpen() throws Exception
+ {
+ TestConfiguredObject parent = new TestConfiguredObject("parent");
+ TestConfiguredObject child1 = new TestConfiguredObject("child1", parent, parent.getTaskExecutor());
+ child1.registerWithParents();
+ TestConfiguredObject child2 = new TestConfiguredObject("child2", parent, parent.getTaskExecutor());
+ child2.registerWithParents();
+
+ child1.setThrowExceptionOnPostResolve(true);
+
+ parent.open();
+
+ assertTrue("Parent should be resolved", parent.isResolved());
+ assertTrue("Parent should be validated", parent.isValidated());
+ assertTrue("Parent should be opened", parent.isOpened());
+ assertEquals("Unexpected parent state", State.ACTIVE, parent.getState());
+
+ assertTrue("Child2 should be resolved", child2.isResolved());
+ assertTrue("Child2 should be validated", child2.isValidated());
+ assertTrue("Child2 should be opened", child2.isOpened());
+ assertEquals("Unexpected child2 state", State.ACTIVE, child2.getState());
+
+ assertFalse("Child2 should not be resolved", child1.isResolved());
+ assertFalse("Child1 should not be validated", child1.isValidated());
+ assertFalse("Child1 should not be opened", child1.isOpened());
+ assertEquals("Unexpected child1 state", State.ERRORED, child1.getState());
+ }
+
+ public void testUnvalidatedChildInERROREDStateIsNotOpenedOrAttainedDesiredStateOnParentOpen() throws Exception
+ {
+ TestConfiguredObject parent = new TestConfiguredObject("parent");
+ TestConfiguredObject child1 = new TestConfiguredObject("child1", parent, parent.getTaskExecutor());
+ child1.registerWithParents();
+ TestConfiguredObject child2 = new TestConfiguredObject("child2", parent, parent.getTaskExecutor());
+ child2.registerWithParents();
+
+ child1.setThrowExceptionOnValidate(true);
+
+ parent.open();
+
+ assertTrue("Parent should be resolved", parent.isResolved());
+ assertTrue("Parent should be validated", parent.isValidated());
+ assertTrue("Parent should be opened", parent.isOpened());
+ assertEquals("Unexpected parent state", State.ACTIVE, parent.getState());
+
+ assertTrue("Child2 should be resolved", child2.isResolved());
+ assertTrue("Child2 should be validated", child2.isValidated());
+ assertTrue("Child2 should be opened", child2.isOpened());
+ assertEquals("Unexpected child2 state", State.ACTIVE, child2.getState());
+
+ assertTrue("Child1 should be resolved", child1.isResolved());
+ assertFalse("Child1 should not be validated", child1.isValidated());
+ assertFalse("Child1 should not be opened", child1.isOpened());
+ assertEquals("Unexpected child1 state", State.ERRORED, child1.getState());
+ }
}
Modified: qpid/trunk/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/testmodel/TestConfiguredObject.java
URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/testmodel/TestConfiguredObject.java?rev=1629226&r1=1629225&r2=1629226&view=diff
==============================================================================
--- qpid/trunk/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/testmodel/TestConfiguredObject.java (original)
+++ qpid/trunk/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/testmodel/TestConfiguredObject.java Fri Oct 3 15:23:49 2014
@@ -2,6 +2,8 @@ package org.apache.qpid.server.model.tes
import static org.mockito.Mockito.mock;
+import java.util.Arrays;
+import java.util.Collection;
import java.util.Collections;
import java.util.Map;
@@ -10,35 +12,40 @@ import org.apache.qpid.server.configurat
import org.apache.qpid.server.configuration.updater.TaskExecutor;
import org.apache.qpid.server.model.AbstractConfiguredObject;
import org.apache.qpid.server.model.ConfiguredObject;
+import org.apache.qpid.server.model.ConfiguredObjectFactory;
+import org.apache.qpid.server.model.ConfiguredObjectFactoryImpl;
+import org.apache.qpid.server.model.ConfiguredObjectTypeRegistry;
import org.apache.qpid.server.model.ManagedObject;
import org.apache.qpid.server.model.Model;
import org.apache.qpid.server.model.State;
import org.apache.qpid.server.model.StateTransition;
+import org.apache.qpid.server.plugin.ConfiguredObjectRegistration;
@ManagedObject
public class TestConfiguredObject extends AbstractConfiguredObject
{
- private boolean _throwExceptionOnOpen;
private boolean _opened;
+ private boolean _validated;
+ private boolean _resolved;
+ private boolean _throwExceptionOnOpen;
private boolean _throwExceptionOnValidationOnCreate;
private boolean _throwExceptionOnPostResolve;
private boolean _throwExceptionOnCreate;
+ private boolean _throwExceptionOnValidate;
- public TestConfiguredObject(String name)
+ public final static Map<Class<? extends ConfiguredObject>, ConfiguredObject<?>> createParents(ConfiguredObject<?> parent)
{
- this(createParents(), Collections.<String, Object>singletonMap(ConfiguredObject.NAME, name), createTaskExecutor(), new TestModel(null));
+ return Collections.<Class<? extends ConfiguredObject>, ConfiguredObject<?>>singletonMap(parent.getCategoryClass(), parent);
}
- public final static Map<Class<? extends ConfiguredObject>, ConfiguredObject<?>> createParents()
+ public TestConfiguredObject(String name)
{
- return Collections.<Class<? extends ConfiguredObject>, ConfiguredObject<?>>singletonMap(null, mock(ConfiguredObject.class));
+ this(name, mock(ConfiguredObject.class), CurrentThreadTaskExecutor.newStartedInstance());
}
- public final static TaskExecutor createTaskExecutor()
+ public TestConfiguredObject(String name, ConfiguredObject<?> parent, TaskExecutor taskExecutor)
{
- TaskExecutor taskExecutor = new CurrentThreadTaskExecutor();
- taskExecutor.start();
- return taskExecutor;
+ this(createParents(parent), Collections.<String, Object>singletonMap(ConfiguredObject.NAME, name), taskExecutor, TestConfiguredObjectModel.INSTANCE);
}
public TestConfiguredObject(Map parents, Map<String, Object> attributes, TaskExecutor taskExecutor, Model model)
@@ -54,6 +61,7 @@ public class TestConfiguredObject extend
{
throw new IllegalConfigurationException("Cannot resolve");
}
+ _resolved = true;
}
@Override
@@ -84,6 +92,16 @@ public class TestConfiguredObject extend
}
}
+ @Override
+ public void onValidate()
+ {
+ if (_throwExceptionOnValidate)
+ {
+ throw new IllegalConfigurationException("Cannot validate");
+ }
+ _validated = true;
+ }
+
@StateTransition( currentState = {State.ERRORED, State.UNINITIALIZED}, desiredState = State.ACTIVE )
protected void activate()
{
@@ -120,4 +138,101 @@ public class TestConfiguredObject extend
{
_throwExceptionOnCreate = throwExceptionOnCreate;
}
+
+ public void setThrowExceptionOnValidate(boolean throwException)
+ {
+ _throwExceptionOnValidate= throwException;
+ }
+
+ public boolean isValidated()
+ {
+ return _validated;
+ }
+
+ public boolean isResolved()
+ {
+ return _resolved;
+ }
+
+ public static class TestConfiguredObjectModel extends Model
+ {
+
+ private Collection<Class<? extends ConfiguredObject>> CATEGORIES = Collections.<Class<? extends ConfiguredObject>>singleton(TestConfiguredObject.class);
+ private ConfiguredObjectFactoryImpl _configuredObjectFactory;
+
+ private static TestConfiguredObjectModel INSTANCE = new TestConfiguredObjectModel();
+ private ConfiguredObjectTypeRegistry _configuredObjectTypeRegistry;
+
+ private TestConfiguredObjectModel()
+ {
+ _configuredObjectFactory = new ConfiguredObjectFactoryImpl(this);
+ ConfiguredObjectRegistration configuredObjectRegistration = new ConfiguredObjectRegistration()
+ {
+ @Override
+ public Collection<Class<? extends ConfiguredObject>> getConfiguredObjectClasses()
+ {
+ return CATEGORIES;
+ }
+
+ @Override
+ public String getType()
+ {
+ return TestConfiguredObjectModel.class.getSimpleName();
+ }
+ };
+ _configuredObjectTypeRegistry = new ConfiguredObjectTypeRegistry(Arrays.asList(configuredObjectRegistration), CATEGORIES);
+ }
+
+ @Override
+ public Collection<Class<? extends ConfiguredObject>> getSupportedCategories()
+ {
+ return CATEGORIES;
+ }
+
+ @Override
+ public Collection<Class<? extends ConfiguredObject>> getChildTypes(Class<? extends ConfiguredObject> parent)
+ {
+ return TestConfiguredObject.class.isAssignableFrom(parent)
+ ? CATEGORIES
+ : Collections.<Class<? extends ConfiguredObject>>emptySet();
+ }
+
+ @Override
+ public Class<? extends ConfiguredObject> getRootCategory()
+ {
+ return TestConfiguredObject.class;
+ }
+
+ @Override
+ public Collection<Class<? extends ConfiguredObject>> getParentTypes(final Class<? extends ConfiguredObject> child)
+ {
+ return TestConfiguredObject.class.isAssignableFrom(child)
+ ? CATEGORIES
+ : Collections.<Class<? extends ConfiguredObject>>emptySet();
+ }
+
+ @Override
+ public int getMajorVersion()
+ {
+ return 99;
+ }
+
+ @Override
+ public int getMinorVersion()
+ {
+ return 99;
+ }
+
+ @Override
+ public ConfiguredObjectFactory getObjectFactory()
+ {
+ return _configuredObjectFactory;
+ }
+
+ @Override
+ public ConfiguredObjectTypeRegistry getTypeRegistry()
+ {
+ return _configuredObjectTypeRegistry;
+ }
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org