HHH-6271 Unifying error logger in XMLHelper and standalone ErrorLogger. Removing the former and integrating some features of the latter into ErrorLogger.

Also using a deferred logging approach.

hibernate-core/src/main/java/org/hibernate/cfg/Configuration.java

@@ -61,7 +61,6 @@ import org.dom4j.Element;
 import org.jboss.logging.Logger;
 import org.xml.sax.EntityResolver;
 import org.xml.sax.InputSource;
-import org.xml.sax.SAXParseException;
 
 import org.hibernate.AnnotationException;
 import org.hibernate.DuplicateMappingException;
@@ -105,6 +104,7 @@ import org.hibernate.internal.util.collections.ArrayHelper;
 import org.hibernate.internal.util.collections.CollectionHelper;
 import org.hibernate.internal.util.collections.JoinedIterator;
 import org.hibernate.internal.util.config.ConfigurationHelper;
+import org.hibernate.internal.util.xml.ErrorLogger;
 import org.hibernate.internal.util.xml.MappingReader;
 import org.hibernate.internal.util.xml.Origin;
 import org.hibernate.internal.util.xml.OriginImpl;
@@ -2002,11 +2002,11 @@ public class Configuration implements Serializable {
 	 */
 	protected Configuration doConfigure(InputStream stream, String resourceName) throws HibernateException {
 		try {
-			List errors = new ArrayList<SAXParseException>();
+			ErrorLogger errorLogger = new ErrorLogger( resourceName );
-			Document document = xmlHelper.createSAXReader( resourceName, errors, entityResolver )
+			Document document = xmlHelper.createSAXReader( errorLogger,  entityResolver )
 					.read( new InputSource( stream ) );
-			if ( errors.size() != 0 ) {
+			if ( errorLogger.hasErrors() ) {
-				throw new MappingException( "invalid configuration", (Throwable) errors.get( 0 ) );
+				throw new MappingException( "invalid configuration", errorLogger.getErrors().get( 0 ) );
 			}
 			doConfigure( document );
 		}

hibernate-core/src/main/java/org/hibernate/internal/util/xml/ErrorLogger.java

@@ -24,6 +24,8 @@
 package org.hibernate.internal.util.xml;
 
 import java.io.Serializable;
+import java.util.ArrayList;
+import java.util.List;
 
 import org.jboss.logging.Logger;
 import org.xml.sax.ErrorHandler;
@@ -33,9 +35,10 @@ import org.hibernate.internal.CoreMessageLogger;
 
 /**
  * Implements an {@link ErrorHandler} that mainly just logs errors/warnings.  However, it does track
- * the initial error it encounters and makes it available via {@link #getError}.
+ * the errors it encounters and makes them available via {@link #getErrors}.
  *
  * @author Steve Ebersole
+ * @author Hardy Ferentschik
  */
 public class ErrorLogger implements ErrorHandler, Serializable {
 
@@ -44,26 +47,25 @@ public class ErrorLogger implements ErrorHandler, Serializable {
 			ErrorLogger.class.getName()
 	);
 
-	private SAXParseException error; // capture the initial error
+	// lazily initalized
+	private List<SAXParseException> errors;
+	private String file;
 
-	/**
+	public ErrorLogger() {
-	 * Retrieve the initial error encountered, or null if no error was encountered.
+	}
-	 *
+
-	 * @return The initial error, or null if none.
+	public ErrorLogger(String file) {
-	 */
+		this.file = file;
-	public SAXParseException getError() {
-		return error;
 	}
 
 	/**
 	 * {@inheritDoc}
 	 */
 	public void error(SAXParseException error) {
-		//LOG.parsingXmlError(error.getLineNumber(), error.getMessage());
+		if ( this.errors == null ) {
-		// if error has not been set yet, keep the first error
+			errors = new ArrayList<SAXParseException>();
-		if ( this.error == null ) {
-			this.error = error;
 		}
+		errors.add( error );
 	}
 
 	/**
@@ -77,10 +79,34 @@ public class ErrorLogger implements ErrorHandler, Serializable {
 	 * {@inheritDoc}
 	 */
 	public void warning(SAXParseException warn) {
-		LOG.parsingXmlWarning( error.getLineNumber(), error.getMessage() );
+		LOG.parsingXmlWarning( warn.getLineNumber(), warn.getMessage() );
+	}
+
+	/**
+	 * @return returns a list of encountered xml parsing errors, or the empty list if there was no error
+	 */
+	public List<SAXParseException> getErrors() {
+		return errors;
 	}
 
 	public void reset() {
-		error = null;
+		errors = null;
+	}
+
+	public boolean hasErrors() {
+		return errors != null && errors.size() > 0;
+	}
+
+	public void logErrors() {
+		if ( errors != null ) {
+			for ( SAXParseException e : errors ) {
+				if ( file == null ) {
+					LOG.parsingXmlError( e.getLineNumber(), e.getMessage() );
+				}
+				else {
+					LOG.parsingXmlErrorForFile( file, e.getLineNumber(), e.getMessage() );
+				}
+			}
+		}
 	}
 }

hibernate-core/src/main/java/org/hibernate/internal/util/xml/MappingReader.java

@@ -42,9 +42,11 @@ import org.hibernate.internal.CoreMessageLogger;
  */
 public class MappingReader {
 
-    private static final CoreMessageLogger LOG = Logger.getMessageLogger(CoreMessageLogger.class, MappingReader.class.getName());
+	private static final CoreMessageLogger LOG = Logger.getMessageLogger(
+			CoreMessageLogger.class,
+			MappingReader.class.getName()
+	);
 
-	public static final String ASSUMED_ORM_XSD_VERSION = "2.0";
 	public static final MappingReader INSTANCE = new MappingReader();
 
 	/**
@@ -74,8 +76,8 @@ public class MappingReader {
 			// first try with orm 2.0 xsd validation
 			setValidationFor( saxReader, "orm_2_0.xsd" );
 			document = saxReader.read( source );
-			if ( errorHandler.getError() != null ) {
+			if ( errorHandler.hasErrors() ) {
-				throw errorHandler.getError();
+				throw errorHandler.getErrors().get( 0 );
 			}
 			return new XmlDocumentImpl( document, origin.getType(), origin.getName() );
 		}
@@ -91,8 +93,9 @@ public class MappingReader {
 				try {
 					setValidationFor( saxReader, "orm_1_0.xsd" );
 					document = saxReader.read( new StringReader( document.asXML() ) );
-					if ( errorHandler.getError() != null ) {
+					if ( errorHandler.hasErrors() ) {
-						throw errorHandler.getError();
+						errorHandler.logErrors();
+						throw errorHandler.getErrors().get( 0 );
 					}
 					return new XmlDocumentImpl( document, origin.getType(), origin.getName() );
 				}

hibernate-core/src/main/java/org/hibernate/internal/util/xml/XMLHelper.java

@@ -23,46 +23,36 @@
  */
 package org.hibernate.internal.util.xml;
 
-import java.util.List;
-
 import org.dom4j.DocumentFactory;
 import org.dom4j.Element;
 import org.dom4j.io.DOMReader;
 import org.dom4j.io.OutputFormat;
 import org.dom4j.io.SAXReader;
 import org.dom4j.io.XMLWriter;
-import org.jboss.logging.Logger;
 import org.xml.sax.EntityResolver;
 import org.xml.sax.ErrorHandler;
-import org.xml.sax.SAXParseException;
-
-import org.hibernate.internal.CoreMessageLogger;
-
 
 /**
  * Small helper class that lazy loads DOM and SAX reader and keep them for fast use afterwards.
  */
 public final class XMLHelper {
 
-    private static final CoreMessageLogger LOG = Logger.getMessageLogger(CoreMessageLogger.class, XMLHelper.class.getName());
-
 	public static final EntityResolver DEFAULT_DTD_RESOLVER = new DTDEntityResolver();
 
 	private DOMReader domReader;
 	private SAXReader saxReader;
 
 	/**
-	 * @param file the file name of the xml file to parse
+	 * @param errorHandler the sax error handler
-	 * @param errorsList a list to which to add all occurring errors
 	 * @param entityResolver an xml entity resolver
 	 *
 	 * @return Create and return a dom4j {@code SAXReader} which will append all validation errors
 	 *         to the passed error list
 	 */
-	public SAXReader createSAXReader(String file, List<SAXParseException> errorsList, EntityResolver entityResolver) {
+	public SAXReader createSAXReader(ErrorHandler errorHandler, EntityResolver entityResolver) {
 		SAXReader saxReader = resolveSAXReader();
 		saxReader.setEntityResolver( entityResolver );
-		saxReader.setErrorHandler( new ErrorLogger(file, errorsList) );
+		saxReader.setErrorHandler( errorHandler );
 		return saxReader;
 	}
 
@@ -79,30 +69,12 @@ public final class XMLHelper {
 	 * @return create and return a dom4j DOMReader
 	 */
 	public DOMReader createDOMReader() {
-		if (domReader==null) domReader = new DOMReader();
+		if ( domReader == null ) {
+			domReader = new DOMReader();
+		}
 		return domReader;
 	}
 
-	public static class ErrorLogger implements ErrorHandler {
-		private String file;
-		private List<SAXParseException> errors;
-
-		private ErrorLogger(String file, List<SAXParseException> errors) {
-			this.file=file;
-			this.errors = errors;
-		}
-		public void error(SAXParseException error) {
-            LOG.parsingXmlErrorForFile(file, error.getLineNumber(), error.getMessage());
-			errors.add(error);
-		}
-		public void fatalError(SAXParseException error) {
-			error(error);
-		}
-		public void warning(SAXParseException warn) {
-            LOG.parsingXmlWarningForFile(file, warn.getLineNumber(), warn.getMessage());
-		}
-	}
-
 	public static Element generateDom4jElement(String elementName) {
 		return getDocumentFactory().createElement( elementName );
 	}
@@ -124,8 +96,8 @@ public final class XMLHelper {
 	public static void dump(Element element) {
 		try {
 			// try to "pretty print" it
-			OutputFormat outformat = OutputFormat.createPrettyPrint();
+			OutputFormat outFormat = OutputFormat.createPrettyPrint();
-			XMLWriter writer = new XMLWriter( System.out, outformat );
+			XMLWriter writer = new XMLWriter( System.out, outFormat );
 			writer.write( element );
 			writer.flush();
 			System.out.println( "" );

hibernate-core/src/test/java/org/hibernate/test/annotations/reflection/JPAOverriddenAnnotationReaderTest.java

@@ -28,8 +28,6 @@ import java.io.IOException;
 import java.io.InputStream;
 import java.lang.reflect.Field;
 import java.lang.reflect.Method;
-import java.util.ArrayList;
-import java.util.List;
 import javax.persistence.AssociationOverrides;
 import javax.persistence.AttributeOverrides;
 import javax.persistence.Basic;
@@ -89,6 +87,7 @@ import org.hibernate.annotations.Columns;
 import org.hibernate.cfg.EJB3DTDEntityResolver;
 import org.hibernate.cfg.annotations.reflection.JPAOverriddenAnnotationReader;
 import org.hibernate.cfg.annotations.reflection.XMLContext;
+import org.hibernate.internal.util.xml.ErrorLogger;
 import org.hibernate.internal.util.xml.XMLHelper;
 import org.hibernate.testing.junit4.BaseUnitTestCase;
 
@@ -101,7 +100,7 @@ import static org.junit.Assert.assertTrue;
 /**
  * @author Emmanuel Bernard
  */
-public class JPAOverridenAnnotationReaderTest extends BaseUnitTestCase {
+public class JPAOverriddenAnnotationReaderTest extends BaseUnitTestCase {
 	@Test
 	public void testMappedSuperclassAnnotations() throws Exception {
 		XMLContext context = buildContext(
@@ -121,8 +120,8 @@ public class JPAOverridenAnnotationReaderTest extends BaseUnitTestCase {
 				reader.getAnnotation( Entity.class ).name()
 		);
 		assertNotNull( reader.getAnnotation( Table.class ) );
-		assertEquals( "@Table not overriden", "tbl_admin", reader.getAnnotation( Table.class ).name() );
+		assertEquals( "@Table not overridden", "tbl_admin", reader.getAnnotation( Table.class ).name() );
-		assertEquals( "Default schema not overriden", "myschema", reader.getAnnotation( Table.class ).schema() );
+		assertEquals( "Default schema not overridden", "myschema", reader.getAnnotation( Table.class ).schema() );
 		assertEquals(
 				"Proper @Table.uniqueConstraints", 2,
 				reader.getAnnotation( Table.class ).uniqueConstraints()[0].columnNames().length
@@ -450,8 +449,8 @@ public class JPAOverridenAnnotationReaderTest extends BaseUnitTestCase {
 		InputStream is = cl.getResourceAsStream( ormfile );
 		assertNotNull( "ORM.xml not found: " + ormfile, is );
 		XMLContext context = new XMLContext();
-		List errors = new ArrayList();
+		ErrorLogger errorLogger = new ErrorLogger();
-		SAXReader saxReader = xmlHelper.createSAXReader( "XML InputStream", errors, EJB3DTDEntityResolver.INSTANCE );
+		SAXReader saxReader = xmlHelper.createSAXReader( errorLogger, EJB3DTDEntityResolver.INSTANCE );
 		//saxReader.setValidation( false );
 		try {
 			saxReader.setFeature( "http://apache.org/xml/features/validation/schema", true );
@@ -461,16 +460,15 @@ public class JPAOverridenAnnotationReaderTest extends BaseUnitTestCase {
 		}
 		org.dom4j.Document doc;
 		try {
-			doc = saxReader
+			doc = saxReader.read( new InputSource( new BufferedInputStream( is ) ) );
-					.read( new InputSource( new BufferedInputStream( is ) ) );
 		}
 		finally {
 			is.close();
 		}
-		if ( errors.size() > 0 ) {
+		if ( errorLogger.hasErrors() ) {
-			System.out.println( errors.get( 0 ) );
+			System.out.println( errorLogger.getErrors().get( 0 ) );
 		}
-		assertEquals( 0, errors.size() );
+		assertFalse( errorLogger.hasErrors() );
 		context.addDocument( doc );
 		return context;
 	}

hibernate-core/src/test/java/org/hibernate/test/annotations/reflection/XMLContextTest.java

@@ -1,10 +1,31 @@
-//$Id$
+/*
+ * Hibernate, Relational Persistence for Idiomatic Java
+ *
+ * Copyright (c) 2011 by Red Hat Inc and/or its affiliates or by
+ * third-party contributors as indicated by either @author tags or express
+ * copyright attribution statements applied by the authors.  All
+ * third-party contributions are distributed under license by Red Hat Inc.
+ *
+ * This copyrighted material is made available to anyone wishing to use, modify,
+ * copy, or redistribute it subject to the terms and conditions of the GNU
+ * Lesser General Public License, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+ * or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU Lesser General Public License
+ * for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this distribution; if not, write to:
+ * Free Software Foundation, Inc.
+ * 51 Franklin Street, Fifth Floor
+ * Boston, MA  02110-1301  USA
+ */
 package org.hibernate.test.annotations.reflection;
+
 import java.io.BufferedInputStream;
 import java.io.IOException;
 import java.io.InputStream;
-import java.util.ArrayList;
-import java.util.List;
 
 import org.dom4j.io.SAXReader;
 import org.junit.Assert;
@@ -14,6 +35,7 @@ import org.xml.sax.SAXNotSupportedException;
 
 import org.hibernate.cfg.EJB3DTDEntityResolver;
 import org.hibernate.cfg.annotations.reflection.XMLContext;
+import org.hibernate.internal.util.xml.ErrorLogger;
 import org.hibernate.internal.util.xml.XMLHelper;
 
 /**
@@ -29,8 +51,8 @@ public class XMLContextTest {
 		);
 		Assert.assertNotNull( "ORM.xml not found", is );
 		XMLContext context = new XMLContext();
-		List errors = new ArrayList();
+		ErrorLogger errorLogger = new ErrorLogger();
-		SAXReader saxReader = xmlHelper.createSAXReader( "XML InputStream", errors, EJB3DTDEntityResolver.INSTANCE );
+		SAXReader saxReader = xmlHelper.createSAXReader( errorLogger, EJB3DTDEntityResolver.INSTANCE );
 		//saxReader.setValidation( false );
 		try {
 			saxReader.setFeature( "http://apache.org/xml/features/validation/schema", true );
@@ -51,7 +73,7 @@ public class XMLContextTest {
 				//log.warn( "Could not close input stream", ioe );
 			}
 		}
-		Assert.assertEquals( 0, errors.size() );
+		Assert.assertFalse( errorLogger.hasErrors() );
 		context.addDocument( doc );
 	}
 }