From fa1183f3f9a93385f77edeeee3b9e33e276fd5dd Mon Sep 17 00:00:00 2001 From: Hardy Ferentschik Date: Thu, 15 Mar 2012 16:55:34 +0100 Subject: [PATCH] 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. --- .../java/org/hibernate/cfg/Configuration.java | 10 +-- .../internal/util/xml/ErrorLogger.java | 56 +++++++++---- .../internal/util/xml/MappingReader.java | 19 +++-- .../internal/util/xml/XMLHelper.java | 78 ++++++------------- ...=> JPAOverriddenAnnotationReaderTest.java} | 22 +++--- .../reflection/XMLContextTest.java | 42 +++++++--- 6 files changed, 124 insertions(+), 103 deletions(-) rename hibernate-core/src/test/java/org/hibernate/test/annotations/reflection/{JPAOverridenAnnotationReaderTest.java => JPAOverriddenAnnotationReaderTest.java} (97%) diff --git a/hibernate-core/src/main/java/org/hibernate/cfg/Configuration.java b/hibernate-core/src/main/java/org/hibernate/cfg/Configuration.java index dcd6b594f2..8a2cda8050 100644 --- a/hibernate-core/src/main/java/org/hibernate/cfg/Configuration.java +++ b/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(); - Document document = xmlHelper.createSAXReader( resourceName, errors, entityResolver ) + ErrorLogger errorLogger = new ErrorLogger( resourceName ); + Document document = xmlHelper.createSAXReader( errorLogger, entityResolver ) .read( new InputSource( stream ) ); - if ( errors.size() != 0 ) { - throw new MappingException( "invalid configuration", (Throwable) errors.get( 0 ) ); + if ( errorLogger.hasErrors() ) { + throw new MappingException( "invalid configuration", errorLogger.getErrors().get( 0 ) ); } doConfigure( document ); } diff --git a/hibernate-core/src/main/java/org/hibernate/internal/util/xml/ErrorLogger.java b/hibernate-core/src/main/java/org/hibernate/internal/util/xml/ErrorLogger.java index 3ff099be08..acbb0762a2 100644 --- a/hibernate-core/src/main/java/org/hibernate/internal/util/xml/ErrorLogger.java +++ b/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 errors; + private String file; - /** - * Retrieve the initial error encountered, or null if no error was encountered. - * - * @return The initial error, or null if none. - */ - public SAXParseException getError() { - return error; + public ErrorLogger() { + } + + public ErrorLogger(String file) { + this.file = file; } /** * {@inheritDoc} */ public void error(SAXParseException error) { - //LOG.parsingXmlError(error.getLineNumber(), error.getMessage()); - // if error has not been set yet, keep the first error - if ( this.error == null ) { - this.error = error; + if ( this.errors == null ) { + errors = new ArrayList(); } + 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 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() ); + } + } + } } } diff --git a/hibernate-core/src/main/java/org/hibernate/internal/util/xml/MappingReader.java b/hibernate-core/src/main/java/org/hibernate/internal/util/xml/MappingReader.java index 445da2c986..d40e2b3eb4 100644 --- a/hibernate-core/src/main/java/org/hibernate/internal/util/xml/MappingReader.java +++ b/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 ) { - throw errorHandler.getError(); + if ( errorHandler.hasErrors() ) { + throw errorHandler.getErrors().get( 0 ); } return new XmlDocumentImpl( document, origin.getType(), origin.getName() ); } @@ -90,15 +92,16 @@ public class MappingReader { // next try with orm 1.0 xsd validation try { setValidationFor( saxReader, "orm_1_0.xsd" ); - document = saxReader.read( new StringReader( document.asXML() ) ); - if ( errorHandler.getError() != null ) { - throw errorHandler.getError(); + document = saxReader.read( new StringReader( document.asXML() ) ); + if ( errorHandler.hasErrors() ) { + errorHandler.logErrors(); + throw errorHandler.getErrors().get( 0 ); } return new XmlDocumentImpl( document, origin.getType(), origin.getName() ); } catch ( Exception orm1Problem ) { if ( LOG.isDebugEnabled() ) { - LOG.debugf("Problem parsing XML using orm 1 xsd : %s", orm1Problem.getMessage()); + LOG.debugf( "Problem parsing XML using orm 1 xsd : %s", orm1Problem.getMessage() ); } } } diff --git a/hibernate-core/src/main/java/org/hibernate/internal/util/xml/XMLHelper.java b/hibernate-core/src/main/java/org/hibernate/internal/util/xml/XMLHelper.java index 36a9d6c817..1d3e889c48 100644 --- a/hibernate-core/src/main/java/org/hibernate/internal/util/xml/XMLHelper.java +++ b/hibernate-core/src/main/java/org/hibernate/internal/util/xml/XMLHelper.java @@ -23,54 +23,44 @@ */ 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 errorsList a list to which to add all occurring errors + * @param errorHandler the sax error handler * @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 + * to the passed error list */ - public SAXReader createSAXReader(String file, List errorsList, EntityResolver entityResolver) { + public SAXReader createSAXReader(ErrorHandler errorHandler, EntityResolver entityResolver) { SAXReader saxReader = resolveSAXReader(); - saxReader.setEntityResolver(entityResolver); - saxReader.setErrorHandler( new ErrorLogger(file, errorsList) ); + saxReader.setEntityResolver( entityResolver ); + saxReader.setErrorHandler( errorHandler ); return saxReader; } private SAXReader resolveSAXReader() { if ( saxReader == null ) { saxReader = new SAXReader(); - saxReader.setMergeAdjacentText(true); - saxReader.setValidation(true); + saxReader.setMergeAdjacentText( true ); + saxReader.setValidation( true ); } return saxReader; } @@ -79,58 +69,40 @@ 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 errors; - - private ErrorLogger(String file, List 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 ); } - public static DocumentFactory getDocumentFactory() { + public static DocumentFactory getDocumentFactory() { - ClassLoader cl = Thread.currentThread().getContextClassLoader(); - DocumentFactory factory; - try { - Thread.currentThread().setContextClassLoader( XMLHelper.class.getClassLoader() ); - factory = DocumentFactory.getInstance(); - } - finally { - Thread.currentThread().setContextClassLoader( cl ); - } - return factory; - } + ClassLoader cl = Thread.currentThread().getContextClassLoader(); + DocumentFactory factory; + try { + Thread.currentThread().setContextClassLoader( XMLHelper.class.getClassLoader() ); + factory = DocumentFactory.getInstance(); + } + finally { + Thread.currentThread().setContextClassLoader( cl ); + } + return factory; + } public static void dump(Element element) { try { // try to "pretty print" it - OutputFormat outformat = OutputFormat.createPrettyPrint(); - XMLWriter writer = new XMLWriter( System.out, outformat ); + OutputFormat outFormat = OutputFormat.createPrettyPrint(); + XMLWriter writer = new XMLWriter( System.out, outFormat ); writer.write( element ); writer.flush(); System.out.println( "" ); } - catch( Throwable t ) { + catch ( Throwable t ) { // otherwise, just dump it System.out.println( element.asXML() ); } diff --git a/hibernate-core/src/test/java/org/hibernate/test/annotations/reflection/JPAOverridenAnnotationReaderTest.java b/hibernate-core/src/test/java/org/hibernate/test/annotations/reflection/JPAOverriddenAnnotationReaderTest.java similarity index 97% rename from hibernate-core/src/test/java/org/hibernate/test/annotations/reflection/JPAOverridenAnnotationReaderTest.java rename to hibernate-core/src/test/java/org/hibernate/test/annotations/reflection/JPAOverriddenAnnotationReaderTest.java index 8a1215ad6c..b60afe7bb4 100644 --- a/hibernate-core/src/test/java/org/hibernate/test/annotations/reflection/JPAOverridenAnnotationReaderTest.java +++ b/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( "Default schema not overriden", "myschema", reader.getAnnotation( Table.class ).schema() ); + assertEquals( "@Table not overridden", "tbl_admin", reader.getAnnotation( Table.class ).name() ); + 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(); - SAXReader saxReader = xmlHelper.createSAXReader( "XML InputStream", errors, EJB3DTDEntityResolver.INSTANCE ); + ErrorLogger errorLogger = new ErrorLogger(); + 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 - .read( new InputSource( new BufferedInputStream( is ) ) ); + doc = saxReader.read( new InputSource( new BufferedInputStream( is ) ) ); } finally { is.close(); } - if ( errors.size() > 0 ) { - System.out.println( errors.get( 0 ) ); + if ( errorLogger.hasErrors() ) { + System.out.println( errorLogger.getErrors().get( 0 ) ); } - assertEquals( 0, errors.size() ); + assertFalse( errorLogger.hasErrors() ); context.addDocument( doc ); return context; } diff --git a/hibernate-core/src/test/java/org/hibernate/test/annotations/reflection/XMLContextTest.java b/hibernate-core/src/test/java/org/hibernate/test/annotations/reflection/XMLContextTest.java index 4b48fd9cc6..f7f5cf18ec 100644 --- a/hibernate-core/src/test/java/org/hibernate/test/annotations/reflection/XMLContextTest.java +++ b/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,28 +35,29 @@ 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; /** * @author Emmanuel Bernard */ public class XMLContextTest { - @Test + @Test public void testAll() throws Exception { XMLHelper xmlHelper = new XMLHelper(); ClassLoader cl = Thread.currentThread().getContextClassLoader(); InputStream is = cl.getResourceAsStream( "org/hibernate/test/annotations/reflection/orm.xml" ); - Assert.assertNotNull( "ORM.xml not found", is ); + Assert.assertNotNull( "ORM.xml not found", is ); XMLContext context = new XMLContext(); - List errors = new ArrayList(); - SAXReader saxReader = xmlHelper.createSAXReader( "XML InputStream", errors, EJB3DTDEntityResolver.INSTANCE ); + ErrorLogger errorLogger = new ErrorLogger(); + SAXReader saxReader = xmlHelper.createSAXReader( errorLogger, EJB3DTDEntityResolver.INSTANCE ); //saxReader.setValidation( false ); try { saxReader.setFeature( "http://apache.org/xml/features/validation/schema", true ); } - catch (SAXNotSupportedException e) { + catch ( SAXNotSupportedException e ) { saxReader.setValidation( false ); } org.dom4j.Document doc; @@ -47,11 +69,11 @@ public class XMLContextTest { try { is.close(); } - catch (IOException ioe) { + catch ( IOException ioe ) { //log.warn( "Could not close input stream", ioe ); } } - Assert.assertEquals( 0, errors.size() ); + Assert.assertFalse( errorLogger.hasErrors() ); context.addDocument( doc ); } }