You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by se...@apache.org on 2014/08/08 00:50:31 UTC
svn commit: r1616617 - in /commons/proper/net/trunk/src: changes/changes.xml
main/java/org/apache/commons/net/io/Util.java
test/java/org/apache/commons/net/util/UtilTest.java
Author: sebb
Date: Thu Aug 7 22:50:31 2014
New Revision: 1616617
URL: http://svn.apache.org/r1616617
Log:
NET-550 Default FTPClient bufferSize results in very slow retrieve transfers
Fix code in Util#copyStream (also copyReader) that failed to use the proper default for buffer size 0
Modified:
commons/proper/net/trunk/src/changes/changes.xml
commons/proper/net/trunk/src/main/java/org/apache/commons/net/io/Util.java
commons/proper/net/trunk/src/test/java/org/apache/commons/net/util/UtilTest.java
Modified: commons/proper/net/trunk/src/changes/changes.xml
URL: http://svn.apache.org/viewvc/commons/proper/net/trunk/src/changes/changes.xml?rev=1616617&r1=1616616&r2=1616617&view=diff
==============================================================================
--- commons/proper/net/trunk/src/changes/changes.xml [utf-8] (original)
+++ commons/proper/net/trunk/src/changes/changes.xml [utf-8] Thu Aug 7 22:50:31 2014
@@ -68,6 +68,10 @@ This is mainly a bug-fix release. See fu
IMAPExportMbox (example app) allows IMAP folders to be exported into an mbox file.
This is the inverse of the IMAPImportMbox example added previously
">
+ <action issue="NET-550" type="fix" dev="sebb" due-to="Geoffrey Hardy">
+ Default FTPClient bufferSize results in very slow retrieve transfers
+ Fix code in Util#copyStream (also copyReader) that failed to use the proper default for buffer size 0
+ </action>
<action issue="NET-551" type="fix" dev="sebb">
Util copyReader calls CopyStreamListener.bytesTransferred with the incorrect value for bytesTransferred
</action>
Modified: commons/proper/net/trunk/src/main/java/org/apache/commons/net/io/Util.java
URL: http://svn.apache.org/viewvc/commons/proper/net/trunk/src/main/java/org/apache/commons/net/io/Util.java?rev=1616617&r1=1616616&r2=1616617&view=diff
==============================================================================
--- commons/proper/net/trunk/src/main/java/org/apache/commons/net/io/Util.java (original)
+++ commons/proper/net/trunk/src/main/java/org/apache/commons/net/io/Util.java Thu Aug 7 22:50:31 2014
@@ -94,7 +94,7 @@ public final class Util
{
int numBytes;
long total = 0;
- byte[] buffer = new byte[bufferSize >= 0 ? bufferSize : DEFAULT_COPY_BUFFER_SIZE];
+ byte[] buffer = new byte[bufferSize > 0 ? bufferSize : DEFAULT_COPY_BUFFER_SIZE];
try
{
@@ -267,7 +267,7 @@ public final class Util
{
int numChars;
long total = 0;
- char[] buffer = new char[bufferSize >= 0 ? bufferSize : DEFAULT_COPY_BUFFER_SIZE];
+ char[] buffer = new char[bufferSize > 0 ? bufferSize : DEFAULT_COPY_BUFFER_SIZE];
try
{
Modified: commons/proper/net/trunk/src/test/java/org/apache/commons/net/util/UtilTest.java
URL: http://svn.apache.org/viewvc/commons/proper/net/trunk/src/test/java/org/apache/commons/net/util/UtilTest.java?rev=1616617&r1=1616616&r2=1616617&view=diff
==============================================================================
--- commons/proper/net/trunk/src/test/java/org/apache/commons/net/util/UtilTest.java (original)
+++ commons/proper/net/trunk/src/test/java/org/apache/commons/net/util/UtilTest.java Thu Aug 7 22:50:31 2014
@@ -30,6 +30,7 @@ import java.io.Writer;
import java.net.Socket;
import org.apache.commons.net.io.CopyStreamEvent;
+import org.apache.commons.net.io.CopyStreamException;
import org.apache.commons.net.io.CopyStreamListener;
import org.apache.commons.net.io.Util;
import org.junit.Assert;
@@ -90,6 +91,78 @@ public class UtilTest {
Util.copyStream(src, dst, bufferSize, streamSize, new CSL(1,1,streamSize));
}
+ @Test
+ public void testNET550_Reader() throws Exception {
+ final char[] buff = new char[]{'a', 'b', 'c', 'd'}; // must be multiple of 2
+ final int bufflen = buff.length;
+ { // Check buffer size 1 processes in chunks of 1
+ Reader rdr = new CharArrayReader(buff);
+ final CSLtotal listener = new CSLtotal(bufflen, 1);
+ Util.copyReader(rdr, dest, 1, 0, listener); // buffer size 1
+ listener.checkExpected();
+ }
+ { // Check bufsize 2 uses chunks of 2
+ Reader rdr = new CharArrayReader(buff);
+ final CSLtotal listener = new CSLtotal(bufflen, 2);
+ Util.copyReader(rdr, dest, 2, 0, listener); // buffer size 2
+ listener.checkExpected();
+ }
+ { // Check bigger size reads the lot
+ Reader rdr = new CharArrayReader(buff);
+ final CSLtotal listener = new CSLtotal(bufflen, bufflen);
+ Util.copyReader(rdr, dest, 20, 0, listener); // buffer size 20
+ listener.checkExpected();
+ }
+ { // Check negative size reads reads full amount
+ Reader rdr = new CharArrayReader(buff);
+ final CSLtotal listener = new CSLtotal(bufflen, bufflen);
+ Util.copyReader(rdr, dest, -1, 0, listener); // buffer size -1
+ listener.checkExpected();
+ }
+ { // Check zero size reads reads full amount
+ Reader rdr = new CharArrayReader(buff);
+ final CSLtotal listener = new CSLtotal(bufflen, bufflen);
+ Util.copyReader(rdr, dest, 0, 0, listener); // buffer size -1
+ listener.checkExpected();
+ }
+ }
+
+ @Test
+ public void testNET550_Stream() throws Exception {
+ final byte[] buff = new byte[]{'a', 'b', 'c', 'd'}; // must be multiple of 2
+ final int bufflen = buff.length;
+ { // Check buffer size 1 processes in chunks of 1
+ InputStream is = new ByteArrayInputStream(buff);
+ final CSLtotal listener = new CSLtotal(bufflen, 1);
+ Util.copyStream(is, dst, 1, 0, listener); // buffer size 1
+ listener.checkExpected();
+ }
+ { // Check bufsize 2 uses chunks of 2
+ InputStream is = new ByteArrayInputStream(buff);
+ final CSLtotal listener = new CSLtotal(bufflen, 2);
+ Util.copyStream(is, dst, 2, 0, listener); // buffer size 2
+ listener.checkExpected();
+ }
+ { // Check bigger size reads the lot
+ InputStream is = new ByteArrayInputStream(buff);
+ final CSLtotal listener = new CSLtotal(bufflen, bufflen);
+ Util.copyStream(is, dst, 20, 0, listener); // buffer size 20
+ listener.checkExpected();
+ }
+ { // Check negative size reads reads full amount
+ InputStream is = new ByteArrayInputStream(buff);
+ final CSLtotal listener = new CSLtotal(bufflen, bufflen);
+ Util.copyStream(is, dst, -1, 0, listener); // buffer size -1
+ listener.checkExpected();
+ }
+ { // Check zero size reads reads full amount
+ InputStream is = new ByteArrayInputStream(buff);
+ final CSLtotal listener = new CSLtotal(bufflen, bufflen);
+ Util.copyStream(is, dst, 0, 0, listener); // buffer size -1
+ listener.checkExpected();
+ }
+ }
+
static class CSL implements CopyStreamListener {
final long expectedTotal;
@@ -106,7 +179,35 @@ public class UtilTest {
public void bytesTransferred(long totalBytesTransferred, int bytesTransferred, long streamSize) {
Assert.assertEquals("Wrong total", expectedTotal, totalBytesTransferred);
Assert.assertEquals("Wrong streamSize", expectedSize, streamSize);
- Assert.assertEquals("Wrong bytes", expectedBytes, bytesTransferred);
+ Assert.assertEquals("Wrong bytes", expectedBytes, bytesTransferred);
+ }
+
+ }
+
+ // Class to check overall counts as well as batch size
+ static class CSLtotal implements CopyStreamListener {
+
+ final long expectedTotal;
+ final long expectedBytes;
+ volatile long totalBytesTransferredTotal;
+ volatile long bytesTransferredTotal;
+
+ CSLtotal(long totalBytesTransferred, long bytesTransferred) {
+ this.expectedTotal = totalBytesTransferred;
+ this.expectedBytes = bytesTransferred;
+ }
+ public void bytesTransferred(CopyStreamEvent event) {
+ }
+
+ public void bytesTransferred(long totalBytesTransferred, int bytesTransferred, long streamSize) {
+ Assert.assertEquals("Wrong bytes", expectedBytes, bytesTransferred);
+ this.totalBytesTransferredTotal = totalBytesTransferred;
+ this.bytesTransferredTotal += bytesTransferred;
+ }
+
+ void checkExpected() {
+ Assert.assertEquals("Wrong totalBytesTransferred total", expectedTotal, totalBytesTransferredTotal);
+ Assert.assertEquals("Total should equal sum of parts", totalBytesTransferredTotal, bytesTransferredTotal);
}
}