JPA persistence.xml files may use jar-file entries relative to the unit root (as per the JPA spec)

Issue: SPR-9797

spring-orm/src/main/java/org/springframework/orm/jpa/persistenceunit/PersistenceUnitReader.java

@@ -173,12 +173,11 @@ class PersistenceUnitReader {
 
 		Element persistence = document.getDocumentElement();
 		String version = persistence.getAttribute(PERSISTENCE_VERSION);
-		URL unitRootURL = determinePersistenceUnitRootUrl(resource);
+		URL rootUrl = determinePersistenceUnitRootUrl(resource);
+
 		List<Element> units = DomUtils.getChildElementsByTagName(persistence, PERSISTENCE_UNIT);
 		for (Element unit : units) {
-			SpringPersistenceUnitInfo info = parsePersistenceUnitInfo(unit, version);
-			info.setPersistenceUnitRootUrl(unitRootURL);
-			infos.add(info);
+			infos.add(parsePersistenceUnitInfo(unit, version, rootUrl));
 		}
 
 		return infos;
@@ -193,46 +192,52 @@ class PersistenceUnitReader {
 	 */
 	protected URL determinePersistenceUnitRootUrl(Resource resource) throws IOException {
 		URL originalURL = resource.getURL();
-		String urlToString = originalURL.toExternalForm();
 
 		// If we get an archive, simply return the jar URL (section 6.2 from the JPA spec)
 		if (ResourceUtils.isJarURL(originalURL)) {
 			return ResourceUtils.extractJarFileURL(originalURL);
 		}
 
-		else {
-			// check META-INF folder
-			if (!urlToString.contains(META_INF)) {
-				if (logger.isInfoEnabled()) {
-					logger.info(resource.getFilename() +
-							" should be located inside META-INF directory; cannot determine persistence unit root URL for " +
-							resource);
-				}
-				return null;
+		// check META-INF folder
+		String urlToString = originalURL.toExternalForm();
+		if (!urlToString.contains(META_INF)) {
+			if (logger.isInfoEnabled()) {
+				logger.info(resource.getFilename() +
+						" should be located inside META-INF directory; cannot determine persistence unit root URL for " +
+						resource);
 			}
-			if (urlToString.lastIndexOf(META_INF) == urlToString.lastIndexOf('/') - (1 + META_INF.length())) {
-				if (logger.isInfoEnabled()) {
-					logger.info(resource.getFilename() +
-							" is not located in the root of META-INF directory; cannot determine persistence unit root URL for " +
-							resource);
-				}
-				return null;
+			return null;
+		}
+		if (urlToString.lastIndexOf(META_INF) == urlToString.lastIndexOf('/') - (1 + META_INF.length())) {
+			if (logger.isInfoEnabled()) {
+				logger.info(resource.getFilename() +
+						" is not located in the root of META-INF directory; cannot determine persistence unit root URL for " +
+						resource);
 			}
+			return null;
+		}
 
-			String persistenceUnitRoot = urlToString.substring(0, urlToString.lastIndexOf(META_INF));
-			return new URL(persistenceUnitRoot);
+		String persistenceUnitRoot = urlToString.substring(0, urlToString.lastIndexOf(META_INF));
+		if (persistenceUnitRoot.endsWith("/")) {
+			persistenceUnitRoot = persistenceUnitRoot.substring(0, persistenceUnitRoot.length() - 1);
 		}
+		return new URL(persistenceUnitRoot);
 	}
 
 	/**
 	 * Parse the unit info DOM element.
 	 */
-	protected SpringPersistenceUnitInfo parsePersistenceUnitInfo(Element persistenceUnit, String version) throws IOException {
+	protected SpringPersistenceUnitInfo parsePersistenceUnitInfo(Element persistenceUnit, String version, URL rootUrl)
+			throws IOException {
+
 		SpringPersistenceUnitInfo unitInfo = new SpringPersistenceUnitInfo();
 
 		// set JPA version (1.0 or 2.0)
 		unitInfo.setPersistenceXMLSchemaVersion(version);
 
+		// set persistence unit root URL
+		unitInfo.setPersistenceUnitRootUrl(rootUrl);
+
 		// set unit name
 		unitInfo.setPersistenceUnitName(persistenceUnit.getAttribute(UNIT_NAME).trim());
 
@@ -277,10 +282,10 @@ class PersistenceUnitReader {
 			unitInfo.setValidationModeName(validationMode);
 		}
 
+		parseProperties(persistenceUnit, unitInfo);
+		parseManagedClasses(persistenceUnit, unitInfo);
 		parseMappingFiles(persistenceUnit, unitInfo);
 		parseJarFiles(persistenceUnit, unitInfo);
-		parseClass(persistenceUnit, unitInfo);
-		parseProperty(persistenceUnit, unitInfo);
 
 		return unitInfo;
 	}
@@ -289,7 +294,7 @@ class PersistenceUnitReader {
 	 * Parse the <code>property</code> XML elements.
 	 */
 	@SuppressWarnings("unchecked")
-	protected void parseProperty(Element persistenceUnit, SpringPersistenceUnitInfo unitInfo) {
+	protected void parseProperties(Element persistenceUnit, SpringPersistenceUnitInfo unitInfo) {
 		Element propRoot = DomUtils.getChildElementByTagName(persistenceUnit, PROPERTIES);
 		if (propRoot == null) {
 			return;
@@ -306,7 +311,7 @@ class PersistenceUnitReader {
 	 * Parse the <code>class</code> XML elements.
 	 */
 	@SuppressWarnings("unchecked")
-	protected void parseClass(Element persistenceUnit, SpringPersistenceUnitInfo unitInfo) {
+	protected void parseManagedClasses(Element persistenceUnit, SpringPersistenceUnitInfo unitInfo) {
 		List<Element> classes = DomUtils.getChildElementsByTagName(persistenceUnit, MANAGED_CLASS_NAME);
 		for (Element element : classes) {
 			String value = DomUtils.getTextValue(element).trim();
@@ -316,32 +321,47 @@ class PersistenceUnitReader {
 	}
 
 	/**
-	 * Parse the <code>jar-file</code> XML elements.
+	 * Parse the <code>mapping-file</code> XML elements.
 	 */
 	@SuppressWarnings("unchecked")
-	protected void parseJarFiles(Element persistenceUnit, SpringPersistenceUnitInfo unitInfo) throws IOException {
-		List<Element> jars = DomUtils.getChildElementsByTagName(persistenceUnit, JAR_FILE_URL);
-		for (Element element : jars) {
+	protected void parseMappingFiles(Element persistenceUnit, SpringPersistenceUnitInfo unitInfo) {
+		List<Element> files = DomUtils.getChildElementsByTagName(persistenceUnit, MAPPING_FILE_NAME);
+		for (Element element : files) {
 			String value = DomUtils.getTextValue(element).trim();
 			if (StringUtils.hasText(value)) {
-				Resource[] resources = this.resourcePatternResolver.getResources(value);
-				for (Resource resource : resources) {
-					unitInfo.addJarFileUrl(resource.getURL());
-				}
+				unitInfo.addMappingFileName(value);
 			}
 		}
 	}
 
 	/**
-	 * Parse the <code>mapping-file</code> XML elements.
+	 * Parse the <code>jar-file</code> XML elements.
 	 */
 	@SuppressWarnings("unchecked")
-	protected void parseMappingFiles(Element persistenceUnit, SpringPersistenceUnitInfo unitInfo) {
-		List<Element> files = DomUtils.getChildElementsByTagName(persistenceUnit, MAPPING_FILE_NAME);
-		for (Element element : files) {
+	protected void parseJarFiles(Element persistenceUnit, SpringPersistenceUnitInfo unitInfo) throws IOException {
+		List<Element> jars = DomUtils.getChildElementsByTagName(persistenceUnit, JAR_FILE_URL);
+		for (Element element : jars) {
 			String value = DomUtils.getTextValue(element).trim();
 			if (StringUtils.hasText(value)) {
-				unitInfo.addMappingFileName(value);
+				Resource[] resources = this.resourcePatternResolver.getResources(value);
+				boolean found = false;
+				for (Resource resource : resources) {
+					if (resource.exists()) {
+						found = true;
+						unitInfo.addJarFileUrl(resource.getURL());
+					}
+				}
+				if (!found) {
+					// relative to the persistence unit root, according to the JPA spec
+					URL rootUrl = unitInfo.getPersistenceUnitRootUrl();
+					if (rootUrl != null) {
+						unitInfo.addJarFileUrl(new URL(rootUrl, value));
+					}
+					else {
+						logger.warn("Cannot resolve jar-file entry [" + value + "] in persistence unit '" +
+								unitInfo.getPersistenceUnitName() + "' without root URL");
+					}
+				}
 			}
 		}
 	}

spring-orm/src/test/java/org/springframework/orm/jpa/META-INF/persistence.xml

@@ -1,5 +1,8 @@
 <persistence xmlns="http://java.sun.com/xml/ns/persistence" version="1.0">
 
-	<persistence-unit name="OrderManagement"/>
+    <persistence-unit name="OrderManagement">
+        <jar-file>order.jar</jar-file>
+        <jar-file>../../../order-supplemental.jar</jar-file>
+    </persistence-unit>
 
 </persistence>

spring-orm/src/test/java/org/springframework/orm/jpa/persistenceunit/PersistenceXmlParsingTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2010 the original author or authors.
+ * Copyright 2002-2012 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -24,7 +24,6 @@ import javax.persistence.spi.PersistenceUnitInfo;
 import javax.persistence.spi.PersistenceUnitTransactionType;
 import javax.sql.DataSource;
 
-import static org.junit.Assert.*;
 import org.junit.Ignore;
 import org.junit.Test;
 
@@ -37,13 +36,32 @@ import org.springframework.jdbc.datasource.lookup.JndiDataSourceLookup;
 import org.springframework.jdbc.datasource.lookup.MapDataSourceLookup;
 import org.springframework.mock.jndi.SimpleNamingContextBuilder;
 
+import static org.junit.Assert.*;
+
 /**
  * Unit and integration tests for the JPA XML resource parsing support.
  * 
  * @author Costin Leau
+ * @author Juergen Hoeller
  */
 public class PersistenceXmlParsingTests {
 
+	@Test
+	public void testMetaInfCase() throws Exception {
+		PersistenceUnitReader reader = new PersistenceUnitReader(
+				new PathMatchingResourcePatternResolver(), new JndiDataSourceLookup());
+		String resource = "/org/springframework/orm/jpa/META-INF/persistence.xml";
+		PersistenceUnitInfo[] info = reader.readPersistenceUnitInfos(resource);
+
+		assertNotNull(info);
+		assertEquals(1, info.length);
+		assertEquals("OrderManagement", info[0].getPersistenceUnitName());
+
+		assertEquals(2, info[0].getJarFileUrls().size());
+		assertEquals(new ClassPathResource("order.jar").getURL(), info[0].getJarFileUrls().get(0));
+		assertEquals(new ClassPathResource("order-supplemental.jar").getURL(), info[0].getJarFileUrls().get(1));
+	}
+
 	@Test
 	public void testExample1() throws Exception {
 		PersistenceUnitReader reader = new PersistenceUnitReader(
@@ -87,6 +105,7 @@ public class PersistenceXmlParsingTests {
 		assertEquals(2, info[0].getJarFileUrls().size());
 		assertEquals(new ClassPathResource("order.jar").getURL(), info[0].getJarFileUrls().get(0));
 		assertEquals(new ClassPathResource("order-supplemental.jar").getURL(), info[0].getJarFileUrls().get(1));
+
 		assertEquals(0, info[0].getProperties().keySet().size());
 		assertNull(info[0].getJtaDataSource());
 		assertNull(info[0].getNonJtaDataSource());
@@ -120,12 +139,6 @@ public class PersistenceXmlParsingTests {
 		assertSame(PersistenceUnitTransactionType.RESOURCE_LOCAL, info[0].getTransactionType());
 		assertEquals(0, info[0].getProperties().keySet().size());
 
-		// TODO this is undefined as yet. Do we look up Spring datasource?
-		// assertNotNull(info[0].getNonJtaDataSource());
-		//
-		// assertEquals(ds .toString(),
-		// info[0].getNonJtaDataSource().toString());
-
 		builder.clear();
 	}
 
@@ -202,9 +215,6 @@ public class PersistenceXmlParsingTests {
 		assertEquals(1, pu2.getMappingFileNames().size());
 		assertEquals("order2.xml", pu2.getMappingFileNames().get(0));
 
-		@SuppressWarnings("unused")
-		Ignore ignore; // the following assertions fail only during coverage runs
-		/*
 		assertEquals(1, pu2.getJarFileUrls().size());
 		assertEquals(new ClassPathResource("order-supplemental.jar").getURL(), pu2.getJarFileUrls().get(0));
 		assertTrue(pu2.excludeUnlistedClasses());
@@ -213,7 +223,6 @@ public class PersistenceXmlParsingTests {
 
 		// TODO need to define behaviour with non jta datasource
 		assertEquals(ds, pu2.getNonJtaDataSource());
-		*/
 	}
 
 	@Test
@@ -265,7 +274,7 @@ public class PersistenceXmlParsingTests {
 		assertNull(url);
 
 		url = reader.determinePersistenceUnitRootUrl(new ClassPathResource("/org/springframework/orm/jpa/META-INF/persistence.xml"));
-		assertTrue("the containing folder should have been returned", url.toString().endsWith("/org/springframework/orm/jpa/"));
+		assertTrue("the containing folder should have been returned", url.toString().endsWith("/org/springframework/orm/jpa"));
 	}
 	
 	@Test