From commits-return-70546-archive-asf-public=cust-asf.ponee.io@maven.apache.org Sat Feb 17 21:30:32 2018 Return-Path: X-Original-To: archive-asf-public@cust-asf.ponee.io Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by mx-eu-01.ponee.io (Postfix) with SMTP id 4F7D918072F for ; Sat, 17 Feb 2018 21:30:30 +0100 (CET) Received: (qmail 52655 invoked by uid 500); 17 Feb 2018 20:30:25 -0000 Mailing-List: contact commits-help@maven.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@maven.apache.org Delivered-To: mailing list commits@maven.apache.org Received: (qmail 52135 invoked by uid 99); 17 Feb 2018 20:30:24 -0000 Received: from git1-us-west.apache.org (HELO git1-us-west.apache.org) (140.211.11.23) by apache.org (qpsmtpd/0.29) with ESMTP; Sat, 17 Feb 2018 20:30:24 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id B0DB1F323D; Sat, 17 Feb 2018 20:30:22 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: khmarbaise@apache.org To: commits@maven.apache.org Date: Sat, 17 Feb 2018 20:30:43 -0000 Message-Id: <8aea635cb4d24146a706bbef564a787d@git.apache.org> In-Reply-To: References: X-Mailer: ASF-Git Admin Mailer Subject: [22/50] [abbrv] maven git commit: [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. [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. Project: http://git-wip-us.apache.org/repos/asf/maven/repo Commit: http://git-wip-us.apache.org/repos/asf/maven/commit/2295c17b Tree: http://git-wip-us.apache.org/repos/asf/maven/tree/2295c17b Diff: http://git-wip-us.apache.org/repos/asf/maven/diff/2295c17b Branch: refs/heads/MNG-5227 Commit: 2295c17b45f46cae0daa46105e0a7856505a108f Parents: df5169b Author: Karl Heinz Marbaise Authored: Thu Dec 28 21:29:46 2017 +0100 Committer: Karl Heinz Marbaise Committed: Sat Dec 30 21:41:18 2017 +0100 ---------------------------------------------------------------------- .../AbstractStringBasedModelInterpolator.java | 6 ++ .../model/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(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/maven/blob/2295c17b/maven-model-builder/src/main/java/org/apache/maven/model/interpolation/AbstractStringBasedModelInterpolator.java ---------------------------------------------------------------------- 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 b47edbe..09b53e4 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." ); http://git-wip-us.apache.org/repos/asf/maven/blob/2295c17b/maven-model-builder/src/main/java/org/apache/maven/model/validation/DefaultModelValidator.java ---------------------------------------------------------------------- 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 d97d8f6..9299b43 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.HashSet; 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 @@ public class DefaultModelValidator 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 @@ public class DefaultModelValidator private boolean validateVersionNoExpression( String fieldName, ModelProblemCollector problems, Severity severity, Version version, String string, InputLocationTracker tracker ) { - if ( !hasExpression( string ) ) { return true; @@ -868,18 +875,19 @@ public class DefaultModelValidator // 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 ); - addViolation( problems, severity, version, fieldName, null, "contains an expression but should be a constant.", - tracker ); + return false; + } + } - return false; + return true; } private boolean hasExpression( String value ) http://git-wip-us.apache.org/repos/asf/maven/blob/2295c17b/maven-model-builder/src/test/java/org/apache/maven/model/validation/DefaultModelValidatorTest.java ---------------------------------------------------------------------- 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 5614daf..0bb3bd4 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 class DefaultModelValidatorTest 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 class DefaultModelValidatorTest 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 class DefaultModelValidatorTest } 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 class DefaultModelValidatorTest } + 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 ) ); + } + } http://git-wip-us.apache.org/repos/asf/maven/blob/2295c17b/maven-model-builder/src/test/resources/poms/validation/raw-model/bad-ci-friendly-sha1plus.xml ---------------------------------------------------------------------- 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 0000000..35642d8 --- /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 http://git-wip-us.apache.org/repos/asf/maven/blob/2295c17b/maven-model-builder/src/test/resources/poms/validation/raw-model/bad-ci-friendly-sha1plus2.xml ---------------------------------------------------------------------- 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 0000000..7f9ab2c --- /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 http://git-wip-us.apache.org/repos/asf/maven/blob/2295c17b/maven-model-builder/src/test/resources/poms/validation/raw-model/bad-ci-friendly.xml ---------------------------------------------------------------------- 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 0000000..9288b35 --- /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 http://git-wip-us.apache.org/repos/asf/maven/blob/2295c17b/maven-model-builder/src/test/resources/poms/validation/raw-model/ok-ci-friendly-all-expressions.xml ---------------------------------------------------------------------- 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 0000000..860b482 --- /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 http://git-wip-us.apache.org/repos/asf/maven/blob/2295c17b/maven-model-builder/src/test/resources/poms/validation/raw-model/ok-ci-friendly-changelist.xml ---------------------------------------------------------------------- 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 0000000..f4a1da7 --- /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 http://git-wip-us.apache.org/repos/asf/maven/blob/2295c17b/maven-model-builder/src/test/resources/poms/validation/raw-model/ok-ci-friendly-revision.xml ---------------------------------------------------------------------- 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 0000000..565cd7b --- /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 http://git-wip-us.apache.org/repos/asf/maven/blob/2295c17b/maven-model-builder/src/test/resources/poms/validation/raw-model/ok-ci-friendly-sha1.xml ---------------------------------------------------------------------- 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 0000000..5287c99 --- /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