diff --git a/proposal/myrmidon/etc/testcases/org/apache/antlib/core/property.ant b/proposal/myrmidon/etc/testcases/org/apache/antlib/core/property.ant index 8dc4321a1..03cad74a0 100644 --- a/proposal/myrmidon/etc/testcases/org/apache/antlib/core/property.ant +++ b/proposal/myrmidon/etc/testcases/org/apache/antlib/core/property.ant @@ -50,4 +50,16 @@ + + + + + + + + + + + + diff --git a/proposal/myrmidon/etc/testcases/org/apache/myrmidon/components/builder/bad-project-name.ant b/proposal/myrmidon/etc/testcases/org/apache/myrmidon/components/builder/bad-project-name.ant new file mode 100644 index 000000000..7b0ed6f75 --- /dev/null +++ b/proposal/myrmidon/etc/testcases/org/apache/myrmidon/components/builder/bad-project-name.ant @@ -0,0 +1,4 @@ + + + + diff --git a/proposal/myrmidon/etc/testcases/org/apache/myrmidon/components/builder/bad-target-name.ant b/proposal/myrmidon/etc/testcases/org/apache/myrmidon/components/builder/bad-target-name.ant new file mode 100644 index 000000000..8c7186956 --- /dev/null +++ b/proposal/myrmidon/etc/testcases/org/apache/myrmidon/components/builder/bad-target-name.ant @@ -0,0 +1,5 @@ + + + + + diff --git a/proposal/myrmidon/src/java/org/apache/myrmidon/components/builder/DefaultProjectBuilder.java b/proposal/myrmidon/src/java/org/apache/myrmidon/components/builder/DefaultProjectBuilder.java index 5d0ff6419..22668c3a8 100644 --- a/proposal/myrmidon/src/java/org/apache/myrmidon/components/builder/DefaultProjectBuilder.java +++ b/proposal/myrmidon/src/java/org/apache/myrmidon/components/builder/DefaultProjectBuilder.java @@ -8,6 +8,7 @@ package org.apache.myrmidon.components.builder; import java.io.File; +import java.net.MalformedURLException; import java.net.URL; import java.util.ArrayList; import java.util.HashMap; @@ -22,11 +23,13 @@ import org.apache.avalon.framework.configuration.Configuration; import org.apache.avalon.framework.configuration.ConfigurationException; import org.apache.avalon.framework.configuration.SAXConfigurationHandler; import org.apache.avalon.framework.logger.AbstractLogEnabled; -import org.apache.myrmidon.framework.conditions.Condition; import org.apache.myrmidon.framework.conditions.AndCondition; +import org.apache.myrmidon.framework.conditions.Condition; import org.apache.myrmidon.framework.conditions.IsSetCondition; import org.apache.myrmidon.framework.conditions.NotCondition; import org.apache.myrmidon.interfaces.builder.ProjectBuilder; +import org.apache.myrmidon.interfaces.builder.ProjectException; +import org.apache.myrmidon.interfaces.model.DefaultNameValidator; import org.apache.myrmidon.interfaces.model.Project; import org.apache.myrmidon.interfaces.model.Target; import org.apache.myrmidon.interfaces.model.TypeLib; @@ -37,6 +40,7 @@ import org.xml.sax.XMLReader; * * @author Peter Donald * @version $Revision$ $Date$ + * * @ant:type type="project-builder" name="xml" * @ant:type type="project-builder" name="ant" */ @@ -54,36 +58,37 @@ public class DefaultProjectBuilder private final static int IMPLICIT_TASKS = 2; private final static int TARGETS = 3; + // Use a name validator with the default rules. + private DefaultNameValidator m_nameValidator = new DefaultNameValidator(); + /** * build a project from file. * * @param source the source * @return the constructed Project - * @exception Exception if an error occurs + * @exception ProjectException if an error occurs */ public Project build( final String source ) - throws Exception + throws ProjectException { final File file = new File( source ); return build( file, new HashMap() ); } private Project build( final File file, final HashMap projects ) - throws Exception + throws ProjectException { - final URL systemID = file.toURL(); + final URL systemID = extractURL( file ); final Project result = (Project)projects.get( systemID.toString() ); if( null != result ) { return result; } - final SAXConfigurationHandler handler = new SAXConfigurationHandler(); - - process( systemID, handler ); - - final Configuration configuration = handler.getConfiguration(); + // Parse the project file + final Configuration configuration = parseProject( systemID ); + // Build the project model final DefaultProject project = buildProject( file, configuration ); projects.put( systemID.toString(), project ); @@ -94,20 +99,33 @@ public class DefaultProjectBuilder return project; } - protected void process( final URL systemID, - final SAXConfigurationHandler handler ) - throws Exception + /** + * Parses the project. + */ + private Configuration parseProject( final URL systemID ) + throws ProjectException { - final SAXParserFactory saxParserFactory = SAXParserFactory.newInstance(); - final SAXParser saxParser = saxParserFactory.newSAXParser(); - final XMLReader parser = saxParser.getXMLReader(); - parser.setFeature( "http://xml.org/sax/features/namespace-prefixes", false ); - parser.setFeature( "http://xml.org/sax/features/namespaces", false ); - //parser.setFeature( "http://xml.org/sax/features/validation", false ); - - parser.setContentHandler( handler ); - parser.setErrorHandler( handler ); - parser.parse( systemID.toString() ); + try + { + final SAXConfigurationHandler handler = new SAXConfigurationHandler(); + final SAXParserFactory saxParserFactory = SAXParserFactory.newInstance(); + final SAXParser saxParser = saxParserFactory.newSAXParser(); + final XMLReader parser = saxParser.getXMLReader(); + parser.setFeature( "http://xml.org/sax/features/namespace-prefixes", false ); + parser.setFeature( "http://xml.org/sax/features/namespaces", false ); + //parser.setFeature( "http://xml.org/sax/features/validation", false ); + + parser.setContentHandler( handler ); + parser.setErrorHandler( handler ); + parser.parse( systemID.toString() ); + + return handler.getConfiguration(); + } + catch( Exception e ) + { + String message = REZ.getString( "ant.project-parse.error" ); + throw new ProjectException( message, e ); + } } /** @@ -116,23 +134,20 @@ public class DefaultProjectBuilder * @param file the file from which configuration was loaded * @param configuration the configuration loaded * @return the created Project - * @exception Exception if an error occurs - * @exception Exception if an error occurs - * @exception ConfigurationException if an error occurs + * @exception ProjectException if an error occurs building the project */ private DefaultProject buildProject( final File file, final Configuration configuration ) - throws Exception + throws ProjectException { if( !configuration.getName().equals( "project" ) ) { final String message = REZ.getString( "ant.no-project-element.error" ); - throw new Exception( message ); + throw new ProjectException( message ); } //get project-level attributes - final String projectName = configuration.getAttribute( "name", - FileUtil.removeExtension( file.getName() ) ); + final String projectName = getProjectName( configuration, file ); final String baseDirectoryName = configuration.getAttribute( "basedir", null ); final String defaultTarget = configuration.getAttribute( "default", "main" ); final Version version = getVersion( configuration ); @@ -141,7 +156,7 @@ public class DefaultProjectBuilder { final String message = REZ.getString( "ant.bad-version.error", VERSION, version ); - throw new Exception( message ); + throw new ProjectException( message ); } //determine base directory for project. Use the directory containing @@ -168,12 +183,52 @@ public class DefaultProjectBuilder return project; } + /** + * Get the project name from the configuration, or create a default name if none + * was supplied. + */ + private String getProjectName( final Configuration configuration, final File file ) + throws ProjectException + { + String projectName = configuration.getAttribute( "name", null ); + + if( projectName == null ) + { + // Create a name based on the file name. + String fileNameBase = FileUtil.removeExtension( file.getName() ); + try + { + projectName = m_nameValidator.makeValidName( fileNameBase ); + } + catch( Exception e ) + { + String message = REZ.getString( "ant.project-create-name.error" ); + throw new ProjectException( message, e ); + } + } + else + { + // Make sure the supplied name is valid. + try + { + m_nameValidator.validate( projectName ); + } + catch( Exception e ) + { + String message = REZ.getString( "ant.project-bad-name.error" ); + throw new ProjectException( message, e ); + } + } + return projectName; + + } + /** * Retrieve the version attribute from the specified configuration element. * Throw exceptions with meaningful errors if malformed or missing. */ private Version getVersion( final Configuration configuration ) - throws Exception + throws ProjectException { try { @@ -183,7 +238,7 @@ public class DefaultProjectBuilder catch( final ConfigurationException ce ) { final String message = REZ.getString( "ant.version-missing.error" ); - throw new ConfigurationException( message, ce ); + throw new ProjectException( message, ce ); } } @@ -191,7 +246,7 @@ public class DefaultProjectBuilder * Utility function to extract version */ private Version parseVersion( final String versionString ) - throws Exception + throws ProjectException { try @@ -202,8 +257,7 @@ public class DefaultProjectBuilder { final String message = REZ.getString( "ant.malformed.version", versionString ); - getLogger().warn( message ); - throw new ConfigurationException( message, e ); + throw new ProjectException( message, e ); } } @@ -212,12 +266,12 @@ public class DefaultProjectBuilder * * @param project the project * @param configuration the Configuration - * @exception Exception if an error occurs + * @exception ProjectException if an error occurs */ private void buildTopLevelProject( final DefaultProject project, final Configuration configuration, final HashMap projects ) - throws Exception + throws ProjectException { final ArrayList implicitTaskList = new ArrayList(); final Configuration[] children = configuration.getChildren(); @@ -277,7 +331,7 @@ public class DefaultProjectBuilder { final String message = REZ.getString( "ant.unknown-toplevel-element.error", name, element.getLocation() ); - throw new Exception( message ); + throw new ProjectException( message ); } } @@ -291,7 +345,7 @@ public class DefaultProjectBuilder private void buildProjectRef( final DefaultProject project, final Configuration element, final HashMap projects ) - throws Exception + throws ProjectException { final String name = element.getAttribute( "name", null ); final String location = element.getAttribute( "location", null ); @@ -300,27 +354,31 @@ public class DefaultProjectBuilder { final String message = REZ.getString( "ant.projectref-no-name.error", element.getLocation() ); - throw new Exception( message ); + throw new ProjectException( message ); } - if( !validName( name ) ) + try + { + m_nameValidator.validate( name ); + } + catch( Exception e ) { final String message = REZ.getString( "ant.projectref-bad-name.error", element.getLocation() ); - throw new Exception( message ); + throw new ProjectException( message, e ); } if( null == location ) { final String message = REZ.getString( "ant.projectref-no-location.error", element.getLocation() ); - throw new Exception( message ); + throw new ProjectException( message ); } // Build the URL of the referenced projects final File baseDirectory = project.getBaseDirectory(); final File file = FileUtil.resolveFile( baseDirectory, location ); - final String systemID = file.toURL().toString(); + final String systemID = extractURL( file ).toString(); // Locate the referenced project, building it if necessary Project other = (Project)projects.get( systemID ); @@ -333,9 +391,22 @@ public class DefaultProjectBuilder project.addProject( name, other ); } + private URL extractURL( final File file ) throws ProjectException + { + try + { + return file.toURL(); + } + catch( MalformedURLException e ) + { + final String message = REZ.getString( "ant.project-unexpected.error" ); + throw new ProjectException( message, e ); + } + } + private void buildTypeLib( final DefaultProject project, final Configuration element ) - throws Exception + throws ProjectException { final String library = element.getAttribute( "library", null ); final String name = element.getAttribute( "name", null ); @@ -345,7 +416,7 @@ public class DefaultProjectBuilder { final String message = REZ.getString( "ant.import-no-library.error", element.getLocation() ); - throw new Exception( message ); + throw new ProjectException( message ); } if( null == name || null == type ) @@ -354,7 +425,7 @@ public class DefaultProjectBuilder { final String message = REZ.getString( "ant.import-malformed.error", element.getLocation() ); - throw new Exception( message ); + throw new ProjectException( message ); } } @@ -368,14 +439,14 @@ public class DefaultProjectBuilder * @param target the Configuration */ private void buildTarget( final DefaultProject project, final Configuration target ) - throws Exception + throws ProjectException { final String name = target.getAttribute( "name", null ); final String depends = target.getAttribute( "depends", null ); final String ifCondition = target.getAttribute( "if", null ); final String unlessCondition = target.getAttribute( "unless", null ); - verifyName( name, target ); + verifyTargetName( name, target ); if( getLogger().isDebugEnabled() ) { @@ -392,24 +463,30 @@ public class DefaultProjectBuilder project.addTarget( name, defaultTarget ); } - private void verifyName( final String name, final Configuration target ) throws Exception + private void verifyTargetName( final String name, final Configuration target ) + throws ProjectException { if( null == name ) { final String message = REZ.getString( "ant.target-noname.error", target.getLocation() ); - throw new Exception( message ); + throw new ProjectException( message ); } - if( !validName( name ) ) + try + { + m_nameValidator.validate( name ); + } + catch( Exception e ) { final String message = REZ.getString( "ant.target-bad-name.error", target.getLocation() ); - throw new Exception( message ); + throw new ProjectException( message, e ); } } - private String[] buildDependsList( final String depends, final Configuration target ) throws Exception + private String[] buildDependsList( final String depends, final Configuration target ) + throws ProjectException { String[] dependencies = null; @@ -428,7 +505,7 @@ public class DefaultProjectBuilder final String message = REZ.getString( "ant.target-bad-dependency.error", target.getName(), target.getLocation() ); - throw new Exception( message ); + throw new ProjectException( message ); } if( getLogger().isDebugEnabled() ) @@ -447,7 +524,6 @@ public class DefaultProjectBuilder private Condition buildCondition( final String ifCondition, final String unlessCondition ) - throws Exception { final AndCondition condition = new AndCondition(); @@ -475,16 +551,4 @@ public class DefaultProjectBuilder return condition; } - - protected boolean validName( final String name ) - { - if( -1 != name.indexOf( "->" ) ) - { - return false; - } - else - { - return true; - } - } } diff --git a/proposal/myrmidon/src/java/org/apache/myrmidon/components/builder/Resources.properties b/proposal/myrmidon/src/java/org/apache/myrmidon/components/builder/Resources.properties index 80cc7f97e..e1dc50fe1 100644 --- a/proposal/myrmidon/src/java/org/apache/myrmidon/components/builder/Resources.properties +++ b/proposal/myrmidon/src/java/org/apache/myrmidon/components/builder/Resources.properties @@ -10,10 +10,14 @@ ati.attribue-unquoted.error=Expecting the value of attribute {0} to be enclosed ant.project-banner.notice=Project {0} base directory: {1}. ant.target-parse.notice=Parsing target: {0}. ant.target-if.notice=Target if condition: {0} -ant.target- unless.notice=Target unless condition: {0} +ant.target-unless.notice=Target unless condition: {0} ant.target-dependency.notice=Target dependency: {0} +ant.project-unexpected.error=Unexpected error building project. +ant.project-parse.error=Error parsing project. ant.no-project-element.error=Project file must be enclosed in project element. ant.unknown-toplevel-element.error=Unknown top-level element {0} at {1}. +ant.project-bad-name.error=Invalid project name. +ant.project-create-name.error=Could not create a name for this project. ant.projectref-no-name.error=Malformed projectref without a name attribute at {0}. ant.projectref-bad-name.error=Projectref with an invalid name attribute at {0}. ant.projectref-no-location.error=Malformed projectref without a location attribute at {0}. diff --git a/proposal/myrmidon/src/java/org/apache/myrmidon/components/workspace/DefaultTaskContext.java b/proposal/myrmidon/src/java/org/apache/myrmidon/components/workspace/DefaultTaskContext.java index 5224a34aa..4c153fa12 100644 --- a/proposal/myrmidon/src/java/org/apache/myrmidon/components/workspace/DefaultTaskContext.java +++ b/proposal/myrmidon/src/java/org/apache/myrmidon/components/workspace/DefaultTaskContext.java @@ -20,6 +20,7 @@ import org.apache.avalon.framework.service.ServiceException; import org.apache.avalon.framework.service.ServiceManager; import org.apache.myrmidon.api.TaskContext; import org.apache.myrmidon.api.TaskException; +import org.apache.myrmidon.interfaces.model.DefaultNameValidator; import org.apache.myrmidon.interfaces.workspace.PropertyResolver; /** @@ -34,6 +35,12 @@ public class DefaultTaskContext private final static Resources REZ = ResourceManager.getPackageResources( DefaultTaskContext.class ); + // Property name validator allows digits, but no internal whitespace. + private static DefaultNameValidator m_propertyNameValidator = new DefaultNameValidator(); + static { + m_propertyNameValidator.setAllowInternalWhitespace( false ); + } + private final Map m_contextData = new Hashtable(); private final TaskContext m_parent; private ServiceManager m_serviceManager; @@ -199,6 +206,7 @@ public class DefaultTaskContext public void setProperty( final String name, final Object value ) throws TaskException { + checkPropertyName( name ); checkPropertyValid( name, value ); m_contextData.put( name, value ); } @@ -362,6 +370,22 @@ public class DefaultTaskContext return value; } + /** + * Checks that the supplied property name is valid. + */ + private void checkPropertyName( final String name ) throws TaskException + { + try + { + m_propertyNameValidator.validate( name ); + } + catch( Exception e ) + { + String message = REZ.getString( "bad-property-name.error" ); + throw new TaskException( message, e ); + } + } + /** * Make sure property is valid if it is one of the "magic" properties. * diff --git a/proposal/myrmidon/src/java/org/apache/myrmidon/components/workspace/Resources.properties b/proposal/myrmidon/src/java/org/apache/myrmidon/components/workspace/Resources.properties index c2db45564..e28947723 100644 --- a/proposal/myrmidon/src/java/org/apache/myrmidon/components/workspace/Resources.properties +++ b/proposal/myrmidon/src/java/org/apache/myrmidon/components/workspace/Resources.properties @@ -13,6 +13,7 @@ exec-task.notice=Executing task {0}. #DefaultTaskContext unknown-prop.error=Unknown property {0}. bad-property.error=Property {0} must have a value of type {1}. +bad-property-name.error=Invalid property name. null-resolved-value.error=Value "{0}" resolved to null. bad-resolve.error=Unable to resolve value "{0}". bad-find-service.error=Could not find service "{0}". diff --git a/proposal/myrmidon/src/java/org/apache/myrmidon/interfaces/builder/ProjectBuilder.java b/proposal/myrmidon/src/java/org/apache/myrmidon/interfaces/builder/ProjectBuilder.java index 9fa3bf6c8..0c4a4df38 100644 --- a/proposal/myrmidon/src/java/org/apache/myrmidon/interfaces/builder/ProjectBuilder.java +++ b/proposal/myrmidon/src/java/org/apache/myrmidon/interfaces/builder/ProjectBuilder.java @@ -25,8 +25,8 @@ public interface ProjectBuilder * * @param source the source * @return the constructed Project - * @exception Exception if an error occurs + * @exception ProjectException if an error occurs */ Project build( String source ) - throws Exception; + throws ProjectException; } diff --git a/proposal/myrmidon/src/java/org/apache/myrmidon/interfaces/builder/ProjectException.java b/proposal/myrmidon/src/java/org/apache/myrmidon/interfaces/builder/ProjectException.java new file mode 100644 index 000000000..86b9dc5d9 --- /dev/null +++ b/proposal/myrmidon/src/java/org/apache/myrmidon/interfaces/builder/ProjectException.java @@ -0,0 +1,56 @@ +/* + * Copyright (C) The Apache Software Foundation. All rights reserved. + * + * This software is published under the terms of the Apache Software License + * version 1.1, a copy of which has been included with this distribution in + * the LICENSE.txt file. + */ +package org.apache.myrmidon.interfaces.builder; + +/** + * A cascading exception thrown on a problem constructing a Project model. + * + * @author Darrell DeBoer + * @version $Revision$ $Date$ + */ +public class ProjectException + extends Exception +{ + /** + * If this exception is cascaded, the cause of this exception. + */ + private final Throwable m_throwable; + + /** + * Constructs an non-cascaded exception with a message + * + * @param message the message + */ + public ProjectException( final String message ) + { + this( message, null ); + } + + /** + * Constructs a cascaded exception with the supplied message, which links the + * Throwable provided. + * + * @param message the message + * @param throwable the throwable that caused this exception + */ + public ProjectException( final String message, final Throwable throwable ) + { + super( message ); + m_throwable = throwable; + } + + /** + * Retrieve root cause of the exception. + * + * @return the root cause + */ + public final Throwable getCause() + { + return m_throwable; + } +} diff --git a/proposal/myrmidon/src/java/org/apache/myrmidon/interfaces/model/DefaultNameValidator.java b/proposal/myrmidon/src/java/org/apache/myrmidon/interfaces/model/DefaultNameValidator.java new file mode 100644 index 000000000..eef258c85 --- /dev/null +++ b/proposal/myrmidon/src/java/org/apache/myrmidon/interfaces/model/DefaultNameValidator.java @@ -0,0 +1,356 @@ +/* + * Copyright (C) The Apache Software Foundation. All rights reserved. + * + * This software is published under the terms of the Apache Software License + * version 1.1, a copy of which has been included with this distribution in + * the LICENSE.txt file. + */ +package org.apache.myrmidon.interfaces.model; + +import org.apache.avalon.excalibur.i18n.ResourceManager; +import org.apache.avalon.excalibur.i18n.Resources; + +/** + * Simple helper class which determines the validity of names used + * in ant projects. + * + * @author Darrell DeBoer + * @version $Revision$ $Date$ + */ +public class DefaultNameValidator + implements NameValidator +{ + private final static Resources REZ = + ResourceManager.getPackageResources( DefaultNameValidator.class ); + + /** + * Determines whether the supplied name may include surrounding whitespace. + */ + private boolean m_allowSurroundingWhitespace; + + // Settings for initial characters. + private boolean m_allowInitialDigit; + private String m_additionalInitialCharacters; + + // Settings for internal characters. + private boolean m_allowInternalDigits; + private boolean m_allowInternalWhitespace; + private String m_additionalInternalCharacters; + + /** + * Construct a default name validator. + * Letters, digits and "_" are permitted as initial character. + * Letters, digits, whitespace and "_-." are permitted as internal characters. + * Surrounding whitespace is not permitted. + */ + public DefaultNameValidator() + { + this( false, true, "_", true, true, "_-." ); + } + + /** + * Contstruct a NameValidator with the specified rules. + * @param allowSurroundingWhitespace + * specified if names are trimmed before checking + * @param allowInitialDigit + * specifies if digits are permitted as intial characters + * @param additionalInitialCharacters + * extra characters to allow as initial characters. + * @param allowInternalDigits + * specifies if digits are permitted as internal characters + * @param allowInternalWhitespace + * specifies if whitespace is permitted internally in names + * @param additionalInternalCharacters + * extra characters permitted in names + */ + public DefaultNameValidator( final boolean allowSurroundingWhitespace, + final boolean allowInitialDigit, + final String additionalInitialCharacters, + final boolean allowInternalDigits, + final boolean allowInternalWhitespace, + final String additionalInternalCharacters ) + { + setAllowSurroundingWhitespace( allowSurroundingWhitespace ); + setAllowInitialDigit( allowInitialDigit ); + setAdditionalInitialCharacters( additionalInitialCharacters ); + setAllowInternalDigits( allowInternalDigits ); + setAllowInternalWhitespace( allowInternalWhitespace ); + setAdditionalInternalCharacters( additionalInternalCharacters ); + } + + /** + * Creates a valid name based on the supplied string value, removing invalid + * characters. If no valid characters are present, an exception is thrown. + * @param baseName the name used to construct the valid name + * @throws Exception if no valid name could be constructed. + */ + public String makeValidName( final String baseName ) throws Exception + { + final StringBuffer buffer = new StringBuffer( baseName ); + while( buffer.length() > 0 && !isValidInitialChar( buffer.charAt( 0 ) ) ) + { + buffer.delete( 0, 1 ); + } + if( buffer.length() == 0 ) + { + final String message = REZ.getString( "name.could-not-create.error", baseName ); + throw new Exception( message ); + } + + for( int i = 1; i < buffer.length(); ) + { + if( !isValidInternalChar( buffer.charAt( i ) ) ) + { + buffer.delete( i, i + 1 ); + } + else + { + i++; + } + } + + return buffer.toString(); + } + + /** + * Validates the supplied name, failing if it is not. + * @throws Exception is the supplied name is not valid. + */ + public void validate( final String name ) throws Exception + { + String testName = name; + + // If surrounding whitespace is allowed, trim it. Otherwise, check. + if( m_allowSurroundingWhitespace ) + { + testName = testName.trim(); + } + else + { + checkSurroundingWhitespace( testName ); + } + + // Zero-length name is invalid. + if( testName.length() == 0 ) + { + final String message = REZ.getString( "name.zero-char-name.error" ); + throw new Exception( message ); + } + + // Check first character. + final char initial = testName.charAt( 0 ); + checkInitialCharacter( initial, testName ); + + // Check the rest of the characters. + for( int i = 1; i < testName.length(); i++ ) + { + final char internal = testName.charAt( i ); + checkInternalCharacter( internal, testName ); + } + } + + /** + * Checks if the supplied character is permitted as an internal character. + * @throws Exception if the character is not permitted + */ + private void checkInternalCharacter( final char internal, final String name ) + throws Exception + { + if( !isValidInternalChar( internal ) ) + { + final String message = REZ.getString( "name.invalid-internal-char.error", + name, + describeValidInternalChars() ); + throw new Exception( message ); + } + } + + /** + * Checks if the supplied character is permitted as an internal character. + * @throws Exception if the character is not permitted + */ + private void checkInitialCharacter( final char initial, final String name ) + throws Exception + { + if( !isValidInitialChar( initial ) ) + { + final String message = REZ.getString( "name.invalid-initial-char.error", + name, + describeValidInitialChars() ); + throw new Exception( message ); + } + } + + /** + * Checks the name for surrounding whitespace + * @throws Exception if surrounding whitespace is found + */ + private void checkSurroundingWhitespace( final String testName ) + throws Exception + { + if( testName.length() == 0 ) + { + return; + } + + if( Character.isWhitespace( testName.charAt( 0 ) ) || + Character.isWhitespace( testName.charAt( testName.length() - 1 ) ) ) + { + final String message = + REZ.getString( "name.enclosing-whitespace.error", testName ); + throw new Exception( message ); + } + } + + /** + * Determines if a character is allowed as the first character in a name. + * Valid characters are Letters, Digits, and defined initial characters ("_"). + * @param chr the character to be assessed + * @return true if the character can be the first character of a name + */ + protected boolean isValidInitialChar( final char chr ) + { + if( Character.isLetter( chr ) ) + { + return true; + } + + if( m_allowInitialDigit + && Character.isDigit( chr ) ) + { + return true; + } + + if( m_additionalInitialCharacters.indexOf( chr ) != -1 ) + { + return true; + } + + return false; + } + + /** + * Determines if a character is allowed as a non-initial character in a name. + * Valid characters are Letters, Digits, whitespace, and defined + * internal characters ("_-."). + * @param chr the character to be assessed + * @return true if the character can be included in a name + */ + protected boolean isValidInternalChar( final char chr ) + { + if( Character.isLetter( chr ) ) + { + return true; + } + + if( m_allowInternalDigits + && Character.isDigit( chr ) ) + { + return true; + } + + if( m_allowInternalWhitespace + && Character.isWhitespace( chr ) ) + { + return true; + } + + if( m_additionalInternalCharacters.indexOf( chr ) != -1 ) + { + return true; + } + + return false; + } + + /** + * Builds a message detailing the valid initial characters. + */ + protected String describeValidInitialChars() + { + StringBuffer validChars = new StringBuffer( "letters" ); + if( m_allowInitialDigit ) + { + validChars.append( ", digits" ); + } + validChars.append( ", and \"" ); + validChars.append( m_additionalInitialCharacters ); + validChars.append( "\"" ); + return validChars.toString(); + } + + /** + * Builds a message detailing the valid internal characters. + */ + protected String describeValidInternalChars() + { + StringBuffer validChars = new StringBuffer( "letters" ); + if( m_allowInternalDigits ) + { + validChars.append( ", digits" ); + } + if( m_allowInternalWhitespace ) + { + validChars.append( ", whitespace" ); + } + validChars.append( ", and \"" ); + validChars.append( m_additionalInternalCharacters ); + validChars.append( "\"" ); + return validChars.toString(); + } + + /** + * @param allowSurroundingWhitespace + * specified if names are trimmed before checking + */ + public void setAllowSurroundingWhitespace( boolean allowSurroundingWhitespace ) + { + m_allowSurroundingWhitespace = allowSurroundingWhitespace; + } + + /** + * @param allowInitialDigit + * specifies if digits are permitted as intial characters + */ + public void setAllowInitialDigit( boolean allowInitialDigit ) + { + m_allowInitialDigit = allowInitialDigit; + } + + /** + * @param additionalInitialCharacters + * extra characters to allow as initial characters. + */ + public void setAdditionalInitialCharacters( String additionalInitialCharacters ) + { + m_additionalInitialCharacters = additionalInitialCharacters; + } + + /** + * @param allowInternalDigits + * specifies if digits are permitted as internal characters + */ + public void setAllowInternalDigits( boolean allowInternalDigits ) + { + m_allowInternalDigits = allowInternalDigits; + } + + /** + * @param allowInternalWhitespace + * specifies if whitespace is permitted internally in names + */ + public void setAllowInternalWhitespace( boolean allowInternalWhitespace ) + { + m_allowInternalWhitespace = allowInternalWhitespace; + } + + /** + * @param additionalInternalCharacters + * extra characters permitted in names + */ + public void setAdditionalInternalCharacters( String additionalInternalCharacters ) + { + m_additionalInternalCharacters = additionalInternalCharacters; + } + +} diff --git a/proposal/myrmidon/src/java/org/apache/myrmidon/interfaces/model/NameValidator.java b/proposal/myrmidon/src/java/org/apache/myrmidon/interfaces/model/NameValidator.java new file mode 100644 index 000000000..c6f71af51 --- /dev/null +++ b/proposal/myrmidon/src/java/org/apache/myrmidon/interfaces/model/NameValidator.java @@ -0,0 +1,23 @@ +/* + * Copyright (C) The Apache Software Foundation. All rights reserved. + * + * This software is published under the terms of the Apache Software License + * version 1.1, a copy of which has been included with this distribution in + * the LICENSE.txt file. + */ +package org.apache.myrmidon.interfaces.model; + +/** + * Determines the validity of names used in projects. + * + * @author Darrell DeBoer + * @version $Revision$ $Date$ + */ +public interface NameValidator +{ + /** + * Validates the supplied name, failing if it is not. + * @throws Exception is the supplied name is not valid. + */ + void validate( String name ) throws Exception; +} diff --git a/proposal/myrmidon/src/java/org/apache/myrmidon/interfaces/model/Resources.properties b/proposal/myrmidon/src/java/org/apache/myrmidon/interfaces/model/Resources.properties new file mode 100644 index 000000000..e0d56cdf6 --- /dev/null +++ b/proposal/myrmidon/src/java/org/apache/myrmidon/interfaces/model/Resources.properties @@ -0,0 +1,6 @@ +# Name validation +name.zero-char-name.error=Name "" is invalid, as it contains no characters. +name.enclosing-whitespace.error=Name "{0}" is invalid, as it contains enclosing whitespace. +name.invalid-initial-char.error=Name "{0}" is invalid, as it begins with an illegal character. Names can start with {1}. +name.invalid-internal-char.error=Name "{0}" is invalid, as it contains an illegal character. Permitted name characters are {1}. +name.could-not-create.error=Could not valid name from "{0}". diff --git a/proposal/myrmidon/src/test/org/apache/antlib/core/PropertyTest.java b/proposal/myrmidon/src/test/org/apache/antlib/core/PropertyTest.java index 7158acef7..b53426441 100644 --- a/proposal/myrmidon/src/test/org/apache/antlib/core/PropertyTest.java +++ b/proposal/myrmidon/src/test/org/apache/antlib/core/PropertyTest.java @@ -12,6 +12,7 @@ import org.apache.avalon.excalibur.i18n.ResourceManager; import org.apache.avalon.excalibur.i18n.Resources; import org.apache.myrmidon.AbstractProjectTest; import org.apache.myrmidon.LogMessageTracker; +import org.apache.myrmidon.components.workspace.DefaultTaskContext; /** * Test cases for task. @@ -91,4 +92,26 @@ public class PropertyTest executeTargetExpectError( projectFile, "too-many-values3", messages ); } + /** + * Tests basic validation of property names. + */ + public void testNameValidation() throws Exception + { + final File projectFile = getTestResource( "property.ant" ); + + final Resources contextResources + = ResourceManager.getPackageResources( DefaultTaskContext.class ); + + // Invalid names + String[] messages = new String[] + { + null, + contextResources.getString( "bad-property-name.error" ), + null + }; + executeTargetExpectError( projectFile, "bad-prop-name1", messages ); + executeTargetExpectError( projectFile, "bad-prop-name2", messages ); + executeTargetExpectError( projectFile, "bad-prop-name3", messages ); + } + } diff --git a/proposal/myrmidon/src/test/org/apache/myrmidon/components/builder/DefaultProjectBuilderTest.java b/proposal/myrmidon/src/test/org/apache/myrmidon/components/builder/DefaultProjectBuilderTest.java new file mode 100644 index 000000000..dfbcbcd94 --- /dev/null +++ b/proposal/myrmidon/src/test/org/apache/myrmidon/components/builder/DefaultProjectBuilderTest.java @@ -0,0 +1,70 @@ +/* + * Copyright (C) The Apache Software Foundation. All rights reserved. + * + * This software is published under the terms of the Apache Software License + * version 1.1, a copy of which has been included with this distribution in + * the LICENSE.txt file. + */ +package org.apache.myrmidon.components.builder; + +import java.io.File; +import org.apache.avalon.excalibur.i18n.ResourceManager; +import org.apache.avalon.excalibur.i18n.Resources; +import org.apache.myrmidon.AbstractMyrmidonTest; + +/** + * Test cases for {@link DefaultProjectBuilder}. + * + * @author Darrell DeBoer + * @version $Revision$ $Date$ + */ +public class DefaultProjectBuilderTest + extends AbstractMyrmidonTest +{ + private final static Resources REZ + = ResourceManager.getPackageResources( DefaultProjectBuilder.class ); + + private DefaultProjectBuilder m_builder; + + public DefaultProjectBuilderTest( String name ) + { + super( name ); + } + + protected void setUp() throws Exception + { + super.setUp(); + m_builder = new DefaultProjectBuilder(); + m_builder.enableLogging( createLogger() ); + } + + /** + * Test validation of project and target names. + */ + public void testNameValidation() throws Exception + { + // Check bad project name + final File badProjectFile = getTestResource( "bad-project-name.ant" ); + try + { + m_builder.build( badProjectFile.getAbsolutePath() ); + fail(); + } + catch( Exception e ) + { + assertSameMessage( REZ.getString( "ant.project-bad-name.error" ), e ); + } + + // Check bad target name + final File badTargetFile = getTestResource( "bad-target-name.ant" ); + try + { + m_builder.build( badTargetFile.getAbsolutePath() ); + fail(); + } + catch( Exception e ) + { + // TODO - check error message + } + } +} diff --git a/proposal/myrmidon/src/test/org/apache/myrmidon/interfaces/model/DefaultNameValidatorTest.java b/proposal/myrmidon/src/test/org/apache/myrmidon/interfaces/model/DefaultNameValidatorTest.java new file mode 100644 index 000000000..51d9710ec --- /dev/null +++ b/proposal/myrmidon/src/test/org/apache/myrmidon/interfaces/model/DefaultNameValidatorTest.java @@ -0,0 +1,122 @@ +/* + * Copyright (C) The Apache Software Foundation. All rights reserved. + * + * This software is published under the terms of the Apache Software License + * version 1.1, a copy of which has been included with this distribution in + * the LICENSE.txt file. + */ +package org.apache.myrmidon.interfaces.model; + +import org.apache.myrmidon.AbstractMyrmidonTest; + +/** + * TestCases for {@link DefaultNameValidator}. + * + * @author Darrell DeBoer + * @version $Revision$ $Date$ + */ +public class DefaultNameValidatorTest + extends AbstractMyrmidonTest +{ + private DefaultNameValidator m_validator = new DefaultNameValidator(); + + public DefaultNameValidatorTest( String name ) + { + super( name ); + } + + /** + * Test valid names for the default validator. + */ + public void testValidNames() throws Exception + { + testValid( "aName" ); + testValid( "123456" ); + testValid( "s p a ce s" ); + testValid( "d-a-s-h-e-s-" ); + testValid( "d.o.t.s." ); + testValid( "_u_n_d_e_r_s_c_o_r_e_s" ); + testValid( "a" ); + testValid( "1" ); + testValid( "_" ); + + } + + /** + * Test invalid names for the default validator. + */ + public void testInvalidNames() throws Exception + { + testInvalid( "" ); + testInvalid( " " ); + testInvalid( " " ); + testInvalid( " bad" ); + testInvalid( "bad " ); + testInvalid( " bad " ); + testInvalid( "-dashfirst" ); + testInvalid( ".dotfirst" ); + testInvalid( "question?" ); + } + + /** + * Test that certain characters are disallowed in the default validator. + */ + public void testReservedChars() throws Exception + { + String reserved = "!@#$%^&*()+=~`{}[]|\\/?<>,:;"; + + for( int pos = 0; pos < reserved.length(); pos++ ) + { + char chr = reserved.charAt( pos ); + testReservedChar( chr ); + } + } + + private void testReservedChar( char chr ) throws Exception + { + String test = "a" + String.valueOf( chr ); + testInvalid( test ); + } + + /** + * Test validation using a restrictive set of validation rules. + */ + public void testStrictNames() throws Exception + { + m_validator = new DefaultNameValidator( false, false, "", false, false, "." ); + + testValid( "name" ); + testValid( "a" ); + testValid( "yep.ok" ); + + testInvalid( "_nogood" ); + testInvalid( "no_good" ); + testInvalid( "nope1" ); + testInvalid( "123" ); + testInvalid( "not ok" ); + } + + private void testValid( String name ) + { + try + { + m_validator.validate( name ); + } + catch( Exception e ) + { + fail( e.getMessage() ); + } + } + + private void testInvalid( String name ) + { + try + { + m_validator.validate( name ); + fail( "Name \"" + name + "\" should be invalid." ); + } + catch( Exception e ) + { + } + } +} diff --git a/proposal/myrmidon/src/testcases/org/apache/antlib/core/PropertyTest.java b/proposal/myrmidon/src/testcases/org/apache/antlib/core/PropertyTest.java index 7158acef7..b53426441 100644 --- a/proposal/myrmidon/src/testcases/org/apache/antlib/core/PropertyTest.java +++ b/proposal/myrmidon/src/testcases/org/apache/antlib/core/PropertyTest.java @@ -12,6 +12,7 @@ import org.apache.avalon.excalibur.i18n.ResourceManager; import org.apache.avalon.excalibur.i18n.Resources; import org.apache.myrmidon.AbstractProjectTest; import org.apache.myrmidon.LogMessageTracker; +import org.apache.myrmidon.components.workspace.DefaultTaskContext; /** * Test cases for task. @@ -91,4 +92,26 @@ public class PropertyTest executeTargetExpectError( projectFile, "too-many-values3", messages ); } + /** + * Tests basic validation of property names. + */ + public void testNameValidation() throws Exception + { + final File projectFile = getTestResource( "property.ant" ); + + final Resources contextResources + = ResourceManager.getPackageResources( DefaultTaskContext.class ); + + // Invalid names + String[] messages = new String[] + { + null, + contextResources.getString( "bad-property-name.error" ), + null + }; + executeTargetExpectError( projectFile, "bad-prop-name1", messages ); + executeTargetExpectError( projectFile, "bad-prop-name2", messages ); + executeTargetExpectError( projectFile, "bad-prop-name3", messages ); + } + } diff --git a/proposal/myrmidon/src/testcases/org/apache/myrmidon/components/builder/DefaultProjectBuilderTest.java b/proposal/myrmidon/src/testcases/org/apache/myrmidon/components/builder/DefaultProjectBuilderTest.java new file mode 100644 index 000000000..dfbcbcd94 --- /dev/null +++ b/proposal/myrmidon/src/testcases/org/apache/myrmidon/components/builder/DefaultProjectBuilderTest.java @@ -0,0 +1,70 @@ +/* + * Copyright (C) The Apache Software Foundation. All rights reserved. + * + * This software is published under the terms of the Apache Software License + * version 1.1, a copy of which has been included with this distribution in + * the LICENSE.txt file. + */ +package org.apache.myrmidon.components.builder; + +import java.io.File; +import org.apache.avalon.excalibur.i18n.ResourceManager; +import org.apache.avalon.excalibur.i18n.Resources; +import org.apache.myrmidon.AbstractMyrmidonTest; + +/** + * Test cases for {@link DefaultProjectBuilder}. + * + * @author Darrell DeBoer + * @version $Revision$ $Date$ + */ +public class DefaultProjectBuilderTest + extends AbstractMyrmidonTest +{ + private final static Resources REZ + = ResourceManager.getPackageResources( DefaultProjectBuilder.class ); + + private DefaultProjectBuilder m_builder; + + public DefaultProjectBuilderTest( String name ) + { + super( name ); + } + + protected void setUp() throws Exception + { + super.setUp(); + m_builder = new DefaultProjectBuilder(); + m_builder.enableLogging( createLogger() ); + } + + /** + * Test validation of project and target names. + */ + public void testNameValidation() throws Exception + { + // Check bad project name + final File badProjectFile = getTestResource( "bad-project-name.ant" ); + try + { + m_builder.build( badProjectFile.getAbsolutePath() ); + fail(); + } + catch( Exception e ) + { + assertSameMessage( REZ.getString( "ant.project-bad-name.error" ), e ); + } + + // Check bad target name + final File badTargetFile = getTestResource( "bad-target-name.ant" ); + try + { + m_builder.build( badTargetFile.getAbsolutePath() ); + fail(); + } + catch( Exception e ) + { + // TODO - check error message + } + } +} diff --git a/proposal/myrmidon/src/testcases/org/apache/myrmidon/interfaces/model/DefaultNameValidatorTest.java b/proposal/myrmidon/src/testcases/org/apache/myrmidon/interfaces/model/DefaultNameValidatorTest.java new file mode 100644 index 000000000..51d9710ec --- /dev/null +++ b/proposal/myrmidon/src/testcases/org/apache/myrmidon/interfaces/model/DefaultNameValidatorTest.java @@ -0,0 +1,122 @@ +/* + * Copyright (C) The Apache Software Foundation. All rights reserved. + * + * This software is published under the terms of the Apache Software License + * version 1.1, a copy of which has been included with this distribution in + * the LICENSE.txt file. + */ +package org.apache.myrmidon.interfaces.model; + +import org.apache.myrmidon.AbstractMyrmidonTest; + +/** + * TestCases for {@link DefaultNameValidator}. + * + * @author Darrell DeBoer + * @version $Revision$ $Date$ + */ +public class DefaultNameValidatorTest + extends AbstractMyrmidonTest +{ + private DefaultNameValidator m_validator = new DefaultNameValidator(); + + public DefaultNameValidatorTest( String name ) + { + super( name ); + } + + /** + * Test valid names for the default validator. + */ + public void testValidNames() throws Exception + { + testValid( "aName" ); + testValid( "123456" ); + testValid( "s p a ce s" ); + testValid( "d-a-s-h-e-s-" ); + testValid( "d.o.t.s." ); + testValid( "_u_n_d_e_r_s_c_o_r_e_s" ); + testValid( "a" ); + testValid( "1" ); + testValid( "_" ); + + } + + /** + * Test invalid names for the default validator. + */ + public void testInvalidNames() throws Exception + { + testInvalid( "" ); + testInvalid( " " ); + testInvalid( " " ); + testInvalid( " bad" ); + testInvalid( "bad " ); + testInvalid( " bad " ); + testInvalid( "-dashfirst" ); + testInvalid( ".dotfirst" ); + testInvalid( "question?" ); + } + + /** + * Test that certain characters are disallowed in the default validator. + */ + public void testReservedChars() throws Exception + { + String reserved = "!@#$%^&*()+=~`{}[]|\\/?<>,:;"; + + for( int pos = 0; pos < reserved.length(); pos++ ) + { + char chr = reserved.charAt( pos ); + testReservedChar( chr ); + } + } + + private void testReservedChar( char chr ) throws Exception + { + String test = "a" + String.valueOf( chr ); + testInvalid( test ); + } + + /** + * Test validation using a restrictive set of validation rules. + */ + public void testStrictNames() throws Exception + { + m_validator = new DefaultNameValidator( false, false, "", false, false, "." ); + + testValid( "name" ); + testValid( "a" ); + testValid( "yep.ok" ); + + testInvalid( "_nogood" ); + testInvalid( "no_good" ); + testInvalid( "nope1" ); + testInvalid( "123" ); + testInvalid( "not ok" ); + } + + private void testValid( String name ) + { + try + { + m_validator.validate( name ); + } + catch( Exception e ) + { + fail( e.getMessage() ); + } + } + + private void testInvalid( String name ) + { + try + { + m_validator.validate( name ); + fail( "Name \"" + name + "\" should be invalid." ); + } + catch( Exception e ) + { + } + } +} diff --git a/proposal/myrmidon/src/xdocs/todo.xml b/proposal/myrmidon/src/xdocs/todo.xml index d77ea4d34..620acfb97 100644 --- a/proposal/myrmidon/src/xdocs/todo.xml +++ b/proposal/myrmidon/src/xdocs/todo.xml @@ -209,12 +209,6 @@
  • Fire ProjectListener events projectStarted() and projectFinished() events on start and finish of referenced projects, adding indicator methods to ProjectEvent.
    • -
  • Validate project and target names in DefaultProjectBuilder - reject dodgy - names like "," or "", or " ". Probably want to reject names that start or - end with white-space (though internal whitespace is probably fine). We also - want to reserve certain punctuation characters like , : ? $ [ ] { } < >, etc for - future use.
    • -
  • Similarly, validate property names, using the same rules.
  • Detect duplicate type names.
  • Add fully qualified type names, based on antlib name and type shorthand name. Allow these to be used in build files in addition to the shorthand names.