Remove compressed flag from HTTP/2

Motivation:

There are still a few places in the HTTP/2 code that have the compressed
flag (from pre-draft 13). Need to remove this flag since it's no longer
used.

Modifications:

Various changes to remove the flag from the writing path.

Result:

No references to the compressed flag.
This commit is contained in:
nmittler 2014-07-23 09:11:57 -07:00 committed by Norman Maurer
parent 1a8b29cfb7
commit 46a6312cba
8 changed files with 23 additions and 28 deletions

View File

@ -338,7 +338,7 @@ public abstract class AbstractHttp2ConnectionHandler extends ByteToMessageDecode
protected ChannelFuture writeData(final ChannelHandlerContext ctx, protected ChannelFuture writeData(final ChannelHandlerContext ctx,
final ChannelPromise promise, int streamId, final ByteBuf data, int padding, final ChannelPromise promise, int streamId, final ByteBuf data, int padding,
boolean endStream, boolean endSegment, boolean compressed) { boolean endStream, boolean endSegment) {
try { try {
if (connection.isGoAway()) { if (connection.isGoAway()) {
throw protocolError("Sending data after connection going away."); throw protocolError("Sending data after connection going away.");
@ -349,7 +349,7 @@ public abstract class AbstractHttp2ConnectionHandler extends ByteToMessageDecode
// Hand control of the frame to the flow controller. // Hand control of the frame to the flow controller.
outboundFlow.sendFlowControlled(streamId, data, padding, endStream, endSegment, outboundFlow.sendFlowControlled(streamId, data, padding, endStream, endSegment,
compressed, new FlowControlWriter(ctx, data, promise)); new FlowControlWriter(ctx, data, promise));
return promise; return promise;
} catch (Http2Exception e) { } catch (Http2Exception e) {
@ -1094,7 +1094,7 @@ public abstract class AbstractHttp2ConnectionHandler extends ByteToMessageDecode
@Override @Override
public void writeFrame(int streamId, ByteBuf data, int padding, public void writeFrame(int streamId, ByteBuf data, int padding,
boolean endStream, boolean endSegment, boolean compressed) { boolean endStream, boolean endSegment) {
if (promise.isDone()) { if (promise.isDone()) {
// Most likely the write already failed. Just release the // Most likely the write already failed. Just release the
// buffer. // buffer.

View File

@ -149,10 +149,10 @@ public class DefaultHttp2OutboundFlowController implements Http2OutboundFlowCont
@Override @Override
public void sendFlowControlled(int streamId, ByteBuf data, int padding, boolean endStream, public void sendFlowControlled(int streamId, ByteBuf data, int padding, boolean endStream,
boolean endSegment, boolean compressed, FrameWriter frameWriter) throws Http2Exception { boolean endSegment, FrameWriter frameWriter) throws Http2Exception {
OutboundFlowState state = stateOrFail(streamId); OutboundFlowState state = stateOrFail(streamId);
OutboundFlowState.Frame frame = OutboundFlowState.Frame frame =
state.newFrame(data, padding, endStream, endSegment, compressed, frameWriter); state.newFrame(data, padding, endStream, endSegment, frameWriter);
int dataLength = data.readableBytes(); int dataLength = data.readableBytes();
if (state.writableWindow() >= dataLength) { if (state.writableWindow() >= dataLength) {
@ -442,8 +442,8 @@ public class DefaultHttp2OutboundFlowController implements Http2OutboundFlowCont
* Creates a new frame with the given values but does not add it to the pending queue. * Creates a new frame with the given values but does not add it to the pending queue.
*/ */
Frame newFrame(ByteBuf data, int padding, boolean endStream, boolean endSegment, Frame newFrame(ByteBuf data, int padding, boolean endStream, boolean endSegment,
boolean compressed, FrameWriter writer) { FrameWriter writer) {
return new Frame(data, padding, endStream, endSegment, compressed, writer); return new Frame(data, padding, endStream, endSegment, writer);
} }
/** /**
@ -530,17 +530,15 @@ public class DefaultHttp2OutboundFlowController implements Http2OutboundFlowCont
private final int padding; private final int padding;
private final boolean endStream; private final boolean endStream;
private final boolean endSegment; private final boolean endSegment;
private final boolean compressed;
private final FrameWriter writer; private final FrameWriter writer;
private boolean enqueued; private boolean enqueued;
Frame(ByteBuf data, int padding, boolean endStream, boolean endSegment, Frame(ByteBuf data, int padding, boolean endStream, boolean endSegment,
boolean compressed, FrameWriter writer) { FrameWriter writer) {
this.data = data; this.data = data;
this.padding = padding; this.padding = padding;
this.endStream = endStream; this.endStream = endStream;
this.endSegment = endSegment; this.endSegment = endSegment;
this.compressed = compressed;
this.writer = writer; this.writer = writer;
} }
@ -580,7 +578,7 @@ public class DefaultHttp2OutboundFlowController implements Http2OutboundFlowCont
int dataLength = data.readableBytes(); int dataLength = data.readableBytes();
connectionState().incrementStreamWindow(-dataLength); connectionState().incrementStreamWindow(-dataLength);
incrementStreamWindow(-dataLength); incrementStreamWindow(-dataLength);
writer.writeFrame(stream.id(), data, padding, endStream, endSegment, compressed); writer.writeFrame(stream.id(), data, padding, endStream, endSegment);
decrementPendingBytes(dataLength); decrementPendingBytes(dataLength);
} }
@ -606,7 +604,7 @@ public class DefaultHttp2OutboundFlowController implements Http2OutboundFlowCont
Frame split(int maxBytes) { Frame split(int maxBytes) {
// TODO: Should padding be included in the chunks or only the last frame? // TODO: Should padding be included in the chunks or only the last frame?
maxBytes = min(maxBytes, data.readableBytes()); maxBytes = min(maxBytes, data.readableBytes());
Frame frame = new Frame(data.readSlice(maxBytes).retain(), 0, false, false, compressed, writer); Frame frame = new Frame(data.readSlice(maxBytes).retain(), 0, false, false, writer);
decrementPendingBytes(maxBytes); decrementPendingBytes(maxBytes);
return frame; return frame;
} }

View File

@ -53,9 +53,8 @@ public class DelegatingHttp2ConnectionHandler extends AbstractHttp2ConnectionHan
@Override @Override
public ChannelFuture writeData(ChannelHandlerContext ctx, ChannelPromise promise, int streamId, public ChannelFuture writeData(ChannelHandlerContext ctx, ChannelPromise promise, int streamId,
ByteBuf data, int padding, boolean endStream, boolean endSegment, boolean compressed) { ByteBuf data, int padding, boolean endStream, boolean endSegment) {
return super.writeData(ctx, promise, streamId, data, padding, endStream, endSegment, return super.writeData(ctx, promise, streamId, data, padding, endStream, endSegment);
compressed);
} }
@Override @Override

View File

@ -31,7 +31,7 @@ public interface Http2OutboundFlowController {
* Writes a single data frame to the remote endpoint. * Writes a single data frame to the remote endpoint.
*/ */
void writeFrame(int streamId, ByteBuf data, int padding, boolean endStream, void writeFrame(int streamId, ByteBuf data, int padding, boolean endStream,
boolean endSegment, boolean compressed); boolean endSegment);
/** /**
* Called if an error occurred before the write could take place. Sets the failure on the * Called if an error occurred before the write could take place. Sets the failure on the
@ -80,10 +80,9 @@ public interface Http2OutboundFlowController {
* @param padding the number of bytes of padding to be added to the frame. * @param padding the number of bytes of padding to be added to the frame.
* @param endStream indicates whether this frames is to be the last sent on this stream. * @param endStream indicates whether this frames is to be the last sent on this stream.
* @param endSegment indicates whether this is to be the last frame in the segment. * @param endSegment indicates whether this is to be the last frame in the segment.
* @param compressed whether the data is compressed using gzip compression.
* @param frameWriter peforms to the write of the frame to the remote endpoint. * @param frameWriter peforms to the write of the frame to the remote endpoint.
* @throws Http2Exception thrown if a protocol-related error occurred. * @throws Http2Exception thrown if a protocol-related error occurred.
*/ */
void sendFlowControlled(int streamId, ByteBuf data, int padding, boolean endStream, void sendFlowControlled(int streamId, ByteBuf data, int padding, boolean endStream,
boolean endSegment, boolean compressed, FrameWriter frameWriter) throws Http2Exception; boolean endSegment, FrameWriter frameWriter) throws Http2Exception;
} }

View File

@ -542,22 +542,21 @@ public class DefaultHttp2OutboundFlowControllerTest {
} }
private void send(int streamId, ByteBuf data) throws Http2Exception { private void send(int streamId, ByteBuf data) throws Http2Exception {
controller.sendFlowControlled(streamId, data, 0, false, false, false, frameWriter); controller.sendFlowControlled(streamId, data, 0, false, false, frameWriter);
} }
private void verifyWrite(int streamId, ByteBuf data) { private void verifyWrite(int streamId, ByteBuf data) {
verify(frameWriter).writeFrame(eq(streamId), eq(data), eq(0), eq(false), eq(false), verify(frameWriter).writeFrame(eq(streamId), eq(data), eq(0), eq(false), eq(false));
eq(false));
} }
private void verifyNoWrite(int streamId) { private void verifyNoWrite(int streamId) {
verify(frameWriter, never()).writeFrame(eq(streamId), any(ByteBuf.class), anyInt(), verify(frameWriter, never()).writeFrame(eq(streamId), any(ByteBuf.class), anyInt(),
anyBoolean(), anyBoolean(), anyBoolean()); anyBoolean(), anyBoolean());
} }
private void captureWrite(int streamId, ArgumentCaptor<ByteBuf> captor, boolean endStream) { private void captureWrite(int streamId, ArgumentCaptor<ByteBuf> captor, boolean endStream) {
verify(frameWriter).writeFrame(eq(streamId), captor.capture(), eq(0), eq(endStream), verify(frameWriter).writeFrame(eq(streamId), captor.capture(), eq(0), eq(endStream),
eq(false), eq(false)); eq(false));
} }
private void setPriority(int stream, int parent, int weight, boolean exclusive) private void setPriority(int stream, int parent, int weight, boolean exclusive)

View File

@ -442,15 +442,15 @@ public class DelegatingHttp2ConnectionHandlerTest {
@Test @Test
public void dataWriteAfterGoAwayShouldFail() throws Exception { public void dataWriteAfterGoAwayShouldFail() throws Exception {
when(connection.isGoAway()).thenReturn(true); when(connection.isGoAway()).thenReturn(true);
ChannelFuture future = handler.writeData(ctx, promise, STREAM_ID, dummyData(), 0, false, false, false); ChannelFuture future = handler.writeData(ctx, promise, STREAM_ID, dummyData(), 0, false, false);
assertTrue(future.awaitUninterruptibly().cause() instanceof Http2Exception); assertTrue(future.awaitUninterruptibly().cause() instanceof Http2Exception);
} }
@Test @Test
public void dataWriteShouldSucceed() throws Exception { public void dataWriteShouldSucceed() throws Exception {
handler.writeData(ctx, promise, STREAM_ID, dummyData(), 0, false, false, false); handler.writeData(ctx, promise, STREAM_ID, dummyData(), 0, false, false);
verify(outboundFlow).sendFlowControlled(eq(STREAM_ID), eq(dummyData()), eq(0), eq(false), verify(outboundFlow).sendFlowControlled(eq(STREAM_ID), eq(dummyData()), eq(0), eq(false),
eq(false), eq(false), any(Http2OutboundFlowController.FrameWriter.class)); eq(false), any(Http2OutboundFlowController.FrameWriter.class));
} }
@Test @Test

View File

@ -121,7 +121,7 @@ public class Http2ConnectionRoundtripTest {
http2Client.writePing(ctx(), newPromise(), Unpooled.copiedBuffer(pingMsg.getBytes())); http2Client.writePing(ctx(), newPromise(), Unpooled.copiedBuffer(pingMsg.getBytes()));
http2Client.writeData( http2Client.writeData(
ctx(), newPromise(), nextStream, ctx(), newPromise(), nextStream,
Unpooled.copiedBuffer(text.getBytes()), 0, true, true, false); Unpooled.copiedBuffer(text.getBytes()), 0, true, true);
} }
} }
}); });

View File

@ -112,6 +112,6 @@ public class HelloWorldHttp2Handler extends AbstractHttp2ConnectionHandler {
Http2Headers headers = DefaultHttp2Headers.newBuilder().status("200").build(); Http2Headers headers = DefaultHttp2Headers.newBuilder().status("200").build();
writeHeaders(ctx(), ctx().newPromise(), streamId, headers, 0, false, false); writeHeaders(ctx(), ctx().newPromise(), streamId, headers, 0, false, false);
writeData(ctx(), ctx().newPromise(), streamId, payload, 0, true, true, false); writeData(ctx(), ctx().newPromise(), streamId, payload, 0, true, true);
} }
} }