HTTP/2 Remove Http2FrameStream#CONNECTION_STREAM

Motivation:
Http2FrameStream#CONNECTION_STREAM is required to identify the
connection stream. However this leads to inconsistent usage from a user
perspective. When a user creates a Http2Frame for a non-connection
stream, the Http2MultiplexCodec automatically sets the stream, and the
user is never exposed to the Http2FrameStream object. However when the
user writes a Http2Frame for a connection stream they are required to
set the Http2FrameStream object. We can remove the Http2FrameStream#CONNECTION_STREAM
and keep the Http2FrameStream object internal, and therefore consistent
between the connection and non-connection use cases.

Modifications:
- Remove Http2FrameStream#CONNECTION_STREAM
- Update Http2FrameCodec to handle Http2Frame#stream() which returns
null

Result:
More consistent usage on http2 parent channel and http2 child channel.
This commit is contained in:
Scott Mitchell 2018-01-11 18:07:06 -08:00 committed by Norman Maurer
parent 33ddb83dc1
commit d6e600548b
4 changed files with 36 additions and 41 deletions

View File

@ -21,21 +21,20 @@ import io.netty.channel.ChannelFutureListener;
import io.netty.channel.ChannelHandlerContext; import io.netty.channel.ChannelHandlerContext;
import io.netty.channel.ChannelInboundHandler; import io.netty.channel.ChannelInboundHandler;
import io.netty.channel.ChannelPromise; import io.netty.channel.ChannelPromise;
import io.netty.handler.codec.UnsupportedMessageTypeException;
import io.netty.handler.codec.http.HttpServerUpgradeHandler.UpgradeEvent;
import io.netty.handler.codec.http2.Http2Connection.PropertyKey; import io.netty.handler.codec.http2.Http2Connection.PropertyKey;
import io.netty.handler.codec.http2.Http2Stream.State; import io.netty.handler.codec.http2.Http2Stream.State;
import io.netty.handler.codec.http2.StreamBufferingEncoder.Http2ChannelClosedException; import io.netty.handler.codec.http2.StreamBufferingEncoder.Http2ChannelClosedException;
import io.netty.handler.codec.http2.StreamBufferingEncoder.Http2GoAwayException; import io.netty.handler.codec.http2.StreamBufferingEncoder.Http2GoAwayException;
import io.netty.handler.codec.UnsupportedMessageTypeException;
import io.netty.handler.codec.http.HttpServerUpgradeHandler.UpgradeEvent;
import io.netty.util.ReferenceCountUtil; import io.netty.util.ReferenceCountUtil;
import io.netty.util.ReferenceCounted; import io.netty.util.ReferenceCounted;
import io.netty.util.internal.UnstableApi; import io.netty.util.internal.UnstableApi;
import io.netty.util.internal.logging.InternalLogger; import io.netty.util.internal.logging.InternalLogger;
import io.netty.util.internal.logging.InternalLoggerFactory; import io.netty.util.internal.logging.InternalLoggerFactory;
import static io.netty.handler.codec.http2.Http2CodecUtil.isStreamIdValid;
import static io.netty.handler.codec.http2.Http2CodecUtil.HTTP_UPGRADE_STREAM_ID; import static io.netty.handler.codec.http2.Http2CodecUtil.HTTP_UPGRADE_STREAM_ID;
import static io.netty.handler.codec.http2.Http2CodecUtil.isStreamIdValid;
/** /**
* <p><em>This API is very immature.</em> The Http2Connection-based API is currently preferred over this API. * <p><em>This API is very immature.</em> The Http2Connection-based API is currently preferred over this API.
@ -274,7 +273,19 @@ public class Http2FrameCodec extends Http2ConnectionHandler {
writeHeadersFrame(ctx, (Http2HeadersFrame) msg, promise); writeHeadersFrame(ctx, (Http2HeadersFrame) msg, promise);
} else if (msg instanceof Http2WindowUpdateFrame) { } else if (msg instanceof Http2WindowUpdateFrame) {
Http2WindowUpdateFrame frame = (Http2WindowUpdateFrame) msg; Http2WindowUpdateFrame frame = (Http2WindowUpdateFrame) msg;
writeWindowUpdate(frame.stream().id(), frame.windowSizeIncrement(), promise); Http2FrameStream frameStream = frame.stream();
// It is legit to send a WINDOW_UPDATE frame for the connection stream. The parent channel doesn't attempt
// to set the Http2FrameStream so we assume if it is null the WINDOW_UPDATE is for the connection stream.
try {
if (frameStream == null) {
increaseInitialConnectionWindow(frame.windowSizeIncrement());
} else {
consumeBytes(frameStream.id(), frame.windowSizeIncrement());
}
promise.setSuccess();
} catch (Throwable t) {
promise.setFailure(t);
}
} else if (msg instanceof Http2ResetFrame) { } else if (msg instanceof Http2ResetFrame) {
Http2ResetFrame rstFrame = (Http2ResetFrame) msg; Http2ResetFrame rstFrame = (Http2ResetFrame) msg;
encoder().writeRstStream(ctx, rstFrame.stream().id(), rstFrame.errorCode(), promise); encoder().writeRstStream(ctx, rstFrame.stream().id(), rstFrame.errorCode(), promise);
@ -293,19 +304,6 @@ public class Http2FrameCodec extends Http2ConnectionHandler {
} }
} }
private void writeWindowUpdate(int streamId, int bytes, ChannelPromise promise) {
try {
if (streamId == 0) {
increaseInitialConnectionWindow(bytes);
} else {
consumeBytes(streamId, bytes);
}
promise.setSuccess();
} catch (Throwable t) {
promise.setFailure(t);
}
}
private void increaseInitialConnectionWindow(int deltaBytes) throws Http2Exception { private void increaseInitialConnectionWindow(int deltaBytes) throws Http2Exception {
Http2LocalFlowController localFlow = connection().local().flowController(); Http2LocalFlowController localFlow = connection().local().flowController();
int targetConnectionWindow = localFlow.initialWindowSize() + deltaBytes; int targetConnectionWindow = localFlow.initialWindowSize() + deltaBytes;

View File

@ -24,23 +24,6 @@ import io.netty.util.internal.UnstableApi;
*/ */
@UnstableApi @UnstableApi
public interface Http2FrameStream { public interface Http2FrameStream {
/**
* The stream with identifier 0, representing the HTTP/2 connection.
*/
Http2FrameStream CONNECTION_STREAM = new Http2FrameStream() {
@Override
public int id() {
return 0;
}
@Override
public State state() {
return State.IDLE;
}
};
/** /**
* Returns the stream identifier. * Returns the stream identifier.
* *

View File

@ -20,8 +20,7 @@ import io.netty.util.internal.UnstableApi;
/** /**
* A frame whose meaning <em>may</em> apply to a particular stream, instead of the entire connection. It is still * A frame whose meaning <em>may</em> apply to a particular stream, instead of the entire connection. It is still
* possible for this frame type to apply to the entire connection. In such cases, the {@link #stream()} must return * possible for this frame type to apply to the entire connection. In such cases, the {@link #stream()} must return
* {@link Http2FrameStream#CONNECTION_STREAM}. If the frame applies to a stream, the {@link Http2FrameStream#id()} must * {@code null}. If the frame applies to a stream, the {@link Http2FrameStream#id()} must be greater than zero.
* be greater than zero.
*/ */
@UnstableApi @UnstableApi
public interface Http2StreamFrame extends Http2Frame { public interface Http2StreamFrame extends Http2Frame {

View File

@ -57,10 +57,25 @@ import java.util.Set;
import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicBoolean;
import static io.netty.handler.codec.http2.Http2CodecUtil.isStreamIdValid; import static io.netty.handler.codec.http2.Http2CodecUtil.isStreamIdValid;
import static io.netty.handler.codec.http2.Http2FrameStream.CONNECTION_STREAM;
import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.instanceOf;
import static org.junit.Assert.*; import static org.junit.Assert.assertEquals;
import static org.mockito.Mockito.*; import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
import static org.mockito.Mockito.any;
import static org.mockito.Mockito.anyBoolean;
import static org.mockito.Mockito.anyInt;
import static org.mockito.Mockito.anyLong;
import static org.mockito.Mockito.anyShort;
import static org.mockito.Mockito.eq;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.same;
import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyNoMoreInteractions;
/** /**
* Unit tests for {@link Http2FrameCodec}. * Unit tests for {@link Http2FrameCodec}.
@ -470,7 +485,7 @@ public class Http2FrameCodecTest {
int windowUpdate = 1024; int windowUpdate = 1024;
channel.write(new DefaultHttp2WindowUpdateFrame(windowUpdate).stream(CONNECTION_STREAM)); channel.write(new DefaultHttp2WindowUpdateFrame(windowUpdate));
assertEquals(initialWindowSizeBefore + windowUpdate, localFlow.initialWindowSize()); assertEquals(initialWindowSizeBefore + windowUpdate, localFlow.initialWindowSize());
} }