garydgregory commented on code in PR #625:
URL: https://github.com/apache/commons-csv/pull/625#discussion_r3614639072


##########
src/test/java/org/apache/commons/csv/CSVParserTest.java:
##########
@@ -1709,6 +1709,47 @@ void testParseWithDelimiterStringWithEscape() throws 
IOException {
         }
     }
 
+    @Test
+    void testParseWithDelimiterStringFromChunkedReader() throws IOException {
+        // A reader that hands out one character at a time and never reports 
itself ready, like a socket or pipe.
+        final Reader chunked = new Reader() {

Review Comment:
   If this is the same as the behavior of the `ChunkedReader` in the other 
test, then refactor for reuse.



##########
src/test/java/org/apache/commons/csv/ExtendedBufferedReaderTest.java:
##########
@@ -240,4 +240,72 @@ void testReadLookahead2() throws Exception {
             assertEquals('d', br.getLastChar());
         }
     }
+
+    @Test
+    void testReadAndPeekArrayFromChunkedReader() throws Exception {
+        try (ExtendedBufferedReader br = new ExtendedBufferedReader(new 
ChunkedReader("abcdef"))) {
+            final char[] peeked = new char[3];
+            assertEquals(3, br.peek(peeked));
+            assertArrayEquals(new char[] { 'a', 'b', 'c' }, peeked);
+            final char[] read = new char[3];
+            assertEquals(3, br.read(read, 0, 3));
+            assertArrayEquals(new char[] { 'a', 'b', 'c' }, read);
+        }
+    }
+
+    @Test
+    void testReadArrayPastEndOfChunkedReader() throws Exception {
+        try (ExtendedBufferedReader br = new ExtendedBufferedReader(new 
ChunkedReader("ab"))) {
+            final char[] read = new char[4];
+            assertEquals(2, br.read(read, 0, 4));
+            assertEquals('a', read[0]);
+            assertEquals('b', read[1]);
+            assertEquals(EOF, br.read(read, 0, 4));
+        }
+    }
+
+    @Test
+    void testPeekArrayPastEndOfChunkedReader() throws Exception {
+        try (ExtendedBufferedReader br = new ExtendedBufferedReader(new 
ChunkedReader("ab"))) {
+            final char[] peeked = new char[4];
+            assertEquals(2, br.peek(peeked));
+            assertEquals('a', peeked[0]);
+            assertEquals('b', peeked[1]);
+        }
+    }
+
+    /**
+     * A reader that returns one character per call and never reports itself 
ready, like a socket or pipe that delivers data in chunks.
+     */
+    private static final class ChunkedReader extends java.io.Reader {

Review Comment:
   Would it be simpler to extend `StringReader` and override those methods?



##########
src/main/java/org/apache/commons/csv/ExtendedBufferedReader.java:
##########
@@ -244,7 +275,16 @@ public int read(final char[] buf, final int offset, final 
int length) throws IOE
         if (length == 0) {
             return 0;
         }
-        final int len = super.read(buf, offset, length);
+        int len = super.read(buf, offset, length);
+        // The underlying buffered reader stops early once the source reports 
it is not ready, so a stream that delivers data in chunks (a socket or a pipe)
+        // yields a short read. Callers match multi-character sequences 
against this buffer, so keep reading until it is full or the source is 
exhausted.

Review Comment:
   Isn't there still a risk that even filling to the end of the buffer breaks 
up multi-character sequences?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to