You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@curator.apache.org by ca...@apache.org on 2018/12/10 03:44:00 UTC
[2/6] curator git commit: Add tests
Add tests
Project: http://git-wip-us.apache.org/repos/asf/curator/repo
Commit: http://git-wip-us.apache.org/repos/asf/curator/commit/7467e596
Tree: http://git-wip-us.apache.org/repos/asf/curator/tree/7467e596
Diff: http://git-wip-us.apache.org/repos/asf/curator/diff/7467e596
Branch: refs/heads/master
Commit: 7467e5966fea5cabfa47d189688a6b0bd88a7a6d
Parents: 6457f26
Author: Roman Leventov <le...@gmail.com>
Authored: Fri Nov 30 20:22:51 2018 +0100
Committer: Roman Leventov <le...@gmail.com>
Committed: Fri Nov 30 20:22:51 2018 +0100
----------------------------------------------------------------------
.../framework/imps/GzipCompressionProvider.java | 22 +++++++-----
.../imps/GzipCompressionProviderTest.java | 37 ++++++++++++++++++++
2 files changed, 50 insertions(+), 9 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/curator/blob/7467e596/curator-framework/src/main/java/org/apache/curator/framework/imps/GzipCompressionProvider.java
----------------------------------------------------------------------
diff --git a/curator-framework/src/main/java/org/apache/curator/framework/imps/GzipCompressionProvider.java b/curator-framework/src/main/java/org/apache/curator/framework/imps/GzipCompressionProvider.java
index 74d0347..fa357d2 100644
--- a/curator-framework/src/main/java/org/apache/curator/framework/imps/GzipCompressionProvider.java
+++ b/curator-framework/src/main/java/org/apache/curator/framework/imps/GzipCompressionProvider.java
@@ -70,11 +70,14 @@ public class GzipCompressionProvider implements CompressionProvider
/** 32-bit CRC and uncompressed data size */
private static final int GZIP_TRAILER_SIZE = Integer.BYTES + Integer.BYTES;
+ /** DEFLATE doesn't produce shorter compressed data */
+ private static final int MIN_COMPRESSED_DATA_SIZE = 2;
+
/**
* Since Deflaters and Inflaters are acquired and returned to the pools in try-finally blocks that are free of
* blocking calls themselves, it's not expected that the number of objects in the pools could exceed the number of
- * hardware threads on the machine much. Therefore it's accepted to have simple pools of strongly-referenced
- * objects.
+ * hardware threads on the machine much. Therefore it's accepted to have simple "ever-growing" (in fact, no) pools
+ * of strongly-referenced objects.
*/
private static final ConcurrentLinkedQueue<Deflater> DEFLATER_POOL = new ConcurrentLinkedQueue<>();
private static final ConcurrentLinkedQueue<Inflater> INFLATER_POOL = new ConcurrentLinkedQueue<>();
@@ -135,8 +138,7 @@ public class GzipCompressionProvider implements CompressionProvider
{
break;
}
- // `+ 1` to ensure some growth on the sizes of 0 or 1
- int newResultLength = result.length + (result.length / 2) + 1;
+ int newResultLength = result.length + (result.length / 2);
result = Arrays.copyOf(result, newResultLength);
}
// Write GZip trailer
@@ -162,7 +164,7 @@ public class GzipCompressionProvider implements CompressionProvider
if ( dataSize < 512 )
{
// Assuming DEFLATE doesn't compress small data well
- conservativeCompressedDataSizeEstimate = dataSize;
+ conservativeCompressedDataSizeEstimate = Math.max(dataSize, MIN_COMPRESSED_DATA_SIZE);
}
else
{
@@ -191,7 +193,7 @@ public class GzipCompressionProvider implements CompressionProvider
ByteBuffer gzippedData = ByteBuffer.wrap(gzippedDataBytes);
gzippedData.order(ByteOrder.LITTLE_ENDIAN);
int headerSize = readGzipHeader(gzippedData);
- if ( gzippedDataBytes.length < headerSize + GZIP_TRAILER_SIZE )
+ if ( gzippedDataBytes.length < headerSize + MIN_COMPRESSED_DATA_SIZE + GZIP_TRAILER_SIZE )
{
throw new EOFException("Too short GZipped data");
}
@@ -220,7 +222,10 @@ public class GzipCompressionProvider implements CompressionProvider
{
break;
}
- else if ( inflater.needsInput() )
+ // Just calling inflater.needsInput() doesn't work as expected, apparently it doesn't uphold it's own
+ // contract and could have needsInput() == true if numDecompressedBytes != 0 and that just means that
+ // there is not enough space in the result array
+ else if ( numDecompressedBytes == 0 && inflater.needsInput() )
{
throw new ZipException("Corrupt GZipped data");
}
@@ -231,9 +236,8 @@ public class GzipCompressionProvider implements CompressionProvider
{
throw new OutOfMemoryError("Unable to uncompress that much data into a single byte[] array");
}
- // `+ 1` to ensure some growth on the sizes of 0 or 1
int newResultLength =
- (int) Math.min((long) result.length + (result.length / 2) + 1, MAX_SAFE_JAVA_BYTE_ARRAY_SIZE);
+ (int) Math.min((long) result.length + (result.length / 2), MAX_SAFE_JAVA_BYTE_ARRAY_SIZE);
if ( result.length != newResultLength )
{
result = Arrays.copyOf(result, newResultLength);
http://git-wip-us.apache.org/repos/asf/curator/blob/7467e596/curator-framework/src/test/java/org/apache/curator/framework/imps/GzipCompressionProviderTest.java
----------------------------------------------------------------------
diff --git a/curator-framework/src/test/java/org/apache/curator/framework/imps/GzipCompressionProviderTest.java b/curator-framework/src/test/java/org/apache/curator/framework/imps/GzipCompressionProviderTest.java
index 11b8c63..51edaba 100644
--- a/curator-framework/src/test/java/org/apache/curator/framework/imps/GzipCompressionProviderTest.java
+++ b/curator-framework/src/test/java/org/apache/curator/framework/imps/GzipCompressionProviderTest.java
@@ -21,8 +21,11 @@ package org.apache.curator.framework.imps;
import org.testng.Assert;
import org.testng.annotations.Test;
+import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.util.Arrays;
+import java.util.concurrent.ThreadLocalRandom;
+import java.util.zip.GZIPOutputStream;
public class GzipCompressionProviderTest
{
@@ -32,6 +35,8 @@ public class GzipCompressionProviderTest
GzipCompressionProvider provider = new GzipCompressionProvider();
byte[] data = "Hello, world!".getBytes();
byte[] compressedData = provider.compress(null, data);
+ byte[] jdkCompressedData = jdkCompress(data);
+ Assert.assertTrue(Arrays.equals(compressedData, jdkCompressedData));
byte[] decompressedData = provider.decompress(null, compressedData);
Assert.assertTrue(Arrays.equals(decompressedData, data));
}
@@ -42,8 +47,10 @@ public class GzipCompressionProviderTest
GzipCompressionProvider provider = new GzipCompressionProvider();
byte[] compressedData = provider.compress(null, new byte[0]);
byte[] compressedData2 = GzipCompressionProvider.doCompress(new byte[0]);
+ byte[] jdkCompress = jdkCompress(new byte[0]);
// Ensures GzipCompressionProvider.COMPRESSED_EMPTY_BYTES value is valid
Assert.assertTrue(Arrays.equals(compressedData, compressedData2));
+ Assert.assertTrue(Arrays.equals(compressedData, jdkCompress));
byte[] decompressedData = provider.decompress(null, compressedData);
Assert.assertEquals(0, decompressedData.length);
}
@@ -85,4 +92,34 @@ public class GzipCompressionProviderTest
}
}
}
+
+ @Test
+ public void smokeTestRandomDataWithJdk() throws IOException
+ {
+ GzipCompressionProvider provider = new GzipCompressionProvider();
+ ThreadLocalRandom random = ThreadLocalRandom.current();
+ for (int len = 1; len < 100; len++)
+ {
+ byte[] data = new byte[len];
+ for (int i = 0; i < 100; i++) {
+ byte[] compressedData = provider.compress(null, data);
+ byte[] jdkCompressedData = jdkCompress(data);
+ Assert.assertTrue(Arrays.equals(compressedData, jdkCompressedData));
+ byte[] decompressedData = provider.decompress(null, compressedData);
+ Assert.assertTrue(Arrays.equals(decompressedData, data));
+ // in the end of the iteration to test empty array first
+ random.nextBytes(data);
+ }
+ }
+ }
+
+ private static byte[] jdkCompress(byte[] data) throws IOException
+ {
+ ByteArrayOutputStream bytes = new ByteArrayOutputStream();
+ try (GZIPOutputStream out = new GZIPOutputStream(bytes)) {
+ out.write(data);
+ out.finish();
+ }
+ return bytes.toByteArray();
+ }
}