Fixed misunderstanding with respect to excludeUnlistedClasses default in JPA 2.0

Issue: SPR-10767

spring-core/src/main/java/org/springframework/util/xml/DomUtils.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2012 the original author or authors.
+ * Copyright 2002-2013 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.
@@ -39,18 +39,17 @@ import org.springframework.util.Assert;
  * @author Costin Leau
  * @author Arjen Poutsma
  * @author Luke Taylor
+ * @since 1.2
  * @see org.w3c.dom.Node
  * @see org.w3c.dom.Element
- * @since 1.2
  */
 public abstract class DomUtils {
 
 	/**
-	 * Retrieve all child elements of the given DOM element that match any of the given element names. Only look at the
-	 * direct child level of the given element; do not go into further depth (in contrast to the DOM API's
-	 * {@code getElementsByTagName} method).
-	 *
-	 * @param ele           the DOM element to analyze
+	 * Retrieves all child elements of the given DOM element that match any of the given element names.
+	 * Only looks at the direct child level of the given element; do not go into further depth
+	 * (in contrast to the DOM API's {@code getElementsByTagName} method).
+	 * @param ele the DOM element to analyze
 	 * @param childEleNames the child element names to look for
 	 * @return a List of child {@code org.w3c.dom.Element} instances
 	 * @see org.w3c.dom.Element
@@ -72,11 +71,10 @@ public abstract class DomUtils {
 	}
 
 	/**
-	 * Retrieve all child elements of the given DOM element that match the given element name. Only look at the direct
-	 * child level of the given element; do not go into further depth (in contrast to the DOM API's
-	 * {@code getElementsByTagName} method).
-	 *
-	 * @param ele          the DOM element to analyze
+	 * Retrieves all child elements of the given DOM element that match the given element name.
+	 * Only look at the direct child level of the given element; do not go into further depth
+	 * (in contrast to the DOM API's {@code getElementsByTagName} method).
+	 * @param ele the DOM element to analyze
 	 * @param childEleName the child element name to look for
 	 * @return a List of child {@code org.w3c.dom.Element} instances
 	 * @see org.w3c.dom.Element
@@ -88,8 +86,7 @@ public abstract class DomUtils {
 
 	/**
 	 * Utility method that returns the first child element identified by its name.
-	 *
-	 * @param ele          the DOM element to analyze
+	 * @param ele the DOM element to analyze
 	 * @param childEleName the child element name to look for
 	 * @return the {@code org.w3c.dom.Element} instance, or {@code null} if none found
 	 */
@@ -108,8 +105,7 @@ public abstract class DomUtils {
 
 	/**
 	 * Utility method that returns the first child element value identified by its name.
-	 *
-	 * @param ele          the DOM element to analyze
+	 * @param ele the DOM element to analyze
 	 * @param childEleName the child element name to look for
 	 * @return the extracted text value, or {@code null} if no child element found
 	 */
@@ -119,9 +115,8 @@ public abstract class DomUtils {
 	}
 
 	/**
-	 * Retrieve all child elements of the given DOM element
-
-	 * @param ele           the DOM element to analyze
+	 * Retrieves all child elements of the given DOM element
+	 * @param ele the DOM element to analyze
 	 * @return a List of child {@code org.w3c.dom.Element} instances
 	 */
 	public static List<Element> getChildElements(Element ele) {
@@ -138,9 +133,10 @@ public abstract class DomUtils {
 	}
 
 	/**
-	 * Extract the text value from the given DOM element, ignoring XML comments. <p>Appends all CharacterData nodes and
-	 * EntityReference nodes into a single String value, excluding Comment nodes.
-	 *
+	 * Extracts the text value from the given DOM element, ignoring XML comments.
+	 * <p>Appends all CharacterData nodes and EntityReference nodes into a single
+	 * String value, excluding Comment nodes. Only exposes actual user-specified
+	 * text, no default values of any kind.
 	 * @see CharacterData
 	 * @see EntityReference
 	 * @see Comment
@@ -159,8 +155,9 @@ public abstract class DomUtils {
 	}
 
 	/**
-	 * Namespace-aware equals comparison. Returns {@code true} if either {@link Node#getLocalName} or {@link
-	 * Node#getNodeName} equals {@code desiredName}, otherwise returns {@code false}.
+	 * Namespace-aware equals comparison. Returns {@code true} if either
+	 * {@link Node#getLocalName} or {@link Node#getNodeName} equals
+	 * {@code desiredName}, otherwise returns {@code false}.
 	 */
 	public static boolean nodeNameEquals(Node node, String desiredName) {
 		Assert.notNull(node, "Node must not be null");
@@ -170,7 +167,6 @@ public abstract class DomUtils {
 
 	/**
 	 * Returns a SAX {@code ContentHandler} that transforms callback calls to DOM {@code Node}s.
-	 *
 	 * @param node the node to publish events to
 	 * @return the content handler
 	 */
@@ -178,12 +174,16 @@ public abstract class DomUtils {
 		return new DomContentHandler(node);
 	}
 
-	/** Matches the given node's name and local name against the given desired name. */
+	/**
+	 * Matches the given node's name and local name against the given desired name.
+	 */
 	private static boolean nodeNameMatch(Node node, String desiredName) {
 		return (desiredName.equals(node.getNodeName()) || desiredName.equals(node.getLocalName()));
 	}
 
-	/** Matches the given node's name and local name against the given desired names. */
+	/**
+	 * Matches the given node's name and local name against the given desired names.
+	 */
 	private static boolean nodeNameMatch(Node node, Collection desiredNames) {
 		return (desiredNames.contains(node.getNodeName()) || desiredNames.contains(node.getLocalName()));
 	}

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2012 the original author or authors.
+ * Copyright 2002-2013 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.
@@ -21,7 +21,6 @@ import java.io.InputStream;
 import java.net.URL;
 import java.util.LinkedList;
 import java.util.List;
-
 import javax.persistence.spi.PersistenceUnitTransactionType;
 import javax.xml.parsers.DocumentBuilder;
 import javax.xml.parsers.DocumentBuilderFactory;
@@ -29,6 +28,11 @@ import javax.xml.parsers.ParserConfigurationException;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
+import org.w3c.dom.Document;
+import org.w3c.dom.Element;
+import org.xml.sax.ErrorHandler;
+import org.xml.sax.SAXException;
+
 import org.springframework.core.io.Resource;
 import org.springframework.core.io.support.ResourcePatternResolver;
 import org.springframework.jdbc.datasource.lookup.DataSourceLookup;
@@ -37,10 +41,6 @@ import org.springframework.util.ResourceUtils;
 import org.springframework.util.StringUtils;
 import org.springframework.util.xml.DomUtils;
 import org.springframework.util.xml.SimpleSaxErrorHandler;
-import org.w3c.dom.Document;
-import org.w3c.dom.Element;
-import org.xml.sax.ErrorHandler;
-import org.xml.sax.SAXException;
 
 /**
  * Internal helper class for reading JPA-compliant {@code persistence.xml} files.
@@ -81,8 +81,6 @@ class PersistenceUnitReader {
 
 	private static final String META_INF = "META-INF";
 
-	private static final String VERSION_1 = "1.0";
-
 
 	private final Log logger = LogFactory.getLog(getClass());
 
@@ -249,7 +247,7 @@ class PersistenceUnitReader {
 			unitInfo.setTransactionType(PersistenceUnitTransactionType.valueOf(txType));
 		}
 
-		// data-source
+		// evaluate data sources
 		String jtaDataSource = DomUtils.getChildElementValueByTagName(persistenceUnit, JTA_DATA_SOURCE);
 		if (StringUtils.hasText(jtaDataSource)) {
 			unitInfo.setJtaDataSource(this.dataSourceLookup.getDataSource(jtaDataSource.trim()));
@@ -268,14 +266,9 @@ class PersistenceUnitReader {
 
 		// exclude unlisted classes
 		Element excludeUnlistedClasses = DomUtils.getChildElementByTagName(persistenceUnit, EXCLUDE_UNLISTED_CLASSES);
-		if (excludeUnlistedClasses == null) {
-			// element is not defined, use default appropriate for version
-			unitInfo.setExcludeUnlistedClasses(!VERSION_1.equals(version));
-		}
-		else {
+		if (excludeUnlistedClasses != null) {
 			String excludeText = DomUtils.getTextValue(excludeUnlistedClasses);
-			unitInfo.setExcludeUnlistedClasses(StringUtils.isEmpty(excludeText) ||
-					Boolean.valueOf(excludeText));
+			unitInfo.setExcludeUnlistedClasses(!StringUtils.hasText(excludeText) || Boolean.valueOf(excludeText));
 		}
 
 		// set JPA 2.0 shared cache mode

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

@@ -351,7 +351,7 @@ public class PersistenceXmlParsingTests {
 		PersistenceUnitInfo noExclude = info[0];
 		assertNotNull("noExclude should not be null.", noExclude);
 		assertEquals("noExclude name is not correct.", "NoExcludeElement", noExclude.getPersistenceUnitName());
-		assertTrue("Exclude unlisted should default true in 2.0.", noExclude.excludeUnlistedClasses());
+		assertFalse("Exclude unlisted still defaults to false in 2.0.", noExclude.excludeUnlistedClasses());
 
 		PersistenceUnitInfo emptyExclude = info[1];
 		assertNotNull("emptyExclude should not be null.", emptyExclude);