Merge pull request #111 from xerial/refactoring

Refactoring

README.md

@@ -165,7 +165,15 @@ snappy-java uses sbt (simple build tool for Scala) as a build tool. Here is a si
     > ~test-only *     # run tests that matches a given name pattern  
     > publishM2        # publish jar to $HOME/.m2/repository
     > package          # create jar file
-    
+    > findbugs         # Produce findbugs report in target/findbugs
+    > jacoco:cover     # Report the code coverage of tests to target/jacoco folder    
+
+If you need to see detailed debug messages, launch sbt with `-Dloglevel=debug` option:
+
+```
+$ ./sbt -Dloglevel=debug
+```
+
 For the details of sbt usage, see my blog post: [Building Java Projects with sbt](http://xerial.org/blog/2014/03/24/sbt/)
 
 ## Miscellaneous Notes

build.sbt

@@ -1,3 +1,4 @@
+import de.johoop.findbugs4sbt.ReportType
 
 name := "snappy-java"
 
@@ -68,6 +69,14 @@ logBuffered in Test := false
 
 incOptions := incOptions.value.withNameHashing(true)
 
+findbugsSettings
+
+findbugsReportType := Some(ReportType.FancyHtml)
+
+findbugsReportPath := Some(crossTarget.value / "findbugs" / "report.html")
+
+jacoco.settings
+
 libraryDependencies ++= Seq(
    "junit" % "junit" % "4.8.2" % "test",
    "org.codehaus.plexus" % "plexus-classworlds" % "2.4" % "test",

project/plugins.sbt

@@ -5,7 +5,7 @@ addSbtPlugin("org.xerial.sbt" % "sbt-sonatype" % "0.5.0")
 
 addSbtPlugin("com.jsuereth" % "sbt-pgp" % "1.0.0")
 
-addSbtPlugin("de.johoop" % "findbugs4sbt" % "1.3.0")
+addSbtPlugin("de.johoop" % "findbugs4sbt" % "1.4.0")
 
 addSbtPlugin("de.johoop" % "jacoco4sbt" % "2.1.5")
 

src/main/java/org/xerial/snappy/Snappy.java

@@ -476,7 +476,7 @@ public class Snappy
             throws IOException
     {
         byte[] result = new byte[Snappy.uncompressedLength(input)];
-        int byteSize = Snappy.uncompress(input, 0, input.length, result, 0);
+        Snappy.uncompress(input, 0, input.length, result, 0);
         return result;
     }
 
@@ -569,7 +569,7 @@ public class Snappy
     {
         int uncompressedLength = Snappy.uncompressedLength(input, offset, length);
         char[] result = new char[uncompressedLength / 2];
-        int byteSize = impl.rawUncompress(input, offset, length, result, 0);
+        impl.rawUncompress(input, offset, length, result, 0);
         return result;
     }
 
@@ -585,7 +585,7 @@ public class Snappy
     {
         int uncompressedLength = Snappy.uncompressedLength(input, 0, input.length);
         double[] result = new double[uncompressedLength / 8];
-        int byteSize = impl.rawUncompress(input, 0, input.length, result, 0);
+        impl.rawUncompress(input, 0, input.length, result, 0);
         return result;
     }
 
@@ -687,7 +687,7 @@ public class Snappy
     {
         int uncompressedLength = Snappy.uncompressedLength(input, offset, length);
         float[] result = new float[uncompressedLength / 4];
-        int byteSize = impl.rawUncompress(input, offset, length, result, 0);
+        impl.rawUncompress(input, offset, length, result, 0);
         return result;
     }
 
@@ -718,7 +718,7 @@ public class Snappy
     {
         int uncompressedLength = Snappy.uncompressedLength(input, offset, length);
         int[] result = new int[uncompressedLength / 4];
-        int byteSize = impl.rawUncompress(input, offset, length, result, 0);
+        impl.rawUncompress(input, offset, length, result, 0);
         return result;
     }
 
@@ -749,7 +749,7 @@ public class Snappy
     {
         int uncompressedLength = Snappy.uncompressedLength(input, offset, length);
         long[] result = new long[uncompressedLength / 8];
-        int byteSize = impl.rawUncompress(input, offset, length, result, 0);
+        impl.rawUncompress(input, offset, length, result, 0);
         return result;
     }
 
@@ -780,7 +780,7 @@ public class Snappy
     {
         int uncompressedLength = Snappy.uncompressedLength(input, offset, length);
         short[] result = new short[uncompressedLength / 2];
-        int byteSize = impl.rawUncompress(input, offset, length, result, 0);
+        impl.rawUncompress(input, offset, length, result, 0);
         return result;
     }
 
@@ -838,7 +838,7 @@ public class Snappy
             UnsupportedEncodingException
     {
         byte[] uncompressed = new byte[uncompressedLength(input, offset, length)];
-        int compressedSize = uncompress(input, offset, length, uncompressed, 0);
+        uncompress(input, offset, length, uncompressed, 0);
         return new String(uncompressed, encoding);
     }
 
@@ -858,7 +858,7 @@ public class Snappy
             UnsupportedEncodingException
     {
         byte[] uncompressed = new byte[uncompressedLength(input, offset, length)];
-        int compressedSize = uncompress(input, offset, length, uncompressed, 0);
+        uncompress(input, offset, length, uncompressed, 0);
         return new String(uncompressed, encoding);
     }
 

src/main/java/org/xerial/snappy/SnappyCodec.java

@@ -48,11 +48,10 @@ import java.util.Arrays;
  */
 public class SnappyCodec
 {
-    public static final byte[] MAGIC_HEADER = new byte[] {-126, 'S', 'N', 'A', 'P', 'P', 'Y', 0};
+    static final byte[] MAGIC_HEADER = new byte[] {-126, 'S', 'N', 'A', 'P', 'P', 'Y', 0};
     public static final int MAGIC_LEN = MAGIC_HEADER.length;
     public static final int HEADER_SIZE = MAGIC_LEN + 8;
     public static final int MAGIC_HEADER_HEAD = SnappyOutputStream.readInt(MAGIC_HEADER, 0);
-    public static final int MAGIC_HEADER_TAIL = SnappyOutputStream.readInt(MAGIC_HEADER, 4);
 
     static {
         assert (MAGIC_HEADER_HEAD < 0);
@@ -60,6 +59,7 @@ public class SnappyCodec
 
     public static final int DEFAULT_VERSION = 1;
     public static final int MINIMUM_COMPATIBLE_VERSION = 1;
+    public static final SnappyCodec currentHeader = new SnappyCodec(MAGIC_HEADER, DEFAULT_VERSION, MINIMUM_COMPATIBLE_VERSION);
 
     public final byte[] magic;
     public final int version;
@@ -86,6 +86,11 @@ public class SnappyCodec
         headerArray = header.toByteArray();
     }
 
+    public static byte[] getMagicHeader()
+    {
+        return MAGIC_HEADER.clone();
+    }
+
     @Override
     public String toString()
     {
@@ -125,6 +130,5 @@ public class SnappyCodec
         int compatibleVersion = d.readInt();
         return new SnappyCodec(magic, version, compatibleVersion);
     }
-
-    public static SnappyCodec currentHeader = new SnappyCodec(MAGIC_HEADER, DEFAULT_VERSION, MINIMUM_COMPATIBLE_VERSION);
 }
+

src/main/java/org/xerial/snappy/SnappyInputStream.java

@@ -95,11 +95,6 @@ public class SnappyInputStream
         }
         if (readBytes < header.length || header[0] != SnappyCodec.MAGIC_HEADER[0]) {
             // do the default uncompression
-            readFully(header, readBytes);
-            return;
-        }
-
-        if (!isValidHeader(header)) {
             // (probably) compressed by Snappy.compress(byte[])
             readFully(header, readBytes);
             return;
@@ -393,8 +388,8 @@ public class SnappyInputStream
             // Concatenated data
             int remainingHeaderSize = SnappyCodec.headerSize() - 4;
             readBytes = readNext(header, 4, remainingHeaderSize);
-            if (readBytes < remainingHeaderSize) {
-                return false;
+            if(readBytes < remainingHeaderSize) {
+                throw new SnappyIOException(SnappyErrorCode.FAILED_TO_UNCOMPRESS, String.format("Insufficient header size in a concatenated block"));
             }
 
             if (isValidHeader(header)) {

src/main/java/org/xerial/snappy/SnappyLoader.java

@@ -87,7 +87,10 @@ public class SnappyLoader
     static void cleanUpExtractedNativeLib()
     {
         if (nativeLibFile != null && nativeLibFile.exists()) {
-            nativeLibFile.delete();
+            boolean deleted = nativeLibFile.delete();
+            if (!deleted) {
+                // Deleting native lib has failed, but it's not serious so simply ignore it here
+            }
         }
     }
 
@@ -217,37 +220,50 @@ public class SnappyLoader
 
         try {
             // Extract a native library file into the target directory
-            InputStream reader = SnappyLoader.class.getResourceAsStream(nativeLibraryFilePath);
-            FileOutputStream writer = new FileOutputStream(extractedLibFile);
+            InputStream reader = null;
+            FileOutputStream writer = null;
             try {
-                byte[] buffer = new byte[8192];
-                int bytesRead = 0;
-                while ((bytesRead = reader.read(buffer)) != -1) {
-                    writer.write(buffer, 0, bytesRead);
+                reader = SnappyLoader.class.getResourceAsStream(nativeLibraryFilePath);
+                try {
+                    writer = new FileOutputStream(extractedLibFile);
+
+                    byte[] buffer = new byte[8192];
+                    int bytesRead = 0;
+                    while ((bytesRead = reader.read(buffer)) != -1) {
+                        writer.write(buffer, 0, bytesRead);
+                    }
+                }
+                finally {
+                    if (writer != null) {
+                        writer.close();
+                    }
                 }
             }
             finally {
-                // Delete the extracted lib file on JVM exit.
-                extractedLibFile.deleteOnExit();
-
-                if (writer != null) {
-                    writer.close();
-                }
                 if (reader != null) {
                     reader.close();
                 }
+
+                // Delete the extracted lib file on JVM exit.
+                extractedLibFile.deleteOnExit();
             }
 
             // Set executable (x) flag to enable Java to load the native library
-            extractedLibFile.setReadable(true);
-            extractedLibFile.setWritable(true, true);
-            extractedLibFile.setExecutable(true);
+            boolean success = extractedLibFile.setReadable(true) &&
+                    extractedLibFile.setWritable(true, true) &&
+                    extractedLibFile.setExecutable(true);
+            if (!success) {
+                // Setting file flag may fail, but in this case another error will be thrown in later phase
+            }
 
             // Check whether the contents are properly copied from the resource folder
             {
-                InputStream nativeIn = SnappyLoader.class.getResourceAsStream(nativeLibraryFilePath);
-                InputStream extractedLibIn = new FileInputStream(extractedLibFile);
+                InputStream nativeIn = null;
+                InputStream extractedLibIn = null;
                 try {
+                    nativeIn = SnappyLoader.class.getResourceAsStream(nativeLibraryFilePath);
+                    extractedLibIn = new FileInputStream(extractedLibFile);
+
                     if (!contentsEquals(nativeIn, extractedLibIn)) {
                         throw new SnappyError(SnappyErrorCode.FAILED_TO_LOAD_NATIVE_LIBRARY, String.format("Failed to write a native library file at %s", extractedLibFile));
                     }
@@ -318,7 +334,10 @@ public class SnappyLoader
         // Temporary folder for the native lib. Use the value of org.xerial.snappy.tempdir or java.io.tmpdir
         File tempFolder = new File(System.getProperty(KEY_SNAPPY_TEMPDIR, System.getProperty("java.io.tmpdir")));
         if (!tempFolder.exists()) {
-            tempFolder.mkdir();
+            boolean created = tempFolder.mkdirs();
+            if (!created) {
+                // if created == false, it will fail eventually in the later part
+            }
         }
 
         // Extract and load a native library inside the jar file

src/main/java/org/xerial/snappy/SnappyOutputStream.java

@@ -86,7 +86,7 @@ public class SnappyOutputStream
      */
     public SnappyOutputStream(OutputStream out, int blockSize)
     {
-        this(out, blockSize, CachedBufferAllocator.factory);
+        this(out, blockSize, CachedBufferAllocator.getBufferAllocatorFactory());
     }
 
     public SnappyOutputStream(OutputStream out, int blockSize, BufferAllocatorFactory bufferAllocatorFactory)

src/main/java/org/xerial/snappy/buffer/CachedBufferAllocator.java

@@ -9,8 +9,7 @@ import java.util.*;
 public class CachedBufferAllocator
         implements BufferAllocator
 {
-
-    public static BufferAllocatorFactory factory = new BufferAllocatorFactory()
+    private static BufferAllocatorFactory factory = new BufferAllocatorFactory()
     {
         @Override
         public BufferAllocator getBufferAllocator(int bufferSize)
@@ -19,10 +18,21 @@ public class CachedBufferAllocator
         }
     };
 
+    public static void setBufferAllocatorFactory(BufferAllocatorFactory factory)
+    {
+        assert (factory != null);
+        CachedBufferAllocator.factory = factory;
+    }
+
+    public static BufferAllocatorFactory getBufferAllocatorFactory()
+    {
+        return factory;
+    }
+
     /**
      * Use SoftReference so that having this queueTable does not prevent the GC of CachedBufferAllocator instances
      */
-    public static Map<Integer, SoftReference<CachedBufferAllocator>> queueTable = new HashMap<Integer, SoftReference<CachedBufferAllocator>>();
+    private static final Map<Integer, SoftReference<CachedBufferAllocator>> queueTable = new HashMap<Integer, SoftReference<CachedBufferAllocator>>();
 
     private final int bufferSize;
     private final Deque<byte[]> bufferQueue;

src/test/java/org/xerial/snappy/SnappyInputStreamTest.java

@@ -35,7 +35,6 @@ import java.io.InputStream;
 import org.junit.Test;
 import org.xerial.util.FileResource;
 import org.xerial.util.log.Logger;
-import scala.Array;
 
 public class SnappyInputStreamTest
 {
@@ -193,4 +192,16 @@ public class SnappyInputStreamTest
         assertArrayEquals(orig1, uncompressed1);
         assertArrayEquals(orig2, uncompressed2);
     }
+
+    @Test
+    public void readSnappyCompressResult()
+            throws Exception
+    {
+        byte[] orig = readResourceFile("alice29.txt");
+        byte[] compressed = Snappy.compress(orig);
+        SnappyInputStream in = new SnappyInputStream(new ByteArrayInputStream(compressed));
+        byte[] uncompressed = readFully(in);
+
+        assertArrayEquals(orig, uncompressed);
+    }
 }

src/test/java/org/xerial/snappy/SnappyOutputStreamTest.java

@@ -181,7 +181,7 @@ public class SnappyOutputStreamTest
         // Regression test for issue #107, a bug where close() was non-idempotent and would release
         // its buffers to the allocator multiple times, which could cause scenarios where two open
         // SnappyOutputStreams could share the same buffers, leading to stream corruption issues.
-        final BufferAllocatorFactory bufferAllocatorFactory = CachedBufferAllocator.factory;
+        final BufferAllocatorFactory bufferAllocatorFactory = CachedBufferAllocator.getBufferAllocatorFactory();
         final int BLOCK_SIZE = 4096;
         // Create a stream, use it, then close it once:
         ByteArrayOutputStream ba1 = new ByteArrayOutputStream();