From 2fad160bd69df9ffa3b5b18272316a931692fdff Mon Sep 17 00:00:00 2001 From: brmeyer Date: Thu, 6 Dec 2012 14:26:13 -0500 Subject: [PATCH] HHH-7835 Inefficient implementation of JarVisitorFactory.getBytesFromInputStream --- .../packaging/internal/JarVisitorFactory.java | 29 ++++---- .../jpa/test/packaging/JarVisitorTest.java | 72 +++++++++++++++++++ .../jpa/test/packaging/PackagingTestCase.java | 13 ++++ .../hibernate/jpa/test/packaging/empty.txt | 0 4 files changed, 99 insertions(+), 15 deletions(-) create mode 100644 hibernate-entitymanager/src/test/resources/org/hibernate/jpa/test/packaging/empty.txt diff --git a/hibernate-entitymanager/src/main/java/org/hibernate/jpa/packaging/internal/JarVisitorFactory.java b/hibernate-entitymanager/src/main/java/org/hibernate/jpa/packaging/internal/JarVisitorFactory.java index 9e62ea9c2a..267cd47d31 100644 --- a/hibernate-entitymanager/src/main/java/org/hibernate/jpa/packaging/internal/JarVisitorFactory.java +++ b/hibernate-entitymanager/src/main/java/org/hibernate/jpa/packaging/internal/JarVisitorFactory.java @@ -21,6 +21,7 @@ */ package org.hibernate.jpa.packaging.internal; +import java.io.ByteArrayOutputStream; import java.io.File; import java.io.IOException; import java.io.InputStream; @@ -28,13 +29,13 @@ import java.net.MalformedURLException; import java.net.URISyntaxException; import java.net.URL; -import org.jboss.logging.Logger; - -import org.hibernate.jpa.internal.EntityManagerMessageLogger; import org.hibernate.internal.util.StringHelper; +import org.hibernate.jpa.internal.EntityManagerMessageLogger; +import org.jboss.logging.Logger; /** * @author Emmanuel Bernard + * @author Brett Meyer */ public class JarVisitorFactory { @@ -194,18 +195,16 @@ public class JarVisitorFactory { } } - public static byte[] getBytesFromInputStream(InputStream inputStream) throws IOException { - int size; - byte[] tmpByte = new byte[ 4096 ]; - byte[] entryBytes = new byte[0]; - for ( ; ; ) { - size = inputStream.read( tmpByte ); - if ( size == -1 ) break; - byte[] current = new byte[ entryBytes.length + size ]; - System.arraycopy( entryBytes, 0, current, 0, entryBytes.length ); - System.arraycopy( tmpByte, 0, current, entryBytes.length, size ); - entryBytes = current; + public static byte[] getBytesFromInputStream( + InputStream inputStream) throws IOException { + + ByteArrayOutputStream buffer = new ByteArrayOutputStream(); + int numBytes; + byte[] data = new byte[4096]; + while ( ( numBytes = inputStream.read( data, 0, data.length ) ) != -1 ) { + buffer.write( data, 0, numBytes ); } - return entryBytes; + buffer.flush(); + return buffer.toByteArray(); } } diff --git a/hibernate-entitymanager/src/test/java/org/hibernate/jpa/test/packaging/JarVisitorTest.java b/hibernate-entitymanager/src/test/java/org/hibernate/jpa/test/packaging/JarVisitorTest.java index 6794708dd4..b1fb148cb1 100644 --- a/hibernate-entitymanager/src/test/java/org/hibernate/jpa/test/packaging/JarVisitorTest.java +++ b/hibernate-entitymanager/src/test/java/org/hibernate/jpa/test/packaging/JarVisitorTest.java @@ -27,9 +27,13 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; +import java.io.BufferedInputStream; import java.io.File; +import java.io.FileInputStream; import java.io.IOException; +import java.io.InputStream; import java.net.URL; import java.net.URLConnection; import java.net.URLStreamHandler; @@ -61,6 +65,7 @@ import org.junit.Test; /** * @author Emmanuel Bernard * @author Hardy Ferentschik + * @author Brett Meyer */ @RequiresDialect( H2Dialect.class ) // Nothing dialect-specific -- no need to run in matrix. @SuppressWarnings("unchecked") @@ -307,6 +312,73 @@ public class JarVisitorTest extends PackagingTestCase { // Entry entry = new Entry( Carpet.class.getName(), null ); // assertTrue( entries[1].contains( entry ) ); } + + @Test + @TestForIssue(jiraKey = "HHH-7835") + public void testGetBytesFromInputStream() { + try { + File file = buildLargeJar(); + + long start = System.currentTimeMillis(); + InputStream stream = new BufferedInputStream( + new FileInputStream( file ) ); + int oldLength = getBytesFromInputStream( stream ).length; + stream.close(); + long oldTime = System.currentTimeMillis() - start; + + start = System.currentTimeMillis(); + stream = new BufferedInputStream( new FileInputStream( file ) ); + int newLength = JarVisitorFactory.getBytesFromInputStream( + stream ).length; + stream.close(); + long newTime = System.currentTimeMillis() - start; + + assertEquals( oldLength, newLength ); + assertTrue( oldTime > newTime ); + } + catch ( Exception e ) { + fail( e.getMessage() ); + } + } + + // This is the old getBytesFromInputStream from JarVisitorFactory before + // it was changed by HHH-7835. Use it as a regression test. + private byte[] getBytesFromInputStream( + InputStream inputStream) throws IOException { + int size; + + byte[] entryBytes = new byte[0]; + for ( ;; ) { + byte[] tmpByte = new byte[4096]; + size = inputStream.read( tmpByte ); + if ( size == -1 ) + break; + byte[] current = new byte[entryBytes.length + size]; + System.arraycopy( entryBytes, 0, current, 0, entryBytes.length ); + System.arraycopy( tmpByte, 0, current, entryBytes.length, size ); + entryBytes = current; + } + return entryBytes; + } + + @Test + @TestForIssue(jiraKey = "HHH-7835") + public void testGetBytesFromZeroInputStream() { + try { + // Ensure that JarVisitorFactory#getBytesFromInputStream + // can handle 0 length streams gracefully. + InputStream emptyStream = new BufferedInputStream( + new FileInputStream( new File( + "src/test/resources/org/hibernate/jpa/test/packaging/empty.txt" ) ) ); + int length = JarVisitorFactory.getBytesFromInputStream( + emptyStream ).length; + assertEquals( length, 0 ); + emptyStream.close(); + } + catch ( Exception e ) { + fail( e.getMessage() ); + } + } private Filter[] getFilters() { return new Filter[] { diff --git a/hibernate-entitymanager/src/test/java/org/hibernate/jpa/test/packaging/PackagingTestCase.java b/hibernate-entitymanager/src/test/java/org/hibernate/jpa/test/packaging/PackagingTestCase.java index 12a82dcd76..00d38de02b 100644 --- a/hibernate-entitymanager/src/test/java/org/hibernate/jpa/test/packaging/PackagingTestCase.java +++ b/hibernate-entitymanager/src/test/java/org/hibernate/jpa/test/packaging/PackagingTestCase.java @@ -73,6 +73,7 @@ import static org.junit.Assert.fail; /** * @author Hardy Ferentschik + * @author Brett Meyer */ public abstract class PackagingTestCase extends BaseCoreFunctionalTestCase { protected static ClassLoader originalClassLoader = Thread.currentThread().getContextClassLoader(); @@ -335,6 +336,18 @@ public abstract class PackagingTestCase extends BaseCoreFunctionalTestCase { return testPackage; } + protected File buildLargeJar() { + String fileName = "large.jar"; + JavaArchive archive = ShrinkWrap.create( JavaArchive.class, fileName ); + // Build a large jar by adding all EntityManager packages and + // subpackages on the classpath. + archive.addPackages(true, "org.hibernate.ejb", "org.hibernate.jpa" ); + + File testPackage = new File( packageTargetDir, fileName ); + archive.as( ZipExporter.class ).exportTo( testPackage, true ); + return testPackage; + } + protected File buildWar() { String fileName = "war.war"; WebArchive archive = ShrinkWrap.create( WebArchive.class, fileName ); diff --git a/hibernate-entitymanager/src/test/resources/org/hibernate/jpa/test/packaging/empty.txt b/hibernate-entitymanager/src/test/resources/org/hibernate/jpa/test/packaging/empty.txt new file mode 100644 index 0000000000..e69de29bb2