HTTP/2 Remove RemoteFlowController.streamWritten

Motivation:
RemoteFlowController.streamWritten is not currently required. We should remove it to keep interfaces minimal.

Modifications:
- Remove RemoteFlowController.streamWritten

Result:
1 Less method in RemoteFlowController interface.
Fixes https://github.com/netty/netty/issues/4600
This commit is contained in:
Scott Mitchell 2015-12-18 13:35:37 -08:00
parent e4cb163be2
commit 7b2f55ec2f
3 changed files with 9 additions and 65 deletions

View File

@ -205,7 +205,7 @@ public class DefaultHttp2RemoteFlowController implements Http2RemoteFlowControll
@Override @Override
public void listener(Listener listener) { public void listener(Listener listener) {
monitor = listener == null ? new DefaultWritabilityMonitor() : new ListenerWritabilityMonitor(listener); monitor = listener == null ? new WritabilityMonitor() : new ListenerWritabilityMonitor(listener);
} }
@Override @Override
@ -627,13 +627,14 @@ public class DefaultHttp2RemoteFlowController implements Http2RemoteFlowControll
/** /**
* Abstract class which provides common functionality for writability monitor implementations. * Abstract class which provides common functionality for writability monitor implementations.
*/ */
private abstract class WritabilityMonitor { private class WritabilityMonitor {
private long totalPendingBytes; private long totalPendingBytes;
private final Writer writer; private final Writer writer = new StreamByteDistributor.Writer() {
@Override
protected WritabilityMonitor(Writer writer) { public void write(Http2Stream stream, int numBytes) {
this.writer = writer; state(stream).writeAllocatedBytes(numBytes);
} }
};
/** /**
* Called when the writability of the underlying channel changes. * Called when the writability of the underlying channel changes.
@ -728,20 +729,6 @@ public class DefaultHttp2RemoteFlowController implements Http2RemoteFlowControll
} }
} }
/**
* Provides no notification or tracking of writablity changes.
*/
private final class DefaultWritabilityMonitor extends WritabilityMonitor {
DefaultWritabilityMonitor() {
super(new StreamByteDistributor.Writer() {
@Override
public void write(Http2Stream stream, int numBytes) {
state(stream).writeAllocatedBytes(numBytes);
}
});
}
}
/** /**
* Writability of a {@code stream} is calculated using the following: * Writability of a {@code stream} is calculated using the following:
* <pre> * <pre>
@ -763,17 +750,7 @@ public class DefaultHttp2RemoteFlowController implements Http2RemoteFlowControll
} }
}; };
ListenerWritabilityMonitor(final Listener listener) { ListenerWritabilityMonitor(Listener listener) {
super(new StreamByteDistributor.Writer() {
@Override
public void write(Http2Stream stream, int numBytes) {
AbstractState state = state(stream);
int written = state.writeAllocatedBytes(numBytes);
if (written != -1) {
listener.streamWritten(state.stream(), written);
}
}
});
this.listener = listener; this.listener = listener;
} }

View File

@ -138,18 +138,6 @@ public interface Http2RemoteFlowController extends Http2FlowController {
* Listener to the number of flow-controlled bytes written per stream. * Listener to the number of flow-controlled bytes written per stream.
*/ */
interface Listener { interface Listener {
/**
* Report the number of {@code writtenBytes} for a {@code stream}. Called after the
* flow-controller has flushed bytes for the given stream.
* <p>
* This method should not throw. Any thrown exceptions are considered a programming error and are ignored.
* @param stream that had bytes written.
* @param writtenBytes the number of bytes written for a stream, can be 0 in the case of an
* empty DATA frame.
*/
void streamWritten(Http2Stream stream, int writtenBytes);
/** /**
* Notification that {@link Http2RemoteFlowController#isWritable(Http2Stream)} has changed for {@code stream}. * Notification that {@link Http2RemoteFlowController#isWritable(Http2Stream)} has changed for {@code stream}.
* <p> * <p>

View File

@ -170,7 +170,6 @@ public abstract class DefaultHttp2RemoteFlowControllerTest {
verifyZeroInteractions(listener); verifyZeroInteractions(listener);
controller.writePendingBytes(); controller.writePendingBytes();
data.assertFullyWritten(); data.assertFullyWritten();
verify(listener, times(1)).streamWritten(stream(STREAM_A), 5);
verifyZeroInteractions(listener); verifyZeroInteractions(listener);
} }
@ -181,7 +180,6 @@ public abstract class DefaultHttp2RemoteFlowControllerTest {
data.assertNotWritten(); data.assertNotWritten();
controller.writePendingBytes(); controller.writePendingBytes();
data.assertFullyWritten(); data.assertFullyWritten();
verify(listener, times(1)).streamWritten(stream(STREAM_A), 0);
verifyZeroInteractions(listener); verifyZeroInteractions(listener);
} }
@ -210,7 +208,6 @@ public abstract class DefaultHttp2RemoteFlowControllerTest {
controller.writePendingBytes(); controller.writePendingBytes();
data1.assertFullyWritten(); data1.assertFullyWritten();
data2.assertNotWritten(); data2.assertNotWritten();
verify(listener, times(1)).streamWritten(stream(STREAM_A), 15);
verify(listener, times(1)).writabilityChanged(stream(STREAM_A)); verify(listener, times(1)).writabilityChanged(stream(STREAM_A));
assertFalse(controller.isWritable(stream(STREAM_A))); assertFalse(controller.isWritable(stream(STREAM_A)));
} }
@ -267,7 +264,6 @@ public abstract class DefaultHttp2RemoteFlowControllerTest {
controller.writePendingBytes(); controller.writePendingBytes();
// Verify that a partial frame of 5 remains to be sent // Verify that a partial frame of 5 remains to be sent
data.assertPartiallyWritten(5); data.assertPartiallyWritten(5);
verify(listener, times(1)).streamWritten(stream(STREAM_A), 5);
verify(listener, times(1)).writabilityChanged(stream(STREAM_A)); verify(listener, times(1)).writabilityChanged(stream(STREAM_A));
assertFalse(controller.isWritable(stream(STREAM_A))); assertFalse(controller.isWritable(stream(STREAM_A)));
verifyNoMoreInteractions(listener); verifyNoMoreInteractions(listener);
@ -286,7 +282,6 @@ public abstract class DefaultHttp2RemoteFlowControllerTest {
controller.writePendingBytes(); controller.writePendingBytes();
data.assertPartiallyWritten(10); data.assertPartiallyWritten(10);
moreData.assertNotWritten(); moreData.assertNotWritten();
verify(listener, times(1)).streamWritten(stream(STREAM_A), 10);
verify(listener, times(1)).writabilityChanged(stream(STREAM_A)); verify(listener, times(1)).writabilityChanged(stream(STREAM_A));
assertFalse(controller.isWritable(stream(STREAM_A))); assertFalse(controller.isWritable(stream(STREAM_A)));
reset(listener); reset(listener);
@ -302,7 +297,6 @@ public abstract class DefaultHttp2RemoteFlowControllerTest {
data.assertFullyWritten(); data.assertFullyWritten();
moreData.assertPartiallyWritten(5); moreData.assertPartiallyWritten(5);
verify(listener, times(1)).streamWritten(stream(STREAM_A), 15);
verify(listener, never()).writabilityChanged(stream(STREAM_A)); verify(listener, never()).writabilityChanged(stream(STREAM_A));
assertFalse(controller.isWritable(stream(STREAM_A))); assertFalse(controller.isWritable(stream(STREAM_A)));
@ -331,7 +325,6 @@ public abstract class DefaultHttp2RemoteFlowControllerTest {
// Verify that the entire frame was sent. // Verify that the entire frame was sent.
controller.initialWindowSize(10); controller.initialWindowSize(10);
data.assertFullyWritten(); data.assertFullyWritten();
verify(listener, times(1)).streamWritten(stream(STREAM_A), 10);
assertWritabilityChanged(0, false); assertWritabilityChanged(0, false);
} }
@ -356,7 +349,6 @@ public abstract class DefaultHttp2RemoteFlowControllerTest {
dataA.assertPartiallyWritten(8); dataA.assertPartiallyWritten(8);
assertEquals(65527, window(STREAM_A)); assertEquals(65527, window(STREAM_A));
assertEquals(0, window(CONNECTION_STREAM_ID)); assertEquals(0, window(CONNECTION_STREAM_ID));
verify(listener, times(1)).streamWritten(stream(STREAM_A), 8);
assertWritabilityChanged(0, false); assertWritabilityChanged(0, false);
reset(listener); reset(listener);
@ -376,11 +368,9 @@ public abstract class DefaultHttp2RemoteFlowControllerTest {
// Verify the rest of A is written. // Verify the rest of A is written.
dataA.assertFullyWritten(); dataA.assertFullyWritten();
assertEquals(65525, window(STREAM_A)); assertEquals(65525, window(STREAM_A));
verify(listener, times(1)).streamWritten(stream(STREAM_A), 2);
dataB.assertFullyWritten(); dataB.assertFullyWritten();
assertEquals(65525, window(STREAM_B)); assertEquals(65525, window(STREAM_B));
verify(listener, times(1)).streamWritten(stream(STREAM_B), 10);
verifyNoMoreInteractions(listener); verifyNoMoreInteractions(listener);
} }
@ -399,7 +389,6 @@ public abstract class DefaultHttp2RemoteFlowControllerTest {
sendData(STREAM_A, data1); sendData(STREAM_A, data1);
controller.writePendingBytes(); controller.writePendingBytes();
data1.assertFullyWritten(); data1.assertFullyWritten();
verify(listener, times(1)).streamWritten(stream(STREAM_A), 20);
assertTrue(window(CONNECTION_STREAM_ID) > 0); assertTrue(window(CONNECTION_STREAM_ID) > 0);
verify(listener, times(1)).writabilityChanged(stream(STREAM_A)); verify(listener, times(1)).writabilityChanged(stream(STREAM_A));
verify(listener, never()).writabilityChanged(stream(STREAM_B)); verify(listener, never()).writabilityChanged(stream(STREAM_B));
@ -513,8 +502,6 @@ public abstract class DefaultHttp2RemoteFlowControllerTest {
data.assertFullyWritten(); data.assertFullyWritten();
data2.assertFullyWritten(); data2.assertFullyWritten();
verify(listener, times(1)).streamWritten(stream(STREAM_A), 10);
} }
@Test @Test
@ -540,7 +527,6 @@ public abstract class DefaultHttp2RemoteFlowControllerTest {
assertTrue(controller.isWritable(stream(STREAM_D))); assertTrue(controller.isWritable(stream(STREAM_D)));
data.assertPartiallyWritten(5); data.assertPartiallyWritten(5);
verify(listener, times(1)).streamWritten(stream(STREAM_A), 5);
} }
@Test @Test
@ -565,7 +551,6 @@ public abstract class DefaultHttp2RemoteFlowControllerTest {
controller.writePendingBytes(); controller.writePendingBytes();
data.assertFullyWritten(); data.assertFullyWritten();
verify(listener, times(1)).streamWritten(stream(STREAM_A), 10);
assertWritabilityChanged(0, false); assertWritabilityChanged(0, false);
assertEquals(0, window(CONNECTION_STREAM_ID)); assertEquals(0, window(CONNECTION_STREAM_ID));
assertEquals(DEFAULT_WINDOW_SIZE - 10, window(STREAM_A)); assertEquals(DEFAULT_WINDOW_SIZE - 10, window(STREAM_A));
@ -594,7 +579,6 @@ public abstract class DefaultHttp2RemoteFlowControllerTest {
controller.writePendingBytes(); controller.writePendingBytes();
data.assertPartiallyWritten(5); data.assertPartiallyWritten(5);
verify(listener, times(1)).streamWritten(stream(STREAM_A), 5);
assertWritabilityChanged(0, false); assertWritabilityChanged(0, false);
assertEquals(0, window(CONNECTION_STREAM_ID)); assertEquals(0, window(CONNECTION_STREAM_ID));
assertEquals(DEFAULT_WINDOW_SIZE - 5, window(STREAM_A)); assertEquals(DEFAULT_WINDOW_SIZE - 5, window(STREAM_A));
@ -637,7 +621,6 @@ public abstract class DefaultHttp2RemoteFlowControllerTest {
data.assertNotWritten(); data.assertNotWritten();
controller.writePendingBytes(); controller.writePendingBytes();
data.assertFullyWritten(); data.assertFullyWritten();
verify(listener, times(1)).streamWritten(stream(STREAM_A), 10);
verify(listener, never()).writabilityChanged(stream(STREAM_A)); verify(listener, never()).writabilityChanged(stream(STREAM_A));
verify(listener, never()).writabilityChanged(stream(STREAM_B)); verify(listener, never()).writabilityChanged(stream(STREAM_B));
verify(listener, never()).writabilityChanged(stream(STREAM_C)); verify(listener, never()).writabilityChanged(stream(STREAM_C));
@ -687,7 +670,6 @@ public abstract class DefaultHttp2RemoteFlowControllerTest {
data.assertNotWritten(); data.assertNotWritten();
controller.writePendingBytes(); controller.writePendingBytes();
data.assertPartiallyWritten(5); data.assertPartiallyWritten(5);
verify(listener, times(1)).streamWritten(stream(STREAM_A), 5);
assertEquals(DEFAULT_WINDOW_SIZE - 5, window(CONNECTION_STREAM_ID)); assertEquals(DEFAULT_WINDOW_SIZE - 5, window(CONNECTION_STREAM_ID));
assertEquals(0, window(STREAM_A)); assertEquals(0, window(STREAM_A));
assertEquals(DEFAULT_WINDOW_SIZE, window(STREAM_B)); assertEquals(DEFAULT_WINDOW_SIZE, window(STREAM_B));
@ -717,7 +699,6 @@ public abstract class DefaultHttp2RemoteFlowControllerTest {
verify(flowControlled, never()).writeComplete(); verify(flowControlled, never()).writeComplete();
assertEquals(90, windowBefore - window(STREAM_A)); assertEquals(90, windowBefore - window(STREAM_A));
verify(listener, times(1)).streamWritten(stream(STREAM_A), 90);
assertWritabilityChanged(0, true); assertWritabilityChanged(0, true);
} }
@ -794,7 +775,6 @@ public abstract class DefaultHttp2RemoteFlowControllerTest {
verify(flowControlled).writeComplete(); verify(flowControlled).writeComplete();
assertEquals(150, windowBefore - window(STREAM_A)); assertEquals(150, windowBefore - window(STREAM_A));
verify(listener, times(1)).streamWritten(stream(STREAM_A), 150);
assertWritabilityChanged(0, true); assertWritabilityChanged(0, true);
} }
@ -820,7 +800,6 @@ public abstract class DefaultHttp2RemoteFlowControllerTest {
verify(flowControlled).write(any(ChannelHandlerContext.class), anyInt()); verify(flowControlled).write(any(ChannelHandlerContext.class), anyInt());
verify(flowControlled).error(any(ChannelHandlerContext.class), any(Throwable.class)); verify(flowControlled).error(any(ChannelHandlerContext.class), any(Throwable.class));
verify(flowControlled, never()).writeComplete(); verify(flowControlled, never()).writeComplete();
verify(listener, times(1)).streamWritten(stream(STREAM_A), 0);
verify(listener, times(1)).writabilityChanged(stream(STREAM_A)); verify(listener, times(1)).writabilityChanged(stream(STREAM_A));
verify(listener, never()).writabilityChanged(stream(STREAM_B)); verify(listener, never()).writabilityChanged(stream(STREAM_B));
verify(listener, never()).writabilityChanged(stream(STREAM_C)); verify(listener, never()).writabilityChanged(stream(STREAM_C));