You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@qpid.apache.org by kw...@apache.org on 2015/07/10 10:15:28 UTC
svn commit: r1690221 - in /qpid/java/trunk/broker-core/src:
main/java/org/apache/qpid/server/model/
test/java/org/apache/qpid/server/model/testmodels/lifecycle/
Author: kwall
Date: Fri Jul 10 08:15:27 2015
New Revision: 1690221
URL: http://svn.apache.org/r1690221
Log:
QPID-6634: [Java Broker] Prevent spurious state change event if state change operation changes
* Removed duplicated state change event
* Added supported unit test
Work done by Lorenz Quack <qu...@gmail.com> and Keith Wall.
Modified:
qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/model/AbstractConfiguredObject.java
qpid/java/trunk/broker-core/src/test/java/org/apache/qpid/server/model/testmodels/lifecycle/AbstractConfiguredObjectTest.java
qpid/java/trunk/broker-core/src/test/java/org/apache/qpid/server/model/testmodels/lifecycle/TestConfiguredObject.java
Modified: qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/model/AbstractConfiguredObject.java
URL: http://svn.apache.org/viewvc/qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/model/AbstractConfiguredObject.java?rev=1690221&r1=1690220&r2=1690221&view=diff
==============================================================================
--- qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/model/AbstractConfiguredObject.java (original)
+++ qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/model/AbstractConfiguredObject.java Fri Jul 10 08:15:27 2015
@@ -1170,7 +1170,7 @@ public abstract class AbstractConfigured
protected ListenableFuture<Void> attainState()
{
- State currentState = getState();
+ final State currentState = getState();
State desiredState = getDesiredState();
ListenableFuture<Void> returnVal;
@@ -1193,13 +1193,12 @@ public abstract class AbstractConfigured
public void run()
{
_attainStateFuture.set(AbstractConfiguredObject.this);
+ if(getState() != currentState)
+ {
+ notifyStateChanged(currentState, getState());
+ }
}
});
- if(getState() != currentState)
- {
- // TODO - KW - shouldn't I be done after too???
- notifyStateChanged(currentState, getState());
- }
}
catch (IllegalAccessException e)
{
@@ -1347,30 +1346,15 @@ public abstract class AbstractConfigured
attributeSet(ConfiguredObject.DESIRED_STATE,
currentDesiredState,
desiredState);
-
- return doAfter(attainStateIfOpenedOrReopenFailed(),new Runnable()
- {
- @Override
- public void run()
- {
- if (getState() == desiredState)
- {
- notifyStateChanged(state, desiredState);
- }
-
- }
- }
- );
+ return attainStateIfOpenedOrReopenFailed();
}
else
{
return Futures.immediateFuture(null);
}
}
-
}
});
-
}
@Override
Modified: qpid/java/trunk/broker-core/src/test/java/org/apache/qpid/server/model/testmodels/lifecycle/AbstractConfiguredObjectTest.java
URL: http://svn.apache.org/viewvc/qpid/java/trunk/broker-core/src/test/java/org/apache/qpid/server/model/testmodels/lifecycle/AbstractConfiguredObjectTest.java?rev=1690221&r1=1690220&r2=1690221&view=diff
==============================================================================
--- qpid/java/trunk/broker-core/src/test/java/org/apache/qpid/server/model/testmodels/lifecycle/AbstractConfiguredObjectTest.java (original)
+++ qpid/java/trunk/broker-core/src/test/java/org/apache/qpid/server/model/testmodels/lifecycle/AbstractConfiguredObjectTest.java Fri Jul 10 08:15:27 2015
@@ -19,8 +19,15 @@
package org.apache.qpid.server.model.testmodels.lifecycle;
import java.util.Collections;
+import java.util.concurrent.atomic.AtomicInteger;
+import java.util.concurrent.atomic.AtomicReference;
+
+import com.google.common.util.concurrent.ListenableFuture;
import org.apache.qpid.server.configuration.IllegalConfigurationException;
+import org.apache.qpid.server.model.ConfigurationChangeListener;
+import org.apache.qpid.server.model.ConfiguredObject;
+import org.apache.qpid.server.model.IllegalStateTransitionException;
import org.apache.qpid.server.model.Port;
import org.apache.qpid.server.model.State;
import org.apache.qpid.test.utils.QpidTestCase;
@@ -214,4 +221,105 @@ public class AbstractConfiguredObjectTes
assertEquals("Unexpected child1 state", State.ERRORED, child1.getState());
}
+ public void testSuccessfulStateTransitionInvokesListener() throws Exception
+ {
+ TestConfiguredObject parent = new TestConfiguredObject("parent");
+ parent.create();
+
+ final AtomicReference<State> newState = new AtomicReference<>();
+ final AtomicInteger callCounter = new AtomicInteger();
+ parent.addChangeListener(new NoopChangeListener()
+ {
+ @Override
+ public void stateChanged(final ConfiguredObject<?> object, final State old, final State state)
+ {
+ super.stateChanged(object, old, state);
+ callCounter.incrementAndGet();
+ newState.set(state);
+ }
+ });
+
+ parent.delete();
+ assertEquals(State.DELETED, newState.get());
+ assertEquals(1, callCounter.get());
+ }
+
+ public void testUnsuccessfulStateTransitionDoesnotInvokesListener() throws Exception
+ {
+ final IllegalStateTransitionException expectedException =
+ new IllegalStateTransitionException("This test fails the state transition.");
+ TestConfiguredObject parent = new TestConfiguredObject("parent")
+ {
+ @Override
+ protected ListenableFuture<Void> doDelete()
+ {
+ throw expectedException;
+ }
+ };
+ parent.create();
+
+ final AtomicInteger callCounter = new AtomicInteger();
+ parent.addChangeListener(new NoopChangeListener()
+ {
+ @Override
+ public void stateChanged(final ConfiguredObject<?> object, final State old, final State state)
+ {
+ super.stateChanged(object, old, state);
+ callCounter.incrementAndGet();
+ }
+ });
+
+ try
+ {
+ parent.delete();
+ fail("Exception not thrown.");
+ }
+ catch (RuntimeException e)
+ {
+ assertSame("State transition threw unexpected exception.", expectedException, e);
+ }
+ assertEquals(0, callCounter.get());
+ }
+
+ private static class NoopChangeListener implements ConfigurationChangeListener
+ {
+ @Override
+ public void stateChanged(final ConfiguredObject<?> object, final State oldState, final State newState)
+ {
+
+ }
+
+ @Override
+ public void childAdded(final ConfiguredObject<?> object, final ConfiguredObject<?> child)
+ {
+
+ }
+
+ @Override
+ public void childRemoved(final ConfiguredObject<?> object, final ConfiguredObject<?> child)
+ {
+
+ }
+
+ @Override
+ public void attributeSet(final ConfiguredObject<?> object,
+ final String attributeName,
+ final Object oldAttributeValue,
+ final Object newAttributeValue)
+ {
+
+ }
+
+ @Override
+ public void bulkChangeStart(final ConfiguredObject<?> object)
+ {
+
+ }
+
+ @Override
+ public void bulkChangeEnd(final ConfiguredObject<?> object)
+ {
+
+ }
+ }
}
Modified: qpid/java/trunk/broker-core/src/test/java/org/apache/qpid/server/model/testmodels/lifecycle/TestConfiguredObject.java
URL: http://svn.apache.org/viewvc/qpid/java/trunk/broker-core/src/test/java/org/apache/qpid/server/model/testmodels/lifecycle/TestConfiguredObject.java?rev=1690221&r1=1690220&r2=1690221&view=diff
==============================================================================
--- qpid/java/trunk/broker-core/src/test/java/org/apache/qpid/server/model/testmodels/lifecycle/TestConfiguredObject.java (original)
+++ qpid/java/trunk/broker-core/src/test/java/org/apache/qpid/server/model/testmodels/lifecycle/TestConfiguredObject.java Fri Jul 10 08:15:27 2015
@@ -146,13 +146,13 @@ public class TestConfiguredObject extend
return Futures.immediateFuture(null);
}
- @StateTransition( currentState = {State.ERRORED, State.UNINITIALIZED}, desiredState = State.DELETED )
+ @StateTransition( currentState = {State.ERRORED, State.UNINITIALIZED, State.ACTIVE}, desiredState = State.DELETED )
protected ListenableFuture<Void> doDelete()
{
setState(State.DELETED);
return Futures.immediateFuture(null);
}
-
+
public boolean isOpened()
{
return _opened;
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org