Cleaning up GOAWAY methods.

Motivation:

The current GOAWAY methods are in each endpoint and are a little
confusing since their called connection.<endpoint>.goAwayReceived().

Modifications:

Moving GOAWAY methods to connection with more clear names
goAwaySent/goAwayReceived.

Result:

The GOAWAY method names are more clear.
This commit is contained in:
nmittler 2014-10-09 16:09:44 -07:00 committed by Norman Maurer
parent a82508d422
commit 74e72ea752
6 changed files with 56 additions and 33 deletions

View File

@ -149,7 +149,7 @@ public class DefaultHttp2Connection implements Http2Connection {
@Override @Override
public boolean isGoAway() { public boolean isGoAway() {
return localEndpoint.isGoAwayReceived() || remoteEndpoint.isGoAwayReceived(); return goAwaySent() || goAwayReceived();
} }
@Override @Override
@ -162,6 +162,26 @@ public class DefaultHttp2Connection implements Http2Connection {
return remote().createStream(streamId, halfClosed); return remote().createStream(streamId, halfClosed);
} }
@Override
public boolean goAwayReceived() {
return localEndpoint.lastKnownStream >= 0;
}
@Override
public void goAwayReceived(int lastKnownStream) {
localEndpoint.lastKnownStream(lastKnownStream);
}
@Override
public boolean goAwaySent() {
return remoteEndpoint.lastKnownStream >= 0;
}
@Override
public void goAwaySent(int lastKnownStream) {
remoteEndpoint.lastKnownStream(lastKnownStream);
}
private void removeStream(DefaultStream stream) { private void removeStream(DefaultStream stream) {
// Notify the listeners of the event first. // Notify the listeners of the event first.
for (Listener listener : listeners) { for (Listener listener : listeners) {
@ -791,16 +811,10 @@ public class DefaultHttp2Connection implements Http2Connection {
@Override @Override
public int lastKnownStream() { public int lastKnownStream() {
return isGoAwayReceived() ? lastKnownStream : lastStreamCreated; return lastKnownStream >= 0 ? lastKnownStream : lastStreamCreated;
} }
@Override private void lastKnownStream(int lastKnownStream) {
public boolean isGoAwayReceived() {
return lastKnownStream >= 0;
}
@Override
public void goAwayReceived(int lastKnownStream) {
boolean alreadyNotified = isGoAway(); boolean alreadyNotified = isGoAway();
this.lastKnownStream = lastKnownStream; this.lastKnownStream = lastKnownStream;
if (!alreadyNotified) { if (!alreadyNotified) {

View File

@ -249,7 +249,7 @@ public class DefaultHttp2ConnectionDecoder implements Http2ConnectionDecoder {
Http2Stream stream = connection.stream(streamId); Http2Stream stream = connection.stream(streamId);
verifyGoAwayNotReceived(); verifyGoAwayNotReceived();
verifyRstStreamNotReceived(stream); verifyRstStreamNotReceived(stream);
if (connection.remote().isGoAwayReceived() || stream != null && shouldIgnoreFrame(stream)) { if (connection.goAwaySent() || stream != null && shouldIgnoreFrame(stream)) {
// Ignore this frame. // Ignore this frame.
return; return;
} }
@ -426,7 +426,7 @@ public class DefaultHttp2ConnectionDecoder implements Http2ConnectionDecoder {
public void onGoAwayRead(ChannelHandlerContext ctx, int lastStreamId, long errorCode, ByteBuf debugData) public void onGoAwayRead(ChannelHandlerContext ctx, int lastStreamId, long errorCode, ByteBuf debugData)
throws Http2Exception { throws Http2Exception {
// Don't allow any more connections to be created. // Don't allow any more connections to be created.
connection.local().goAwayReceived(lastStreamId); connection.goAwayReceived(lastStreamId);
listener.onGoAwayRead(ctx, lastStreamId, errorCode, debugData); listener.onGoAwayRead(ctx, lastStreamId, errorCode, debugData);
} }
@ -461,7 +461,7 @@ public class DefaultHttp2ConnectionDecoder implements Http2ConnectionDecoder {
* stream/connection. * stream/connection.
*/ */
private boolean shouldIgnoreFrame(Http2Stream stream) { private boolean shouldIgnoreFrame(Http2Stream stream) {
if (connection.remote().isGoAwayReceived() && connection.remote().lastStreamCreated() <= stream.id()) { if (connection.goAwaySent() && connection.remote().lastStreamCreated() <= stream.id()) {
// Frames from streams created after we sent a go-away should be ignored. // Frames from streams created after we sent a go-away should be ignored.
// Frames for the connection stream ID (i.e. 0) will always be allowed. // Frames for the connection stream ID (i.e. 0) will always be allowed.
return true; return true;
@ -476,7 +476,7 @@ public class DefaultHttp2ConnectionDecoder implements Http2ConnectionDecoder {
* exception. * exception.
*/ */
private void verifyGoAwayNotReceived() throws Http2Exception { private void verifyGoAwayNotReceived() throws Http2Exception {
if (connection.local().isGoAwayReceived()) { if (connection.goAwayReceived()) {
throw protocolError("Received frames after receiving GO_AWAY"); throw protocolError("Received frames after receiving GO_AWAY");
} }
} }

View File

@ -186,17 +186,6 @@ public interface Http2Connection {
*/ */
int lastKnownStream(); int lastKnownStream();
/**
* Indicates whether or not a GOAWAY was received by this endpoint.
*/
boolean isGoAwayReceived();
/**
* Indicates that a GOAWAY was received from the opposite endpoint and sets the last known stream
* created by this endpoint.
*/
void goAwayReceived(int lastKnownStream);
/** /**
* Gets the {@link Endpoint} opposite this one. * Gets the {@link Endpoint} opposite this one.
*/ */
@ -265,6 +254,26 @@ public interface Http2Connection {
*/ */
Http2Stream createRemoteStream(int streamId, boolean halfClosed) throws Http2Exception; Http2Stream createRemoteStream(int streamId, boolean halfClosed) throws Http2Exception;
/**
* Indicates whether or not a {@code GOAWAY} was received from the remote endpoint.
*/
boolean goAwayReceived();
/**
* Indicates that a {@code GOAWAY} was received from the remote endpoint and sets the last known stream.
*/
void goAwayReceived(int lastKnownStream);
/**
* Indicates whether or not a {@code GOAWAY} was sent to the remote endpoint.
*/
boolean goAwaySent();
/**
* Indicates that a {@code GOAWAY} was sent to the remote endpoint and sets the last known stream.
*/
void goAwaySent(int lastKnownStream);
/** /**
* Indicates whether or not either endpoint has received a GOAWAY. * Indicates whether or not either endpoint has received a GOAWAY.
*/ */

View File

@ -342,7 +342,7 @@ public class Http2ConnectionHandler extends ByteToMessageDecoder implements Http
ChannelFuture future = frameWriter().writeGoAway(ctx, lastStreamId, errorCode, debugData, promise); ChannelFuture future = frameWriter().writeGoAway(ctx, lastStreamId, errorCode, debugData, promise);
ctx.flush(); ctx.flush();
connection().remote().goAwayReceived(lastStreamId); connection().goAwaySent(lastStreamId);
return future; return future;
} }

View File

@ -159,7 +159,7 @@ public class DefaultHttp2ConnectionDecoderTest {
@Test @Test
public void dataReadAfterGoAwayShouldApplyFlowControl() throws Exception { public void dataReadAfterGoAwayShouldApplyFlowControl() throws Exception {
when(remote.isGoAwayReceived()).thenReturn(true); when(connection.goAwaySent()).thenReturn(true);
final ByteBuf data = dummyData(); final ByteBuf data = dummyData();
try { try {
decode().onDataRead(ctx, STREAM_ID, data, 10, true); decode().onDataRead(ctx, STREAM_ID, data, 10, true);
@ -187,7 +187,7 @@ public class DefaultHttp2ConnectionDecoderTest {
@Test @Test
public void headersReadAfterGoAwayShouldBeIgnored() throws Exception { public void headersReadAfterGoAwayShouldBeIgnored() throws Exception {
when(remote.isGoAwayReceived()).thenReturn(true); when(connection.goAwaySent()).thenReturn(true);
decode().onHeadersRead(ctx, STREAM_ID, EmptyHttp2Headers.INSTANCE, 0, false); decode().onHeadersRead(ctx, STREAM_ID, EmptyHttp2Headers.INSTANCE, 0, false);
verify(remote, never()).createStream(eq(STREAM_ID), eq(false)); verify(remote, never()).createStream(eq(STREAM_ID), eq(false));
@ -235,7 +235,7 @@ public class DefaultHttp2ConnectionDecoderTest {
@Test @Test
public void pushPromiseReadAfterGoAwayShouldBeIgnored() throws Exception { public void pushPromiseReadAfterGoAwayShouldBeIgnored() throws Exception {
when(remote.isGoAwayReceived()).thenReturn(true); when(connection.goAwaySent()).thenReturn(true);
decode().onPushPromiseRead(ctx, STREAM_ID, PUSH_STREAM_ID, EmptyHttp2Headers.INSTANCE, 0); decode().onPushPromiseRead(ctx, STREAM_ID, PUSH_STREAM_ID, EmptyHttp2Headers.INSTANCE, 0);
verify(remote, never()).reservePushStream(anyInt(), any(Http2Stream.class)); verify(remote, never()).reservePushStream(anyInt(), any(Http2Stream.class));
verify(listener, never()).onPushPromiseRead(eq(ctx), anyInt(), anyInt(), any(Http2Headers.class), anyInt()); verify(listener, never()).onPushPromiseRead(eq(ctx), anyInt(), anyInt(), any(Http2Headers.class), anyInt());
@ -251,7 +251,7 @@ public class DefaultHttp2ConnectionDecoderTest {
@Test @Test
public void priorityReadAfterGoAwayShouldBeIgnored() throws Exception { public void priorityReadAfterGoAwayShouldBeIgnored() throws Exception {
when(remote.isGoAwayReceived()).thenReturn(true); when(connection.goAwaySent()).thenReturn(true);
decode().onPriorityRead(ctx, STREAM_ID, 0, (short) 255, true); decode().onPriorityRead(ctx, STREAM_ID, 0, (short) 255, true);
verify(stream, never()).setPriority(anyInt(), anyShort(), anyBoolean()); verify(stream, never()).setPriority(anyInt(), anyShort(), anyBoolean());
verify(listener, never()).onPriorityRead(eq(ctx), anyInt(), anyInt(), anyShort(), anyBoolean()); verify(listener, never()).onPriorityRead(eq(ctx), anyInt(), anyInt(), anyShort(), anyBoolean());
@ -266,7 +266,7 @@ public class DefaultHttp2ConnectionDecoderTest {
@Test @Test
public void windowUpdateReadAfterGoAwayShouldBeIgnored() throws Exception { public void windowUpdateReadAfterGoAwayShouldBeIgnored() throws Exception {
when(remote.isGoAwayReceived()).thenReturn(true); when(connection.goAwaySent()).thenReturn(true);
decode().onWindowUpdateRead(ctx, STREAM_ID, 10); decode().onWindowUpdateRead(ctx, STREAM_ID, 10);
verify(encoder, never()).updateOutboundWindowSize(anyInt(), anyInt()); verify(encoder, never()).updateOutboundWindowSize(anyInt(), anyInt());
verify(listener, never()).onWindowUpdateRead(eq(ctx), anyInt(), anyInt()); verify(listener, never()).onWindowUpdateRead(eq(ctx), anyInt(), anyInt());
@ -287,7 +287,7 @@ public class DefaultHttp2ConnectionDecoderTest {
@Test @Test
public void rstStreamReadAfterGoAwayShouldSucceed() throws Exception { public void rstStreamReadAfterGoAwayShouldSucceed() throws Exception {
when(remote.isGoAwayReceived()).thenReturn(true); when(connection.goAwaySent()).thenReturn(true);
decode().onRstStreamRead(ctx, STREAM_ID, PROTOCOL_ERROR.code()); decode().onRstStreamRead(ctx, STREAM_ID, PROTOCOL_ERROR.code());
verify(lifecycleManager).closeStream(eq(stream), eq(future)); verify(lifecycleManager).closeStream(eq(stream), eq(future));
verify(listener).onRstStreamRead(eq(ctx), anyInt(), anyLong()); verify(listener).onRstStreamRead(eq(ctx), anyInt(), anyLong());
@ -342,7 +342,7 @@ public class DefaultHttp2ConnectionDecoderTest {
@Test @Test
public void goAwayShouldReadShouldUpdateConnectionState() throws Exception { public void goAwayShouldReadShouldUpdateConnectionState() throws Exception {
decode().onGoAwayRead(ctx, 1, 2L, EMPTY_BUFFER); decode().onGoAwayRead(ctx, 1, 2L, EMPTY_BUFFER);
verify(local).goAwayReceived(1); verify(connection).goAwayReceived(1);
verify(listener).onGoAwayRead(eq(ctx), eq(1), eq(2L), eq(EMPTY_BUFFER)); verify(listener).onGoAwayRead(eq(ctx), eq(1), eq(2L), eq(EMPTY_BUFFER));
} }

View File

@ -174,7 +174,7 @@ public class DefaultHttp2ConnectionTest {
@Test(expected = Http2Exception.class) @Test(expected = Http2Exception.class)
public void goAwayReceivedShouldDisallowCreation() throws Http2Exception { public void goAwayReceivedShouldDisallowCreation() throws Http2Exception {
server.local().goAwayReceived(0); server.goAwayReceived(0);
server.remote().createStream(3, true); server.remote().createStream(3, true);
} }