You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maven.apache.org by mi...@apache.org on 2019/07/10 19:28:48 UTC
[maven] branch master updated: [MNG-6702] Improve
DefaultModelValidator performance:
This is an automated email from the ASF dual-hosted git repository.
michaelo pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/maven.git
The following commit(s) were added to refs/heads/master by this push:
new 9b8ae7d [MNG-6702] Improve DefaultModelValidator performance:
9b8ae7d is described below
commit 9b8ae7d5281cbb4e789e50fb42ae42734a54a6dd
Author: Guillaume Nodet <gn...@gmail.com>
AuthorDate: Fri Jun 28 10:44:10 2019 +0200
[MNG-6702] Improve DefaultModelValidator performance:
Avoid tons of string concatenations that are done but usually not used
This closes #268
---
.../model/validation/DefaultModelValidator.java | 279 +++++++++++++--------
.../validation/DefaultModelValidatorTest.java | 6 +-
2 files changed, 176 insertions(+), 109 deletions(-)
diff --git a/maven-model-builder/src/main/java/org/apache/maven/model/validation/DefaultModelValidator.java b/maven-model-builder/src/main/java/org/apache/maven/model/validation/DefaultModelValidator.java
index 551bd2a..cfa6e20 100644
--- a/maven-model-builder/src/main/java/org/apache/maven/model/validation/DefaultModelValidator.java
+++ b/maven-model-builder/src/main/java/org/apache/maven/model/validation/DefaultModelValidator.java
@@ -82,6 +82,10 @@ public class DefaultModelValidator
private static final String ILLEGAL_REPO_ID_CHARS = ILLEGAL_FS_CHARS;
+ private static final String EMPTY = "";
+
+ private final Set<String> validIds = new HashSet<>();
+
@Override
public void validateRawModel( Model m, ModelBuildingRequest request, ModelProblemCollector problems )
{
@@ -142,7 +146,7 @@ public class DefaultModelValidator
validateStringNotEmpty( "version", problems, Severity.FATAL, Version.V20, m.getVersion(), m );
}
- validate20RawDependencies( problems, m.getDependencies(), "dependencies.dependency", request );
+ validate20RawDependencies( problems, m.getDependencies(), "dependencies.dependency.", EMPTY, request );
validate20RawDependenciesSelfReferencing( problems, m, m.getDependencies(), "dependencies.dependency",
request );
@@ -150,24 +154,24 @@ public class DefaultModelValidator
if ( m.getDependencyManagement() != null )
{
validate20RawDependencies( problems, m.getDependencyManagement().getDependencies(),
- "dependencyManagement.dependencies.dependency", request );
+ "dependencyManagement.dependencies.dependency.", EMPTY, request );
}
- validateRawRepositories( problems, m.getRepositories(), "repositories.repository", request );
+ validateRawRepositories( problems, m.getRepositories(), "repositories.repository.", EMPTY, request );
- validateRawRepositories( problems, m.getPluginRepositories(), "pluginRepositories.pluginRepository",
- request );
+ validateRawRepositories( problems, m.getPluginRepositories(), "pluginRepositories.pluginRepository.",
+ EMPTY, request );
Build build = m.getBuild();
if ( build != null )
{
- validate20RawPlugins( problems, build.getPlugins(), "build.plugins.plugin", request );
+ validate20RawPlugins( problems, build.getPlugins(), "build.plugins.plugin.", EMPTY, request );
PluginManagement mgmt = build.getPluginManagement();
if ( mgmt != null )
{
- validate20RawPlugins( problems, mgmt.getPlugins(), "build.pluginManagement.plugins.plugin",
- request );
+ validate20RawPlugins( problems, mgmt.getPlugins(), "build.pluginManagement.plugins.plugin.",
+ EMPTY, request );
}
}
@@ -175,7 +179,7 @@ public class DefaultModelValidator
for ( Profile profile : m.getProfiles() )
{
- String prefix = "profiles.profile[" + profile.getId() + "]";
+ String prefix = "profiles.profile[" + profile.getId() + "].";
if ( !profileIds.add( profile.getId() ) )
{
@@ -184,32 +188,32 @@ public class DefaultModelValidator
}
validate30RawProfileActivation( problems, profile.getActivation(), profile.getId(),
- prefix + ".activation", request );
+ prefix, "activation", request );
- validate20RawDependencies( problems, profile.getDependencies(), prefix + ".dependencies.dependency",
+ validate20RawDependencies( problems, profile.getDependencies(), prefix, "dependencies.dependency.",
request );
if ( profile.getDependencyManagement() != null )
{
validate20RawDependencies( problems, profile.getDependencyManagement().getDependencies(),
- prefix + ".dependencyManagement.dependencies.dependency", request );
+ prefix, "dependencyManagement.dependencies.dependency.", request );
}
- validateRawRepositories( problems, profile.getRepositories(), prefix + ".repositories.repository",
+ validateRawRepositories( problems, profile.getRepositories(), prefix, "repositories.repository.",
request );
validateRawRepositories( problems, profile.getPluginRepositories(),
- prefix + ".pluginRepositories.pluginRepository", request );
+ prefix, "pluginRepositories.pluginRepository.", request );
BuildBase buildBase = profile.getBuild();
if ( buildBase != null )
{
- validate20RawPlugins( problems, buildBase.getPlugins(), prefix + ".plugins.plugin", request );
+ validate20RawPlugins( problems, buildBase.getPlugins(), prefix, "plugins.plugin.", request );
PluginManagement mgmt = buildBase.getPluginManagement();
if ( mgmt != null )
{
- validate20RawPlugins( problems, mgmt.getPlugins(), prefix + ".pluginManagement.plugins.plugin",
+ validate20RawPlugins( problems, mgmt.getPlugins(), prefix, "pluginManagement.plugins.plugin.",
request );
}
}
@@ -218,7 +222,8 @@ public class DefaultModelValidator
}
private void validate30RawProfileActivation( ModelProblemCollector problems, Activation activation,
- String sourceHint, String prefix, ModelBuildingRequest request )
+ String sourceHint, String prefix, String fieldName,
+ ModelBuildingRequest request )
{
if ( activation == null )
{
@@ -250,7 +255,7 @@ public class DefaultModelValidator
if ( path.contains( "${project.basedir}" ) )
{
addViolation( problems, Severity.WARNING, Version.V30,
- prefix + ( missing ? ".file.missing" : ".file.exists" ), null,
+ prefix + fieldName + ( missing ? ".file.missing" : ".file.exists" ), null,
"Failed to interpolate file location " + path + " for profile " + sourceHint
+ ": ${project.basedir} expression not supported during profile activation, "
+ "use ${basedir} instead",
@@ -259,7 +264,7 @@ public class DefaultModelValidator
else if ( hasProjectExpression( path ) )
{
addViolation( problems, Severity.WARNING, Version.V30,
- prefix + ( missing ? ".file.missing" : ".file.exists" ), null,
+ prefix + fieldName + ( missing ? ".file.missing" : ".file.exists" ), null,
"Failed to interpolate file location " + path + " for profile " + sourceHint
+ ": ${project.*} expressions are not supported during profile activation",
file.getLocation( missing ? "missing" : "exists" ) );
@@ -268,7 +273,7 @@ public class DefaultModelValidator
}
private void validate20RawPlugins( ModelProblemCollector problems, List<Plugin> plugins, String prefix,
- ModelBuildingRequest request )
+ String prefix2, ModelBuildingRequest request )
{
Severity errOn31 = getSeverity( request, ModelBuildingRequest.VALIDATION_LEVEL_MAVEN_3_1 );
@@ -279,21 +284,21 @@ public class DefaultModelValidator
if ( plugin.getGroupId() == null
|| ( plugin.getGroupId() != null && plugin.getGroupId().trim().isEmpty() ) )
{
- addViolation( problems, Severity.FATAL, Version.V20, prefix + ".(groupId:artifactId)", null,
+ addViolation( problems, Severity.FATAL, Version.V20, prefix + prefix2 + "(groupId:artifactId)", null,
"groupId of a plugin must be defined. ", plugin );
}
if ( plugin.getArtifactId() == null
|| ( plugin.getArtifactId() != null && plugin.getArtifactId().trim().isEmpty() ) )
{
- addViolation( problems, Severity.FATAL, Version.V20, prefix + ".(groupId:artifactId)", null,
+ addViolation( problems, Severity.FATAL, Version.V20, prefix + prefix2 + "(groupId:artifactId)", null,
"artifactId of a plugin must be defined. ", plugin );
}
// This will catch cases like <version></version> or <version/>
if ( plugin.getVersion() != null && plugin.getVersion().trim().isEmpty() )
{
- addViolation( problems, Severity.FATAL, Version.V20, prefix + ".(groupId:artifactId)", null,
+ addViolation( problems, Severity.FATAL, Version.V20, prefix + prefix2 + "(groupId:artifactId)", null,
"version of a plugin must be defined. ", plugin );
}
@@ -303,7 +308,7 @@ public class DefaultModelValidator
if ( existing != null )
{
- addViolation( problems, errOn31, Version.V20, prefix + ".(groupId:artifactId)", null,
+ addViolation( problems, errOn31, Version.V20, prefix + prefix2 + "(groupId:artifactId)", null,
"must be unique but found duplicate declaration of plugin " + key, plugin );
}
else
@@ -318,7 +323,7 @@ public class DefaultModelValidator
if ( !executionIds.add( exec.getId() ) )
{
addViolation( problems, Severity.ERROR, Version.V20,
- prefix + "[" + plugin.getKey() + "].executions.execution.id", null,
+ prefix + prefix2 + "[" + plugin.getKey() + "].executions.execution.id", null,
"must be unique but found duplicate execution with id " + exec.getId(), exec );
}
}
@@ -383,7 +388,7 @@ public class DefaultModelValidator
Severity errOn31 = getSeverity( request, ModelBuildingRequest.VALIDATION_LEVEL_MAVEN_3_1 );
- validateBannedCharacters( "version", problems, errOn31, Version.V20, m.getVersion(), null, m,
+ validateBannedCharacters( EMPTY, "version", problems, errOn31, Version.V20, m.getVersion(), null, m,
ILLEGAL_VERSION_CHARS );
validate20ProperSnapshotVersion( "version", problems, errOn31, Version.V20, m.getVersion(), null, m );
@@ -401,18 +406,18 @@ public class DefaultModelValidator
validate20PluginVersion( "build.plugins.plugin.version", problems, p.getVersion(), p.getKey(), p,
request );
- validateBoolean( "build.plugins.plugin.inherited", problems, errOn30, Version.V20, p.getInherited(),
- p.getKey(), p );
+ validateBoolean( "build.plugins.plugin.inherited", EMPTY, problems, errOn30, Version.V20,
+ p.getInherited(), p.getKey(), p );
- validateBoolean( "build.plugins.plugin.extensions", problems, errOn30, Version.V20,
+ validateBoolean( "build.plugins.plugin.extensions", EMPTY, problems, errOn30, Version.V20,
p.getExtensions(), p.getKey(), p );
validate20EffectivePluginDependencies( problems, p, request );
}
- validate20RawResources( problems, build.getResources(), "build.resources.resource", request );
+ validate20RawResources( problems, build.getResources(), "build.resources.resource.", request );
- validate20RawResources( problems, build.getTestResources(), "build.testResources.testResource",
+ validate20RawResources( problems, build.getTestResources(), "build.testResources.testResource.",
request );
}
@@ -431,12 +436,12 @@ public class DefaultModelValidator
for ( Repository repository : m.getRepositories() )
{
- validate20EffectiveRepository( problems, repository, "repositories.repository", request );
+ validate20EffectiveRepository( problems, repository, "repositories.repository.", request );
}
for ( Repository repository : m.getPluginRepositories() )
{
- validate20EffectiveRepository( problems, repository, "pluginRepositories.pluginRepository", request );
+ validate20EffectiveRepository( problems, repository, "pluginRepositories.pluginRepository.", request );
}
DistributionManagement distMgmt = m.getDistributionManagement();
@@ -448,16 +453,16 @@ public class DefaultModelValidator
"must not be specified.", distMgmt );
}
- validate20EffectiveRepository( problems, distMgmt.getRepository(), "distributionManagement.repository",
+ validate20EffectiveRepository( problems, distMgmt.getRepository(), "distributionManagement.repository.",
request );
validate20EffectiveRepository( problems, distMgmt.getSnapshotRepository(),
- "distributionManagement.snapshotRepository", request );
+ "distributionManagement.snapshotRepository.", request );
}
}
}
private void validate20RawDependencies( ModelProblemCollector problems, List<Dependency> dependencies,
- String prefix, ModelBuildingRequest request )
+ String prefix, String prefix2, ModelBuildingRequest request )
{
Severity errOn30 = getSeverity( request, ModelBuildingRequest.VALIDATION_LEVEL_MAVEN_3_0 );
Severity errOn31 = getSeverity( request, ModelBuildingRequest.VALIDATION_LEVEL_MAVEN_3_1 );
@@ -472,12 +477,12 @@ public class DefaultModelValidator
{
if ( !"pom".equals( dependency.getType() ) )
{
- addViolation( problems, Severity.WARNING, Version.V20, prefix + ".type", key,
+ addViolation( problems, Severity.WARNING, Version.V20, prefix + prefix2 + "type", key,
"must be 'pom' to import the managed dependencies.", dependency );
}
else if ( StringUtils.isNotEmpty( dependency.getClassifier() ) )
{
- addViolation( problems, errOn30, Version.V20, prefix + ".classifier", key,
+ addViolation( problems, errOn30, Version.V20, prefix + prefix2 + "classifier", key,
"must be empty, imported POM cannot have a classifier.", dependency );
}
}
@@ -486,7 +491,7 @@ public class DefaultModelValidator
if ( request.getValidationLevel() >= ModelBuildingRequest.VALIDATION_LEVEL_MAVEN_3_1 )
{
- addViolation( problems, Severity.WARNING, Version.V31, prefix + ".scope", key,
+ addViolation( problems, Severity.WARNING, Version.V31, prefix + prefix2 + "scope", key,
"declares usage of deprecated 'system' scope ", dependency );
}
@@ -495,12 +500,12 @@ public class DefaultModelValidator
{
if ( !hasExpression( sysPath ) )
{
- addViolation( problems, Severity.WARNING, Version.V20, prefix + ".systemPath", key,
+ addViolation( problems, Severity.WARNING, Version.V20, prefix + prefix2 + "systemPath", key,
"should use a variable instead of a hard-coded path " + sysPath, dependency );
}
else if ( sysPath.contains( "${basedir}" ) || sysPath.contains( "${project.basedir}" ) )
{
- addViolation( problems, Severity.WARNING, Version.V20, prefix + ".systemPath", key,
+ addViolation( problems, Severity.WARNING, Version.V20, prefix + prefix2 + "systemPath", key,
"should not point at files within the project directory, " + sysPath
+ " will be unresolvable by dependent projects",
dependency );
@@ -510,7 +515,7 @@ public class DefaultModelValidator
if ( equals( "LATEST", dependency.getVersion() ) || equals( "RELEASE", dependency.getVersion() ) )
{
- addViolation( problems, Severity.WARNING, Version.BASE, prefix + ".version", key,
+ addViolation( problems, Severity.WARNING, Version.BASE, prefix + prefix2 + "version", key,
"is either LATEST or RELEASE (both of them are being deprecated)", dependency );
}
@@ -530,8 +535,8 @@ public class DefaultModelValidator
+ StringUtils.defaultString( dependency.getVersion(), "(?)" );
}
- addViolation( problems, errOn31, Version.V20, prefix + ".(groupId:artifactId:type:classifier)", null,
- "must be unique: " + key + " -> " + msg, dependency );
+ addViolation( problems, errOn31, Version.V20, prefix + prefix2 + "(groupId:artifactId:type:classifier)",
+ null, "must be unique: " + key + " -> " + msg, dependency );
}
else
{
@@ -552,15 +557,15 @@ public class DefaultModelValidator
for ( Dependency dependency : dependencies )
{
String key = dependency.getGroupId() + ":" + dependency.getArtifactId() + ":" + dependency.getVersion()
- + ( dependency.getClassifier() != null ? ":" + dependency.getClassifier() : "" );
+ + ( dependency.getClassifier() != null ? ":" + dependency.getClassifier() : EMPTY );
String mKey = m.getGroupId() + ":" + m.getArtifactId() + ":" + m.getVersion();
if ( key.equals( mKey ) )
{
// This means a module which is build has a dependency which has the same
// groupId, artifactId, version and classifier coordinates. This is in consequence
// a self reference or in other words a circular reference which can not being resolved.
- addViolation( problems, Severity.FATAL, Version.V31, prefix + " " + key, key, "is referencing itself.",
- dependency );
+ addViolation( problems, Severity.FATAL, Version.V31, prefix + "[" + key + "]", key,
+ "is referencing itself.", dependency );
}
}
@@ -579,26 +584,26 @@ public class DefaultModelValidator
if ( request.getValidationLevel() >= ModelBuildingRequest.VALIDATION_LEVEL_MAVEN_2_0 )
{
- validateBoolean( prefix + "optional", problems, errOn30, Version.V20, d.getOptional(),
+ validateBoolean( prefix, "optional", problems, errOn30, Version.V20, d.getOptional(),
d.getManagementKey(), d );
if ( !management )
{
- validateVersion( prefix + "version", problems, errOn30, Version.V20, d.getVersion(),
+ validateVersion( prefix, "version", problems, errOn30, Version.V20, d.getVersion(),
d.getManagementKey(), d );
/*
* TODO Extensions like Flex Mojos use custom scopes like "merged", "internal", "external", etc. In
* order to don't break backward-compat with those, only warn but don't error out.
*/
- validateEnum( prefix + "scope", problems, Severity.WARNING, Version.V20, d.getScope(),
+ validateEnum( prefix, "scope", problems, Severity.WARNING, Version.V20, d.getScope(),
d.getManagementKey(), d, "provided", "compile", "runtime", "test", "system" );
validateEffectiveModelAgainstDependency( prefix, problems, m, d, request );
}
else
{
- validateEnum( prefix + "scope", problems, Severity.WARNING, Version.V20, d.getScope(),
+ validateEnum( prefix, "scope", problems, Severity.WARNING, Version.V20, d.getScope(),
d.getManagementKey(), d, "provided", "compile", "runtime", "test", "system",
"import" );
}
@@ -610,14 +615,15 @@ public class DefaultModelValidator
Dependency d, ModelBuildingRequest request )
{
String key = d.getGroupId() + ":" + d.getArtifactId() + ":" + d.getVersion()
- + ( d.getClassifier() != null ? ":" + d.getClassifier() : "" );
+ + ( d.getClassifier() != null ? ":" + d.getClassifier() : EMPTY );
String mKey = m.getGroupId() + ":" + m.getArtifactId() + ":" + m.getVersion();
if ( key.equals( mKey ) )
{
// This means a module which is build has a dependency which has the same
// groupId, artifactId, version and classifier coordinates. This is in consequence
// a self reference or in other words a circular reference which can not being resolved.
- addViolation( problems, Severity.FATAL, Version.V31, prefix + " " + key, key, "is referencing itself.", d );
+ addViolation( problems, Severity.FATAL, Version.V31, prefix + "[" + key + "]", key,
+ "is referencing itself.", d );
}
@@ -638,10 +644,10 @@ public class DefaultModelValidator
{
validateEffectiveDependency( problems, d, false, prefix, request );
- validateVersion( prefix + "version", problems, errOn30, Version.BASE, d.getVersion(),
+ validateVersion( prefix, "version", problems, errOn30, Version.BASE, d.getVersion(),
d.getManagementKey(), d );
- validateEnum( prefix + "scope", problems, errOn30, Version.BASE, d.getScope(), d.getManagementKey(), d,
+ validateEnum( prefix, "scope", problems, errOn30, Version.BASE, d.getScope(), d.getManagementKey(), d,
"compile", "runtime", "system" );
}
}
@@ -650,15 +656,15 @@ public class DefaultModelValidator
private void validateEffectiveDependency( ModelProblemCollector problems, Dependency d, boolean management,
String prefix, ModelBuildingRequest request )
{
- validateId( prefix + "artifactId", problems, Severity.ERROR, Version.BASE, d.getArtifactId(),
+ validateId( prefix, "artifactId", problems, Severity.ERROR, Version.BASE, d.getArtifactId(),
d.getManagementKey(), d );
- validateId( prefix + "groupId", problems, Severity.ERROR, Version.BASE, d.getGroupId(), d.getManagementKey(),
- d );
+ validateId( prefix, "groupId", problems, Severity.ERROR, Version.BASE, d.getGroupId(),
+ d.getManagementKey(), d );
if ( !management )
{
- validateStringNotEmpty( prefix + "type", problems, Severity.ERROR, Version.BASE, d.getType(),
+ validateStringNotEmpty( prefix, "type", problems, Severity.ERROR, Version.BASE, d.getType(),
d.getManagementKey(), d );
validateDependencyVersion( problems, d, prefix );
@@ -686,7 +692,7 @@ public class DefaultModelValidator
String msg = "refers to a non-existing file " + sysFile.getAbsolutePath();
systemPath = systemPath.replace( '/', File.separatorChar ).replace( '\\', File.separatorChar );
String jdkHome =
- request.getSystemProperties().getProperty( "java.home", "" ) + File.separator + "..";
+ request.getSystemProperties().getProperty( "java.home", EMPTY ) + File.separator + "..";
if ( systemPath.startsWith( jdkHome ) )
{
msg += ". Please verify that you run Maven using a JDK and not just a JRE.";
@@ -709,18 +715,18 @@ public class DefaultModelValidator
{
if ( request.getValidationLevel() < ModelBuildingRequest.VALIDATION_LEVEL_MAVEN_3_0 )
{
- validateId( prefix + "exclusions.exclusion.groupId", problems, Severity.WARNING, Version.V20,
+ validateId( prefix, "exclusions.exclusion.groupId", problems, Severity.WARNING, Version.V20,
exclusion.getGroupId(), d.getManagementKey(), exclusion );
- validateId( prefix + "exclusions.exclusion.artifactId", problems, Severity.WARNING, Version.V20,
+ validateId( prefix, "exclusions.exclusion.artifactId", problems, Severity.WARNING, Version.V20,
exclusion.getArtifactId(), d.getManagementKey(), exclusion );
}
else
{
- validateIdWithWildcards( prefix + "exclusions.exclusion.groupId", problems, Severity.WARNING,
+ validateIdWithWildcards( prefix, "exclusions.exclusion.groupId", problems, Severity.WARNING,
Version.V30, exclusion.getGroupId(), d.getManagementKey(), exclusion );
- validateIdWithWildcards( prefix + "exclusions.exclusion.artifactId", problems, Severity.WARNING,
+ validateIdWithWildcards( prefix, "exclusions.exclusion.artifactId", problems, Severity.WARNING,
Version.V30, exclusion.getArtifactId(), d.getManagementKey(), exclusion );
}
}
@@ -732,22 +738,22 @@ public class DefaultModelValidator
*/
protected void validateDependencyVersion( ModelProblemCollector problems, Dependency d, String prefix )
{
- validateStringNotEmpty( prefix + "version", problems, Severity.ERROR, Version.BASE, d.getVersion(),
+ validateStringNotEmpty( prefix, "version", problems, Severity.ERROR, Version.BASE, d.getVersion(),
d.getManagementKey(), d );
}
private void validateRawRepositories( ModelProblemCollector problems, List<Repository> repositories, String prefix,
- ModelBuildingRequest request )
+ String prefix2, ModelBuildingRequest request )
{
Map<String, Repository> index = new HashMap<>();
for ( Repository repository : repositories )
{
- validateStringNotEmpty( prefix + ".id", problems, Severity.ERROR, Version.V20, repository.getId(),
- repository );
+ validateStringNotEmpty( prefix, prefix2, "id", problems, Severity.ERROR, Version.V20, repository.getId(),
+ null, repository );
- validateStringNotEmpty( prefix + "[" + repository.getId() + "].url", problems, Severity.ERROR, Version.V20,
- repository.getUrl(), repository );
+ validateStringNotEmpty( prefix, prefix2, "[" + repository.getId() + "].url", problems, Severity.ERROR,
+ Version.V20, repository.getUrl(), null, repository );
String key = repository.getId();
@@ -757,7 +763,7 @@ public class DefaultModelValidator
{
Severity errOn30 = getSeverity( request, ModelBuildingRequest.VALIDATION_LEVEL_MAVEN_3_0 );
- addViolation( problems, errOn30, Version.V20, prefix + ".id", null, "must be unique: "
+ addViolation( problems, errOn30, Version.V20, prefix + prefix2 + "id", null, "must be unique: "
+ repository.getId() + " -> " + existing.getUrl() + " vs " + repository.getUrl(), repository );
}
else
@@ -774,12 +780,12 @@ public class DefaultModelValidator
{
Severity errOn31 = getSeverity( request, ModelBuildingRequest.VALIDATION_LEVEL_MAVEN_3_1 );
- validateBannedCharacters( prefix + ".id", problems, errOn31, Version.V20, repository.getId(), null,
+ validateBannedCharacters( prefix, "id", problems, errOn31, Version.V20, repository.getId(), null,
repository, ILLEGAL_REPO_ID_CHARS );
if ( "local".equals( repository.getId() ) )
{
- addViolation( problems, errOn31, Version.V20, prefix + ".id", null,
+ addViolation( problems, errOn31, Version.V20, prefix + "id", null,
"must not be 'local'" + ", this identifier is reserved for the local repository"
+ ", using it for other repositories will corrupt your repository metadata.",
repository );
@@ -787,7 +793,7 @@ public class DefaultModelValidator
if ( "legacy".equals( repository.getLayout() ) )
{
- addViolation( problems, Severity.WARNING, Version.V20, prefix + ".layout", repository.getId(),
+ addViolation( problems, Severity.WARNING, Version.V20, prefix + "layout", repository.getId(),
"uses the unsupported value 'legacy', artifact resolution might fail.", repository );
}
}
@@ -800,10 +806,10 @@ public class DefaultModelValidator
for ( Resource resource : resources )
{
- validateStringNotEmpty( prefix + ".directory", problems, Severity.ERROR, Version.V20,
- resource.getDirectory(), resource );
+ validateStringNotEmpty( prefix, "directory", problems, Severity.ERROR, Version.V20,
+ resource.getDirectory(), null, resource );
- validateBoolean( prefix + ".filtering", problems, errOn30, Version.V20, resource.getFiltering(),
+ validateBoolean( prefix, "filtering", problems, errOn30, Version.V20, resource.getFiltering(),
resource.getDirectory(), resource );
}
}
@@ -815,13 +821,18 @@ public class DefaultModelValidator
private boolean validateId( String fieldName, ModelProblemCollector problems, String id,
InputLocationTracker tracker )
{
- return validateId( fieldName, problems, Severity.ERROR, Version.BASE, id, null, tracker );
+ return validateId( EMPTY, fieldName, problems, Severity.ERROR, Version.BASE, id, null, tracker );
}
- private boolean validateId( String fieldName, ModelProblemCollector problems, Severity severity, Version version,
- String id, String sourceHint, InputLocationTracker tracker )
+ @SuppressWarnings( "checkstyle:parameternumber" )
+ private boolean validateId( String prefix, String fieldName, ModelProblemCollector problems, Severity severity,
+ Version version, String id, String sourceHint, InputLocationTracker tracker )
{
- if ( !validateStringNotEmpty( fieldName, problems, severity, version, id, sourceHint, tracker ) )
+ if ( validIds.contains( id ) )
+ {
+ return true;
+ }
+ if ( !validateStringNotEmpty( prefix, fieldName, problems, severity, version, id, sourceHint, tracker ) )
{
return false;
}
@@ -829,10 +840,11 @@ public class DefaultModelValidator
{
if ( !isValidId( id ) )
{
- addViolation( problems, severity, version, fieldName, sourceHint,
+ addViolation( problems, severity, version, prefix + fieldName, sourceHint,
"with value '" + id + "' does not match a valid id pattern.", tracker );
return false;
}
+ validIds.add( id );
return true;
}
}
@@ -856,11 +868,12 @@ public class DefaultModelValidator
return c >= 'a' && c <= 'z' || c >= 'A' && c <= 'Z' || c >= '0' && c <= '9' || c == '-' || c == '_' || c == '.';
}
- private boolean validateIdWithWildcards( String fieldName, ModelProblemCollector problems, Severity severity,
- Version version, String id, String sourceHint,
+ @SuppressWarnings( "checkstyle:parameternumber" )
+ private boolean validateIdWithWildcards( String prefix, String fieldName, ModelProblemCollector problems,
+ Severity severity, Version version, String id, String sourceHint,
InputLocationTracker tracker )
{
- if ( !validateStringNotEmpty( fieldName, problems, severity, version, id, sourceHint, tracker ) )
+ if ( !validateStringNotEmpty( prefix, fieldName, problems, severity, version, id, sourceHint, tracker ) )
{
return false;
}
@@ -868,7 +881,7 @@ public class DefaultModelValidator
{
if ( !isValidIdWithWildCards( id ) )
{
- addViolation( problems, severity, version, fieldName, sourceHint,
+ addViolation( problems, severity, version, prefix + fieldName, sourceHint,
"with value '" + id + "' does not match a valid id pattern.", tracker );
return false;
}
@@ -951,7 +964,7 @@ public class DefaultModelValidator
private boolean validateStringNotEmpty( String fieldName, ModelProblemCollector problems, Severity severity,
Version version, String string, InputLocationTracker tracker )
{
- return validateStringNotEmpty( fieldName, problems, severity, version, string, null, tracker );
+ return validateStringNotEmpty( EMPTY, fieldName, problems, severity, version, string, null, tracker );
}
/**
@@ -962,11 +975,40 @@ public class DefaultModelValidator
* <li><code>string.length > 0</code>
* </ul>
*/
- private boolean validateStringNotEmpty( String fieldName, ModelProblemCollector problems, Severity severity,
- Version version, String string, String sourceHint,
+ @SuppressWarnings( "checkstyle:parameternumber" )
+ private boolean validateStringNotEmpty( String prefix, String prefix2, String fieldName,
+ ModelProblemCollector problems, Severity severity, Version version,
+ String string, String sourceHint, InputLocationTracker tracker )
+ {
+ if ( !validateNotNull( prefix, prefix2, fieldName, problems, severity, version, string, sourceHint, tracker ) )
+ {
+ return false;
+ }
+
+ if ( !string.isEmpty() )
+ {
+ return true;
+ }
+
+ addViolation( problems, severity, version, prefix + prefix2 + fieldName, sourceHint, "is missing.", tracker );
+
+ return false;
+ }
+
+ /**
+ * Asserts:
+ * <p/>
+ * <ul>
+ * <li><code>string != null</code>
+ * <li><code>string.length > 0</code>
+ * </ul>
+ */
+ @SuppressWarnings( "checkstyle:parameternumber" )
+ private boolean validateStringNotEmpty( String prefix, String fieldName, ModelProblemCollector problems,
+ Severity severity, Version version, String string, String sourceHint,
InputLocationTracker tracker )
{
- if ( !validateNotNull( fieldName, problems, severity, version, string, sourceHint, tracker ) )
+ if ( !validateNotNull( prefix, fieldName, problems, severity, version, string, sourceHint, tracker ) )
{
return false;
}
@@ -976,7 +1018,7 @@ public class DefaultModelValidator
return true;
}
- addViolation( problems, severity, version, fieldName, sourceHint, "is missing.", tracker );
+ addViolation( problems, severity, version, prefix + fieldName, sourceHint, "is missing.", tracker );
return false;
}
@@ -988,7 +1030,8 @@ public class DefaultModelValidator
* <li><code>string != null</code>
* </ul>
*/
- private boolean validateNotNull( String fieldName, ModelProblemCollector problems, Severity severity,
+ @SuppressWarnings( "checkstyle:parameternumber" )
+ private boolean validateNotNull( String prefix, String fieldName, ModelProblemCollector problems, Severity severity,
Version version, Object object, String sourceHint, InputLocationTracker tracker )
{
if ( object != null )
@@ -996,12 +1039,35 @@ public class DefaultModelValidator
return true;
}
- addViolation( problems, severity, version, fieldName, sourceHint, "is missing.", tracker );
+ addViolation( problems, severity, version, prefix + fieldName, sourceHint, "is missing.", tracker );
return false;
}
- private boolean validateBoolean( String fieldName, ModelProblemCollector problems, Severity severity,
+ /**
+ * Asserts:
+ * <p/>
+ * <ul>
+ * <li><code>string != null</code>
+ * </ul>
+ */
+ @SuppressWarnings( "checkstyle:parameternumber" )
+ private boolean validateNotNull( String prefix, String prefix2, String fieldName,
+ ModelProblemCollector problems, Severity severity, Version version,
+ Object object, String sourceHint, InputLocationTracker tracker )
+ {
+ if ( object != null )
+ {
+ return true;
+ }
+
+ addViolation( problems, severity, version, prefix + prefix2 + fieldName, sourceHint, "is missing.", tracker );
+
+ return false;
+ }
+
+ @SuppressWarnings( "checkstyle:parameternumber" )
+ private boolean validateBoolean( String prefix, String fieldName, ModelProblemCollector problems, Severity severity,
Version version, String string, String sourceHint, InputLocationTracker tracker )
{
if ( string == null || string.length() <= 0 )
@@ -1014,15 +1080,15 @@ public class DefaultModelValidator
return true;
}
- addViolation( problems, severity, version, fieldName, sourceHint,
+ addViolation( problems, severity, version, prefix + fieldName, sourceHint,
"must be 'true' or 'false' but is '" + string + "'.", tracker );
return false;
}
@SuppressWarnings( "checkstyle:parameternumber" )
- private boolean validateEnum( String fieldName, ModelProblemCollector problems, Severity severity, Version version,
- String string, String sourceHint, InputLocationTracker tracker,
+ private boolean validateEnum( String prefix, String fieldName, ModelProblemCollector problems, Severity severity,
+ Version version, String string, String sourceHint, InputLocationTracker tracker,
String... validValues )
{
if ( string == null || string.length() <= 0 )
@@ -1037,7 +1103,7 @@ public class DefaultModelValidator
return true;
}
- addViolation( problems, severity, version, fieldName, sourceHint,
+ addViolation( problems, severity, version, prefix + fieldName, sourceHint,
"must be one of " + values + " but is '" + string + "'.", tracker );
return false;
@@ -1121,8 +1187,8 @@ public class DefaultModelValidator
}
@SuppressWarnings( "checkstyle:parameternumber" )
- private boolean validateBannedCharacters( String fieldName, ModelProblemCollector problems, Severity severity,
- Version version, String string, String sourceHint,
+ private boolean validateBannedCharacters( String prefix, String fieldName, ModelProblemCollector problems,
+ Severity severity, Version version, String string, String sourceHint,
InputLocationTracker tracker, String banned )
{
if ( string != null )
@@ -1131,7 +1197,7 @@ public class DefaultModelValidator
{
if ( banned.indexOf( string.charAt( i ) ) >= 0 )
{
- addViolation( problems, severity, version, fieldName, sourceHint,
+ addViolation( problems, severity, version, prefix + fieldName, sourceHint,
"must not contain any of these characters " + banned + " but found "
+ string.charAt( i ),
tracker );
@@ -1143,7 +1209,8 @@ public class DefaultModelValidator
return true;
}
- private boolean validateVersion( String fieldName, ModelProblemCollector problems, Severity severity,
+ @SuppressWarnings( "checkstyle:parameternumber" )
+ private boolean validateVersion( String prefix, String fieldName, ModelProblemCollector problems, Severity severity,
Version version, String string, String sourceHint, InputLocationTracker tracker )
{
if ( string == null || string.length() <= 0 )
@@ -1153,12 +1220,12 @@ public class DefaultModelValidator
if ( hasExpression( string ) )
{
- addViolation( problems, severity, version, fieldName, sourceHint,
+ addViolation( problems, severity, version, prefix + fieldName, sourceHint,
"must be a valid version but is '" + string + "'.", tracker );
return false;
}
- return validateBannedCharacters( fieldName, problems, severity, version, string, sourceHint, tracker,
+ return validateBannedCharacters( prefix, fieldName, problems, severity, version, string, sourceHint, tracker,
ILLEGAL_VERSION_CHARS );
}
@@ -1194,7 +1261,7 @@ public class DefaultModelValidator
Severity errOn30 = getSeverity( request, ModelBuildingRequest.VALIDATION_LEVEL_MAVEN_3_0 );
- if ( !validateVersion( fieldName, problems, errOn30, Version.V20, string, sourceHint, tracker ) )
+ if ( !validateVersion( EMPTY, fieldName, problems, errOn30, Version.V20, string, sourceHint, tracker ) )
{
return false;
}
@@ -1264,7 +1331,7 @@ public class DefaultModelValidator
if ( location == null )
{
- location = tracker.getLocation( "" );
+ location = tracker.getLocation( EMPTY );
}
}
diff --git a/maven-model-builder/src/test/java/org/apache/maven/model/validation/DefaultModelValidatorTest.java b/maven-model-builder/src/test/java/org/apache/maven/model/validation/DefaultModelValidatorTest.java
index 99ddd30..3e07c57 100644
--- a/maven-model-builder/src/test/java/org/apache/maven/model/validation/DefaultModelValidatorTest.java
+++ b/maven-model-builder/src/test/java/org/apache/maven/model/validation/DefaultModelValidatorTest.java
@@ -295,11 +295,11 @@ public class DefaultModelValidatorTest
assertEquals( "'repositories.repository.id' is missing.", result.getErrors().get( 0 ) );
- assertEquals( "'repositories.repository[null].url' is missing.", result.getErrors().get( 1 ) );
+ assertEquals( "'repositories.repository.[null].url' is missing.", result.getErrors().get( 1 ) );
assertEquals( "'pluginRepositories.pluginRepository.id' is missing.", result.getErrors().get( 2 ) );
- assertEquals( "'pluginRepositories.pluginRepository[null].url' is missing.", result.getErrors().get( 3 ) );
+ assertEquals( "'pluginRepositories.pluginRepository.[null].url' is missing.", result.getErrors().get( 3 ) );
}
public void testMissingResourceDirectory()
@@ -735,7 +735,7 @@ public class DefaultModelValidatorTest
assertViolations( result, 1, 0, 0 );
- assertEquals( "'dependencies.dependency com.example.group:testinvalidpom:0.0.1-SNAPSHOT' for com.example.group:testinvalidpom:0.0.1-SNAPSHOT is referencing itself.",
+ assertEquals( "'dependencies.dependency[com.example.group:testinvalidpom:0.0.1-SNAPSHOT]' for com.example.group:testinvalidpom:0.0.1-SNAPSHOT is referencing itself.",
result.getFatals().get( 0 ) );
}