HHH-4161 - persistence.xml <jar-file> not following JSR220 spec

hibernate-core/src/main/java/org/hibernate/boot/archive/internal/StandardArchiveDescriptorFactory.java

@@ -7,20 +7,26 @@
 package org.hibernate.boot.archive.internal;
 
 import java.io.File;
+import java.net.MalformedURLException;
 import java.net.URISyntaxException;
 import java.net.URL;
 
 import org.hibernate.boot.archive.spi.ArchiveDescriptor;
 import org.hibernate.boot.archive.spi.ArchiveDescriptorFactory;
+import org.hibernate.boot.archive.spi.JarFileEntryUrlAdjuster;
 import org.hibernate.internal.util.StringHelper;
 
+import org.jboss.logging.Logger;
+
 /**
  * Standard implementation of ArchiveDescriptorFactory
  *
  * @author Emmanuel Bernard
  * @author Steve Ebersole
  */
-public class StandardArchiveDescriptorFactory implements ArchiveDescriptorFactory {
+public class StandardArchiveDescriptorFactory implements ArchiveDescriptorFactory, JarFileEntryUrlAdjuster {
+	private static final Logger log = Logger.getLogger( StandardArchiveDescriptorFactory.class );
+
 	/**
 	 * Singleton access
 	 */
@@ -41,33 +47,7 @@ public class StandardArchiveDescriptorFactory implements ArchiveDescriptorFactor
 				|| "file".equals( protocol )
 				|| "vfszip".equals( protocol )
 				|| "vfsfile".equals( protocol ) ) {
-			final File file;
+			final File file = new File( extractLocalFilePath( url ) );
-			try {
-				final String filePart = url.getFile();
-				if ( filePart != null && filePart.indexOf( ' ' ) != -1 ) {
-					//unescaped (from the container), keep as is
-					file = new File( url.getFile() );
-				}
-				else {
-					file = new File( url.toURI().getSchemeSpecificPart() );
-				}
-
-				if ( ! file.exists() ) {
-					throw new IllegalArgumentException(
-							String.format(
-									"File [%s] referenced by given URL [%s] does not exist",
-									filePart,
-									url.toExternalForm()
-							)
-					);
-				}
-			}
-			catch (URISyntaxException e) {
-				throw new IllegalArgumentException(
-						"Unable to visit JAR " + url + ". Cause: " + e.getMessage(), e
-				);
-			}
-
 			if ( file.isDirectory() ) {
 				return new ExplodedArchiveDescriptor( this, url, entry );
 			}
@@ -81,6 +61,24 @@ public class StandardArchiveDescriptorFactory implements ArchiveDescriptorFactor
 		}
 	}
 
+	protected String extractLocalFilePath(URL url) {
+		final String filePart = url.getFile();
+		if ( filePart != null && filePart.indexOf( ' ' ) != -1 ) {
+			//unescaped (from the container), keep as is
+			return filePart;
+		}
+		else {
+			try {
+				return url.toURI().getSchemeSpecificPart();
+			}
+			catch (URISyntaxException e) {
+				throw new IllegalArgumentException(
+						"Unable to visit JAR " + url + ". Cause: " + e.getMessage(), e
+				);
+			}
+		}
+	}
+
 	@Override
 	public URL getJarURLFromURLEntry(URL url, String entry) throws IllegalArgumentException {
 		return ArchiveHelper.getJarURLFromURLEntry( url, entry );
@@ -90,4 +88,57 @@ public class StandardArchiveDescriptorFactory implements ArchiveDescriptorFactor
 	public URL getURLFromPath(String jarPath) {
 		return ArchiveHelper.getURLFromPath( jarPath );
 	}
+
+	@Override
+	public URL adjustJarFileEntryUrl(URL url, URL rootUrl) {
+		final String protocol = url.getProtocol();
+		final boolean check = StringHelper.isEmpty( protocol )
+				|| "file".equals( protocol )
+				|| "vfszip".equals( protocol )
+				|| "vfsfile".equals( protocol );
+		if ( !check ) {
+			return url;
+		}
+
+		final String filePart = extractLocalFilePath( url );
+		if ( filePart.startsWith( "/" ) ) {
+			// the URL is already an absolute form
+			return url;
+		}
+		else {
+			// prefer to resolve the relative URL relative to the root PU URL per
+			// JPA 2.0 clarification.
+			final File rootUrlFile = new File( extractLocalFilePath( rootUrl ) );
+			try {
+				if ( rootUrlFile.isDirectory() ) {
+					// The PU root is a directory (exploded).  Here we can just build
+					// the relative File reference and use the Filesystem API to convert
+					// to URI and then a URL
+					final File combined = new File( rootUrlFile, filePart );
+					// make sure it exists..
+					if ( combined.exists() ) {
+						return combined.toURI().toURL();
+					}
+				}
+				else {
+					// The PU root is an archive.  Here we have to build a JAR URL to properly
+					// handle the nested entry reference (the !/ part).
+					return new URL(
+							"jar:" + protocol + "://" + rootUrlFile.getAbsolutePath() + "!/" + filePart
+					);
+				}
+			}
+			catch (MalformedURLException e) {
+				// allow to pass through to return the original URL
+				log.debugf(
+						e,
+						"Unable to adjust relative <jar-file/> URL [%s] relative to root URL [%s]",
+						filePart,
+						rootUrlFile.getAbsolutePath()
+				);
+			}
+
+			return url;
+		}
+	}
 }

