From 74e72ea752decf59bc8ee4b7a1c3cc6e8532079f Mon Sep 17 00:00:00 2001 From: nmittler Date: Thu, 9 Oct 2014 16:09:44 -0700 Subject: [PATCH] Cleaning up GOAWAY methods. Motivation: The current GOAWAY methods are in each endpoint and are a little confusing since their called connection..goAwayReceived(). Modifications: Moving GOAWAY methods to connection with more clear names goAwaySent/goAwayReceived. Result: The GOAWAY method names are more clear. --- .../codec/http2/DefaultHttp2Connection.java | 32 +++++++++++++------ .../http2/DefaultHttp2ConnectionDecoder.java | 8 ++--- .../handler/codec/http2/Http2Connection.java | 31 +++++++++++------- .../codec/http2/Http2ConnectionHandler.java | 2 +- .../DefaultHttp2ConnectionDecoderTest.java | 14 ++++---- .../http2/DefaultHttp2ConnectionTest.java | 2 +- 6 files changed, 56 insertions(+), 33 deletions(-) diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2Connection.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2Connection.java index c69cc5bec2..043c2b8d9d 100644 --- a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2Connection.java +++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2Connection.java @@ -149,7 +149,7 @@ public class DefaultHttp2Connection implements Http2Connection { @Override public boolean isGoAway() { - return localEndpoint.isGoAwayReceived() || remoteEndpoint.isGoAwayReceived(); + return goAwaySent() || goAwayReceived(); } @Override @@ -162,6 +162,26 @@ public class DefaultHttp2Connection implements Http2Connection { 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) { // Notify the listeners of the event first. for (Listener listener : listeners) { @@ -791,16 +811,10 @@ public class DefaultHttp2Connection implements Http2Connection { @Override public int lastKnownStream() { - return isGoAwayReceived() ? lastKnownStream : lastStreamCreated; + return lastKnownStream >= 0 ? lastKnownStream : lastStreamCreated; } - @Override - public boolean isGoAwayReceived() { - return lastKnownStream >= 0; - } - - @Override - public void goAwayReceived(int lastKnownStream) { + private void lastKnownStream(int lastKnownStream) { boolean alreadyNotified = isGoAway(); this.lastKnownStream = lastKnownStream; if (!alreadyNotified) { diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionDecoder.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionDecoder.java index ee4c0da8ac..d27fc6dc5f 100644 --- a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionDecoder.java +++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionDecoder.java @@ -249,7 +249,7 @@ public class DefaultHttp2ConnectionDecoder implements Http2ConnectionDecoder { Http2Stream stream = connection.stream(streamId); verifyGoAwayNotReceived(); verifyRstStreamNotReceived(stream); - if (connection.remote().isGoAwayReceived() || stream != null && shouldIgnoreFrame(stream)) { + if (connection.goAwaySent() || stream != null && shouldIgnoreFrame(stream)) { // Ignore this frame. return; } @@ -426,7 +426,7 @@ public class DefaultHttp2ConnectionDecoder implements Http2ConnectionDecoder { public void onGoAwayRead(ChannelHandlerContext ctx, int lastStreamId, long errorCode, ByteBuf debugData) throws Http2Exception { // Don't allow any more connections to be created. - connection.local().goAwayReceived(lastStreamId); + connection.goAwayReceived(lastStreamId); listener.onGoAwayRead(ctx, lastStreamId, errorCode, debugData); } @@ -461,7 +461,7 @@ public class DefaultHttp2ConnectionDecoder implements Http2ConnectionDecoder { * stream/connection. */ 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 for the connection stream ID (i.e. 0) will always be allowed. return true; @@ -476,7 +476,7 @@ public class DefaultHttp2ConnectionDecoder implements Http2ConnectionDecoder { * exception. */ private void verifyGoAwayNotReceived() throws Http2Exception { - if (connection.local().isGoAwayReceived()) { + if (connection.goAwayReceived()) { throw protocolError("Received frames after receiving GO_AWAY"); } } diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2Connection.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2Connection.java index 8d77c3ad13..45985304db 100644 --- a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2Connection.java +++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2Connection.java @@ -186,17 +186,6 @@ public interface Http2Connection { */ 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. */ @@ -265,6 +254,26 @@ public interface Http2Connection { */ 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. */ diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2ConnectionHandler.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2ConnectionHandler.java index 7d53e3cd09..41824c9910 100644 --- a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2ConnectionHandler.java +++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2ConnectionHandler.java @@ -342,7 +342,7 @@ public class Http2ConnectionHandler extends ByteToMessageDecoder implements Http ChannelFuture future = frameWriter().writeGoAway(ctx, lastStreamId, errorCode, debugData, promise); ctx.flush(); - connection().remote().goAwayReceived(lastStreamId); + connection().goAwaySent(lastStreamId); return future; } diff --git a/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionDecoderTest.java b/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionDecoderTest.java index 9de6880a0e..b660542aad 100644 --- a/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionDecoderTest.java +++ b/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionDecoderTest.java @@ -159,7 +159,7 @@ public class DefaultHttp2ConnectionDecoderTest { @Test public void dataReadAfterGoAwayShouldApplyFlowControl() throws Exception { - when(remote.isGoAwayReceived()).thenReturn(true); + when(connection.goAwaySent()).thenReturn(true); final ByteBuf data = dummyData(); try { decode().onDataRead(ctx, STREAM_ID, data, 10, true); @@ -187,7 +187,7 @@ public class DefaultHttp2ConnectionDecoderTest { @Test public void headersReadAfterGoAwayShouldBeIgnored() throws Exception { - when(remote.isGoAwayReceived()).thenReturn(true); + when(connection.goAwaySent()).thenReturn(true); decode().onHeadersRead(ctx, STREAM_ID, EmptyHttp2Headers.INSTANCE, 0, false); verify(remote, never()).createStream(eq(STREAM_ID), eq(false)); @@ -235,7 +235,7 @@ public class DefaultHttp2ConnectionDecoderTest { @Test 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); verify(remote, never()).reservePushStream(anyInt(), any(Http2Stream.class)); verify(listener, never()).onPushPromiseRead(eq(ctx), anyInt(), anyInt(), any(Http2Headers.class), anyInt()); @@ -251,7 +251,7 @@ public class DefaultHttp2ConnectionDecoderTest { @Test public void priorityReadAfterGoAwayShouldBeIgnored() throws Exception { - when(remote.isGoAwayReceived()).thenReturn(true); + when(connection.goAwaySent()).thenReturn(true); decode().onPriorityRead(ctx, STREAM_ID, 0, (short) 255, true); verify(stream, never()).setPriority(anyInt(), anyShort(), anyBoolean()); verify(listener, never()).onPriorityRead(eq(ctx), anyInt(), anyInt(), anyShort(), anyBoolean()); @@ -266,7 +266,7 @@ public class DefaultHttp2ConnectionDecoderTest { @Test public void windowUpdateReadAfterGoAwayShouldBeIgnored() throws Exception { - when(remote.isGoAwayReceived()).thenReturn(true); + when(connection.goAwaySent()).thenReturn(true); decode().onWindowUpdateRead(ctx, STREAM_ID, 10); verify(encoder, never()).updateOutboundWindowSize(anyInt(), anyInt()); verify(listener, never()).onWindowUpdateRead(eq(ctx), anyInt(), anyInt()); @@ -287,7 +287,7 @@ public class DefaultHttp2ConnectionDecoderTest { @Test public void rstStreamReadAfterGoAwayShouldSucceed() throws Exception { - when(remote.isGoAwayReceived()).thenReturn(true); + when(connection.goAwaySent()).thenReturn(true); decode().onRstStreamRead(ctx, STREAM_ID, PROTOCOL_ERROR.code()); verify(lifecycleManager).closeStream(eq(stream), eq(future)); verify(listener).onRstStreamRead(eq(ctx), anyInt(), anyLong()); @@ -342,7 +342,7 @@ public class DefaultHttp2ConnectionDecoderTest { @Test public void goAwayShouldReadShouldUpdateConnectionState() throws Exception { 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)); } diff --git a/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionTest.java b/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionTest.java index 46596cd3f9..e64544591b 100644 --- a/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionTest.java +++ b/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionTest.java @@ -174,7 +174,7 @@ public class DefaultHttp2ConnectionTest { @Test(expected = Http2Exception.class) public void goAwayReceivedShouldDisallowCreation() throws Http2Exception { - server.local().goAwayReceived(0); + server.goAwayReceived(0); server.remote().createStream(3, true); }