Repository: netty/netty
Issue: Rename HTTP/2 Stream methods indicating writability

The remoteSideOpen() and localSideOpen() are a little confusing.  It would be clearer to have isWritable and isReadable.


Issue comments:
+1

Closing this since we decided to keep the current naming for the time being (in #3491)


Pull request review comments:

Patch that resolved the issue:
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 bc3f6d8129a..7a306e49edc 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
@@ -404,7 +404,7 @@ public Http2Stream close() {
         }
 
         @Override
-        public Http2Stream closeLocalSide() {
+        public Http2Stream closeForWriting() {
             switch (state) {
             case OPEN:
                 state = HALF_CLOSED_LOCAL;
@@ -420,7 +420,7 @@ public Http2Stream closeLocalSide() {
         }
 
         @Override
-        public Http2Stream closeRemoteSide() {
+        public Http2Stream closeForReading() {
             switch (state) {
             case OPEN:
                 state = HALF_CLOSED_REMOTE;
@@ -442,12 +442,12 @@ private void notifyHalfClosed(Http2Stream stream) {
         }
 
         @Override
-        public final boolean remoteSideOpen() {
+        public final boolean isReadable() {
             return state == HALF_CLOSED_LOCAL || state == OPEN || state == RESERVED_REMOTE;
         }
 
         @Override
-        public final boolean localSideOpen() {
+        public final boolean isWritable() {
             return state == HALF_CLOSED_REMOTE || state == OPEN || state == RESERVED_LOCAL;
         }
 
@@ -671,12 +671,12 @@ public Http2Stream close() {
         }
 
         @Override
-        public Http2Stream closeLocalSide() {
+        public Http2Stream closeForWriting() {
             throw new UnsupportedOperationException();
         }
 
         @Override
-        public Http2Stream closeRemoteSide() {
+        public Http2Stream closeForReading() {
             throw new UnsupportedOperationException();
         }
     }
@@ -759,7 +759,7 @@ public DefaultStream reservePushStream(int streamId, Http2Stream parent) throws
             if (parent == null) {
                 throw connectionError(PROTOCOL_ERROR, "Parent stream missing");
             }
-            if (isLocal() ? !parent.localSideOpen() : !parent.remoteSideOpen()) {
+            if (isLocal() ? !parent.isWritable() : !parent.isReadable()) {
                 throw connectionError(PROTOCOL_ERROR, "Stream %d is not open for sending push promise", parent.id());
             }
             if (!opposite().allowPushTo()) {
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 f2406b2ec15..a92f5e0098b 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
@@ -289,7 +289,7 @@ public int onDataRead(final ChannelHandlerContext ctx, int streamId, ByteBuf dat
                 }
 
                 if (endOfStream) {
-                    lifecycleManager.closeRemoteSide(stream, ctx.newSucceededFuture());
+                    lifecycleManager.closeForReading(stream, ctx.newSucceededFuture());
                 }
             }
         }
@@ -352,7 +352,7 @@ public void onHeadersRead(ChannelHandlerContext ctx, int streamId, Http2Headers
 
             // If the headers completes this stream, close it.
             if (endOfStream) {
-                lifecycleManager.closeRemoteSide(stream, ctx.newSucceededFuture());
+                lifecycleManager.closeForReading(stream, ctx.newSucceededFuture());
             }
         }
 
diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionEncoder.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionEncoder.java
index 313fab02f18..48b43fafa36 100644
--- a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionEncoder.java
+++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionEncoder.java
@@ -536,7 +536,7 @@ public FlowControlledBase(final ChannelHandlerContext ctx, final Http2Stream str
         @Override
         public void writeComplete() {
             if (endOfStream) {
-                lifecycleManager.closeLocalSide(stream, promise);
+                lifecycleManager.closeForWriting(stream, promise);
             }
         }
 
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 f5fc5dc6fd6..16e34867d1d 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
@@ -208,19 +208,12 @@ public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) throws E
         }
     }
 
-    /**
-     * Closes the local side of the given stream. If this causes the stream to be closed, adds a
-     * hook to close the channel after the given future completes.
-     *
-     * @param stream the stream to be half closed.
-     * @param future If closing, the future after which to close the channel.
-     */
     @Override
-    public void closeLocalSide(Http2Stream stream, ChannelFuture future) {
+    public void closeForWriting(Http2Stream stream, ChannelFuture future) {
         switch (stream.state()) {
             case HALF_CLOSED_LOCAL:
             case OPEN:
-                stream.closeLocalSide();
+                stream.closeForWriting();
                 break;
             default:
                 closeStream(stream, future);
@@ -228,19 +221,12 @@ public void closeLocalSide(Http2Stream stream, ChannelFuture future) {
         }
     }
 
-    /**
-     * Closes the remote side of the given stream. If this causes the stream to be closed, adds a
-     * hook to close the channel after the given future completes.
-     *
-     * @param stream the stream to be half closed.
-     * @param future If closing, the future after which to close the channel.
-     */
     @Override
-    public void closeRemoteSide(Http2Stream stream, ChannelFuture future) {
+    public void closeForReading(Http2Stream stream, ChannelFuture future) {
         switch (stream.state()) {
             case HALF_CLOSED_REMOTE:
             case OPEN:
-                stream.closeRemoteSide();
+                stream.closeForReading();
                 break;
             default:
                 closeStream(stream, future);
diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2LifecycleManager.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2LifecycleManager.java
index 082d734ef4d..ec13ec099ca 100644
--- a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2LifecycleManager.java
+++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2LifecycleManager.java
@@ -26,22 +26,22 @@
 public interface Http2LifecycleManager {
 
     /**
-     * Closes the local side of the given stream. If this causes the stream to be closed, adds a
+     * Closes the given stream for writing (i.e. closes the local side). If this causes the stream to be closed, adds a
      * hook to deactivate the stream and close the channel after the given future completes.
      *
      * @param stream the stream to be half closed.
      * @param future If closing, the future after which to close the channel.
      */
-    void closeLocalSide(Http2Stream stream, ChannelFuture future);
+    void closeForWriting(Http2Stream stream, ChannelFuture future);
 
     /**
-     * Closes the remote side of the given stream. If this causes the stream to be closed, adds a
+     * Closes the given stream for reading (i.e. closes the remote side). If this causes the stream to be closed, adds a
      * hook to deactivate the stream and close the channel after the given future completes.
      *
      * @param stream the stream to be half closed.
      * @param future If closing, the future after which to close the channel.
      */
-    void closeRemoteSide(Http2Stream stream, ChannelFuture future);
+    void closeForReading(Http2Stream stream, ChannelFuture future);
 
     /**
      * Closes the given stream and adds a hook to deactivate the stream and close the channel after
diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2Stream.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2Stream.java
index 676213d341c..55df4154f06 100644
--- a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2Stream.java
+++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2Stream.java
@@ -65,16 +65,14 @@ enum State {
     Http2Stream close();
 
     /**
-     * Closes the local side of this stream. If this makes the stream closed, the child is closed as
-     * well.
+     * Closes the this stream for writing (i.e. closes the local side).
      */
-    Http2Stream closeLocalSide();
+    Http2Stream closeForWriting();
 
     /**
-     * Closes the remote side of this stream. If this makes the stream closed, the child is closed
-     * as well.
+     * Closes this stream for reading (i.e. closes the remote side).
      */
-    Http2Stream closeRemoteSide();
+    Http2Stream closeForReading();
 
     /**
      * Indicates whether a {@code RST_STREAM} frame has been sent from the local endpoint for this stream.
@@ -88,16 +86,16 @@ enum State {
     Http2Stream resetSent();
 
     /**
-     * Indicates whether the remote side of this stream is open (i.e. the state is either
-     * {@link State#OPEN} or {@link State#HALF_CLOSED_LOCAL}).
+     * Indicates whether the remote side of this stream is open, allowing the stream to be readable (i.e.
+     * the state is either {@link State#OPEN} or {@link State#HALF_CLOSED_LOCAL}).
      */
-    boolean remoteSideOpen();
+    boolean isReadable();
 
     /**
-     * Indicates whether the local side of this stream is open (i.e. the state is either
-     * {@link State#OPEN} or {@link State#HALF_CLOSED_REMOTE}).
+     * Indicates whether the local side of this stream is open, allowing the stream to be written (i.e.
+     * the state is either {@link State#OPEN} or {@link State#HALF_CLOSED_REMOTE}).
      */
-    boolean localSideOpen();
+    boolean isWritable();
 
     /**
      * Associates the application-defined data with this stream.


Test patch:
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 0737811c378..fd0a525b282 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
@@ -253,7 +253,7 @@ public void dataReadWithEndOfStreamShouldCloseRemoteSide() throws Exception {
         try {
             decode().onDataRead(ctx, STREAM_ID, data, 10, true);
             verify(localFlow).receiveFlowControlledFrame(eq(ctx), eq(stream), eq(data), eq(10), eq(true));
-            verify(lifecycleManager).closeRemoteSide(eq(stream), eq(future));
+            verify(lifecycleManager).closeForReading(eq(stream), eq(future));
             verify(listener).onDataRead(eq(ctx), eq(STREAM_ID), eq(data), eq(10), eq(true));
         } finally {
             data.release();
@@ -296,7 +296,7 @@ public Integer answer(InvocationOnMock in) throws Throwable {
         } catch (RuntimeException cause) {
             verify(localFlow)
                     .receiveFlowControlledFrame(eq(ctx), eq(stream), eq(data), eq(padding), eq(true));
-            verify(lifecycleManager).closeRemoteSide(eq(stream), eq(future));
+            verify(lifecycleManager).closeForReading(eq(stream), eq(future));
             verify(listener).onDataRead(eq(ctx), eq(STREAM_ID), eq(data), eq(padding), eq(true));
             assertEquals(0, localFlow.unconsumedBytes(stream));
         } finally {
@@ -353,7 +353,7 @@ public void headersReadForPromisedStreamShouldCloseStream() throws Exception {
         when(stream.state()).thenReturn(RESERVED_REMOTE);
         decode().onHeadersRead(ctx, STREAM_ID, EmptyHttp2Headers.INSTANCE, 0, true);
         verify(stream).open(true);
-        verify(lifecycleManager).closeRemoteSide(eq(stream), eq(future));
+        verify(lifecycleManager).closeForReading(eq(stream), eq(future));
         verify(listener).onHeadersRead(eq(ctx), eq(STREAM_ID), eq(EmptyHttp2Headers.INSTANCE), eq(0),
                 eq(DEFAULT_PRIORITY_WEIGHT), eq(false), eq(0), eq(true));
     }
diff --git a/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionEncoderTest.java b/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionEncoderTest.java
index 45ccc991208..bd267986c03 100644
--- a/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionEncoderTest.java
+++ b/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionEncoderTest.java
@@ -36,7 +36,6 @@
 import static org.mockito.Mockito.doAnswer;
 import static org.mockito.Mockito.doNothing;
 import static org.mockito.Mockito.never;
-import static org.mockito.Mockito.reset;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
@@ -351,7 +350,7 @@ public void headersWriteShouldOpenStreamForPush() throws Exception {
         when(stream.state()).thenReturn(RESERVED_LOCAL);
         encoder.writeHeaders(ctx, STREAM_ID, EmptyHttp2Headers.INSTANCE, 0, false, promise);
         verify(stream).open(false);
-        verify(stream, never()).closeLocalSide();
+        verify(stream, never()).closeForWriting();
         assertNotNull(payloadCaptor.getValue());
         payloadCaptor.getValue().write(0);
         verify(writer).writeHeaders(eq(ctx), eq(STREAM_ID), eq(EmptyHttp2Headers.INSTANCE), eq(0),
@@ -442,7 +441,7 @@ public void dataWriteShouldCreateHalfClosedStream() {
         ByteBuf data = dummyData();
         encoder.writeData(ctx, STREAM_ID, data.retain(), 0, true, promise);
         verify(remoteFlow).sendFlowControlled(eq(ctx), eq(stream), any(FlowControlled.class));
-        verify(lifecycleManager).closeLocalSide(stream, promise);
+        verify(lifecycleManager).closeForWriting(stream, promise);
         assertEquals(data.toString(UTF_8), writtenData.get(0));
         data.release();
     }
@@ -464,7 +463,7 @@ public void headersWriteShouldHalfCloseStream() throws Exception {
         // Trigger the write and mark the promise successful to trigger listeners
         payloadCaptor.getValue().write(0);
         promise.trySuccess();
-        verify(lifecycleManager).closeLocalSide(eq(stream), eq(promise));
+        verify(lifecycleManager).closeForWriting(eq(stream), eq(promise));
     }
 
     @Test
@@ -479,7 +478,7 @@ public void headersWriteShouldHalfClosePushStream() throws Exception {
         verify(stream).open(true);
 
         promise.trySuccess();
-        verify(lifecycleManager).closeLocalSide(eq(stream), eq(promise));
+        verify(lifecycleManager).closeForWriting(eq(stream), eq(promise));
     }
 
     private void mockSendFlowControlledWriteEverything() {
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 5e394104b48..49a651b589e 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
@@ -191,7 +191,7 @@ public void closeShouldSucceed() throws Http2Exception {
     @Test
     public void closeLocalWhenOpenShouldSucceed() throws Http2Exception {
         Http2Stream stream = server.remote().createStream(3).open(false);
-        stream.closeLocalSide();
+        stream.closeForWriting();
         assertEquals(State.HALF_CLOSED_LOCAL, stream.state());
         assertEquals(1, server.activeStreams().size());
     }
@@ -199,7 +199,7 @@ public void closeLocalWhenOpenShouldSucceed() throws Http2Exception {
     @Test
     public void closeRemoteWhenOpenShouldSucceed() throws Http2Exception {
         Http2Stream stream = server.remote().createStream(3).open(false);
-        stream.closeRemoteSide();
+        stream.closeForReading();
         assertEquals(State.HALF_CLOSED_REMOTE, stream.state());
         assertEquals(1, server.activeStreams().size());
     }
@@ -207,7 +207,7 @@ public void closeRemoteWhenOpenShouldSucceed() throws Http2Exception {
     @Test
     public void closeOnlyOpenSideShouldClose() throws Http2Exception {
         Http2Stream stream = server.remote().createStream(3).open(true);
-        stream.closeLocalSide();
+        stream.closeForWriting();
         assertEquals(State.CLOSED, stream.state());
         assertTrue(server.activeStreams().isEmpty());
     }
diff --git a/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2RemoteFlowControllerTest.java b/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2RemoteFlowControllerTest.java
index d508e72893a..7abc89ffcea 100644
--- a/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2RemoteFlowControllerTest.java
+++ b/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2RemoteFlowControllerTest.java
@@ -35,7 +35,6 @@
 import io.netty.channel.ChannelHandlerContext;
 import io.netty.channel.ChannelPromise;
 import io.netty.handler.codec.http2.Http2FrameWriter.Configuration;
-import io.netty.handler.codec.http2.Http2RemoteFlowController.FlowControlled;
 import io.netty.util.collection.IntObjectHashMap;
 import io.netty.util.collection.IntObjectMap;
 
@@ -1001,7 +1000,7 @@ public void flowControlledWriteThrowsAnException() throws Exception {
         final Http2Stream stream = stream(STREAM_A);
         doAnswer(new Answer<Void>() {
             public Void answer(InvocationOnMock invocationOnMock) {
-                stream.closeLocalSide();
+                stream.closeForWriting();
                 return null;
             }
         }).when(flowControlled).error(any(Throwable.class));