Skip to content

Commit

Permalink
Fixed mojohaus#20.
Browse files Browse the repository at this point in the history
When using Validator.validate, most validation errors will be ignored. The only reliable way to capture
validation errors is to use an error handler, as Validator.validate only throws fatal errors (validation
errors are not fatal - see javadoc for ErrorHandler)

Solution applied refactors existing error handling approach to always use an ErrorHandler to capture errors.
As a bonus, you should get log messages showing multiple failures, rather than just the first.
  • Loading branch information
rosslamont committed Aug 28, 2017
2 parents 7312af1 + 7049405 commit b940a44
Show file tree
Hide file tree
Showing 3 changed files with 234 additions and 75 deletions.
154 changes: 84 additions & 70 deletions src/main/java/org/codehaus/mojo/xml/ValidateMojo.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

import java.io.File;
import java.io.IOException;
import java.util.List;

import javax.xml.XMLConstants;
import javax.xml.parsers.ParserConfigurationException;
Expand All @@ -36,6 +37,7 @@
import org.apache.maven.plugins.annotations.LifecyclePhase;
import org.apache.maven.plugins.annotations.Mojo;
import org.apache.maven.plugins.annotations.Parameter;
import org.codehaus.mojo.xml.validation.ValidationErrorHandler;
import org.codehaus.mojo.xml.validation.ValidationSet;
import org.xml.sax.ErrorHandler;
import org.xml.sax.InputSource;
Expand Down Expand Up @@ -129,20 +131,22 @@ private Schema getSchema( Resolver pResolver, ValidationSet pValidationSet )
* @param pFile The file to parse or validate.
* @throws MojoExecutionException Parsing or validating the file failed.
*/
private void validate( final Resolver pResolver, ValidationSet pValidationSet, Schema pSchema, File pFile )
private void validate( final Resolver pResolver, ValidationSet pValidationSet, Schema pSchema, File pFile, ValidationErrorHandler errorHandler )
throws MojoExecutionException
{
errorHandler.setContext(pFile);
try
{
if ( pSchema == null )
{
getLog().debug( "Parsing " + pFile.getPath() );
parse( pResolver, pValidationSet, pFile );
parse( pResolver, pValidationSet, pFile ,errorHandler);
}
else
{
getLog().debug( "Validating " + pFile.getPath() );
Validator validator = pSchema.newValidator();
validator.setErrorHandler(errorHandler);
if ( pResolver != null )
{
validator.setResourceResolver( pResolver );
Expand All @@ -157,7 +161,7 @@ private void validate( final Resolver pResolver, ValidationSet pValidationSet, S
InputSource isource = new InputSource( pFile.toURI().toASCIIString() );
XMLReader xmlReader = spf.newSAXParser().getXMLReader();
xmlReader.setEntityResolver( pResolver );

validator.validate( new SAXSource( xmlReader, isource ) );
}
else
Expand All @@ -168,50 +172,12 @@ private void validate( final Resolver pResolver, ValidationSet pValidationSet, S
}
catch ( SAXParseException e )
{
final String publicId = e.getPublicId();
final String systemId = e.getSystemId();
final int lineNum = e.getLineNumber();
final int colNum = e.getColumnNumber();
final String location;
if ( publicId == null && systemId == null && lineNum == -1 && colNum == -1 )
{
location = "";
try{
errorHandler.fatalError(e);
}
else
{
final StringBuffer loc = new StringBuffer();
String sep = "";
if ( publicId != null )
{
loc.append( "Public ID " );
loc.append( publicId );
sep = ", ";
}
if ( systemId != null )
{
loc.append( sep );
loc.append( systemId );
sep = ", ";
}
if ( lineNum != -1 )
{
loc.append( sep );
loc.append( "line " );
loc.append( lineNum );
sep = ", ";
}
if ( colNum != -1 )
{
loc.append( sep );
loc.append( " column " );
loc.append( colNum );
sep = ", ";
}
location = loc.toString();
catch(SAXException se){
throw new MojoExecutionException( "While parsing " + pFile + ": " + e.getMessage(), se );
}
final String msg = "While parsing " + pFile.getPath() + ( "".equals( location ) ? "" : ", at " + location )
+ ": " + e.getMessage();
throw new MojoExecutionException( msg, e );
}
catch ( Exception e )
{
Expand Down Expand Up @@ -273,35 +239,15 @@ private SAXParserFactory newSAXParserFactory( ValidationSet pValidationSet )
* @throws SAXException Parsing the file failed.
* @throws ParserConfigurationException Creating an XML parser failed.
*/
private void parse( Resolver pResolver, ValidationSet pValidationSet, File pFile )
private void parse( Resolver pResolver, ValidationSet pValidationSet, File pFile , ErrorHandler errorHandler)
throws IOException, SAXException, ParserConfigurationException
{
XMLReader xr = newSAXParserFactory( pValidationSet ).newSAXParser().getXMLReader();
if ( pResolver != null )
{
xr.setEntityResolver( pResolver );
}
xr.setErrorHandler( new ErrorHandler()
{
public void error( SAXParseException pException )
throws SAXException
{
throw pException;
}

public void fatalError( SAXParseException pException )
throws SAXException
{
throw pException;
}

public void warning( SAXParseException pException )
throws SAXException
{
throw pException;
}

} );
xr.setErrorHandler( errorHandler );
xr.parse( pFile.toURI().toURL().toExternalForm() );
}

Expand All @@ -313,7 +259,7 @@ public void warning( SAXParseException pException )
* @throws MojoExecutionException Validating the set of files failed.
* @throws MojoFailureException A configuration error was detected.
*/
private void validate( Resolver pResolver, ValidationSet pValidationSet )
private void validate( Resolver pResolver, ValidationSet pValidationSet,ValidationErrorHandler errorHandler )
throws MojoExecutionException, MojoFailureException
{
final Schema schema = getSchema( pResolver, pValidationSet );
Expand All @@ -327,7 +273,7 @@ private void validate( Resolver pResolver, ValidationSet pValidationSet )
}
for ( int i = 0; i < files.length; i++ )
{
validate( pResolver, pValidationSet, schema, files[i] );
validate( pResolver, pValidationSet, schema, files[i],errorHandler );
}
}

Expand All @@ -346,6 +292,7 @@ public void execute()
return;
}

final ValidationErrorHandler errorHandler = new ValidationErrorHandler();
if ( validationSets == null || validationSets.length == 0 )
{
throw new MojoFailureException( "No ValidationSets configured." );
Expand All @@ -360,12 +307,79 @@ public void execute()
ValidationSet validationSet = validationSets[i];
resolver.setXincludeAware(validationSet.isValidating() );
resolver.setValidating( validationSet.isValidating() );
validate( resolver, validationSet );
validate( resolver, validationSet ,errorHandler);
}
List<ValidationErrorHandler.ErrorRecord> errorRecords=errorHandler.getErrors();
if (!errorRecords.isEmpty()){
final StringBuffer message=new StringBuffer();
for(ValidationErrorHandler.ErrorRecord error:errorRecords){
appendMessage(message,error);
}
if (errorHandler.getErrorCount()+errorHandler.getFatalCount()>0){
throw new MojoExecutionException( message.toString());
}
else{
getLog().warn(message.toString());
}
}
}
finally
{
passivateProxy( oldProxySettings );
}
}

private void appendMessage(StringBuffer messageBuffer,ValidationErrorHandler.ErrorRecord error) {
SAXParseException e = error.getException();
final String publicId = e.getPublicId();
final String systemId = e.getSystemId();
final int lineNum = e.getLineNumber();
final int colNum = e.getColumnNumber();
final String location;
if ( publicId == null && systemId == null && lineNum == -1 && colNum == -1 )
{
location = "";
}
else
{
final StringBuffer loc = new StringBuffer();
String sep = "";
if ( publicId != null )
{
loc.append( "Public ID " );
loc.append( publicId );
sep = ", ";
}
if ( systemId != null )
{
loc.append( sep );
loc.append( systemId );
sep = ", ";
}
if ( lineNum != -1 )
{
loc.append( sep );
loc.append( "line " );
loc.append( lineNum );
sep = ", ";
}
if ( colNum != -1 )
{
loc.append( sep );
loc.append( " column " );
loc.append( colNum );
sep = ", ";
}
location = loc.toString();
}
messageBuffer.append("While parsing ");
messageBuffer.append(error.getContext().getPath());
messageBuffer.append(( "".equals( location ) ? "" : ", at " + location ));
messageBuffer.append(": " );
messageBuffer.append(error.getType().toString());
messageBuffer.append(": " );
messageBuffer.append(e.getMessage());
messageBuffer.append(System.lineSeparator());

}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,144 @@
/*
* Copyright 2017 The Apache Software Foundation.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.codehaus.mojo.xml.validation;

import java.io.File;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import org.xml.sax.ErrorHandler;
import org.xml.sax.SAXException;
import org.xml.sax.SAXParseException;

/**
*
* @author rlamont
*/
public class ValidationErrorHandler implements ErrorHandler{

private final List<ErrorRecord> errors=new ArrayList<ErrorRecord>();
private final List<ErrorRecord> publicErrors = Collections.unmodifiableList(errors);
private int warningCount=0;
private int errorCount=0;
private int fatalCount=0;
private File context;

@Override
public void warning(SAXParseException exception) throws SAXException {
warningCount++;
errors.add(new ErrorRecord(ErrorType.WARNING, exception,context));
}

@Override
public void error(SAXParseException exception) throws SAXException {
errorCount++;
errors.add(new ErrorRecord(ErrorType.ERROR, exception,context));
}

@Override
public void fatalError(SAXParseException exception) throws SAXException {
fatalCount++;
errors.add(new ErrorRecord(ErrorType.FATAL, exception,context));
}

public List<ErrorRecord> getErrors(){
return publicErrors;
}

public int getWarningCount() {
return warningCount;
}

public int getErrorCount() {
return errorCount;
}

public int getFatalCount() {
return fatalCount;
}

public void setContext(File context) {
this.context = context;
}





public enum ErrorType{
WARNING{
@Override
public String toString() {
return "warning";
}

},
ERROR{
@Override
public String toString() {
return "error";
}

},
FATAL{
@Override
public String toString() {
return "fatal error";
}

}
}

public class ErrorRecord{
final ErrorType type;
final SAXParseException exception;
final File context;

public ErrorRecord(ErrorType type, SAXParseException exception,File context) {
this.type = type;
this.exception = exception;
this.context=context;
}

public boolean isError() {
return type==ErrorType.ERROR;
}

public boolean isWarning() {
return type==ErrorType.WARNING;
}

public boolean isFatal() {
return type==ErrorType.ERROR;
}

public ErrorType getType(){
return type;
}

public SAXParseException getException() {
return exception;
}

public File getContext() {
return context;
}




}
}
Loading

0 comments on commit b940a44

Please sign in to comment.