hibernate-core/src/main/java/org/hibernate/boot/archive/scan/spi/AbstractScannerImpl.java

@@ -16,6 +16,7 @@ import org.hibernate.boot.archive.spi.ArchiveDescriptor;
 import org.hibernate.boot.archive.spi.ArchiveDescriptorFactory;
 import org.hibernate.boot.archive.spi.ArchiveEntry;
 import org.hibernate.boot.archive.spi.ArchiveEntryHandler;
+import org.hibernate.boot.archive.spi.JarFileEntryUrlAdjuster;
 
 /**
  * @author Steve Ebersole
@@ -35,14 +36,14 @@ public abstract class AbstractScannerImpl implements Scanner {
 		if ( environment.getNonRootUrls() != null ) {
 			final ArchiveContext context = new ArchiveContextImpl( false, collector );
 			for ( URL url : environment.getNonRootUrls() ) {
-				final ArchiveDescriptor descriptor = buildArchiveDescriptor( url, false );
+				final ArchiveDescriptor descriptor = buildArchiveDescriptor( url, environment, false );
 				descriptor.visitArchive( context );
 			}
 		}
 
 		if ( environment.getRootUrl() != null ) {
 			final ArchiveContext context = new ArchiveContextImpl( true, collector );
-			final ArchiveDescriptor descriptor = buildArchiveDescriptor( environment.getRootUrl(), true );
+			final ArchiveDescriptor descriptor = buildArchiveDescriptor( environment.getRootUrl(), environment, true );
 			descriptor.visitArchive( context );
 		}
 
@@ -50,10 +51,16 @@ public abstract class AbstractScannerImpl implements Scanner {
 	}
 
 
-	private ArchiveDescriptor buildArchiveDescriptor(URL url, boolean isRootUrl) {
+	private ArchiveDescriptor buildArchiveDescriptor(
+			URL url,
+			ScanEnvironment environment,
+			boolean isRootUrl) {
 		final ArchiveDescriptor descriptor;
 		final ArchiveDescriptorInfo descriptorInfo = archiveDescriptorCache.get( url );
 		if ( descriptorInfo == null ) {
+			if ( !isRootUrl && archiveDescriptorFactory instanceof JarFileEntryUrlAdjuster ) {
+				url = ( (JarFileEntryUrlAdjuster) archiveDescriptorFactory ).adjustJarFileEntryUrl( url, environment.getRootUrl() );
+			}
 			descriptor = archiveDescriptorFactory.buildArchiveDescriptor( url );
 			archiveDescriptorCache.put(
 					url,
@@ -67,6 +74,17 @@ public abstract class AbstractScannerImpl implements Scanner {
 		return descriptor;
 	}
 
+	/**
+	 * Handle <jar-file/> references from a persistence.xml file.
+	 *
+	 * JPA allows for  to be specific
+	 * @param url
+	 * @return
+	 */
+	protected URL resolveNonRootUrl(URL url) {
+		return null;
+	}
+
 	// This needs to be protected and attributes/constructor visible in case
 	// a custom scanner needs to override validateReuse.
 	protected static class ArchiveDescriptorInfo {

hibernate-core/src/main/java/org/hibernate/boot/archive/spi/JarFileEntryUrlAdjuster.java

@@ -0,0 +1,29 @@
+/*
+ * Hibernate, Relational Persistence for Idiomatic Java
+ *
+ * License: GNU Lesser General Public License (LGPL), version 2.1 or later.
+ * See the lgpl.txt file in the root directory or <http://www.gnu.org/licenses/lgpl-2.1.html>.
+ */
+package org.hibernate.boot.archive.spi;
+
+import java.net.URL;
+
+/**
+ * Optional contract for ArchiveDescriptorFactory implementations to implement
+ * to be able to adjust {@code <jar-file/>} URL's defined in {@code persistence.xml}
+ * files.  The intent is to account for relative URLs in a protocol specific way
+ * according to the protocol(s) handled by the ArchiveDescriptorFactory.
+ *
+ * @author Steve Ebersole
+ */
+public interface JarFileEntryUrlAdjuster {
+	/**
+	 * Adjust the given URL, if needed.
+	 *
+	 * @param url The url to adjust
+	 * @param rootUrl The root URL, for resolving relative URLs
+	 *
+	 * @return The adjusted url.
+	 */
+	URL adjustJarFileEntryUrl(URL url, URL rootUrl);
+}

hibernate-entitymanager/src/test/bundles/explicitpar2/META-INF/orm.xml

@@ -0,0 +1,22 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<!--
+  ~ Hibernate, Relational Persistence for Idiomatic Java
+  ~
+  ~ License: GNU Lesser General Public License (LGPL), version 2.1 or later.
+  ~ See the lgpl.txt file in the root directory or <http://www.gnu.org/licenses/lgpl-2.1.html>.
+  -->
+<entity-mappings xmlns="http://java.sun.com/xml/ns/persistence/orm"
+                 xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+                 xsi:schemaLocation="http://java.sun.com/xml/ns/persistence/orm orm_2_0.xsd"
+                 version="2.0"
+        >
+    <package>org.hibernate.jpa.test.pack.various</package>
+    <entity class="Seat" access="PROPERTY" metadata-complete="true">
+        <attributes>
+            <id name="number">
+            	<column name="`number`" />
+            </id>
+        </attributes>
+    </entity>
+</entity-mappings>

hibernate-entitymanager/src/test/bundles/explicitpar2/META-INF/persistence.xml

@@ -0,0 +1,56 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+  ~ Hibernate, Relational Persistence for Idiomatic Java
+  ~
+  ~ License: GNU Lesser General Public License (LGPL), version 2.1 or later.
+  ~ See the lgpl.txt file in the root directory or <http://www.gnu.org/licenses/lgpl-2.1.html>.
+  -->
+<persistence xmlns="http://java.sun.com/xml/ns/persistence"
+             xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+             xsi:schemaLocation="http://java.sun.com/xml/ns/persistence http://java.sun.com/xml/ns/persistence/persistence_2_0.xsd"
+             version="2.0">
+    <persistence-unit name="manager1" transaction-type="RESOURCE_LOCAL">
+        <jar-file>META-INF/externaljar2.jar</jar-file>
+
+        <class>org.hibernate.jpa.test.Cat</class>
+		<class>org.hibernate.jpa.test.Kitten</class>
+        <class>org.hibernate.jpa.test.Distributor</class>
+        <class>org.hibernate.jpa.test.Item</class>
+
+        <exclude-unlisted-classes>true</exclude-unlisted-classes>
+
+        <properties>
+            <!-- custom scanner test -->
+            <!--<property name="hibernate.ejb.resource_scanner" value="org.hibernate.jpa.test.packaging.CustomScanner"/>-->
+
+            <property name="hibernate.dialect" value="@db.dialect@"/>
+            <property name="hibernate.connection.driver_class" value="@jdbc.driver@"/>
+            <property name="hibernate.connection.username" value="@jdbc.user@"/>
+            <property name="hibernate.connection.password" value="@jdbc.pass@"/>
+            <property name="hibernate.connection.url" value="@jdbc.url@"/>
+            <property name="hibernate.cache.use_query_cache" value="true"/>
+            <property name="hibernate.cache.region_prefix" value="hibernate.test"/>
+            <property name="hibernate.jdbc.use_streams_for_binary" value="true"/>
+            <property name="hibernate.jdbc.batch_size" value="0"/>
+            <property name="hibernate.max_fetch_depth" value="3"/>
+            <property name="hibernate.hbm2ddl.auto" value="create-drop"/>
+            <property name="hibernate.generate_statistics" value="true"/>
+			<property name="hibernate.cache.region.factory_class" value="org.hibernate.testing.cache.CachingRegionFactory" />
+            <property name="hibernate.physical_naming_strategy" value="org.hibernate.jpa.test.MyNamingStrategy"/>
+            <!-- test naming strategy and fall back to element content -->
+            <!-- property name="hibernate.ejb.naming_strategy">MyNamingStrategy</property -->
+
+            <!-- cache configuration -->
+            <property name="hibernate.ejb.classcache.org.hibernate.jpa.test.Item" value="read-write"/>
+            <property name="hibernate.ejb.collectioncache.org.hibernate.jpa.test.Item.distributors"
+                      value="read-write, RegionName"/>
+
+            <!-- event overriding -->
+            <property name="hibernate.ejb.event.pre-insert" value="org.hibernate.jpa.test.NoOpListener"/>
+            <!-- remove JACC and validator -->
+
+            <!-- alternatively to <class> and <property> declarations, you can use a regular hibernate.cfg.xml file -->
+            <!-- property name="hibernate.ejb.cfgfile" value="/resource-path/to/hibernate.cfg.xml"/ -->
+        </properties>
+    </persistence-unit>
+</persistence>

hibernate-entitymanager/src/test/java/org/hibernate/jpa/test/packaging/PackagedEntityManagerTest.java

@@ -450,6 +450,38 @@ public class PackagedEntityManagerTest extends PackagingTestCase {
 		emf.close();
 	}
 
+	@Test
+	public void testRelativeJarReferences() throws Exception {
+		File externalJar = buildExternalJar2();
+		File testPackage = buildExplicitPar2();
+		addPackageToClasspath( testPackage, externalJar );
+
+		// if the jar cannot be resolved, this call should fail
+		EntityManagerFactory emf = Persistence.createEntityManagerFactory( "manager1", new HashMap() );
+
+		// but to make sure, also verify that the entity defined in the external jar was found
+		emf.getMetamodel().entity( Airplane.class );
+		emf.getMetamodel().entity( Scooter.class );
+
+		// additionally, try to use them
+		EntityManager em = emf.createEntityManager();
+		Scooter s = new Scooter();
+		s.setModel( "Abadah" );
+		s.setSpeed( 85l );
+		em.getTransaction().begin();
+		em.persist( s );
+		em.getTransaction().commit();
+		em.close();
+		em = emf.createEntityManager();
+		em.getTransaction().begin();
+		s = em.find( Scooter.class, s.getModel() );
+		assertEquals( Long.valueOf( 85 ), s.getSpeed() );
+		em.remove( s );
+		em.getTransaction().commit();
+		em.close();
+		emf.close();
+	}
+
 	@Test
 	public void testORMFileOnMainAndExplicitJars() throws Exception {
 		File testPackage = buildExplicitPar();

hibernate-entitymanager/src/test/java/org/hibernate/jpa/test/packaging/PackagingTestCase.java

@@ -224,6 +224,31 @@ public abstract class PackagingTestCase extends BaseCoreFunctionalTestCase {
 		return testPackage;
 	}
 
+	protected File buildExplicitPar2() {
+		// explicitpar/persistence.xml references externaljar.jar so build that from here.
+		// this is the reason for tests failing after clean at least on my (Steve) local system
+		File jar = buildExternalJar2();
+
+		String fileName = "explicitpar2.par";
+		JavaArchive archive = ShrinkWrap.create( JavaArchive.class, fileName );
+		archive.addClasses(
+				Airplane.class,
+				Seat.class,
+				Cat.class,
+				Kitten.class,
+				Distributor.class,
+				Item.class
+		);
+
+		archive.addAsResource( "explicitpar2/META-INF/orm.xml", ArchivePaths.create( "META-INF/orm.xml" ) );
+		archive.addAsResource( "explicitpar2/META-INF/persistence.xml", ArchivePaths.create( "META-INF/persistence.xml" ) );
+		archive.addAsResource( jar, ArchivePaths.create( "META-INF/externaljar2.jar" ) );
+
+		File testPackage = new File( packageTargetDir, fileName );
+		archive.as( ZipExporter.class ).exportTo( testPackage, true );
+		return testPackage;
+	}
+
 	protected File buildExplodedPar() {
 		String fileName = "explodedpar";
 		JavaArchive archive = ShrinkWrap.create(  JavaArchive.class,fileName );
@@ -334,6 +359,21 @@ public abstract class PackagingTestCase extends BaseCoreFunctionalTestCase {
 		return testPackage;
 	}
 
+	protected File buildExternalJar2() {
+		String fileName = "externaljar2.jar";
+		JavaArchive archive = ShrinkWrap.create( JavaArchive.class, fileName );
+		archive.addClasses(
+				Scooter.class
+		);
+
+		ArchivePath path = ArchivePaths.create( "META-INF/orm.xml" );
+		archive.addAsResource( "externaljar/META-INF/orm.xml", path );
+
+		File testPackage = new File( packageTargetDir, fileName );
+		archive.as( ZipExporter.class ).exportTo( testPackage, true );
+		return testPackage;
+	}
+
 	protected File buildLargeJar() {
 		String fileName = "large.jar";
 		JavaArchive archive = ShrinkWrap.create( JavaArchive.class, fileName );