This is an automated email from the ASF dual-hosted git repository. rfscholte pushed a commit to branch MNG-6656 in repository https://gitbox.apache.org/repos/asf/maven.git
commit d9cc0e475952bc88f1187e00a0f0ff142f4c6f2a Author: rfscholte <[email protected]> AuthorDate: Wed Dec 18 23:00:34 2019 +0100 [MNG-6656] Improve ModelValidator --- .../apache/maven/project/ProjectBuilderTest.java | 2 ++ .../maven/model/building/DefaultModelBuilder.java | 4 +++ .../building/DefaultModelBuildingRequest.java | 14 ++++++++++ .../model/building/FilterModelBuildingRequest.java | 13 +++++++++ .../maven/model/building/ModelBuildingRequest.java | 15 +++++++++++ .../model/validation/DefaultModelValidator.java | 31 ++++++++++++++++------ .../maven/model/validation/ModelValidator.java | 16 +++++++++-- .../validation/DefaultModelValidatorTest.java | 31 +++++++++++----------- 8 files changed, 101 insertions(+), 25 deletions(-) diff --git a/maven-core/src/test/java/org/apache/maven/project/ProjectBuilderTest.java b/maven-core/src/test/java/org/apache/maven/project/ProjectBuilderTest.java index efa8a4c..d402f96 100644 --- a/maven-core/src/test/java/org/apache/maven/project/ProjectBuilderTest.java +++ b/maven-core/src/test/java/org/apache/maven/project/ProjectBuilderTest.java @@ -240,6 +240,7 @@ public class ProjectBuilderTest try { projectBuilder.build( pomFile, configuration ); + fail(); } catch ( InvalidArtifactRTException iarte ) { @@ -250,6 +251,7 @@ public class ProjectBuilderTest try { projectBuilder.build( Collections.singletonList( pomFile ), false, configuration ); + fail(); } catch ( ProjectBuildingException ex ) { diff --git a/maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelBuilder.java b/maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelBuilder.java index 8a46aed..5bfe617 100644 --- a/maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelBuilder.java +++ b/maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelBuilder.java @@ -660,6 +660,10 @@ public class DefaultModelBuilder model.setPomFile( pomFile ); problems.setSource( model ); + + modelValidator.validateFileModel( model, request, problems ); + request.setFileModel( model ); + modelValidator.validateRawModel( model, request, problems ); if ( hasFatalErrors( problems ) ) diff --git a/maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelBuildingRequest.java b/maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelBuildingRequest.java index b7b54d8..3cf3a89 100644 --- a/maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelBuildingRequest.java +++ b/maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelBuildingRequest.java @@ -38,6 +38,7 @@ import org.apache.maven.model.resolution.WorkspaceModelResolver; public class DefaultModelBuildingRequest implements ModelBuildingRequest { + private transient Model fileModel; private Model rawModel; @@ -383,6 +384,19 @@ public class DefaultModelBuildingRequest } @Override + public Model getFileModel() + { + return fileModel; + } + + @Override + public ModelBuildingRequest setFileModel( Model fileModel ) + { + this.fileModel = fileModel; + return this; + } + + @Override public Model getRawModel() { return rawModel; diff --git a/maven-model-builder/src/main/java/org/apache/maven/model/building/FilterModelBuildingRequest.java b/maven-model-builder/src/main/java/org/apache/maven/model/building/FilterModelBuildingRequest.java index 527a257..fb38f16 100644 --- a/maven-model-builder/src/main/java/org/apache/maven/model/building/FilterModelBuildingRequest.java +++ b/maven-model-builder/src/main/java/org/apache/maven/model/building/FilterModelBuildingRequest.java @@ -257,6 +257,19 @@ class FilterModelBuildingRequest } @Override + public Model getFileModel() + { + return request.getFileModel(); + } + + @Override + public ModelBuildingRequest setFileModel( Model fileModel ) + { + request.setFileModel( fileModel ); + return this; + } + + @Override public Model getRawModel() { return request.getRawModel(); diff --git a/maven-model-builder/src/main/java/org/apache/maven/model/building/ModelBuildingRequest.java b/maven-model-builder/src/main/java/org/apache/maven/model/building/ModelBuildingRequest.java index b8ae4f8..1e0b3da 100644 --- a/maven-model-builder/src/main/java/org/apache/maven/model/building/ModelBuildingRequest.java +++ b/maven-model-builder/src/main/java/org/apache/maven/model/building/ModelBuildingRequest.java @@ -63,6 +63,21 @@ public interface ModelBuildingRequest * Denotes strict validation as recommended by the current Maven version. */ int VALIDATION_LEVEL_STRICT = VALIDATION_LEVEL_MAVEN_3_0; + + /** + * + * @return the file model + * @since 3.7.0 + */ + Model getFileModel(); + + /** + * + * @param fileModel + * @return This request, never {@code null}. + * @since 3.7.0 + */ + ModelBuildingRequest setFileModel( Model fileModel ); /** * Gets the raw model to build. If not set, model source will be used to load raw model. 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 86670eb..862c8ec 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 @@ -88,7 +88,7 @@ public class DefaultModelValidator private final Set<String> validIds = new HashSet<>(); @Override - public void validateRawModel( Model m, ModelBuildingRequest request, ModelProblemCollector problems ) + public void validateFileModel( Model m, ModelBuildingRequest request, ModelProblemCollector problems ) { Parent parent = m.getParent(); if ( parent != null ) @@ -99,13 +99,6 @@ public class DefaultModelValidator validateStringNotEmpty( "parent.artifactId", problems, Severity.FATAL, Version.BASE, parent.getArtifactId(), parent ); - // resolvedModel will assign version based on relativePath - if ( !Features.buildConsumer().isActive() ) - { - validateStringNotEmpty( "parent.version", problems, Severity.FATAL, Version.BASE, parent.getVersion(), - parent ); - } - if ( equals( parent.getGroupId(), m.getGroupId() ) && equals( parent.getArtifactId(), m.getArtifactId() ) ) { addViolation( problems, Severity.FATAL, Version.BASE, "parent.artifactId", null, @@ -225,6 +218,28 @@ public class DefaultModelValidator } } } + + @Override + public void validateRawModel( Model m, ModelBuildingRequest request, ModelProblemCollector problems ) + { + Parent parent = m.getParent(); + + if ( parent != null ) + { + InputLocationTracker locationTracker; + if ( Features.buildConsumer().isActive() ) + { + locationTracker = request.getFileModel().getParent(); + } + else + { + locationTracker = parent; + } + + validateStringNotEmpty( "parent.version", problems, Severity.FATAL, Version.BASE, parent.getVersion(), + locationTracker ); + } + } private void validate30RawProfileActivation( ModelProblemCollector problems, Activation activation, String sourceHint, String prefix, String fieldName, diff --git a/maven-model-builder/src/main/java/org/apache/maven/model/validation/ModelValidator.java b/maven-model-builder/src/main/java/org/apache/maven/model/validation/ModelValidator.java index 84e3fad..198ba5a 100644 --- a/maven-model-builder/src/main/java/org/apache/maven/model/validation/ModelValidator.java +++ b/maven-model-builder/src/main/java/org/apache/maven/model/validation/ModelValidator.java @@ -30,15 +30,27 @@ import org.apache.maven.model.building.ModelProblemCollector; */ public interface ModelValidator { - /** - * Checks the specified (raw) model for missing or invalid values. The raw model is directly created from the POM + * Checks the specified file model for missing or invalid values. This model is directly created from the POM * file and has not been subjected to inheritance, interpolation or profile/default injection. * * @param model The model to validate, must not be {@code null}. * @param request The model building request that holds further settings, must not be {@code null}. * @param problems The container used to collect problems that were encountered, must not be {@code null}. */ + default void validateFileModel( Model model, ModelBuildingRequest request, ModelProblemCollector problems ) + { + // do nothing + } + + /** + * Checks the specified (raw) model for missing or invalid values. The raw model is the file model + buildpom filter + * transformation and has not been subjected to inheritance, interpolation or profile/default injection. + * + * @param model The model to validate, must not be {@code null}. + * @param request The model building request that holds further settings, must not be {@code null}. + * @param problems The container used to collect problems that were encountered, must not be {@code null}. + */ void validateRawModel( Model model, ModelBuildingRequest request, ModelProblemCollector problems ); /** 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 bf6b088..b90fcc6 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 @@ -22,7 +22,6 @@ package org.apache.maven.model.validation; import java.io.InputStream; import java.util.List; -import org.apache.maven.feature.Features; import org.apache.maven.model.Model; import org.apache.maven.model.building.DefaultModelBuildingRequest; import org.apache.maven.model.building.ModelBuildingRequest; @@ -65,10 +64,14 @@ public class DefaultModelValidatorTest throws Exception { ModelBuildingRequest request = new DefaultModelBuildingRequest().setValidationLevel( level ); + + Model model = read( pom ); - SimpleProblemCollector problems = new SimpleProblemCollector( read( pom ) ); + SimpleProblemCollector problems = new SimpleProblemCollector( model ); + + request.setFileModel( model ); - validator.validateEffectiveModel( problems.getModel(), request, problems ); + validator.validateEffectiveModel( model, request, problems ); return problems; } @@ -78,9 +81,15 @@ public class DefaultModelValidatorTest { ModelBuildingRequest request = new DefaultModelBuildingRequest().setValidationLevel( level ); - SimpleProblemCollector problems = new SimpleProblemCollector( read( pom ) ); + Model model = read( pom ); + + SimpleProblemCollector problems = new SimpleProblemCollector( model ); - validator.validateRawModel( problems.getModel(), request, problems ); + validator.validateFileModel( model, request, problems ); + + request.setFileModel( model ); + + validator.validateRawModel( model, request, problems ); return problems; } @@ -416,18 +425,10 @@ public class DefaultModelValidatorTest { SimpleProblemCollector result = validateRaw( "incomplete-parent.xml" ); - if ( Features.buildConsumer().isActive() ) - { - assertViolations( result, 2, 0, 0 ); - } - else - { - assertViolations( result, 3, 0, 0 ); - assertTrue( result.getFatals().get( 2 ).contains( "parent.version" ) ); - } - + assertViolations( result, 3, 0, 0 ); assertTrue( result.getFatals().get( 0 ).contains( "parent.groupId" ) ); assertTrue( result.getFatals().get( 1 ).contains( "parent.artifactId" ) ); + assertTrue( result.getFatals().get( 2 ).contains( "parent.version" ) ); } public void testHardCodedSystemPath()
