From 2295c17b45f46cae0daa46105e0a7856505a108f Mon Sep 17 00:00:00 2001 From: Karl Heinz Marbaise Date: Thu, 28 Dec 2017 21:29:46 +0100 Subject: [PATCH] [MNG-6305] Validation of CI friendly version incorrect o Checkin that only the three expression changelist, revision and sha1 are valid in a version. o Added some tests. --- .../AbstractStringBasedModelInterpolator.java | 6 ++ .../validation/DefaultModelValidator.java | 30 ++++--- .../validation/DefaultModelValidatorTest.java | 84 +++++++++++++++---- .../raw-model/bad-ci-friendly-sha1plus.xml | 31 +++++++ .../raw-model/bad-ci-friendly-sha1plus2.xml | 31 +++++++ .../validation/raw-model/bad-ci-friendly.xml | 31 +++++++ .../ok-ci-friendly-all-expressions.xml | 31 +++++++ .../raw-model/ok-ci-friendly-changelist.xml | 31 +++++++ .../raw-model/ok-ci-friendly-revision.xml | 31 +++++++ .../raw-model/ok-ci-friendly-sha1.xml | 31 +++++++ 10 files changed, 311 insertions(+), 26 deletions(-) create mode 100644 maven-model-builder/src/test/resources/poms/validation/raw-model/bad-ci-friendly-sha1plus.xml create mode 100644 maven-model-builder/src/test/resources/poms/validation/raw-model/bad-ci-friendly-sha1plus2.xml create mode 100644 maven-model-builder/src/test/resources/poms/validation/raw-model/bad-ci-friendly.xml create mode 100644 maven-model-builder/src/test/resources/poms/validation/raw-model/ok-ci-friendly-all-expressions.xml create mode 100644 maven-model-builder/src/test/resources/poms/validation/raw-model/ok-ci-friendly-changelist.xml create mode 100644 maven-model-builder/src/test/resources/poms/validation/raw-model/ok-ci-friendly-revision.xml create mode 100644 maven-model-builder/src/test/resources/poms/validation/raw-model/ok-ci-friendly-sha1.xml diff --git a/maven-model-builder/src/main/java/org/apache/maven/model/interpolation/AbstractStringBasedModelInterpolator.java b/maven-model-builder/src/main/java/org/apache/maven/model/interpolation/AbstractStringBasedModelInterpolator.java index b47edbe989..09b53e46eb 100644 --- a/maven-model-builder/src/main/java/org/apache/maven/model/interpolation/AbstractStringBasedModelInterpolator.java +++ b/maven-model-builder/src/main/java/org/apache/maven/model/interpolation/AbstractStringBasedModelInterpolator.java @@ -61,6 +61,12 @@ public abstract class AbstractStringBasedModelInterpolator public static final String CHANGELIST_PROPERTY = "changelist"; public static final String REVISION_PROPERTY = "revision"; + + public static final String SHA1_PROPERTY_EXPRESSION = "${" + SHA1_PROPERTY + "}"; + + public static final String CHANGELIST_PROPERTY_EXPRESSION = "${" + CHANGELIST_PROPERTY + "}"; + + public static final String REVISION_PROPERTY_EXPRESSION = "${" + REVISION_PROPERTY + "}"; private static final List PROJECT_PREFIXES = Arrays.asList( "pom.", "project." ); 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 d97d8f6f6f..9299b4377d 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 @@ -26,6 +26,7 @@ import java.util.List; import java.util.Map; import java.util.Set; +import java.util.regex.Matcher; import java.util.regex.Pattern; import org.apache.maven.model.Activation; @@ -65,6 +66,13 @@ public class DefaultModelValidator implements ModelValidator { + private static final Pattern CI_FRIENDLY_EXPRESSION = Pattern.compile( "\\$\\{(.+?)\\}" ); + + private static final List CI_FRIENDLY_POSSIBLE_PROPERTY_NAMES = + Arrays.asList( AbstractStringBasedModelInterpolator.REVISION_PROPERTY, + AbstractStringBasedModelInterpolator.CHANGELIST_PROPERTY, + AbstractStringBasedModelInterpolator.SHA1_PROPERTY ); + private static final Pattern ID_REGEX = Pattern.compile( "[A-Za-z0-9_\\-.]+" ); private static final Pattern ID_WITH_WILDCARDS_REGEX = Pattern.compile( "[A-Za-z0-9_\\-.?*]+" ); @@ -532,7 +540,7 @@ private void validate20RawDependenciesSelfReferencing( ModelProblemCollector pro ModelBuildingRequest request ) { // We only check for groupId/artifactId cause if there is another - // module with the same groupId/artifactId this will fail the build + // module with the same groupId/artifactId this will fail the build // earlier like "Project '...' is duplicated in the reactor. // So it is sufficient to check only groupId/artifactId and not the // packaging type. @@ -855,7 +863,6 @@ private boolean validateStringNoExpression( String fieldName, ModelProblemCollec private boolean validateVersionNoExpression( String fieldName, ModelProblemCollector problems, Severity severity, Version version, String string, InputLocationTracker tracker ) { - if ( !hasExpression( string ) ) { return true; @@ -868,18 +875,19 @@ private boolean validateVersionNoExpression( String fieldName, ModelProblemColle // revision // sha1 // - string = string.trim(); - if ( string.contains( "${" + AbstractStringBasedModelInterpolator.CHANGELIST_PROPERTY + "}" ) - || string.contains( "${" + AbstractStringBasedModelInterpolator.REVISION_PROPERTY + "}" ) - || string.contains( "${" + AbstractStringBasedModelInterpolator.SHA1_PROPERTY + "}" ) ) + Matcher m = CI_FRIENDLY_EXPRESSION.matcher( string.trim() ); + while ( m.find() ) { - return true; + if ( !CI_FRIENDLY_POSSIBLE_PROPERTY_NAMES.contains( m.group( 1 ) ) ) + { + addViolation( problems, severity, version, fieldName, null, + "contains an expression but should be a constant.", tracker ); + + return false; + } } - addViolation( problems, severity, version, fieldName, null, "contains an expression but should be a constant.", - tracker ); - - return false; + return true; } private boolean hasExpression( String value ) 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 5614daf0df..0bb3bd4fb0 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 @@ -419,18 +419,19 @@ public void testHardCodedSystemPath() assertViolations( result, 0, 0, 1 ); assertContains( result.getWarnings().get( 0 ), - "'dependencies.dependency.systemPath' for test:a:jar should use a variable instead of a hard-coded path" ); + "'dependencies.dependency.systemPath' for test:a:jar should use a variable instead of a hard-coded path" ); - SimpleProblemCollector result_31 = validateRaw( "hard-coded-system-path.xml", ModelBuildingRequest.VALIDATION_LEVEL_MAVEN_3_1 ); + SimpleProblemCollector result_31 = + validateRaw( "hard-coded-system-path.xml", ModelBuildingRequest.VALIDATION_LEVEL_MAVEN_3_1 ); assertViolations( result_31, 0, 0, 3 ); assertContains( result_31.getWarnings().get( 0 ), - "'dependencies.dependency.scope' for test:a:jar declares usage of deprecated 'system' scope" ); + "'dependencies.dependency.scope' for test:a:jar declares usage of deprecated 'system' scope" ); assertContains( result_31.getWarnings().get( 1 ), - "'dependencies.dependency.systemPath' for test:a:jar should use a variable instead of a hard-coded path" ); + "'dependencies.dependency.systemPath' for test:a:jar should use a variable instead of a hard-coded path" ); assertContains( result_31.getWarnings().get( 2 ), - "'dependencies.dependency.scope' for test:b:jar declares usage of deprecated 'system' scope" ); + "'dependencies.dependency.scope' for test:b:jar declares usage of deprecated 'system' scope" ); } @@ -625,22 +626,23 @@ public void testSystemPathRefersToProjectBasedir() assertViolations( result, 0, 0, 2 ); assertContains( result.getWarnings().get( 0 ), - "'dependencies.dependency.systemPath' for test:a:jar should not point at files within the project directory" ); + "'dependencies.dependency.systemPath' for test:a:jar should not point at files within the project directory" ); assertContains( result.getWarnings().get( 1 ), - "'dependencies.dependency.systemPath' for test:b:jar should not point at files within the project directory" ); + "'dependencies.dependency.systemPath' for test:b:jar should not point at files within the project directory" ); - SimpleProblemCollector result_31 = validateRaw( "basedir-system-path.xml", ModelBuildingRequest.VALIDATION_LEVEL_MAVEN_3_1 ); + SimpleProblemCollector result_31 = + validateRaw( "basedir-system-path.xml", ModelBuildingRequest.VALIDATION_LEVEL_MAVEN_3_1 ); assertViolations( result_31, 0, 0, 4 ); assertContains( result_31.getWarnings().get( 0 ), - "'dependencies.dependency.scope' for test:a:jar declares usage of deprecated 'system' scope" ); + "'dependencies.dependency.scope' for test:a:jar declares usage of deprecated 'system' scope" ); assertContains( result_31.getWarnings().get( 1 ), - "'dependencies.dependency.systemPath' for test:a:jar should not point at files within the project directory" ); + "'dependencies.dependency.systemPath' for test:a:jar should not point at files within the project directory" ); assertContains( result_31.getWarnings().get( 2 ), - "'dependencies.dependency.scope' for test:b:jar declares usage of deprecated 'system' scope" ); + "'dependencies.dependency.scope' for test:b:jar declares usage of deprecated 'system' scope" ); assertContains( result_31.getWarnings().get( 3 ), - "'dependencies.dependency.systemPath' for test:b:jar should not point at files within the project directory" ); + "'dependencies.dependency.systemPath' for test:b:jar should not point at files within the project directory" ); } public void testInvalidVersionInPluginManagement() @@ -703,16 +705,16 @@ public void testMissingReportPluginVersion() } public void testDeprecatedDependencyMetaversionsLatestAndRelease() - throws Exception + throws Exception { SimpleProblemCollector result = validateRaw( "deprecated-dependency-metaversions-latest-and-release.xml" ); assertViolations( result, 0, 0, 2 ); assertContains( result.getWarnings().get( 0 ), - "'dependencies.dependency.version' for test:a:jar is either LATEST or RELEASE (both of them are being deprecated)" ); + "'dependencies.dependency.version' for test:a:jar is either LATEST or RELEASE (both of them are being deprecated)" ); assertContains( result.getWarnings().get( 1 ), - "'dependencies.dependency.version' for test:b:jar is either LATEST or RELEASE (both of them are being deprecated)" ); + "'dependencies.dependency.version' for test:b:jar is either LATEST or RELEASE (both of them are being deprecated)" ); } public void testSelfReferencingDependencyInRawModel() @@ -727,4 +729,56 @@ public void testSelfReferencingDependencyInRawModel() } + public void testCiFriendlySha1() + throws Exception + { + SimpleProblemCollector result = validateRaw( "raw-model/ok-ci-friendly-sha1.xml" ); + assertViolations( result, 0, 0, 0 ); + } + + public void testCiFriendlyRevision() + throws Exception + { + SimpleProblemCollector result = validateRaw( "raw-model/ok-ci-friendly-revision.xml" ); + assertViolations( result, 0, 0, 0 ); + } + + public void testCiFriendlyChangeList() + throws Exception + { + SimpleProblemCollector result = validateRaw( "raw-model/ok-ci-friendly-changelist.xml" ); + assertViolations( result, 0, 0, 0 ); + } + + public void testCiFriendlyAllExpressions() + throws Exception + { + SimpleProblemCollector result = validateRaw( "raw-model/ok-ci-friendly-all-expressions.xml" ); + assertViolations( result, 0, 0, 0 ); + } + + public void testCiFriendlyBad() + throws Exception + { + SimpleProblemCollector result = validateRaw( "raw-model/bad-ci-friendly.xml" ); + assertViolations( result, 0, 0, 1 ); + assertEquals( "'version' contains an expression but should be a constant.", result.getWarnings().get( 0 ) ); + } + + public void testCiFriendlyBadSha1Plus() + throws Exception + { + SimpleProblemCollector result = validateRaw( "raw-model/bad-ci-friendly-sha1plus.xml" ); + assertViolations( result, 0, 0, 1 ); + assertEquals( "'version' contains an expression but should be a constant.", result.getWarnings().get( 0 ) ); + } + + public void testCiFriendlyBadSha1Plus2() + throws Exception + { + SimpleProblemCollector result = validateRaw( "raw-model/bad-ci-friendly-sha1plus2.xml" ); + assertViolations( result, 0, 0, 1 ); + assertEquals( "'version' contains an expression but should be a constant.", result.getWarnings().get( 0 ) ); + } + } diff --git a/maven-model-builder/src/test/resources/poms/validation/raw-model/bad-ci-friendly-sha1plus.xml b/maven-model-builder/src/test/resources/poms/validation/raw-model/bad-ci-friendly-sha1plus.xml new file mode 100644 index 0000000000..35642d8bac --- /dev/null +++ b/maven-model-builder/src/test/resources/poms/validation/raw-model/bad-ci-friendly-sha1plus.xml @@ -0,0 +1,31 @@ + + + + 4.0.0 + com.example.group + valid-version-sha1plus + ${sha1}${wrong} + + + This will test if the validation for the ci friendly versions + is working correct. + + \ No newline at end of file diff --git a/maven-model-builder/src/test/resources/poms/validation/raw-model/bad-ci-friendly-sha1plus2.xml b/maven-model-builder/src/test/resources/poms/validation/raw-model/bad-ci-friendly-sha1plus2.xml new file mode 100644 index 0000000000..7f9ab2c171 --- /dev/null +++ b/maven-model-builder/src/test/resources/poms/validation/raw-model/bad-ci-friendly-sha1plus2.xml @@ -0,0 +1,31 @@ + + + + 4.0.0 + com.example.group + valid-version-sha1plus + ${sha1}${wrong}${revision} + + + This will test if the validation for the ci friendly versions + is working correct. + + \ No newline at end of file diff --git a/maven-model-builder/src/test/resources/poms/validation/raw-model/bad-ci-friendly.xml b/maven-model-builder/src/test/resources/poms/validation/raw-model/bad-ci-friendly.xml new file mode 100644 index 0000000000..9288b359b1 --- /dev/null +++ b/maven-model-builder/src/test/resources/poms/validation/raw-model/bad-ci-friendly.xml @@ -0,0 +1,31 @@ + + + + 4.0.0 + com.example.group + valid-version-wrong + ${wrong} + + + This will test if the validation for the ci friendly versions + is working correct. + + \ No newline at end of file diff --git a/maven-model-builder/src/test/resources/poms/validation/raw-model/ok-ci-friendly-all-expressions.xml b/maven-model-builder/src/test/resources/poms/validation/raw-model/ok-ci-friendly-all-expressions.xml new file mode 100644 index 0000000000..860b482774 --- /dev/null +++ b/maven-model-builder/src/test/resources/poms/validation/raw-model/ok-ci-friendly-all-expressions.xml @@ -0,0 +1,31 @@ + + + + 4.0.0 + com.example.group + valid-version-sha1 + ${revision}${changelist}${sha1} + + + This will test if the validation for the ci friendly versions + is working correct. + + \ No newline at end of file diff --git a/maven-model-builder/src/test/resources/poms/validation/raw-model/ok-ci-friendly-changelist.xml b/maven-model-builder/src/test/resources/poms/validation/raw-model/ok-ci-friendly-changelist.xml new file mode 100644 index 0000000000..f4a1da7f89 --- /dev/null +++ b/maven-model-builder/src/test/resources/poms/validation/raw-model/ok-ci-friendly-changelist.xml @@ -0,0 +1,31 @@ + + + + 4.0.0 + com.example.group + valid-version-changelist + ${changelist} + + + This will test if the validation for the ci friendly versions + is working correct. + + \ No newline at end of file diff --git a/maven-model-builder/src/test/resources/poms/validation/raw-model/ok-ci-friendly-revision.xml b/maven-model-builder/src/test/resources/poms/validation/raw-model/ok-ci-friendly-revision.xml new file mode 100644 index 0000000000..565cd7ba32 --- /dev/null +++ b/maven-model-builder/src/test/resources/poms/validation/raw-model/ok-ci-friendly-revision.xml @@ -0,0 +1,31 @@ + + + + 4.0.0 + com.example.group + valid-version-revision + ${revision} + + + This will test if the validation for the ci friendly versions + is working correct. + + \ No newline at end of file diff --git a/maven-model-builder/src/test/resources/poms/validation/raw-model/ok-ci-friendly-sha1.xml b/maven-model-builder/src/test/resources/poms/validation/raw-model/ok-ci-friendly-sha1.xml new file mode 100644 index 0000000000..5287c99fab --- /dev/null +++ b/maven-model-builder/src/test/resources/poms/validation/raw-model/ok-ci-friendly-sha1.xml @@ -0,0 +1,31 @@ + + + + 4.0.0 + com.example.group + valid-version-sha1 + ${sha1} + + + This will test if the validation for the ci friendly versions + is working correct. This c + + \ No newline at end of file