You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@qpid.apache.org by ro...@apache.org on 2018/10/30 14:57:32 UTC
qpid-proton-j git commit: PROTON-1938: align TransportImpl
set/getCondition behaviour,
plus stop the closed method replacing an existing ErrorCondition
Repository: qpid-proton-j
Updated Branches:
refs/heads/master 1a5e19369 -> 19bf22f7e
PROTON-1938: align TransportImpl set/getCondition behaviour, plus stop the closed method replacing an existing ErrorCondition
Project: http://git-wip-us.apache.org/repos/asf/qpid-proton-j/repo
Commit: http://git-wip-us.apache.org/repos/asf/qpid-proton-j/commit/19bf22f7
Tree: http://git-wip-us.apache.org/repos/asf/qpid-proton-j/tree/19bf22f7
Diff: http://git-wip-us.apache.org/repos/asf/qpid-proton-j/diff/19bf22f7
Branch: refs/heads/master
Commit: 19bf22f7e9b88e73db4b195f674d1527dbd713f3
Parents: 1a5e193
Author: Robbie Gemmell <ro...@apache.org>
Authored: Tue Oct 30 14:47:44 2018 +0000
Committer: Robbie Gemmell <ro...@apache.org>
Committed: Tue Oct 30 14:47:44 2018 +0000
----------------------------------------------------------------------
.../qpid/proton/engine/impl/TransportImpl.java | 39 ++++---
.../proton/engine/impl/TransportImplTest.java | 107 +++++++++++++++++++
2 files changed, 133 insertions(+), 13 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/qpid-proton-j/blob/19bf22f7/proton-j/src/main/java/org/apache/qpid/proton/engine/impl/TransportImpl.java
----------------------------------------------------------------------
diff --git a/proton-j/src/main/java/org/apache/qpid/proton/engine/impl/TransportImpl.java b/proton-j/src/main/java/org/apache/qpid/proton/engine/impl/TransportImpl.java
index 8947fa3..e07edbb 100644
--- a/proton-j/src/main/java/org/apache/qpid/proton/engine/impl/TransportImpl.java
+++ b/proton-j/src/main/java/org/apache/qpid/proton/engine/impl/TransportImpl.java
@@ -125,7 +125,7 @@ public class TransportImpl extends EndpointImpl
private FrameHandler _frameHandler = this;
private boolean _head_closed = false;
- private ErrorCondition _condition = null;
+ private boolean _conditionSet;
private boolean postedHeadClosed = false;
private boolean postedTailClosed = false;
@@ -250,7 +250,19 @@ public class TransportImpl extends EndpointImpl
@Override
public ErrorCondition getCondition()
{
- return _condition;
+ // Get the ErrorCondition, but only return it if its condition field is populated.
+ // This somewhat retains prior TransportImpl behaviour of returning null when no
+ // condition had been set (by TransportImpl itself) rather than the 'empty' ErrorCondition
+ // object historically used in the other areas.
+ ErrorCondition errorCondition = super.getCondition();
+ return errorCondition.getCondition() != null ? errorCondition : null;
+ }
+
+ @Override
+ public void setCondition(ErrorCondition error)
+ {
+ super.setCondition(error);
+ _conditionSet = error != null && error.getCondition() != null;
}
@Override
@@ -844,7 +856,7 @@ public class TransportImpl extends EndpointImpl
private void processOpen()
{
- if (!_isOpenSent && (_condition != null ||
+ if (!_isOpenSent && (_conditionSet ||
(_connectionEndpoint != null &&
_connectionEndpoint.getLocalState() != EndpointState.UNINITIALIZED)))
{
@@ -1042,7 +1054,7 @@ public class TransportImpl extends EndpointImpl
private void processClose()
{
- if ((_condition != null ||
+ if ((_conditionSet ||
(_connectionEndpoint != null &&
_connectionEndpoint.getLocalState() == EndpointState.CLOSED)) &&
!_isCloseSent) {
@@ -1053,12 +1065,12 @@ public class TransportImpl extends EndpointImpl
ErrorCondition localError;
if (_connectionEndpoint == null) {
- localError = _condition;
+ localError = getCondition();
} else {
localError = _connectionEndpoint.getCondition();
}
- if(localError.getCondition() != null)
+ if(localError != null && localError.getCondition() != null)
{
close.setError(localError);
}
@@ -1419,19 +1431,20 @@ public class TransportImpl extends EndpointImpl
public void closed(TransportException error)
{
if (!_closeReceived || error != null) {
- if (error == null) {
- _condition = new ErrorCondition(ConnectionError.FRAMING_ERROR,
- "connection aborted");
- } else {
- _condition = new ErrorCondition(ConnectionError.FRAMING_ERROR,
- error.toString());
+ // Set an error condition, but only if one was not already set
+ if(!_conditionSet) {
+ String description = error == null ? "connection aborted" : error.toString();
+ setCondition(new ErrorCondition(ConnectionError.FRAMING_ERROR, description));
}
+
_head_closed = true;
}
- if (_condition != null && !postedTransportError) {
+
+ if (_conditionSet && !postedTransportError) {
put(Event.Type.TRANSPORT_ERROR, this);
postedTransportError = true;
}
+
if (!postedTailClosed) {
put(Event.Type.TRANSPORT_TAIL_CLOSED, this);
postedTailClosed = true;
http://git-wip-us.apache.org/repos/asf/qpid-proton-j/blob/19bf22f7/proton-j/src/test/java/org/apache/qpid/proton/engine/impl/TransportImplTest.java
----------------------------------------------------------------------
diff --git a/proton-j/src/test/java/org/apache/qpid/proton/engine/impl/TransportImplTest.java b/proton-j/src/test/java/org/apache/qpid/proton/engine/impl/TransportImplTest.java
index fbc9b8f..203274f 100644
--- a/proton-j/src/test/java/org/apache/qpid/proton/engine/impl/TransportImplTest.java
+++ b/proton-j/src/test/java/org/apache/qpid/proton/engine/impl/TransportImplTest.java
@@ -24,6 +24,7 @@ import static org.apache.qpid.proton.engine.impl.TransportTestHelper.stringOfLen
import static org.junit.Assert.assertArrayEquals;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
@@ -38,6 +39,7 @@ import java.util.Random;
import org.apache.qpid.proton.Proton;
import org.apache.qpid.proton.amqp.Binary;
+import org.apache.qpid.proton.amqp.Symbol;
import org.apache.qpid.proton.amqp.UnsignedInteger;
import org.apache.qpid.proton.amqp.UnsignedShort;
import org.apache.qpid.proton.amqp.messaging.Accepted;
@@ -46,9 +48,11 @@ import org.apache.qpid.proton.amqp.messaging.Released;
import org.apache.qpid.proton.amqp.transport.Attach;
import org.apache.qpid.proton.amqp.transport.Begin;
import org.apache.qpid.proton.amqp.transport.Close;
+import org.apache.qpid.proton.amqp.transport.ConnectionError;
import org.apache.qpid.proton.amqp.transport.Detach;
import org.apache.qpid.proton.amqp.transport.Disposition;
import org.apache.qpid.proton.amqp.transport.End;
+import org.apache.qpid.proton.amqp.transport.ErrorCondition;
import org.apache.qpid.proton.amqp.transport.Flow;
import org.apache.qpid.proton.amqp.transport.FrameBody;
import org.apache.qpid.proton.amqp.transport.Open;
@@ -3530,4 +3534,107 @@ public class TransportImplTest
assertEquals("Unexpected delivery count", UnsignedInteger.valueOf(3), sentFlow.getDeliveryCount());
assertEquals("Unexpected credit", UnsignedInteger.valueOf(123), sentFlow.getLinkCredit());
}
+
+ @Test
+ public void testErrorConditionDefault() {
+ TransportImpl transport = new TransportImpl();
+ assertNull("Expected null ErrorCondition given historic behaviour", transport.getCondition());
+ }
+
+ @Test
+ public void testErrorConditionSetGet() {
+ // Try setting with an empty condition object, expect to get a null back per historic behaviour.
+ TransportImpl transport = new TransportImpl();
+
+ ErrorCondition emptyErrorCondition = new ErrorCondition();
+ assertNull("Expected empty Condition given historic behaviour", emptyErrorCondition.getCondition());
+ transport.setCondition(emptyErrorCondition);
+ assertNull("Expected null ErrorCondition given historic behaviour", transport.getCondition());
+
+ // Try setting with a populated condition object.
+ transport = new TransportImpl();
+
+ Symbol condition = Symbol.getSymbol("some-error");
+ String description = "some-error-description";
+ ErrorCondition populatedErrorCondition = new ErrorCondition();
+ populatedErrorCondition.setCondition(condition);
+ populatedErrorCondition.setDescription(description);
+ assertNotNull("Expected a Condition", populatedErrorCondition.getCondition());
+
+ transport.setCondition(populatedErrorCondition);
+ assertNotNull("Expected an ErrorCondition to be returned", transport.getCondition());
+ assertEquals("Unexpected ErrorCondition returned", populatedErrorCondition, transport.getCondition());
+
+ // Try setting again with another populated condition object.
+ Symbol otherCondition = Symbol.getSymbol("some-other-error");
+ String otherDescription = "some-other-error-description";
+ ErrorCondition otherErrorCondition = new ErrorCondition();
+ otherErrorCondition.setCondition(otherCondition);
+ otherErrorCondition.setDescription(otherDescription);
+ assertNotNull("Expected a Condition", otherErrorCondition.getCondition());
+
+ assertNotEquals(condition, otherCondition);
+ assertNotEquals(populatedErrorCondition.getCondition(), otherErrorCondition.getCondition());
+ assertNotEquals(description, otherDescription);
+ assertNotEquals(populatedErrorCondition.getDescription(), otherErrorCondition.getDescription());
+ assertNotEquals(populatedErrorCondition, otherErrorCondition);
+
+ transport.setCondition(otherErrorCondition);
+ assertNotNull("Expected an ErrorCondition to be returned", transport.getCondition());
+ assertEquals("Unexpected ErrorCondition returned", otherErrorCondition, transport.getCondition());
+
+ // Try setting again with an empty condition object, expect to get a null back per historic behaviour.
+ transport.setCondition(emptyErrorCondition);
+ assertNull("Expected null ErrorCondition given historic behaviour", transport.getCondition());
+ }
+
+ @Test
+ public void testErrorConditionAfterTransportClosed() {
+ Symbol condition = Symbol.getSymbol("some-error");
+ String description = "some-error-description";
+ ErrorCondition origErrorCondition = new ErrorCondition();
+ origErrorCondition.setCondition(condition);
+ origErrorCondition.setDescription(description);
+ assertNotNull("Expected a Condition", origErrorCondition.getCondition());
+
+ // Set an error condition, then call 'closed' specifying an error.
+ // Expect the original condition which was set to remain.
+ TransportImpl transport = new TransportImpl();
+
+ transport.setCondition(origErrorCondition);
+ transport.closed(new TransportException("my-ignored-exception"));
+
+ assertNotNull("Expected an ErrorCondition to be returned", transport.getCondition());
+ assertEquals("Unexpected ErrorCondition returned", origErrorCondition, transport.getCondition());
+
+ // Set an error condition, then call 'closed' without an error.
+ // Expect the original condition which was set to remain.
+ transport = new TransportImpl();
+
+ transport.setCondition(origErrorCondition);
+ transport.closed(null);
+
+ assertNotNull("Expected an ErrorCondition to be returned", transport.getCondition());
+ assertEquals("Unexpected ErrorCondition returned", origErrorCondition, transport.getCondition());
+
+ // Without having set an error condition, call 'closed' specifying an error.
+ // Expect a condition to be set.
+ String errorDescription = "some-error-description";
+ transport = new TransportImpl();
+ transport.closed(new TransportException(errorDescription));
+
+ assertNotNull("Expected an ErrorCondition to be returned", transport.getCondition());
+ assertEquals("Unexpected condition returned", ConnectionError.FRAMING_ERROR, transport.getCondition().getCondition());
+ assertEquals("Unexpected description returned", "org.apache.qpid.proton.engine.TransportException: " + errorDescription, transport.getCondition().getDescription());
+
+ // Without having set an error condition, call 'closed' without an error.
+ // Expect a condition to be set.
+ transport = new TransportImpl();
+
+ transport.closed(null);
+
+ assertNotNull("Expected an ErrorCondition to be returned", transport.getCondition());
+ assertEquals("Unexpected ErrorCondition returned", ConnectionError.FRAMING_ERROR, transport.getCondition().getCondition());
+ assertEquals("Unexpected description returned", "connection aborted", transport.getCondition().getDescription());
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org