Skip to content

Commit c73e67a

Browse files
committed
Add test validating write without onWritePossible is buffered
Add a new test 'writeBytes_withoutOnWritePossible_isBuffered' that: - Calls writeBytes, checks it executes - Calls writeBytes AGAIN without onWritePossible in between - Asserts second write is NOT executed (buffered) - Then calls onWritePossible and asserts buffered write drains This validates the core Tomcat fix: writeBytes sets readyAndDrained=false, so subsequent writes must go through onWritePossible callback. Also update existing tests' comments to clarify buffering is triggered by readyAndDrained=false (from first write), not by isReady being false. Addressed Copilot review on PR #12790
1 parent d79bdb6 commit c73e67a

1 file changed

Lines changed: 52 additions & 12 deletions

File tree

servlet/src/test/java/io/grpc/servlet/AsyncServletOutputStreamWriterTest.java

Lines changed: 52 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -33,19 +33,59 @@
3333
/**
3434
* Unit test for {@link AsyncServletOutputStreamWriter} with a mock isReady supplier.
3535
* Tests two scenarios: (1) writes with isReady always true, where onWritePossible()
36-
* is called between writes; (2) writes with isReady becoming false, where buffered
37-
* writes are only drained when onWritePossible() is called.
36+
* is called between writes; (2) writes with readyAndDrained=false (triggered by first
37+
* write), where subsequent writes are buffered until onWritePossible() drains them.
3838
*/
3939
@RunWith(JUnit4.class)
4040
public class AsyncServletOutputStreamWriterTest {
4141

42+
/**
43+
* Test that a write without an intervening onWritePossible() is buffered and
44+
* does not execute immediately. This validates the Tomcat fix: writeBytes sets
45+
* readyAndDrained=false so subsequent writes go through the container callback.
46+
*/
47+
@Test
48+
public void writeBytes_withoutOnWritePossible_isBuffered() throws IOException {
49+
AtomicBoolean isReady = new AtomicBoolean(true);
50+
List<String> actions = new ArrayList<>();
51+
52+
BiFunction<byte[], Integer, ActionItem> writeAction =
53+
(bytes, numBytes) -> () -> {
54+
actions.add("write");
55+
};
56+
57+
ActionItem flushAction = () -> {};
58+
59+
ActionItem completeAction = () -> {};
60+
61+
BooleanSupplier isReadySupplier = () -> isReady.get();
62+
63+
AsyncServletOutputStreamWriter writer =
64+
new AsyncServletOutputStreamWriter(writeAction, flushAction, completeAction, isReadySupplier, new Log() {});
65+
66+
// Initial onWritePossible to set readyAndDrained=true
67+
writer.onWritePossible();
68+
69+
// First write - goes direct, readyAndDrained becomes false
70+
byte[] data1 = new byte[]{1};
71+
writer.writeBytes(data1, 1);
72+
assertEquals("First write should execute", 1, actions.size());
73+
74+
// Second write without onWritePossible - should be buffered since readyAndDrained=false
75+
byte[] data2 = new byte[]{2};
76+
writer.writeBytes(data2, 1);
77+
78+
// Without onWritePossible, second write should be buffered, not executed
79+
assertEquals("Second write should be buffered until onWritePossible", 1, actions.size());
80+
81+
// onWritePossible drains the buffered write
82+
writer.onWritePossible();
83+
assertEquals("Buffered write should drain after onWritePossible", 2, actions.size());
84+
}
85+
4286
/**
4387
* Test that multiple consecutive writes succeed when isReady() always returns true
4488
* and the container calls onWritePossible() between writes.
45-
*
46-
* <p>Each writeBytes() call is followed by onWritePossible() to drain any buffered
47-
* writes. This validates that the writer can handle the Tomcat-style callback pattern
48-
* where isReady() stays true but onWritePossible() fires between writes.
4989
*/
5090
@Test
5191
public void writeBytes_onWritePossibleBetweenWrites_succeeds() throws IOException {
@@ -83,8 +123,8 @@ public void writeBytes_onWritePossibleBetweenWrites_succeeds() throws IOExceptio
83123
}
84124

85125
/**
86-
* Test that when isReady() returns false, writes are buffered and only drain
87-
* when onWritePossible() is called.
126+
* Test that when isReady() returns false after a direct write, subsequent writes
127+
* are buffered and only drain when onWritePossible() is called.
88128
*/
89129
@Test
90130
public void writeBytes_isReadyFalse_buffersUntilOnWritePossible() throws IOException {
@@ -110,21 +150,21 @@ public void writeBytes_isReadyFalse_buffersUntilOnWritePossible() throws IOExcep
110150
// Initial onWritePossible to set readyAndDrained=true
111151
writer.onWritePossible();
112152

113-
// First write - isReady is true, goes direct, readyAndDrained=false
153+
// First write - goes direct, readyAndDrained becomes false
114154
byte[] data1 = new byte[]{1};
115155
writer.writeBytes(data1, 1);
116156

117-
// Simulate isReady becoming false (container buffer full)
157+
// After first write, readyAndDrained=false, so any subsequent write is buffered
118158
isReady.set(false);
119159

120-
// Second write - should be buffered since isReady=false
160+
// Second write - should be buffered since readyAndDrained=false
121161
byte[] data2 = new byte[]{2};
122162
writer.writeBytes(data2, 1);
123163

124164
// Only the first write should have executed
125165
assertEquals("First write should complete, second buffered", 1, actions.size());
126166

127-
// Container calls onWritePossible to signal readiness
167+
// Container calls onWritePossible to drain buffered writes
128168
isReady.set(true);
129169
writer.onWritePossible();
130170

0 commit comments

Comments
 (0)